Skip to content

Fix stale tokens returned during concurrent refresh-token renewal #2186

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.

Sign up for GitHub

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

Open
ThorstenKunz wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix stale tokens returned during concurrent refresh-token renewal #2186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 143 additions & 1 deletion projects/angular-auth-oidc-client/src/lib/callback/refresh-session.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { fakeAsync, TestBed, tick, waitForAsync } from '@angular/core/testing';
import { of, throwError } from 'rxjs';
import { of, Subject, throwError } from 'rxjs';
import { delay } from 'rxjs/operators';
import { mockProvider } from '../../test/auto-mock';
import { AuthStateService } from '../auth-state/auth-state.service';
Expand All @@ -10,6 +10,7 @@ import { RefreshSessionIframeService } from '../iframe/refresh-session-iframe.se
import { SilentRenewService } from '../iframe/silent-renew.service';
import { LoggerService } from '../logging/logger.service';
import { LoginResponse } from '../login/login-response';
import { EventTypes } from '../public-events/event-types';
import { PublicEventsService } from '../public-events/public-events.service';
import { StoragePersistenceService } from '../storage/storage-persistence.service';
import { UserService } from '../user-data/user.service';
Expand All @@ -30,6 +31,8 @@ describe('RefreshSessionService ', () => {
let refreshSessionIframeService: RefreshSessionIframeService;
let refreshSessionRefreshTokenService: RefreshSessionRefreshTokenService;
let authWellKnownService: AuthWellKnownService;
let publicEventsService: PublicEventsService;
let userService: UserService;

beforeEach(() => {
TestBed.configureTestingModule({
Expand Down Expand Up @@ -63,6 +66,8 @@ describe('RefreshSessionService ', () => {
silentRenewService = TestBed.inject(SilentRenewService);
authWellKnownService = TestBed.inject(AuthWellKnownService);
storagePersistenceService = TestBed.inject(StoragePersistenceService);
publicEventsService = TestBed.inject(PublicEventsService);
userService = TestBed.inject(UserService);
});

it('should create', () => {
Expand Down Expand Up @@ -285,6 +290,143 @@ describe('RefreshSessionService ', () => {
});
}));

it('returns tokens from the completed refresh result when auth-state getters are stale', waitForAsync(() => {
const allConfigs = [
{
configId: 'configId1',
silentRenewTimeoutInSeconds: 10,
},
];
const refreshResult = {
authResult: {
id_token: 'fresh-id-token',
access_token: 'fresh-access-token',
},
} as CallbackContext;

spyOn(
flowHelper,
'isCurrentFlowCodeFlowWithRefreshTokens'
).and.returnValue(true);
spyOn(
refreshSessionService as any,
'waitForRunningRefreshSessionIfRequired'
).and.returnValue(of(false));
spyOn(
refreshSessionService as any,
'startRefreshSession'
).and.returnValue(of(refreshResult));
spyOn(authStateService, 'areAuthStorageTokensValid').and.returnValue(
true
);
spyOn(authStateService, 'getIdToken').and.returnValue('stale-id-token');
spyOn(authStateService, 'getAccessToken').and.returnValue(
'stale-access-token'
);
spyOn(userService, 'getUserDataFromStore').and.returnValue({
sub: '123',
} as any);

refreshSessionService
.forceRefreshSession(allConfigs[0], allConfigs)
.subscribe((result) => {
expect(result).toEqual({
idToken: 'fresh-id-token',
accessToken: 'fresh-access-token',
userData: { sub: '123' },
isAuthenticated: true,
configId: 'configId1',
});
});
}));

it('falls back to auth-state getters when no refresh auth result is available', waitForAsync(() => {
const allConfigs = [
{
configId: 'configId1',
silentRenewTimeoutInSeconds: 10,
},
];

spyOn(
flowHelper,
'isCurrentFlowCodeFlowWithRefreshTokens'
).and.returnValue(true);
spyOn(
refreshSessionService as any,
'waitForRunningRefreshSessionIfRequired'
).and.returnValue(of(false));
spyOn(
refreshSessionService as any,
'startRefreshSession'
).and.returnValue(of(null));
spyOn(authStateService, 'areAuthStorageTokensValid').and.returnValue(
true
);
spyOn(authStateService, 'getIdToken').and.returnValue('stored-id-token');
spyOn(authStateService, 'getAccessToken').and.returnValue(
'stored-access-token'
);

refreshSessionService
.forceRefreshSession(allConfigs[0], allConfigs)
.subscribe((result) => {
expect(result.idToken).toBe('stored-id-token');
expect(result.accessToken).toBe('stored-access-token');
});
}));

it('waits for the running renew process to publish a refresh result before emitting', fakeAsync(() => {
const allConfigs = [
{
configId: 'configId1',
silentRenewTimeoutInSeconds: 10,
},
];
const events$ = new Subject<any>();
let actualResult: LoginResponse | undefined;

spyOn(
flowHelper,
'isCurrentFlowCodeFlowWithRefreshTokens'
).and.returnValue(true);
spyOn(flowsDataService, 'isSilentRenewRunning').and.returnValue(true);
spyOn(publicEventsService, 'registerForEvents').and.returnValue(events$);
spyOn(authStateService, 'areAuthStorageTokensValid').and.returnValue(
true
);
spyOn(authStateService, 'getIdToken').and.returnValue('updated-id-token');
spyOn(authStateService, 'getAccessToken').and.returnValue(
'updated-access-token'
);

refreshSessionService
.forceRefreshSession(allConfigs[0], allConfigs)
.subscribe((result) => {
actualResult = result;
});

tick();
expect(actualResult).toBeUndefined();

events$.next({
type: EventTypes.NewAuthenticationResult,
value: {
configId: 'configId1',
isRenewProcess: true,
},
});
tick();

expect(actualResult).toEqual({
idToken: 'updated-id-token',
accessToken: 'updated-access-token',
userData: undefined,
isAuthenticated: true,
configId: 'configId1',
});
}));

it('calls start refresh session and waits for completed, returns idtoken and accesstoken if auth is true', waitForAsync(() => {
spyOn(
flowHelper,
Expand Down
69 changes: 63 additions & 6 deletions projects/angular-auth-oidc-client/src/lib/callback/refresh-session.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
timer,
} from 'rxjs';
import {
catchError,
filter,
map,
mergeMap,
Expand All @@ -26,6 +27,8 @@ import { RefreshSessionIframeService } from '../iframe/refresh-session-iframe.se
import { SilentRenewService } from '../iframe/silent-renew.service';
import { LoggerService } from '../logging/logger.service';
import { LoginResponse } from '../login/login-response';
import { EventTypes } from '../public-events/event-types';
import { PublicEventsService } from '../public-events/public-events.service';
import { StoragePersistenceService } from '../storage/storage-persistence.service';
import { UserService } from '../user-data/user.service';
import { FlowHelper } from '../utils/flowHelper/flow-helper.service';
Expand All @@ -50,6 +53,7 @@ export class RefreshSessionService {
private readonly refreshSessionRefreshTokenService = inject(
RefreshSessionRefreshTokenService
);
private readonly publicEventsService = inject(PublicEventsService);
private readonly userService = inject(UserService);

userForceRefreshSession(
Expand Down Expand Up @@ -85,15 +89,31 @@ export class RefreshSessionService {
};

if (this.flowHelper.isCurrentFlowCodeFlowWithRefreshTokens(config)) {
return this.startRefreshSession(config, allConfigs, mergedParams).pipe(
map(() => {
return this.waitForRunningRefreshSessionIfRequired(config).pipe(
switchMap((shouldWaitForRunningRenew) => {
if (shouldWaitForRunningRenew) {
return of(null);
}

return this.startRefreshSession(config, allConfigs, mergedParams);
}),
map((refreshSessionResult) => {
const isAuthenticated =
this.authStateService.areAuthStorageTokensValid(config);

if (isAuthenticated) {
const authResult =
refreshSessionResult && typeof refreshSessionResult !== 'boolean'
? refreshSessionResult.authResult
: null;

return {
idToken: this.authStateService.getIdToken(config),
accessToken: this.authStateService.getAccessToken(config),
idToken:
authResult?.id_token ??
this.authStateService.getIdToken(config),
accessToken:
authResult?.access_token ??
this.authStateService.getAccessToken(config),
userData: this.userService.getUserDataFromStore(config),
isAuthenticated,
configId,
Expand Down Expand Up @@ -216,12 +236,17 @@ export class RefreshSessionService {
return of(null);
}

this.flowsDataService.setSilentRenewRunning(config);

return this.authWellKnownService
.queryAndStoreAuthWellKnownEndPoints(config)
.pipe(
switchMap(() => {
this.flowsDataService.setSilentRenewRunning(config);
catchError((error) => {
this.flowsDataService.resetSilentRenewRunning(config);

return throwError(() => error);
}),
switchMap(() => {
if (this.flowHelper.isCurrentFlowCodeFlowWithRefreshTokens(config)) {
// Refresh Session using Refresh tokens
return this.refreshSessionRefreshTokenService.refreshSessionWithRefreshTokens(
Expand All @@ -239,4 +264,36 @@ export class RefreshSessionService {
})
);
}

private waitForRunningRefreshSessionIfRequired(
config: OpenIdConfiguration
): Observable<boolean> {
const isSilentRenewRunning =
this.flowsDataService.isSilentRenewRunning(config);

if (!isSilentRenewRunning) {
return of(false);
}

return this.publicEventsService.registerForEvents().pipe(
filter(
(notification) =>
notification.type === EventTypes.NewAuthenticationResult
),
map((notification) => notification.value),
filter(
(
authStateResult
): authStateResult is {
isRenewProcess: boolean;
configId?: string;
} =>
!!authStateResult &&
authStateResult.isRenewProcess === true &&
authStateResult.configId === config.configId
),
take(1),
map(() => true)
);
}
}
Loading