Merge pull request #1058 from adiessl/main

Enable handling users closing login popup
damienbod · Apr 25, 2021 · 5fa422d · 5fa422d
2 parents cb0011b + 5ba629c
commit 5fa422d
Show file tree

Hide file tree

Showing 11 changed files with 194 additions and 34 deletions.
diff --git a/docs/authorizing-popup.md b/docs/authorizing-popup.md
@@ -22,10 +22,11 @@ This allows you to have the provider's consent prompt display in a popup window
   }
 
   loginWithPopup() {
-    this.oidcSecurityService.authorizeWithPopUp().subscribe(({ isAuthenticated, userData, accessToken }) => {
+    this.oidcSecurityService.authorizeWithPopUp().subscribe(({ isAuthenticated, userData, accessToken, errorMessage }) => {
       console.log(isAuthenticated);
       console.log(userData);
       console.log(accessToken);
+      console.log(errorMessage);
     });
   }
 ```
@@ -40,7 +41,7 @@ loginWithPopup() {
   const somePopupOptions = { width: 500, height: 500, left: 50, top: 50 };
 
   this.oidcSecurityService.authorizeWithPopUp(null, somePopupOptions)
-    .subscribe(({ isAuthenticated, userData, accessToken }) => {
+    .subscribe(({ isAuthenticated, userData, accessToken, errorMessage }) => {
     /* ... */
     });
 }

diff --git a/projects/angular-auth-oidc-client/src/lib/login/login-response.ts b/projects/angular-auth-oidc-client/src/lib/login/login-response.ts
@@ -1,5 +1,6 @@
 export interface LoginResponse {
   isAuthenticated: boolean;
-  userData: any;
-  accessToken: string;
+  userData?: any;
+  accessToken?: string;
+  errorMessage?: string;
 }
diff --git a/projects/angular-auth-oidc-client/src/lib/login/par/par-login.service.spec.ts b/projects/angular-auth-oidc-client/src/lib/login/par/par-login.service.spec.ts
@@ -14,6 +14,7 @@ import { UserServiceMock } from '../../userData/user-service-mock';
 import { RedirectService } from '../../utils/redirect/redirect.service';
 import { UrlService } from '../../utils/url/url.service';
 import { UrlServiceMock } from '../../utils/url/url.service-mock';
+import { PopupResult } from '../popup/popup-result';
 import { PopUpService } from '../popup/popup.service';
 import { PopUpServiceMock } from '../popup/popup.service-mock';
 import { ResponseTypeValidationService } from '../response-type-validation/response-type-validation.service';
@@ -360,6 +361,7 @@ describe('ParLoginService', () => {
 
         spyOn(parService, 'postParRequest').and.returnValue(of({ requestUri: 'requestUri' } as ParResponse));
         spyOn(urlService, 'getAuthorizeParUrl').and.returnValue('some-par-url');
+        spyOnProperty(popupService, 'result$').and.returnValue(of({}));
         const spy = spyOn(popupService, 'openPopUp');
 
         service.loginWithPopUpPar().subscribe((result) => {
@@ -385,7 +387,8 @@ describe('ParLoginService', () => {
         const checkAuthSpy = spyOn(checkAuthService, 'checkAuth').and.returnValue(of(true));
         const getUserDataFromStoreSpy = spyOn(userService, 'getUserDataFromStore').and.returnValue({ any: 'userData' });
         const getAccessTokenSpy = spyOn(authStateService, 'getAccessToken').and.returnValue('anyAccessToken');
-        spyOnProperty(popupService, 'receivedUrl$').and.returnValue(of('someUrl'));
+        const popupResult: PopupResult = { userClosed: false, receivedUrl: 'someUrl' };
+        spyOnProperty(popupService, 'result$').and.returnValue(of(popupResult));
 
         service.loginWithPopUpPar().subscribe((result) => {
           expect(checkAuthSpy).toHaveBeenCalledWith('someUrl');
@@ -396,5 +399,35 @@ describe('ParLoginService', () => {
         });
       })
     );
+
+    it(
+      'returns correct properties if popup was closed by user',
+      waitForAsync(() => {
+        spyOn(responseTypeValidationService, 'hasConfigValidResponseType').and.returnValue(true);
+        spyOn(configurationProvider, 'getOpenIDConfiguration').and.returnValue({
+          authWellknownEndpoint: 'authWellknownEndpoint',
+          responseType: 'stubValue',
+        });
+
+        spyOn(authWellKnownService, 'getAuthWellKnownEndPoints').and.returnValue(of({}));
+
+        spyOn(parService, 'postParRequest').and.returnValue(of({ requestUri: 'requestUri' } as ParResponse));
+        spyOn(urlService, 'getAuthorizeParUrl').and.returnValue('some-par-url');
+
+        const checkAuthSpy = spyOn(checkAuthService, 'checkAuth');
+        const getUserDataFromStoreSpy = spyOn(userService, 'getUserDataFromStore');
+        const getAccessTokenSpy = spyOn(authStateService, 'getAccessToken');
+        const popupResult: PopupResult = { userClosed: true };
+        spyOnProperty(popupService, 'result$').and.returnValue(of(popupResult));
+
+        service.loginWithPopUpPar().subscribe((result) => {
+          expect(checkAuthSpy).not.toHaveBeenCalled();
+          expect(getUserDataFromStoreSpy).not.toHaveBeenCalled();
+          expect(getAccessTokenSpy).not.toHaveBeenCalled();
+
+          expect(result).toEqual({ isAuthenticated: false, errorMessage: 'User closed popup' });
+        });
+      })
+    );
   });
 });
diff --git a/projects/angular-auth-oidc-client/src/lib/login/par/par-login.service.ts b/projects/angular-auth-oidc-client/src/lib/login/par/par-login.service.ts
@@ -1,5 +1,5 @@
 import { Injectable } from '@angular/core';
-import { Observable, throwError } from 'rxjs';
+import { Observable, of, throwError } from 'rxjs';
 import { map, switchMap, take } from 'rxjs/operators';
 import { AuthStateService } from '../../authState/auth-state.service';
 import { CheckAuthService } from '../../check-auth.service';
@@ -109,14 +109,19 @@ export class ParLoginService {
 
         this.popupService.openPopUp(url, popupOptions);
 
-        return this.popupService.receivedUrl$.pipe(
+        return this.popupService.result$.pipe(
           take(1),
-          switchMap((receivedUrl: string) => this.checkAuthService.checkAuth(receivedUrl)),
-          map((isAuthenticated) => ({
-            isAuthenticated,
-            userData: this.userService.getUserDataFromStore(),
-            accessToken: this.authStateService.getAccessToken(),
-          }))
+          switchMap((result) =>
+            result.userClosed === true
+              ? of({ isAuthenticated: false, errorMessage: 'User closed popup' })
+              : this.checkAuthService.checkAuth(result.receivedUrl).pipe(
+                  map((isAuthenticated) => ({
+                    isAuthenticated,
+                    userData: this.userService.getUserDataFromStore(),
+                    accessToken: this.authStateService.getAccessToken(),
+                  }))
+                )
+          )
         );
       })
     );

diff --git a/projects/angular-auth-oidc-client/src/lib/login/popup/popup-login.service.spec.ts b/projects/angular-auth-oidc-client/src/lib/login/popup/popup-login.service.spec.ts
@@ -18,6 +18,7 @@ import { ResponseTypeValidationService } from '../response-type-validation/respo
 import { ResponseTypeValidationServiceMock } from '../response-type-validation/response-type-validation.service.mock';
 import { UrlServiceMock } from './../../utils/url/url.service-mock';
 import { PopUpLoginService } from './popup-login.service';
+import { PopupResult } from './popup-result';
 import { PopUpService } from './popup.service';
 import { PopUpServiceMock } from './popup.service-mock';
 
@@ -111,6 +112,7 @@ describe('PopUpLoginService', () => {
         });
         spyOn(responseTypValidationService, 'hasConfigValidResponseType').and.returnValue(true);
         spyOn(authWellKnownService, 'getAuthWellKnownEndPoints').and.returnValue(of({}));
+        spyOnProperty(popupService, 'result$').and.returnValue(of({}));
         const spy = spyOn(urlService, 'getAuthorizeUrl');
 
         popUpLoginService.loginWithPopUpStandard().subscribe(() => {
@@ -129,6 +131,7 @@ describe('PopUpLoginService', () => {
         spyOn(responseTypValidationService, 'hasConfigValidResponseType').and.returnValue(true);
         spyOn(authWellKnownService, 'getAuthWellKnownEndPoints').and.returnValue(of({}));
         spyOn(urlService, 'getAuthorizeUrl');
+        spyOnProperty(popupService, 'result$').and.returnValue(of({}));
         const popupSpy = spyOn(popupService, 'openPopUp');
 
         popUpLoginService.loginWithPopUpStandard().subscribe(() => {
@@ -151,7 +154,8 @@ describe('PopUpLoginService', () => {
         const checkAuthSpy = spyOn(checkAuthService, 'checkAuth').and.returnValue(of(true));
         const getUserDataFromStoreSpy = spyOn(userService, 'getUserDataFromStore').and.returnValue({ any: 'userData' });
         const getAccessTokenSpy = spyOn(authStateService, 'getAccessToken').and.returnValue('anyAccessToken');
-        spyOnProperty(popupService, 'receivedUrl$').and.returnValue(of('someUrl'));
+        const popupResult: PopupResult = { userClosed: false, receivedUrl: 'someUrl' };
+        spyOnProperty(popupService, 'result$').and.returnValue(of(popupResult));
 
         popUpLoginService.loginWithPopUpStandard().subscribe((result) => {
           expect(checkAuthSpy).toHaveBeenCalledWith('someUrl');
@@ -162,5 +166,32 @@ describe('PopUpLoginService', () => {
         });
       })
     );
+
+    it(
+      'returns two properties if popup was closed by user',
+      waitForAsync(() => {
+        spyOn(configurationProvider, 'getOpenIDConfiguration').and.returnValue({
+          authWellknownEndpoint: 'authWellknownEndpoint',
+          responseType: 'stubValue',
+        });
+        spyOn(responseTypValidationService, 'hasConfigValidResponseType').and.returnValue(true);
+        spyOn(authWellKnownService, 'getAuthWellKnownEndPoints').and.returnValue(of({}));
+        spyOn(urlService, 'getAuthorizeUrl');
+        spyOn(popupService, 'openPopUp');
+        const checkAuthSpy = spyOn(checkAuthService, 'checkAuth');
+        const getUserDataFromStoreSpy = spyOn(userService, 'getUserDataFromStore');
+        const getAccessTokenSpy = spyOn(authStateService, 'getAccessToken');
+        const popupResult: PopupResult = { userClosed: true };
+        spyOnProperty(popupService, 'result$').and.returnValue(of(popupResult));
+
+        popUpLoginService.loginWithPopUpStandard().subscribe((result) => {
+          expect(checkAuthSpy).not.toHaveBeenCalled();
+          expect(getUserDataFromStoreSpy).not.toHaveBeenCalled();
+          expect(getAccessTokenSpy).not.toHaveBeenCalled();
+
+          expect(result).toEqual({ isAuthenticated: false, errorMessage: 'User closed popup' });
+        });
+      })
+    );
   });
 });
diff --git a/projects/angular-auth-oidc-client/src/lib/login/popup/popup-login.service.ts b/projects/angular-auth-oidc-client/src/lib/login/popup/popup-login.service.ts
@@ -1,5 +1,5 @@
 import { Injectable } from '@angular/core';
-import { Observable, throwError } from 'rxjs';
+import { Observable, of, throwError } from 'rxjs';
 import { map, switchMap, take } from 'rxjs/operators';
 import { AuthStateService } from '../../authState/auth-state.service';
 import { CheckAuthService } from '../../check-auth.service';
@@ -53,14 +53,19 @@ export class PopUpLoginService {
 
         this.popupService.openPopUp(authUrl, popupOptions);
 
-        return this.popupService.receivedUrl$.pipe(
+        return this.popupService.result$.pipe(
           take(1),
-          switchMap((url: string) => this.checkAuthService.checkAuth(url)),
-          map((isAuthenticated) => ({
-            isAuthenticated,
-            userData: this.userService.getUserDataFromStore(),
-            accessToken: this.authStateService.getAccessToken(),
-          }))
+          switchMap((result) =>
+            result.userClosed === true
+              ? of({ isAuthenticated: false, errorMessage: 'User closed popup' })
+              : this.checkAuthService.checkAuth(result.receivedUrl).pipe(
+                  map((isAuthenticated) => ({
+                    isAuthenticated,
+                    userData: this.userService.getUserDataFromStore(),
+                    accessToken: this.authStateService.getAccessToken(),
+                  }))
+                )
+          )
         );
       })
     );

diff --git a/projects/angular-auth-oidc-client/src/lib/login/popup/popup-result.ts b/projects/angular-auth-oidc-client/src/lib/login/popup/popup-result.ts
@@ -0,0 +1,10 @@
+export interface PopupResultUserClosed {
+  userClosed: true;
+}
+
+export interface PopupResultReceivedUrl {
+  userClosed: false;
+  receivedUrl: string;
+}
+
+export type PopupResult = PopupResultUserClosed | PopupResultReceivedUrl;
diff --git a/projects/angular-auth-oidc-client/src/lib/login/popup/popup.service-mock.ts b/projects/angular-auth-oidc-client/src/lib/login/popup/popup.service-mock.ts
@@ -4,7 +4,7 @@ import { PopupOptions } from './popup-options';
 
 @Injectable({ providedIn: 'root' })
 export class PopUpServiceMock {
-  get receivedUrl$() {
+  get result$() {
     return of(null);
   }
 

diff --git a/projects/angular-auth-oidc-client/src/lib/login/popup/popup.service.spec.ts b/projects/angular-auth-oidc-client/src/lib/login/popup/popup.service.spec.ts
@@ -1,4 +1,5 @@
-import { TestBed, waitForAsync } from '@angular/core/testing';
+import { fakeAsync, TestBed, tick, waitForAsync } from '@angular/core/testing';
+import { PopupResult } from './popup-result';
 import { PopUpService } from './popup.service';
 
 describe('PopUpService', () => {
@@ -55,15 +56,17 @@ describe('PopUpService', () => {
     });
   });
 
-  describe('receivedUrl$', () => {
+  describe('result$', () => {
     it(
       'emits when internal subject is called',
       waitForAsync(() => {
-        popUpService.receivedUrl$.subscribe((result) => {
-          expect(result).toBe('some-url1111');
+        const popupResult: PopupResult = { userClosed: false, receivedUrl: 'some-url1111' };
+
+        popUpService.result$.subscribe((result) => {
+          expect(result).toBe(popupResult);
         });
 
-        (popUpService as any).receivedUrlInternal$.next('some-url1111');
+        (popUpService as any).resultInternal$.next(popupResult);
       })
     );
   });
@@ -76,6 +79,8 @@ describe('PopUpService', () => {
           () =>
             ({
               sessionStorage: mockStorage,
+              closed: true,
+              close: () => {},
             } as Window)
         );
         popUpService.openPopUp('url');
@@ -91,13 +96,69 @@ describe('PopUpService', () => {
           () =>
             ({
               sessionStorage: mockStorage,
+              closed: true,
+              close: () => {},
             } as Window)
         );
         popUpService.openPopUp('url', { width: 100 });
 
         expect(popupSpy).toHaveBeenCalledOnceWith('url', '_blank', 'width=100,height=500,left=50,top=50');
       })
     );
+
+    describe('popup closed', () => {
+      let popup: Window;
+      let popupResult: PopupResult;
+      let cleanUpSpy: jasmine.Spy;
+
+      beforeEach(() => {
+        popup = {
+          sessionStorage: mockStorage,
+          closed: false,
+          close: () => {},
+        } as Window;
+
+        spyOn(window, 'open').and.returnValue(popup);
+
+        cleanUpSpy = spyOn(popUpService as any, 'cleanUp').and.callThrough();
+
+        popupResult = undefined;
+
+        popUpService.result$.subscribe((result) => (popupResult = result));
+      });
+
+      it('message received', fakeAsync(() => {
+        let listener: (event: MessageEvent) => void;
+
+        spyOn(window, 'addEventListener').and.callFake((_, func) => (listener = func));
+
+        popUpService.openPopUp('url');
+
+        expect(popupResult).toBeUndefined();
+        expect(cleanUpSpy).not.toHaveBeenCalled();
+
+        listener(new MessageEvent('message', { data: 'some-url1111' }));
+
+        tick(200);
+
+        expect(popupResult).toEqual({ userClosed: false, receivedUrl: 'some-url1111' });
+        expect(cleanUpSpy).toHaveBeenCalledWith(listener);
+      }));
+
+      it('user closed', fakeAsync(() => {
+        popUpService.openPopUp('url');
+
+        expect(popupResult).toBeUndefined();
+        expect(cleanUpSpy).not.toHaveBeenCalled();
+
+        (popup as any).closed = true;
+
+        tick(200);
+
+        expect(popupResult).toEqual({ userClosed: true });
+        expect(cleanUpSpy).toHaveBeenCalled();
+      }));
+    });
   });
 
   describe('sendMessageToMainWindow', () => {