Skip to content

Commit

Permalink
RPInitiatedLogoutRequest fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
cblakeley committed Oct 11, 2018
1 parent e1d7ad2 commit 3cb81b7
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 34 deletions.
48 changes: 30 additions & 18 deletions src/handlers/RPInitiatedLogoutRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '/'

Expand Down Expand Up @@ -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))
}

Expand Down Expand Up @@ -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'
})
}
Expand All @@ -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
Expand Down Expand Up @@ -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'
})
}
Expand All @@ -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'
})
}
Expand Down Expand Up @@ -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 })
Expand All @@ -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
19 changes: 3 additions & 16 deletions test/handlers/RPInitiatedLogoutRequestSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const IDToken = require('../../src/IDToken')

const provider = {
host: {
logout: () => {},
defaults: {
post_logout_redirect_uri: '/goodbye'
}
Expand Down Expand Up @@ -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 = {}
Expand All @@ -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({
Expand All @@ -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())
Expand All @@ -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())
Expand Down

0 comments on commit 3cb81b7

Please sign in to comment.