fix(passport): update access token expiry check (#2657)

Author: CodeSchwert

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

@@ -29,6 +29,17 @@ const redirectUri = 'example.com';
 const popupRedirectUri = 'example.com';
 const logoutRedirectUri = 'example.com';
 const clientId = 'clientId123';
+const now = Math.floor(Date.now() / 1000);
+const oneHourLater = now + 3600;
+
+const mockValidAccessToken = encode({
+  iss: 'https://example.auth0.com/',
+  aud: 'https://api.example.com/',
+  sub: 'sub123',
+  iat: now,
+  exp: oneHourLater,
+}, 'secret');
+
 const mockOidcUser = {
   profile: {
     sub: 'sub123',
@@ -37,13 +48,20 @@ const mockOidcUser = {
   },
   expired: false,
   id_token: mockValidIdToken,
-  access_token: 'accessToken123',
+  access_token: mockValidAccessToken,
   refresh_token: 'refreshToken123',
 };
 
 const mockOidcUserZkevm = {
   ...mockOidcUser,
   id_token: encode({
+    iss: 'https://example.auth0.com/',
+    aud: 'clientId123',
+    sub: 'sub123',
+    iat: now,
+    exp: oneHourLater,
+    email: 'test@example.com',
+    nickname: 'test',
     passport: {
       zkevm_eth_address: mockUserZkEvm.zkEvm.ethAddress,
       zkevm_user_admin_address: mockUserZkEvm.zkEvm.userAdminAddress,

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

@@ -6,7 +6,7 @@ import Overlay from './overlay';
 import { PassportError, PassportErrorType } from './errors/passportError';
 import { PassportConfiguration } from './config';
 import { mockUser, mockUserImx, mockUserZkEvm } from './test/mocks';
-import { isTokenExpired } from './utils/token';
+import { isAccessTokenExpiredOrExpiring } from './utils/token';
 import { isUserZkEvm, PassportModuleConfiguration } from './types';
 
 jest.mock('jwt-decode');
@@ -352,7 +352,7 @@ describe('AuthManager', () => {
     describe('when getUser returns a user', () => {
       it('should return the user', async () => {
         mockGetUser.mockReturnValue(mockOidcUser);
-        (isTokenExpired as jest.Mock).mockReturnValue(false);
+        (isAccessTokenExpiredOrExpiring as jest.Mock).mockReturnValue(false);
 
         const result = await authManager.getUserOrLogin();
 
@@ -364,7 +364,7 @@ describe('AuthManager', () => {
       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);
+        (isAccessTokenExpiredOrExpiring as jest.Mock).mockReturnValue(false);
 
         const result = await authManager.getUserOrLogin();
 
@@ -510,16 +510,67 @@ describe('AuthManager', () => {
   describe('getUser', () => {
     it('should retrieve the user from the userManager and return the domain model', async () => {
       mockGetUser.mockReturnValue(mockOidcUser);
-      (isTokenExpired as jest.Mock).mockReturnValue(false);
+      (isAccessTokenExpiredOrExpiring as jest.Mock).mockReturnValue(false);
 
       const result = await authManager.getUser();
 
       expect(result).toEqual(mockUser);
     });
 
+    it('should return null when user has no idToken', async () => {
+      const userWithoutIdToken = { ...mockOidcUser, id_token: undefined, refresh_token: undefined };
+      mockGetUser.mockReturnValue(userWithoutIdToken);
+      // Restore real function behavior for this test
+      (isAccessTokenExpiredOrExpiring as jest.Mock).mockImplementation(
+        jest.requireActual('./utils/token').isAccessTokenExpiredOrExpiring,
+      );
+
+      const result = await authManager.getUser();
+
+      expect(result).toBeNull();
+    });
+
+    it('should refresh token when access token is expired or expiring', async () => {
+      const userWithExpiringAccessToken = { ...mockOidcUser };
+      mockGetUser.mockReturnValue(userWithExpiringAccessToken);
+      (isAccessTokenExpiredOrExpiring as jest.Mock).mockReturnValue(true);
+      mockSigninSilent.mockResolvedValue(mockOidcUser);
+
+      const result = await authManager.getUser();
+
+      expect(mockSigninSilent).toBeCalledTimes(1);
+      expect(result).toEqual(mockUser);
+    });
+
+    it('should handle user with missing access token', async () => {
+      const userWithoutAccessToken = { ...mockOidcUser, access_token: undefined };
+      mockGetUser.mockReturnValue(userWithoutAccessToken);
+      // Restore real function behavior for this test
+      (isAccessTokenExpiredOrExpiring as jest.Mock).mockImplementation(
+        jest.requireActual('./utils/token').isAccessTokenExpiredOrExpiring,
+      );
+      mockSigninSilent.mockResolvedValue(mockOidcUser);
+
+      const result = await authManager.getUser();
+
+      expect(mockSigninSilent).toBeCalledTimes(1);
+      expect(result).toEqual(mockUser);
+    });
+
+    it('should return user directly when access token is not expired or expiring', async () => {
+      const freshUser = { ...mockOidcUser };
+      mockGetUser.mockReturnValue(freshUser);
+      (isAccessTokenExpiredOrExpiring as jest.Mock).mockReturnValue(false);
+
+      const result = await authManager.getUser();
+
+      expect(mockSigninSilent).not.toHaveBeenCalled();
+      expect(result).toEqual(mockUser);
+    });
+
     it('should call signinSilent and returns user when user token is expired with the refresh token', async () => {
       mockGetUser.mockReturnValue(mockOidcExpiredUser);
-      (isTokenExpired as jest.Mock).mockReturnValue(true);
+      (isAccessTokenExpiredOrExpiring as jest.Mock).mockReturnValue(true);
       mockSigninSilent.mockResolvedValue(mockOidcUser);
 
       const result = await authManager.getUser();
@@ -530,7 +581,7 @@ describe('AuthManager', () => {
 
     it('should reject with an error when signinSilent throws a string', async () => {
       mockGetUser.mockReturnValue(mockOidcExpiredUser);
-      (isTokenExpired as jest.Mock).mockReturnValue(true);
+      (isAccessTokenExpiredOrExpiring as jest.Mock).mockReturnValue(true);
       mockSigninSilent.mockRejectedValue('oops');
 
       await expect(() => authManager.getUser()).rejects.toThrow(
@@ -543,7 +594,7 @@ describe('AuthManager', () => {
 
     it('should return null when the user token is expired without refresh token', async () => {
       mockGetUser.mockReturnValue(mockOidcExpiredNoRefreshTokenUser);
-      (isTokenExpired as jest.Mock).mockReturnValue(true);
+      (isAccessTokenExpiredOrExpiring as jest.Mock).mockReturnValue(true);
 
       const result = await authManager.getUser();
 
@@ -553,7 +604,7 @@ describe('AuthManager', () => {
 
     it('should return null when the user token is expired with the refresh token, but signinSilent returns null', async () => {
       mockGetUser.mockReturnValue(mockOidcExpiredUser);
-      (isTokenExpired as jest.Mock).mockReturnValue(true);
+      (isAccessTokenExpiredOrExpiring as jest.Mock).mockReturnValue(true);
       mockSigninSilent.mockResolvedValue(null);
       const result = await authManager.getUser();
 
@@ -584,7 +635,7 @@ describe('AuthManager', () => {
       describe('when the user is expired', () => {
         it('should only call refresh the token once', async () => {
           mockGetUser.mockReturnValue(mockOidcExpiredUser);
-          (isTokenExpired as jest.Mock).mockReturnValue(true);
+          (isAccessTokenExpiredOrExpiring as jest.Mock).mockReturnValue(true);
           mockSigninSilent.mockReturnValue(mockOidcUser);
 
           await Promise.allSettled([
@@ -600,7 +651,7 @@ describe('AuthManager', () => {
     describe('when the user does not meet the type assertion', () => {
       it('should return null', async () => {
         mockGetUser.mockReturnValue(mockOidcUser);
-        (isTokenExpired as jest.Mock).mockReturnValue(false);
+        (isAccessTokenExpiredOrExpiring as jest.Mock).mockReturnValue(false);
 
         const result = await authManager.getUser(isUserZkEvm);
 
@@ -617,7 +668,7 @@ describe('AuthManager', () => {
             zkevm_user_admin_address: mockUserZkEvm.zkEvm.userAdminAddress,
           },
         });
-        (isTokenExpired as jest.Mock).mockReturnValue(false);
+        (isAccessTokenExpiredOrExpiring as jest.Mock).mockReturnValue(false);
 
         const result = await authManager.getUser(isUserZkEvm);
 

packages/passport/sdk/src/authManager.ts

@@ -13,7 +13,7 @@ import { getDetail, Detail } from '@imtbl/metrics';
 import localForage from 'localforage';
 import DeviceCredentialsManager from './storage/device_credentials_manager';
 import logger from './utils/logger';
-import { isTokenExpired } from './utils/token';
+import { isAccessTokenExpiredOrExpiring } from './utils/token';
 import { PassportError, PassportErrorType, withPassportError } from './errors/passportError';
 import {
   DirectLoginMethod,
@@ -475,13 +475,15 @@ export default class AuthManager {
     const oidcUser = await this.userManager.getUser();
     if (!oidcUser) return null;
 
-    if (!isTokenExpired(oidcUser)) {
+    // if the token is not expired or expiring in 30 seconds or less, return the user
+    if (!isAccessTokenExpiredOrExpiring(oidcUser)) {
       const user = AuthManager.mapOidcUserToDomainModel(oidcUser);
       if (user && typeAssertion(user)) {
         return user;
       }
     }
 
+    // if the token is expired or expiring in 30 seconds or less, refresh the token
     if (oidcUser.refresh_token) {
       const user = await this.refreshTokenAndUpdatePromise();
       if (user && typeAssertion(user)) {

packages/passport/sdk/src/utils/token.test.ts

@@ -2,57 +2,120 @@ import encode from 'jwt-encode';
 import {
   User as OidcUser,
 } from 'oidc-client-ts';
-import { isIdTokenExpired, isTokenExpired } from './token';
+import { isAccessTokenExpiredOrExpiring } from './token';
 
 const now = Math.floor(Date.now() / 1000);
 const oneHourLater = now + 3600;
 const oneHourBefore = now - 3600;
+const fifteenSecondsLater = now + 15;
+const fortyFiveSecondsLater = now + 45;
 
 const mockExpiredIdToken = encode({
   iat: oneHourBefore,
   exp: oneHourBefore,
 }, 'secret');
+
 export const mockValidIdToken = encode({
   iat: now,
   exp: oneHourLater,
 }, 'secret');
 
-describe('isIdTokenExpired', () => {
-  it('should return false if idToken is undefined', () => {
-    expect(isIdTokenExpired(undefined)).toBe(false);
-  });
+const mockFreshAccessToken = encode({
+  exp: fortyFiveSecondsLater, // Expires in 45 seconds (outside 30-second buffer)
+}, 'secret');
 
-  it('should return true if idToken is expired', () => {
-    expect(isIdTokenExpired(mockExpiredIdToken)).toBe(true);
+describe('isAccessTokenExpiredOrExpiring', () => {
+  it('should return true if access token is missing', () => {
+    const user = {
+      id_token: mockValidIdToken,
+      access_token: undefined,
+    } as unknown as OidcUser;
+    expect(isAccessTokenExpiredOrExpiring(user)).toBe(true);
   });
 
-  it('should return false if idToken is not expired', () => {
-    expect(isIdTokenExpired(mockValidIdToken)).toBe(false);
+  it('should return true if id token is missing', () => {
+    const mockValidAccessToken = encode({
+      exp: oneHourLater,
+    }, 'secret');
+
+    const user = {
+      id_token: undefined,
+      access_token: mockValidAccessToken,
+    } as unknown as OidcUser;
+    expect(isAccessTokenExpiredOrExpiring(user)).toBe(true);
   });
-});
 
-describe('isTokenExpired', () => {
-  it('should return true if expired is true', () => {
+  it('should return true if access token is expired', () => {
+    const mockExpiredAccessToken = encode({
+      exp: oneHourBefore,
+    }, 'secret');
+
     const user = {
       id_token: mockValidIdToken,
-      expired: true,
+      access_token: mockExpiredAccessToken,
     } as unknown as OidcUser;
-    expect(isTokenExpired(user)).toBe(true);
+    expect(isAccessTokenExpiredOrExpiring(user)).toBe(true);
   });
 
-  it('should return false if idToken is valid', () => {
+  it('should return true if access token is expiring within 30 seconds', () => {
+    const mockExpiringAccessToken = encode({
+      exp: fifteenSecondsLater, // Expires in 15 seconds (within 30-second buffer)
+    }, 'secret');
+
     const user = {
       id_token: mockValidIdToken,
-      expired: false,
+      access_token: mockExpiringAccessToken,
     } as unknown as OidcUser;
-    expect(isTokenExpired(user)).toBe(false);
+    expect(isAccessTokenExpiredOrExpiring(user)).toBe(true);
   });
 
-  it('should return true idToken is expired', () => {
+  it('should return true if access token is valid but id token is expired', () => {
     const user = {
       id_token: mockExpiredIdToken,
-      expired: false,
+      access_token: mockFreshAccessToken,
+    } as unknown as OidcUser;
+    expect(isAccessTokenExpiredOrExpiring(user)).toBe(true);
+  });
+
+  it('should return true if access token is valid but id token is expiring within 30 seconds', () => {
+    const expiringIdToken = encode({
+      iat: now,
+      exp: now + 15, // Expires in 15 seconds
+    }, 'secret');
+
+    const user = {
+      id_token: expiringIdToken,
+      access_token: mockFreshAccessToken,
+    } as unknown as OidcUser;
+    expect(isAccessTokenExpiredOrExpiring(user)).toBe(true);
+  });
+
+  it('should return false if both tokens are valid and not expiring', () => {
+    const user = {
+      id_token: mockValidIdToken,
+      access_token: mockFreshAccessToken,
+    } as unknown as OidcUser;
+    expect(isAccessTokenExpiredOrExpiring(user)).toBe(false);
+  });
+
+  it('should return true if access token is malformed', () => {
+    const user = {
+      id_token: mockValidIdToken,
+      access_token: 'invalid-jwt-token',
+    } as unknown as OidcUser;
+    expect(isAccessTokenExpiredOrExpiring(user)).toBe(true);
+  });
+
+  it('should return true if access token has no exp claim (security vulnerability)', () => {
+    const accessTokenWithoutExp = encode({
+      iat: now,
+      sub: 'user123',
+    }, 'secret');
+
+    const user = {
+      id_token: mockValidIdToken,
+      access_token: accessTokenWithoutExp,
     } as unknown as OidcUser;
-    expect(isTokenExpired(user)).toBe(true);
+    expect(isAccessTokenExpiredOrExpiring(user)).toBe(true);
   });
 });