From 28d4f8b064eab20563ceb92ebe8cd2de85892063 Mon Sep 17 00:00:00 2001 From: Hayden Fowler Date: Thu, 15 Feb 2024 12:39:51 +1100 Subject: [PATCH] refactor: Added `getUserOrLogin` method to authManager --- packages/passport/sdk/src/authManager.test.ts | 41 +++++++++++++++---- packages/passport/sdk/src/authManager.ts | 12 ++++++ .../passportImxProviderFactory.test.ts | 21 ++++------ .../src/starkEx/passportImxProviderFactory.ts | 11 +---- .../sdk/src/zkEvm/user/loginZkEvmUser.test.ts | 18 ++++---- .../sdk/src/zkEvm/user/loginZkEvmUser.ts | 12 +----- 6 files changed, 61 insertions(+), 54 deletions(-) diff --git a/packages/passport/sdk/src/authManager.test.ts b/packages/passport/sdk/src/authManager.test.ts index 08491fc961..46353fd727 100644 --- a/packages/passport/sdk/src/authManager.test.ts +++ b/packages/passport/sdk/src/authManager.test.ts @@ -73,7 +73,7 @@ describe('AuthManager', () => { afterEach(jest.resetAllMocks); let authManager: AuthManager; - let mockSignIn: jest.Mock; + let mockSigninPopup: jest.Mock; let mockSigninPopupCallback: jest.Mock; let mockSignoutRedirect: jest.Mock; let mockGetUser: jest.Mock; @@ -82,7 +82,7 @@ describe('AuthManager', () => { let mockStoreUser: jest.Mock; beforeEach(() => { - mockSignIn = jest.fn(); + mockSigninPopup = jest.fn(); mockSigninPopupCallback = jest.fn(); mockSignoutRedirect = jest.fn(); mockGetUser = jest.fn(); @@ -90,7 +90,7 @@ describe('AuthManager', () => { mockSignoutSilent = jest.fn(); mockStoreUser = jest.fn(); (UserManager as jest.Mock).mockReturnValue({ - signinPopup: mockSignIn, + signinPopup: mockSigninPopup, signinPopupCallback: mockSigninPopupCallback, signoutRedirect: mockSignoutRedirect, signoutSilent: mockSignoutSilent, @@ -160,7 +160,7 @@ describe('AuthManager', () => { describe('login', () => { describe('when the user has not registered for any rollup', () => { it('should get the login user and return the domain model', async () => { - mockSignIn.mockResolvedValue(mockOidcUser); + mockSigninPopup.mockResolvedValue(mockOidcUser); const result = await authManager.login(); @@ -170,7 +170,7 @@ describe('AuthManager', () => { describe('when the user has registered for imx', () => { it('should populate the imx object', async () => { - mockSignIn.mockResolvedValue({ + mockSigninPopup.mockResolvedValue({ ...mockOidcUser, profile: { ...mockOidcUser.profile, @@ -188,7 +188,7 @@ describe('AuthManager', () => { describe('when the user has registered for zkEvm', () => { it('should populate the zkEvm object', async () => { - mockSignIn.mockResolvedValue({ + mockSigninPopup.mockResolvedValue({ ...mockOidcUser, profile: { ...mockOidcUser.profile, @@ -206,7 +206,7 @@ describe('AuthManager', () => { describe('when the user has registered for imx & zkEvm', () => { it('should populate the imx & zkEvm objects', async () => { - mockSignIn.mockResolvedValue({ + mockSigninPopup.mockResolvedValue({ ...mockOidcUser, profile: { ...mockOidcUser.profile, @@ -235,7 +235,7 @@ describe('AuthManager', () => { }); it('should throw the error if user is failed to login', async () => { - mockSignIn.mockRejectedValue(new Error(mockErrorMsg)); + mockSigninPopup.mockRejectedValue(new Error(mockErrorMsg)); await expect(() => authManager.login()).rejects.toThrow( new PassportError( @@ -246,6 +246,31 @@ describe('AuthManager', () => { }); }); + describe('getUserOrLogin', () => { + describe('when getUser returns a user', () => { + it('should return the user', async () => { + mockGetUser.mockReturnValue(mockOidcUser); + (isTokenExpired as jest.Mock).mockReturnValue(false); + + const result = await authManager.getUserOrLogin(); + + expect(result).toEqual(mockUser); + }); + }); + + describe('when getUser throws an error', () => { + it('calls attempts to sign in the user using signinPopup', async () => { + mockGetUser.mockRejectedValue(new Error(mockErrorMsg)); + mockSigninPopup.mockReturnValue(mockOidcUser); + (isTokenExpired as jest.Mock).mockReturnValue(false); + + const result = await authManager.getUserOrLogin(); + + expect(result).toEqual(mockUser); + }); + }); + }); + describe('loginCallback', () => { it('should call login callback', async () => { await authManager.loginCallback(); diff --git a/packages/passport/sdk/src/authManager.ts b/packages/passport/sdk/src/authManager.ts index aff4b707cd..a5bbb8604d 100644 --- a/packages/passport/sdk/src/authManager.ts +++ b/packages/passport/sdk/src/authManager.ts @@ -174,6 +174,18 @@ export default class AuthManager { }, PassportErrorType.AUTHENTICATION_ERROR); } + public async getUserOrLogin(): Promise { + let user = null; + try { + user = await this.getUser(); + } catch (err) { + // eslint-disable-next-line no-console + console.warn('failed to retrieve a cached user session:', err); + } + + return user || this.login(); + } + public async loginCallback(): Promise { return withPassportError( async () => this.userManager.signinPopupCallback(), diff --git a/packages/passport/sdk/src/starkEx/passportImxProviderFactory.test.ts b/packages/passport/sdk/src/starkEx/passportImxProviderFactory.test.ts index 903cd09307..bc0ea3a278 100644 --- a/packages/passport/sdk/src/starkEx/passportImxProviderFactory.test.ts +++ b/packages/passport/sdk/src/starkEx/passportImxProviderFactory.test.ts @@ -1,12 +1,10 @@ import { IMXClient } from '@imtbl/x-client'; import { ImxApiClients } from '@imtbl/generated-clients'; -import registerPassportStarkEx from './workflows/registration'; import { PassportImxProviderFactory } from './passportImxProviderFactory'; import MagicAdapter from '../magicAdapter'; import AuthManager from '../authManager'; import { PassportError, PassportErrorType } from '../errors/passportError'; import { PassportEventMap } from '../types'; -import { mockUserImx } from '../test/mocks'; import TypedEventEmitter from '../utils/typedEventEmitter'; import { PassportImxProvider } from './passportImxProvider'; import GuardianClient from '../guardian'; @@ -18,8 +16,7 @@ jest.mock('@imtbl/generated-clients'); describe('PassportImxProviderFactory', () => { const mockAuthManager = { getUser: jest.fn(), - forceUserRefresh: jest.fn(), - login: jest.fn(), + getUserOrLogin: jest.fn(), }; const imxApiClients = new ImxApiClients({} as any); @@ -41,7 +38,6 @@ describe('PassportImxProviderFactory', () => { beforeEach(() => { jest.restoreAllMocks(); - (registerPassportStarkEx as jest.Mock).mockResolvedValue(null); (PassportImxProvider as jest.Mock).mockImplementation(() => mockPassportImxProvider); }); @@ -61,7 +57,7 @@ describe('PassportImxProviderFactory', () => { describe('getProvider', () => { describe('when the user has no idToken', () => { it('should throw an error', async () => { - mockAuthManager.login.mockResolvedValue({ idToken: null }); + mockAuthManager.getUserOrLogin.mockResolvedValue({ idToken: null }); await expect(() => passportImxProviderFactory.getProvider()).rejects.toThrow( new PassportError( @@ -69,20 +65,17 @@ describe('PassportImxProviderFactory', () => { PassportErrorType.WALLET_CONNECTION_ERROR, ), ); - expect(mockAuthManager.login).toHaveBeenCalledTimes(1); + expect(mockAuthManager.getUserOrLogin).toHaveBeenCalledTimes(1); }); }); - it('should return a PassportImxProvider instance if silentLogin throws error', async () => { - mockAuthManager.login.mockResolvedValue(mockUserImx); - mockAuthManager.getUser.mockRejectedValue(new Error('error')); - mockAuthManager.login.mockResolvedValue(mockUserImx); + it('should return a PassportImxProvider instance', async () => { + mockAuthManager.getUserOrLogin.mockResolvedValue({ idToken: 'id123' }); + const result = await passportImxProviderFactory.getProvider(); expect(result).toBe(mockPassportImxProvider); - expect(mockAuthManager.getUser).toHaveBeenCalledTimes(1); - expect(mockAuthManager.login).toHaveBeenCalledTimes(1); - expect(registerPassportStarkEx).not.toHaveBeenCalled(); + expect(mockAuthManager.getUserOrLogin).toHaveBeenCalledTimes(1); expect(PassportImxProvider).toHaveBeenCalledWith({ magicAdapter: mockMagicAdapter, authManager: mockAuthManager, diff --git a/packages/passport/sdk/src/starkEx/passportImxProviderFactory.ts b/packages/passport/sdk/src/starkEx/passportImxProviderFactory.ts index 0dfcc7771d..897ba59682 100644 --- a/packages/passport/sdk/src/starkEx/passportImxProviderFactory.ts +++ b/packages/passport/sdk/src/starkEx/passportImxProviderFactory.ts @@ -48,16 +48,7 @@ export class PassportImxProviderFactory { } public async getProvider(): Promise { - let user = null; - try { - user = await this.authManager.getUser(); - } catch (e) { - // eslint-disable-next-line no-console - console.warn(e); - } - if (!user) { - user = await this.authManager.login(); - } + const user = await this.authManager.getUserOrLogin(); return this.createProviderInstance(user); } diff --git a/packages/passport/sdk/src/zkEvm/user/loginZkEvmUser.test.ts b/packages/passport/sdk/src/zkEvm/user/loginZkEvmUser.test.ts index 952d157f68..d08cc6c36e 100644 --- a/packages/passport/sdk/src/zkEvm/user/loginZkEvmUser.test.ts +++ b/packages/passport/sdk/src/zkEvm/user/loginZkEvmUser.test.ts @@ -11,11 +11,9 @@ jest.mock('./registerZkEvmUser'); describe('loginZkEvmUser', () => { afterEach(jest.resetAllMocks); - const getUserMock = jest.fn(); - const authLoginMock = jest.fn(); + const getUserOrLoginMock = jest.fn(); const authManager = { - getUser: getUserMock, - login: authLoginMock, + getUserOrLogin: getUserOrLoginMock, } as unknown as AuthManager; const magicProvider = {}; const magicAdapter = { @@ -25,7 +23,7 @@ describe('loginZkEvmUser', () => { const multiRollupApiClients = { } as unknown as MultiRollupApiClients; it('should return a user that has registered with zkEvm', async () => { - getUserMock.mockResolvedValue(mockUserZkEvm); + getUserOrLoginMock.mockResolvedValue(mockUserZkEvm); const result = await loginZkEvmUser({ authManager, magicAdapter, @@ -37,12 +35,11 @@ describe('loginZkEvmUser', () => { user: mockUserZkEvm, magicProvider, }); - expect(authManager.login).toBeCalledTimes(0); expect(registerZkEvmUser).toBeCalledTimes(0); }); it('should register and return a user if the user has not registered with zkEvm', async () => { - authLoginMock.mockResolvedValue(mockUser); + getUserOrLoginMock.mockResolvedValue(mockUser); (registerZkEvmUser as unknown as jest.Mock).mockResolvedValue(mockUserZkEvm); const result = await loginZkEvmUser({ authManager, @@ -55,12 +52,11 @@ describe('loginZkEvmUser', () => { user: mockUserZkEvm, magicProvider, }); - expect(authManager.login).toBeCalledTimes(1); expect(registerZkEvmUser).toBeCalledTimes(1); }); - it('should returns a user that has invalid refresh token but login again', async () => { - (authManager.login as unknown as jest.Mock).mockResolvedValue(mockUserZkEvm); + it('should return a user that has invalid refresh token but login again', async () => { + (authManager.getUserOrLogin as unknown as jest.Mock).mockResolvedValue(mockUserZkEvm); const result = await loginZkEvmUser({ authManager, magicAdapter, @@ -72,7 +68,7 @@ describe('loginZkEvmUser', () => { user: mockUserZkEvm, magicProvider, }); - expect(authManager.login).toBeCalledTimes(1); + expect(authManager.getUserOrLogin).toBeCalledTimes(1); expect(registerZkEvmUser).toBeCalledTimes(0); }); }); diff --git a/packages/passport/sdk/src/zkEvm/user/loginZkEvmUser.ts b/packages/passport/sdk/src/zkEvm/user/loginZkEvmUser.ts index b38b7c784c..db1720c347 100644 --- a/packages/passport/sdk/src/zkEvm/user/loginZkEvmUser.ts +++ b/packages/passport/sdk/src/zkEvm/user/loginZkEvmUser.ts @@ -23,17 +23,7 @@ export const loginZkEvmUser = async ({ multiRollupApiClients, jsonRpcProvider, }: LoginZkEvmUserInput): Promise => { - let user = null; - try { - user = await authManager.getUser(); - } catch (err) { - // eslint-disable-next-line no-console - console.warn('eth_requestAccounts` failed to retrieve a cached user session:', err); - } - - if (!user) { - user = await authManager.login(); - } + const user = await authManager.getUserOrLogin(); if (!user.idToken) { throw new Error('User is missing idToken'); }