diff --git a/lib/actions/end_session.js b/lib/actions/end_session.js index f4262a011..23e23b029 100644 --- a/lib/actions/end_session.js +++ b/lib/actions/end_session.js @@ -180,17 +180,10 @@ module.exports = { if (session.authorizations) { await Promise.all( Object.entries(session.authorizations).map(async ([clientId, { grantId }]) => { - // 1) drop the grants for the client that requested a logout - // 2) drop the grants without offline_access + // Drop the grants without offline_access // Note: tokens that don't get dropped due to offline_access having being added // later will still not work, as such they will be orphaned until their TTL hits - if ( - grantId - && ( - clientId === state.clientId - || !session.authorizationFor(clientId).persistsLogout - ) - ) { + if (grantId && !session.authorizationFor(clientId).persistsLogout) { const client = await ctx.oidc.provider.Client.find(clientId).catch(() => {}); await revokeGrant(ctx.oidc.provider, client, grantId); ctx.oidc.provider.emit('grant.revoked', ctx, grantId); @@ -228,7 +221,7 @@ module.exports = { ); } else if (state.clientId) { const grantId = session.grantIdFor(state.clientId); - if (grantId) { + if (grantId && !session.authorizationFor(state.clientId).persistsLogout) { const client = await ctx.oidc.provider.Client.find(state.clientId).catch(() => {}); await revokeGrant(ctx.oidc.provider, client, grantId); ctx.oidc.provider.emit('grant.revoked', ctx, grantId); diff --git a/test/end_session/end_session.config.js b/test/end_session/end_session.config.js index 6df2c7a52..6811a835f 100644 --- a/test/end_session/end_session.config.js +++ b/test/end_session/end_session.config.js @@ -7,16 +7,16 @@ module.exports = { clients: [ { client_id: 'client', - response_types: ['id_token'], - grant_types: ['implicit'], + response_types: ['id_token', 'code'], + grant_types: ['implicit', 'authorization_code'], redirect_uris: ['https://client.example.com/cb'], token_endpoint_auth_method: 'none', }, { client_id: 'client-hmac', client_secret: 'secret', - response_types: ['id_token'], - grant_types: ['implicit'], + response_types: ['id_token', 'code'], + grant_types: ['implicit', 'authorization_code'], redirect_uris: ['https://client.example.com/cb'], token_endpoint_auth_method: 'none', id_token_signed_response_alg: 'HS256', diff --git a/test/end_session/end_session.test.js b/test/end_session/end_session.test.js index e0a908ab4..755e2ef6a 100644 --- a/test/end_session/end_session.test.js +++ b/test/end_session/end_session.test.js @@ -439,30 +439,46 @@ describe('logout endpoint', () => { it('destroys complete session if user wants to', function () { const sessionId = this.getSessionId(); - const adapter = this.TestAdapter.for('Session'); - sinon.spy(adapter, 'destroy'); - sinon.spy(adapter, 'upsert'); + const sessionAdapter = this.TestAdapter.for('Session'); + sinon.spy(sessionAdapter, 'destroy'); + sinon.spy(sessionAdapter, 'upsert'); + const authorizationCodeAdapter = this.TestAdapter.for('AuthorizationCode'); + sinon.spy(authorizationCodeAdapter, 'revokeByGrantId'); + const session = this.getSession(); - this.getSession().state = { secret: '123', postLogoutRedirectUri: '/', clientId: 'client' }; + session.state = { secret: '123', postLogoutRedirectUri: '/', clientId: 'client' }; + session.authorizations.client.persistsLogout = true; + + const [firstGrant, secondGrant] = Object.keys(session.authorizations) + .map((x) => session.authorizations[x].grantId); return this.agent.post('/session/end/confirm') .send({ xsrf: '123', logout: 'yes' }) .type('form') .expect(302) .expect((response) => { - expect(adapter.destroy.called).to.be.true; - expect(adapter.upsert.called).not.to.be.true; - expect(adapter.destroy.withArgs(sessionId).calledOnce).to.be.true; + expect(sessionAdapter.destroy.called).to.be.true; + expect(sessionAdapter.upsert.called).not.to.be.true; + expect(sessionAdapter.destroy.withArgs(sessionId).calledOnce).to.be.true; expect(parseUrl(response.headers.location, true).query).not.to.have.property('client_id'); + expect(authorizationCodeAdapter + .revokeByGrantId.calledOnce).to.be.true; + expect(authorizationCodeAdapter + .revokeByGrantId.withArgs(firstGrant).calledOnce).to.be.false; + expect(authorizationCodeAdapter + .revokeByGrantId.withArgs(secondGrant).calledOnce).to.be.true; }); }); it('only clears one clients session if user doesnt wanna log out (using post_logout_redirect_uri)', function () { const adapter = this.TestAdapter.for('Session'); sinon.spy(adapter, 'destroy'); + const authorizationCodeAdapter = this.TestAdapter.for('AuthorizationCode'); + sinon.spy(authorizationCodeAdapter, 'revokeByGrantId'); let session = this.getSession(); const oldId = this.getSessionId(); session.state = { secret: '123', postLogoutRedirectUri: 'https://rp.example.com/logout/cb', clientId: 'client' }; + session.authorizations.client.persistsLogout = true; expect(session.authorizations.client).to.be.ok; @@ -477,6 +493,8 @@ describe('logout endpoint', () => { expect(this.getSessionId()).not.to.eql(oldId); expect(adapter.destroy.calledOnceWith(oldId)).to.be.true; expect(parseUrl(response.headers.location, true).query).not.to.have.key('client_id'); + expect(authorizationCodeAdapter + .revokeByGrantId.called).to.be.false; }); });