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

[Bug]: Silent refresh fails using localStorage when multiple tabs trying to refresh at the same time #1662

Closed
medeirosrich opened this issue Jan 26, 2023 · 19 comments

Comments

@medeirosrich
Copy link

medeirosrich commented Jan 26, 2023

Version

5.0.3

Please provide a link to a minimal reproduction of the bug

No response

Please provide the exception or error you saw

silent renew failed! Error: Error: authorizedCallback, token(s) validation failed, resetting.

Steps to reproduce the behavior

1) Set up OIDC Code Flow PKCE using refresh tokens (used 'ng add angular-auth-oidc-client')

2) Set up custom storage using localStorage (used example from https://angular-auth-oidc-client.com/docs/documentation/custom-storage)

4) To speed up testing, setup your authority to have short lived access and refresh tokens. (i.e. 1 or 2 minutes) 

3) Login using one browser [chrome] tab (i.e. this.oidcSecurityService.authorize()). Open console and verify refresh is working

4) Open a few more tabs. Open console and verify they are authorized and refreshing

5) Wait until error occurs. It shouldn't take long - usually 2-10 minutes with 1 minute tokens. Open more tabs if needed.

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

access token should refresh successfully and all tabs remain authorized

Additional context

It seems that multiple tabs simultaneously attempting to refresh the token using a shared localStorage is causing a race condition, resulting in validation failure.

@TarasKovalenko
Copy link
Contributor

@medeirosrich I tried to reproduce the issue, but from my side, everything works well. Can you please prove the AuthModule configuration?

@medeirosrich
Copy link
Author

medeirosrich commented Jan 30, 2023

@medeirosrich I tried to reproduce the issue, but from my side, everything works well. Can you please prove the AuthModule configuration?

import { NgModule } from '@angular/core';
import { AbstractSecurityStorage, AuthModule, LogLevel } from 'angular-auth-oidc-client';
import { LocalStorageManagerService } from './local-storage-manager.service';

@NgModule({
    imports: [AuthModule.forRoot({
        config: {
            authority: 'https://localhost/Dev/SecurityServer',
            redirectUrl: window.location.origin + "/Dev/Test/",
            postLogoutRedirectUri: window.location.origin + "/Dev/Test/",
            clientId: 'Portal',
            scope: 'openid email profile offline_access roles internal_api external_api', // 'openid profile offline_access ' + your scopes
            responseType: 'code',
            silentRenew: true,
            useRefreshToken: true,
            renewTimeBeforeTokenExpiresInSeconds: 30,
            ignoreNonceAfterRefresh: true,
            logLevel: LogLevel.Debug
        }
    })],
    exports: [AuthModule],
    providers: [{ provide: AbstractSecurityStorage, useClass: LocalStorageManagerService }],
})
export class AuthConfigModule { }

@TarasKovalenko
Copy link
Contributor

@medeirosrich I have the same configuration and everything works well with multiple tabs.

@medeirosrich
Copy link
Author

I don't know if this helps, but here is a side by side of the logs from the two tabs that are colliding

image

@TarasKovalenko
Copy link
Contributor

@medeirosrich your AuthConfigModule was added to the imports of the main AppModule? and can you please show the code of your LocalStorageManagerService? because localstorage by default shared all data between all tabs and windows from the same origin.

@medeirosrich
Copy link
Author

@TarasKovalenko Yes
Here is the AppModule

import { NgModule } from '@angular/core';
import { BrowserModule } from '@angular/platform-browser';

import { AppRoutingModule } from './app-routing.module';
import { AppComponent } from './app.component';
import { AuthConfigModule } from './auth/auth-config.module';

@NgModule({
  declarations: [
    AppComponent
  ],
  imports: [
    BrowserModule,
    AppRoutingModule,
    AuthConfigModule
  ],
  providers: [],
  bootstrap: [AppComponent]
})
export class AppModule { }

And the LocalStorageManagerService

import { Injectable } from "@angular/core";
import { AbstractSecurityStorage } from "angular-auth-oidc-client";

@Injectable()
export class LocalStorageManagerService implements AbstractSecurityStorage {
  read(key: string) {
    return localStorage.getItem(key);
  }

  write(key: string, value: any): void {
    localStorage.setItem(key, value);
  }

  remove(key: string): void {
    localStorage.removeItem(key);
  }

  clear(): void {
    localStorage.clear();
  }
}

@VaclavChalupa
Copy link

VaclavChalupa commented Mar 7, 2023

Hi @TarasKovalenko, is local storage with multiple browser tabs/windows supported?
It requires some internal synchronization to prevent concurrent refresh token issues.

@TarasKovalenko
Copy link
Contributor

Hi @VaclavChalupa ,
https://developer.mozilla.org/en-US/docs/Web/API/Window/localStorage
localStorage data is specific to the protocol of the document. In particular, for a site loaded over HTTP (e.g., http://example.com), localStorage returns a different object than localStorage for the corresponding site loaded over HTTPS (e.g., https://example.com).

is local storage with multiple browser tabs/windows supported - about tabs for sure, but about windows it's hard to say. data will be saved even if you close and open the browser again. so I guess the answer is yes (but not related to the incognito window)

@Adrii77
Copy link

Adrii77 commented Apr 13, 2023

Hello,

We’re facing the same issue… We try so many things to solve it, but we’re stuck.

Versions :
Angular : 15.2.5
angular-auth-oidc-client : 15.0.3

Contexte :

We are on Azure.

We’re using localStorage with a custom class the same as the documentation, but with providedIn ‘root’ in order to use the same instance in other places inside our app.

We’re using silentRenew (without Iframe).

When we’re using the application in a single tab, no problem : silent renew works perfectly.
But in multi tabs, when each of them ask to refresh the token (at the same time), it doesn’t work (see below) :

unknown

Has-new is an HTTP polling request. Left screen is a tab, the right another. 

Error always appeared after the « keys » http call. The call response is always return from disk cache (I don’t know if its a problem as it works in single tab with this behavior).
Note : the "has-new" call between "token" and "keys" send the old token. Is it normal ?
unknown

The encountered error :
MicrosoftTeams-image

@medeirosrich Did you find a solution ?
@TarasKovalenko Did you face the error since your last message ?

Thanks in advance

@medeirosrich
Copy link
Author

@Adrii77 We did not. We switched to using sessionStorage for now.

@medeirosrich
Copy link
Author

@damienbod I see you closed this bug, does that mean the concurrency issue when using localStorage is fixed?

@Adrii77
Copy link

Adrii77 commented Apr 17, 2023

@medeirosrich I guess not. Damien answered on this topic : #1716 (comment)

My client often need to open my app into another tab. If I use sessionStorage, he'll need to sign in on each tab... that's not a great UX.

@joewIST
Copy link

joewIST commented Apr 20, 2023

I am also facing similar issues. Session storage is not ideal as we want the authentication state to be preserved when the user opens the app in a new tab. Any joy?

@Adrii77
Copy link

Adrii77 commented Apr 20, 2023

We found a temporary solution with localStorage, which we're still experimenting right now :

We set the renewTimeBeforeTokenExpiresInSeconds with a random value between 100 and 500. This allows us to reduce the probability of tabs refreshing the token at the same time, as each tab will rarely has the same value (but, of course, zero risk does not exist...)

renewTimeBeforeTokenExpiresInSeconds: Math.floor(Math.random() * 400) + 100

@Coldfen
Copy link

Coldfen commented Sep 7, 2023

Hello, I have the same problem when I use localStorage. A temporary solution does not suit me well, because my token functions for 300 seconds. Do you have any advice or solutions for me?

Version: 16.0.0

@joewIST
Copy link

joewIST commented Sep 7, 2023

#1753 do you get any errors like this @Coldfen ?

@Coldfen
Copy link

Coldfen commented Sep 7, 2023

@joewIST I didn't see that error.

My error:
image

We use localStorage, and when we use multi tabs, we get this error.

@joewIST
Copy link

joewIST commented Jan 2, 2024

This is still an issue for us, has anyone been able to find a workaround for using localStorage + silentRenew? It definitely seems to be linked to multiple tabs refreshing at the same time... I'm not sure it should be closed.

@lemonCMS
Copy link

lemonCMS commented Apr 2, 2024

We are running in to exactly the same problem using localStorage + silentRenew and multiple open tabs.

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

No branches or pull requests

8 participants