diff --git a/packages/passport/sdk/src/Passport.int.test.ts b/packages/passport/sdk/src/Passport.int.test.ts index f50d3b57a7..4ad38106d5 100644 --- a/packages/passport/sdk/src/Passport.int.test.ts +++ b/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, diff --git a/packages/passport/sdk/src/authManager.test.ts b/packages/passport/sdk/src/authManager.test.ts index 209fccdaa0..b85b8c4311 100644 --- a/packages/passport/sdk/src/authManager.test.ts +++ b/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); diff --git a/packages/passport/sdk/src/authManager.ts b/packages/passport/sdk/src/authManager.ts index 4eba9d222c..910ef4a52c 100644 --- a/packages/passport/sdk/src/authManager.ts +++ b/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 { PassportMetadata, @@ -458,13 +458,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)) { diff --git a/packages/passport/sdk/src/utils/token.test.ts b/packages/passport/sdk/src/utils/token.test.ts index 876265c567..e42bee942b 100644 --- a/packages/passport/sdk/src/utils/token.test.ts +++ b/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); }); }); diff --git a/packages/passport/sdk/src/utils/token.ts b/packages/passport/sdk/src/utils/token.ts index 2d39048764..659674a2fe 100644 --- a/packages/passport/sdk/src/utils/token.ts +++ b/packages/passport/sdk/src/utils/token.ts @@ -2,22 +2,34 @@ import jwt_decode from 'jwt-decode'; import { User as OidcUser, } from 'oidc-client-ts'; -import { IdTokenPayload } from '../types'; +import { IdTokenPayload, TokenPayload } from '../types'; -export function isIdTokenExpired(idToken: string | undefined): boolean { - if (!idToken) { - return false; - } +function isTokenExpiredOrExpiring(token: string): boolean { + try { + // try to decode the token as access token payload or id token payload + const decodedToken = jwt_decode(token); + const now = Math.floor(Date.now() / 1000); + + // Tokens without expiration claims are invalid (security vulnerability) + if (!decodedToken.exp) { + return true; + } - const decodedToken = jwt_decode(idToken); - const now = Math.floor(Date.now() / 1000); - return decodedToken.exp < now; + // Check if token is expired or expiring in 30 seconds or less + return decodedToken.exp <= now + 30; + } catch (error) { + // If we can't decode the token, assume it's invalid + return true; + } } -export function isTokenExpired(oidcUser: OidcUser): boolean { - const { id_token: idToken, expired } = oidcUser; - if (expired) { +export function isAccessTokenExpiredOrExpiring(oidcUser: OidcUser): boolean { + const { id_token: idToken, access_token: accessToken } = oidcUser; + + if (!accessToken || !idToken) { return true; } - return isIdTokenExpired(idToken); + + // Check if either token is expired or expiring + return isTokenExpiredOrExpiring(accessToken) || isTokenExpiredOrExpiring(idToken); }