Identityserver4: client list per session

[ic21]

Hi,

I am a steward of our authorization server (implementing identity server 4 and asp net identity) and an _issue_ was brought to my attention - wondering if it is an issue or desired functionality.

Case

The integrating RP is a net 4.x MVC application with a vue.js front end using an AuthCode with PKCE client. On logout, a call is made from js client to application server which issues a redirect with the id_token to /connect/endsession at the auth server. The AS logout handler makes the following calls:

// delete local authentication cookie
await _signInManager.SignOutAsync();

// revoke all user tokens for this session
await _interactionService.RevokeTokensForCurrentSessionAsync();

// raise the logout event
await _eventService.RaiseAsync(new UserLogoutSuccessEvent(user.GetSubjectId().ToString(), user.GetDisplayName()));

_note: we use the_ DefaultIdentityServerInteractionService

My question arrises in _interactionService.RevokeTokensForCurrentSessionAsync();, specifically, it's call to _userSession.GetClientListAsync(); (DefaultUserSession) - it appears the result is a list of all clients the subject has authenticated with in all sessions.
i.e. if a subject has authenticated with the same client on multiple browsers, machines etc - a logout call on one will consequently logout all.

Question(s)

• Is this behaviour expected?
• If so, any suggestions to limit scope of token revocation? (probably better suited for SO, but since am here figured I'd ask)

Many thanks in advance.

[ic21]

So it looks like https://github.com/IdentityServer/IdentityServer4/pull/4210 addresses this in part with grant store storing sid, but the RemoveAllGrantsAsync contract still only taking client and subject. Should the signature include sid or should there be another method for only revoking session grants?

[brockallen]

So looking at the code, what you're saying is correct. I also have the same reaction -- I'd expect that to only revoke the current session. While review this, I do recall us discussing this and raising this as a questions and for some reason (which I can't recall) we decided to do it this way.

I'm happy to change this API to accept a flag to filter based on the current session id, if that would be an acceptable outcome? Or we discuss if changing the default semantics to do pure session based would just make more sense.

In the meantime, you could always implement this interface yourself and override the behavior for that one API.

[brockallen]

Actually, looking at it more I think I know why... we don't provide a formal column for the session id in the grants table. At least we didn't used to, but we will in the upcoming release. So yes, I will augment the IPersistedGrantService to allow for session id based revocation.

[ic21]

Thanks for confirming, @brockallen - I'll go ahead and close question.

Edit: Reopened - wasn't sure if I should close prior to release if there is work to be done - do I need to add a separate tag?

[brockallen]

PR was merged to support this.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.