diff --git a/lib/create-app.js b/lib/create-app.js index 90bfcc832..62d0cedf1 100644 --- a/lib/create-app.js +++ b/lib/create-app.js @@ -169,6 +169,27 @@ function initWebId (argv, app, ldp) { 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) => { + 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({ authMethod: argv.auth, @@ -187,30 +208,20 @@ 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))) } } +/** + * 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. * 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 }) })