From 0170a221c2edef027562d0e2e9b6600effe5d448 Mon Sep 17 00:00:00 2001 From: Ruben Verborgh Date: Tue, 2 Oct 2018 21:10:37 +0200 Subject: [PATCH 1/3] Allow cookies from all subdomains. --- lib/models/solid-host.js | 19 +++++++++++++------ test/unit/solid-host-test.js | 14 +++++++------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/models/solid-host.js b/lib/models/solid-host.js index 1a926e97e..663546f62 100644 --- a/lib/models/solid-host.js +++ b/lib/models/solid-host.js @@ -72,12 +72,14 @@ class SolidHost { allowsSessionFor (userId, origin) { // Allow no user or an empty origin if (!userId || !origin) return true - // Allow the server's main domain - if (origin === this.serverUri) return true - // Allow the user's subdomain - const userIdHost = userId.replace(/([^:/])\/.*/, '$1') - if (origin === userIdHost) return true - // Disallow everything else + // Allow the server and subdomains + const originHost = getHostName(origin) + 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 return false } @@ -109,4 +111,9 @@ class SolidHost { } } +function getHostName (url) { + const match = url.match(/^\w+:\/*([^/]+)/) + return match ? match[1] : '' +} + module.exports = SolidHost diff --git a/test/unit/solid-host-test.js b/test/unit/solid-host-test.js index 04cdad308..19b7e95d7 100644 --- a/test/unit/solid-host-test.js +++ b/test/unit/solid-host-test.js @@ -55,23 +55,23 @@ describe('SolidHost', () => { }) it('should allow a userId with empty origin', () => { - expect(host.allowsSessionFor('https://user.test.local/profile/card#me', '')).to.be.true + expect(host.allowsSessionFor('https://user.own/profile/card#me', '')).to.be.true }) it('should allow a userId with the user subdomain as origin', () => { - expect(host.allowsSessionFor('https://user.test.local/profile/card#me', 'https://user.test.local')).to.be.true + expect(host.allowsSessionFor('https://user.own/profile/card#me', 'https://user.own')).to.be.true }) - it('should disallow a userId with another subdomain as origin', () => { - expect(host.allowsSessionFor('https://user.test.local/profile/card#me', 'https://other.test.local')).to.be.false + it('should allow a userId with the server domain as origin', () => { + expect(host.allowsSessionFor('https://user.own/profile/card#me', 'https://test.local')).to.be.true }) - it('should allow a userId with the server domain as origin', () => { - expect(host.allowsSessionFor('https://user.test.local/profile/card#me', 'https://test.local')).to.be.true + it('should allow a userId with a server subdomain as origin', () => { + expect(host.allowsSessionFor('https://user.own/profile/card#me', 'https://other.test.local')).to.be.true }) it('should disallow a userId from a different domain', () => { - expect(host.allowsSessionFor('https://user.test.local/profile/card#me', 'https://other.remote')).to.be.false + expect(host.allowsSessionFor('https://user.own/profile/card#me', 'https://other.remote')).to.be.false }) }) From e3050b9bfe9aeb4f9010bb80b9625f4a00ae5f40 Mon Sep 17 00:00:00 2001 From: Ruben Verborgh Date: Tue, 2 Oct 2018 22:24:45 +0200 Subject: [PATCH 2/3] Revert "Allow session to reach auth handlers." This reverts commit 4d0c094c157103c04cd83048af20a3346a6b000b. --- lib/create-app.js | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/lib/create-app.js b/lib/create-app.js index 90bfcc832..77c33803a 100644 --- a/lib/create-app.js +++ b/lib/create-app.js @@ -168,7 +168,24 @@ function initWebId (argv, app, ldp) { // (for same-domain browsing by people only) const useSecureCookies = !!argv.sslKey // use secure cookies when over HTTPS const sessionHandler = session(sessionSettings(useSecureCookies, argv.host)) - app.use(sessionHandler) + app.use((req, res, next) => { + sessionHandler(req, res, () => { + // Reject cookies from third-party applications. + // Otherwise, when a user is logged in to their Solid server, + // any third-party application could perform authenticated requests + // without permission by including the credentials set by the Solid server. + const origin = req.headers.origin + const userId = req.session.userId + if (!argv.host.allowsSessionFor(userId, origin)) { + debug(`Rejecting session for ${userId} from ${origin}`) + // Destroy session data + delete req.session.userId + // Ensure this modified session is not saved + req.session.save = (done) => done() + } + next() + }) + }) let accountManager = AccountManager.from({ authMethod: argv.auth, @@ -187,25 +204,6 @@ function initWebId (argv, app, ldp) { // Set up authentication-related API endpoints and app.locals initAuthentication(app, argv) - // Protect against requests from third-party applications - app.use((req, res, next) => { - // Reject cookies from third-party applications. - // Otherwise, when a user is logged in to their Solid server, - // any third-party application could perform authenticated requests - // without permission by including the credentials set by the Solid server. - const origin = req.headers.origin - const userId = req.session.userId - if (!argv.host.allowsSessionFor(userId, origin)) { - debug(`Rejecting session for ${userId} from ${origin}`) - // Destroy session data - delete req.session.userId - // Ensure this modified session is not saved - req.session.save = done => done() - } - next() - }) - - // Set up per-host LDP middleware if (argv.multiuser) { app.use(vhost('*', LdpMiddleware(corsSettings))) } From 0a48614be6597f29eb46a54ee5c4d85a5e94a00f Mon Sep 17 00:00:00 2001 From: Ruben Verborgh Date: Tue, 2 Oct 2018 22:29:38 +0200 Subject: [PATCH 3/3] Allow logout requests from all third-party apps. --- lib/create-app.js | 45 +++++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/lib/create-app.js b/lib/create-app.js index 77c33803a..62d0cedf1 100644 --- a/lib/create-app.js +++ b/lib/create-app.js @@ -168,23 +168,27 @@ function initWebId (argv, app, ldp) { // (for same-domain browsing by people only) const useSecureCookies = !!argv.sslKey // use secure cookies when over HTTPS const sessionHandler = session(sessionSettings(useSecureCookies, argv.host)) + app.use(sessionHandler) + // Reject cookies from third-party applications. + // Otherwise, when a user is logged in to their Solid server, + // any third-party application could perform authenticated requests + // without permission by including the credentials set by the Solid server. app.use((req, res, next) => { - sessionHandler(req, res, () => { - // Reject cookies from third-party applications. - // Otherwise, when a user is logged in to their Solid server, - // any third-party application could perform authenticated requests - // without permission by including the credentials set by the Solid server. - const origin = req.headers.origin - const userId = req.session.userId - if (!argv.host.allowsSessionFor(userId, origin)) { - debug(`Rejecting session for ${userId} from ${origin}`) - // Destroy session data - delete req.session.userId - // Ensure this modified session is not saved - req.session.save = (done) => done() - } - next() - }) + const origin = req.headers.origin + 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) && !isLogoutRequest(req)) { + debug(`Rejecting session for ${userId} from ${origin}`) + // Destroy session data + delete req.session.userId + // Ensure this modified session is not saved + req.session.save = (done) => done() + } + next() }) let accountManager = AccountManager.from({ @@ -209,6 +213,15 @@ function initWebId (argv, app, ldp) { } } +/** + * Determines whether the given request is a logout request + */ +function isLogoutRequest (req) { + // TODO: this is a hack that hard-codes OIDC paths, + // this code should live in the OIDC module + return req.path === '/logout' || req.path === '/goodbye' +} + /** * Sets up authentication-related routes and handlers for the app. *