diff --git a/packages/seedless-onboarding-controller/CHANGELOG.md b/packages/seedless-onboarding-controller/CHANGELOG.md index 64ce4032900..4f4d6f44bfe 100644 --- a/packages/seedless-onboarding-controller/CHANGELOG.md +++ b/packages/seedless-onboarding-controller/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Token refresh now uses a dedicated private `#reAuthenticate` method that only accepts the tokens returned by the JWT-refresh service (`idTokens`, `accessToken`, `metadataAccessToken`), making the re-authentication path stricter than the initial `authenticate` call ([#8148](https://github.com/MetaMask/core/pull/8148)) +- After a successful JWT refresh the controller proactively rotates the refresh/revoke token pair (when the vault is unlocked) via the public `rotateRefreshToken` method ([#8148](https://github.com/MetaMask/core/pull/8148)) +- Token expiry checks now use a 90% lifetime threshold for proactive refresh: access tokens and metadata access tokens are refreshed when less than 10% of their lifetime remains (requires `iat`), while node auth tokens fall back to exact-expiry checking ([#8148](https://github.com/MetaMask/core/pull/8148)) + +### Removed + +- **BREAKING:** Removed public `renewRefreshToken` method — refresh token rotation is now handled by `rotateRefreshToken`, which can also be called directly ([#8148](https://github.com/MetaMask/core/pull/8148)) + ## [8.1.0] ### Changed diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index 2c87ed22f4b..a0fbe0e3b67 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts @@ -1033,6 +1033,33 @@ describe('SeedlessOnboardingController', () => { ); }); }); + + it('should skip the controller lock when skipLock is true', async () => { + await withController(async ({ controller, toprfClient }) => { + jest.spyOn(toprfClient, 'authenticate').mockResolvedValue({ + nodeAuthTokens: MOCK_NODE_AUTH_TOKENS, + isNewUser: false, + }); + + const authResult = await controller.authenticate({ + idTokens, + authConnectionId, + userId, + authConnection, + socialLoginEmail, + refreshToken, + revokeToken, + accessToken, + metadataAccessToken, + skipLock: true, + }); + + expect(authResult.isNewUser).toBe(false); + expect(controller.state.isSeedlessOnboardingUserAuthenticated).toBe( + true, + ); + }); + }); }); describe('checkPasswordOutdated', () => { @@ -2578,6 +2605,47 @@ describe('SeedlessOnboardingController', () => { }, ); }); + + it('should throw InvalidRevokeToken if vault data is missing revokeToken', async () => { + const mockToprfEncryptor = createMockToprfEncryptor(); + const encKey = mockToprfEncryptor.deriveEncKey(MOCK_PASSWORD); + const pwEncKey = mockToprfEncryptor.derivePwEncKey(MOCK_PASSWORD); + const authKeyPair = mockToprfEncryptor.deriveAuthKeyPair(MOCK_PASSWORD); + const encryptor = createMockVaultEncryptor(); + + // Build a vault without the revokeToken field. + const vaultPayload = JSON.stringify({ + toprfEncryptionKey: bytesToBase64(encKey), + toprfPwEncryptionKey: bytesToBase64(pwEncKey), + toprfAuthKeyPair: JSON.stringify({ + sk: `0x${authKeyPair.sk.toString(16)}`, + pk: bytesToBase64(authKeyPair.pk), + }), + accessToken, + // revokeToken intentionally omitted + }); + + const { vault: vaultWithoutRevokeToken, exportedKeyString } = + await encryptor.encryptWithDetail(MOCK_PASSWORD, vaultPayload); + + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + vault: vaultWithoutRevokeToken, + vaultEncryptionKey: exportedKeyString, + vaultEncryptionSalt: JSON.parse(vaultWithoutRevokeToken).salt, + }), + }, + async ({ controller }) => { + await expect( + controller.submitPassword(MOCK_PASSWORD), + ).rejects.toThrow( + SeedlessOnboardingControllerErrorMessage.InvalidRevokeToken, + ); + }, + ); + }); }); describe('verifyPassword', () => { @@ -4407,6 +4475,58 @@ describe('SeedlessOnboardingController', () => { }, ); }); + + it('should return true when node auth token is past its expiry', async () => { + const expiredToken = createMockNodeAuthToken({ + exp: Math.floor(Date.now() / 1000) - 1, + }); + await withController( + { + state: { + ...getMockInitialControllerState({ + withMockAuthenticatedUser: true, + }), + // assertIsAuthenticatedUser requires ≥ 3 tokens; only the first is checked + nodeAuthTokens: MOCK_NODE_AUTH_TOKENS.map((token, i) => ({ + ...token, + authToken: i === 0 ? expiredToken : token.authToken, + })), + }, + }, + async ({ controller }) => { + jest.spyOn(controller, 'checkNodeAuthTokenExpired').mockRestore(); + + const isExpired = controller.checkNodeAuthTokenExpired(); + expect(isExpired).toBe(true); + }, + ); + }); + + it('should return false when node auth token has not yet reached its expiry', async () => { + const validToken = createMockNodeAuthToken({ + exp: Math.floor(Date.now() / 1000) + 7000, + }); + await withController( + { + state: { + ...getMockInitialControllerState({ + withMockAuthenticatedUser: true, + }), + // assertIsAuthenticatedUser requires ≥ 3 tokens; only the first is checked + nodeAuthTokens: MOCK_NODE_AUTH_TOKENS.map((token, idx) => ({ + ...token, + authToken: idx === 0 ? validToken : token.authToken, + })), + }, + }, + async ({ controller }) => { + jest.spyOn(controller, 'checkNodeAuthTokenExpired').mockRestore(); + + const isExpired = controller.checkNodeAuthTokenExpired(); + expect(isExpired).toBe(false); + }, + ); + }); }); describe('checkIsPasswordOutdated with token refresh', () => { @@ -4673,6 +4793,10 @@ describe('SeedlessOnboardingController', () => { // Unlock controller first await controller.submitPassword(OLD_PASSWORD); + // Capture before the call — proactive renewRefreshToken inside + // refreshAuthTokens will rotate state.refreshToken afterwards. + const originalRefreshToken = controller.state.refreshToken; + const recoverEncKeySpy = jest.spyOn(toprfClient, 'recoverEncKey'); const encryptorSpy = jest.spyOn(encryptor, 'encryptWithDetail'); @@ -4713,7 +4837,7 @@ describe('SeedlessOnboardingController', () => { // Verify that getNewRefreshToken was called expect(mockRefreshJWTToken).toHaveBeenCalledWith({ connection: controller.state.authConnection, - refreshToken: controller.state.refreshToken, + refreshToken: originalRefreshToken, }); // Verify that recoverEncKey was called twice (once failed, once succeeded) @@ -5192,6 +5316,10 @@ describe('SeedlessOnboardingController', () => { async ({ controller, toprfClient, mockRefreshJWTToken }) => { await controller.submitPassword(MOCK_PASSWORD); + // Capture before the call — proactive renewRefreshToken inside + // refreshAuthTokens will rotate state.refreshToken afterwards. + const originalRefreshToken = controller.state.refreshToken; + // Mock authenticate for token refresh jest.spyOn(toprfClient, 'authenticate').mockResolvedValue({ nodeAuthTokens: MOCK_NODE_AUTH_TOKENS, @@ -5202,7 +5330,7 @@ describe('SeedlessOnboardingController', () => { expect(mockRefreshJWTToken).toHaveBeenCalledWith({ connection: controller.state.authConnection, - refreshToken: controller.state.refreshToken, + refreshToken: originalRefreshToken, }); expect(toprfClient.authenticate).toHaveBeenCalledWith({ @@ -5560,66 +5688,214 @@ describe('SeedlessOnboardingController', () => { ); }); - it('should use live state.refreshToken in authenticate when token was rotated concurrently', async () => { + it('should re-authenticate using only the new idTokens from the JWT refresh', async () => { await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, }), }, - async ({ controller, mockRefreshJWTToken }) => { - const originalRefreshToken = controller.state.refreshToken; - const rotatedRefreshToken = 'rotated-refresh-token-from-renew'; + async ({ controller, toprfClient, mockRefreshJWTToken }) => { + const newIdTokens = ['newIdToken']; + mockRefreshJWTToken.mockResolvedValueOnce({ + idTokens: newIdTokens, + metadataAccessToken: 'mock-metadata-access-token', + accessToken, + }); - let resolveRefresh!: () => void; - const refreshBarrier = new Promise((resolve) => { - resolveRefresh = resolve; + const toprfAuthSpy = jest + .spyOn(toprfClient, 'authenticate') + .mockResolvedValue({ + nodeAuthTokens: MOCK_NODE_AUTH_TOKENS, + isNewUser: false, + }); + + await controller.refreshAuthTokens(); + + // #reAuthenticate passes only idTokens to the TOPRF client — + // refreshToken and revokeToken are structurally absent. + expect(toprfAuthSpy).toHaveBeenCalledWith( + expect.objectContaining({ idTokens: newIdTokens }), + ); + }, + ); + }); + + it('should proactively call renewRefreshToken after vault update when vault is unlocked', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + vault: encryptedMockVault, + vaultEncryptionKey, + vaultEncryptionSalt, + }), + }, + async ({ + controller, + toprfClient, + mockRenewRefreshToken, + mockRefreshJWTToken, + }) => { + await controller.submitPassword(MOCK_PASSWORD); + + mockRefreshJWTToken.mockResolvedValueOnce({ + idTokens: ['newIdToken'], + metadataAccessToken: 'mock-metadata-access-token', + accessToken, }); - mockRefreshJWTToken.mockImplementation(async () => { - await refreshBarrier; - return { - idTokens: ['newIdToken'], - metadataAccessToken: 'mock-metadata-access-token', - accessToken, - }; + jest.spyOn(toprfClient, 'authenticate').mockResolvedValue({ + nodeAuthTokens: MOCK_NODE_AUTH_TOKENS, + isNewUser: false, }); - // Spy on the controller's own authenticate method — this is the - // call that receives the refreshToken parameter. The inner - // toprfClient.authenticate call does not carry refreshToken. - const authenticateSpy = jest - .spyOn(controller, 'authenticate') - .mockResolvedValue(); + await controller.refreshAuthTokens(); - // Start the refresh — it is now blocked at the HTTP call. - const refreshPromise = controller.refreshAuthTokens(); + expect(mockRenewRefreshToken).toHaveBeenCalled(); + }, + ); + }); - // Simulate renewRefreshToken writing a new token to state while the - // HTTP call is in-flight. refreshAuthTokens holds no controller lock, - // so these two can genuinely overlap in production. - // @ts-expect-error Accessing protected method for testing - controller.update((state) => { - state.refreshToken = rotatedRefreshToken; + it('should not call renewRefreshToken when vault is locked', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + }), + }, + async ({ controller, toprfClient, mockRenewRefreshToken }) => { + jest.spyOn(toprfClient, 'authenticate').mockResolvedValue({ + nodeAuthTokens: MOCK_NODE_AUTH_TOKENS, + isNewUser: false, }); - // Release the HTTP call and let refreshAuthTokens complete. - resolveRefresh(); - await refreshPromise; - - // authenticate must have received the live state value, not the - // value captured at the start of the method. Using the stale - // captured value would overwrite the newer token in state, causing - // all subsequent refreshes to 401 once the old token is revoked. - expect(authenticateSpy).toHaveBeenCalledWith( - expect.objectContaining({ refreshToken: rotatedRefreshToken }), + await controller.refreshAuthTokens(); + + expect(mockRenewRefreshToken).not.toHaveBeenCalled(); + }, + ); + }); + + it('should swallow renewRefreshToken errors and still resolve when vault is unlocked', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + vault: encryptedMockVault, + vaultEncryptionKey, + vaultEncryptionSalt, + }), + }, + async ({ + controller, + toprfClient, + mockRenewRefreshToken, + mockRefreshJWTToken, + }) => { + await controller.submitPassword(MOCK_PASSWORD); + + mockRefreshJWTToken.mockResolvedValueOnce({ + idTokens: ['newIdToken'], + metadataAccessToken: 'mock-metadata-access-token', + accessToken, + }); + + jest.spyOn(toprfClient, 'authenticate').mockResolvedValue({ + nodeAuthTokens: MOCK_NODE_AUTH_TOKENS, + isNewUser: false, + }); + + // Make the proactive renewal fail — the error is caught and logged. + mockRenewRefreshToken.mockRejectedValueOnce( + new Error('renewal failed'), ); - expect(authenticateSpy).not.toHaveBeenCalledWith( - expect.objectContaining({ refreshToken: originalRefreshToken }), + + // refreshAuthTokens must still resolve even when renewal fails. + expect(await controller.refreshAuthTokens()).toBeUndefined(); + }, + ); + }); + + it('should queue old refresh token for revocation after successful rotation', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + vault: encryptedMockVault, + vaultEncryptionKey, + vaultEncryptionSalt, + }), + }, + async ({ controller, toprfClient, mockRefreshJWTToken }) => { + await controller.submitPassword(MOCK_PASSWORD); + + const originalRefreshToken = controller.state.refreshToken; + const originalRevokeToken = controller.state.revokeToken; + + mockRefreshJWTToken.mockResolvedValueOnce({ + idTokens: ['newIdToken'], + metadataAccessToken: 'mock-metadata-access-token', + accessToken, + }); + + jest.spyOn(toprfClient, 'authenticate').mockResolvedValue({ + nodeAuthTokens: MOCK_NODE_AUTH_TOKENS, + isNewUser: false, + }); + + await controller.refreshAuthTokens(); + + // After successful rotation the old token pair should be queued + // for background revocation. + expect(controller.state.pendingToBeRevokedTokens).toStrictEqual( + expect.arrayContaining([ + { + refreshToken: originalRefreshToken, + revokeToken: originalRevokeToken, + }, + ]), ); }, ); }); + + it('should update state.refreshToken after successful rotation', async () => { + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + vault: encryptedMockVault, + vaultEncryptionKey, + vaultEncryptionSalt, + }), + }, + async ({ controller, toprfClient, mockRefreshJWTToken }) => { + await controller.submitPassword(MOCK_PASSWORD); + + const originalRefreshToken = controller.state.refreshToken; + + mockRefreshJWTToken.mockResolvedValueOnce({ + idTokens: ['newIdToken'], + metadataAccessToken: 'mock-metadata-access-token', + accessToken, + }); + + jest.spyOn(toprfClient, 'authenticate').mockResolvedValue({ + nodeAuthTokens: MOCK_NODE_AUTH_TOKENS, + isNewUser: false, + }); + + await controller.refreshAuthTokens(); + + // rotateRefreshToken writes the new refresh token to state. + expect(controller.state.refreshToken).not.toBe( + originalRefreshToken, + ); + expect(controller.state.refreshToken).toBe('newRefreshToken'); + }, + ); + }); }); }); @@ -5691,12 +5967,72 @@ describe('SeedlessOnboardingController', () => { expect(controller.refreshAuthTokens).toHaveBeenCalled(); }); + + it('should call refreshAuthTokens when 90% of token lifetime has elapsed', async () => { + const now = Math.floor(Date.now() / 1000); + // 10 000 s lifetime; 9 500 s have elapsed → past the 90 % mark + const nearExpiryToken = createMockJWTToken({ + iat: now - 9500, + exp: now + 500, + }); + const { messenger } = mockSeedlessOnboardingMessenger(); + const controller = new SeedlessOnboardingController({ + messenger, + encryptor: createMockVaultEncryptor(), + refreshJWTToken: jest.fn(), + revokeRefreshToken: jest.fn(), + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + metadataAccessToken: nearExpiryToken, + }), + renewRefreshToken: jest.fn(), + }); + + jest.spyOn(controller, 'refreshAuthTokens').mockResolvedValue(); + + await controller.fetchMetadataAccessCreds(); + + expect(controller.refreshAuthTokens).toHaveBeenCalled(); + }); + + it('should not call refreshAuthTokens when less than 90% of token lifetime has elapsed', async () => { + const now = Math.floor(Date.now() / 1000); + // 10 000 s lifetime; only 3 000 s have elapsed → well below 90 % + const freshToken = createMockJWTToken({ + iat: now - 3000, + exp: now + 7000, + }); + const { messenger } = mockSeedlessOnboardingMessenger(); + const controller = new SeedlessOnboardingController({ + messenger, + encryptor: createMockVaultEncryptor(), + refreshJWTToken: jest.fn(), + revokeRefreshToken: jest.fn(), + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + metadataAccessToken: freshToken, + }), + renewRefreshToken: jest.fn(), + }); + + jest.spyOn(controller, 'refreshAuthTokens').mockResolvedValue(); + + const result = await controller.fetchMetadataAccessCreds(); + + expect(controller.refreshAuthTokens).not.toHaveBeenCalled(); + expect(result).toStrictEqual({ + metadataAccessToken: freshToken, + }); + }); }); describe('checkMetadataAccessTokenExpired', () => { it('should return false if metadata access token is not expired', async () => { - const futureExp = Math.floor(Date.now() / 1000) + 3600; // 1 hour from now - const validToken = createMockJWTToken({ exp: futureExp }); + const now = Math.floor(Date.now() / 1000); + const validToken = createMockJWTToken({ + iat: now, + exp: now + 3600, // 1 hour lifetime, 0% elapsed + }); await withController( { @@ -5718,8 +6054,11 @@ describe('SeedlessOnboardingController', () => { }); it('should return true if metadata access token is expired', async () => { - const pastExp = Math.floor(Date.now() / 1000) - 3600; // 1 hour ago - const expiredToken = createMockJWTToken({ exp: pastExp }); + const now = Math.floor(Date.now() / 1000); + const expiredToken = createMockJWTToken({ + iat: now - 7200, + exp: now - 3600, // issued 2 h ago, expired 1 h ago + }); await withController( { @@ -5740,6 +6079,85 @@ describe('SeedlessOnboardingController', () => { ); }); + it('should return true when 90% of token lifetime has elapsed', async () => { + const now = Math.floor(Date.now() / 1000); + // 10 000 s lifetime; 9 001 s have elapsed → just past the 90 % mark + const tokenAt90Percent = createMockJWTToken({ + iat: now - 9001, + exp: now + 999, + }); + + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + metadataAccessToken: tokenAt90Percent, + }), + }, + async ({ controller }) => { + jest + .spyOn(controller, 'checkMetadataAccessTokenExpired') + .mockRestore(); + + const result = controller.checkMetadataAccessTokenExpired(); + expect(result).toBe(true); + }, + ); + }); + + it('should return false when less than 90% of token lifetime has elapsed', async () => { + const now = Math.floor(Date.now() / 1000); + // 10 000 s lifetime; only 3 000 s have elapsed → well below the 90 % mark + const tokenBelow90Percent = createMockJWTToken({ + iat: now - 3000, + exp: now + 7000, + }); + + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + metadataAccessToken: tokenBelow90Percent, + }), + }, + async ({ controller }) => { + jest + .spyOn(controller, 'checkMetadataAccessTokenExpired') + .mockRestore(); + + const result = controller.checkMetadataAccessTokenExpired(); + expect(result).toBe(false); + }, + ); + }); + + it('should return true when token has zero lifetime (iat === exp)', async () => { + const now = Math.floor(Date.now() / 1000); + // Zero lifetime: iat === exp. Falls back to exact-expiry check; + // exp is in the past so returns true. + const malformedToken = createMockJWTToken({ + iat: now - 10, + exp: now - 10, + }); + + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + metadataAccessToken: malformedToken, + }), + }, + async ({ controller }) => { + jest + .spyOn(controller, 'checkMetadataAccessTokenExpired') + .mockRestore(); + + const result = controller.checkMetadataAccessTokenExpired(); + expect(result).toBe(true); + }, + ); + }); + it('should return true if user is not authenticated', async () => { await withController(async ({ controller }) => { // Restore the original implementation to test the real logic @@ -5773,8 +6191,11 @@ describe('SeedlessOnboardingController', () => { describe('checkAccessTokenExpired', () => { it('should return false if access token is not expired', async () => { - const futureExp = Math.floor(Date.now() / 1000) + 3600; // 1 hour from now - const validToken = createMockJWTToken({ exp: futureExp }); + const now = Math.floor(Date.now() / 1000); + const validToken = createMockJWTToken({ + iat: now, + exp: now + 3600, // 1 hour lifetime, 0% elapsed + }); await withController( { @@ -5794,8 +6215,11 @@ describe('SeedlessOnboardingController', () => { }); it('should return true if access token is expired', async () => { - const pastExp = Math.floor(Date.now() / 1000) - 3600; // 1 hour ago - const expiredToken = createMockJWTToken({ exp: pastExp }); + const now = Math.floor(Date.now() / 1000); + const expiredToken = createMockJWTToken({ + iat: now - 7200, + exp: now - 3600, // issued 2 h ago, expired 1 h ago + }); await withController( { @@ -5814,6 +6238,54 @@ describe('SeedlessOnboardingController', () => { ); }); + it('should return true when 90% of token lifetime has elapsed', async () => { + const now = Math.floor(Date.now() / 1000); + // 10 000 s lifetime; 9 001 s have elapsed → just past the 90 % mark + const tokenAt90Percent = createMockJWTToken({ + iat: now - 9001, + exp: now + 999, + }); + + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + accessToken: tokenAt90Percent, + }), + }, + async ({ controller }) => { + jest.spyOn(controller, 'checkAccessTokenExpired').mockRestore(); + + const result = controller.checkAccessTokenExpired(); + expect(result).toBe(true); + }, + ); + }); + + it('should return false when less than 90% of token lifetime has elapsed', async () => { + const now = Math.floor(Date.now() / 1000); + // 10 000 s lifetime; only 3 000 s have elapsed → well below the 90 % mark + const tokenBelow90Percent = createMockJWTToken({ + iat: now - 3000, + exp: now + 7000, + }); + + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + accessToken: tokenBelow90Percent, + }), + }, + async ({ controller }) => { + jest.spyOn(controller, 'checkAccessTokenExpired').mockRestore(); + + const result = controller.checkAccessTokenExpired(); + expect(result).toBe(false); + }, + ); + }); + it('should return true if user is not authenticated', async () => { await withController(async ({ controller }) => { // Restore the original implementation to test the real logic @@ -6134,11 +6606,11 @@ describe('SeedlessOnboardingController', () => { }); }); - describe('renewRefreshToken', () => { + describe('rotateRefreshToken', () => { const MOCK_PASSWORD = 'mock-password'; const MOCK_REVOKE_TOKEN = 'newRevokeToken'; - it('should successfully renew refresh token and update vault', async () => { + it('should call the renewal service with connection and revokeToken from vault', async () => { const mockToprfEncryptor = createMockToprfEncryptor(); const MOCK_ENCRYPTION_KEY = mockToprfEncryptor.deriveEncKey(MOCK_PASSWORD); @@ -6165,17 +6637,20 @@ describe('SeedlessOnboardingController', () => { }), }, async ({ controller, mockRenewRefreshToken }) => { - await controller.renewRefreshToken(MOCK_PASSWORD); + // Unlock vault so #cachedDecryptedVaultData is populated. + await controller.submitPassword(MOCK_PASSWORD); + + await controller.rotateRefreshToken(); expect(mockRenewRefreshToken).toHaveBeenCalledWith({ connection: controller.state.authConnection, - revokeToken: controller.state.revokeToken, + revokeToken: MOCK_REVOKE_TOKEN, }); }, ); }); - it('should throw error if revoke token is missing', async () => { + it('should update state with new tokens and queue old token for revocation', async () => { const mockToprfEncryptor = createMockToprfEncryptor(); const MOCK_ENCRYPTION_KEY = mockToprfEncryptor.deriveEncKey(MOCK_PASSWORD); @@ -6184,42 +6659,105 @@ describe('SeedlessOnboardingController', () => { const MOCK_AUTH_KEY_PAIR = mockToprfEncryptor.deriveAuthKeyPair(MOCK_PASSWORD); - // Create vault data without revoke token manually - const encryptor = createMockVaultEncryptor(); - const serializedKeyData = JSON.stringify({ - toprfEncryptionKey: bytesToBase64(MOCK_ENCRYPTION_KEY), - toprfPwEncryptionKey: bytesToBase64(MOCK_PASSWORD_ENCRYPTION_KEY), - toprfAuthKeyPair: JSON.stringify({ - sk: `0x${MOCK_AUTH_KEY_PAIR.sk.toString(16)}`, - pk: bytesToBase64(MOCK_AUTH_KEY_PAIR.pk), - }), - // Intentionally omit revokeToken - accessToken, + const mockResult = await createMockVault( + MOCK_ENCRYPTION_KEY, + MOCK_PASSWORD_ENCRYPTION_KEY, + MOCK_AUTH_KEY_PAIR, + MOCK_PASSWORD, + MOCK_REVOKE_TOKEN, + ); + + await withController( + { + state: getMockInitialControllerState({ + withMockAuthenticatedUser: true, + vault: mockResult.encryptedMockVault, + vaultEncryptionKey: mockResult.vaultEncryptionKey, + vaultEncryptionSalt: mockResult.vaultEncryptionSalt, + }), + }, + async ({ controller }) => { + // Unlock vault so #cachedDecryptedVaultData is populated. + await controller.submitPassword(MOCK_PASSWORD); + + const originalRefreshToken = controller.state.refreshToken; + + await controller.rotateRefreshToken(); + + // State should be updated with new tokens + expect(controller.state.refreshToken).toBe('newRefreshToken'); + expect(controller.state.revokeToken).toBe('newRevokeToken'); + + // Old token should be queued for revocation + expect(controller.state.pendingToBeRevokedTokens).toHaveLength(1); + expect(controller.state.pendingToBeRevokedTokens?.[0]).toStrictEqual({ + refreshToken: originalRefreshToken, + revokeToken: MOCK_REVOKE_TOKEN, + }); + }, + ); + }); + + it('should throw when user is not authenticated', async () => { + await withController(async ({ controller }) => { + await expect(controller.rotateRefreshToken()).rejects.toThrow( + SeedlessOnboardingControllerErrorMessage.MissingAuthUserInfo, + ); }); + }); - const { vault: encryptedMockVault, exportedKeyString } = - await encryptor.encryptWithDetail(MOCK_PASSWORD, serializedKeyData); + it('should not update state when renewal returns null tokens', async () => { + const mockToprfEncryptor = createMockToprfEncryptor(); + const MOCK_ENCRYPTION_KEY = + mockToprfEncryptor.deriveEncKey(MOCK_PASSWORD); + const MOCK_PASSWORD_ENCRYPTION_KEY = + mockToprfEncryptor.derivePwEncKey(MOCK_PASSWORD); + const MOCK_AUTH_KEY_PAIR = + mockToprfEncryptor.deriveAuthKeyPair(MOCK_PASSWORD); + + const mockResult = await createMockVault( + MOCK_ENCRYPTION_KEY, + MOCK_PASSWORD_ENCRYPTION_KEY, + MOCK_AUTH_KEY_PAIR, + MOCK_PASSWORD, + MOCK_REVOKE_TOKEN, + ); await withController( { state: getMockInitialControllerState({ withMockAuthenticatedUser: true, - vault: encryptedMockVault, - vaultEncryptionKey: exportedKeyString, - vaultEncryptionSalt: JSON.parse(encryptedMockVault).salt, + vault: mockResult.encryptedMockVault, + vaultEncryptionKey: mockResult.vaultEncryptionKey, + vaultEncryptionSalt: mockResult.vaultEncryptionSalt, }), }, - async ({ controller }) => { - await expect( - controller.renewRefreshToken(MOCK_PASSWORD), - ).rejects.toThrow( - SeedlessOnboardingControllerErrorMessage.InvalidRevokeToken, + async ({ controller, mockRenewRefreshToken }) => { + // Unlock vault so #cachedDecryptedVaultData is populated. + await controller.submitPassword(MOCK_PASSWORD); + + const originalRefreshToken = controller.state.refreshToken; + const originalRevokeToken = controller.state.revokeToken; + + // Mock renewal to return null tokens + mockRenewRefreshToken.mockResolvedValueOnce({ + newRevokeToken: null, + newRefreshToken: null, + }); + + await controller.rotateRefreshToken(); + + // State should remain unchanged + expect(controller.state.refreshToken).toBe(originalRefreshToken); + expect(controller.state.revokeToken).toBe(originalRevokeToken); + expect(controller.state.pendingToBeRevokedTokens ?? []).toHaveLength( + 0, ); }, ); }); - it('should not queue old token for revocation when vault creation fails', async () => { + it('should not queue old token for revocation when vault update fails', async () => { const mockToprfEncryptor = createMockToprfEncryptor(); const MOCK_ENC_KEY = mockToprfEncryptor.deriveEncKey(MOCK_PASSWORD); const MOCK_PW_ENC_KEY = mockToprfEncryptor.derivePwEncKey(MOCK_PASSWORD); @@ -6244,18 +6782,29 @@ describe('SeedlessOnboardingController', () => { }), }, async ({ controller, encryptor }) => { - // Force vault re-encryption to fail so #createNewVaultWithAuthData throws. + // Unlock vault so #cachedDecryptedVaultData is populated. + await controller.submitPassword(MOCK_PASSWORD); + + // Fail the vault update (new revokeToken inside rotateRefreshToken). + const originalEncryptWithKey = + encryptor.encryptWithKey.bind(encryptor); jest .spyOn(encryptor, 'encryptWithKey') - .mockRejectedValueOnce(new Error('Storage full')); + .mockImplementation(async () => { + throw new Error('Storage full'); + }); - await expect( - controller.renewRefreshToken(MOCK_PASSWORD), - ).rejects.toThrow('Storage full'); + // rotateRefreshToken should throw since the vault update failed. + await expect(controller.rotateRefreshToken()).rejects.toThrow( + 'Storage full', + ); + + // Restore encryptor so we can verify state + jest + .spyOn(encryptor, 'encryptWithKey') + .mockImplementation(originalEncryptWithKey); - // The old token must NOT be in pendingToBeRevokedTokens. Queueing it - // before confirming the new vault was persisted would schedule the - // user's only valid refresh token for revocation. + // The old token must NOT be in pendingToBeRevokedTokens. expect(controller.state.pendingToBeRevokedTokens ?? []).toHaveLength( 0, ); diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts index 56df91704de..1424744e831 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts @@ -463,10 +463,11 @@ export class SeedlessOnboardingController< ); } - // Check if token is expired and refresh if needed + // Use the same 90%-lifetime threshold as checkMetadataAccessTokenExpired + // so both code paths agree on when a refresh is needed. const decodedToken = decodeJWTToken(metadataAccessToken); - if (decodedToken.exp < Math.floor(Date.now() / 1000)) { - // Token is expired, refresh it + if (isTokenNearExpiry(decodedToken.exp, decodedToken.iat)) { + // Token is near expiry or already expired, refresh it await this.refreshAuthTokens(); // Get the new token after refresh @@ -503,7 +504,7 @@ export class SeedlessOnboardingController< * @param params.userId - user email or id from Social login * @param params.groupedAuthConnectionId - Optional grouped authConnectionId to be used for the authenticate request. * @param params.socialLoginEmail - The user email from Social login. - * @param params.refreshToken - refresh token for refreshing expired nodeAuthTokens. + * @param params.refreshToken - Refresh token issued during OAuth login. Written to state when provided. * @param params.revokeToken - revoke token for revoking refresh token and get new refresh token and new revoke token. * @param params.accessToken - Access token for pairing with profile sync auth service and to access other services. * @param params.metadataAccessToken - Metadata access token for accessing the metadata service before the vault is created or unlocked. @@ -2118,13 +2119,21 @@ export class SeedlessOnboardingController< * @returns A promise that resolves when the tokens have been refreshed. */ async refreshAuthTokens(): Promise { + // Coalesce concurrent calls to avoid issuing parallel HTTP requests + // with the same refresh token. if (this.#pendingRefreshPromise) { return this.#pendingRefreshPromise; } - this.#pendingRefreshPromise = this.#doRefreshAuthTokens().finally(() => { - this.#pendingRefreshPromise = undefined; + + const promise = this.#doRefreshAuthTokens().finally(() => { + if (this.#pendingRefreshPromise === promise) { + this.#pendingRefreshPromise = undefined; + } }); - return this.#pendingRefreshPromise; + + this.#pendingRefreshPromise = promise; + + return promise; } /** @@ -2161,25 +2170,12 @@ export class SeedlessOnboardingController< try { const { idTokens, accessToken, metadataAccessToken } = res; - // re-authenticate with the new id tokens to set new node auth tokens - // NOTE: here we can't provide the `revokeToken` value to the `authenticate` method because `refreshAuthTokens` method can be called when the wallet (vault) is locked - // NOTE: use this.state.refreshToken (current value) rather than the - // `refreshToken` captured at the start of this method. If renewRefreshToken() - // ran concurrently while our HTTP call was in-flight, it will have already - // updated state.refreshToken to the new token. Using the captured (now-stale) - // value would overwrite that newer token and revert the state, causing the - // old token to be used for subsequent refreshes — and once that old token is - // revoked (15 s after the renewal), every subsequent refresh will return 401. - await this.authenticate({ + // Re-authenticate with the refreshed id tokens to update node auth + // tokens, accessToken, and metadataAccessToken in state. + await this.#reAuthenticate({ idTokens, accessToken, metadataAccessToken, - authConnection: this.state.authConnection, - authConnectionId: this.state.authConnectionId, - groupedAuthConnectionId: this.state.groupedAuthConnectionId, - userId: this.state.userId, - refreshToken: this.state.refreshToken, - skipLock: true, }); // update the vault with new access token if wallet is unlocked @@ -2194,6 +2190,19 @@ export class SeedlessOnboardingController< vaultData: updatedVaultData, pwEncKey, }); + + // Proactively rotate the refresh token now that we have vault access. + await this.rotateRefreshToken().catch((error) => { + // Rotation failure is intentionally non-fatal: the JWT refresh + // itself succeeded and the caller should not be blocked. + // However the user is now operating with a stale refresh token + // that may be revoked server-side, so log prominently. + log( + 'Failed to rotate refresh token after successful JWT refresh. ' + + 'The user may be logged out when the old token is revoked.', + error, + ); + }); } } catch (error) { log('Error refreshing node auth tokens', error); @@ -2207,55 +2216,124 @@ export class SeedlessOnboardingController< } /** - * Renew the refresh token - get new refresh token and new revoke token - * and also updates the vault with the new revoke token. - * This method is to be called after user is authenticated. + * Re-authenticate the user using freshly issued tokens from a JWT refresh. * - * @param password - The password to encrypt the vault. - * @returns A Promise that resolves to void. + * Unlike the public `authenticate` method, this variant is called exclusively + * from token-refresh paths where the user's identity has already been + * established. It only accepts the tokens returned by the JWT-refresh + * service (`idTokens`, `accessToken`, `metadataAccessToken`) and reads all + * other auth-connection details from the existing controller state. + * + * `refreshToken` and `revokeToken` are intentionally absent — token-refresh + * paths must not touch `state.refreshToken` directly; that field is managed + * exclusively by `rotateRefreshToken`. + * + * @param params - Tokens issued by the JWT-refresh service. + * @param params.idTokens - New node id tokens. + * @param params.accessToken - New access token. + * @param params.metadataAccessToken - New metadata access token. + * @returns A promise that resolves to the authentication result. */ - async renewRefreshToken(password: string): Promise { - return await this.#withControllerLock(async () => { - this.#assertIsAuthenticatedUser(this.state); - const { refreshToken, vaultEncryptionKey } = this.state; - const { - toprfEncryptionKey: rawToprfEncryptionKey, - toprfPwEncryptionKey: rawToprfPwEncryptionKey, - toprfAuthKeyPair: rawToprfAuthKeyPair, - revokeToken, - } = await this.#unlockVaultAndGetVaultData({ - password, - encryptionKey: vaultEncryptionKey, + async #reAuthenticate(params: { + idTokens: string[]; + accessToken: string; + metadataAccessToken: string; + }): Promise { + const { idTokens, accessToken, metadataAccessToken } = params; + const { + authConnection, + authConnectionId, + groupedAuthConnectionId, + userId, + socialLoginEmail, + } = this.state; + + try { + const authenticationResult = await this.toprfClient.authenticate({ + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + authConnectionId: authConnectionId!, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + userId: userId!, + idTokens, + groupedAuthConnectionId, }); - const { newRevokeToken, newRefreshToken } = await this.#renewRefreshToken( + this.update((state) => { + state.nodeAuthTokens = authenticationResult.nodeAuthTokens; + state.authConnection = authConnection; + state.authConnectionId = authConnectionId; + state.groupedAuthConnectionId = groupedAuthConnectionId; + state.userId = userId; + state.socialLoginEmail = socialLoginEmail; + state.metadataAccessToken = metadataAccessToken; + state.accessToken = accessToken; + assertIsSeedlessOnboardingUserAuthenticated(state); + state.isSeedlessOnboardingUserAuthenticated = true; + }); + + return authenticationResult; + } catch (error) { + log('Error re-authenticating user', error); + throw new SeedlessOnboardingError( + SeedlessOnboardingControllerErrorMessage.AuthenticationError, { - connection: this.state.authConnection, - revokeToken, + cause: error, }, ); + } + } - if (newRevokeToken && newRefreshToken) { - this.update((state) => { - // set new revoke token in state temporarily for persisting in vault - state.revokeToken = newRevokeToken; - // set new refresh token to persist in state - state.refreshToken = newRefreshToken; - }); + /** + * Rotate the refresh token — fetch a new refresh/revoke token pair from the + * auth service and persist the new revoke token in the vault. + * + * This method should be called after a successful JWT refresh. + * + * @returns A Promise that resolves to void. + */ + async rotateRefreshToken(): Promise { + this.#assertIsAuthenticatedUser(this.state); - await this.#createNewVaultWithAuthData({ - password, - rawToprfEncryptionKey, - rawToprfPwEncryptionKey, - rawToprfAuthKeyPair, - }); - // add the old refresh token to the list to be revoked later when possible - this.#addRefreshTokenToRevokeList({ - refreshToken, - revokeToken, - }); - } + const vaultData = this.#cachedDecryptedVaultData; + // Safety net: the caller (#doRefreshAuthTokens) already guards with + // `this.#isUnlocked && this.#cachedDecryptedVaultData`, so this branch + // is unreachable in normal flow. This check is added to satisfy the linter and + // to make sure any future caller of this method does not bypass the check. + /* istanbul ignore if */ + if (!vaultData) { + throw new SeedlessOnboardingError( + SeedlessOnboardingControllerErrorMessage.VaultLocked, + ); + } + const { refreshToken } = this.state; + const { toprfPwEncryptionKey: pwEncKey, revokeToken } = vaultData; + const { newRevokeToken, newRefreshToken } = await this.#renewRefreshToken({ + connection: this.state.authConnection, + revokeToken, }); + + if (newRevokeToken && newRefreshToken) { + // Persist the new revoke token to the vault first. Only update state after + // the vault write succeeds, so state and vault stay in sync if the write fails. + await this.#updateVault({ + vaultData: { + ...vaultData, + revokeToken: newRevokeToken, + }, + pwEncKey, + }); + + this.update((state) => { + state.revokeToken = newRevokeToken; + state.refreshToken = newRefreshToken; + }); + + // add the old refresh token to the list to be revoked later when possible + this.#addRefreshTokenToRevokeList({ + refreshToken, + revokeToken, + }); + } } /** @@ -2466,14 +2544,16 @@ export class SeedlessOnboardingController< const firstAuthToken = nodeAuthTokens[0]?.authToken; // node auth token is base64 encoded json object const decodedToken = decodeNodeAuthToken(firstAuthToken); - // check if the token is expired - return decodedToken.exp < Date.now() / 1000; + // Node auth tokens do not carry a reliable iat field — use exact expiry. + return isTokenNearExpiry(decodedToken.exp); } /** - * Check if the current metadata access token is expired. + * Check if the current metadata access token should be refreshed. + * Returns true when the token is expired or when less than 10% of its + * lifetime remains (proactive refresh). * - * @returns True if the metadata access token is expired, false otherwise. + * @returns True if the metadata access token should be refreshed, false otherwise. */ public checkMetadataAccessTokenExpired(): boolean { try { @@ -2481,17 +2561,19 @@ export class SeedlessOnboardingController< const { metadataAccessToken } = this.state; // assertIsAuthenticatedUser will throw if metadataAccessToken is missing const decodedToken = decodeJWTToken(metadataAccessToken as string); - return decodedToken.exp < Math.floor(Date.now() / 1000); + return isTokenNearExpiry(decodedToken.exp, decodedToken.iat); } catch { return true; // Consider unauthenticated user as having expired tokens } } /** - * Check if the current access token is expired. + * Check if the current access token should be refreshed. + * Returns true when the token is expired or when less than 10% of its + * lifetime remains (proactive refresh). * When the vault is locked, the access token is not accessible, so we return false. * - * @returns True if the access token is expired, false otherwise. + * @returns True if the access token should be refreshed, false otherwise. */ public checkAccessTokenExpired(): boolean { try { @@ -2501,13 +2583,40 @@ export class SeedlessOnboardingController< return true; // Consider missing token as expired } const decodedToken = decodeJWTToken(accessToken); - return decodedToken.exp < Math.floor(Date.now() / 1000); + return isTokenNearExpiry(decodedToken.exp, decodedToken.iat); } catch { return true; // Consider unauthenticated user as having expired tokens } } } +/** + * Determine whether a token should be proactively refreshed. + * + * When `iat` is provided: returns `true` when less than 10% of the token's + * lifetime remains (i.e. we are in the last 10% before expiry). + * When `iat` is omitted (e.g. node auth tokens): returns `true` when the token + * is already expired. + * + * @param exp - Token expiration time in seconds (Unix epoch). + * @param iat - Optional issued-at time in seconds (Unix epoch). Required for 10% threshold. + * @returns True if the token should be refreshed. + */ +function isTokenNearExpiry(exp: number, iat?: number): boolean { + const now = Date.now() / 1000; + if (iat === undefined) { + return now >= exp; + } + const lifetime = exp - iat; + // Guard against malformed tokens where iat >= exp (zero or negative lifetime). + // Fall back to exact-expiry check so bad tokens are always considered stale. + if (lifetime <= 0) { + return now >= exp; + } + const remaining = exp - now; + return remaining <= 0.1 * lifetime; +} + /** * Lock the given mutex before executing the given function, * and release it after the function is resolved or after an diff --git a/packages/seedless-onboarding-controller/src/constants.ts b/packages/seedless-onboarding-controller/src/constants.ts index 4ceb62c3930..7338ec17bb8 100644 --- a/packages/seedless-onboarding-controller/src/constants.ts +++ b/packages/seedless-onboarding-controller/src/constants.ts @@ -26,6 +26,7 @@ export enum SecretMetadataVersion { export enum SeedlessOnboardingControllerErrorMessage { ControllerLocked = `${controllerName} - The operation cannot be completed while the controller is locked.`, + VaultLocked = `${controllerName} - The operation cannot be completed while the vault is locked.`, AuthenticationError = `${controllerName} - Authentication error`, MissingAuthUserInfo = `${controllerName} - Missing authenticated user information`, FailedToPersistOprfKey = `${controllerName} - Failed to persist OPRF key`,