Skip to content

Commit

Permalink
Merge pull request #942 from Expelz/bug/app-fully-hang-during-silent-…
Browse files Browse the repository at this point in the history 
…renew

Bug. App fully hang during silent renew.
  • Loading branch information
@damienbod
damienbod authored Jan 22, 2021
2 parents 96f20e2 + 47f4cfc commit bcdf00d
Show file tree
Hide file tree
Showing 9 changed files with 207 additions and 9 deletions.
1 change: 1 addition & 0 deletions docs/configuration.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ In this document are all the values which can be set to configure this library.
| `startCheckSession` | `boolean` | Starts the OpenID session management for this client. | No |
| `silentRenew` | `boolean` | Renews the client tokens, once the token_id expires. Can use the iframes, or the refresh tokens | No |
| `silentRenewUrl` | `string` | URL which can be used for a lightweight renew callback. See silent renew. | No |
| `silentRenewTimeoutInSeconds` | `number` | Sets the maximum waiting time for silent renew process. If this time is exceeded, the silent renew state will be reset. Default = <em>20</em>. | No |
| `renewTimeBeforeTokenExpiresInSeconds` | `number` | Makes it possible to add an offset to the silent renew check in seconds. By entering a value, you can renew the tokens, before the tokens expire. | No |
| `useRefreshToken` | `boolean` | boolean property set to false. Standard silent renew mode used per default. Refresh tokens can be activated. | No |
| `ignoreNonceAfterRefresh` | `boolean` | A token obtained by using a refresh token normally doesn't contain a nonce value. The library checks it is not there. <br> However some oidc endpoint implementations do send one. Setting ignore_nonce_after_refresh to true disables the check if a nonce is present. <br> Please note that the nonce value, if present, will not be verified. Default is false. | No |
Expand Down
62 changes: 60 additions & 2 deletions projects/angular-auth-oidc-client/src/lib/callback/refresh-session.service.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { HttpClientModule } from '@angular/common/http';
import { TestBed, waitForAsync } from '@angular/core/testing';
import { fakeAsync, TestBed, tick, waitForAsync } from '@angular/core/testing';
import { RouterTestingModule } from '@angular/router/testing';
import { of } from 'rxjs';
import { of, throwError, TimeoutError } from 'rxjs';
import { AuthStateService } from '../authState/auth-state.service';
import { AuthStateServiceMock } from '../authState/auth-state.service-mock';
import { AuthWellKnownService } from '../config/auth-well-known.service';
Expand Down Expand Up @@ -81,6 +81,7 @@ describe('RefreshSessionService ', () => {
spyOn(flowHelper, 'isCurrentFlowCodeFlowWithRefreshTokens').and.returnValue(true);
spyOn(refreshSessionService as any, 'startRefreshSession').and.returnValue(of(null));
spyOn(authStateService, 'areAuthStorageTokensValid').and.returnValue(true);
spyOnProperty(configurationProvider, 'openIDConfiguration').and.returnValue({ silentRenewTimeoutInSeconds: 10 });

refreshSessionService.forceRefreshSession().subscribe((result) => {
expect(result.idToken).not.toBeUndefined();
Expand All @@ -95,6 +96,7 @@ describe('RefreshSessionService ', () => {
spyOn(flowHelper, 'isCurrentFlowCodeFlowWithRefreshTokens').and.returnValue(true);
spyOn(refreshSessionService as any, 'startRefreshSession').and.returnValue(of(null));
spyOn(authStateService, 'areAuthStorageTokensValid').and.returnValue(false);
spyOnProperty(configurationProvider, 'openIDConfiguration').and.returnValue({ silentRenewTimeoutInSeconds: 10 });

refreshSessionService.forceRefreshSession().subscribe((result) => {
expect(result).toBeNull();
Expand All @@ -108,6 +110,7 @@ describe('RefreshSessionService ', () => {
spyOn(flowHelper, 'isCurrentFlowCodeFlowWithRefreshTokens').and.returnValue(false);
spyOn(refreshSessionService as any, 'startRefreshSession').and.returnValue(of(null));
spyOn(authStateService, 'areAuthStorageTokensValid').and.returnValue(true);
spyOnProperty(configurationProvider, 'openIDConfiguration').and.returnValue({ silentRenewTimeoutInSeconds: 10 });

refreshSessionService.forceRefreshSession().subscribe((result) => {
expect(result.idToken).toBeDefined();
Expand All @@ -126,6 +129,7 @@ describe('RefreshSessionService ', () => {
spyOn(flowHelper, 'isCurrentFlowCodeFlowWithRefreshTokens').and.returnValue(false);
spyOn(refreshSessionService as any, 'startRefreshSession').and.returnValue(of(null));
spyOn(authStateService, 'areAuthStorageTokensValid').and.returnValue(false);
spyOnProperty(configurationProvider, 'openIDConfiguration').and.returnValue({ silentRenewTimeoutInSeconds: 10 });

refreshSessionService.forceRefreshSession().subscribe((result) => {
expect(result).toBeNull();
Expand All @@ -137,13 +141,66 @@ describe('RefreshSessionService ', () => {
})
);

it('occurs timeout error and retry mechanism exhausted max retry count throws error', fakeAsync(() => {
const openIDConfiguration = {
silentRenewTimeoutInSeconds: 10,
};

spyOn(flowHelper, 'isCurrentFlowCodeFlowWithRefreshTokens').and.returnValue(false);
spyOn(refreshSessionService as any, 'startRefreshSession').and.returnValue(of(null));
spyOn(authStateService, 'areAuthStorageTokensValid').and.returnValue(false);
spyOnProperty(configurationProvider, 'openIDConfiguration').and.returnValue(openIDConfiguration);

const resetSilentRenewRunningSpy = spyOn(flowsDataService, 'resetSilentRenewRunning');
const expectedInvokeCount = (refreshSessionService as any).MAX_RETRY_ATTEMPTS;

refreshSessionService.forceRefreshSession().subscribe(
() => {
fail('It should not return any result.');
},
(error) => {
expect(error).toBeInstanceOf(TimeoutError);
expect(resetSilentRenewRunningSpy).toHaveBeenCalledTimes(expectedInvokeCount);
}
);

tick(openIDConfiguration.silentRenewTimeoutInSeconds * 10000);
}));

it('occurs unknown error throws it to subscriber', fakeAsync(() => {
const openIDConfiguration = {
silentRenewTimeoutInSeconds: 10,
};

const expectedErrorMessage = 'Test error message';

spyOn(flowHelper, 'isCurrentFlowCodeFlowWithRefreshTokens').and.returnValue(false);
spyOn(refreshSessionService as any, 'startRefreshSession').and.returnValue(throwError(new Error(expectedErrorMessage)));
spyOn(authStateService, 'areAuthStorageTokensValid').and.returnValue(false);
spyOnProperty(configurationProvider, 'openIDConfiguration').and.returnValue(openIDConfiguration);

const resetSilentRenewRunningSpy = spyOn(flowsDataService, 'resetSilentRenewRunning');

refreshSessionService.forceRefreshSession().subscribe(
() => {
fail('It should not return any result.');
},
(error) => {
expect(error).toBeInstanceOf(Error);
expect((error as Error).message === expectedErrorMessage).toBeTruthy();
expect(resetSilentRenewRunningSpy).not.toHaveBeenCalled();
}
);
}));

describe('NOT isCurrentFlowCodeFlowWithRefreshTokens', () => {
it(
'does return null when not authenticated',
waitForAsync(() => {
spyOn(flowHelper, 'isCurrentFlowCodeFlowWithRefreshTokens').and.returnValue(false);
spyOn(refreshSessionService as any, 'startRefreshSession').and.returnValue(of(null));
spyOn(authStateService, 'areAuthStorageTokensValid').and.returnValue(false);
spyOnProperty(configurationProvider, 'openIDConfiguration').and.returnValue({ silentRenewTimeoutInSeconds: 10 });

refreshSessionService.forceRefreshSession().subscribe((result) => {
expect(result).toBeNull();
Expand All @@ -160,6 +217,7 @@ describe('RefreshSessionService ', () => {
waitForAsync(() => {
spyOn(flowHelper, 'isCurrentFlowCodeFlowWithRefreshTokens').and.returnValue(false);
spyOn(refreshSessionService as any, 'startRefreshSession').and.returnValue(of(null));
spyOnProperty(configurationProvider, 'openIDConfiguration').and.returnValue({ silentRenewTimeoutInSeconds: 10 });
const spyInsideMap = spyOn(authStateService, 'areAuthStorageTokensValid').and.returnValue(true);

refreshSessionService
Expand Down
26 changes: 24 additions & 2 deletions projects/angular-auth-oidc-client/src/lib/callback/refresh-session.service.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Injectable } from '@angular/core';
import { forkJoin, of } from 'rxjs';
import { map, switchMap, take } from 'rxjs/operators';
import { forkJoin, Observable, of, throwError, TimeoutError, timer } from 'rxjs';
import { map, mergeMap, retryWhen, switchMap, take, timeout } from 'rxjs/operators';
import { AuthStateService } from '../authState/auth-state.service';
import { AuthWellKnownService } from '../config/auth-well-known.service';
import { ConfigurationProvider } from '../config/config.provider';
Expand All @@ -13,6 +13,8 @@ import { RefreshSessionRefreshTokenService } from './refresh-session-refresh-tok

@Injectable({ providedIn: 'root' })
export class RefreshSessionService {
private MAX_RETRY_ATTEMPTS = 3;

constructor(
private flowHelper: FlowHelper,
private configurationProvider: ConfigurationProvider,
Expand Down Expand Up @@ -43,6 +45,8 @@ export class RefreshSessionService {
}

return forkJoin([this.startRefreshSession(), this.silentRenewService.refreshSessionWithIFrameCompleted$.pipe(take(1))]).pipe(
timeout(this.configurationProvider.openIDConfiguration.silentRenewTimeoutInSeconds * 1000),
retryWhen(this.TimeoutRetryStrategy.bind(this)),
map(([_, callbackContext]) => {
const isAuthenticated = this.authStateService.areAuthStorageTokensValid();
if (isAuthenticated) {
Expand Down Expand Up @@ -85,4 +89,22 @@ export class RefreshSessionService {
})
);
}

private TimeoutRetryStrategy(errorAttempts: Observable<any>) {
return errorAttempts.pipe(
mergeMap((error, index) => {
const scalingDuration = 1000;
const currentAttempt = index + 1;

if (!(error instanceof TimeoutError) || currentAttempt > this.MAX_RETRY_ATTEMPTS) {
return throwError(error);
}

this.loggerService.logDebug(`forceRefreshSession timeout. Attempt #${currentAttempt}`);

this.flowsDataService.resetSilentRenewRunning();
return timer(currentAttempt * scalingDuration);
})
);
}
}
3 changes: 3 additions & 0 deletions projects/angular-auth-oidc-client/src/lib/config/config.provider.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ describe('ConfigurationProviderTests', () => {
startCheckSession: false,
silentRenew: false,
silentRenewUrl: 'https://please_set',
silentRenewTimeoutInSeconds: 20,
renewTimeBeforeTokenExpiresInSeconds: 0,
useRefreshToken: false,
ignoreNonceAfterRefresh: false,
Expand Down Expand Up @@ -97,6 +98,7 @@ describe('ConfigurationProviderTests', () => {
startCheckSession: false,
silentRenew: false,
silentRenewUrl: 'https://please_set',
silentRenewTimeoutInSeconds: 20,
renewTimeBeforeTokenExpiresInSeconds: 0,
useRefreshToken: false,
ignoreNonceAfterRefresh: false,
Expand Down Expand Up @@ -142,6 +144,7 @@ describe('ConfigurationProviderTests', () => {
startCheckSession: false,
silentRenew: false,
silentRenewUrl: 'https://please_set',
silentRenewTimeoutInSeconds: 20,
renewTimeBeforeTokenExpiresInSeconds: 0,
useRefreshToken: false,
ignoreNonceAfterRefresh: false,
Expand Down
1 change: 1 addition & 0 deletions projects/angular-auth-oidc-client/src/lib/config/default-config.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const DEFAULT_CONFIG: OpenIdConfiguration = {
startCheckSession: false,
silentRenew: false,
silentRenewUrl: 'https://please_set',
silentRenewTimeoutInSeconds: 20,
renewTimeBeforeTokenExpiresInSeconds: 0,
useRefreshToken: false,
ignoreNonceAfterRefresh: false,
Expand Down
1 change: 1 addition & 0 deletions projects/angular-auth-oidc-client/src/lib/config/openid-configuration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export interface OpenIdConfiguration {
startCheckSession?: boolean;
silentRenew?: boolean;
silentRenewUrl?: string;
silentRenewTimeoutInSeconds?: number;
renewTimeBeforeTokenExpiresInSeconds?: number;
useRefreshToken?: boolean;
ignoreNonceAfterRefresh?: boolean;
Expand Down
Loading

0 comments on commit bcdf00d

Please sign in to comment.