Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2284,6 +2284,55 @@ describe('SeedlessOnboardingController', () => {
},
);
});

it('should handle error in revoke refresh token background operation', async () => {
// Setup mock vault data with revoke token
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', // revokeToken
);

await withController(
{
state: getMockInitialControllerState({
withMockAuthenticatedUser: true,
vault: mockResult.encryptedMockVault,
vaultEncryptionKey: mockResult.vaultEncryptionKey,
vaultEncryptionSalt: mockResult.vaultEncryptionSalt,
}),
},
async ({ controller, mockRevokeRefreshToken }) => {
// Mock the revoke refresh token to throw an error
mockRevokeRefreshToken.mockRejectedValueOnce(
new Error('Revoke refresh token failed'),
);

expect(
await controller.submitPassword(MOCK_PASSWORD),
).toBeUndefined();

// Verify the controller is unlocked by calling a method that requires unlocked state
expect(() => {
controller.updateBackupMetadataState({
keyringId: 'test-keyring-id',
data: stringToBytes('test-data'),
type: SecretType.Mnemonic,
});
}).not.toThrow();
},
);
});
});

describe('verifyPassword', () => {
Expand Down Expand Up @@ -3714,6 +3763,87 @@ describe('SeedlessOnboardingController', () => {
},
);
});

it('should handle error in revoke refresh token background operation', async () => {
// Setup mock vault data with revoke token - use RECOVERED_PASSWORD for initial vault creation
const mockToprfEncryptor = createMockToprfEncryptor();
const MOCK_ENCRYPTION_KEY =
mockToprfEncryptor.deriveEncKey(RECOVERED_PASSWORD);
const MOCK_PASSWORD_ENCRYPTION_KEY =
mockToprfEncryptor.derivePwEncKey(RECOVERED_PASSWORD);
const MOCK_AUTH_KEY_PAIR =
mockToprfEncryptor.deriveAuthKeyPair(RECOVERED_PASSWORD);

const mockResult = await createMockVault(
MOCK_ENCRYPTION_KEY,
MOCK_PASSWORD_ENCRYPTION_KEY,
MOCK_AUTH_KEY_PAIR,
RECOVERED_PASSWORD,
'mock-revoke-token', // revokeToken
);

// Create encryptedSeedlessEncryptionKey manually using the recovered password encryption key
const aes = managedNonce(gcm)(MOCK_PASSWORD_ENCRYPTION_KEY);
const encryptedSeedlessEncryptionKey = aes.encrypt(
utf8ToBytes(mockResult.vaultEncryptionKey),
);

await withController(
{
state: {
...getMockInitialControllerState({
withMockAuthenticatedUser: true,
withMockAuthPubKey: true,
vault: mockResult.encryptedMockVault,
vaultEncryptionKey: mockResult.vaultEncryptionKey,
vaultEncryptionSalt: mockResult.vaultEncryptionSalt,
encryptedKeyringEncryptionKey: bytesToBase64(
mockResult.encryptedKeyringEncryptionKey,
),
}),
encryptedSeedlessEncryptionKey: bytesToBase64(
encryptedSeedlessEncryptionKey,
),
},
},
async ({ controller, toprfClient, mockRevokeRefreshToken }) => {
// Mock the revoke refresh token to throw an error
mockRevokeRefreshToken.mockRejectedValueOnce(
new Error('Revoke refresh token failed'),
);

// Mock recoverEncKey for the global password
const latestEncKey = mockToprfEncryptor.deriveEncKey(GLOBAL_PASSWORD);
const latestPwEncKey =
mockToprfEncryptor.derivePwEncKey(GLOBAL_PASSWORD);
const latestAuthKeyPair =
mockToprfEncryptor.deriveAuthKeyPair(GLOBAL_PASSWORD);
jest.spyOn(toprfClient, 'recoverEncKey').mockResolvedValueOnce({
encKey: latestEncKey,
authKeyPair: latestAuthKeyPair,
pwEncKey: latestPwEncKey,
rateLimitResetResult: Promise.resolve(),
keyShareIndex: 1,
});

// Mock toprfClient.recoverPwEncKey - return the device-specific password encryption key
const currentDevicePwEncKey =
mockToprfEncryptor.derivePwEncKey(RECOVERED_PASSWORD);
jest.spyOn(toprfClient, 'recoverPwEncKey').mockResolvedValueOnce({
pwEncKey: currentDevicePwEncKey,
});

controller.setLocked();

// submitGlobalPasswordAndSync should complete successfully despite the error
expect(
await controller.submitGlobalPasswordAndSync({
globalPassword: GLOBAL_PASSWORD,
}),
).toBeUndefined();
},
);
});
});

describe('token refresh functionality', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -654,16 +654,27 @@ export class SeedlessOnboardingController<EncryptionKey> extends BaseController<
revokeToken,
} = await this.#unlockVaultAndGetVaultData(password);
this.#setUnlocked();

await this.#createNewVaultWithAuthData({
password,
rawToprfEncryptionKey: toprfEncryptionKey,
rawToprfPwEncryptionKey: toprfPwEncryptionKey,
rawToprfAuthKeyPair: toprfAuthKeyPair,
});
Comment on lines +657 to +662
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that #createNewVaultWithAuthData is potentially an expensive operation.
it runs encryptWithDetail(pw, vaultData), which involves the intentionally slow pbkdf2 key derivation step.

as you are increasing the usage of vault encryption, you are driving up cpu usage.

if (revokeToken) {
await this.#revokeRefreshTokenAndUpdateState(revokeToken);
// re-creating vault to persist the new revoke token
await this.#createNewVaultWithAuthData({
password,
rawToprfEncryptionKey: toprfEncryptionKey,
rawToprfPwEncryptionKey: toprfPwEncryptionKey,
rawToprfAuthKeyPair: toprfAuthKeyPair,
});
// this call is not critical for unlocking, so we can do it in the background without await.
this.#revokeRefreshTokenAndUpdateState(revokeToken)
.then(() => {
// re-creating vault to persist the new revoke token
return this.#createNewVaultWithAuthData({
password,
rawToprfEncryptionKey: toprfEncryptionKey,
rawToprfPwEncryptionKey: toprfPwEncryptionKey,
rawToprfAuthKeyPair: toprfAuthKeyPair,
});
})
.catch((error) => {
log('Error revoking refresh token', error);
});
Comment on lines +665 to +677
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

writing the vault asynchronously like this is dangerous as it can lead to vault corruption.
imagine another piece of code updates the vault in the meantime. when this code here is executed after that, the vault will be overwritten with some old data.

}
});
}
Expand Down Expand Up @@ -759,18 +770,30 @@ export class SeedlessOnboardingController<EncryptionKey> extends BaseController<
vaultKey,
);
this.#setUnlocked();

if (revokeToken) {
// revoke and recyle refresh token after unlock to keep refresh token fresh, avoid malicious use of leaked refresh token
await this.#revokeRefreshTokenAndUpdateState(revokeToken);
}
// re-creating vault to persist the new revoke token
await this.#createNewVaultWithAuthData({
password: globalPassword,
rawToprfEncryptionKey: latestEncKey,
rawToprfPwEncryptionKey: latestPwEncKey,
rawToprfAuthKeyPair: latestAuthKeyPair,
});
if (revokeToken) {
// // revoke and recyle refresh token after unlock to keep refresh token fresh, avoid malicious use of leaked refresh token
// // this call is not critical for unlocking, so we can do it in the background without await.
this.#revokeRefreshTokenAndUpdateState(revokeToken)
.then(() => {
// re-creating vault to persist the new revoke token.
// TODO: Optimize this function such that updates to vault wont require re-creating the vault.
return this.#createNewVaultWithAuthData({
password: globalPassword,
rawToprfEncryptionKey: latestEncKey,
rawToprfPwEncryptionKey: latestPwEncKey,
rawToprfAuthKeyPair: latestAuthKeyPair,
});
})
.catch((error) => {
log('Error revoking refresh token', error);
});
Comment on lines +782 to +795
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment above, we can't do this

}

// restore the current keyring encryption key with the new global password
await this.storeKeyringEncryptionKey(keyringEncryptionKey);
Expand Down
Loading