From b659f02d8ca8d42e2c8b33f33fb3ecefb99ff19d Mon Sep 17 00:00:00 2001 From: Hayden Fowler Date: Thu, 30 May 2024 16:58:02 +1000 Subject: [PATCH] ID-1632 Added blocked popup overlay to login popup --- packages/passport/sdk/src/authManager.test.ts | 85 ++++++++++++++++++- packages/passport/sdk/src/authManager.ts | 62 +++++++------- .../sdk/src/confirmation/confirmation.ts | 2 +- .../sdk/src/overlay/{overlay.ts => index.ts} | 0 .../passport/sdk/src/overlay/overlay.test.ts | 2 +- 5 files changed, 119 insertions(+), 32 deletions(-) rename packages/passport/sdk/src/overlay/{overlay.ts => index.ts} (100%) diff --git a/packages/passport/sdk/src/authManager.test.ts b/packages/passport/sdk/src/authManager.test.ts index 82b42f08ee..c02d59d98a 100644 --- a/packages/passport/sdk/src/authManager.test.ts +++ b/packages/passport/sdk/src/authManager.test.ts @@ -2,6 +2,7 @@ import { Environment, ImmutableConfiguration } from '@imtbl/config'; import { User as OidcUser, UserManager, WebStorageStateStore } from 'oidc-client-ts'; import jwt_decode from 'jwt-decode'; import AuthManager from './authManager'; +import Overlay from './overlay'; import { PassportError, PassportErrorType } from './errors/passportError'; import { PassportConfiguration } from './config'; import { mockUser, mockUserImx, mockUserZkEvm } from './test/mocks'; @@ -9,13 +10,14 @@ import { isTokenExpired } from './utils/token'; import { isUserZkEvm, PassportModuleConfiguration } from './types'; jest.mock('jwt-decode'); -jest.mock('./utils/token'); jest.mock('oidc-client-ts', () => ({ ...jest.requireActual('oidc-client-ts'), InMemoryWebStorage: jest.fn(), UserManager: jest.fn(), WebStorageStateStore: jest.fn(), })); +jest.mock('./utils/token'); +jest.mock('./overlay'); const authenticationDomain = 'auth.immutable.com'; const clientId = '11111'; @@ -87,6 +89,8 @@ describe('AuthManager', () => { let mockSigninSilent: jest.Mock; let mockSignoutSilent: jest.Mock; let mockStoreUser: jest.Mock; + let mockOverlayAppend: jest.Mock; + let mockOverlayRemove: jest.Mock; beforeEach(() => { mockSigninPopup = jest.fn(); @@ -96,6 +100,8 @@ describe('AuthManager', () => { mockSigninSilent = jest.fn(); mockSignoutSilent = jest.fn(); mockStoreUser = jest.fn(); + mockOverlayAppend = jest.fn(); + mockOverlayRemove = jest.fn(); (UserManager as jest.Mock).mockReturnValue({ signinPopup: mockSigninPopup, signinPopupCallback: mockSigninPopupCallback, @@ -105,6 +111,10 @@ describe('AuthManager', () => { signinSilent: mockSigninSilent, storeUser: mockStoreUser, }); + (Overlay as jest.Mock).mockReturnValue({ + append: mockOverlayAppend, + remove: mockOverlayRemove, + }); authManager = new AuthManager(getConfig()); }); @@ -250,6 +260,79 @@ describe('AuthManager', () => { ), ); }); + + describe('when the popup is blocked', () => { + beforeEach(() => { + mockSigninPopup.mockRejectedValueOnce(new Error('Attempted to navigate on a disposed window')); + }); + + it('should render the blocked popup overlay', async () => { + const configWithPopupOverlayOptions = getConfig({ + popupOverlayOptions: { + disableGenericPopupOverlay: false, + disableBlockedPopupOverlay: false, + }, + }); + const am = new AuthManager(configWithPopupOverlayOptions); + + mockSigninPopup.mockReturnValue(mockOidcUser); + // Simulate `tryAgainOnClick` being called so that the `login()` promise can resolve + mockOverlayAppend.mockImplementation(async (tryAgainOnClick: () => Promise) => { + await tryAgainOnClick(); + }); + + const result = await am.login(); + + expect(result).toEqual(mockUser); + expect(Overlay).toHaveBeenCalledWith(configWithPopupOverlayOptions.popupOverlayOptions, true); + expect(mockOverlayAppend).toHaveBeenCalledTimes(1); + }); + + describe('when tryAgainOnClick is called once', () => { + beforeEach(() => { + mockOverlayAppend.mockImplementation(async (tryAgainOnClick: () => Promise) => { + await tryAgainOnClick(); + }); + }); + + it('should return a user', async () => { + mockSigninPopup.mockReturnValue(mockOidcUser); + + const result = await authManager.login(); + + expect(result).toEqual(mockUser); + expect(mockSigninPopup).toHaveBeenCalledTimes(2); + expect(mockOverlayRemove).toHaveBeenCalled(); + }); + + describe('and the user closes the popup', () => { + it('should throw an error', async () => { + mockSigninPopup.mockRejectedValueOnce(new Error('Popup closed by user')); + + await expect(() => authManager.login()).rejects.toThrow( + new Error('Popup closed by user'), + ); + + expect(mockSigninPopup).toHaveBeenCalledTimes(2); + expect(mockOverlayRemove).toHaveBeenCalled(); + }); + }); + }); + + describe('when onCloseClick is called', () => { + it('should remove the overlay', async () => { + mockOverlayAppend.mockImplementation(async (_: () => Promise, onCloseClick: () => void) => { + onCloseClick(); + }); + + await expect(() => authManager.login()).rejects.toThrow( + new Error('Popup closed by user'), + ); + + expect(mockOverlayRemove).toHaveBeenCalled(); + }); + }); + }); }); describe('getUserOrLogin', () => { diff --git a/packages/passport/sdk/src/authManager.ts b/packages/passport/sdk/src/authManager.ts index 5c41f6c184..cb9277caba 100644 --- a/packages/passport/sdk/src/authManager.ts +++ b/packages/passport/sdk/src/authManager.ts @@ -30,7 +30,7 @@ import { isUserImx, } from './types'; import { PassportConfiguration } from './config'; -import Overlay from './overlay/overlay'; +import Overlay from './overlay'; const formUrlEncodedHeader = { headers: { @@ -199,39 +199,43 @@ export default class AuthManager { resolve(AuthManager.mapOidcUserToDomainModel(oidcUser)); }) .catch((error: unknown) => { - // If the popup was blocked, append the blocked popup overlay. - if (error instanceof Error && error.message === 'Attempted to navigate on a disposed window') { - const overlay = new Overlay(this.config.popupOverlayOptions, true); - let popupHasOpened: boolean = false; - overlay.append( - async () => { - try { - if (!popupHasOpened) { + // reject with the error if it is not caused by a blocked popup + if (!(error instanceof Error) || error.message !== 'Attempted to navigate on a disposed window') { + reject(error); + } + + // Popup was blocked; append the blocked popup overlay to allow the user to try again. + let popupHasBeenOpened: boolean = false; + const overlay = new Overlay(this.config.popupOverlayOptions, true); + overlay.append( + async () => { + try { + if (!popupHasBeenOpened) { // The user is attempting to open the popup again. It's safe to assume that this will not fail, // as there are no async operations between the button interaction & the popup being opened. - popupHasOpened = true; - const oidcUser = await signinPopup(); - overlay.remove(); - resolve(AuthManager.mapOidcUserToDomainModel(oidcUser)); - } else { - // The popup has already been opened. By calling `window.open` with the same target as the - // previously opened popup, no new window will be opened. Instead, the existing popup - // will be focused and brought to the front reliably. - window.open('', popupWindowTarget); - } - } catch (retryError: unknown) { + popupHasBeenOpened = true; + const oidcUser = await signinPopup(); overlay.remove(); - reject(retryError); + resolve(AuthManager.mapOidcUserToDomainModel(oidcUser)); + } else { + // The popup has already been opened. By calling `window.open` with the same target as the + // previously opened popup, no new window will be opened. Instead, the existing popup + // will be focused. This works as expected in most browsers at the time of implementation, but + // the following exceptions do exist: + // - Safari: Only the initial call will focus the window, subsequent calls will do nothing. + // - Firefox: The window will not be focussed, nothing will happen. + window.open('', popupWindowTarget); } - }, - () => { + } catch (retryError: unknown) { overlay.remove(); - reject(new Error('Popup closed by user')); - }, - ); - } else { - reject(error); - } + reject(retryError); + } + }, + () => { + overlay.remove(); + reject(new Error('Popup closed by user')); + }, + ); }); }); }, PassportErrorType.AUTHENTICATION_ERROR); diff --git a/packages/passport/sdk/src/confirmation/confirmation.ts b/packages/passport/sdk/src/confirmation/confirmation.ts index 65c4993b49..fee78e3eb0 100644 --- a/packages/passport/sdk/src/confirmation/confirmation.ts +++ b/packages/passport/sdk/src/confirmation/confirmation.ts @@ -7,7 +7,7 @@ import { } from './types'; import { openPopupCenter } from './popup'; import { PassportConfiguration } from '../config'; -import Overlay from '../overlay/overlay'; +import Overlay from '../overlay'; const CONFIRMATION_WINDOW_TITLE = 'Confirm this transaction'; const CONFIRMATION_WINDOW_HEIGHT = 720; diff --git a/packages/passport/sdk/src/overlay/overlay.ts b/packages/passport/sdk/src/overlay/index.ts similarity index 100% rename from packages/passport/sdk/src/overlay/overlay.ts rename to packages/passport/sdk/src/overlay/index.ts diff --git a/packages/passport/sdk/src/overlay/overlay.test.ts b/packages/passport/sdk/src/overlay/overlay.test.ts index d729efba8a..ba811be03a 100644 --- a/packages/passport/sdk/src/overlay/overlay.test.ts +++ b/packages/passport/sdk/src/overlay/overlay.test.ts @@ -1,4 +1,4 @@ -import Overlay from './overlay'; +import Overlay from './index'; describe('overlay', () => { beforeEach(() => {