From d9ba5bd6991bf87ad76db6122c9d5f01ad87dbbf Mon Sep 17 00:00:00 2001 From: Frederik Prijck Date: Mon, 12 Jul 2021 18:36:31 +0000 Subject: [PATCH] [SDK-2635] Avoid emitting error when calling endpoints using allowAnonymous (#180) --- .../src/lib/auth.interceptor.spec.ts | 65 ++++++-- .../auth0-angular/src/lib/auth.interceptor.ts | 60 ++++++-- .../auth0-angular/src/lib/auth.service.ts | 103 +++---------- projects/auth0-angular/src/lib/auth.state.ts | 140 ++++++++++++++++++ 4 files changed, 256 insertions(+), 112 deletions(-) create mode 100644 projects/auth0-angular/src/lib/auth.state.ts diff --git a/projects/auth0-angular/src/lib/auth.interceptor.spec.ts b/projects/auth0-angular/src/lib/auth.interceptor.spec.ts index ebc71402..e5517c14 100644 --- a/projects/auth0-angular/src/lib/auth.interceptor.spec.ts +++ b/projects/auth0-angular/src/lib/auth.interceptor.spec.ts @@ -13,16 +13,19 @@ import { AuthClientConfig, HttpInterceptorConfig, } from './auth.config'; -import { AuthService } from './auth.service'; -import { of, throwError } from 'rxjs'; +import { throwError } from 'rxjs'; +import { Auth0Client } from '@auth0/auth0-spa-js'; +import { Auth0ClientService } from './auth.client'; +import { AuthState } from './auth.state'; // NOTE: Read Async testing: https://github.com/angular/angular/issues/25733#issuecomment-636154553 describe('The Auth HTTP Interceptor', () => { let httpClient: HttpClient; let httpTestingController: HttpTestingController; - let authService: Partial; + let auth0Client: Auth0Client; let req: TestRequest; + let authState: AuthState; const testData: Data = { message: 'Hello, world' }; const assertAuthorizedApiCallTo = ( @@ -50,12 +53,13 @@ describe('The Auth HTTP Interceptor', () => { beforeEach(() => { req = undefined as any; - authService = jasmine.createSpyObj('AuthService', [ - 'getAccessTokenSilently', - ]); - (authService.getAccessTokenSilently as jasmine.Spy).and.returnValue( - of('access-token') - ); + + auth0Client = new Auth0Client({ + domain: '', + client_id: '', + }); + + spyOn(auth0Client, 'getTokenSilently').and.resolveTo('access-token'); config = { httpInterceptor: { @@ -102,7 +106,10 @@ describe('The Auth HTTP Interceptor', () => { useClass: AuthHttpInterceptor, multi: true, }, - { provide: AuthService, useValue: authService }, + { + provide: Auth0ClientService, + useValue: auth0Client, + }, { provide: AuthClientConfig, useValue: { get: () => config }, @@ -112,6 +119,9 @@ describe('The Auth HTTP Interceptor', () => { httpClient = TestBed.inject(HttpClient); httpTestingController = TestBed.inject(HttpTestingController); + authState = TestBed.inject(AuthState); + + spyOn(authState, 'setError').and.callThrough(); }); afterEach(() => { @@ -191,7 +201,7 @@ describe('The Auth HTTP Interceptor', () => { // Testing { uri: /api/addresses } (exact match) assertAuthorizedApiCallTo('https://my-api.com/api/addresses', done); - expect(authService.getAccessTokenSilently).toHaveBeenCalledWith({ + expect(auth0Client.getTokenSilently).toHaveBeenCalledWith({ audience: 'audience', scope: 'scope', }); @@ -224,7 +234,7 @@ describe('The Auth HTTP Interceptor', () => { it('does not execute HTTP call when not able to retrieve a token', fakeAsync(( done: () => void ) => { - (authService.getAccessTokenSilently as jasmine.Spy).and.returnValue( + (auth0Client.getTokenSilently as jasmine.Spy).and.returnValue( throwError({ error: 'login_required' }) ); @@ -239,12 +249,39 @@ describe('The Auth HTTP Interceptor', () => { it('does execute HTTP call when not able to retrieve a token but allowAnonymous is set to true', fakeAsync(( done: () => void ) => { - (authService.getAccessTokenSilently as jasmine.Spy).and.returnValue( + (auth0Client.getTokenSilently as jasmine.Spy).and.returnValue( throwError({ error: 'login_required' }) ); assertPassThruApiCallTo('https://my-api.com/api/orders', done); })); + + it('emit error when not able to retrieve a token but allowAnonymous is set to false', fakeAsync(( + done: () => void + ) => { + (auth0Client.getTokenSilently as jasmine.Spy).and.callFake(() => { + return Promise.reject({ error: 'login_required' }); + }); + + httpClient.request('get', 'https://my-api.com/api/calendar').subscribe({ + error: (err) => expect(err).toEqual({ error: 'login_required' }), + }); + + httpTestingController.expectNone('https://my-api.com/api/calendar'); + flush(); + + expect(authState.setError).toHaveBeenCalled(); + })); + + it('does not emit error when not able to retrieve a token but allowAnonymous is set to true', fakeAsync(() => { + (auth0Client.getTokenSilently as jasmine.Spy).and.callFake(() => { + return Promise.reject({ error: 'login_required' }); + }); + + assertPassThruApiCallTo('https://my-api.com/api/orders', () => { + expect(authState.setError).not.toHaveBeenCalled(); + }); + })); }); describe('Requests that are configured using an uri matcher', () => { @@ -261,7 +298,7 @@ describe('The Auth HTTP Interceptor', () => { // Testing { uriMatcher: (uri) => uri.indexOf('/api/contact') !== -1 } assertAuthorizedApiCallTo('https://my-api.com/api/contact', done, 'post'); - expect(authService.getAccessTokenSilently).toHaveBeenCalledWith({ + expect(auth0Client.getTokenSilently).toHaveBeenCalledWith({ audience: 'audience', scope: 'scope', }); diff --git a/projects/auth0-angular/src/lib/auth.interceptor.ts b/projects/auth0-angular/src/lib/auth.interceptor.ts index c3ecd400..394400ab 100644 --- a/projects/auth0-angular/src/lib/auth.interceptor.ts +++ b/projects/auth0-angular/src/lib/auth.interceptor.ts @@ -6,7 +6,7 @@ import { } from '@angular/common/http'; import { Observable, from, of, iif, throwError } from 'rxjs'; -import { Injectable } from '@angular/core'; +import { Inject, Injectable } from '@angular/core'; import { ApiRouteDefinition, @@ -15,15 +15,24 @@ import { HttpInterceptorConfig, } from './auth.config'; -import { switchMap, first, concatMap, pluck, catchError } from 'rxjs/operators'; -import { AuthService } from './auth.service'; -import { GetTokenSilentlyOptions } from '@auth0/auth0-spa-js'; +import { + switchMap, + first, + concatMap, + pluck, + catchError, + tap, +} from 'rxjs/operators'; +import { Auth0Client, GetTokenSilentlyOptions } from '@auth0/auth0-spa-js'; +import { Auth0ClientService } from './auth.client'; +import { AuthState } from './auth.state'; @Injectable() export class AuthHttpInterceptor implements HttpInterceptor { constructor( private configFactory: AuthClientConfig, - private authService: AuthService + @Inject(Auth0ClientService) private auth0Client: Auth0Client, + private authState: AuthState ) {} intercept( @@ -44,16 +53,19 @@ export class AuthHttpInterceptor implements HttpInterceptor { // outgoing request of(route).pipe( pluck('tokenOptions'), - concatMap>((options) => - this.authService.getAccessTokenSilently(options).pipe( - catchError((err) => { - if (this.allowAnonymous(route, err)) { - return of(''); - } - - return throwError(err); - }) - ) + concatMap>( + (options) => { + return this.getAccessTokenSilently(options).pipe( + catchError((err) => { + if (this.allowAnonymous(route, err)) { + return of(''); + } + + this.authState.setError(err); + return throwError(err); + }) + ); + } ), switchMap((token: string) => { // Clone the request and attach the bearer token @@ -77,6 +89,24 @@ export class AuthHttpInterceptor implements HttpInterceptor { ); } + /** + * Duplicate of AuthService.getAccessTokenSilently, but with a slightly different error handling. + * Only used internally in the interceptor. + * @param options The options for configuring the token fetch. + */ + private getAccessTokenSilently( + options?: GetTokenSilentlyOptions + ): Observable { + return of(this.auth0Client).pipe( + concatMap((client) => client.getTokenSilently(options)), + tap((token) => this.authState.setAccessToken(token)), + catchError((error) => { + this.authState.refresh(); + return throwError(error); + }) + ); + } + /** * Strips the query and fragment from the given uri * @param uri The uri to remove the query and fragment from diff --git a/projects/auth0-angular/src/lib/auth.service.ts b/projects/auth0-angular/src/lib/auth.service.ts index 86ac636b..8e395f3f 100644 --- a/projects/auth0-angular/src/lib/auth.service.ts +++ b/projects/auth0-angular/src/lib/auth.service.ts @@ -15,13 +15,11 @@ import { import { of, from, - BehaviorSubject, Subject, Observable, iif, defer, ReplaySubject, - merge, throwError, } from 'rxjs'; @@ -29,13 +27,10 @@ import { concatMap, tap, map, - filter, takeUntil, distinctUntilChanged, catchError, switchMap, - mergeMap, - scan, withLatestFrom, } from 'rxjs/operators'; @@ -43,15 +38,12 @@ import { Auth0ClientService } from './auth.client'; import { AbstractNavigator } from './abstract-navigator'; import { Location } from '@angular/common'; import { AuthClientConfig } from './auth.config'; +import { AuthState } from './auth.state'; @Injectable({ providedIn: 'root', }) export class AuthService implements OnDestroy { - private isLoadingSubject$ = new BehaviorSubject(true); - private errorSubject$ = new ReplaySubject(1); - private refreshState$ = new Subject(); - private accessToken$ = new ReplaySubject(1); private appStateSubject$ = new ReplaySubject(1); // https://stackoverflow.com/a/41177163 @@ -59,84 +51,28 @@ export class AuthService implements OnDestroy { /** * Emits boolean values indicating the loading state of the SDK. */ - readonly isLoading$ = this.isLoadingSubject$.asObservable(); - - /** - * Trigger used to pull User information from the Auth0Client. - * Triggers when the access token has changed. - */ - private accessTokenTrigger$ = this.accessToken$.pipe( - scan( - ( - acc: { current: string | null; previous: string | null }, - current: string | null - ) => { - return { - previous: acc.current, - current, - }; - }, - { current: null, previous: null } - ), - filter(({ previous, current }) => previous !== current) - ); - - /** - * Trigger used to pull User information from the Auth0Client. - * Triggers when an event occurs that needs to retrigger the User Profile information. - * Events: Login, Access Token change and Logout - */ - private isAuthenticatedTrigger$ = this.isLoading$.pipe( - filter((loading) => !loading), - distinctUntilChanged(), - switchMap(() => - // To track the value of isAuthenticated over time, we need to merge: - // - the current value - // - the value whenever the access token changes. (this should always be true of there is an access token - // but it is safer to pass this through this.auth0Client.isAuthenticated() nevertheless) - // - the value whenever refreshState$ emits - merge( - defer(() => this.auth0Client.isAuthenticated()), - this.accessTokenTrigger$.pipe( - mergeMap(() => this.auth0Client.isAuthenticated()) - ), - this.refreshState$.pipe( - mergeMap(() => this.auth0Client.isAuthenticated()) - ) - ) - ) - ); + readonly isLoading$ = this.authState.isLoading$; /** * Emits boolean values indicating the authentication state of the user. If `true`, it means a user has authenticated. * This depends on the value of `isLoading$`, so there is no need to manually check the loading state of the SDK. */ - readonly isAuthenticated$ = this.isAuthenticatedTrigger$.pipe( - distinctUntilChanged() - ); + readonly isAuthenticated$ = this.authState.isAuthenticated$; /** * Emits details about the authenticated user, or null if not authenticated. */ - readonly user$ = this.isAuthenticatedTrigger$.pipe( - concatMap((authenticated) => - authenticated ? this.auth0Client.getUser() : of(null) - ) - ); + readonly user$ = this.authState.user$; /** * Emits ID token claims when authenticated, or null if not authenticated. */ - readonly idTokenClaims$ = this.isAuthenticatedTrigger$.pipe( - concatMap((authenticated) => - authenticated ? this.auth0Client.getIdTokenClaims() : of(null) - ) - ); + readonly idTokenClaims$ = this.authState.idTokenClaims$; /** * Emits errors that occur during login, or when checking for an active session on startup. */ - readonly error$ = this.errorSubject$.asObservable(); + readonly error$ = this.authState.error$; /** * Emits the value (if any) that was passed to the `loginWithRedirect` method call @@ -148,7 +84,8 @@ export class AuthService implements OnDestroy { @Inject(Auth0ClientService) private auth0Client: Auth0Client, private configFactory: AuthClientConfig, private location: Location, - private navigator: AbstractNavigator + private navigator: AbstractNavigator, + private authState: AuthState ) { const checkSessionOrCallback$ = (isCallback: boolean) => iif( @@ -163,14 +100,14 @@ export class AuthService implements OnDestroy { checkSessionOrCallback$(isCallback).pipe( catchError((error) => { const config = this.configFactory.get(); - this.errorSubject$.next(error); + this.authState.setError(error); this.navigator.navigateByUrl(config.errorPath || '/'); return of(undefined); }) ) ), tap(() => { - this.isLoadingSubject$.next(false); + this.authState.setIsLoading(false); }), takeUntil(this.ngUnsubscribe$) ) @@ -224,7 +161,7 @@ export class AuthService implements OnDestroy { ): Observable { return from( this.auth0Client.loginWithPopup(options, config).then(() => { - this.refreshState$.next(); + this.authState.refresh(); }) ); } @@ -248,7 +185,7 @@ export class AuthService implements OnDestroy { this.auth0Client.logout(options); if (options?.localOnly) { - this.refreshState$.next(); + this.authState.refresh(); } } @@ -284,10 +221,10 @@ export class AuthService implements OnDestroy { ): Observable { return of(this.auth0Client).pipe( concatMap((client) => client.getTokenSilently(options)), - tap((token) => this.accessToken$.next(token)), + tap((token) => this.authState.setAccessToken(token)), catchError((error) => { - this.errorSubject$.next(error); - this.refreshState$.next(); + this.authState.setError(error); + this.authState.refresh(); return throwError(error); }) ); @@ -310,10 +247,10 @@ export class AuthService implements OnDestroy { ): Observable { return of(this.auth0Client).pipe( concatMap((client) => client.getTokenWithPopup(options)), - tap((token) => this.accessToken$.next(token)), + tap((token) => this.authState.setAccessToken(token)), catchError((error) => { - this.errorSubject$.next(error); - this.refreshState$.next(); + this.authState.setError(error); + this.authState.refresh(); return throwError(error); }) ); @@ -335,10 +272,10 @@ export class AuthService implements OnDestroy { */ handleRedirectCallback(url?: string): Observable { return defer(() => this.auth0Client.handleRedirectCallback(url)).pipe( - withLatestFrom(this.isLoadingSubject$), + withLatestFrom(this.authState.isLoading$), tap(([result, isLoading]) => { if (!isLoading) { - this.refreshState$.next(); + this.authState.refresh(); } const appState = result?.appState; const target = appState?.target ?? '/'; diff --git a/projects/auth0-angular/src/lib/auth.state.ts b/projects/auth0-angular/src/lib/auth.state.ts new file mode 100644 index 00000000..dd88bcf9 --- /dev/null +++ b/projects/auth0-angular/src/lib/auth.state.ts @@ -0,0 +1,140 @@ +import { Inject, Injectable } from '@angular/core'; +import { Auth0Client } from '@auth0/auth0-spa-js'; +import { + BehaviorSubject, + defer, + merge, + of, + ReplaySubject, + Subject, +} from 'rxjs'; +import { + concatMap, + distinctUntilChanged, + filter, + mergeMap, + scan, + switchMap, +} from 'rxjs/operators'; +import { Auth0ClientService } from './auth.client'; + +/** + * @ignore + */ +@Injectable({ providedIn: 'root' }) +export class AuthState { + private isLoadingSubject$ = new BehaviorSubject(true); + private refresh$ = new Subject(); + private accessToken$ = new ReplaySubject(1); + private errorSubject$ = new ReplaySubject(1); + + /** + * Emits boolean values indicating the loading state of the SDK. + */ + public readonly isLoading$ = this.isLoadingSubject$.asObservable(); + + /** + * Trigger used to pull User information from the Auth0Client. + * Triggers when the access token has changed. + */ + private accessTokenTrigger$ = this.accessToken$.pipe( + scan( + ( + acc: { current: string | null; previous: string | null }, + current: string | null + ) => { + return { + previous: acc.current, + current, + }; + }, + { current: null, previous: null } + ), + filter(({ previous, current }) => previous !== current) + ); + + /** + * Trigger used to pull User information from the Auth0Client. + * Triggers when an event occurs that needs to retrigger the User Profile information. + * Events: Login, Access Token change and Logout + */ + private readonly isAuthenticatedTrigger$ = this.isLoading$.pipe( + filter((loading) => !loading), + distinctUntilChanged(), + switchMap(() => + // To track the value of isAuthenticated over time, we need to merge: + // - the current value + // - the value whenever the access token changes. (this should always be true of there is an access token + // but it is safer to pass this through this.auth0Client.isAuthenticated() nevertheless) + // - the value whenever refreshState$ emits + merge( + defer(() => this.auth0Client.isAuthenticated()), + this.accessTokenTrigger$.pipe( + mergeMap(() => this.auth0Client.isAuthenticated()) + ), + this.refresh$.pipe(mergeMap(() => this.auth0Client.isAuthenticated())) + ) + ) + ); + + /** + * Emits boolean values indicating the authentication state of the user. If `true`, it means a user has authenticated. + * This depends on the value of `isLoading$`, so there is no need to manually check the loading state of the SDK. + */ + readonly isAuthenticated$ = this.isAuthenticatedTrigger$.pipe( + distinctUntilChanged() + ); + + /** + * Emits details about the authenticated user, or null if not authenticated. + */ + readonly user$ = this.isAuthenticatedTrigger$.pipe( + concatMap((authenticated) => + authenticated ? this.auth0Client.getUser() : of(null) + ) + ); + + /** + * Emits ID token claims when authenticated, or null if not authenticated. + */ + readonly idTokenClaims$ = this.isAuthenticatedTrigger$.pipe( + concatMap((authenticated) => + authenticated ? this.auth0Client.getIdTokenClaims() : of(null) + ) + ); + + /** + * Emits errors that occur during login, or when checking for an active session on startup. + */ + public readonly error$ = this.errorSubject$.asObservable(); + + constructor(@Inject(Auth0ClientService) private auth0Client: Auth0Client) {} + + public setIsLoading(isLoading: boolean): void { + this.isLoadingSubject$.next(isLoading); + } + + /** + * Refresh the state to ensure the `isAuthenticated`, `user$` and `idTokenClaims$` + * reflect the most up-to-date values from Auth0Client. + */ + public refresh(): void { + this.refresh$.next(); + } + + /** + * Update the access token, doing so will also refresh the state. + * @param accessToken The new Access Token + */ + public setAccessToken(accessToken: string): void { + this.accessToken$.next(accessToken); + } + + /** + * Emits the error in the `error$` observable. + * @param error The new error + */ + public setError(error: any): void { + this.errorSubject$.next(error); + } +}