Merge pull request #1580 from damienbod/fabiangosebrink/removing-depr…

…ecated-is-loading-property

Removing deprecated isLoading Property

docs/site/angular-auth-oidc-client/docs/documentation/05-public-events.md

@@ -10,15 +10,21 @@ The library exposes several events which are happening during the runtime. You c
 Currently the following events are supported:
 
 ```ts
-{
-    ConfigLoaded,
-    ConfigLoadingFailed,
-    CheckSessionReceived,
-    UserDataChanged,
-    NewAuthenticationResult,
-    TokenExpired,
-    IdTokenExpired,
-    SilentRenewStarted,
+export enum EventTypes {
+  /**
+   *  This only works in the AppModule Constructor
+   */
+  ConfigLoaded,
+  CheckingAuth,
+  CheckingAuthFinished,
+  CheckingAuthFinishedWithError,
+  ConfigLoadingFailed,
+  CheckSessionReceived,
+  UserDataChanged,
+  NewAuthenticationResult,
+  TokenExpired,
+  IdTokenExpired,
+  SilentRenewStarted,
 }
 ```
 

docs/site/angular-auth-oidc-client/docs/migrations/v14-to-v15.md

@@ -4,8 +4,17 @@ sidebar_position: 5
 
 # Version 14 to 15
 
+## TL;DR
+
+- `enableIdTokenExpiredValidationInRenew` was renamed to `triggerRefreshWhenIdTokenExpired`
+- `isLoading$` property was removed -> use Public Event Service instead
+
 All changes are described below.
 
 ## `enableIdTokenExpiredValidationInRenew` was renamed to `triggerRefreshWhenIdTokenExpired`
 
 The configuration `enableIdTokenExpiredValidationInRenew` was renamed to `triggerRefreshWhenIdTokenExpired` to match its function. The `triggerRefreshWhenIdTokenExpired` parameter can be set to `false` and the renew process with not be triggered by an expired id_token.
+
+## `isLoading$` property was removed -> use Public Event Service instead
+
+The `isLoading$` was marked as deprecated and is removed now. If you want to know when `checkAuth` is finished, use the [PublicEventsService](https://www.angular-auth-oidc-client.com/docs/documentation/public-events) and listen to the Events [CheckingAuth, CheckinfAuthFinished](https://github.com/damienbod/angular-auth-oidc-client/blob/main/projects/angular-auth-oidc-client/src/lib/public-events/event-types.ts#L7-L8)

projects/angular-auth-oidc-client/src/lib/oidc.security.service.spec.ts

@@ -1,6 +1,5 @@
 import { TestBed, waitForAsync } from '@angular/core/testing';
-import { Observable, of, throwError } from 'rxjs';
-import { catchError, switchMap } from 'rxjs/operators';
+import { Observable, of } from 'rxjs';
 import { mockClass } from '../test/auto-mock';
 import { AuthStateService } from './auth-state/auth-state.service';
 import { CheckAuthService } from './auth-state/check-auth.service';
@@ -613,82 +612,4 @@ describe('OidcSecurityService', () => {
       });
     }));
   });
-
-  describe('isLoading$', () => {
-    it('should emit true', waitForAsync(() => {
-      oidcSecurityService.isLoading$.subscribe((x) => expect(x).toBeTrue());
-    }));
-
-    it('should emit false after checkauth is called', waitForAsync(() => {
-      const config = { configId: 'configId1' };
-
-      spyOn(configurationService, 'getOpenIDConfigurations').and.returnValue(of({ allConfigs: [config], currentConfig: config }));
-      spyOn(checkAuthService, 'checkAuth').and.returnValue(of(null));
-
-      oidcSecurityService
-        .checkAuth()
-        .pipe(switchMap(() => oidcSecurityService.isLoading$))
-        .subscribe((x) => expect(x).toBeFalse());
-    }));
-
-    it('should emit false on error in checkauth', waitForAsync(() => {
-      const config = { configId: 'configId1' };
-
-      spyOn(configurationService, 'getOpenIDConfigurations').and.returnValue(of({ allConfigs: [config], currentConfig: config }));
-      spyOn(checkAuthService, 'checkAuth').and.returnValue(throwError(() => new Error('Error')));
-
-      oidcSecurityService
-        .checkAuth()
-        .pipe(catchError(() => oidcSecurityService.isLoading$))
-        .subscribe((x) => expect(x).toBeFalse());
-    }));
-
-    it('should emit false after checkauthMultiple is called', waitForAsync(() => {
-      const config = { configId: 'configId1' };
-
-      spyOn(configurationService, 'getOpenIDConfigurations').and.returnValue(of({ allConfigs: [config], currentConfig: config }));
-      spyOn(checkAuthService, 'checkAuthMultiple').and.returnValue(of(null));
-
-      oidcSecurityService
-        .checkAuthMultiple()
-        .pipe(switchMap(() => oidcSecurityService.isLoading$))
-        .subscribe((x) => expect(x).toBeFalse());
-    }));
-
-    it('should emit false on error in checkauthMultiple', waitForAsync(() => {
-      const config = { configId: 'configId1' };
-
-      spyOn(configurationService, 'getOpenIDConfigurations').and.returnValue(of({ allConfigs: [config], currentConfig: config }));
-      spyOn(checkAuthService, 'checkAuthMultiple').and.returnValue(throwError(() => new Error('Error')));
-
-      oidcSecurityService
-        .checkAuthMultiple()
-        .pipe(catchError(() => oidcSecurityService.isLoading$))
-        .subscribe((x) => expect(x).toBeFalse());
-    }));
-
-    it('should emit false after checkAuthIncludingServer is called', waitForAsync(() => {
-      const config = { configId: 'configId1' };
-
-      spyOn(configurationService, 'getOpenIDConfigurations').and.returnValue(of({ allConfigs: [config], currentConfig: config }));
-      spyOn(checkAuthService, 'checkAuthIncludingServer').and.returnValue(of(null));
-
-      oidcSecurityService
-        .checkAuthIncludingServer()
-        .pipe(switchMap(() => oidcSecurityService.isLoading$))
-        .subscribe((x) => expect(x).toBeFalse());
-    }));
-
-    it('should emit false on error in checkAuthIncludingServer', waitForAsync(() => {
-      const config = { configId: 'configId1' };
-
-      spyOn(configurationService, 'getOpenIDConfigurations').and.returnValue(of({ allConfigs: [config], currentConfig: config }));
-      spyOn(checkAuthService, 'checkAuthIncludingServer').and.returnValue(throwError(() => new Error('Error')));
-
-      oidcSecurityService
-        .checkAuthIncludingServer()
-        .pipe(catchError(() => oidcSecurityService.isLoading$))
-        .subscribe((x) => expect(x).toBeFalse());
-    }));
-  });
 });

projects/angular-auth-oidc-client/src/lib/oidc.security.service.ts

@@ -1,6 +1,6 @@
 import { Injectable } from '@angular/core';
-import { BehaviorSubject, Observable, throwError } from 'rxjs';
-import { catchError, map, switchMap, tap } from 'rxjs/operators';
+import { Observable } from 'rxjs';
+import { map, switchMap } from 'rxjs/operators';
 import { AuthOptions } from './auth-options';
 import { AuthenticatedResult } from './auth-state/auth-result';
 import { AuthStateService } from './auth-state/auth-state.service';
@@ -62,16 +62,6 @@ export class OidcSecurityService {
     return this.callbackService.stsCallback$;
   }
 
-  /**
-   * @deprecated This property should not be used. Please use the `PublicEventsService` instead. This property is removed in future versions
-   * Emits false when the observable, returned by one of the checkAuth() methods, emits a value, or errors. Initial value: true.
-   */
-  get isLoading$(): Observable<boolean> {
-    return this.isLoading.asObservable();
-  }
-
-  private readonly isLoading: BehaviorSubject<boolean> = new BehaviorSubject(true);
-
   constructor(
     private readonly checkSessionService: CheckSessionService,
     private readonly checkAuthService: CheckAuthService,
@@ -132,11 +122,9 @@ export class OidcSecurityService {
    * @returns An object `LoginResponse` containing all information about the login
    */
   checkAuth(url?: string, configId?: string): Observable<LoginResponse> {
-    return this.configurationService.getOpenIDConfigurations(configId).pipe(
-      switchMap(({ allConfigs, currentConfig }) => this.checkAuthService.checkAuth(currentConfig, allConfigs, url)),
-      tap(this.finishLoading),
-      catchError(this.finishLoadingOnError)
-    );
+    return this.configurationService
+      .getOpenIDConfigurations(configId)
+      .pipe(switchMap(({ allConfigs, currentConfig }) => this.checkAuthService.checkAuth(currentConfig, allConfigs, url)));
   }
 
   /**
@@ -152,11 +140,9 @@ export class OidcSecurityService {
    * @returns An array of `LoginResponse` objects containing all information about the logins
    */
   checkAuthMultiple(url?: string): Observable<LoginResponse[]> {
-    return this.configurationService.getOpenIDConfigurations().pipe(
-      switchMap(({ allConfigs }) => this.checkAuthService.checkAuthMultiple(allConfigs, url)),
-      tap(this.finishLoading),
-      catchError(this.finishLoadingOnError)
-    );
+    return this.configurationService
+      .getOpenIDConfigurations()
+      .pipe(switchMap(({ allConfigs }) => this.checkAuthService.checkAuthMultiple(allConfigs, url)));
   }
 
   /**
@@ -174,11 +160,9 @@ export class OidcSecurityService {
    * Checks the server for an authenticated session using the iframe silent renew if not locally authenticated.
    */
   checkAuthIncludingServer(configId?: string): Observable<LoginResponse> {
-    return this.configurationService.getOpenIDConfigurations(configId).pipe(
-      switchMap(({ allConfigs, currentConfig }) => this.checkAuthService.checkAuthIncludingServer(currentConfig, allConfigs)),
-      tap(this.finishLoading),
-      catchError(this.finishLoadingOnError)
-    );
+    return this.configurationService
+      .getOpenIDConfigurations(configId)
+      .pipe(switchMap(({ allConfigs, currentConfig }) => this.checkAuthService.checkAuthIncludingServer(currentConfig, allConfigs)));
   }
 
   /**
@@ -447,14 +431,4 @@ export class OidcSecurityService {
       .getOpenIDConfiguration(configId)
       .pipe(switchMap((config) => this.urlService.getAuthorizeUrl(config, customParams ? { customParams } : undefined)));
   }
-
-  private readonly finishLoading = (): void => {
-    this.isLoading.next(false);
-  };
-
-  private readonly finishLoadingOnError = (err: any): Observable<never> => {
-    this.isLoading.next(false);
-
-    return throwError(() => err);
-  };
 }

projects/angular-auth-oidc-client/src/lib/validation/state-validation.service.spec.ts

@@ -21,6 +21,7 @@ describe('State Validation Service', () => {
   let config: OpenIdConfiguration;
   let authWellKnownEndpoints: AuthWellKnownEndpoints;
   let storagePersistenceService: StoragePersistenceService;
+  let flowHelper: FlowHelper;
 
   beforeEach(() => {
     TestBed.configureTestingModule({
@@ -52,6 +53,7 @@ describe('State Validation Service', () => {
     tokenHelperService = TestBed.inject(TokenHelperService);
     loggerService = TestBed.inject(LoggerService);
     storagePersistenceService = TestBed.inject(StoragePersistenceService);
+    flowHelper = TestBed.inject(FlowHelper);
   });
 
   beforeEach(() => {
@@ -1502,5 +1504,92 @@ describe('State Validation Service', () => {
         expect(state.authResponseIsValid).toBe(true);
       });
     }));
+
+    it('should return OK if disableIdTokenValidation is true', waitForAsync(() => {
+      spyOn(tokenValidationService, 'validateStateFromHashCallback').and.returnValue(true);
+      spyOn(flowHelper, 'isCurrentFlowImplicitFlowWithAccessToken').and.returnValue(false);
+      spyOn(flowHelper, 'isCurrentFlowCodeFlow').and.returnValue(false);
+
+      config.responseType = 'id_token token';
+      config.maxIdTokenIatOffsetAllowedInSeconds = 0;
+      config.disableIdTokenValidation = true;
+
+      const callbackContext = {
+        code: 'fdffsdfsdf',
+        refreshToken: null,
+        state: 'fdffsggggggdfsdf',
+        sessionState: 'fdffsggggggdfsdf',
+        existingIdToken: null,
+        authResult: {},
+        isRenewProcess: false,
+        jwtKeys: null,
+        validationResult: null,
+      };
+
+      const isValidObs$ = stateValidationService.getValidatedStateResult(callbackContext, config);
+
+      isValidObs$.subscribe((isValid) => {
+        expect(isValid.state).toBe(ValidationResult.Ok);
+        expect(isValid.authResponseIsValid).toBe(false);
+      });
+    }));
+
+    it('should return OK if disableIdTokenValidation is true', waitForAsync(() => {
+      spyOn(tokenValidationService, 'validateStateFromHashCallback').and.returnValue(true);
+      spyOn(flowHelper, 'isCurrentFlowImplicitFlowWithAccessToken').and.returnValue(false);
+      spyOn(flowHelper, 'isCurrentFlowCodeFlow').and.returnValue(false);
+
+      config.responseType = 'id_token token';
+      config.maxIdTokenIatOffsetAllowedInSeconds = 0;
+      config.disableIdTokenValidation = true;
+
+      const callbackContext = {
+        code: 'fdffsdfsdf',
+        refreshToken: null,
+        state: 'fdffsggggggdfsdf',
+        sessionState: 'fdffsggggggdfsdf',
+        existingIdToken: null,
+        authResult: {},
+        isRenewProcess: false,
+        jwtKeys: null,
+        validationResult: null,
+      };
+
+      const isValidObs$ = stateValidationService.getValidatedStateResult(callbackContext, config);
+
+      isValidObs$.subscribe((isValid) => {
+        expect(isValid.state).toBe(ValidationResult.Ok);
+        expect(isValid.authResponseIsValid).toBe(false);
+      });
+    }));
+
+    it('should return OK if disableIdTokenValidation is false but inrefreshtokenflow and no id token is returned', waitForAsync(() => {
+      spyOn(tokenValidationService, 'validateStateFromHashCallback').and.returnValue(true);
+      spyOn(flowHelper, 'isCurrentFlowImplicitFlowWithAccessToken').and.returnValue(false);
+      spyOn(flowHelper, 'isCurrentFlowCodeFlow').and.returnValue(false);
+
+      config.responseType = 'id_token token';
+      config.maxIdTokenIatOffsetAllowedInSeconds = 0;
+      config.disableIdTokenValidation = false;
+
+      const callbackContext = {
+        code: 'fdffsdfsdf',
+        refreshToken: 'something',
+        state: 'fdffsggggggdfsdf',
+        sessionState: 'fdffsggggggdfsdf',
+        existingIdToken: null,
+        authResult: {},
+        isRenewProcess: true,
+        jwtKeys: null,
+        validationResult: null,
+      };
+
+      const isValidObs$ = stateValidationService.getValidatedStateResult(callbackContext, config);
+
+      isValidObs$.subscribe((isValid) => {
+        expect(isValid.state).toBe(ValidationResult.Ok);
+        expect(isValid.authResponseIsValid).toBe(false);
+      });
+    }));
   });
 });