Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[web] unify link behaviour across tabs #9325

Closed
exalate-issue-sync bot opened this issue Jun 30, 2023 · 4 comments · Fixed by #9386
Closed

[web] unify link behaviour across tabs #9325

exalate-issue-sync bot opened this issue Jun 30, 2023 · 4 comments · Fixed by #9386
Labels

Comments

@exalate-issue-sync
Copy link

Description

User Stories

  • As a user I want that pasting links into the browser address bar works independently of the browser tab so that I don't need to search fot the right tab.

  • As a user I want to login as less as possible because it distracts me from work.

Value

  • ease of use
  • improve collaboration

Acceptance Criteria

  • If a user pastes a public link into the addressbar, it should resolve consistently (not depending if the current tab holds a active session)
  • if a user closes a tab, he/she should not always get forced to login
  • add config option which browser storage should be user for session information (sessionStorage, localStorage[=default])
  • Token renewal is not concurrent if multiple tabs are open

Note: concurrent token renewal could be managed via serviceworker.

Definition of ready

[ ] everybody needs to understand the value written in the user story
[ ] acceptance criteria has to be defined
[ ] all dependencies of the user story need to be identified
[ ] feature should be seen from an end user perspective
[ ] user story has to be estimated
[ ] story points need to be less then 20

Definition of done

  • Functional requirements
    [ ] functionality described in the user story works
    [ ] acceptance criteria are fulfilled
  • Quality
    [ ] code review happened
    [ ] CI is green
    [ ] critical code received unit tests by the developer
    [ ] automated tests passed (if automated tests are not available, this test needs to be created and passed
  • Non-functional requirements
    [ ] no sonar cloud issues
@exalate-issue-sync
Copy link
Author

Jan Ackermann commented: https://developer.mozilla.org/en-US/docs/Web/API/Web_Locks_API seems to be good for tab locking

@JammingBen
Copy link
Contributor

I did a bit of research on the concurrent token renewal. Honestly, at this point, I'm not even 100% sure what the real issue is for us. So from my understanding:

Let's say you have 2 tabs opened at the same time while storing the access token in the local storage. Now there is a chance that both tabs try to do token renewal at the same time. However, that alone is not yet an issue...? As Web does a silent token renewal 10 seconds before a token expires, the worst thing that could happen is that the silent token renewal takes place 2 times which results in a) one unnecessary renewal and b) an orphaned access token which is (in theory) still valid. Am I missing something?

There doesn't seem to be a go-to way to solve these issues. There are a few possibilities:

  • Web Locks: That way we could ensure that the token renewal never happens at the very exact time. Still, all opened tabs would do a token renewal at some point because actions are simply queued when a lock is applied. So I'm not sure if we would gain anything really from this.
  • Window storage event: This event fires when any other tab/window changes something in the local storage. That way we could detect if an access token was renewed by another tab and skip token renewal... that sounds very error prone and hackish to me.
  • Randomize the accessTokenExpiringNotificationTimeInSeconds to minimize the risk of renewal happening at the same time. Seems like another half-ass solution IMO.

So all in all, I'm really not sure what to do. What's the real benefit of having the access token in the local storage? When using the session storage and opening Web in another tab or window, the user is being logged in automatically. That takes 1-2 seconds, but IMO that's okay. The only drawback is that opening public links in a new tab/window would require the user to log in.

@lookacat
Copy link
Contributor

lookacat commented Jun 30, 2023

A clean way to do it would be to extract the token renewal logic into a service worker, as far as I can remember you then would have only one instance that refreshes the token (which is running in kindof an sandbox mode) and you can then use an eventbus to communicate between the service worker and all clients (browser windows).
E.g. the code to send to all clients looks like this (running in a service worker)

async sendEventToAllClients(event) {
  const clients = await self.clients.matchAll({type: 'window'});
  for (const client of clients) {
    client.postMessage(event);
  }
}

Cons would be that we would have to setup all the service worker stuff which would take some time and isn't that easy but I already did an PoC a long time ago:
#7902
Pro would be if we set it up correctly we can use service worker for more tasks in the future.

@JammingBen
Copy link
Contributor

I get the idea behind that (and it would probably be the ultimate solution), although making it work with the token renewal process sounds hard. Simply extracting the token renewal method to a service worker won't cut it I think. All that is so deeply integrated in our authService and userManager (which extends the oidc-client-ts lib) so I guess we would need to move all that into a service worker - oof! 🙈

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

Successfully merging a pull request may close this issue.

2 participants