1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-16 00:03:56 +00:00

[PM-15605] Add new device protection opt out (#12880)

* feat(newdeviceVerificaiton) : adding component and request model

* feat(newDeviceverification) : adding state structure to track verify devices for active user; added API call to server.

* feat(newDeviceVerification) : added visual elements for opting out of new device verification.

* Fixing tests for account service.
fixed DI for account service

* Fixing strict lint issues

* debt(deauthorizeSessionsModal) : changed modal to dialog. fixed strict typing for the new dialog for deviceVerification.

* fixing tests

* fixing desktop build DI

* changed dialog to standalone fixed names and comments.

* Adding tests for AccountService

* fix linting

* PM-15605 - AccountComp - fix ngOnDestroy erroring as it was incorrectly decorated with removed property.

* PM-15605 - SetAccountVerifyDevicesDialogComponent - only show warning about turning off new device verification if user doensn't have 2FA configured per task description

---------

Co-authored-by: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com>
Co-authored-by: Jared Snider <jsnider@bitwarden.com>
This commit is contained in:
Ike
2025-01-29 07:49:56 -07:00
committed by GitHub
parent 81943cd4f6
commit 60e569ed9d
22 changed files with 447 additions and 95 deletions

View File

@@ -1,6 +1,7 @@
import { RegisterFinishRequest } from "../models/request/registration/register-finish.request";
import { RegisterSendVerificationEmailRequest } from "../models/request/registration/register-send-verification-email.request";
import { RegisterVerificationEmailClickedRequest } from "../models/request/registration/register-verification-email-clicked.request";
import { SetVerifyDevicesRequest } from "../models/request/set-verify-devices.request";
import { Verification } from "../types/verification";
export abstract class AccountApiService {
@@ -18,7 +19,7 @@ export abstract class AccountApiService {
*
* @param request - The request object containing
* information needed to send the verification email, such as the user's email address.
* @returns A promise that resolves to a string tokencontaining the user's encrypted
* @returns A promise that resolves to a string token containing the user's encrypted
* information which must be submitted to complete registration or `null` if
* email verification is enabled (users must get the token by clicking a
* link in the email that will be sent to them).
@@ -33,7 +34,7 @@ export abstract class AccountApiService {
*
* @param request - The request object containing the email verification token and the
* user's email address (which is required to validate the token)
* @returns A promise that resolves when the event is logged on the server succcessfully or a bad
* @returns A promise that resolves when the event is logged on the server successfully or a bad
* request if the token is invalid for any reason.
*/
abstract registerVerificationEmailClicked(
@@ -50,4 +51,15 @@ export abstract class AccountApiService {
* registration process is successfully completed.
*/
abstract registerFinish(request: RegisterFinishRequest): Promise<string>;
/**
* Sets the [dbo].[User].[VerifyDevices] flag to true or false.
*
* @param request - The request object is a SecretVerificationRequest extension
* that also contains the boolean value that the VerifyDevices property is being
* set to.
* @returns A promise that resolves when the process is successfully completed or
* a bad request if secret verification fails.
*/
abstract setVerifyDevices(request: SetVerifyDevicesRequest): Promise<string>;
}

View File

@@ -43,6 +43,8 @@ export abstract class AccountService {
* Observable of the last activity time for each account.
*/
accountActivity$: Observable<Record<UserId, Date>>;
/** Observable of the new device login verification property for the account. */
accountVerifyNewDeviceLogin$: Observable<boolean>;
/** Account list in order of descending recency */
sortedUserIds$: Observable<UserId[]>;
/** Next account that is not the current active account */
@@ -73,6 +75,15 @@ export abstract class AccountService {
* @param emailVerified
*/
abstract setAccountEmailVerified(userId: UserId, emailVerified: boolean): Promise<void>;
/**
* updates the `accounts$` observable with the new VerifyNewDeviceLogin property for the account.
* @param userId
* @param VerifyNewDeviceLogin
*/
abstract setAccountVerifyNewDeviceLogin(
userId: UserId,
verifyNewDeviceLogin: boolean,
): Promise<void>;
/**
* Updates the `activeAccount$` observable with the new active account.
* @param userId

View File

@@ -0,0 +1,8 @@
import { SecretVerificationRequest } from "./secret-verification.request";
export class SetVerifyDevicesRequest extends SecretVerificationRequest {
/**
* This is the input for a user update that controls [dbo].[Users].[VerifyDevices]
*/
verifyDevices!: boolean;
}

View File

@@ -10,6 +10,7 @@ import { UserVerificationService } from "../abstractions/user-verification/user-
import { RegisterFinishRequest } from "../models/request/registration/register-finish.request";
import { RegisterSendVerificationEmailRequest } from "../models/request/registration/register-send-verification-email.request";
import { RegisterVerificationEmailClickedRequest } from "../models/request/registration/register-verification-email-clicked.request";
import { SetVerifyDevicesRequest } from "../models/request/set-verify-devices.request";
import { Verification } from "../types/verification";
export class AccountApiServiceImplementation implements AccountApiService {
@@ -102,4 +103,21 @@ export class AccountApiServiceImplementation implements AccountApiService {
throw e;
}
}
async setVerifyDevices(request: SetVerifyDevicesRequest): Promise<string> {
try {
const response = await this.apiService.send(
"POST",
"/accounts/verify-devices",
request,
true,
true,
);
return response;
} catch (e: unknown) {
this.logService.error(e);
throw e;
}
}
}

View File

@@ -7,7 +7,10 @@ import { MockProxy, mock } from "jest-mock-extended";
import { firstValueFrom } from "rxjs";
import { FakeGlobalState } from "../../../spec/fake-state";
import { FakeGlobalStateProvider } from "../../../spec/fake-state-provider";
import {
FakeGlobalStateProvider,
FakeSingleUserStateProvider,
} from "../../../spec/fake-state-provider";
import { trackEmissions } from "../../../spec/utils";
import { LogService } from "../../platform/abstractions/log.service";
import { MessagingService } from "../../platform/abstractions/messaging.service";
@@ -19,6 +22,7 @@ import {
ACCOUNT_ACCOUNTS,
ACCOUNT_ACTIVE_ACCOUNT_ID,
ACCOUNT_ACTIVITY,
ACCOUNT_VERIFY_NEW_DEVICE_LOGIN,
AccountServiceImplementation,
} from "./account.service";
@@ -66,6 +70,7 @@ describe("accountService", () => {
let messagingService: MockProxy<MessagingService>;
let logService: MockProxy<LogService>;
let globalStateProvider: FakeGlobalStateProvider;
let singleUserStateProvider: FakeSingleUserStateProvider;
let sut: AccountServiceImplementation;
let accountsState: FakeGlobalState<Record<UserId, AccountInfo>>;
let activeAccountIdState: FakeGlobalState<UserId>;
@@ -77,8 +82,14 @@ describe("accountService", () => {
messagingService = mock();
logService = mock();
globalStateProvider = new FakeGlobalStateProvider();
singleUserStateProvider = new FakeSingleUserStateProvider();
sut = new AccountServiceImplementation(messagingService, logService, globalStateProvider);
sut = new AccountServiceImplementation(
messagingService,
logService,
globalStateProvider,
singleUserStateProvider,
);
accountsState = globalStateProvider.getFake(ACCOUNT_ACCOUNTS);
activeAccountIdState = globalStateProvider.getFake(ACCOUNT_ACTIVE_ACCOUNT_ID);
@@ -128,6 +139,22 @@ describe("accountService", () => {
});
});
describe("accountsVerifyNewDeviceLogin$", () => {
it("returns expected value", async () => {
// Arrange
const expected = true;
// we need to set this state since it is how we initialize the VerifyNewDeviceLogin$
activeAccountIdState.stateSubject.next(userId);
singleUserStateProvider.getFake(userId, ACCOUNT_VERIFY_NEW_DEVICE_LOGIN).nextState(expected);
// Act
const result = await firstValueFrom(sut.accountVerifyNewDeviceLogin$);
// Assert
expect(result).toEqual(expected);
});
});
describe("addAccount", () => {
it("should emit the new account", async () => {
await sut.addAccount(userId, userInfo);
@@ -226,6 +253,33 @@ describe("accountService", () => {
});
});
describe("setAccountVerifyNewDeviceLogin", () => {
const initialState = true;
beforeEach(() => {
activeAccountIdState.stateSubject.next(userId);
singleUserStateProvider
.getFake(userId, ACCOUNT_VERIFY_NEW_DEVICE_LOGIN)
.nextState(initialState);
});
it("should update the VerifyNewDeviceLogin", async () => {
const expected = false;
expect(await firstValueFrom(sut.accountVerifyNewDeviceLogin$)).toEqual(initialState);
await sut.setAccountVerifyNewDeviceLogin(userId, expected);
const currentState = await firstValueFrom(sut.accountVerifyNewDeviceLogin$);
expect(currentState).toEqual(expected);
});
it("should NOT update VerifyNewDeviceLogin when userId is null", async () => {
await sut.setAccountVerifyNewDeviceLogin(null, false);
const currentState = await firstValueFrom(sut.accountVerifyNewDeviceLogin$);
expect(currentState).toEqual(initialState);
});
});
describe("clean", () => {
beforeEach(() => {
accountsState.stateSubject.next({ [userId]: userInfo });

View File

@@ -7,6 +7,7 @@ import {
shareReplay,
combineLatest,
Observable,
switchMap,
filter,
timeout,
of,
@@ -26,6 +27,8 @@ import {
GlobalState,
GlobalStateProvider,
KeyDefinition,
SingleUserStateProvider,
UserKeyDefinition,
} from "../../platform/state";
import { UserId } from "../../types/guid";
@@ -45,6 +48,15 @@ export const ACCOUNT_ACTIVITY = KeyDefinition.record<Date, UserId>(ACCOUNT_DISK,
deserializer: (activity) => new Date(activity),
});
export const ACCOUNT_VERIFY_NEW_DEVICE_LOGIN = new UserKeyDefinition<boolean>(
ACCOUNT_DISK,
"verifyNewDeviceLogin",
{
deserializer: (verifyDevices) => verifyDevices,
clearOn: ["logout"],
},
);
const LOGGED_OUT_INFO: AccountInfo = {
email: "",
emailVerified: false,
@@ -76,6 +88,7 @@ export class AccountServiceImplementation implements InternalAccountService {
accounts$: Observable<Record<UserId, AccountInfo>>;
activeAccount$: Observable<Account | null>;
accountActivity$: Observable<Record<UserId, Date>>;
accountVerifyNewDeviceLogin$: Observable<boolean>;
sortedUserIds$: Observable<UserId[]>;
nextUpAccount$: Observable<Account>;
@@ -83,6 +96,7 @@ export class AccountServiceImplementation implements InternalAccountService {
private messagingService: MessagingService,
private logService: LogService,
private globalStateProvider: GlobalStateProvider,
private singleUserStateProvider: SingleUserStateProvider,
) {
this.accountsState = this.globalStateProvider.get(ACCOUNT_ACCOUNTS);
this.activeAccountIdState = this.globalStateProvider.get(ACCOUNT_ACTIVE_ACCOUNT_ID);
@@ -117,6 +131,12 @@ export class AccountServiceImplementation implements InternalAccountService {
return nextId ? { id: nextId, ...accounts[nextId] } : null;
}),
);
this.accountVerifyNewDeviceLogin$ = this.activeAccountIdState.state$.pipe(
switchMap(
(userId) =>
this.singleUserStateProvider.get(userId, ACCOUNT_VERIFY_NEW_DEVICE_LOGIN).state$,
),
);
}
async addAccount(userId: UserId, accountData: AccountInfo): Promise<void> {
@@ -203,6 +223,20 @@ export class AccountServiceImplementation implements InternalAccountService {
);
}
async setAccountVerifyNewDeviceLogin(
userId: UserId,
setVerifyNewDeviceLogin: boolean,
): Promise<void> {
if (!Utils.isGuid(userId)) {
// only store for valid userIds
return;
}
await this.singleUserStateProvider.get(userId, ACCOUNT_VERIFY_NEW_DEVICE_LOGIN).update(() => {
return setVerifyNewDeviceLogin;
});
}
async removeAccountActivity(userId: UserId): Promise<void> {
await this.globalStateProvider.get(ACCOUNT_ACTIVITY).update(
(activity) => {