Making proper use of strictOrigins

We had some faulty handling where we returned 403 even if the user wasn't authenticated. (This fixes #1117.)

1. I discovered some redundant tests in test/integration/authentication-oidc-test.js, and moved some tests into groups for better overview. This makes for a lot of changes, but I think it's worth it.
2. I duplicated authentication-oidc-test into authentication-oidc-with-strict-origins-turned-off-test to make sure we handle those permutations correctly as well. This required a lot of setup, which are all of the new files in this commit.
3. I tried to consolidate the use of getTrustedOrigins in create-app.js, which made a test obsolete

Author: megoth

lib/acl-checker.js

@@ -56,17 +56,14 @@ class ACLChecker {
     const aclFile = rdf.sym(acl.acl)
     const agent = user ? rdf.sym(user) : null
     const modes = [ACL(mode)]
-    const agentOrigin = this.agentOrigin ? rdf.sym(this.agentOrigin) : null
-    const trustedOrigins = this.trustedOrigins ? this.trustedOrigins.map(trustedOrigin => rdf.sym(trustedOrigin)) : null
+    const agentOrigin = this.strictOrigin && this.agentOrigin ? rdf.sym(this.agentOrigin) : null
+    const trustedOrigins = this.strictOrigin && this.trustedOrigins ? this.trustedOrigins.map(trustedOrigin => rdf.sym(trustedOrigin)) : null
     const accessDenied = aclCheck.accessDenied(acl.graph, resource, directory, aclFile, agent, modes, agentOrigin, trustedOrigins)
-    if (accessDenied && this.agentOrigin && this.resourceUrl.origin !== this.agentOrigin) {
-      this.messagesCached[cacheKey].push(HTTPError(403, accessDenied))
-    } else if (accessDenied && user) {
+
+    if (accessDenied && user) {
       this.messagesCached[cacheKey].push(HTTPError(403, accessDenied))
-    } else if (accessDenied && !user) {
-      this.messagesCached[cacheKey].push(HTTPError(401, 'Unauthenticated'))
     } else if (accessDenied) {
-      this.messagesCached[cacheKey].push(HTTPError(401, accessDenied))
+      this.messagesCached[cacheKey].push(HTTPError(401, 'Unauthenticated'))
     }
     this.aclCached[cacheKey] = Promise.resolve(!accessDenied)
     return this.aclCached[cacheKey]

lib/create-app.js

@@ -206,14 +206,17 @@ function initWebId (argv, app, ldp) {
   // without permission by including the credentials set by the Solid server.
   app.use((req, res, next) => {
     const origin = req.get('origin')
-    const trustedOrigins = argv.trustedOrigins
+    const trustedOrigins = ldp.getTrustedOrigins(req)
     const userId = req.session.userId
     // Exception: allow logout requests from all third-party apps
     // such that OIDC client can log out via cookie auth
     // TODO: remove this exception when OIDC clients
     // use Bearer token to authenticate instead of cookie
     // (https://github.com/solid/node-solid-server/pull/835#issuecomment-426429003)
-    if (!argv.host.allowsSessionFor(userId, origin, trustedOrigins) && !isLogoutRequest(req)) {
+    // We only want to do this check if strictOrigin is set to false, since we trust WebACL to handle origins otherwise
+    // If we don't allow authentication with cookies we can't handle ACLs properly wrt trusted origins
+    // https://github.com/solid/node-solid-server/issues/1117
+    if (!argv.strictOrigin && !argv.host.allowsSessionFor(userId, origin, trustedOrigins) && !isLogoutRequest(req)) {
       debug.authentication(`Rejecting session for ${userId} from ${origin}`)
       // Destroy session data
       delete req.session.userId

lib/models/solid-host.js

@@ -77,11 +77,7 @@ class SolidHost {
     const serverHost = getHostName(this.serverUri)
     if (originHost === serverHost) return true
     if (originHost.endsWith('.' + serverHost)) return true
-    // Allow the user's own domain
-    const userHost = getHostName(userId)
-    if (originHost === userHost) return true
-    if (trustedOrigins.includes(origin)) return true
-    return false
+    return !!trustedOrigins.includes(origin)
   }
 
   /**

test/integration/acl-tls-test.js

@@ -332,7 +332,7 @@ describe('ACL with WebID+TLS', function () {
 
       request.head(options, function (error, response, body) {
         assert.equal(error, null)
-        assert.equal(response.statusCode, 403)
+        assert.equal(response.statusCode, 401)
         done()
       })
     })
@@ -354,7 +354,7 @@ describe('ACL with WebID+TLS', function () {
 
         request.head(options, function (error, response, body) {
           assert.equal(error, null)
-          assert.equal(response.statusCode, 403)
+          assert.equal(response.statusCode, 401)
           done()
         })
       })
@@ -433,7 +433,7 @@ describe('ACL with WebID+TLS', function () {
 
       request.head(options, function (error, response, body) {
         assert.equal(error, null)
-        assert.equal(response.statusCode, 403)
+        assert.equal(response.statusCode, 401)
         done()
       })
     })
@@ -455,7 +455,7 @@ describe('ACL with WebID+TLS', function () {
 
         request.head(options, function (error, response, body) {
           assert.equal(error, null)
-          assert.equal(response.statusCode, 403)
+          assert.equal(response.statusCode, 401)
           done()
         })
       })