mirror of
https://github.com/bitwarden/browser
synced 2025-12-11 05:43:41 +00:00
feat(user-decryption-options) [PM-26413]: Remove ActiveUserState from UserDecryptionOptionsService (#16894)
* feat(user-decryption-options) [PM-26413]: Update UserDecryptionOptionsService and tests to use UserId-only APIs. * feat(user-decryption-options) [PM-26413]: Update InternalUserDecryptionOptionsService call sites to use UserId-only API. * feat(user-decryption-options) [PM-26413] Update userDecryptionOptions$ call sites to use the UserId-only API. * feat(user-decryption-options) [PM-26413]: Update additional call sites. * feat(user-decryption-options) [PM-26413]: Update dependencies and an additional call site. * feat(user-verification-service) [PM-26413]: Replace where allowed by unrestricted imports invocation of UserVerificationService.hasMasterPassword (deprecated) with UserDecryptionOptions.hasMasterPasswordById$. Additional work to complete as tech debt tracked in PM-27009. * feat(user-decryption-options) [PM-26413]: Update for non-null strict adherence. * feat(user-decryption-options) [PM-26413]: Update type safety and defensive returns. * chore(user-decryption-options) [PM-26413]: Comment cleanup. * feat(user-decryption-options) [PM-26413]: Update tests. * feat(user-decryption-options) [PM-26413]: Standardize null-checking on active account id for new API consumption. * feat(vault-timeout-settings-service) [PM-26413]: Add test cases to illustrate null active account from AccountService. * fix(fido2-user-verification-service-spec) [PM-26413]: Update test harness to use FakeAccountService. * fix(downstream-components) [PM-26413]: Prefer use of the getUserId operator in all authenticated contexts for user id provided to UserDecryptionOptionsService. --------- Co-authored-by: bnagawiecki <107435978+bnagawiecki@users.noreply.github.com>
This commit is contained in:
@@ -135,7 +135,7 @@ export class LoginDecryptionOptionsComponent implements OnInit {
|
||||
|
||||
try {
|
||||
const userDecryptionOptions = await firstValueFrom(
|
||||
this.userDecryptionOptionsService.userDecryptionOptions$,
|
||||
this.userDecryptionOptionsService.userDecryptionOptionsById$(this.activeAccountId),
|
||||
);
|
||||
|
||||
if (
|
||||
|
||||
@@ -460,7 +460,7 @@ export class SsoComponent implements OnInit {
|
||||
|
||||
// must come after 2fa check since user decryption options aren't available if 2fa is required
|
||||
const userDecryptionOpts = await firstValueFrom(
|
||||
this.userDecryptionOptionsService.userDecryptionOptions$,
|
||||
this.userDecryptionOptionsService.userDecryptionOptionsById$(authResult.userId),
|
||||
);
|
||||
|
||||
const tdeEnabled = userDecryptionOpts.trustedDeviceOption
|
||||
|
||||
@@ -176,7 +176,9 @@ describe("TwoFactorAuthComponent", () => {
|
||||
selectedUserDecryptionOptions = new BehaviorSubject<UserDecryptionOptions>(
|
||||
mockUserDecryptionOpts.withMasterPassword,
|
||||
);
|
||||
mockUserDecryptionOptionsService.userDecryptionOptions$ = selectedUserDecryptionOptions;
|
||||
mockUserDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue(
|
||||
selectedUserDecryptionOptions,
|
||||
);
|
||||
|
||||
TestBed.configureTestingModule({
|
||||
declarations: [TestTwoFactorComponent],
|
||||
|
||||
@@ -473,7 +473,7 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy {
|
||||
}
|
||||
|
||||
const userDecryptionOpts = await firstValueFrom(
|
||||
this.userDecryptionOptionsService.userDecryptionOptions$,
|
||||
this.userDecryptionOptionsService.userDecryptionOptionsById$(authResult.userId),
|
||||
);
|
||||
|
||||
const tdeEnabled = await this.isTrustedDeviceEncEnabled(userDecryptionOpts.trustedDeviceOption);
|
||||
|
||||
@@ -1,34 +1,45 @@
|
||||
import { Observable } from "rxjs";
|
||||
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
|
||||
import { UserDecryptionOptions } from "../models";
|
||||
|
||||
/**
|
||||
* Public service for reading user decryption options.
|
||||
* For use in components and services that need to evaluate user decryption settings.
|
||||
*/
|
||||
export abstract class UserDecryptionOptionsServiceAbstraction {
|
||||
/**
|
||||
* Returns what decryption options are available for the current user.
|
||||
* @remark This is sent from the server on authentication.
|
||||
* Returns the user decryption options for the given user id.
|
||||
* Will only emit when options are set (does not emit null/undefined
|
||||
* for an unpopulated state), and should not be called in an unauthenticated context.
|
||||
* @param userId The user id to check.
|
||||
*/
|
||||
abstract userDecryptionOptions$: Observable<UserDecryptionOptions>;
|
||||
abstract userDecryptionOptionsById$(userId: UserId): Observable<UserDecryptionOptions>;
|
||||
/**
|
||||
* Uses user decryption options to determine if current user has a master password.
|
||||
* @remark This is sent from the server, and does not indicate if the master password
|
||||
* was used to login and/or if a master key is saved locally.
|
||||
*/
|
||||
abstract hasMasterPassword$: Observable<boolean>;
|
||||
|
||||
/**
|
||||
* Returns the user decryption options for the given user id.
|
||||
* @param userId The user id to check.
|
||||
*/
|
||||
abstract userDecryptionOptionsById$(userId: string): Observable<UserDecryptionOptions>;
|
||||
abstract hasMasterPasswordById$(userId: UserId): Observable<boolean>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Internal service for managing user decryption options.
|
||||
* For use only in authentication flows that need to update decryption options
|
||||
* (e.g., login strategies). Extends consumer methods from {@link UserDecryptionOptionsServiceAbstraction}.
|
||||
* @remarks Most consumers should use UserDecryptionOptionsServiceAbstraction instead.
|
||||
*/
|
||||
export abstract class InternalUserDecryptionOptionsServiceAbstraction extends UserDecryptionOptionsServiceAbstraction {
|
||||
/**
|
||||
* Sets the current decryption options for the user, contains the current configuration
|
||||
* Sets the current decryption options for the user. Contains the current configuration
|
||||
* of the users account related to how they can decrypt their vault.
|
||||
* @remark Intended to be used when user decryption options are received from server, does
|
||||
* not update the server. Consider syncing instead of updating locally.
|
||||
* @param userDecryptionOptions Current user decryption options received from server.
|
||||
*/
|
||||
abstract setUserDecryptionOptions(userDecryptionOptions: UserDecryptionOptions): Promise<void>;
|
||||
abstract setUserDecryptionOptionsById(
|
||||
userId: UserId,
|
||||
userDecryptionOptions: UserDecryptionOptions,
|
||||
): Promise<void>;
|
||||
}
|
||||
|
||||
@@ -257,7 +257,8 @@ describe("LoginStrategy", () => {
|
||||
|
||||
expect(environmentService.seedUserEnvironment).toHaveBeenCalled();
|
||||
|
||||
expect(userDecryptionOptionsService.setUserDecryptionOptions).toHaveBeenCalledWith(
|
||||
expect(userDecryptionOptionsService.setUserDecryptionOptionsById).toHaveBeenCalledWith(
|
||||
userId,
|
||||
UserDecryptionOptions.fromResponse(idTokenResponse),
|
||||
);
|
||||
expect(masterPasswordService.mock.setMasterPasswordUnlockData).toHaveBeenCalledWith(
|
||||
|
||||
@@ -195,7 +195,8 @@ export abstract class LoginStrategy {
|
||||
|
||||
// We must set user decryption options before retrieving vault timeout settings
|
||||
// as the user decryption options help determine the available timeout actions.
|
||||
await this.userDecryptionOptionsService.setUserDecryptionOptions(
|
||||
await this.userDecryptionOptionsService.setUserDecryptionOptionsById(
|
||||
userId,
|
||||
UserDecryptionOptions.fromResponse(tokenResponse),
|
||||
);
|
||||
|
||||
|
||||
@@ -134,7 +134,9 @@ describe("SsoLoginStrategy", () => {
|
||||
);
|
||||
|
||||
const userDecryptionOptions = new UserDecryptionOptions();
|
||||
userDecryptionOptionsService.userDecryptionOptions$ = of(userDecryptionOptions);
|
||||
userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue(
|
||||
of(userDecryptionOptions),
|
||||
);
|
||||
|
||||
ssoLoginStrategy = new SsoLoginStrategy(
|
||||
{} as SsoLoginStrategyData,
|
||||
|
||||
@@ -393,7 +393,7 @@ export class SsoLoginStrategy extends LoginStrategy {
|
||||
|
||||
// Check for TDE-related conditions
|
||||
const userDecryptionOptions = await firstValueFrom(
|
||||
this.userDecryptionOptionsService.userDecryptionOptions$,
|
||||
this.userDecryptionOptionsService.userDecryptionOptionsById$(userId),
|
||||
);
|
||||
|
||||
if (!userDecryptionOptions) {
|
||||
|
||||
@@ -1,12 +1,8 @@
|
||||
import { firstValueFrom } from "rxjs";
|
||||
|
||||
import { Utils } from "@bitwarden/common/platform/misc/utils";
|
||||
import {
|
||||
FakeAccountService,
|
||||
FakeStateProvider,
|
||||
mockAccountServiceWith,
|
||||
} from "@bitwarden/common/spec";
|
||||
import { FakeSingleUserStateProvider } from "@bitwarden/common/spec";
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
import { newGuid } from "@bitwarden/guid";
|
||||
|
||||
import { UserDecryptionOptions } from "../../models/domain/user-decryption-options";
|
||||
|
||||
@@ -17,15 +13,10 @@ import {
|
||||
|
||||
describe("UserDecryptionOptionsService", () => {
|
||||
let sut: UserDecryptionOptionsService;
|
||||
|
||||
const fakeUserId = Utils.newGuid() as UserId;
|
||||
let fakeAccountService: FakeAccountService;
|
||||
let fakeStateProvider: FakeStateProvider;
|
||||
let fakeStateProvider: FakeSingleUserStateProvider;
|
||||
|
||||
beforeEach(() => {
|
||||
fakeAccountService = mockAccountServiceWith(fakeUserId);
|
||||
fakeStateProvider = new FakeStateProvider(fakeAccountService);
|
||||
|
||||
fakeStateProvider = new FakeSingleUserStateProvider();
|
||||
sut = new UserDecryptionOptionsService(fakeStateProvider);
|
||||
});
|
||||
|
||||
@@ -42,55 +33,71 @@ describe("UserDecryptionOptionsService", () => {
|
||||
},
|
||||
};
|
||||
|
||||
describe("userDecryptionOptions$", () => {
|
||||
it("should return the active user's decryption options", async () => {
|
||||
await fakeStateProvider.setUserState(USER_DECRYPTION_OPTIONS, userDecryptionOptions);
|
||||
describe("userDecryptionOptionsById$", () => {
|
||||
it("should return user decryption options for a specific user", async () => {
|
||||
const userId = newGuid() as UserId;
|
||||
|
||||
const result = await firstValueFrom(sut.userDecryptionOptions$);
|
||||
fakeStateProvider.getFake(userId, USER_DECRYPTION_OPTIONS).nextState(userDecryptionOptions);
|
||||
|
||||
const result = await firstValueFrom(sut.userDecryptionOptionsById$(userId));
|
||||
|
||||
expect(result).toEqual(userDecryptionOptions);
|
||||
});
|
||||
});
|
||||
|
||||
describe("hasMasterPassword$", () => {
|
||||
it("should return the hasMasterPassword property of the active user's decryption options", async () => {
|
||||
await fakeStateProvider.setUserState(USER_DECRYPTION_OPTIONS, userDecryptionOptions);
|
||||
describe("hasMasterPasswordById$", () => {
|
||||
it("should return true when user has a master password", async () => {
|
||||
const userId = newGuid() as UserId;
|
||||
|
||||
const result = await firstValueFrom(sut.hasMasterPassword$);
|
||||
fakeStateProvider.getFake(userId, USER_DECRYPTION_OPTIONS).nextState(userDecryptionOptions);
|
||||
|
||||
const result = await firstValueFrom(sut.hasMasterPasswordById$(userId));
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("userDecryptionOptionsById$", () => {
|
||||
it("should return the user decryption options for the given user", async () => {
|
||||
const givenUser = Utils.newGuid() as UserId;
|
||||
await fakeAccountService.addAccount(givenUser, {
|
||||
name: "Test User 1",
|
||||
email: "test1@email.com",
|
||||
emailVerified: false,
|
||||
});
|
||||
await fakeStateProvider.setUserState(
|
||||
USER_DECRYPTION_OPTIONS,
|
||||
userDecryptionOptions,
|
||||
givenUser,
|
||||
);
|
||||
it("should return false when user does not have a master password", async () => {
|
||||
const userId = newGuid() as UserId;
|
||||
const optionsWithoutMasterPassword = {
|
||||
...userDecryptionOptions,
|
||||
hasMasterPassword: false,
|
||||
};
|
||||
|
||||
const result = await firstValueFrom(sut.userDecryptionOptionsById$(givenUser));
|
||||
fakeStateProvider
|
||||
.getFake(userId, USER_DECRYPTION_OPTIONS)
|
||||
.nextState(optionsWithoutMasterPassword);
|
||||
|
||||
expect(result).toEqual(userDecryptionOptions);
|
||||
const result = await firstValueFrom(sut.hasMasterPasswordById$(userId));
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("setUserDecryptionOptions", () => {
|
||||
it("should set the active user's decryption options", async () => {
|
||||
await sut.setUserDecryptionOptions(userDecryptionOptions);
|
||||
describe("setUserDecryptionOptionsById", () => {
|
||||
it("should set user decryption options for a specific user", async () => {
|
||||
const userId = newGuid() as UserId;
|
||||
|
||||
const result = await firstValueFrom(
|
||||
fakeStateProvider.getActive(USER_DECRYPTION_OPTIONS).state$,
|
||||
);
|
||||
await sut.setUserDecryptionOptionsById(userId, userDecryptionOptions);
|
||||
|
||||
const fakeState = fakeStateProvider.getFake(userId, USER_DECRYPTION_OPTIONS);
|
||||
const result = await firstValueFrom(fakeState.state$);
|
||||
|
||||
expect(result).toEqual(userDecryptionOptions);
|
||||
});
|
||||
|
||||
it("should overwrite existing user decryption options", async () => {
|
||||
const userId = newGuid() as UserId;
|
||||
const initialOptions = { ...userDecryptionOptions, hasMasterPassword: false };
|
||||
const updatedOptions = { ...userDecryptionOptions, hasMasterPassword: true };
|
||||
|
||||
const fakeState = fakeStateProvider.getFake(userId, USER_DECRYPTION_OPTIONS);
|
||||
fakeState.nextState(initialOptions);
|
||||
|
||||
await sut.setUserDecryptionOptionsById(userId, updatedOptions);
|
||||
|
||||
const result = await firstValueFrom(fakeState.state$);
|
||||
|
||||
expect(result).toEqual(updatedOptions);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,16 +1,11 @@
|
||||
// FIXME: Update this file to be type safe and remove this and next line
|
||||
// @ts-strict-ignore
|
||||
import { Observable, map } from "rxjs";
|
||||
import { Observable, filter, map } from "rxjs";
|
||||
|
||||
import {
|
||||
ActiveUserState,
|
||||
StateProvider,
|
||||
SingleUserStateProvider,
|
||||
USER_DECRYPTION_OPTIONS_DISK,
|
||||
UserKeyDefinition,
|
||||
} from "@bitwarden/common/platform/state";
|
||||
// FIXME: remove `src` and fix import
|
||||
// eslint-disable-next-line no-restricted-imports
|
||||
import { UserId } from "@bitwarden/common/src/types/guid";
|
||||
import { UserId } from "@bitwarden/common/types/guid";
|
||||
|
||||
import { InternalUserDecryptionOptionsServiceAbstraction } from "../../abstractions/user-decryption-options.service.abstraction";
|
||||
import { UserDecryptionOptions } from "../../models";
|
||||
@@ -27,25 +22,26 @@ export const USER_DECRYPTION_OPTIONS = new UserKeyDefinition<UserDecryptionOptio
|
||||
export class UserDecryptionOptionsService
|
||||
implements InternalUserDecryptionOptionsServiceAbstraction
|
||||
{
|
||||
private userDecryptionOptionsState: ActiveUserState<UserDecryptionOptions>;
|
||||
constructor(private singleUserStateProvider: SingleUserStateProvider) {}
|
||||
|
||||
userDecryptionOptions$: Observable<UserDecryptionOptions>;
|
||||
hasMasterPassword$: Observable<boolean>;
|
||||
userDecryptionOptionsById$(userId: UserId): Observable<UserDecryptionOptions> {
|
||||
return this.singleUserStateProvider
|
||||
.get(userId, USER_DECRYPTION_OPTIONS)
|
||||
.state$.pipe(filter((options): options is UserDecryptionOptions => options != null));
|
||||
}
|
||||
|
||||
constructor(private stateProvider: StateProvider) {
|
||||
this.userDecryptionOptionsState = this.stateProvider.getActive(USER_DECRYPTION_OPTIONS);
|
||||
|
||||
this.userDecryptionOptions$ = this.userDecryptionOptionsState.state$;
|
||||
this.hasMasterPassword$ = this.userDecryptionOptions$.pipe(
|
||||
map((options) => options?.hasMasterPassword ?? false),
|
||||
hasMasterPasswordById$(userId: UserId): Observable<boolean> {
|
||||
return this.userDecryptionOptionsById$(userId).pipe(
|
||||
map((options) => options.hasMasterPassword ?? false),
|
||||
);
|
||||
}
|
||||
|
||||
userDecryptionOptionsById$(userId: UserId): Observable<UserDecryptionOptions> {
|
||||
return this.stateProvider.getUser(userId, USER_DECRYPTION_OPTIONS).state$;
|
||||
}
|
||||
|
||||
async setUserDecryptionOptions(userDecryptionOptions: UserDecryptionOptions): Promise<void> {
|
||||
await this.userDecryptionOptionsState.update((_) => userDecryptionOptions);
|
||||
async setUserDecryptionOptionsById(
|
||||
userId: UserId,
|
||||
userDecryptionOptions: UserDecryptionOptions,
|
||||
): Promise<void> {
|
||||
await this.singleUserStateProvider
|
||||
.get(userId, USER_DECRYPTION_OPTIONS)
|
||||
.update((_) => userDecryptionOptions);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user