Merge pull request #1073 from solid/feature/prevent-api-spoofing

Restricting access to some HTTP API endpoints to top domain

Author: kjetilk

lib/api/accounts/user-accounts.js

@@ -4,6 +4,8 @@ const express = require('express')
 const bodyParser = require('body-parser').urlencoded({ extended: false })
 const debug = require('../../debug').accounts
 
+const restrictToTopDomain = require('../../handlers/restrict-to-top-domain')
+
 const CreateAccountRequest = require('../../requests/create-account-request')
 const AddCertificateRequest = require('../../requests/add-cert-request')
 const DeleteAccountRequest = require('../../requests/delete-account-request')
@@ -65,16 +67,16 @@ function middleware (accountManager) {
 
   router.get('/', checkAccountExists(accountManager))
 
-  router.post('/api/accounts/new', bodyParser, CreateAccountRequest.post)
-  router.get(['/register', '/api/accounts/new'], CreateAccountRequest.get)
+  router.post('/api/accounts/new', restrictToTopDomain, bodyParser, CreateAccountRequest.post)
+  router.get(['/register', '/api/accounts/new'], restrictToTopDomain, CreateAccountRequest.get)
 
-  router.post('/api/accounts/cert', bodyParser, newCertificate(accountManager))
+  router.post('/api/accounts/cert', restrictToTopDomain, bodyParser, newCertificate(accountManager))
 
-  router.get('/account/delete', DeleteAccountRequest.get)
-  router.post('/account/delete', bodyParser, DeleteAccountRequest.post)
+  router.get('/account/delete', restrictToTopDomain, DeleteAccountRequest.get)
+  router.post('/account/delete', restrictToTopDomain, bodyParser, DeleteAccountRequest.post)
 
-  router.get('/account/delete/confirm', DeleteAccountConfirmRequest.get)
-  router.post('/account/delete/confirm', bodyParser, DeleteAccountConfirmRequest.post)
+  router.get('/account/delete/confirm', restrictToTopDomain, DeleteAccountConfirmRequest.get)
+  router.post('/account/delete/confirm', restrictToTopDomain, bodyParser, DeleteAccountConfirmRequest.post)
 
   return router
 }

lib/api/authn/webid-oidc.js

@@ -9,6 +9,8 @@ const bodyParser = require('body-parser').urlencoded({ extended: false })
 const OidcManager = require('../../models/oidc-manager')
 const { LoginRequest } = require('../../requests/login-request')
 
+const restrictToTopDomain = require('../../handlers/restrict-to-top-domain')
+
 const PasswordResetEmailRequest = require('../../requests/password-reset-email-request')
 const PasswordChangeRequest = require('../../requests/password-change-request')
 
@@ -71,11 +73,11 @@ function middleware (oidc) {
 
   router.post('/login/tls', bodyParser, LoginRequest.loginTls)
 
-  router.get('/account/password/reset', PasswordResetEmailRequest.get)
-  router.post('/account/password/reset', bodyParser, PasswordResetEmailRequest.post)
+  router.get('/account/password/reset', restrictToTopDomain, PasswordResetEmailRequest.get)
+  router.post('/account/password/reset', restrictToTopDomain, bodyParser, PasswordResetEmailRequest.post)
 
-  router.get('/account/password/change', PasswordChangeRequest.get)
-  router.post('/account/password/change', bodyParser, PasswordChangeRequest.post)
+  router.get('/account/password/change', restrictToTopDomain, PasswordChangeRequest.get)
+  router.post('/account/password/change', restrictToTopDomain, bodyParser, PasswordChangeRequest.post)
 
   router.get('/.well-known/solid/logout/', (req, res) => res.redirect('/logout'))
 

lib/capability-discovery.js

@@ -3,22 +3,7 @@
  * @module capability-discovery
  */
 const express = require('express')
-
-const serviceConfigDefaults = {
-  'api': {
-    'accounts': {
-      // 'changePassword': '/api/account/changePassword',
-      // 'delete': '/api/accounts/delete',
-
-      // Create new user (see IdentityProvider.post() in identity-provider.js)
-      'new': '/api/accounts/new',
-      'recover': '/api/accounts/recover',
-      'signin': '/login',
-      'signout': '/logout',
-      'validateToken': '/api/accounts/validateToken'
-    }
-  }
-}
+const { URL } = require('url')
 
 module.exports = capabilityDiscovery
 
@@ -31,7 +16,7 @@ function capabilityDiscovery () {
   const router = express.Router('/')
 
   // Advertise the server capability discover endpoint
-  router.get('/.well-known/solid', serviceCapabilityDocument(serviceConfigDefaults))
+  router.get('/.well-known/solid', serviceCapabilityDocument())
   return router
 }
 
@@ -43,12 +28,27 @@ function capabilityDiscovery () {
  * @param res
  * @param next
  */
-function serviceCapabilityDocument (serviceConfig) {
+function serviceCapabilityDocument () {
   return (req, res) => {
-    // Add the server root url
-    serviceConfig.root = req.app.locals.ldp.resourceMapper.resolveUrl(req.hostname, req.path)
-    // Add the 'apps' urls section
-    serviceConfig.apps = req.app.locals.appUrls
-    res.json(serviceConfig)
+    const ldp = req.app.locals.ldp
+    res.json({
+      // Add the server root url
+      root: req.app.locals.ldp.resourceMapper.resolveUrl(req.hostname, req.path),
+      // Add the 'apps' urls section
+      apps: req.app.locals.appUrls,
+      api: {
+        accounts: {
+          // 'changePassword': '/api/account/changePassword',
+          // 'delete': '/api/accounts/delete',
+
+          // Create new user (see IdentityProvider.post() in identity-provider.js)
+          new: new URL('/api/accounts/new', ldp.serverUri),
+          recover: new URL('/api/accounts/recover', ldp.serverUri),
+          signin: req.app.locals.ldp.resourceMapper.resolveUrl(req.hostname, '/login'),
+          signout: req.app.locals.ldp.resourceMapper.resolveUrl(req.hostname, '/logout'),
+          validateToken: new URL('/api/accounts/validateToken', ldp.serverUri)
+        }
+      }
+    })
   }
 }

lib/handlers/restrict-to-top-domain.js

@@ -0,0 +1,13 @@
+const HTTPError = require('../http-error')
+
+module.exports = function (req, res, next) {
+  const locals = req.app.locals
+  const ldp = locals.ldp
+  const serverUri = locals.host.serverUri
+  const hostname = ldp.resourceMapper.resolveUrl(req.hostname)
+  if (hostname === serverUri) {
+    return next()
+  }
+  const isLoggedIn = !!(req.session && req.session.userId)
+  return next(new HTTPError(isLoggedIn ? 403 : 401, 'Not allowed to access top-level APIs on accounts'))
+}

test/integration/account-creation-oidc-test.js

@@ -80,21 +80,21 @@ describe('AccountManager (OIDC account creation tests)', function () {
     })
 
     it('should not create WebID if no username is given', (done) => {
-      let subdomain = supertest('https://nicola.' + host)
+      let subdomain = supertest('https://' + host)
       subdomain.post('/api/accounts/new')
         .send('username=&password=12345')
         .expect(400, done)
     })
 
     it('should not create WebID if no password is given', (done) => {
-      let subdomain = supertest('https://nicola.' + host)
+      let subdomain = supertest('https://' + host)
       subdomain.post('/api/accounts/new')
         .send('username=nicola&password=')
         .expect(400, done)
     })
 
     it('should not create a WebID if it already exists', function (done) {
-      var subdomain = supertest('https://nicola.' + host)
+      var subdomain = supertest('https://' + host)
       subdomain.post('/api/accounts/new')
         .send('username=nicola&password=12345&acceptToc=true')
         .expect(302)
@@ -112,14 +112,14 @@ describe('AccountManager (OIDC account creation tests)', function () {
     })
 
     it('should not create WebID if T&C is not accepted', (done) => {
-      let subdomain = supertest('https://nicola.' + host)
+      let subdomain = supertest('https://' + host)
       subdomain.post('/api/accounts/new')
         .send('username=nicola&password=12345&acceptToc=')
         .expect(400, done)
     })
 
     it('should create the default folders', function (done) {
-      var subdomain = supertest('https://nicola.' + host)
+      var subdomain = supertest('https://' + host)
       subdomain.post('/api/accounts/new')
         .send('username=nicola&password=12345&acceptToc=true')
         .expect(302)
@@ -150,14 +150,15 @@ describe('AccountManager (OIDC account creation tests)', function () {
     }).timeout(20000)
 
     it('should link WebID to the root account', function (done) {
-      var subdomain = supertest('https://nicola.' + host)
-      subdomain.post('/api/accounts/new')
+      const domain = supertest('https://' + host)
+      domain.post('/api/accounts/new')
         .send('username=nicola&password=12345&acceptToc=true')
         .expect(302)
         .end(function (err) {
           if (err) {
             return done(err)
           }
+          const subdomain = supertest('https://nicola.' + host)
           subdomain.get('/.meta')
             .expect(200)
             .end(function (err, data) {
@@ -185,7 +186,7 @@ describe('AccountManager (OIDC account creation tests)', function () {
 
     describe('after setting up account', () => {
       beforeEach(done => {
-        var subdomain = supertest('https://nicola.' + host)
+        var subdomain = supertest('https://' + host)
         subdomain.post('/api/accounts/new')
           .send('username=nicola&password=12345&acceptToc=true')
           .end(done)
@@ -294,7 +295,7 @@ describe('Signup page where Terms & Conditions are not being enforced', () => {
   })
 
   it('should not enforce T&C upon creating account', function (done) {
-    var subdomain = supertest('https://nicola.' + host)
+    var subdomain = supertest('https://' + host)
     subdomain.post('/api/accounts/new')
       .send('username=nicola&password=12345')
       .expect(302, done)