From ed61bfd359bdd653caab11bdaedf7e7fac9a616e Mon Sep 17 00:00:00 2001 From: Ike Kottlowski <ikottlowski@bitwarden.com> Date: Tue, 14 Jan 2025 22:58:38 -0800 Subject: [PATCH] feat(newDeviceVerification) : added visual elements for opting out of new device verification. --- .../settings/account/account.component.html | 18 ++++++ .../settings/account/account.component.ts | 33 +++++++++-- .../account/danger-zone.component.html | 8 --- .../delete-account-dialog.component.ts | 2 - ...count-verify-devices-dialog.component.html | 28 ++++++++-- ...account-verify-devices-dialog.component.ts | 55 +++++++++++++------ .../src/app/shared/loose-components.module.ts | 2 + apps/web/src/locales/en/messages.json | 23 +++++++- .../src/services/jslib-services.module.ts | 2 +- .../src/auth/services/account.service.ts | 27 ++++----- 10 files changed, 144 insertions(+), 54 deletions(-) diff --git a/apps/web/src/app/auth/settings/account/account.component.html b/apps/web/src/app/auth/settings/account/account.component.html index 4055f14219c..a754148be6e 100644 --- a/apps/web/src/app/auth/settings/account/account.component.html +++ b/apps/web/src/app/auth/settings/account/account.component.html @@ -9,6 +9,24 @@ <h1 bitTypography="h1">{{ "changeEmail" | i18n }}</h1> </div> <app-danger-zone> + <button + *ngIf="verifyNewDeviceLogin && showSetNewDeviceLoginProtection$ | async" + type="button" + bitButton + buttonType="danger" + [bitAction]="setNewDeviceLoginProtection" + > + {{ "turnOffNewDeviceLoginProtection" | i18n }} + </button> + <button + *ngIf="!verifyNewDeviceLogin && showSetNewDeviceLoginProtection$ | async" + type="button" + bitButton + buttonType="secondary" + [bitAction]="setNewDeviceLoginProtection" + > + {{ "turnOnNewDeviceLoginProtection" | i18n }} + </button> <button type="button" bitButton buttonType="danger" (click)="deauthorizeSessions()"> {{ "deauthorizeSessions" | i18n }} </button> diff --git a/apps/web/src/app/auth/settings/account/account.component.ts b/apps/web/src/app/auth/settings/account/account.component.ts index 7e1be937a22..7b90d03d231 100644 --- a/apps/web/src/app/auth/settings/account/account.component.ts +++ b/apps/web/src/app/auth/settings/account/account.component.ts @@ -1,10 +1,9 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore -import { Component, OnInit, ViewChild, ViewContainerRef } from "@angular/core"; -import { combineLatest, from, lastValueFrom, map, Observable } from "rxjs"; +import { Component, OnInit, ViewChild, ViewContainerRef, OnDestroy } from "@angular/core"; +import { combineLatest, from, lastValueFrom, map, Observable, Subject, takeUntil } from "rxjs"; import { ModalService } from "@bitwarden/angular/services/modal.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; @@ -14,21 +13,26 @@ import { PurgeVaultComponent } from "../../../vault/settings/purge-vault.compone import { DeauthorizeSessionsComponent } from "./deauthorize-sessions.component"; import { DeleteAccountDialogComponent } from "./delete-account-dialog.component"; +import { SetAccountVerifyDevicesDialogComponent } from "./set-account-verify-devices-dialog.component"; @Component({ selector: "app-account", templateUrl: "account.component.html", }) -export class AccountComponent implements OnInit { +export class AccountComponent implements OnInit, OnDestroy { @ViewChild("deauthorizeSessionsTemplate", { read: ViewContainerRef, static: true }) deauthModalRef: ViewContainerRef; + private destroy$ = new Subject<void>(); showChangeEmail$: Observable<boolean>; showPurgeVault$: Observable<boolean>; showDeleteAccount$: Observable<boolean>; + showSetNewDeviceLoginProtection$: Observable<boolean>; + verifyNewDeviceLogin: boolean; constructor( private modalService: ModalService, + private accountService: AccountService, private dialogService: DialogService, private userVerificationService: UserVerificationService, private configService: ConfigService, @@ -36,6 +40,10 @@ export class AccountComponent implements OnInit { ) {} async ngOnInit() { + this.showSetNewDeviceLoginProtection$ = this.configService.getFeatureFlag$( + FeatureFlag.NewDeviceVerification, + ); + const isAccountDeprovisioningEnabled$ = this.configService.getFeatureFlag$( FeatureFlag.AccountDeprovisioning, ); @@ -76,6 +84,11 @@ export class AccountComponent implements OnInit { !isAccountDeprovisioningEnabled || !userIsManagedByOrganization, ), ); + this.accountService.accountVerifyDevices$ + .pipe(takeUntil(this.destroy$)) + .subscribe((verifyDevices) => { + this.verifyNewDeviceLogin = verifyDevices; + }); } async deauthorizeSessions() { @@ -91,4 +104,14 @@ export class AccountComponent implements OnInit { const dialogRef = DeleteAccountDialogComponent.open(this.dialogService); await lastValueFrom(dialogRef.closed); }; + + setNewDeviceLoginProtection = async () => { + const dialogRef = SetAccountVerifyDevicesDialogComponent.open(this.dialogService); + await lastValueFrom(dialogRef.closed); + }; + + ngOnDestroy() { + this.destroy$.next(); + this.destroy$.complete(); + } } diff --git a/apps/web/src/app/auth/settings/account/danger-zone.component.html b/apps/web/src/app/auth/settings/account/danger-zone.component.html index 1e7c73a3cc6..68173e12fd9 100644 --- a/apps/web/src/app/auth/settings/account/danger-zone.component.html +++ b/apps/web/src/app/auth/settings/account/danger-zone.component.html @@ -1,14 +1,6 @@ <h1 bitTypography="h1" class="tw-mt-16 tw-pb-2.5 !tw-text-danger">{{ "dangerZone" | i18n }}</h1> <div class="tw-rounded tw-border tw-border-solid tw-border-danger-600 tw-p-5"> - <p> - {{ - (accountDeprovisioningEnabled$ | async) && content.children.length === 1 - ? ("dangerZoneDescSingular" | i18n) - : ("dangerZoneDesc" | i18n) - }} - </p> - <div #content class="tw-flex tw-flex-row tw-gap-2"> <ng-content></ng-content> </div> diff --git a/apps/web/src/app/auth/settings/account/delete-account-dialog.component.ts b/apps/web/src/app/auth/settings/account/delete-account-dialog.component.ts index aa5cfa3c1dc..64d7dc1b0da 100644 --- a/apps/web/src/app/auth/settings/account/delete-account-dialog.component.ts +++ b/apps/web/src/app/auth/settings/account/delete-account-dialog.component.ts @@ -8,7 +8,6 @@ import { AccountApiService } from "@bitwarden/common/auth/abstractions/account-a import { Verification } from "@bitwarden/common/auth/types/verification"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { DialogService, ToastService } from "@bitwarden/components"; @Component({ @@ -22,7 +21,6 @@ export class DeleteAccountDialogComponent { constructor( private i18nService: I18nService, - private platformUtilsService: PlatformUtilsService, private formBuilder: FormBuilder, private accountApiService: AccountApiService, private dialogRef: DialogRef, diff --git a/apps/web/src/app/auth/settings/account/set-account-verify-devices-dialog.component.html b/apps/web/src/app/auth/settings/account/set-account-verify-devices-dialog.component.html index 9999b027261..86a7ea5b2cb 100644 --- a/apps/web/src/app/auth/settings/account/set-account-verify-devices-dialog.component.html +++ b/apps/web/src/app/auth/settings/account/set-account-verify-devices-dialog.component.html @@ -1,8 +1,13 @@ <form [formGroup]="setVerifyDevicesForm" [bitSubmit]="submit"> <bit-dialog dialogSize="default" [title]="'newDeviceLoginProtection' | i18n"> <ng-container bitDialogContent> - <p bitTypography="body1">{{ dialogDesc }}</p> - <bit-callout type="warning">{{ + <p *ngIf="verifyNewDeviceLogin" bitTypography="body1"> + {{ "turnOffNewDeviceLoginProtectionModalDesc" | i18n }} + </p> + <p *ngIf="!verifyNewDeviceLogin" bitTypography="body1"> + {{ "turnOnNewDeviceLoginProtectionModalDesc" | i18n }} + </p> + <bit-callout *ngIf="verifyNewDeviceLogin" type="warning">{{ "turnOffNewDeviceLoginProtectionWarning" | i18n }}</bit-callout> <app-user-verification-form-input @@ -12,8 +17,23 @@ ></app-user-verification-form-input> </ng-container> <ng-container bitDialogFooter> - <button bitButton bitFormButton type="submit" buttonType="danger"> - {{ dialogSubmitButtonDesc }} + <button + bitButton + *ngIf="verifyNewDeviceLogin" + bitFormButton + type="submit" + buttonType="danger" + > + {{ "disable" | i18n }} + </button> + <button + bitButton + *ngIf="!verifyNewDeviceLogin" + bitFormButton + type="submit" + buttonType="primary" + > + {{ "enable" | i18n }} </button> <button bitButton bitFormButton type="button" buttonType="secondary" bitDialogClose> {{ "close" | i18n }} diff --git a/apps/web/src/app/auth/settings/account/set-account-verify-devices-dialog.component.ts b/apps/web/src/app/auth/settings/account/set-account-verify-devices-dialog.component.ts index 02b2e2efc60..36cc5b1a32b 100644 --- a/apps/web/src/app/auth/settings/account/set-account-verify-devices-dialog.component.ts +++ b/apps/web/src/app/auth/settings/account/set-account-verify-devices-dialog.component.ts @@ -1,57 +1,70 @@ import { DialogRef } from "@angular/cdk/dialog"; -import { Component } from "@angular/core"; +import { Component, OnDestroy } from "@angular/core"; import { FormBuilder } from "@angular/forms"; +import { Subject, takeUntil } from "rxjs"; import { AccountApiService } from "@bitwarden/common/auth/abstractions/account-api.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { SetVerifyDevicesRequest } from "@bitwarden/common/auth/models/request/set-verify-devices.request"; import { Verification } from "@bitwarden/common/auth/types/verification"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { UserId } from "@bitwarden/common/types/guid"; import { DialogService, ToastService } from "@bitwarden/components"; @Component({ templateUrl: "./set-account-verify-devices-dialog.component.html", }) -export class SetAccountVerifyDevicesDialogComponent { +export class SetAccountVerifyDevicesDialogComponent implements OnDestroy { + // use this subject for all subscriptions to ensure all subscripts are completed + private destroy$ = new Subject<void>(); + // the default for new device verification is true + verifyNewDeviceLogin: boolean = true; + activeUserId: UserId; + setVerifyDevicesForm = this.formBuilder.group({ verification: undefined as Verification | undefined, }); invalidSecret: boolean = false; - verifyNewDeviceLogin: boolean = true; - dialogDesc: string = ""; // on or off - dialogSubmitButtonDesc: string = ""; // on or off - dialogBodyDesc: string = ""; // turn on or off constructor( private i18nService: I18nService, private formBuilder: FormBuilder, private accountApiService: AccountApiService, private accountService: AccountService, + private userVerificationService: UserVerificationService, private dialogRef: DialogRef, private toastService: ToastService, ) { - //todo set dialog text based on account information - this.verifyNewDeviceLogin = getVerifyDevices; - this.accountService.activeAccount$.pipe( - (a) => (this.verifyNewDeviceLogin = a?.verifyDevices ?? true), - ); - this.dialogDesc = this.i18nService.t("accountNewDeviceLoginProtection"); - this.dialogSubmitButtonDesc = this.i18nService.t("accountNewDeviceLoginProtectionSave"); - this.dialogBodyDesc = this.i18nService.t("accountNewDeviceLoginProtectionDesc"); + this.accountService.accountVerifyDevices$ + .pipe(takeUntil(this.destroy$)) + .subscribe((verifyDevices) => { + this.verifyNewDeviceLogin = verifyDevices; + }); + this.accountService.activeAccount$ + .pipe(takeUntil(this.destroy$)) + .subscribe((account) => (this.activeUserId = account.id)); } submit = async () => { try { - // const verification = this.setVerifyDevicesForm.get("verification").value; //todo create request object - const request: SetVerifyDevicesRequest = null; + const verification = this.setVerifyDevicesForm.get("verification").value; + const request: SetVerifyDevicesRequest = await this.userVerificationService.buildRequest( + verification, + SetVerifyDevicesRequest, + ); + // set verify device opposite what is currently is. + request.verifyDevices = !this.verifyNewDeviceLogin; + await this.accountApiService.setVerifyDevices(request); + await this.accountService.setAccountVerifyDevices(this.activeUserId, request.verifyDevices); this.dialogRef.close(); this.toastService.showToast({ variant: "success", - title: this.i18nService.t("accountNewDeviceLoginProtectionSaved"), - message: this.i18nService.t("accountNewDeviceLoginProtectionSavedDesc"), + title: null, + message: this.i18nService.t("accountNewDeviceLoginProtectionSaved"), }); } catch (e) { if (e instanceof ErrorResponse && e.statusCode === 400) { @@ -64,4 +77,10 @@ export class SetAccountVerifyDevicesDialogComponent { static open(dialogService: DialogService) { return dialogService.open(SetAccountVerifyDevicesDialogComponent); } + + // closes subscription leaks + ngOnDestroy() { + this.destroy$.next(); + this.destroy$.complete(); + } } diff --git a/apps/web/src/app/shared/loose-components.module.ts b/apps/web/src/app/shared/loose-components.module.ts index 3176ac81c1a..af3b2ae1ce7 100644 --- a/apps/web/src/app/shared/loose-components.module.ts +++ b/apps/web/src/app/shared/loose-components.module.ts @@ -30,6 +30,7 @@ import { DangerZoneComponent } from "../auth/settings/account/danger-zone.compon import { DeauthorizeSessionsComponent } from "../auth/settings/account/deauthorize-sessions.component"; import { DeleteAccountDialogComponent } from "../auth/settings/account/delete-account-dialog.component"; import { ProfileComponent } from "../auth/settings/account/profile.component"; +import { SetAccountVerifyDevicesDialogComponent } from "../auth/settings/account/set-account-verify-devices-dialog.component"; import { EmergencyAccessAttachmentsComponent } from "../auth/settings/emergency-access/attachments/emergency-access-attachments.component"; import { EmergencyAccessConfirmComponent } from "../auth/settings/emergency-access/confirm/emergency-access-confirm.component"; import { EmergencyAccessAddEditComponent } from "../auth/settings/emergency-access/emergency-access-add-edit.component"; @@ -155,6 +156,7 @@ import { SharedModule } from "./shared.module"; SecurityKeysComponent, SelectableAvatarComponent, SendAddEditComponent, + SetAccountVerifyDevicesDialogComponent, SetPasswordComponent, SponsoredFamiliesComponent, SponsoringOrgRowComponent, diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 1c7baa31756..2d41f41e3a5 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -1726,7 +1726,7 @@ }, "requestPending": { "message": "Request pending" - }, + }, "logBackInOthersToo": { "message": "Please log back in. If you are using other Bitwarden applications log out and back in to those as well." }, @@ -1809,6 +1809,27 @@ "deauthorizeSessionsWarning": { "message": "Proceeding will also log you out of your current session, requiring you to log back in. You will also be prompted for two-step login again, if set up. Active sessions on other devices may continue to remain active for up to one hour." }, + "newDeviceLoginProtection": { + "message":"New device login" + }, + "turnOffNewDeviceLoginProtection": { + "message":"Turn off new device login protection" + }, + "turnOnNewDeviceLoginProtection": { + "message":"Turn on new device login protection" + }, + "turnOffNewDeviceLoginProtectionModalDesc": { + "message":"Proceed below to turn off the verification emails bitwarden sends when you login from a new device." + }, + "turnOnNewDeviceLoginProtectionModalDesc": { + "message":"Proceed below to have bitwarden send you verification emails when you login from a new device." + }, + "turnOffNewDeviceLoginProtectionWarning": { + "message":"With new device login protection turned off, anyone with your master password can access your account from any device. To protect your account without verification emails, set up two-step login." + }, + "accountNewDeviceLoginProtectionSaved": { + "message": "New device login protection changes saved" + }, "sessionsDeauthorized": { "message": "All sessions deauthorized" }, diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index f5940b8e144..1d5df7f689f 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -551,7 +551,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: InternalAccountService, useClass: AccountServiceImplementation, - deps: [MessagingServiceAbstraction, LogService, GlobalStateProvider], + deps: [MessagingServiceAbstraction, LogService, GlobalStateProvider, SingleUserStateProvider], }), safeProvider({ provide: AccountServiceAbstraction, diff --git a/libs/common/src/auth/services/account.service.ts b/libs/common/src/auth/services/account.service.ts index f9efd095247..6e26837cb3d 100644 --- a/libs/common/src/auth/services/account.service.ts +++ b/libs/common/src/auth/services/account.service.ts @@ -7,6 +7,7 @@ import { shareReplay, combineLatest, Observable, + switchMap, } from "rxjs"; import { @@ -20,10 +21,10 @@ import { MessagingService } from "../../platform/abstractions/messaging.service" import { Utils } from "../../platform/misc/utils"; import { ACCOUNT_DISK, - ActiveUserStateProvider, GlobalState, GlobalStateProvider, KeyDefinition, + SingleUserStateProvider, UserKeyDefinition, } from "../../platform/state"; import { UserId } from "../../types/guid"; @@ -92,7 +93,7 @@ export class AccountServiceImplementation implements InternalAccountService { private messagingService: MessagingService, private logService: LogService, private globalStateProvider: GlobalStateProvider, - private activeUserStateProvider: ActiveUserStateProvider, + private singleUserStateProvider: SingleUserStateProvider, ) { this.accountsState = this.globalStateProvider.get(ACCOUNT_ACCOUNTS); this.activeAccountIdState = this.globalStateProvider.get(ACCOUNT_ACTIVE_ACCOUNT_ID); @@ -127,10 +128,12 @@ export class AccountServiceImplementation implements InternalAccountService { return nextId ? { id: nextId, ...accounts[nextId] } : null; }), ); - - this.accountVerifyDevices$ = this.activeUserStateProvider - .get(ACCOUNT_VERIFY_NEW_DEVICE_LOGIN) - .state$.pipe(map((verifyDevices) => verifyDevices ?? true)); + this.accountVerifyDevices$ = this.activeAccountIdState.state$.pipe( + switchMap( + (userId) => + this.singleUserStateProvider.get(userId, ACCOUNT_VERIFY_NEW_DEVICE_LOGIN).state$, + ), + ); } async addAccount(userId: UserId, accountData: AccountInfo): Promise<void> { @@ -216,15 +219,9 @@ export class AccountServiceImplementation implements InternalAccountService { return; } - await this.activeUserStateProvider.get(ACCOUNT_VERIFY_NEW_DEVICE_LOGIN).update( - () => { - return setVerifyNewDeviceLogin; - }, - { - shouldUpdate: (oldNewDeviceVerification) => - oldNewDeviceVerification !== setVerifyNewDeviceLogin, - }, - ); + await this.singleUserStateProvider.get(userId, ACCOUNT_VERIFY_NEW_DEVICE_LOGIN).update(() => { + return setVerifyNewDeviceLogin; + }); } async removeAccountActivity(userId: UserId): Promise<void> {