[ID-1897] Improve magic session management

packages/passport/sdk/src/Passport.int.test.ts

@@ -77,6 +77,7 @@ describe('Passport', () => {
   const mockGetUser = jest.fn();
   const mockLoginWithOidc = jest.fn();
   const mockMagicRequest = jest.fn();
+  const mockIsMagicLoggedIn = jest.fn();
 
   beforeEach(() => {
     jest.resetAllMocks();
@@ -94,6 +95,7 @@ describe('Passport', () => {
       openid: { loginWithOIDC: mockLoginWithOidc },
       rpcProvider: { request: mockMagicRequest },
       preload: jest.fn(),
+      user: { isLoggedIn: mockIsMagicLoggedIn },
     }));
   });
 
@@ -179,7 +181,6 @@ describe('Passport', () => {
           });
 
           expect(accounts).toEqual([mockUserZkEvm.zkEvm.ethAddress]);
-          expect(mockGetUser).toHaveBeenCalledTimes(2);
         });
       });
 
@@ -210,20 +211,23 @@ describe('Passport', () => {
 
           const zkEvmProvider = getZkEvmProvider();
 
+          mockGetUser.mockResolvedValue(mockOidcUser);
+          mockIsMagicLoggedIn.mockResolvedValue(true);
+
           const accounts = await zkEvmProvider.request({
             method: 'eth_requestAccounts',
           });
 
           expect(accounts).toEqual([mockUserZkEvm.zkEvm.ethAddress]);
-          expect(mockGetUser).toHaveBeenCalledTimes(3);
         });
 
         describe('when the registration request fails', () => {
           it('throws an error', async () => {
             mockSigninPopup.mockResolvedValue(mockOidcUser);
             mockGetUser.mockResolvedValueOnce(null);
-            mockGetUser.mockResolvedValueOnce(mockOidcUser);
+            mockGetUser.mockResolvedValueOnce(null);
             mockSigninSilent.mockResolvedValue(mockOidcUser);
+            mockGetUser.mockResolvedValueOnce(mockOidcUser);
             useMswHandlers([
               mswHandlers.counterfactualAddress.internalServerError,
               mswHandlers.api.chains.success,
@@ -282,81 +286,14 @@ describe('Passport', () => {
           params: [transaction],
         });
 
-        expect(result).toEqual(transactionHash);
-        expect(mockGetUser).toHaveBeenCalledTimes(6);
-      });
-
-      it('ethSigner is initialised if user logs in after connectEvm', async () => {
-        const transferToAddress = '0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC';
-
-        useMswHandlers([
-          mswHandlers.counterfactualAddress.success,
-          mswHandlers.rpcProvider.success,
-          mswHandlers.relayer.success,
-          mswHandlers.guardian.evaluateTransaction.success,
-        ]);
-        mockMagicRequest.mockImplementation(({ method }: RequestArguments) => {
-          switch (method) {
-            case 'eth_chainId': {
-              return Promise.resolve(chainIdHex);
-            }
-            case 'eth_accounts': {
-              return Promise.resolve([magicWalletAddress]);
-            }
-            case 'personal_sign': {
-              return Promise.resolve('0x6b168cf5d90189eaa51d02ff3fa8ffc8956b1ea20fdd34280f521b1acca092305b9ace24e643fe64a30c528323065f5b77e1fb4045bd330aad01e7b9a07591f91b');
-            }
-            default: {
-              throw new Error(`Unexpected method: ${method}`);
-            }
-          }
-        });
-        mockGetUser.mockResolvedValueOnce(Promise.resolve(null));
-        mockSigninPopup.mockResolvedValue(mockOidcUserZkevm);
-        mockSigninSilent.mockResolvedValueOnce(mockOidcUserZkevm);
-
-        const passport = new Passport({
-          baseConfig: new ImmutableConfiguration({
-            environment: Environment.SANDBOX,
-          }),
-          audience: 'platform_api',
-          clientId,
-          redirectUri,
-          logoutRedirectUri,
-          scope: 'openid offline_access profile email transact',
-        });
-
-        // user isn't logged in, so wont set signer when provider is instantiated
-        // #doc request-accounts
-        const provider = passport.connectEvm();
-        const accounts = await provider.request({
-          method: 'eth_requestAccounts',
-        });
-        // #enddoc request-accounts
-
-        // user logs in, ethSigner is initialised
-        await passport.login();
-
-        mockGetUser.mockResolvedValue(Promise.resolve(mockOidcUserZkevm));
-
-        expect(accounts).toEqual([mockUserZkEvm.zkEvm.ethAddress]);
-
-        const transaction: TransactionRequest = {
-          to: transferToAddress,
-          value: '5000000000000000',
-          data: '0x00',
-        };
-        const result = await provider.request({
-          method: 'eth_sendTransaction',
-          params: [transaction],
-        });
-
         expect(result).toEqual(transactionHash);
       });
     });
 
     describe('eth_accounts', () => {
       it('returns no addresses if the user is not logged in', async () => {
+        mockGetUser.mockResolvedValue(null);
+
         const zkEvmProvider = getZkEvmProvider();
         const accounts = await zkEvmProvider.request({
           method: 'eth_accounts',

packages/passport/sdk/src/Passport.ts

@@ -53,11 +53,12 @@ const buildImxApiClients = (passportModuleConfiguration: PassportModuleConfigura
 
 export const buildPrivateVars = (passportModuleConfiguration: PassportModuleConfiguration) => {
   const config = new PassportConfiguration(passportModuleConfiguration);
-  const authManager = new AuthManager(config);
-  const magicAdapter = new MagicAdapter(config);
+  const passportEventEmitter = new TypedEventEmitter<PassportEventMap>();
+
+  const authManager = new AuthManager(config, passportEventEmitter);
+  const magicAdapter = new MagicAdapter(config, authManager, passportEventEmitter);
   const confirmationScreen = new ConfirmationScreen(config);
   const multiRollupApiClients = new MultiRollupApiClients(config.multiRollupConfig);
-  const passportEventEmitter = new TypedEventEmitter<PassportEventMap>();
 
   const immutableXClient = passportModuleConfiguration.overrides
     ? passportModuleConfiguration.overrides.immutableXClient
@@ -196,7 +197,6 @@ export class Passport {
       identify({
         passportId: user.profile.sub,
       });
-      this.passportEventEmitter.emit(PassportEvents.LOGGED_IN, user);
     }
 
     return user ? user.profile : null;
@@ -222,7 +222,6 @@ export class Passport {
       interval,
       timeoutMs,
     );
-    this.passportEventEmitter.emit(PassportEvents.LOGGED_IN, user);
     return user.profile;
   }
 
@@ -238,7 +237,6 @@ export class Passport {
       authorizationCode,
       state,
     );
-    this.passportEventEmitter.emit(PassportEvents.LOGGED_IN, user);
     return user.profile;
   }
 

packages/passport/sdk/src/authManager.test.ts

@@ -7,7 +7,8 @@ import { PassportError, PassportErrorType } from './errors/passportError';
 import { PassportConfiguration } from './config';
 import { mockUser, mockUserImx, mockUserZkEvm } from './test/mocks';
 import { isTokenExpired } from './utils/token';
-import { isUserZkEvm, PassportModuleConfiguration } from './types';
+import { isUserZkEvm, PassportEventMap, PassportModuleConfiguration } from './types';
+import TypedEventEmitter from './utils/typedEventEmitter';
 
 jest.mock('jwt-decode');
 jest.mock('oidc-client-ts', () => ({
@@ -81,6 +82,8 @@ const mockErrorMsg = 'NONO';
 describe('AuthManager', () => {
   afterEach(jest.resetAllMocks);
 
+  const passportEventEmitter = new TypedEventEmitter<PassportEventMap>();
+
   let authManager: AuthManager;
   let mockSigninPopup: jest.Mock;
   let mockSigninPopupCallback: jest.Mock;
@@ -115,13 +118,13 @@ describe('AuthManager', () => {
       append: mockOverlayAppend,
       remove: mockOverlayRemove,
     });
-    authManager = new AuthManager(getConfig());
+    authManager = new AuthManager(getConfig(), passportEventEmitter);
   });
 
   describe('constructor', () => {
     it('should initialise AuthManager with the correct default configuration', () => {
       const config = getConfig();
-      const am = new AuthManager(config);
+      const am = new AuthManager(config, passportEventEmitter);
       expect(am).toBeDefined();
       expect(UserManager).toBeCalledWith({
         authority: config.authenticationDomain,
@@ -147,7 +150,7 @@ describe('AuthManager', () => {
         const configWithAudience = getConfig({
           audience: 'audience',
         });
-        const am = new AuthManager(configWithAudience);
+        const am = new AuthManager(configWithAudience, passportEventEmitter);
         expect(am).toBeDefined();
         expect(UserManager).toBeCalledWith(expect.objectContaining({
           extraQueryParams: {
@@ -160,7 +163,7 @@ describe('AuthManager', () => {
     describe('when a logoutRedirectUri is specified', () => {
       it('should set the endSessionEndpoint `returnTo` and `client_id` query string params', () => {
         const configWithLogoutRedirectUri = getConfig({ logoutRedirectUri });
-        const am = new AuthManager(configWithLogoutRedirectUri);
+        const am = new AuthManager(configWithLogoutRedirectUri, passportEventEmitter);
 
         const uri = new URL(logoutEndpoint, `https://${authenticationDomain}`);
         uri.searchParams.append('client_id', clientId);
@@ -274,7 +277,7 @@ describe('AuthManager', () => {
             disableBlockedPopupOverlay: false,
           },
         });
-        const am = new AuthManager(configWithPopupOverlayOptions);
+        const am = new AuthManager(configWithPopupOverlayOptions, passportEventEmitter);
 
         mockSigninPopup.mockReturnValue(mockOidcUser);
         // Simulate `tryAgainOnClick` being called so that the `login()` promise can resolve
@@ -374,7 +377,7 @@ describe('AuthManager', () => {
       const configuration = getConfig({
         logoutMode: 'redirect',
       });
-      const manager = new AuthManager(configuration);
+      const manager = new AuthManager(configuration, passportEventEmitter);
 
       await manager.logout();
 
@@ -385,7 +388,7 @@ describe('AuthManager', () => {
       const configuration = getConfig({
         logoutMode: undefined,
       });
-      const manager = new AuthManager(configuration);
+      const manager = new AuthManager(configuration, passportEventEmitter);
 
       await manager.logout();
 
@@ -396,7 +399,7 @@ describe('AuthManager', () => {
       const configuration = getConfig({
         logoutMode: 'silent',
       });
-      const manager = new AuthManager(configuration);
+      const manager = new AuthManager(configuration, passportEventEmitter);
 
       await manager.logout();
 
@@ -407,7 +410,7 @@ describe('AuthManager', () => {
       const configuration = getConfig({
         logoutMode: 'redirect',
       });
-      const manager = new AuthManager(configuration);
+      const manager = new AuthManager(configuration, passportEventEmitter);
 
       mockSignoutRedirect.mockRejectedValue(new Error(mockErrorMsg));
 
@@ -578,7 +581,7 @@ describe('AuthManager', () => {
         it('should set the endSessionEndpoint `returnTo` and `client_id` query string params', async () => {
           mockGetUser.mockReturnValue(mockOidcUser);
 
-          const am = new AuthManager(getConfig({ logoutRedirectUri }));
+          const am = new AuthManager(getConfig({ logoutRedirectUri }), passportEventEmitter);
           const result = await am.getDeviceFlowEndSessionEndpoint();
           const uri = new URL(result);
 
@@ -593,7 +596,7 @@ describe('AuthManager', () => {
         it('should return the endSessionEndpoint without a `returnTo` or `client_id` query string params', async () => {
           mockGetUser.mockReturnValue(mockOidcUser);
 
-          const am = new AuthManager(getConfig());
+          const am = new AuthManager(getConfig(), passportEventEmitter);
           const result = await am.getDeviceFlowEndSessionEndpoint();
           const uri = new URL(result);
 

packages/passport/sdk/src/authManager.ts

@@ -17,7 +17,6 @@ import logger from './utils/logger';
 import { isTokenExpired } from './utils/token';
 import { PassportError, PassportErrorType, withPassportError } from './errors/passportError';
 import {
-  PassportMetadata,
   User,
   DeviceCodeResponse,
   DeviceConnectResponse,
@@ -29,9 +28,11 @@ import {
   isUserZkEvm,
   UserImx,
   isUserImx,
+  PassportEventMap,
 } from './types';
 import { PassportConfiguration } from './config';
 import Overlay from './overlay';
+import TypedEventEmitter from './utils/typedEventEmitter';
 import { LocalForageAsyncStorage } from './storage/LocalForageAsyncStorage';
 
 const formUrlEncodedHeader = {
@@ -111,26 +112,29 @@ export default class AuthManager {
 
   private readonly config: PassportConfiguration;
 
+  private readonly passportEventEmitter: TypedEventEmitter<PassportEventMap>;
+
   private readonly logoutMode: Exclude<OidcConfiguration['logoutMode'], undefined>;
 
   /**
    * Promise that is used to prevent multiple concurrent calls to the refresh token endpoint.
    */
   private refreshingPromise: Promise<User | null> | null = null;
 
-  constructor(config: PassportConfiguration) {
+  constructor(config: PassportConfiguration, passportEventEmitter: TypedEventEmitter<PassportEventMap>) {
     this.config = config;
+    this.passportEventEmitter = passportEventEmitter;
     this.userManager = new UserManager(getAuthConfiguration(config));
     this.deviceCredentialsManager = new DeviceCredentialsManager();
     this.logoutMode = config.oidcConfiguration.logoutMode || 'redirect';
   }
 
   private static mapOidcUserToDomainModel = (oidcUser: OidcUser): User => {
-    let passport: PassportMetadata | undefined;
-    if (oidcUser.id_token) {
-      passport = jwt_decode<IdTokenPayload>(oidcUser.id_token)?.passport;
+    if (!oidcUser.id_token) {
+      throw new Error('Failed to obtain ID token');
     }
 
+    const passport = jwt_decode<IdTokenPayload>(oidcUser.id_token)?.passport;
     const user: User = {
       expired: oidcUser.expired,
       idToken: oidcUser.id_token,
@@ -205,7 +209,8 @@ export default class AuthManager {
       return new Promise((resolve, reject) => {
         signinPopup()
           .then((oidcUser) => {
-            resolve(AuthManager.mapOidcUserToDomainModel(oidcUser));
+            const user = AuthManager.mapOidcUserToDomainModel(oidcUser);
+            resolve(user);
           })
           .catch((error: unknown) => {
             // Reject with the error if it is not caused by a blocked popup
@@ -226,7 +231,8 @@ export default class AuthManager {
                     popupHasBeenOpened = true;
                     const oidcUser = await signinPopup();
                     overlay.remove();
-                    resolve(AuthManager.mapOidcUserToDomainModel(oidcUser));
+                    const user = AuthManager.mapOidcUserToDomainModel(oidcUser);
+                    resolve(user);
                   } 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
@@ -313,7 +319,6 @@ export default class AuthManager {
           const oidcUser = AuthManager.mapDeviceTokenResponseToOidcUser(tokenResponse);
           const user = AuthManager.mapOidcUserToDomainModel(oidcUser);
           await this.userManager.storeUser(oidcUser);
-
           return user;
         } catch (error) {
           if (axios.isAxiosError(error)) {