Merge pull request #990 from damienbod/damien/bugfix-autouserinfo-che…

…cksession

Bugfix Set session state is set when the auto info is false

CHANGELOG.md

@@ -1,5 +1,10 @@
 ## Angular Lib for OpenID Connect/OAuth2 Changelog
 
+### 2021-02-28 Version 11.6.2
+
+- Bugfix: Check session does not work when autoUserinfo is set to false in code flow with PKCE
+  - [PR](https://github.com/damienbod/angular-auth-oidc-client/pull/990), [ISSUE](https://github.com/damienbod/angular-auth-oidc-client/pull/864)
+
 ### 2021-02-27 Version 11.6.1
 
 - Added AutoLoginGuard

package.json

@@ -8,7 +8,7 @@
   "bugs": {
     "url": "https://github.com/damienbod/angular-auth-oidc-client/issues"
   },
-  "version": "11.6.1",
+  "version": "11.6.2",
   "scripts": {
     "ng": "ng",
     "build": "npm run build-lib",

projects/angular-auth-oidc-client/package.json

@@ -37,7 +37,7 @@
     "authorization"
   ],
   "license": "MIT",
-  "version": "11.6.1",
+  "version": "11.6.2",
   "description": "Angular Lib for OpenID Connect & OAuth2",
   "schematics": "./schematics/collection.json"
 }

projects/angular-auth-oidc-client/src/lib/flows/callback-handling/user-callback-handler.service.spec.ts

@@ -1,4 +1,4 @@
-import { TestBed } from '@angular/core/testing';
+import { TestBed, waitForAsync } from '@angular/core/testing';
 import { AuthStateService } from '../../authState/auth-state.service';
 import { AuthStateServiceMock } from '../../authState/auth-state.service-mock';
 import { ConfigurationProvider } from '../../config/config.provider';
@@ -7,6 +7,7 @@ import { LoggerService } from '../../logging/logger.service';
 import { LoggerServiceMock } from '../../logging/logger.service-mock';
 import { UserService } from '../../userData/user-service';
 import { UserServiceMock } from '../../userData/user-service-mock';
+import { StateValidationResult } from '../../validation/state-validation-result';
 import { FlowsDataService } from '../flows-data.service';
 import { FlowsDataServiceMock } from '../flows-data.service-mock';
 import { ResetAuthDataService } from '../reset-auth-data.service';
@@ -16,9 +17,9 @@ import { UserCallbackHandlerService } from './user-callback-handler.service';
 describe('UserCallbackHandlerService', () => {
   let service: UserCallbackHandlerService;
   // let loggerService: LoggerService;
-  // let configurationProvider: ConfigurationProvider;
-  // let authStateService: AuthStateService;
-  // let flowsDataService: FlowsDataService;
+  let configurationProvider: ConfigurationProvider;
+  let authStateService: AuthStateService;
+  let flowsDataService: FlowsDataService;
   // let userService: UserService;
   // let resetAuthDataService: ResetAuthDataService;
 
@@ -38,9 +39,9 @@ describe('UserCallbackHandlerService', () => {
 
   beforeEach(() => {
     service = TestBed.inject(UserCallbackHandlerService);
-    // configurationProvider = TestBed.inject(ConfigurationProvider);
-    // flowsDataService = TestBed.inject(FlowsDataService);
-    // authStateService = TestBed.inject(AuthStateService);
+    configurationProvider = TestBed.inject(ConfigurationProvider);
+    flowsDataService = TestBed.inject(FlowsDataService);
+    authStateService = TestBed.inject(AuthStateService);
     // loggerService = TestBed.inject(LoggerService);
     // userService = TestBed.inject(UserService);
     // resetAuthDataService = TestBed.inject(ResetAuthDataService);
@@ -49,4 +50,114 @@ describe('UserCallbackHandlerService', () => {
   it('should create', () => {
     expect(service).toBeTruthy();
   });
+
+  describe('callbackUser', () => {
+    it(
+      'calls flowsDataService.setSessionState with correct params if autoUserInfo is false, isRenewProcess is false and refreshToken is null',
+      waitForAsync(() => {
+        const svr = new StateValidationResult('accesstoken', 'idtoken', true, 'decoded');
+        const callbackContext = {
+          code: null,
+          refreshToken: null,
+          state: null,
+          sessionState: null,
+          authResult: { session_state: 'mystate' },
+          isRenewProcess: false,
+          jwtKeys: null,
+          validationResult: svr,
+          existingIdToken: null,
+        };
+
+        spyOnProperty(configurationProvider, 'openIDConfiguration').and.returnValue({ autoUserinfo: false });
+        const spy = spyOn(flowsDataService, 'setSessionState');
+
+        service.callbackUser(callbackContext).subscribe((resultCallbackContext) => {
+          expect(spy).toHaveBeenCalledOnceWith('mystate');
+          expect(resultCallbackContext).toEqual(callbackContext);
+        });
+      })
+    );
+
+    it(
+      'does NOT call flowsDataService.setSessionState if autoUserInfo is false, isRenewProcess is true and refreshToken is null',
+      waitForAsync(() => {
+        const svr = new StateValidationResult('accesstoken', 'idtoken', true, 'decoded');
+        const callbackContext = {
+          code: null,
+          refreshToken: null,
+          state: null,
+          sessionState: null,
+          authResult: { session_state: 'mystate' },
+          isRenewProcess: true,
+          jwtKeys: null,
+          validationResult: svr,
+          existingIdToken: null,
+        };
+
+        spyOnProperty(configurationProvider, 'openIDConfiguration').and.returnValue({ autoUserinfo: false });
+        const spy = spyOn(flowsDataService, 'setSessionState');
+
+        service.callbackUser(callbackContext).subscribe((resultCallbackContext) => {
+          expect(spy).not.toHaveBeenCalled();
+          expect(resultCallbackContext).toEqual(callbackContext);
+        });
+      })
+    );
+
+    it(
+      'does NOT call flowsDataService.setSessionState if autoUserInfo is false isRenewProcess is false, refreshToken has value',
+      waitForAsync(() => {
+        const svr = new StateValidationResult('accesstoken', 'idtoken', true, 'decoded');
+        const callbackContext = {
+          code: null,
+          refreshToken: 'somerefreshtoken',
+          state: null,
+          sessionState: null,
+          authResult: { session_state: 'mystate' },
+          isRenewProcess: false,
+          jwtKeys: null,
+          validationResult: svr,
+          existingIdToken: null,
+        };
+
+        spyOnProperty(configurationProvider, 'openIDConfiguration').and.returnValue({ autoUserinfo: false });
+        const spy = spyOn(flowsDataService, 'setSessionState');
+
+        service.callbackUser(callbackContext).subscribe((resultCallbackContext) => {
+          expect(spy).not.toHaveBeenCalled();
+          expect(resultCallbackContext).toEqual(callbackContext);
+        });
+      })
+    );
+
+    it(
+      'calls authStateService.updateAndPublishAuthState with correct params if autoUserInfo is false',
+      waitForAsync(() => {
+        const svr = new StateValidationResult('accesstoken', 'idtoken', true, 'decoded');
+        const callbackContext = {
+          code: null,
+          refreshToken: 'somerefreshtoken',
+          state: null,
+          sessionState: null,
+          authResult: { session_state: 'mystate' },
+          isRenewProcess: false,
+          jwtKeys: null,
+          validationResult: svr,
+          existingIdToken: null,
+        };
+
+        spyOnProperty(configurationProvider, 'openIDConfiguration').and.returnValue({ autoUserinfo: false });
+        const updateAndPublishAuthStateSpy = spyOn(authStateService, 'updateAndPublishAuthState');
+
+        service.callbackUser(callbackContext).subscribe((resultCallbackContext) => {
+          expect(updateAndPublishAuthStateSpy).toHaveBeenCalledOnceWith({
+            authorizationState: 'Authorized',
+            validationResult: 'NotSet',
+            isRenewProcess: false,
+          });
+          expect(resultCallbackContext).toEqual(callbackContext);
+        });
+      })
+    );
+  });
 });

projects/angular-auth-oidc-client/src/lib/flows/callback-handling/user-callback-handler.service.ts

@@ -24,44 +24,45 @@ export class UserCallbackHandlerService {
 
   // STEP 5 userData
   callbackUser(callbackContext: CallbackContext): Observable<CallbackContext> {
+    const { isRenewProcess, validationResult, authResult, refreshToken } = callbackContext;
+
     if (!this.configurationProvider.openIDConfiguration.autoUserinfo) {
-      if (!callbackContext.isRenewProcess) {
+      if (!isRenewProcess) {
         // userData is set to the id_token decoded, auto get user data set to false
-        this.userService.setUserDataToStore(callbackContext.validationResult.decodedIdToken);
+        this.userService.setUserDataToStore(validationResult.decodedIdToken);
+        if (!refreshToken) {
+          this.flowsDataService.setSessionState(authResult.session_state);
+        }
       }
 
-      this.publishAuthorizedState(callbackContext.validationResult, callbackContext.isRenewProcess);
+      this.publishAuthorizedState(validationResult, isRenewProcess);
       return of(callbackContext);
     }
 
-    return this.userService
-      .getAndPersistUserDataInStore(
-        callbackContext.isRenewProcess,
-        callbackContext.validationResult.idToken,
-        callbackContext.validationResult.decodedIdToken
-      )
-      .pipe(
-        switchMap((userData) => {
-          if (!!userData) {
-            if (!callbackContext.refreshToken) {
-              this.flowsDataService.setSessionState(callbackContext.authResult.session_state);
-            }
-            this.publishAuthorizedState(callbackContext.validationResult, callbackContext.isRenewProcess);
-            return of(callbackContext);
-          } else {
-            this.resetAuthDataService.resetAuthorizationData();
-            this.publishUnauthorizedState(callbackContext.validationResult, callbackContext.isRenewProcess);
-            const errorMessage = `Called for userData but they were ${userData}`;
-            this.loggerService.logWarning(errorMessage);
-            return throwError(errorMessage);
+    return this.userService.getAndPersistUserDataInStore(isRenewProcess, validationResult.idToken, validationResult.decodedIdToken).pipe(
+      switchMap((userData) => {
+        if (!!userData) {
+          if (!refreshToken) {
+            this.flowsDataService.setSessionState(authResult.session_state);
           }
-        }),
-        catchError((err) => {
-          const errorMessage = `Failed to retrieve user info with error:  ${err}`;
+
+          this.publishAuthorizedState(validationResult, isRenewProcess);
+
+          return of(callbackContext);
+        } else {
+          this.resetAuthDataService.resetAuthorizationData();
+          this.publishUnauthorizedState(validationResult, isRenewProcess);
+          const errorMessage = `Called for userData but they were ${userData}`;
           this.loggerService.logWarning(errorMessage);
           return throwError(errorMessage);
-        })
-      );
+        }
+      }),
+      catchError((err) => {
+        const errorMessage = `Failed to retrieve user info with error:  ${err}`;
+        this.loggerService.logWarning(errorMessage);
+        return throwError(errorMessage);
+      })
+    );
   }
 
   private publishAuthorizedState(stateValidationResult: StateValidationResult, isRenewProcess: boolean) {

projects/schematics/package.json

@@ -1,6 +1,6 @@
 {
   "name": "schematics",
-  "version": "11.6.1",
+  "version": "11.6.2",
   "description": "A schematic for the Angular Lib for OpenID Connect & OAuth2",
   "scripts": {
     "build": "tsc -p tsconfig.json",

projects/schematics/src/ng-add/actions/add-dependencies.ts

@@ -4,7 +4,7 @@ import { addPackageJsonDependency, NodeDependency, NodeDependencyType } from '@s
 const dependenciesToAdd = [
   {
     name: 'angular-auth-oidc-client',
-    version: '11.6.1',
+    version: '11.6.2',
   },
 ];