Merge pull request #11 from OpenLinkSoftware/opl_181011_oidc_op_issue_10

RPInitiatedLogoutRequest fixes

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

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())