1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-11-21 11:35:34 +01:00

Revert "Auth/PM-6689 - Migrate Security Stamp to Token Service and State Prov…" (#8860)

This reverts commit 91f1d9fb86.
This commit is contained in:
Jared Snider 2024-04-22 12:06:43 -04:00 committed by GitHub
parent 300b17aaeb
commit 100b43dd8f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 63 additions and 126 deletions

View File

@ -813,7 +813,6 @@ export default class MainBackground {
this.avatarService, this.avatarService,
logoutCallback, logoutCallback,
this.billingAccountProfileStateService, this.billingAccountProfileStateService,
this.tokenService,
); );
this.eventUploadService = new EventUploadService( this.eventUploadService = new EventUploadService(
this.apiService, this.apiService,

View File

@ -631,7 +631,6 @@ export class Main {
this.avatarService, this.avatarService,
async (expired: boolean) => await this.logout(), async (expired: boolean) => await this.logout(),
this.billingAccountProfileStateService, this.billingAccountProfileStateService,
this.tokenService,
); );
this.totpService = new TotpService(this.cryptoFunctionService, this.logService); this.totpService = new TotpService(this.cryptoFunctionService, this.logService);

View File

@ -628,7 +628,6 @@ const safeProviders: SafeProvider[] = [
AvatarServiceAbstraction, AvatarServiceAbstraction,
LOGOUT_CALLBACK, LOGOUT_CALLBACK,
BillingAccountProfileStateService, BillingAccountProfileStateService,
TokenServiceAbstraction,
], ],
}), }),
safeProvider({ safeProvider({

View File

@ -27,6 +27,7 @@ import { Utils } from "@bitwarden/common/platform/misc/utils";
import { import {
Account, Account,
AccountProfile, AccountProfile,
AccountTokens,
AccountKeys, AccountKeys,
} from "@bitwarden/common/platform/models/domain/account"; } from "@bitwarden/common/platform/models/domain/account";
import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
@ -212,6 +213,9 @@ describe("LoginStrategy", () => {
kdfType: kdf, kdfType: kdf,
}, },
}, },
tokens: {
...new AccountTokens(),
},
keys: new AccountKeys(), keys: new AccountKeys(),
}), }),
); );

View File

@ -27,7 +27,11 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service"
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
import { Account, AccountProfile } from "@bitwarden/common/platform/models/domain/account"; import {
Account,
AccountProfile,
AccountTokens,
} from "@bitwarden/common/platform/models/domain/account";
import { UserId } from "@bitwarden/common/types/guid"; import { UserId } from "@bitwarden/common/types/guid";
import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../abstractions/user-decryption-options.service.abstraction";
@ -188,6 +192,9 @@ export abstract class LoginStrategy {
kdfType: tokenResponse.kdf, kdfType: tokenResponse.kdf,
}, },
}, },
tokens: {
...new AccountTokens(),
},
}), }),
); );

View File

@ -213,10 +213,4 @@ export abstract class TokenService {
* @returns A promise that resolves with a boolean representing the user's external authN status. * @returns A promise that resolves with a boolean representing the user's external authN status.
*/ */
getIsExternal: () => Promise<boolean>; getIsExternal: () => Promise<boolean>;
/** Gets the active or passed in user's security stamp */
getSecurityStamp: (userId?: UserId) => Promise<string | null>;
/** Sets the security stamp for the active or passed in user */
setSecurityStamp: (securityStamp: string, userId?: UserId) => Promise<void>;
} }

View File

@ -23,7 +23,6 @@ import {
EMAIL_TWO_FACTOR_TOKEN_RECORD_DISK_LOCAL, EMAIL_TWO_FACTOR_TOKEN_RECORD_DISK_LOCAL,
REFRESH_TOKEN_DISK, REFRESH_TOKEN_DISK,
REFRESH_TOKEN_MEMORY, REFRESH_TOKEN_MEMORY,
SECURITY_STAMP_MEMORY,
} from "./token.state"; } from "./token.state";
describe("TokenService", () => { describe("TokenService", () => {
@ -2192,84 +2191,6 @@ describe("TokenService", () => {
}); });
}); });
describe("Security Stamp methods", () => {
const mockSecurityStamp = "securityStamp";
describe("setSecurityStamp", () => {
it("should throw an error if no user id is provided and there is no active user in global state", async () => {
// Act
// note: don't await here because we want to test the error
const result = tokenService.setSecurityStamp(mockSecurityStamp);
// Assert
await expect(result).rejects.toThrow("User id not found. Cannot set security stamp.");
});
it("should set the security stamp in memory when there is an active user in global state", async () => {
// Arrange
globalStateProvider
.getFake(ACCOUNT_ACTIVE_ACCOUNT_ID)
.stateSubject.next(userIdFromAccessToken);
// Act
await tokenService.setSecurityStamp(mockSecurityStamp);
// Assert
expect(
singleUserStateProvider.getFake(userIdFromAccessToken, SECURITY_STAMP_MEMORY).nextMock,
).toHaveBeenCalledWith(mockSecurityStamp);
});
it("should set the security stamp in memory for the specified user id", async () => {
// Act
await tokenService.setSecurityStamp(mockSecurityStamp, userIdFromAccessToken);
// Assert
expect(
singleUserStateProvider.getFake(userIdFromAccessToken, SECURITY_STAMP_MEMORY).nextMock,
).toHaveBeenCalledWith(mockSecurityStamp);
});
});
describe("getSecurityStamp", () => {
it("should throw an error if no user id is provided and there is no active user in global state", async () => {
// Act
// note: don't await here because we want to test the error
const result = tokenService.getSecurityStamp();
// Assert
await expect(result).rejects.toThrow("User id not found. Cannot get security stamp.");
});
it("should return the security stamp from memory with no user id specified (uses global active user)", async () => {
// Arrange
globalStateProvider
.getFake(ACCOUNT_ACTIVE_ACCOUNT_ID)
.stateSubject.next(userIdFromAccessToken);
singleUserStateProvider
.getFake(userIdFromAccessToken, SECURITY_STAMP_MEMORY)
.stateSubject.next([userIdFromAccessToken, mockSecurityStamp]);
// Act
const result = await tokenService.getSecurityStamp();
// Assert
expect(result).toEqual(mockSecurityStamp);
});
it("should return the security stamp from memory for the specified user id", async () => {
// Arrange
singleUserStateProvider
.getFake(userIdFromAccessToken, SECURITY_STAMP_MEMORY)
.stateSubject.next([userIdFromAccessToken, mockSecurityStamp]);
// Act
const result = await tokenService.getSecurityStamp(userIdFromAccessToken);
// Assert
expect(result).toEqual(mockSecurityStamp);
});
});
});
// Helpers // Helpers
function createTokenService(supportsSecureStorage: boolean) { function createTokenService(supportsSecureStorage: boolean) {
return new TokenService( return new TokenService(

View File

@ -32,7 +32,6 @@ import {
EMAIL_TWO_FACTOR_TOKEN_RECORD_DISK_LOCAL, EMAIL_TWO_FACTOR_TOKEN_RECORD_DISK_LOCAL,
REFRESH_TOKEN_DISK, REFRESH_TOKEN_DISK,
REFRESH_TOKEN_MEMORY, REFRESH_TOKEN_MEMORY,
SECURITY_STAMP_MEMORY,
} from "./token.state"; } from "./token.state";
export enum TokenStorageLocation { export enum TokenStorageLocation {
@ -851,30 +850,6 @@ export class TokenService implements TokenServiceAbstraction {
return Array.isArray(decoded.amr) && decoded.amr.includes("external"); return Array.isArray(decoded.amr) && decoded.amr.includes("external");
} }
async getSecurityStamp(userId?: UserId): Promise<string | null> {
userId ??= await firstValueFrom(this.activeUserIdGlobalState.state$);
if (!userId) {
throw new Error("User id not found. Cannot get security stamp.");
}
const securityStamp = await this.getStateValueByUserIdAndKeyDef(userId, SECURITY_STAMP_MEMORY);
return securityStamp;
}
async setSecurityStamp(securityStamp: string, userId?: UserId): Promise<void> {
userId ??= await firstValueFrom(this.activeUserIdGlobalState.state$);
if (!userId) {
throw new Error("User id not found. Cannot set security stamp.");
}
await this.singleUserStateProvider
.get(userId, SECURITY_STAMP_MEMORY)
.update((_) => securityStamp);
}
private async getStateValueByUserIdAndKeyDef( private async getStateValueByUserIdAndKeyDef(
userId: UserId, userId: UserId,
storageLocation: UserKeyDefinition<string>, storageLocation: UserKeyDefinition<string>,

View File

@ -10,7 +10,6 @@ import {
EMAIL_TWO_FACTOR_TOKEN_RECORD_DISK_LOCAL, EMAIL_TWO_FACTOR_TOKEN_RECORD_DISK_LOCAL,
REFRESH_TOKEN_DISK, REFRESH_TOKEN_DISK,
REFRESH_TOKEN_MEMORY, REFRESH_TOKEN_MEMORY,
SECURITY_STAMP_MEMORY,
} from "./token.state"; } from "./token.state";
describe.each([ describe.each([
@ -23,7 +22,6 @@ describe.each([
[API_KEY_CLIENT_ID_MEMORY, "apiKeyClientIdMemory"], [API_KEY_CLIENT_ID_MEMORY, "apiKeyClientIdMemory"],
[API_KEY_CLIENT_SECRET_DISK, "apiKeyClientSecretDisk"], [API_KEY_CLIENT_SECRET_DISK, "apiKeyClientSecretDisk"],
[API_KEY_CLIENT_SECRET_MEMORY, "apiKeyClientSecretMemory"], [API_KEY_CLIENT_SECRET_MEMORY, "apiKeyClientSecretMemory"],
[SECURITY_STAMP_MEMORY, "securityStamp"],
])( ])(
"deserializes state key definitions", "deserializes state key definitions",
( (

View File

@ -69,8 +69,3 @@ export const API_KEY_CLIENT_SECRET_MEMORY = new UserKeyDefinition<string>(
clearOn: [], // Manually handled clearOn: [], // Manually handled
}, },
); );
export const SECURITY_STAMP_MEMORY = new UserKeyDefinition<string>(TOKEN_MEMORY, "securityStamp", {
deserializer: (securityStamp) => securityStamp,
clearOn: ["logout"],
});

View File

@ -181,6 +181,8 @@ export abstract class StateService<T extends Account = Account> {
* Sets the user's Pin, encrypted by the user key * Sets the user's Pin, encrypted by the user key
*/ */
setProtectedPin: (value: string, options?: StorageOptions) => Promise<void>; setProtectedPin: (value: string, options?: StorageOptions) => Promise<void>;
getSecurityStamp: (options?: StorageOptions) => Promise<string>;
setSecurityStamp: (value: string, options?: StorageOptions) => Promise<void>;
getUserId: (options?: StorageOptions) => Promise<string>; getUserId: (options?: StorageOptions) => Promise<string>;
getVaultTimeout: (options?: StorageOptions) => Promise<number>; getVaultTimeout: (options?: StorageOptions) => Promise<number>;
setVaultTimeout: (value: number, options?: StorageOptions) => Promise<void>; setVaultTimeout: (value: number, options?: StorageOptions) => Promise<void>;

View File

@ -0,0 +1,9 @@
import { AccountTokens } from "./account";
describe("AccountTokens", () => {
describe("fromJSON", () => {
it("should deserialize to an instance of itself", () => {
expect(AccountTokens.fromJSON({})).toBeInstanceOf(AccountTokens);
});
});
});

View File

@ -1,4 +1,4 @@
import { Account, AccountKeys, AccountProfile, AccountSettings } from "./account"; import { Account, AccountKeys, AccountProfile, AccountSettings, AccountTokens } from "./account";
describe("Account", () => { describe("Account", () => {
describe("fromJSON", () => { describe("fromJSON", () => {
@ -10,12 +10,14 @@ describe("Account", () => {
const keysSpy = jest.spyOn(AccountKeys, "fromJSON"); const keysSpy = jest.spyOn(AccountKeys, "fromJSON");
const profileSpy = jest.spyOn(AccountProfile, "fromJSON"); const profileSpy = jest.spyOn(AccountProfile, "fromJSON");
const settingsSpy = jest.spyOn(AccountSettings, "fromJSON"); const settingsSpy = jest.spyOn(AccountSettings, "fromJSON");
const tokensSpy = jest.spyOn(AccountTokens, "fromJSON");
Account.fromJSON({}); Account.fromJSON({});
expect(keysSpy).toHaveBeenCalled(); expect(keysSpy).toHaveBeenCalled();
expect(profileSpy).toHaveBeenCalled(); expect(profileSpy).toHaveBeenCalled();
expect(settingsSpy).toHaveBeenCalled(); expect(settingsSpy).toHaveBeenCalled();
expect(tokensSpy).toHaveBeenCalled();
}); });
}); });
}); });

View File

@ -171,11 +171,24 @@ export class AccountSettings {
} }
} }
export class AccountTokens {
securityStamp?: string;
static fromJSON(obj: Jsonify<AccountTokens>): AccountTokens {
if (obj == null) {
return null;
}
return Object.assign(new AccountTokens(), obj);
}
}
export class Account { export class Account {
data?: AccountData = new AccountData(); data?: AccountData = new AccountData();
keys?: AccountKeys = new AccountKeys(); keys?: AccountKeys = new AccountKeys();
profile?: AccountProfile = new AccountProfile(); profile?: AccountProfile = new AccountProfile();
settings?: AccountSettings = new AccountSettings(); settings?: AccountSettings = new AccountSettings();
tokens?: AccountTokens = new AccountTokens();
constructor(init: Partial<Account>) { constructor(init: Partial<Account>) {
Object.assign(this, { Object.assign(this, {
@ -195,6 +208,10 @@ export class Account {
...new AccountSettings(), ...new AccountSettings(),
...init?.settings, ...init?.settings,
}, },
tokens: {
...new AccountTokens(),
...init?.tokens,
},
}); });
} }
@ -208,6 +225,7 @@ export class Account {
data: AccountData.fromJSON(json?.data), data: AccountData.fromJSON(json?.data),
profile: AccountProfile.fromJSON(json?.profile), profile: AccountProfile.fromJSON(json?.profile),
settings: AccountSettings.fromJSON(json?.settings), settings: AccountSettings.fromJSON(json?.settings),
tokens: AccountTokens.fromJSON(json?.tokens),
}); });
} }
} }

View File

@ -839,6 +839,23 @@ export class StateService<
); );
} }
async getSecurityStamp(options?: StorageOptions): Promise<string> {
return (
await this.getAccount(this.reconcileOptions(options, await this.defaultInMemoryOptions()))
)?.tokens?.securityStamp;
}
async setSecurityStamp(value: string, options?: StorageOptions): Promise<void> {
const account = await this.getAccount(
this.reconcileOptions(options, await this.defaultInMemoryOptions()),
);
account.tokens.securityStamp = value;
await this.saveAccount(
account,
this.reconcileOptions(options, await this.defaultInMemoryOptions()),
);
}
async getUserId(options?: StorageOptions): Promise<string> { async getUserId(options?: StorageOptions): Promise<string> {
return ( return (
await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions())) await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))

View File

@ -15,7 +15,6 @@ import { AccountService } from "../../../auth/abstractions/account.service";
import { AvatarService } from "../../../auth/abstractions/avatar.service"; import { AvatarService } from "../../../auth/abstractions/avatar.service";
import { KeyConnectorService } from "../../../auth/abstractions/key-connector.service"; import { KeyConnectorService } from "../../../auth/abstractions/key-connector.service";
import { InternalMasterPasswordServiceAbstraction } from "../../../auth/abstractions/master-password.service.abstraction"; import { InternalMasterPasswordServiceAbstraction } from "../../../auth/abstractions/master-password.service.abstraction";
import { TokenService } from "../../../auth/abstractions/token.service";
import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason"; import { ForceSetPasswordReason } from "../../../auth/models/domain/force-set-password-reason";
import { DomainSettingsService } from "../../../autofill/services/domain-settings.service"; import { DomainSettingsService } from "../../../autofill/services/domain-settings.service";
import { BillingAccountProfileStateService } from "../../../billing/abstractions/account/billing-account-profile-state.service"; import { BillingAccountProfileStateService } from "../../../billing/abstractions/account/billing-account-profile-state.service";
@ -74,7 +73,6 @@ export class SyncService implements SyncServiceAbstraction {
private avatarService: AvatarService, private avatarService: AvatarService,
private logoutCallback: (expired: boolean) => Promise<void>, private logoutCallback: (expired: boolean) => Promise<void>,
private billingAccountProfileStateService: BillingAccountProfileStateService, private billingAccountProfileStateService: BillingAccountProfileStateService,
private tokenService: TokenService,
) {} ) {}
async getLastSync(): Promise<Date> { async getLastSync(): Promise<Date> {
@ -311,7 +309,7 @@ export class SyncService implements SyncServiceAbstraction {
} }
private async syncProfile(response: ProfileResponse) { private async syncProfile(response: ProfileResponse) {
const stamp = await this.tokenService.getSecurityStamp(response.id as UserId); const stamp = await this.stateService.getSecurityStamp();
if (stamp != null && stamp !== response.securityStamp) { if (stamp != null && stamp !== response.securityStamp) {
if (this.logoutCallback != null) { if (this.logoutCallback != null) {
await this.logoutCallback(true); await this.logoutCallback(true);
@ -325,7 +323,7 @@ export class SyncService implements SyncServiceAbstraction {
await this.cryptoService.setProviderKeys(response.providers); await this.cryptoService.setProviderKeys(response.providers);
await this.cryptoService.setOrgKeys(response.organizations, response.providerOrganizations); await this.cryptoService.setOrgKeys(response.organizations, response.providerOrganizations);
await this.avatarService.setSyncAvatarColor(response.id as UserId, response.avatarColor); await this.avatarService.setSyncAvatarColor(response.id as UserId, response.avatarColor);
await this.tokenService.setSecurityStamp(response.securityStamp, response.id as UserId); await this.stateService.setSecurityStamp(response.securityStamp);
await this.stateService.setEmailVerified(response.emailVerified); await this.stateService.setEmailVerified(response.emailVerified);
await this.billingAccountProfileStateService.setHasPremium( await this.billingAccountProfileStateService.setHasPremium(