Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(oidc): ensure unique tabIds when page is duplicated (release) #1448

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

krzempekk
Copy link
Contributor

This is a follow-up for #1423. Mechanism introduced in #1402 works under assumption that session storage for each tab is independent and opening new tab creates fresh instance of it. Which is generally true, except for one browser gimmick - if page is duplicated (in Chrome -> right click on tab name then "Duplicate") session storage is copied to duplicated tab (see https://developer.mozilla.org/en-US/docs/Web/API/Window/sessionStorage). This creates an issue in following scenario:

  1. User opens application and logs in
  2. User duplicates tab
  3. User logs out from both tabs
  4. User tries to log in again on both tabs
    This scenario can cause various issues, depending on specific IDP, which are caused by two tabs having the same id.

A picture tells a thousand words

Before this PR

After this PR

This PR introduces solution inspired by https://github.com/haricane8133/unique-tabid/tree/master, which is based on BroadcastChannel. Idea is that each Oidc class instance has channel listening for messages. When new instance is created (either in duplicated tab, if app url was opened in empty tab or if page was refreshed), following steps are done:

  1. BroadcastChannel instance is created
  2. Session storage is checked
    • If ID is not present, then new ID is generated and logic ends
    • If ID is present then SEARCH message is sent via channel
  3. Other tabs, if there are any, are receiving SEARCH message and comparing it with their own ID. If there is a match, FOUND message is sent.
  4. If FOUND message is received, new ID is generated. Otherwise, if no message is received after some timeout (assumed 500ms here), ID from session storage is used.

@krzempekk
Copy link
Contributor Author

Hi @guillaume-chervet ! Could you take a look at this?

@guillaume-chervet
Copy link
Contributor

Hi @krzempekk , I test it as soon as possible (like today) Thank you again. I did not know that tab clon can bring some problems. Thank you!

@guillaume-chervet
Copy link
Contributor

I do not know how but it would be awesome to move the main part of the code in initWorker.js .

@krzempekk
Copy link
Contributor Author

Hi @guillaume-chervet! I tried to put this code in initWorker.js, but I haven't found reasonable way to do this. Adding tabId and channel to Oidc class helps ensuring that there is only one instance of them per tab.

@guillaume-chervet guillaume-chervet merged commit 4f9ca8e into AxaFrance:main Sep 11, 2024
13 checks passed
@guillaume-chervet
Copy link
Contributor

Let ='s try it as a beta.
I Will try to find an idea but let's iterate.

Thank you @krzempekk :) again !

@krzempekk
Copy link
Contributor Author

Hi @guillaume-chervet ! Thanks for merging! Can you trigger release with this fix? I see that it was not triggered

@guillaume-chervet
Copy link
Contributor

guillaume-chervet commented Sep 12, 2024

It was published yesterdays as a beta. I prefer to test it well before release it! @krzempekk

@krzempekk
Copy link
Contributor Author

Sure, totally understand! Could you share beta version so I can test it as well?

@guillaume-chervet
Copy link
Contributor

Version 7.22.25-beta.1555 is already on npm @krzempekk

@grpawel
Copy link

grpawel commented Oct 28, 2024

Hi @guillaume-chervet, I noticed this PR didn't end up in any released version, looks like it was removed from main branch.
Could you shed some light if you've discovered some issues in beta version? We would like to fix the solution so it's good enough to be published in a patch release.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants