Open krzempekk opened 2 months ago
Thank you @krzempekk that a good issue !
To be able to reproduce it is the most difficult part thank you :)
I'am in Holiday (3 weeks) I do not know when I will be able to fixed it!
Thanks @guillaume-chervet for the response!
No worries, enjoy your holidays :D. It's not a blocker for us, just wanted to report this.
I have seen similar behavior, but I don't think I can reproduce it outside of a service worker context; in principle each tab should have its own session storage. There is some behavior to copy session storage when cloning a tab in the browser; but that would not explain this issue I think (since the value from the original tab should only be editable from the same tab). @krzempekk are you able to reproduce this without a service worker?
@guillaume-chervet What are you thinking in terms of a solution? Here are my thoughts:
If this is indeed only related to the service worker, what could be done is generate a tabId
which is stored in session storage. Then, when calling setState, or setNonce, (and getState, getNonce), we also send the tabId from the storage to the service worker, which will in turn set e.g. currentDatabase.state[tabId] = state
.
The service worker only checks the nonce; the state is checked in the oidc client.
The nonce check in the service worker is done during a fetch, so I am not confident we can get a tabId of the tab that initiated the fetch. It seems like the only solution is to check all the available nonces for one that matches.
Edit: although if I look at the interface of the service worker api, it seems there are things like client id for the fetch and client for the messages. If those are able to identify tabs consistently, potentially there would be no impact on the oidc-client at all, only on the service-worker
Edit2: It does appear that the source
attribute is not usable, since it is specific to a page rather than a session.
@guillaume-chervet I have made some progress on this issue (I think), as well as made some changes in the examples/react-oidc-demo
package to make sure the existing tests are passing. I have not created any new tests to cover the functionality. I am going on holiday though, so this will be somewhat of a commit-and-run.
If you were going to look into this issue, it could be a starting point.
https://github.com/meesvandongen/oidc-client/tree/tab-specific-state-nonce
Thank you very much @meesvandongen iI am very busy these days. May you run a draft pullrequest to be able to see the change easily? Event if it is not finished yet.
Issue and Steps to Reproduce
oidc-client
saves state for login requests in sessionStorage underoidc.state.${configurationName}
key. This causes issues in following scenario:This can be reproduced on oidc-client demo app (https://black-rock-0dc6b0d03.1.azurestaticapps.net/) using above steps.
Versions
7.22.4
Expected
If login is initiated from multiple tabs, completing it on each tab should succeed.
Actual
If login is initiated from multiple tabs, completing it is succeeding only on last tab that initiated login flow.
Additional Details
Possible solution might be reusing exiting state from session storage. Or creating unique
state
session storage entry for each initiated login flow.