[Bug]: /token call with localStorage and multiple tabs does not work

[scurk1415]

Version

15.0.3

Please provide a link to a minimal reproduction of the bug

Can be reproduced on your own examples

Please provide the exception or error you saw

HttpRequest error with status 400:

Body:
error: "invalid_grant"
error_description: "PKCE verification failed"

Steps to reproduce the behavior

1. Set { provide: AbstractSecurityStorage, useClass: DefaultLocalStorageService } (or your own version of LocalStorage storage)
2. Open 1 browser tab (when not authenticated)
3. Open another browser tab (when not authenticated)
4. Go to the first tab and login
5. You get redirected to the unauthorizedRoute (with /token endpoint throwing an error)

A clear and concise description of what you expected to happen.

I expect to be redirected to the redirectUrl and the application to work

Additional context

This flow seems to be working with the default sessionStorage.
It works if we login on the 2nd tab (specified in the steps to reproduce), but not if we login on the 1st tab. I guess it has something to do with the nonce or code_challenge parameters that get appended to the url when we get refirected to the login page (we use Keycloak, but i guess that it is not that important).

We also use your AutoLoginAllRoutesGuard which indefinitely triggers the /token endpoint and blocks all processes (but i guess this can also be caused by our routing, so you can probably ignore this part).

[damienbod]

Yes localstorage is shared across tabs. So if you open multiple tabs, all use the same localstorage. It would be more safe to use session storage. We do you not use this?

Greetings Damien

[scurk1415]

The customer wants to be logged out from all sessions when they logout on one tab. With sessionStorage they get logged out just in one tab. With localStorage they get logged out from all, but then logging in becomes a problem (as described in the issue).

I think you can probably reuse the parameters (nonce/code_challenge) for certain scenarios but i guess you would need a new config for that or the AbstractSecurityStorage would need another method so that you know when to reuse.

[damienbod]

Sounds reasonable, the client should know that the overall security is worse by using local storage and and global client logout compared with session storage. The logout is supported, but you shared sensitive date for all js scripts running.

You could implement a custom storage, but I don't think we could separate the security bits, these are not made public I think...

Greetings Damien

[vds-simon-lapointe]

@damienbod

You could enforce the codeVerifier to be stored in sessionStorage instead of using the provided implementation of AbstractSecurityStorage.

[fritzwf]

You can use sessionStorage for tokens etc and use localStorage to logout all of the logged-in tabs like this:

    window.addEventListener("storage", (event) => {
       if (localStorage.getItem(event.key || "") === REMOTE_LOGOUT_REQUEST) {
          this.logout();
       }
    });

[gmiked]

@fritzwf would you also need to add the below in the function where the logout button has been clicked?

localStorage.setItem('logout', 'remote-logout');

[fritzwf]

@fritzwf would you also need to add the below in the function where the logout button has been clicked?

localStorage.setItem('logout', 'remote-logout');

Yes, you would either use the this.logoff() function itself, or you could instead send an event using rxjs. That event would go to were ever the logoff() function is located and it would call the logoff() function.

Also, when the next login starts, remove the logout from the localStorage, that will have it ready for the next login. Also, when the logout is called, you set the localStorage: localStorage.setItem(LOGOUT_REQUEST_KEY, REMOTE_LOGOUT_REQUEST), that will trigger the localStorage event so every tab will get the event and logoff.