search

IdentityModel / oidc-client-js

OpenID Connect (OIDC) and OAuth2 protocol support for browser-based JavaScript applications
Apache License 2.0
2.43k stars 842 forks source link

Monitor session_state when user not logged in #749

Open amherrington13 opened 5 years ago

amherrington13 commented 5 years ago

We recently pulled in this library as a means to integrate OAuth2/OIDC clients with our own OIDC provider. We’ve greatly enjoyed all that this library has to offer, and the ease with which the OIDC session management capabilities can be configured via the UserManager class. One thing we have come up against in implementation, though, is that it appears the library in its current state does not track the session_state (via the OP iframe) unless/until a user is logged in. Per the OpenID Connect Session Management specification Section 4.1,

Session state SHOULD be returned upon an authentication failure.

We have chosen to implement this in our provider such that session_state is returned on both authentication successes and failures. Our reason for doing this is so that session state is monitored even when a user is not logged in.

Consider this scenario: a user is browsing app A (app A participates in OIDC session management) in a not logged in state. In another tab, the user logs in to a different app (app B) also participating in session management via the same OIDC provider as app A. Our intent is for app A to detect the change in session state from “not logged in” to “logged in” via the OP iframe.

This currently is not possible with the library’s implementation, as session_state is part of the User class, and is only ever monitored via the OP iframe when/if a user is logged in.

brockallen commented 5 years ago

I think there's already an issue related to this. Can you dig it up and see if what you're asking is already covered?

amherrington13 commented 5 years ago

I've been unable to find an issue covering this specifically. I found a similar issue in 340, but this appears to cover the scenario where there is already a logged in user and something about the session changes, not where the session status changes from not logged in to logged in.

brockallen commented 5 years ago

I'll add it an an enhancement but I'm not sure if/when I would get to it.

rmmeans commented 5 years ago

@brockallen - my colleague @amherrington13 and I are working on a contribution to resolve this. Hope you don't mind our activity in submitting PR's for your project over the coming weeks! We'd much rather take the time to contribute to your OSS library than run a fork.

Thank you so much for this library!