Session change not fired if identity token does not contain a sid claim

[bachratyg]

See here:

oidc-client-ts/src/SessionMonitor.ts

Line 156 in b7266eb

if (session.sid === this._sid) {

If there is no sid claim in the id token then undefined === undefined evaluates to true and the userSessionChanged event is never fired.

The sid claim is not part of the session management spec, it is only defined in frontchannel logout spec and it's optional. This claim should not be checked at all or at least the user session should be assumed to have changed when the claim is not present.

[pamapa]

Thanks for reporting, would be nice when you provide a fix for this.

[bachratyg]

I can help. There are a couple of options to handle this:

1. Check the sub claim, ignore the sid claim. This is a straightforward fix but a behavioral breaking change. It might be ok considering the current behavior is incorrect.
2. Check the sub claim, check the sid claim but only fire the event if it's either different or missing. This avoids the break if the sid is always present, but still causes false alarms when the presence of the sid varies e.g. by requested scope.
3. Opt-in to 1 by exposing a new property on UserManagerSettings e.g. checkSessionIgnoreSid?: boolean. This would avoid the break but might not be optimal for consumers to discover.
4. A generalization of 3: configure which checks to make and as a consequence which events to fire. This is a more involved change.

Option 4 can also tackle the issue that the callback hits the OP via querySessionStatus (a silent sign-in with no scopes) to determine which event to fire. If the event handler(s) are going to refresh the profile/tokens anyway (e.g. signinSilent) or just kick out the user, then this is redundant load on the OP. The fix would be an opt-in to forego all checks and always fire the userSessionChanged event without any processing, never the other two. For example angular-auth-oidc-client does this, it's entirely up to the user how to handle the change.

[pamapa]

@bachratyg Thanks for taking interests here, currently is a good time for a breaking change (i am planing v3), so lets focus on following the specs and doing correctly without fallback to old behavior.