NOJIRA Fix Passport registration issue not polling user info (#935)

immutable · Oct 4, 2023 · f6105c0 · f6105c0
1 parent 5c353cc
commit f6105c0
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 13 deletions.
diff --git a/packages/passport/sdk/src/Passport.int.test.ts b/packages/passport/sdk/src/Passport.int.test.ts
@@ -134,8 +134,7 @@ describe('Passport', () => {
 
         it('registers the user and returns the ether key', async () => {
           mockSigninPopup.mockResolvedValue(mockOidcUser);
-          mockGetUser.mockResolvedValueOnce(null);
-          mockGetUser.mockResolvedValueOnce(mockOidcUserZkevm);
+          mockSigninSilent.mockResolvedValueOnce(mockOidcUserZkevm);
           useMswHandlers([
             mswHandlers.counterfactualAddress.success,
           ]);
@@ -147,7 +146,7 @@ describe('Passport', () => {
           });
 
           expect(accounts).toEqual([mockOidcUserZkevm.profile.passport.zkevm_eth_address]);
-          expect(mockGetUser).toHaveBeenCalledTimes(2);
+          expect(mockGetUser).toHaveBeenCalledTimes(1);
           expect(mockMagicRequest).toHaveBeenCalledTimes(3);
         });
 

diff --git a/packages/passport/sdk/src/authManager.test.ts b/packages/passport/sdk/src/authManager.test.ts
@@ -334,6 +334,18 @@ describe('AuthManager', () => {
   });
 
   describe('getUser', () => {
+    describe('when forceRefresh is set to true', () => {
+      it('should call signinSilent and return the domain model', async () => {
+        signinSilentMock.mockReturnValue(mockOidcUser);
+
+        const result = await authManager.getUser({ forceRefresh: true });
+
+        expect(result).toEqual(mockUser);
+        expect(signinSilentMock).toBeCalled();
+        expect(getUserMock).not.toBeCalled();
+      });
+    });
+
     it('should retrieve the user from the userManager and return the domain model', async () => {
       getUserMock.mockReturnValue(mockOidcUser);
       (isTokenExpired as jest.Mock).mockReturnValue(false);

diff --git a/packages/passport/sdk/src/authManager.ts b/packages/passport/sdk/src/authManager.ts
@@ -369,15 +369,27 @@ export default class AuthManager {
     }, PassportErrorType.LOGOUT_ERROR);
   }
 
-  public async loginSilent(): Promise<User | null> {
-    return withPassportError<User | null>(async () => this.getUser(), PassportErrorType.SILENT_LOGIN_ERROR);
+  public async loginSilent({ forceRefresh } = { forceRefresh: false }): Promise<User | null> {
+    // eslint-disable-next-line arrow-body-style
+    return withPassportError<User | null>(async () => {
+      return this.getUser({ forceRefresh });
+    }, PassportErrorType.SILENT_LOGIN_ERROR);
   }
 
-  private async getWebUser() : Promise<User | null> {
-    const oidcUser = await this.userManager.getUser();
-    if (!oidcUser) {
-      return null;
+  /**
+   * Get the user from the cache or refresh the token if it's expired.
+   * @param forceRefresh If set to true, force an HTTP call to the OIDC server's authorization endpoint. This call will
+   * throw an error if there's no refresh token.
+   */
+  private async getWebUser({ forceRefresh = false }: { forceRefresh: boolean }) : Promise<User | null> {
+    if (forceRefresh) {
+      const newOidcUser = await this.userManager.signinSilent();
+      return newOidcUser ? AuthManager.mapOidcUserToDomainModel(newOidcUser) : null;
     }
+
+    const oidcUser = await this.userManager.getUser();
+    if (!oidcUser) return null;
+
     const tokenExpired = isTokenExpired(oidcUser);
     if (!tokenExpired) {
       return AuthManager.mapOidcUserToDomainModel(oidcUser);
@@ -391,9 +403,9 @@ export default class AuthManager {
     return null;
   }
 
-  public async getUser(): Promise<User | null> {
+  public async getUser({ forceRefresh } = { forceRefresh: false }): Promise<User | null> {
     return withPassportError<User | null>(async () => {
-      const user = await this.getWebUser();
+      const user = await this.getWebUser({ forceRefresh });
       if (user) {
         return user;
       }

diff --git a/packages/passport/sdk/src/starkEx/passportImxProviderFactory.test.ts b/packages/passport/sdk/src/starkEx/passportImxProviderFactory.test.ts
@@ -108,6 +108,10 @@ describe('PassportImxProviderFactory', () => {
             usersApi: immutableXClient.usersApi,
           }, mockUser.accessToken);
           expect(authManagerMock.loginSilent).toHaveBeenCalledTimes(4);
+          expect(authManagerMock.loginSilent).toHaveBeenNthCalledWith(1, { forceRefresh: true });
+          expect(authManagerMock.loginSilent).toHaveBeenNthCalledWith(2, { forceRefresh: true });
+          expect(authManagerMock.loginSilent).toHaveBeenNthCalledWith(3, { forceRefresh: true });
+          expect(authManagerMock.loginSilent).toHaveBeenCalledWith({ forceRefresh: true });
         });
       });
 
@@ -131,6 +135,7 @@ describe('PassportImxProviderFactory', () => {
             usersApi: immutableXClient.usersApi,
           }, mockUserImx.accessToken);
           expect(authManagerMock.loginSilent).toHaveBeenCalledTimes(1);
+          expect(authManagerMock.loginSilent).toHaveBeenCalledWith({ forceRefresh: true });
           expect(PassportImxProvider).toHaveBeenCalledWith({
             user: mockUserImx,
             starkSigner: starkSignerMock,

diff --git a/packages/passport/sdk/src/starkEx/passportImxProviderFactory.ts b/packages/passport/sdk/src/starkEx/passportImxProviderFactory.ts
@@ -140,7 +140,7 @@ export class PassportImxProviderFactory {
 
       // User metadata is updated asynchronously. Poll userinfo endpoint until it is updated.
       const updatedUser = await retryWithDelay<User | null>(async () => {
-        const user = await this.authManager.loginSilent();
+        const user = await this.authManager.loginSilent({ forceRefresh: true }); // force refresh to get updated user info
         const metadataExists = !!user?.imx;
         if (metadataExists) {
           return user;

diff --git a/packages/passport/sdk/src/zkEvm/user/registerZkEvmUser.test.ts b/packages/passport/sdk/src/zkEvm/user/registerZkEvmUser.test.ts
@@ -122,5 +122,6 @@ describe('registerZkEvmUser', () => {
         Authorization: `Bearer ${accessToken}`,
       },
     });
+    expect(authManager.loginSilent).toHaveBeenCalledWith({ forceRefresh: true });
   });
 });
diff --git a/packages/passport/sdk/src/zkEvm/user/registerZkEvmUser.ts b/packages/passport/sdk/src/zkEvm/user/registerZkEvmUser.ts
@@ -52,7 +52,7 @@ export async function registerZkEvmUser({
     }
     user = await authManager.refreshToken(credentials.refresh_token);
   } else {
-    user = await authManager.loginSilent();
+    user = await authManager.loginSilent({ forceRefresh: true });
   }
   if (!user?.zkEvm) {
     throw new JsonRpcError(RpcErrorCode.INTERNAL_ERROR, 'Failed to refresh user details');

diff --git a/sdk/package.json b/sdk/package.json
@@ -24,6 +24,7 @@
     "bn.js": "^5.2.1",
     "enc-utils": "^3.0.0",
     "ethers": "^5.7.2",
+    "jwt-decode": "^3.1.2",
     "magic-sdk": "^13.3.1",
     "oidc-client-ts": "^2.2.1"
   },

diff --git a/yarn.lock b/yarn.lock
@@ -3755,6 +3755,7 @@ __metadata:
     eslint: ^8.40.0
     ethers: ^5.7.2
     glob: ^10.2.3
+    jwt-decode: ^3.1.2
     magic-sdk: ^13.3.1
     oidc-client-ts: ^2.2.1
     rollup: ^3.17.2