From 3cb81b755fd5e25df953e09675805ba3b47e6a23 Mon Sep 17 00:00:00 2001 From: Carl Blakeley Date: Thu, 11 Oct 2018 16:38:54 +0100 Subject: [PATCH] RPInitiatedLogoutRequest fixes --- src/handlers/RPInitiatedLogoutRequest.js | 48 ++++++++++++------- test/handlers/RPInitiatedLogoutRequestSpec.js | 19 ++------ 2 files changed, 33 insertions(+), 34 deletions(-) diff --git a/src/handlers/RPInitiatedLogoutRequest.js b/src/handlers/RPInitiatedLogoutRequest.js index 3428779..3f5aa5c 100644 --- a/src/handlers/RPInitiatedLogoutRequest.js +++ b/src/handlers/RPInitiatedLogoutRequest.js @@ -7,6 +7,7 @@ const qs = require('qs') const BaseRequest = require('./BaseRequest') const IDToken = require('../IDToken') +const { JWKSet } = require('@solid/jose') const DEFAULT_POST_LOGOUT_URI = '/' @@ -40,20 +41,17 @@ class RPInitiatedLogoutRequest extends BaseRequest { * @returns {Promise} */ static handle (req, res, provider) { - const { host } = provider const request = new RPInitiatedLogoutRequest(req, res, provider) return Promise .resolve(request) .then(request.validate) - // From: https://openid.net/specs/openid-connect-session-1_0.html#RPLogout // At the logout endpoint, the OP SHOULD ask the End-User whether they want // to log out of the OP as well. If the End-User says "yes", then the OP // MUST log out the End-User. - .then(host.logout) - - .then(request.redirectToGoodbye.bind(request)) + .then(request.clearUserSession.bind(request)) + .then(request.redirectToPostLogoutUri.bind(request)) .catch(request.error.bind(request)) } @@ -87,12 +85,15 @@ class RPInitiatedLogoutRequest extends BaseRequest { try { decodedHint = await IDToken.decode(idTokenHint) } catch (error) { - console.error('Error decoding ID Token hint (', idTokenHint, '):', error) - this.badRequest({error_description: 'Error decoding ID Token hint'}) + request.badRequest({error_description: 'Error decoding ID Token hint'}) } - if (!decodedHint.resolveKeys(provider.keys.jwks)) { - this.badRequest({ + // Importing the provider keys creates a CryptoKey property on each. + // A CryptoKey object is required for verifying the ID token. + let jwks = await JWKSet.importKeys(provider.keys.jwks); + // Resolve which signing key should be used to verify the ID token. + if (!decodedHint.resolveKeys(jwks)) { + request.badRequest({ error_description: 'ID Token hint signing keys cannot be resolved' }) } @@ -101,7 +102,7 @@ class RPInitiatedLogoutRequest extends BaseRequest { await decodedHint.verify() } catch (cause) { console.error('Could not verify ID Token hint:', decodedHint) - this.badRequest({error_description: 'Could not verify ID Token hint'}) + request.badRequest({error_description: 'Could not verify ID Token hint'}) } request.params.decodedHint = decodedHint @@ -132,7 +133,7 @@ class RPInitiatedLogoutRequest extends BaseRequest { } if (!decodedHint) { - return this.badRequest({ + return request.badRequest({ error_description: 'post_logout_redirect_uri requires id_token_hint' }) } @@ -143,14 +144,14 @@ class RPInitiatedLogoutRequest extends BaseRequest { const client = await provider.backend.get('clients', clientId) if (!client) { - return this.badRequest({ + return request.badRequest({ error_description: 'Invalid ID Token hint (client not found)' }) } // Check that the post logout uri has been registered - if (!client['post_logout_redirect_uris'].includes(uri)) { - return this.badRequest({ + if (!client['post_logout_redirect_uris'] || !client['post_logout_redirect_uris'].includes(uri)) { + return request.badRequest({ error_description: 'post_logout_redirect_uri must be pre-registered' }) } @@ -200,13 +201,17 @@ class RPInitiatedLogoutRequest extends BaseRequest { * this is still respectful of the OAuth2 threat model, and mitigates the * risk of rogue redirects. */ - redirectToGoodbye () { + redirectToPostLogoutUri () { const { host, res, params } = this const { state } = params - let uri = params['post_logout_redirect_uri'] || - host.defaults['post_logout_redirect_uri'] || - DEFAULT_POST_LOGOUT_URI + let uri = null + if (params && params['post_logout_redirect_uri']) + uri = params['post_logout_redirect_uri'] + if (!uri && host.defaults && host.defaults['post_logout_redirect_uri']) + uri = host.defaults['post_logout_redirect_uri'] + if (!uri) + uri = DEFAULT_POST_LOGOUT_URI if (state) { const queryString = qs.stringify({ state }) @@ -215,6 +220,13 @@ class RPInitiatedLogoutRequest extends BaseRequest { res.redirect(uri) // 302 redirect } + + clearUserSession () { + let session = this.req.session + session.cookie.expires = new Date(Date.now()) + session.userId = null + session.subject = null + } } module.exports = RPInitiatedLogoutRequest diff --git a/test/handlers/RPInitiatedLogoutRequestSpec.js b/test/handlers/RPInitiatedLogoutRequestSpec.js index 05de46a..1939db2 100644 --- a/test/handlers/RPInitiatedLogoutRequestSpec.js +++ b/test/handlers/RPInitiatedLogoutRequestSpec.js @@ -14,7 +14,6 @@ const IDToken = require('../../src/IDToken') const provider = { host: { - logout: () => {}, defaults: { post_logout_redirect_uri: '/goodbye' } @@ -43,18 +42,6 @@ const reqWithParams = HttpMocks.createRequest({ }) describe('RPInitiatedLogoutRequest', () => { - describe('handle()', () => { - it('should invoke injected host.logout', () => { - let res = HttpMocks.createResponse() - let logoutSpy = sinon.stub(provider.host, 'logout').resolves() - - return RPInitiatedLogoutRequest.handle(reqNoParams, res, provider) - .then(() => { - expect(logoutSpy).to.have.been.called() - }) - }) - }) - describe('constructor()', () => { it('should parse the incoming request params', () => { let res = {} @@ -71,7 +58,7 @@ describe('RPInitiatedLogoutRequest', () => { it('should validate that `post_logout_redirect_uri` has been registered') }) - describe('redirectToGoodbye()', () => { + describe('redirectToPostLogoutUri()', () => { it('should redirect to RP if logout uri provided', () => { let res = HttpMocks.createResponse() let req = HttpMocks.createRequest({ @@ -83,7 +70,7 @@ describe('RPInitiatedLogoutRequest', () => { }) let request = new RPInitiatedLogoutRequest(req, res, provider) - request.redirectToGoodbye() + request.redirectToPostLogoutUri() expect(res.statusCode).to.equal(302) expect(res._getRedirectUrl()) @@ -94,7 +81,7 @@ describe('RPInitiatedLogoutRequest', () => { let res = HttpMocks.createResponse() let request = new RPInitiatedLogoutRequest(reqNoParams, res, provider) - request.redirectToGoodbye() + request.redirectToPostLogoutUri() expect(res.statusCode).to.equal(302) expect(res._getRedirectUrl())