From 985942ac058de35d2bf5e939cc80a203177c1f0f Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Mon, 10 Mar 2025 11:07:04 -0700 Subject: [PATCH 01/23] collapse collections initially (#13646) --- .../components/collection-filter.component.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/libs/angular/src/vault/vault-filter/components/collection-filter.component.ts b/libs/angular/src/vault/vault-filter/components/collection-filter.component.ts index d104026f2f6..168afbdd72a 100644 --- a/libs/angular/src/vault/vault-filter/components/collection-filter.component.ts +++ b/libs/angular/src/vault/vault-filter/components/collection-filter.component.ts @@ -1,6 +1,6 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { Directive, EventEmitter, Input, Output } from "@angular/core"; +import { Directive, EventEmitter, Input, OnInit, Output } from "@angular/core"; import { CollectionView } from "@bitwarden/admin-console/common"; import { ITreeNodeObject } from "@bitwarden/common/vault/models/domain/tree-node"; @@ -10,7 +10,7 @@ import { TopLevelTreeNode } from "../models/top-level-tree-node.model"; import { VaultFilter } from "../models/vault-filter.model"; @Directive() -export class CollectionFilterComponent { +export class CollectionFilterComponent implements OnInit { @Input() hide = false; @Input() collapsedFilterNodes: Set; @Input() collectionNodes: DynamicTreeNode; @@ -51,4 +51,13 @@ export class CollectionFilterComponent { async toggleCollapse(node: ITreeNodeObject) { this.onNodeCollapseStateChange.emit(node); } + + ngOnInit() { + // Populate the set with all node IDs so all nodes are collapsed initially. + if (this.collectionNodes?.fullList) { + this.collectionNodes.fullList.forEach((node) => { + this.collapsedFilterNodes.add(node.id); + }); + } + } } From a19bf1687e9c57375bc60e40c6b04b4ea495e4a8 Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Mon, 10 Mar 2025 11:07:22 -0700 Subject: [PATCH 02/23] [PM-12557] - center align custom field buttons (#13670) * center align custom field buttons * add margin --- .../components/custom-fields/custom-fields.component.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.html b/libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.html index fab3c8f1ab1..c7c5f4a930e 100644 --- a/libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.html +++ b/libs/vault/src/cipher-form/components/custom-fields/custom-fields.component.html @@ -87,7 +87,7 @@ (click)="openAddEditCustomFieldDialog({ index: i, label: field.value.name })" [appA11yTitle]="'editFieldLabel' | i18n: field.value.name" bitIconButton="bwi-pencil-square" - class="tw-self-end" + class="tw-self-center tw-mt-2" data-testid="edit-custom-field-button" *ngIf="!isPartialEdit" > @@ -95,7 +95,7 @@ diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/vault-filter-section.type.ts b/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/vault-filter-section.type.ts index f89a72b5d2b..0f949e17146 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/vault-filter-section.type.ts +++ b/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/vault-filter-section.type.ts @@ -31,7 +31,7 @@ export type VaultFilterSection = { }; action: (filterNode: TreeNode) => Promise; edit?: { - text: string; + filterName: string; action: (filter: VaultFilterType) => void; }; add?: { diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 3888b42fe76..1948f589661 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -491,6 +491,19 @@ "editFolder": { "message": "Edit folder" }, + "editWithName": { + "message": "Edit $ITEM$: $NAME$", + "placeholders": { + "item": { + "content": "$1", + "example": "login" + }, + "name": { + "content": "$2", + "example": "Social" + } + } + }, "newFolder": { "message": "New folder" }, From d943f53477613dc4572df981e23b22a99dd38266 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Mon, 10 Mar 2025 11:12:02 -0700 Subject: [PATCH 04/23] refactor(routing): [Auth/PM-18783] Remove Unauth UI route swapping for all components except 2FA (#13645) Removes `unauthUiRefreshSwap()` from all routing modules for all refreshed components except for 2FA. This does not remove the legacy components themselves, just the routing to them. --------- Co-authored-by: Todd Martin --- .../src/auth/popup/environment.component.html | 2 +- .../auth/popup/set-password.component.html | 2 +- .../popup/utils/auth-popout-window.spec.ts | 2 +- .../auth/popup/utils/auth-popout-window.ts | 2 +- apps/browser/src/popup/app-routing.module.ts | 330 +++++++---------- apps/browser/src/popup/app.component.ts | 4 +- apps/desktop/src/app/app-routing.module.ts | 269 ++++++-------- apps/web/src/app/oss-routing.module.ts | 333 ++++++------------ .../auth/src/angular/login/login.component.ts | 32 +- 9 files changed, 343 insertions(+), 633 deletions(-) diff --git a/apps/browser/src/auth/popup/environment.component.html b/apps/browser/src/auth/popup/environment.component.html index ff19739548a..21e69fbbc39 100644 --- a/apps/browser/src/auth/popup/environment.component.html +++ b/apps/browser/src/auth/popup/environment.component.html @@ -1,7 +1,7 @@
- +

{{ "appName" | i18n }} diff --git a/apps/browser/src/auth/popup/set-password.component.html b/apps/browser/src/auth/popup/set-password.component.html index 6261608c345..71a2e3ac588 100644 --- a/apps/browser/src/auth/popup/set-password.component.html +++ b/apps/browser/src/auth/popup/set-password.component.html @@ -1,7 +1,7 @@
- +

{{ "setMasterPassword" | i18n }} diff --git a/apps/browser/src/auth/popup/utils/auth-popout-window.spec.ts b/apps/browser/src/auth/popup/utils/auth-popout-window.spec.ts index 0b47fa4287e..b2c20ba2849 100644 --- a/apps/browser/src/auth/popup/utils/auth-popout-window.spec.ts +++ b/apps/browser/src/auth/popup/utils/auth-popout-window.spec.ts @@ -72,7 +72,7 @@ describe("AuthPopoutWindow", () => { it("closes any existing popup window types that are open to the login extension route", async () => { const loginTab = createChromeTabMock({ - url: chrome.runtime.getURL("popup/index.html#/home"), + url: chrome.runtime.getURL("popup/index.html#/login"), }); jest.spyOn(BrowserApi, "tabsQuery").mockResolvedValue([loginTab]); jest.spyOn(BrowserApi, "removeWindow"); diff --git a/apps/browser/src/auth/popup/utils/auth-popout-window.ts b/apps/browser/src/auth/popup/utils/auth-popout-window.ts index 2f135038315..0646b684b22 100644 --- a/apps/browser/src/auth/popup/utils/auth-popout-window.ts +++ b/apps/browser/src/auth/popup/utils/auth-popout-window.ts @@ -13,7 +13,7 @@ const AuthPopoutType = { const extensionUnlockUrls = new Set([ chrome.runtime.getURL("popup/index.html#/lock"), - chrome.runtime.getURL("popup/index.html#/home"), + chrome.runtime.getURL("popup/index.html#/login"), ]); /** diff --git a/apps/browser/src/popup/app-routing.module.ts b/apps/browser/src/popup/app-routing.module.ts index 8f5d754b554..b33940a68d2 100644 --- a/apps/browser/src/popup/app-routing.module.ts +++ b/apps/browser/src/popup/app-routing.module.ts @@ -7,7 +7,6 @@ import { EnvironmentSelectorRouteData, ExtensionDefaultOverlayPosition, } from "@bitwarden/angular/auth/components/environment-selector.component"; -import { unauthUiRefreshRedirect } from "@bitwarden/angular/auth/functions/unauth-ui-refresh-redirect"; import { unauthUiRefreshSwap } from "@bitwarden/angular/auth/functions/unauth-ui-refresh-route-swap"; import { activeAuthGuard, @@ -58,15 +57,9 @@ import { ExtensionAnonLayoutWrapperComponent, ExtensionAnonLayoutWrapperData, } from "../auth/popup/extension-anon-layout-wrapper/extension-anon-layout-wrapper.component"; -import { HintComponent } from "../auth/popup/hint.component"; -import { HomeComponent } from "../auth/popup/home.component"; -import { LoginDecryptionOptionsComponentV1 } from "../auth/popup/login-decryption-options/login-decryption-options-v1.component"; -import { LoginComponentV1 } from "../auth/popup/login-v1.component"; -import { LoginViaAuthRequestComponentV1 } from "../auth/popup/login-via-auth-request-v1.component"; import { RemovePasswordComponent } from "../auth/popup/remove-password.component"; import { SetPasswordComponent } from "../auth/popup/set-password.component"; import { AccountSecurityComponent } from "../auth/popup/settings/account-security.component"; -import { SsoComponentV1 } from "../auth/popup/sso-v1.component"; import { TwoFactorOptionsComponentV1 } from "../auth/popup/two-factor-options-v1.component"; import { TwoFactorComponentV1 } from "../auth/popup/two-factor-v1.component"; import { UpdateTempPasswordComponent } from "../auth/popup/update-temp-password.component"; @@ -131,20 +124,19 @@ const routes: Routes = [ children: [], // Children lets us have an empty component. canActivate: [ popupRouterCacheGuard, - redirectGuard({ loggedIn: "/tabs/current", loggedOut: "/home", locked: "/lock" }), + redirectGuard({ loggedIn: "/tabs/current", loggedOut: "/login", locked: "/lock" }), ], }, + { + path: "home", + redirectTo: "login", + pathMatch: "full", + }, { path: "vault", redirectTo: "/tabs/vault", pathMatch: "full", }, - { - path: "home", - component: HomeComponent, - canActivate: [unauthGuardFn(unauthRouteOverrides), unauthUiRefreshRedirect("/login")], - data: { elevation: 1 } satisfies RouteDataProperties, - }, { path: "fido2", component: Fido2Component, @@ -206,40 +198,6 @@ const routes: Routes = [ canActivate: [unauthGuardFn(unauthRouteOverrides)], data: { elevation: 1 } satisfies RouteDataProperties, }, - ...unauthUiRefreshSwap( - SsoComponentV1, - ExtensionAnonLayoutWrapperComponent, - { - path: "sso", - canActivate: [unauthGuardFn(unauthRouteOverrides)], - data: { elevation: 1 } satisfies RouteDataProperties, - }, - { - path: "sso", - canActivate: [unauthGuardFn(unauthRouteOverrides)], - data: { - pageIcon: VaultIcon, - pageTitle: { - key: "enterpriseSingleSignOn", - }, - pageSubtitle: { - key: "singleSignOnEnterOrgIdentifierText", - }, - elevation: 1, - } satisfies RouteDataProperties & ExtensionAnonLayoutWrapperData, - children: [ - { path: "", component: SsoComponent }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - data: { - overlayPosition: ExtensionDefaultOverlayPosition, - } satisfies EnvironmentSelectorRouteData, - }, - ], - }, - ), { path: "device-verification", component: ExtensionAnonLayoutWrapperComponent, @@ -420,158 +378,7 @@ const routes: Routes = [ canActivate: [authGuard], data: { elevation: 1 } satisfies RouteDataProperties, }, - ...unauthUiRefreshSwap( - LoginViaAuthRequestComponentV1, - ExtensionAnonLayoutWrapperComponent, - { - path: "login-with-device", - data: { elevation: 1 } satisfies RouteDataProperties, - }, - { - path: "login-with-device", - data: { - pageIcon: DevicesIcon, - pageTitle: { - key: "logInRequestSent", - }, - pageSubtitle: { - key: "aNotificationWasSentToYourDevice", - }, - showLogo: false, - showBackButton: true, - elevation: 1, - } satisfies RouteDataProperties & ExtensionAnonLayoutWrapperData, - children: [ - { path: "", component: LoginViaAuthRequestComponent }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - }, - ], - }, - ), - ...unauthUiRefreshSwap( - LoginViaAuthRequestComponentV1, - ExtensionAnonLayoutWrapperComponent, - { - path: "admin-approval-requested", - data: { elevation: 1 } satisfies RouteDataProperties, - }, - { - path: "admin-approval-requested", - data: { - pageIcon: DevicesIcon, - pageTitle: { - key: "adminApprovalRequested", - }, - pageSubtitle: { - key: "adminApprovalRequestSentToAdmins", - }, - showLogo: false, - showBackButton: true, - elevation: 1, - } satisfies RouteDataProperties & ExtensionAnonLayoutWrapperData, - children: [{ path: "", component: LoginViaAuthRequestComponent }], - }, - ), - ...unauthUiRefreshSwap( - HintComponent, - ExtensionAnonLayoutWrapperComponent, - { - path: "hint", - canActivate: [unauthGuardFn(unauthRouteOverrides)], - data: { - elevation: 1, - } satisfies RouteDataProperties, - }, - { - path: "", - children: [ - { - path: "hint", - canActivate: [unauthGuardFn(unauthRouteOverrides)], - data: { - pageTitle: { - key: "requestPasswordHint", - }, - pageSubtitle: { - key: "enterYourAccountEmailAddressAndYourPasswordHintWillBeSentToYou", - }, - pageIcon: UserLockIcon, - showBackButton: true, - elevation: 1, - } satisfies RouteDataProperties & ExtensionAnonLayoutWrapperData, - children: [ - { path: "", component: PasswordHintComponent }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - data: { - overlayPosition: ExtensionDefaultOverlayPosition, - } satisfies EnvironmentSelectorRouteData, - }, - ], - }, - ], - }, - ), - ...unauthUiRefreshSwap( - LoginComponentV1, - ExtensionAnonLayoutWrapperComponent, - { - path: "login", - canActivate: [unauthGuardFn(unauthRouteOverrides)], - data: { elevation: 1 }, - }, - { - path: "", - children: [ - { - path: "login", - canActivate: [unauthGuardFn(unauthRouteOverrides)], - data: { - pageIcon: VaultIcon, - pageTitle: { - key: "logInToBitwarden", - }, - elevation: 1, - showAcctSwitcher: true, - } satisfies RouteDataProperties & ExtensionAnonLayoutWrapperData, - children: [ - { path: "", component: LoginComponent }, - { path: "", component: LoginSecondaryContentComponent, outlet: "secondary" }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - data: { - overlayPosition: ExtensionDefaultOverlayPosition, - } satisfies EnvironmentSelectorRouteData, - }, - ], - }, - ], - }, - ), - ...unauthUiRefreshSwap( - LoginDecryptionOptionsComponentV1, - ExtensionAnonLayoutWrapperComponent, - { - path: "login-initiated", - canActivate: [tdeDecryptionRequiredGuard()], - data: { elevation: 1 } satisfies RouteDataProperties, - }, - { - path: "login-initiated", - canActivate: [tdeDecryptionRequiredGuard()], - data: { - pageIcon: DevicesIcon, - }, - children: [{ path: "", component: LoginDecryptionOptionsComponent }], - }, - ), + { path: "", component: ExtensionAnonLayoutWrapperComponent, @@ -597,7 +404,7 @@ const routes: Routes = [ component: RegistrationStartSecondaryComponent, outlet: "secondary", data: { - loginRoute: "/home", + loginRoute: "/login", } satisfies RegistrationStartSecondaryComponentData, }, ], @@ -617,6 +424,127 @@ const routes: Routes = [ }, ], }, + { + path: "login", + canActivate: [unauthGuardFn(unauthRouteOverrides)], + data: { + pageIcon: VaultIcon, + pageTitle: { + key: "logInToBitwarden", + }, + elevation: 1, + showAcctSwitcher: true, + } satisfies RouteDataProperties & ExtensionAnonLayoutWrapperData, + children: [ + { path: "", component: LoginComponent }, + { path: "", component: LoginSecondaryContentComponent, outlet: "secondary" }, + { + path: "", + component: EnvironmentSelectorComponent, + outlet: "environment-selector", + data: { + overlayPosition: ExtensionDefaultOverlayPosition, + } satisfies EnvironmentSelectorRouteData, + }, + ], + }, + { + path: "sso", + canActivate: [unauthGuardFn(unauthRouteOverrides)], + data: { + pageIcon: VaultIcon, + pageTitle: { + key: "enterpriseSingleSignOn", + }, + pageSubtitle: { + key: "singleSignOnEnterOrgIdentifierText", + }, + elevation: 1, + } satisfies RouteDataProperties & ExtensionAnonLayoutWrapperData, + children: [ + { path: "", component: SsoComponent }, + { + path: "", + component: EnvironmentSelectorComponent, + outlet: "environment-selector", + data: { + overlayPosition: ExtensionDefaultOverlayPosition, + } satisfies EnvironmentSelectorRouteData, + }, + ], + }, + { + path: "login-with-device", + data: { + pageIcon: DevicesIcon, + pageTitle: { + key: "logInRequestSent", + }, + pageSubtitle: { + key: "aNotificationWasSentToYourDevice", + }, + showBackButton: true, + elevation: 1, + } satisfies RouteDataProperties & ExtensionAnonLayoutWrapperData, + children: [ + { path: "", component: LoginViaAuthRequestComponent }, + { + path: "", + component: EnvironmentSelectorComponent, + outlet: "environment-selector", + }, + ], + }, + { + path: "hint", + canActivate: [unauthGuardFn(unauthRouteOverrides)], + data: { + pageTitle: { + key: "requestPasswordHint", + }, + pageSubtitle: { + key: "enterYourAccountEmailAddressAndYourPasswordHintWillBeSentToYou", + }, + pageIcon: UserLockIcon, + showBackButton: true, + elevation: 1, + } satisfies RouteDataProperties & ExtensionAnonLayoutWrapperData, + children: [ + { path: "", component: PasswordHintComponent }, + { + path: "", + component: EnvironmentSelectorComponent, + outlet: "environment-selector", + data: { + overlayPosition: ExtensionDefaultOverlayPosition, + } satisfies EnvironmentSelectorRouteData, + }, + ], + }, + { + path: "admin-approval-requested", + data: { + pageIcon: DevicesIcon, + pageTitle: { + key: "adminApprovalRequested", + }, + pageSubtitle: { + key: "adminApprovalRequestSentToAdmins", + }, + showLogo: false, + showBackButton: true, + elevation: 1, + } satisfies RouteDataProperties & ExtensionAnonLayoutWrapperData, + children: [{ path: "", component: LoginViaAuthRequestComponent }], + }, + { + path: "login-initiated", + canActivate: [tdeDecryptionRequiredGuard()], + data: { + pageIcon: DevicesIcon, + }, + children: [{ path: "", component: LoginDecryptionOptionsComponent }], + }, { path: "lock", canActivate: [lockGuard()], diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 9a3b6429e61..2a7fbdce254 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -113,9 +113,7 @@ export class AppComponent implements OnInit, OnDestroy { }); this.changeDetectorRef.detectChanges(); } else if (msg.command === "authBlocked" || msg.command === "goHome") { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.router.navigate(["home"]); + await this.router.navigate(["login"]); } else if ( msg.command === "locked" && (msg.userId == null || msg.userId == this.activeUserId) diff --git a/apps/desktop/src/app/app-routing.module.ts b/apps/desktop/src/app/app-routing.module.ts index 19b92d4762a..3a30629b444 100644 --- a/apps/desktop/src/app/app-routing.module.ts +++ b/apps/desktop/src/app/app-routing.module.ts @@ -51,13 +51,8 @@ import { import { AccessibilityCookieComponent } from "../auth/accessibility-cookie.component"; import { maxAccountsGuardFn } from "../auth/guards/max-accounts.guard"; -import { HintComponent } from "../auth/hint.component"; -import { LoginDecryptionOptionsComponentV1 } from "../auth/login/login-decryption-options/login-decryption-options-v1.component"; -import { LoginComponentV1 } from "../auth/login/login-v1.component"; -import { LoginViaAuthRequestComponentV1 } from "../auth/login/login-via-auth-request-v1.component"; import { RemovePasswordComponent } from "../auth/remove-password.component"; import { SetPasswordComponent } from "../auth/set-password.component"; -import { SsoComponentV1 } from "../auth/sso-v1.component"; import { TwoFactorComponentV1 } from "../auth/two-factor-v1.component"; import { UpdateTempPasswordComponent } from "../auth/update-temp-password.component"; import { VaultComponent } from "../vault/app/vault/vault.component"; @@ -167,33 +162,6 @@ const routes: Routes = [ }, { path: "accessibility-cookie", component: AccessibilityCookieComponent }, { path: "set-password", component: SetPasswordComponent }, - ...unauthUiRefreshSwap( - SsoComponentV1, - AnonLayoutWrapperComponent, - { - path: "sso", - }, - { - path: "sso", - data: { - pageIcon: VaultIcon, - pageTitle: { - key: "enterpriseSingleSignOn", - }, - pageSubtitle: { - key: "singleSignOnEnterOrgIdentifierText", - }, - } satisfies AnonLayoutWrapperData, - children: [ - { path: "", component: SsoComponent }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - }, - ], - }, - ), { path: "send", component: SendComponent, @@ -209,139 +177,6 @@ const routes: Routes = [ component: RemovePasswordComponent, canActivate: [authGuard], }, - ...unauthUiRefreshSwap( - LoginViaAuthRequestComponentV1, - AnonLayoutWrapperComponent, - { - path: "login-with-device", - }, - { - path: "login-with-device", - data: { - pageIcon: DevicesIcon, - pageTitle: { - key: "logInRequestSent", - }, - pageSubtitle: { - key: "aNotificationWasSentToYourDevice", - }, - } satisfies AnonLayoutWrapperData, - children: [ - { path: "", component: LoginViaAuthRequestComponent }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - }, - ], - }, - ), - ...unauthUiRefreshSwap( - LoginViaAuthRequestComponentV1, - AnonLayoutWrapperComponent, - { - path: "admin-approval-requested", - }, - { - path: "admin-approval-requested", - data: { - pageIcon: DevicesIcon, - pageTitle: { - key: "adminApprovalRequested", - }, - pageSubtitle: { - key: "adminApprovalRequestSentToAdmins", - }, - } satisfies AnonLayoutWrapperData, - children: [{ path: "", component: LoginViaAuthRequestComponent }], - }, - ), - ...unauthUiRefreshSwap( - HintComponent, - AnonLayoutWrapperComponent, - { - path: "hint", - canActivate: [unauthGuardFn()], - }, - { - path: "", - children: [ - { - path: "hint", - canActivate: [unauthGuardFn()], - data: { - pageTitle: { - key: "requestPasswordHint", - }, - pageSubtitle: { - key: "enterYourAccountEmailAddressAndYourPasswordHintWillBeSentToYou", - }, - pageIcon: UserLockIcon, - } satisfies AnonLayoutWrapperData, - children: [ - { path: "", component: PasswordHintComponent }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - }, - ], - }, - ], - }, - ), - ...unauthUiRefreshSwap( - LoginComponentV1, - AnonLayoutWrapperComponent, - { - path: "login", - component: LoginComponentV1, - canActivate: [maxAccountsGuardFn()], - }, - { - path: "", - children: [ - { - path: "login", - canActivate: [maxAccountsGuardFn()], - data: { - pageTitle: { - key: "logInToBitwarden", - }, - pageIcon: VaultIcon, - }, - children: [ - { path: "", component: LoginComponent }, - { path: "", component: LoginSecondaryContentComponent, outlet: "secondary" }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - data: { - overlayPosition: DesktopDefaultOverlayPosition, - }, - }, - ], - }, - ], - }, - ), - ...unauthUiRefreshSwap( - LoginDecryptionOptionsComponentV1, - AnonLayoutWrapperComponent, - { - path: "login-initiated", - canActivate: [tdeDecryptionRequiredGuard()], - }, - { - path: "login-initiated", - canActivate: [tdeDecryptionRequiredGuard()], - data: { - pageIcon: DevicesIcon, - }, - children: [{ path: "", component: LoginDecryptionOptionsComponent }], - }, - ), { path: "", component: AnonLayoutWrapperComponent, @@ -383,6 +218,110 @@ const routes: Routes = [ }, ], }, + { + path: "login", + canActivate: [maxAccountsGuardFn()], + data: { + pageTitle: { + key: "logInToBitwarden", + }, + pageIcon: VaultIcon, + }, + children: [ + { path: "", component: LoginComponent }, + { path: "", component: LoginSecondaryContentComponent, outlet: "secondary" }, + { + path: "", + component: EnvironmentSelectorComponent, + outlet: "environment-selector", + data: { + overlayPosition: DesktopDefaultOverlayPosition, + }, + }, + ], + }, + { + path: "login-initiated", + canActivate: [tdeDecryptionRequiredGuard()], + data: { + pageIcon: DevicesIcon, + }, + children: [{ path: "", component: LoginDecryptionOptionsComponent }], + }, + { + path: "sso", + data: { + pageIcon: VaultIcon, + pageTitle: { + key: "enterpriseSingleSignOn", + }, + pageSubtitle: { + key: "singleSignOnEnterOrgIdentifierText", + }, + } satisfies AnonLayoutWrapperData, + children: [ + { path: "", component: SsoComponent }, + { + path: "", + component: EnvironmentSelectorComponent, + outlet: "environment-selector", + }, + ], + }, + { + path: "login-with-device", + data: { + pageIcon: DevicesIcon, + pageTitle: { + key: "logInRequestSent", + }, + pageSubtitle: { + key: "aNotificationWasSentToYourDevice", + }, + } satisfies AnonLayoutWrapperData, + children: [ + { path: "", component: LoginViaAuthRequestComponent }, + { + path: "", + component: EnvironmentSelectorComponent, + outlet: "environment-selector", + }, + ], + }, + { + path: "admin-approval-requested", + data: { + pageIcon: DevicesIcon, + pageTitle: { + key: "adminApprovalRequested", + }, + pageSubtitle: { + key: "adminApprovalRequestSentToAdmins", + }, + } satisfies AnonLayoutWrapperData, + children: [{ path: "", component: LoginViaAuthRequestComponent }], + }, + { + path: "hint", + canActivate: [unauthGuardFn()], + data: { + pageTitle: { + key: "requestPasswordHint", + }, + pageSubtitle: { + key: "enterYourAccountEmailAddressAndYourPasswordHintWillBeSentToYou", + }, + pageIcon: UserLockIcon, + } satisfies AnonLayoutWrapperData, + children: [ + { path: "", component: PasswordHintComponent }, + { + path: "", + component: EnvironmentSelectorComponent, + outlet: "environment-selector", + }, + ], + }, { path: "lock", canActivate: [lockGuard()], diff --git a/apps/web/src/app/oss-routing.module.ts b/apps/web/src/app/oss-routing.module.ts index adb38ef43f0..c531f358b34 100644 --- a/apps/web/src/app/oss-routing.module.ts +++ b/apps/web/src/app/oss-routing.module.ts @@ -55,10 +55,6 @@ import { AcceptFamilySponsorshipComponent } from "./admin-console/organizations/ import { FamiliesForEnterpriseSetupComponent } from "./admin-console/organizations/sponsorships/families-for-enterprise-setup.component"; import { CreateOrganizationComponent } from "./admin-console/settings/create-organization.component"; import { deepLinkGuard } from "./auth/guards/deep-link.guard"; -import { HintComponent } from "./auth/hint.component"; -import { LoginDecryptionOptionsComponentV1 } from "./auth/login/login-decryption-options/login-decryption-options-v1.component"; -import { LoginComponentV1 } from "./auth/login/login-v1.component"; -import { LoginViaAuthRequestComponentV1 } from "./auth/login/login-via-auth-request-v1.component"; import { LoginViaWebAuthnComponent } from "./auth/login/login-via-webauthn/login-via-webauthn.component"; import { AcceptOrganizationComponent } from "./auth/organization-invite/accept-organization.component"; import { RecoverDeleteComponent } from "./auth/recover-delete.component"; @@ -69,7 +65,6 @@ import { AccountComponent } from "./auth/settings/account/account.component"; import { EmergencyAccessComponent } from "./auth/settings/emergency-access/emergency-access.component"; import { EmergencyAccessViewComponent } from "./auth/settings/emergency-access/view/emergency-access-view.component"; import { SecurityRoutingModule } from "./auth/settings/security/security-routing.module"; -import { SsoComponentV1 } from "./auth/sso-v1.component"; import { TwoFactorComponentV1 } from "./auth/two-factor-v1.component"; import { UpdatePasswordComponent } from "./auth/update-password.component"; import { UpdateTempPasswordComponent } from "./auth/update-temp-password.component"; @@ -172,172 +167,6 @@ const routes: Routes = [ }, ], }, - ...unauthUiRefreshSwap( - LoginViaAuthRequestComponentV1, - AnonLayoutWrapperComponent, - { - path: "login-with-device", - data: { titleId: "loginWithDevice" } satisfies RouteDataProperties, - }, - { - path: "login-with-device", - data: { - pageIcon: DevicesIcon, - pageTitle: { - key: "logInRequestSent", - }, - pageSubtitle: { - key: "aNotificationWasSentToYourDevice", - }, - titleId: "loginInitiated", - } satisfies RouteDataProperties & AnonLayoutWrapperData, - children: [ - { path: "", component: LoginViaAuthRequestComponent }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - }, - ], - }, - ), - ...unauthUiRefreshSwap( - LoginViaAuthRequestComponentV1, - AnonLayoutWrapperComponent, - { - path: "admin-approval-requested", - data: { titleId: "adminApprovalRequested" } satisfies RouteDataProperties, - }, - { - path: "admin-approval-requested", - data: { - pageIcon: DevicesIcon, - pageTitle: { - key: "adminApprovalRequested", - }, - pageSubtitle: { - key: "adminApprovalRequestSentToAdmins", - }, - titleId: "adminApprovalRequested", - } satisfies RouteDataProperties & AnonLayoutWrapperData, - children: [{ path: "", component: LoginViaAuthRequestComponent }], - }, - ), - ...unauthUiRefreshSwap( - AnonLayoutWrapperComponent, - AnonLayoutWrapperComponent, - { - path: "login", - canActivate: [unauthGuardFn()], - children: [ - { - path: "", - component: LoginComponentV1, - }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - }, - ], - data: { - pageTitle: { - key: "logIn", - }, - }, - }, - { - path: "login", - canActivate: [unauthGuardFn()], - data: { - pageTitle: { - key: "logInToBitwarden", - }, - pageIcon: VaultIcon, - } satisfies RouteDataProperties & AnonLayoutWrapperData, - children: [ - { - path: "", - component: LoginComponent, - }, - { - path: "", - component: LoginSecondaryContentComponent, - outlet: "secondary", - }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - }, - ], - }, - ), - ...unauthUiRefreshSwap( - LoginDecryptionOptionsComponentV1, - AnonLayoutWrapperComponent, - { - path: "login-initiated", - canActivate: [tdeDecryptionRequiredGuard()], - }, - { - path: "login-initiated", - canActivate: [tdeDecryptionRequiredGuard()], - data: { - pageIcon: DevicesIcon, - }, - children: [{ path: "", component: LoginDecryptionOptionsComponent }], - }, - ), - ...unauthUiRefreshSwap( - AnonLayoutWrapperComponent, - AnonLayoutWrapperComponent, - { - path: "hint", - canActivate: [unauthGuardFn()], - data: { - pageTitle: { - key: "passwordHint", - }, - titleId: "passwordHint", - }, - children: [ - { path: "", component: HintComponent }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - }, - ], - }, - { - path: "", - children: [ - { - path: "hint", - canActivate: [unauthGuardFn()], - data: { - pageTitle: { - key: "requestPasswordHint", - }, - pageSubtitle: { - key: "enterYourAccountEmailAddressAndYourPasswordHintWillBeSentToYou", - }, - pageIcon: UserLockIcon, - state: "hint", - }, - children: [ - { path: "", component: PasswordHintComponent }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - }, - ], - }, - ], - }, - ), { path: "", component: AnonLayoutWrapperComponent, @@ -381,6 +210,97 @@ const routes: Routes = [ }, ], }, + { + path: "login", + canActivate: [unauthGuardFn()], + data: { + pageTitle: { + key: "logInToBitwarden", + }, + pageIcon: VaultIcon, + } satisfies RouteDataProperties & AnonLayoutWrapperData, + children: [ + { + path: "", + component: LoginComponent, + }, + { + path: "", + component: LoginSecondaryContentComponent, + outlet: "secondary", + }, + { + path: "", + component: EnvironmentSelectorComponent, + outlet: "environment-selector", + }, + ], + }, + { + path: "login-with-device", + data: { + pageIcon: DevicesIcon, + pageTitle: { + key: "logInRequestSent", + }, + pageSubtitle: { + key: "aNotificationWasSentToYourDevice", + }, + titleId: "loginInitiated", + } satisfies RouteDataProperties & AnonLayoutWrapperData, + children: [ + { path: "", component: LoginViaAuthRequestComponent }, + { + path: "", + component: EnvironmentSelectorComponent, + outlet: "environment-selector", + }, + ], + }, + { + path: "admin-approval-requested", + data: { + pageIcon: DevicesIcon, + pageTitle: { + key: "adminApprovalRequested", + }, + pageSubtitle: { + key: "adminApprovalRequestSentToAdmins", + }, + titleId: "adminApprovalRequested", + } satisfies RouteDataProperties & AnonLayoutWrapperData, + children: [{ path: "", component: LoginViaAuthRequestComponent }], + }, + { + path: "hint", + canActivate: [unauthGuardFn()], + data: { + pageTitle: { + key: "requestPasswordHint", + }, + pageSubtitle: { + key: "enterYourAccountEmailAddressAndYourPasswordHintWillBeSentToYou", + }, + pageIcon: UserLockIcon, + state: "hint", + }, + children: [ + { path: "", component: PasswordHintComponent }, + { + path: "", + component: EnvironmentSelectorComponent, + outlet: "environment-selector", + }, + ], + }, + { + path: "login-initiated", + canActivate: [tdeDecryptionRequiredGuard()], + data: { + pageIcon: DevicesIcon, + }, + children: [{ path: "", component: LoginDecryptionOptionsComponent }], + }, { path: "send/:sendId/:key", data: { @@ -432,64 +352,24 @@ const routes: Routes = [ }, ], }, - ...unauthUiRefreshSwap( - SsoComponentV1, - SsoComponent, - { - path: "sso", - canActivate: [unauthGuardFn()], - data: { - pageTitle: { - key: "enterpriseSingleSignOn", - }, - titleId: "enterpriseSingleSignOn", - } satisfies RouteDataProperties & AnonLayoutWrapperData, - children: [ - { - path: "", - component: SsoComponentV1, - }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - }, - ], - }, - { - path: "sso", - canActivate: [unauthGuardFn()], - data: { - pageTitle: { - key: "singleSignOn", - }, - titleId: "enterpriseSingleSignOn", - pageSubtitle: { - key: "singleSignOnEnterOrgIdentifierText", - }, - titleAreaMaxWidth: "md", - pageIcon: SsoKeyIcon, - } satisfies RouteDataProperties & AnonLayoutWrapperData, - children: [ - { - path: "", - component: SsoComponent, - }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - }, - ], - }, - ), { - path: "login", + path: "sso", canActivate: [unauthGuardFn()], + data: { + pageTitle: { + key: "singleSignOn", + }, + titleId: "enterpriseSingleSignOn", + pageSubtitle: { + key: "singleSignOnEnterOrgIdentifierText", + }, + titleAreaMaxWidth: "md", + pageIcon: SsoKeyIcon, + } satisfies RouteDataProperties & AnonLayoutWrapperData, children: [ { path: "", - component: LoginComponent, + component: SsoComponent, }, { path: "", @@ -497,11 +377,6 @@ const routes: Routes = [ outlet: "environment-selector", }, ], - data: { - pageTitle: { - key: "logIn", - }, - }, }, ...unauthUiRefreshSwap( TwoFactorComponentV1, diff --git a/libs/auth/src/angular/login/login.component.ts b/libs/auth/src/angular/login/login.component.ts index c291a64a8c5..cc38ec5dfb3 100644 --- a/libs/auth/src/angular/login/login.component.ts +++ b/libs/auth/src/angular/login/login.component.ts @@ -2,7 +2,7 @@ import { CommonModule } from "@angular/common"; import { Component, ElementRef, NgZone, OnDestroy, OnInit, ViewChild } from "@angular/core"; import { FormBuilder, FormControl, ReactiveFormsModule, Validators } from "@angular/forms"; import { ActivatedRoute, Router, RouterModule } from "@angular/router"; -import { firstValueFrom, Subject, take, takeUntil, tap } from "rxjs"; +import { firstValueFrom, Subject, take, takeUntil } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { @@ -19,11 +19,9 @@ import { DevicesApiServiceAbstraction } from "@bitwarden/common/auth/abstraction import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { ClientType, HttpStatusCode } from "@bitwarden/common/enums"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service"; import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; @@ -121,7 +119,6 @@ export class LoginComponent implements OnInit, OnDestroy { private toastService: ToastService, private logService: LogService, private validationService: ValidationService, - private configService: ConfigService, private loginSuccessHandlerService: LoginSuccessHandlerService, ) { this.clientType = this.platformUtilsService.getClientType(); @@ -131,9 +128,6 @@ export class LoginComponent implements OnInit, OnDestroy { // Add popstate listener to listen for browser back button clicks window.addEventListener("popstate", this.handlePopState); - // TODO: remove this when the UnauthenticatedExtensionUIRefresh feature flag is removed. - this.listenForUnauthUiRefreshFlagChanges(); - await this.defaultOnInit(); if (this.clientType === ClientType.Desktop) { @@ -154,30 +148,6 @@ export class LoginComponent implements OnInit, OnDestroy { this.destroy$.complete(); } - private listenForUnauthUiRefreshFlagChanges() { - this.configService - .getFeatureFlag$(FeatureFlag.UnauthenticatedExtensionUIRefresh) - .pipe( - tap(async (flag) => { - // If the flag is turned OFF, we must force a reload to ensure the correct UI is shown - if (!flag) { - const qParams = await firstValueFrom(this.activatedRoute.queryParams); - const uniqueQueryParams = { - ...qParams, - // adding a unique timestamp to the query params to force a reload - t: new Date().getTime().toString(), // Adding a unique timestamp as a query parameter - }; - - await this.router.navigate(["/"], { - queryParams: uniqueQueryParams, - }); - } - }), - takeUntil(this.destroy$), - ) - .subscribe(); - } - submit = async (): Promise => { if (this.clientType === ClientType.Desktop) { if (this.loginUiState !== LoginUiState.MASTER_PASSWORD_ENTRY) { From c3c4c9c54c51fab19ac56f3cee94d898139e8606 Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Mon, 10 Mar 2025 11:12:24 -0700 Subject: [PATCH 05/23] bold new settings callout link (#13664) --- .../new-settings-callout/new-settings-callout.component.html | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/browser/src/vault/popup/components/vault-v2/new-settings-callout/new-settings-callout.component.html b/apps/browser/src/vault/popup/components/vault-v2/new-settings-callout/new-settings-callout.component.html index a6abe8ed3ac..6cc60eed6d5 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/new-settings-callout/new-settings-callout.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/new-settings-callout/new-settings-callout.component.html @@ -17,6 +17,7 @@ Date: Mon, 10 Mar 2025 14:58:11 -0400 Subject: [PATCH 06/23] add tw class (#13765) --- .../manage/device-approvals/device-approvals.component.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/device-approvals/device-approvals.component.html b/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/device-approvals/device-approvals.component.html index 7723324781b..cafd0744a8f 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/device-approvals/device-approvals.component.html +++ b/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/device-approvals/device-approvals.component.html @@ -1,7 +1,7 @@ From 0568a09212f56c58435a59935b3284ad93734bbf Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Mon, 10 Mar 2025 12:17:46 -0700 Subject: [PATCH 07/23] refactor(device-trust-toasts): [Auth/PM-11225] Refactor Toasts from Auth Services (#13665) Refactor toast calls out of auth services. Toasts are now triggered by an observable emission that gets picked up by an observable pipeline in a new `DeviceTrustToastService` (libs/angular). That observable pipeline is then subscribed by by consuming the `AppComponent` for each client. --- apps/browser/src/popup/app.component.ts | 7 +- apps/desktop/src/app/app.component.ts | 7 +- apps/web/src/app/app.component.ts | 7 +- .../device-trust-toast.service.abstraction.ts | 9 + ...vice-trust-toast.service.implementation.ts | 44 +++++ .../device-trust-toast.service.spec.ts | 167 ++++++++++++++++++ .../src/services/jslib-services.module.ts | 12 ++ .../auth-request.service.abstraction.ts | 15 ++ .../login-strategies/sso-login.strategy.ts | 3 +- .../auth-request/auth-request.service.ts | 9 + .../device-trust.service.abstraction.ts | 6 + .../device-trust.service.implementation.ts | 9 +- 12 files changed, 289 insertions(+), 6 deletions(-) create mode 100644 libs/angular/src/auth/services/device-trust-toast.service.abstraction.ts create mode 100644 libs/angular/src/auth/services/device-trust-toast.service.implementation.ts create mode 100644 libs/angular/src/auth/services/device-trust-toast.service.spec.ts diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 2a7fbdce254..6a08bf007bb 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -1,9 +1,11 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { ChangeDetectorRef, Component, NgZone, OnDestroy, OnInit, inject } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { NavigationEnd, Router, RouterOutlet } from "@angular/router"; import { Subject, takeUntil, firstValueFrom, concatMap, filter, tap } from "rxjs"; +import { DeviceTrustToastService } from "@bitwarden/angular/auth/services/device-trust-toast.service.abstraction"; import { LogoutReason } from "@bitwarden/auth/common"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; @@ -68,7 +70,10 @@ export class AppComponent implements OnInit, OnDestroy { private animationControlService: AnimationControlService, private biometricStateService: BiometricStateService, private biometricsService: BiometricsService, - ) {} + private deviceTrustToastService: DeviceTrustToastService, + ) { + this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); + } async ngOnInit() { initPopupClosedListener(); diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 85c159f0278..1ef03e5bb71 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -10,10 +10,12 @@ import { ViewChild, ViewContainerRef, } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { Router } from "@angular/router"; import { filter, firstValueFrom, map, Subject, takeUntil, timeout, withLatestFrom } from "rxjs"; import { CollectionService } from "@bitwarden/admin-console/common"; +import { DeviceTrustToastService } from "@bitwarden/angular/auth/services/device-trust-toast.service.abstraction"; import { ModalRef } from "@bitwarden/angular/components/modal/modal.ref"; import { ModalService } from "@bitwarden/angular/services/modal.service"; import { FingerprintDialogComponent, LoginApprovalComponent } from "@bitwarden/auth/angular"; @@ -157,7 +159,10 @@ export class AppComponent implements OnInit, OnDestroy { private stateEventRunnerService: StateEventRunnerService, private accountService: AccountService, private organizationService: OrganizationService, - ) {} + private deviceTrustToastService: DeviceTrustToastService, + ) { + this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); + } ngOnInit() { this.accountService.activeAccount$.pipe(takeUntil(this.destroy$)).subscribe((account) => { diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index 9d2afb22688..f6e038f85d9 100644 --- a/apps/web/src/app/app.component.ts +++ b/apps/web/src/app/app.component.ts @@ -2,11 +2,13 @@ // @ts-strict-ignore import { DOCUMENT } from "@angular/common"; import { Component, Inject, NgZone, OnDestroy, OnInit } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { NavigationEnd, Router } from "@angular/router"; import * as jq from "jquery"; import { Subject, filter, firstValueFrom, map, takeUntil, timeout } from "rxjs"; import { CollectionService } from "@bitwarden/admin-console/common"; +import { DeviceTrustToastService } from "@bitwarden/angular/auth/services/device-trust-toast.service.abstraction"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { EventUploadService } from "@bitwarden/common/abstractions/event/event-upload.service"; import { SearchService } from "@bitwarden/common/abstractions/search.service"; @@ -95,7 +97,10 @@ export class AppComponent implements OnDestroy, OnInit { private apiService: ApiService, private appIdService: AppIdService, private processReloadService: ProcessReloadServiceAbstraction, - ) {} + private deviceTrustToastService: DeviceTrustToastService, + ) { + this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); + } ngOnInit() { this.i18nService.locale$.pipe(takeUntil(this.destroy$)).subscribe((locale) => { diff --git a/libs/angular/src/auth/services/device-trust-toast.service.abstraction.ts b/libs/angular/src/auth/services/device-trust-toast.service.abstraction.ts new file mode 100644 index 00000000000..3de288168b1 --- /dev/null +++ b/libs/angular/src/auth/services/device-trust-toast.service.abstraction.ts @@ -0,0 +1,9 @@ +import { Observable } from "rxjs"; + +export abstract class DeviceTrustToastService { + /** + * An observable pipeline that observes any cross-application toast messages + * that need to be shown as part of the trusted device encryption (TDE) process. + */ + abstract setupListeners$: Observable; +} diff --git a/libs/angular/src/auth/services/device-trust-toast.service.implementation.ts b/libs/angular/src/auth/services/device-trust-toast.service.implementation.ts new file mode 100644 index 00000000000..330519683f3 --- /dev/null +++ b/libs/angular/src/auth/services/device-trust-toast.service.implementation.ts @@ -0,0 +1,44 @@ +import { merge, Observable, tap } from "rxjs"; + +import { AuthRequestServiceAbstraction } from "@bitwarden/auth/common"; +import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { ToastService } from "@bitwarden/components"; + +import { DeviceTrustToastService as DeviceTrustToastServiceAbstraction } from "./device-trust-toast.service.abstraction"; + +export class DeviceTrustToastService implements DeviceTrustToastServiceAbstraction { + private adminLoginApproved$: Observable; + private deviceTrusted$: Observable; + + setupListeners$: Observable; + + constructor( + private authRequestService: AuthRequestServiceAbstraction, + private deviceTrustService: DeviceTrustServiceAbstraction, + private i18nService: I18nService, + private toastService: ToastService, + ) { + this.adminLoginApproved$ = this.authRequestService.adminLoginApproved$.pipe( + tap(() => { + this.toastService.showToast({ + variant: "success", + title: "", + message: this.i18nService.t("loginApproved"), + }); + }), + ); + + this.deviceTrusted$ = this.deviceTrustService.deviceTrusted$.pipe( + tap(() => { + this.toastService.showToast({ + variant: "success", + title: "", + message: this.i18nService.t("deviceTrusted"), + }); + }), + ); + + this.setupListeners$ = merge(this.adminLoginApproved$, this.deviceTrusted$); + } +} diff --git a/libs/angular/src/auth/services/device-trust-toast.service.spec.ts b/libs/angular/src/auth/services/device-trust-toast.service.spec.ts new file mode 100644 index 00000000000..cd9c6b0acf5 --- /dev/null +++ b/libs/angular/src/auth/services/device-trust-toast.service.spec.ts @@ -0,0 +1,167 @@ +import { mock, MockProxy } from "jest-mock-extended"; +import { EMPTY, of } from "rxjs"; + +import { AuthRequestServiceAbstraction } from "@bitwarden/auth/common"; +import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { ToastService } from "@bitwarden/components"; + +import { DeviceTrustToastService as DeviceTrustToastServiceAbstraction } from "./device-trust-toast.service.abstraction"; +import { DeviceTrustToastService } from "./device-trust-toast.service.implementation"; + +describe("DeviceTrustToastService", () => { + let authRequestService: MockProxy; + let deviceTrustService: MockProxy; + let i18nService: MockProxy; + let toastService: MockProxy; + + let sut: DeviceTrustToastServiceAbstraction; + + beforeEach(() => { + authRequestService = mock(); + deviceTrustService = mock(); + i18nService = mock(); + toastService = mock(); + + i18nService.t.mockImplementation((key: string) => key); // just return the key that was given + }); + + const initService = () => { + return new DeviceTrustToastService( + authRequestService, + deviceTrustService, + i18nService, + toastService, + ); + }; + + const loginApprovalToastOptions = { + variant: "success", + title: "", + message: "loginApproved", + }; + + const deviceTrustedToastOptions = { + variant: "success", + title: "", + message: "deviceTrusted", + }; + + describe("setupListeners$", () => { + describe("given adminLoginApproved$ emits and deviceTrusted$ emits", () => { + beforeEach(() => { + // Arrange + authRequestService.adminLoginApproved$ = of(undefined); + deviceTrustService.deviceTrusted$ = of(undefined); + sut = initService(); + }); + + it("should trigger a toast for login approval", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).toHaveBeenCalledWith(loginApprovalToastOptions); // Assert + done(); + }, + }); + }); + + it("should trigger a toast for device trust", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).toHaveBeenCalledWith(deviceTrustedToastOptions); // Assert + done(); + }, + }); + }); + }); + + describe("given adminLoginApproved$ emits and deviceTrusted$ does not emit", () => { + beforeEach(() => { + // Arrange + authRequestService.adminLoginApproved$ = of(undefined); + deviceTrustService.deviceTrusted$ = EMPTY; + sut = initService(); + }); + + it("should trigger a toast for login approval", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).toHaveBeenCalledWith(loginApprovalToastOptions); // Assert + done(); + }, + }); + }); + + it("should NOT trigger a toast for device trust", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).not.toHaveBeenCalledWith(deviceTrustedToastOptions); // Assert + done(); + }, + }); + }); + }); + + describe("given adminLoginApproved$ does not emit and deviceTrusted$ emits", () => { + beforeEach(() => { + // Arrange + authRequestService.adminLoginApproved$ = EMPTY; + deviceTrustService.deviceTrusted$ = of(undefined); + sut = initService(); + }); + + it("should NOT trigger a toast for login approval", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).not.toHaveBeenCalledWith(loginApprovalToastOptions); // Assert + done(); + }, + }); + }); + + it("should trigger a toast for device trust", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).toHaveBeenCalledWith(deviceTrustedToastOptions); // Assert + done(); + }, + }); + }); + }); + + describe("given adminLoginApproved$ does not emit and deviceTrusted$ does not emit", () => { + beforeEach(() => { + // Arrange + authRequestService.adminLoginApproved$ = EMPTY; + deviceTrustService.deviceTrusted$ = EMPTY; + sut = initService(); + }); + + it("should NOT trigger a toast for login approval", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).not.toHaveBeenCalledWith(loginApprovalToastOptions); // Assert + done(); + }, + }); + }); + + it("should NOT trigger a toast for device trust", (done) => { + // Act + sut.setupListeners$.subscribe({ + complete: () => { + expect(toastService.showToast).not.toHaveBeenCalledWith(deviceTrustedToastOptions); // Assert + done(); + }, + }); + }); + }); + }); +}); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 9ee49a30689..93e29846e69 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -317,6 +317,8 @@ import { IndividualVaultExportServiceAbstraction, } from "@bitwarden/vault-export-core"; +import { DeviceTrustToastService as DeviceTrustToastServiceAbstraction } from "../auth/services/device-trust-toast.service.abstraction"; +import { DeviceTrustToastService } from "../auth/services/device-trust-toast.service.implementation"; import { FormValidationErrorsService as FormValidationErrorsServiceAbstraction } from "../platform/abstractions/form-validation-errors.service"; import { ViewCacheService } from "../platform/abstractions/view-cache.service"; import { FormValidationErrorsService } from "../platform/services/form-validation-errors.service"; @@ -1463,6 +1465,16 @@ const safeProviders: SafeProvider[] = [ useClass: DefaultTaskService, deps: [StateProvider, ApiServiceAbstraction, OrganizationServiceAbstraction, ConfigService], }), + safeProvider({ + provide: DeviceTrustToastServiceAbstraction, + useClass: DeviceTrustToastService, + deps: [ + AuthRequestServiceAbstraction, + DeviceTrustServiceAbstraction, + I18nServiceAbstraction, + ToastService, + ], + }), ]; @NgModule({ diff --git a/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts b/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts index 1829fe6b0c9..75bb8686163 100644 --- a/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts +++ b/libs/auth/src/common/abstractions/auth-request.service.abstraction.ts @@ -12,6 +12,12 @@ export abstract class AuthRequestServiceAbstraction { /** Emits an auth request id when an auth request has been approved. */ authRequestPushNotification$: Observable; + /** + * Emits when a login has been approved by an admin. This emission is specifically for the + * purpose of notifying the consuming component to display a toast informing the user. + */ + adminLoginApproved$: Observable; + /** * Returns an admin auth request for the given user if it exists. * @param userId The user id. @@ -106,4 +112,13 @@ export abstract class AuthRequestServiceAbstraction { * @returns The dash-delimited fingerprint phrase. */ abstract getFingerprintPhrase(email: string, publicKey: Uint8Array): Promise; + + /** + * Passes a value to the adminLoginApprovedSubject via next(), which causes the + * adminLoginApproved$ observable to emit. + * + * The purpose is to notify consuming components (of adminLoginApproved$) to display + * a toast informing the user that a login has been approved by an admin. + */ + abstract emitAdminLoginApproved(): void; } diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.ts index e9d1e0a6c88..ac00ff69a4d 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.ts @@ -278,7 +278,8 @@ export class SsoLoginStrategy extends LoginStrategy { // TODO: eventually we post and clean up DB as well once consumed on client await this.authRequestService.clearAdminAuthRequest(userId); - this.platformUtilsService.showToast("success", null, this.i18nService.t("loginApproved")); + // This notification will be picked up by the SsoComponent to handle displaying a toast to the user + this.authRequestService.emitAdminLoginApproved(); } } } diff --git a/libs/auth/src/common/services/auth-request/auth-request.service.ts b/libs/auth/src/common/services/auth-request/auth-request.service.ts index 5bc200ae1e8..a6841afe0ff 100644 --- a/libs/auth/src/common/services/auth-request/auth-request.service.ts +++ b/libs/auth/src/common/services/auth-request/auth-request.service.ts @@ -43,6 +43,10 @@ export class AuthRequestService implements AuthRequestServiceAbstraction { private authRequestPushNotificationSubject = new Subject(); authRequestPushNotification$: Observable; + // Observable emission is used to trigger a toast in consuming components + private adminLoginApprovedSubject = new Subject(); + adminLoginApproved$: Observable; + constructor( private appIdService: AppIdService, private accountService: AccountService, @@ -53,6 +57,7 @@ export class AuthRequestService implements AuthRequestServiceAbstraction { private stateProvider: StateProvider, ) { this.authRequestPushNotification$ = this.authRequestPushNotificationSubject.asObservable(); + this.adminLoginApproved$ = this.adminLoginApprovedSubject.asObservable(); } async getAdminAuthRequest(userId: UserId): Promise { @@ -207,4 +212,8 @@ export class AuthRequestService implements AuthRequestServiceAbstraction { async getFingerprintPhrase(email: string, publicKey: Uint8Array): Promise { return (await this.keyService.getFingerprint(email.toLowerCase(), publicKey)).join("-"); } + + emitAdminLoginApproved(): void { + this.adminLoginApprovedSubject.next(); + } } diff --git a/libs/common/src/auth/abstractions/device-trust.service.abstraction.ts b/libs/common/src/auth/abstractions/device-trust.service.abstraction.ts index 24a5d4e8413..2de63b36cc7 100644 --- a/libs/common/src/auth/abstractions/device-trust.service.abstraction.ts +++ b/libs/common/src/auth/abstractions/device-trust.service.abstraction.ts @@ -16,6 +16,12 @@ export abstract class DeviceTrustServiceAbstraction { */ supportsDeviceTrust$: Observable; + /** + * Emits when a device has been trusted. This emission is specifically for the purpose of notifying + * the consuming component to display a toast informing the user the device has been trusted. + */ + deviceTrusted$: Observable; + /** * @description Checks if the device trust feature is supported for the given user. */ diff --git a/libs/common/src/auth/services/device-trust.service.implementation.ts b/libs/common/src/auth/services/device-trust.service.implementation.ts index fe43df53c48..4a1b901d5ef 100644 --- a/libs/common/src/auth/services/device-trust.service.implementation.ts +++ b/libs/common/src/auth/services/device-trust.service.implementation.ts @@ -1,6 +1,6 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { firstValueFrom, map, Observable } from "rxjs"; +import { firstValueFrom, map, Observable, Subject } from "rxjs"; import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; import { KeyService } from "@bitwarden/key-management"; @@ -63,6 +63,10 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { supportsDeviceTrust$: Observable; + // Observable emission is used to trigger a toast in consuming components + private deviceTrustedSubject = new Subject(); + deviceTrusted$ = this.deviceTrustedSubject.asObservable(); + constructor( private keyGenerationService: KeyGenerationService, private cryptoFunctionService: CryptoFunctionService, @@ -177,7 +181,8 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { // store device key in local/secure storage if enc keys posted to server successfully await this.setDeviceKey(userId, deviceKey); - this.platformUtilsService.showToast("success", null, this.i18nService.t("deviceTrusted")); + // This emission will be picked up by consuming components to handle displaying a toast to the user + this.deviceTrustedSubject.next(); return deviceResponse; } From 337597cf818712f872ac1dd64fece2b2f5c9aa6e Mon Sep 17 00:00:00 2001 From: Alec Rippberger <127791530+alec-livefront@users.noreply.github.com> Date: Mon, 10 Mar 2025 14:23:42 -0500 Subject: [PATCH 08/23] fix(auth): [PM-10775] Fix spacing of horizontal rules in SSO component - Remove horizontal rule above "Member decryption options" section - Add 1rem margin below horizontal rule before "type" section Resolves PM-10775 --- bitwarden_license/bit-web/src/app/auth/sso/sso.component.html | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bitwarden_license/bit-web/src/app/auth/sso/sso.component.html b/bitwarden_license/bit-web/src/app/auth/sso/sso.component.html index 22887cb7094..9cead9d21f3 100644 --- a/bitwarden_license/bit-web/src/app/auth/sso/sso.component.html +++ b/bitwarden_license/bit-web/src/app/auth/sso/sso.component.html @@ -38,8 +38,6 @@ -
- {{ "memberDecryptionOption" | i18n }} @@ -156,7 +154,7 @@ -
+
{{ "type" | i18n }} From f682870e4117aea3fd8cb2973bdb8a8e36d09ff4 Mon Sep 17 00:00:00 2001 From: Brandon Treston Date: Mon, 10 Mar 2025 15:36:21 -0400 Subject: [PATCH 09/23] remove class, add tw class (#13768) --- .../domain-verification/domain-verification.component.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/domain-verification/domain-verification.component.html b/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/domain-verification/domain-verification.component.html index c292d51ebda..adf9fcd2dcf 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/domain-verification/domain-verification.component.html +++ b/bitwarden_license/bit-web/src/app/admin-console/organizations/manage/domain-verification/domain-verification.component.html @@ -24,7 +24,7 @@ @@ -74,7 +74,7 @@ {{ orgDomain.lastCheckedDate | date: "medium" }} - +

- + { + const actual = jest.requireActual("@angular/cdk/drag-drop"); + return { + ...actual, + moveItemInArray: jest.fn(actual.moveItemInArray), + }; +}); + describe("AutofillOptionsComponent", () => { let component: AutofillOptionsComponent; let fixture: ComponentFixture; @@ -255,4 +264,111 @@ describe("AutofillOptionsComponent", () => { expect(component.autofillOptionsForm.value.uris.length).toEqual(1); }); + + describe("Drag & Drop Functionality", () => { + beforeEach(() => { + // Prevent auto‑adding an empty URI by setting a non‑null initial value. + // This overrides the call to initNewCipher. + + // Now clear any existing URIs (including the auto‑added one) + component.autofillOptionsForm.controls.uris.clear(); + + // Add exactly three URIs that we want to test reordering on. + component.addUri({ uri: "https://first.com", matchDetection: null }); + component.addUri({ uri: "https://second.com", matchDetection: null }); + component.addUri({ uri: "https://third.com", matchDetection: null }); + fixture.detectChanges(); + }); + + it("should reorder URI inputs on drop event", () => { + // Simulate a drop event that moves the first URI (index 0) to the last position (index 2). + const dropEvent: CdkDragDrop = { + previousIndex: 0, + currentIndex: 2, + container: null, + previousContainer: null, + isPointerOverContainer: true, + item: null, + distance: { x: 0, y: 0 }, + } as any; + + component.onUriItemDrop(dropEvent); + fixture.detectChanges(); + + expect(moveItemInArray).toHaveBeenCalledWith( + component.autofillOptionsForm.controls.uris.controls, + 0, + 2, + ); + }); + + it("should reorder URI input via keyboard ArrowUp", async () => { + // Clear and add exactly two URIs. + component.autofillOptionsForm.controls.uris.clear(); + component.addUri({ uri: "https://first.com", matchDetection: null }); + component.addUri({ uri: "https://second.com", matchDetection: null }); + fixture.detectChanges(); + + // Simulate pressing ArrowUp on the second URI (index 1) + const keyEvent = { + key: "ArrowUp", + preventDefault: jest.fn(), + target: document.createElement("button"), + } as unknown as KeyboardEvent; + + // Force requestAnimationFrame to run synchronously + jest.spyOn(window, "requestAnimationFrame").mockImplementation((cb: FrameRequestCallback) => { + cb(new Date().getTime()); + return 0; + }); + (liveAnnouncer.announce as jest.Mock).mockResolvedValue(null); + + await component.onUriItemKeydown(keyEvent, 1); + fixture.detectChanges(); + + expect(moveItemInArray).toHaveBeenCalledWith( + component.autofillOptionsForm.controls.uris.controls, + 1, + 0, + ); + expect(liveAnnouncer.announce).toHaveBeenCalledWith( + "reorderFieldUp websiteUri 1 2", + "assertive", + ); + }); + + it("should reorder URI input via keyboard ArrowDown", async () => { + // Clear and add exactly three URIs. + component.autofillOptionsForm.controls.uris.clear(); + component.addUri({ uri: "https://first.com", matchDetection: null }); + component.addUri({ uri: "https://second.com", matchDetection: null }); + component.addUri({ uri: "https://third.com", matchDetection: null }); + fixture.detectChanges(); + + // Simulate pressing ArrowDown on the second URI (index 1) + const keyEvent = { + key: "ArrowDown", + preventDefault: jest.fn(), + target: document.createElement("button"), + } as unknown as KeyboardEvent; + + jest.spyOn(window, "requestAnimationFrame").mockImplementation((cb: FrameRequestCallback) => { + cb(new Date().getTime()); + return 0; + }); + (liveAnnouncer.announce as jest.Mock).mockResolvedValue(null); + + await component.onUriItemKeydown(keyEvent, 1); + + expect(moveItemInArray).toHaveBeenCalledWith( + component.autofillOptionsForm.controls.uris.controls, + 1, + 2, + ); + expect(liveAnnouncer.announce).toHaveBeenCalledWith( + "reorderFieldDown websiteUri 3 3", + "assertive", + ); + }); + }); }); diff --git a/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.ts b/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.ts index c3b2a0fb9f9..5b1e4eca103 100644 --- a/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.ts +++ b/libs/vault/src/cipher-form/components/autofill-options/autofill-options.component.ts @@ -1,6 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { LiveAnnouncer } from "@angular/cdk/a11y"; +import { CdkDragDrop, DragDropModule, moveItemInArray } from "@angular/cdk/drag-drop"; import { AsyncPipe, NgForOf, NgIf } from "@angular/common"; import { Component, OnInit, QueryList, ViewChildren } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; @@ -41,6 +42,7 @@ interface UriField { templateUrl: "./autofill-options.component.html", standalone: true, imports: [ + DragDropModule, SectionComponent, SectionHeaderComponent, TypographyModule, @@ -229,4 +231,58 @@ export class AutofillOptionsComponent implements OnInit { removeUri(i: number) { this.autofillOptionsForm.controls.uris.removeAt(i); } + + /** Create a new list of LoginUriViews from the form objects and update the cipher */ + private updateUriFields() { + this.cipherFormContainer.patchCipher((cipher) => { + cipher.login.uris = this.uriControls.map( + (control) => + Object.assign(new LoginUriView(), { + uri: control.value.uri, + matchDetection: control.value.matchDetection ?? null, + }) as LoginUriView, + ); + return cipher; + }); + } + + /** Reorder the controls to match the new order after a "drop" event */ + onUriItemDrop(event: CdkDragDrop) { + moveItemInArray(this.uriControls, event.previousIndex, event.currentIndex); + this.updateUriFields(); + } + + /** Handles a uri item keyboard up or down event */ + async onUriItemKeydown(event: KeyboardEvent, index: number) { + if (event.key === "ArrowUp" && index !== 0) { + await this.reorderUriItems(event, index, "Up"); + } + + if (event.key === "ArrowDown" && index !== this.uriControls.length - 1) { + await this.reorderUriItems(event, index, "Down"); + } + } + + /** Reorders the uri items from a keyboard up or down event */ + async reorderUriItems(event: KeyboardEvent, previousIndex: number, direction: "Up" | "Down") { + const currentIndex = previousIndex + (direction === "Up" ? -1 : 1); + event.preventDefault(); + await this.liveAnnouncer.announce( + this.i18nService.t( + `reorderField${direction}`, + this.i18nService.t("websiteUri"), + currentIndex + 1, + this.uriControls.length, + ), + "assertive", + ); + moveItemInArray(this.uriControls, previousIndex, currentIndex); + this.updateUriFields(); + // Refocus the button after the reorder + // Angular re-renders the list when moving an item up which causes the focus to be lost + // Wait for the next tick to ensure the button is rendered before focusing + requestAnimationFrame(() => { + (event.target as HTMLButtonElement).focus(); + }); + } } diff --git a/libs/vault/src/cipher-form/components/autofill-options/uri-option.component.html b/libs/vault/src/cipher-form/components/autofill-options/uri-option.component.html index a55716083de..5301e4f32b9 100644 --- a/libs/vault/src/cipher-form/components/autofill-options/uri-option.component.html +++ b/libs/vault/src/cipher-form/components/autofill-options/uri-option.component.html @@ -1,35 +1,50 @@ - - {{ uriLabel }} - - - - - - - {{ "matchDetection" | i18n }} - - - - +
+
+ + {{ uriLabel }} + + + + +
+ +
+
+ + {{ "matchDetection" | i18n }} + + + + +
diff --git a/libs/vault/src/cipher-form/components/autofill-options/uri-option.component.ts b/libs/vault/src/cipher-form/components/autofill-options/uri-option.component.ts index f712e3114e0..07bf7bef775 100644 --- a/libs/vault/src/cipher-form/components/autofill-options/uri-option.component.ts +++ b/libs/vault/src/cipher-form/components/autofill-options/uri-option.component.ts @@ -1,5 +1,6 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore +import { DragDropModule } from "@angular/cdk/drag-drop"; import { NgForOf, NgIf } from "@angular/common"; import { Component, @@ -43,6 +44,7 @@ import { }, ], imports: [ + DragDropModule, FormFieldModule, ReactiveFormsModule, IconButtonModule, @@ -74,6 +76,12 @@ export class UriOptionComponent implements ControlValueAccessor { { label: this.i18nService.t("never"), value: UriMatchStrategy.Never }, ]; + /** + * Whether the option can be reordered. If false, the reorder button will be hidden. + */ + @Input({ required: true }) + canReorder: boolean; + /** * Whether the URI can be removed from the form. If false, the remove button will be hidden. */ @@ -101,6 +109,9 @@ export class UriOptionComponent implements ControlValueAccessor { */ @Input({ required: true }) index: number; + @Output() + onKeydown = new EventEmitter(); + /** * Emits when the remove button is clicked and URI should be removed from the form. */ @@ -132,6 +143,10 @@ export class UriOptionComponent implements ControlValueAccessor { private onChange: any = () => {}; private onTouched: any = () => {}; + protected handleKeydown(event: KeyboardEvent) { + this.onKeydown.emit(event); + } + constructor( private formBuilder: FormBuilder, private i18nService: I18nService, From 3b9be21fd7e37c9a42b7de722c974ed21f5f777b Mon Sep 17 00:00:00 2001 From: Patrick-Pimentel-Bitwarden Date: Mon, 10 Mar 2025 21:20:11 -0400 Subject: [PATCH 12/23] fix(auth-routing): [PM-19018] SSO TDE Routing Fix - Fixed routing logic. (#13778) * fix(auth-routing): [PM-19018] SSO TDE Routing Fix - Fixed routing logic. * PM-19018 - TwoFactorAuthTests - remove tests that are no longer applicable as 2FA comp isn't responsible for setting admin account recovery flag into state. * PM-19018 - LoginStrategyTests - add test for processing forcePasswordReset response --------- Co-authored-by: Jared Snider --- .../two-factor-auth.component.spec.ts | 59 +------------------ .../two-factor-auth.component.ts | 14 +---- .../login-strategies/login.strategy.spec.ts | 25 ++++++++ .../common/login-strategies/login.strategy.ts | 25 +++++--- .../login-strategies/sso-login.strategy.ts | 9 --- 5 files changed, 44 insertions(+), 88 deletions(-) diff --git a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts index 46b27a5aa42..6b7fca47ad5 100644 --- a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts +++ b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts @@ -226,20 +226,6 @@ describe("TwoFactorAuthComponent", () => { }); }; - const testForceResetOnSuccessfulLogin = (reasonString: string) => { - it(`navigates to the component's defined forcePasswordResetRoute route when response.forcePasswordReset is ${reasonString}`, async () => { - // Act - await component.submit("testToken"); - - // expect(mockRouter.navigate).toHaveBeenCalledTimes(1); - expect(mockRouter.navigate).toHaveBeenCalledWith(["update-temp-password"], { - queryParams: { - identifier: component.orgSsoIdentifier, - }, - }); - }); - }; - describe("Standard 2FA scenarios", () => { describe("submit", () => { const token = "testToken"; @@ -311,26 +297,6 @@ describe("TwoFactorAuthComponent", () => { }); }); - describe("Force Master Password Reset scenarios", () => { - [ - ForceSetPasswordReason.AdminForcePasswordReset, - ForceSetPasswordReason.WeakMasterPassword, - ].forEach((forceResetPasswordReason) => { - const reasonString = ForceSetPasswordReason[forceResetPasswordReason]; - - beforeEach(() => { - // use standard user with MP because this test is not concerned with password reset. - selectedUserDecryptionOptions.next(mockUserDecryptionOpts.withMasterPassword); - - const authResult = new AuthResult(); - authResult.forcePasswordReset = forceResetPasswordReason; - mockLoginStrategyService.logInTwoFactor.mockResolvedValue(authResult); - }); - - testForceResetOnSuccessfulLogin(reasonString); - }); - }); - it("navigates to the component's defined success route (vault is default) when the login is successful", async () => { mockLoginStrategyService.logInTwoFactor.mockResolvedValue(new AuthResult()); @@ -407,29 +373,7 @@ describe("TwoFactorAuthComponent", () => { }); }); - describe("Given Trusted Device Encryption is enabled, user doesn't need to set a MP, and forcePasswordReset is required", () => { - [ - ForceSetPasswordReason.AdminForcePasswordReset, - ForceSetPasswordReason.WeakMasterPassword, - ].forEach((forceResetPasswordReason) => { - const reasonString = ForceSetPasswordReason[forceResetPasswordReason]; - - beforeEach(() => { - // use standard user with MP because this test is not concerned with password reset. - selectedUserDecryptionOptions.next( - mockUserDecryptionOpts.withMasterPasswordAndTrustedDevice, - ); - - const authResult = new AuthResult(); - authResult.forcePasswordReset = forceResetPasswordReason; - mockLoginStrategyService.logInTwoFactor.mockResolvedValue(authResult); - }); - - testForceResetOnSuccessfulLogin(reasonString); - }); - }); - - describe("Given Trusted Device Encryption is enabled, user doesn't need to set a MP, and forcePasswordReset is not required", () => { + describe("Given Trusted Device Encryption is enabled and user doesn't need to set a MP", () => { let authResult; beforeEach(() => { selectedUserDecryptionOptions.next( @@ -437,7 +381,6 @@ describe("TwoFactorAuthComponent", () => { ); authResult = new AuthResult(); - authResult.forcePasswordReset = ForceSetPasswordReason.None; mockLoginStrategyService.logInTwoFactor.mockResolvedValue(authResult); }); diff --git a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts index 296316198b0..c5e174484b0 100644 --- a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts +++ b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts @@ -396,11 +396,6 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy { ); } - // note: this flow affects both TDE & standard users - if (this.isForcePasswordResetRequired(authResult)) { - return await this.handleForcePasswordReset(this.orgSsoIdentifier); - } - const userDecryptionOpts = await firstValueFrom( this.userDecryptionOptionsService.userDecryptionOptions$, ); @@ -415,6 +410,7 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy { const requireSetPassword = !userDecryptionOpts.hasMasterPassword && userDecryptionOpts.keyConnectorOption === undefined; + // New users without a master password must set a master password before advancing. if (requireSetPassword || authResult.resetMasterPassword) { // Change implies going no password -> password in this case return await this.handleChangePasswordRequired(this.orgSsoIdentifier); @@ -524,14 +520,6 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy { return forceResetReasons.includes(authResult.forcePasswordReset); } - private async handleForcePasswordReset(orgIdentifier: string | undefined) { - await this.router.navigate(["update-temp-password"], { - queryParams: { - identifier: orgIdentifier, - }, - }); - } - showContinueButton() { return ( this.selectedProviderType != null && diff --git a/libs/auth/src/common/login-strategies/login.strategy.spec.ts b/libs/auth/src/common/login-strategies/login.strategy.spec.ts index b4a1e6a77d9..290345a90c7 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -306,6 +306,31 @@ describe("LoginStrategy", () => { expect(result).toEqual(expected); }); + it("processes a forcePasswordReset response properly", async () => { + const tokenResponse = identityTokenResponseFactory(); + tokenResponse.forcePasswordReset = true; + + apiService.postIdentityToken.mockResolvedValue(tokenResponse); + + const result = await passwordLoginStrategy.logIn(credentials); + + const expected = new AuthResult(); + expected.userId = userId; + expected.forcePasswordReset = ForceSetPasswordReason.AdminForcePasswordReset; + expected.resetMasterPassword = false; + expected.twoFactorProviders = {} as Partial< + Record> + >; + expected.captchaSiteKey = ""; + expected.twoFactorProviders = null; + expect(result).toEqual(expected); + + expect(masterPasswordService.mock.setForceSetPasswordReason).toHaveBeenCalledWith( + ForceSetPasswordReason.AdminForcePasswordReset, + userId, + ); + }); + it("rejects login if CAPTCHA is required", async () => { // Sample CAPTCHA response const tokenResponse = new IdentityCaptchaResponse({ diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index 89802c609c0..1d4c23d3bab 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -271,17 +271,24 @@ export abstract class LoginStrategy { } } - result.resetMasterPassword = response.resetMasterPassword; - - // Convert boolean to enum - if (response.forcePasswordReset) { - result.forcePasswordReset = ForceSetPasswordReason.AdminForcePasswordReset; - } - - // Must come before setting keys, user key needs email to update additional keys + // Must come before setting keys, user key needs email to update additional keys. const userId = await this.saveAccountInformation(response); result.userId = userId; + result.resetMasterPassword = response.resetMasterPassword; + + // Convert boolean to enum and set the state for the master password service to + // so we know when we reach the auth guard that we need to guide them properly to admin + // password reset. + if (response.forcePasswordReset) { + result.forcePasswordReset = ForceSetPasswordReason.AdminForcePasswordReset; + + await this.masterPasswordService.setForceSetPasswordReason( + ForceSetPasswordReason.AdminForcePasswordReset, + userId, + ); + } + if (response.twoFactorToken != null) { // note: we can read email from access token b/c it was saved in saveAccountInformation const userEmail = await this.tokenService.getEmail(); @@ -300,7 +307,9 @@ export abstract class LoginStrategy { // The keys comes from different sources depending on the login strategy protected abstract setMasterKey(response: IdentityTokenResponse, userId: UserId): Promise; + protected abstract setUserKey(response: IdentityTokenResponse, userId: UserId): Promise; + protected abstract setPrivateKey(response: IdentityTokenResponse, userId: UserId): Promise; // Old accounts used master key for encryption. We are forcing migrations but only need to diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.ts index ac00ff69a4d..f4eaa10c319 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.ts @@ -6,7 +6,6 @@ import { Jsonify } from "type-fest"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-connector.service"; import { AuthResult } from "@bitwarden/common/auth/models/domain/auth-result"; -import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { SsoTokenRequest } from "@bitwarden/common/auth/models/request/identity-token/sso-token.request"; import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response"; import { IdentityTokenResponse } from "@bitwarden/common/auth/models/response/identity-token.response"; @@ -108,14 +107,6 @@ export class SsoLoginStrategy extends LoginStrategy { const email = ssoAuthResult.email; const ssoEmail2FaSessionToken = ssoAuthResult.ssoEmail2FaSessionToken; - // Auth guard currently handles redirects for this. - if (ssoAuthResult.forcePasswordReset == ForceSetPasswordReason.AdminForcePasswordReset) { - await this.masterPasswordService.setForceSetPasswordReason( - ssoAuthResult.forcePasswordReset, - ssoAuthResult.userId, - ); - } - this.cache.next({ ...this.cache.value, email, From 7e6f2fa79803bfd8231c44b609133d4f31b5eb89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=85berg?= Date: Tue, 11 Mar 2025 09:03:28 +0100 Subject: [PATCH 13/23] Enable Basic Desktop Modal Support (#11484) Co-authored-by: Justin Baur <19896123+justindbaur@users.noreply.github.com> Co-authored-by: Colton Hurst Co-authored-by: Andreas Coroiu --- .github/CODEOWNERS | 1 + apps/desktop/src/app/app-routing.module.ts | 5 + .../components/fido2placeholder.component.ts | 36 ++++++ apps/desktop/src/main.ts | 2 + apps/desktop/src/main/tray.main.ts | 43 +++++++- apps/desktop/src/main/window.main.ts | 103 +++++++++++++++--- .../src/platform/popup-modal-styles.ts | 52 +++++++++ .../services/desktop-settings.service.ts | 24 ++++ 8 files changed, 248 insertions(+), 18 deletions(-) create mode 100644 apps/desktop/src/app/components/fido2placeholder.component.ts create mode 100644 apps/desktop/src/platform/popup-modal-styles.ts diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index d23cfa58283..5ba84c1f195 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -119,6 +119,7 @@ apps/browser/src/autofill @bitwarden/team-autofill-dev apps/desktop/src/autofill @bitwarden/team-autofill-dev libs/common/src/autofill @bitwarden/team-autofill-dev apps/desktop/macos/autofill-extension @bitwarden/team-autofill-dev +apps/desktop/src/app/components/fido2placeholder.component.ts @bitwarden/team-autofill-dev apps/desktop/desktop_native/windows-plugin-authenticator @bitwarden/team-autofill-dev # DuckDuckGo integration apps/desktop/native-messaging-test-runner @bitwarden/team-autofill-dev diff --git a/apps/desktop/src/app/app-routing.module.ts b/apps/desktop/src/app/app-routing.module.ts index 3a30629b444..36e267c9355 100644 --- a/apps/desktop/src/app/app-routing.module.ts +++ b/apps/desktop/src/app/app-routing.module.ts @@ -57,6 +57,7 @@ import { TwoFactorComponentV1 } from "../auth/two-factor-v1.component"; import { UpdateTempPasswordComponent } from "../auth/update-temp-password.component"; import { VaultComponent } from "../vault/app/vault/vault.component"; +import { Fido2PlaceholderComponent } from "./components/fido2placeholder.component"; import { SendComponent } from "./tools/send/send.component"; /** @@ -177,6 +178,10 @@ const routes: Routes = [ component: RemovePasswordComponent, canActivate: [authGuard], }, + { + path: "passkeys", + component: Fido2PlaceholderComponent, + }, { path: "", component: AnonLayoutWrapperComponent, diff --git a/apps/desktop/src/app/components/fido2placeholder.component.ts b/apps/desktop/src/app/components/fido2placeholder.component.ts new file mode 100644 index 00000000000..b3302d63241 --- /dev/null +++ b/apps/desktop/src/app/components/fido2placeholder.component.ts @@ -0,0 +1,36 @@ +import { Component } from "@angular/core"; +import { Router } from "@angular/router"; + +import { DesktopSettingsService } from "../../platform/services/desktop-settings.service"; + +@Component({ + standalone: true, + template: ` +
+

Select your passkey

+
+ +
+ `, +}) +export class Fido2PlaceholderComponent { + constructor( + private readonly desktopSettingsService: DesktopSettingsService, + private readonly router: Router, + ) {} + + async closeModal() { + await this.router.navigate(["/"]); + await this.desktopSettingsService.setInModalMode(false); + } +} diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index 4e167f30ec8..e4894b159fe 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -284,6 +284,8 @@ export class Main { this.migrationRunner.run().then( async () => { await this.toggleHardwareAcceleration(); + // Reset modal mode to make sure main window is displayed correctly + await this.desktopSettingsService.resetInModalMode(); await this.windowMain.init(); await this.i18nService.init(); await this.messagingMain.init(); diff --git a/apps/desktop/src/main/tray.main.ts b/apps/desktop/src/main/tray.main.ts index 9fa7fe6143f..e63e2a00c85 100644 --- a/apps/desktop/src/main/tray.main.ts +++ b/apps/desktop/src/main/tray.main.ts @@ -1,6 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import * as path from "path"; +import * as url from "url"; import { app, BrowserWindow, Menu, MenuItemConstructorOptions, nativeImage, Tray } from "electron"; import { firstValueFrom } from "rxjs"; @@ -9,6 +10,7 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { BiometricStateService, BiometricsService } from "@bitwarden/key-management"; import { DesktopSettingsService } from "../platform/services/desktop-settings.service"; +import { cleanUserAgent, isDev } from "../utils"; import { WindowMain } from "./window.main"; @@ -49,6 +51,11 @@ export class TrayMain { label: this.i18nService.t("showHide"), click: () => this.toggleWindow(), }, + { + visible: isDev(), + label: "Fake Popup", + click: () => this.fakePopup(), + }, { type: "separator" }, { label: this.i18nService.t("exit"), @@ -190,7 +197,7 @@ export class TrayMain { this.hideDock(); } } else { - this.windowMain.win.show(); + this.windowMain.show(); if (this.isDarwin()) { this.showDock(); } @@ -203,4 +210,38 @@ export class TrayMain { this.windowMain.win.close(); } } + + /** + * This method is used to test modal behavior during development and could be removed in the future. + * @returns + */ + private async fakePopup() { + if (this.windowMain.win == null || this.windowMain.win.isDestroyed()) { + await this.windowMain.createWindow("modal-app"); + return; + } + + // Restyle existing + const existingWin = this.windowMain.win; + + await this.desktopSettingsService.setInModalMode(true); + await existingWin.loadURL( + url.format({ + protocol: "file:", + //pathname: `${__dirname}/index.html`, + pathname: path.join(__dirname, "/index.html"), + slashes: true, + hash: "/passkeys", + query: { + redirectUrl: "/passkeys", + }, + }), + { + userAgent: cleanUserAgent(existingWin.webContents.userAgent), + }, + ); + existingWin.once("ready-to-show", () => { + existingWin.show(); + }); + } } diff --git a/apps/desktop/src/main/window.main.ts b/apps/desktop/src/main/window.main.ts index 17f74b78d4c..ca154400ff5 100644 --- a/apps/desktop/src/main/window.main.ts +++ b/apps/desktop/src/main/window.main.ts @@ -5,7 +5,7 @@ import * as path from "path"; import * as url from "url"; import { app, BrowserWindow, ipcMain, nativeTheme, screen, session } from "electron"; -import { firstValueFrom } from "rxjs"; +import { concatMap, firstValueFrom, pairwise } from "rxjs"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service"; @@ -14,6 +14,7 @@ import { processisolations } from "@bitwarden/desktop-napi"; import { BiometricStateService } from "@bitwarden/key-management"; import { WindowState } from "../platform/models/domain/window-state"; +import { applyMainWindowStyles, applyPopupModalStyles } from "../platform/popup-modal-styles"; import { DesktopSettingsService } from "../platform/services/desktop-settings.service"; import { cleanUserAgent, isDev, isLinux, isMac, isMacAppStore, isWindows } from "../utils"; @@ -77,6 +78,24 @@ export class WindowMain { } }); + this.desktopSettingsService.inModalMode$ + .pipe( + pairwise(), + concatMap(async ([lastValue, newValue]) => { + if (lastValue && !newValue) { + // Reset the window state to the main window state + applyMainWindowStyles(this.win, this.windowStates[mainWindowSizeKey]); + // Because modal is used in front of another app, UX wise it makes sense to hide the main window when leaving modal mode. + this.win.hide(); + } else if (!lastValue && newValue) { + // Apply the popup modal styles + applyPopupModalStyles(this.win); + this.win.show(); + } + }), + ) + .subscribe(); + this.desktopSettingsService.preventScreenshots$.subscribe((prevent) => { if (this.win == null) { return; @@ -182,7 +201,20 @@ export class WindowMain { }); } - async createWindow(): Promise { + /// Show the window with main window styles + show() { + if (this.win != null) { + applyMainWindowStyles(this.win, this.windowStates[mainWindowSizeKey]); + this.win.show(); + } + } + + /** + * Creates the main window. The template argument is used to determine the styling of the window and what url will be loaded. + * When the template is "modal-app", the window will be styled as a modal and the passkeys page will be loaded. + * TODO: We might want to refactor the template argument to accomodate more target pages, e.g. ssh-agent. + */ + async createWindow(template: "full-app" | "modal-app" = "full-app"): Promise { this.windowStates[mainWindowSizeKey] = await this.getWindowState( this.defaultWidth, this.defaultHeight, @@ -216,6 +248,12 @@ export class WindowMain { }, }); + if (template === "modal-app") { + applyPopupModalStyles(this.win); + } else { + applyMainWindowStyles(this.win, this.windowStates[mainWindowSizeKey]); + } + this.win.webContents.on("dom-ready", () => { this.win.webContents.zoomFactor = this.windowStates[mainWindowSizeKey].zoomFactor ?? 1.0; }); @@ -225,21 +263,41 @@ export class WindowMain { } // Show it later since it might need to be maximized. - this.win.show(); + // use once event to avoid flash on unstyled content. + this.win.once("ready-to-show", () => { + this.win.show(); + }); - // and load the index.html of the app. - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.win.loadURL( - url.format({ - protocol: "file:", - pathname: path.join(__dirname, "/index.html"), - slashes: true, - }), - { - userAgent: cleanUserAgent(this.win.webContents.userAgent), - }, - ); + if (template === "full-app") { + // and load the index.html of the app. + // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. + void this.win.loadURL( + url.format({ + protocol: "file:", + pathname: path.join(__dirname, "/index.html"), + slashes: true, + }), + { + userAgent: cleanUserAgent(this.win.webContents.userAgent), + }, + ); + } else { + // we're in modal mode - load the passkeys page + await this.win.loadURL( + url.format({ + protocol: "file:", + pathname: path.join(__dirname, "/index.html"), + slashes: true, + hash: "/passkeys", + query: { + redirectUrl: "/passkeys", + }, + }), + { + userAgent: cleanUserAgent(this.win.webContents.userAgent), + }, + ); + } // Open the DevTools. if (isDev()) { @@ -336,6 +394,12 @@ export class WindowMain { return; } + const inModalMode = await firstValueFrom(this.desktopSettingsService.inModalMode$); + + if (inModalMode) { + return; + } + try { const bounds = win.getBounds(); @@ -346,9 +410,14 @@ export class WindowMain { } } - this.windowStates[configKey].isMaximized = win.isMaximized(); + // We treat fullscreen as maximized (would be even better to store isFullscreen as its own flag). + this.windowStates[configKey].isMaximized = win.isMaximized() || win.isFullScreen(); this.windowStates[configKey].displayBounds = screen.getDisplayMatching(bounds).bounds; + // Maybe store these as well? + // win.isFocused(); + // win.isVisible(); + if (!win.isMaximized() && !win.isMinimized() && !win.isFullScreen()) { this.windowStates[configKey].x = bounds.x; this.windowStates[configKey].y = bounds.y; diff --git a/apps/desktop/src/platform/popup-modal-styles.ts b/apps/desktop/src/platform/popup-modal-styles.ts new file mode 100644 index 00000000000..9c3f06b34bf --- /dev/null +++ b/apps/desktop/src/platform/popup-modal-styles.ts @@ -0,0 +1,52 @@ +import { BrowserWindow } from "electron"; + +import { WindowState } from "./models/domain/window-state"; + +// change as needed, however limited by mainwindow minimum size +const popupWidth = 680; +const popupHeight = 500; + +export function applyPopupModalStyles(window: BrowserWindow) { + window.unmaximize(); + window.setSize(popupWidth, popupHeight); + window.center(); + window.setWindowButtonVisibility?.(false); + window.setMenuBarVisibility?.(false); + window.setResizable(false); + window.setAlwaysOnTop(true); + + // Adjusting from full screen is a bit more hassle + if (window.isFullScreen()) { + window.setFullScreen(false); + window.once("leave-full-screen", () => { + window.setSize(popupWidth, popupHeight); + window.center(); + }); + } +} + +export function applyMainWindowStyles(window: BrowserWindow, existingWindowState: WindowState) { + window.setMinimumSize(680, 500); + + // need to guard against null/undefined values + + if (existingWindowState?.width && existingWindowState?.height) { + window.setSize(existingWindowState.width, existingWindowState.height); + } + + if (existingWindowState?.x && existingWindowState?.y) { + window.setPosition(existingWindowState.x, existingWindowState.y); + } + + window.setWindowButtonVisibility?.(true); + window.setMenuBarVisibility?.(true); + window.setResizable(true); + window.setAlwaysOnTop(false); + + // We're currently not recovering the maximized state, mostly due to conflicts with hiding the window. + // window.setFullScreen(existingWindowState.isMaximized); + + // if (existingWindowState.isMaximized) { + // window.maximize(); + // } +} diff --git a/apps/desktop/src/platform/services/desktop-settings.service.ts b/apps/desktop/src/platform/services/desktop-settings.service.ts index f0d5d124de2..efac0cda252 100644 --- a/apps/desktop/src/platform/services/desktop-settings.service.ts +++ b/apps/desktop/src/platform/services/desktop-settings.service.ts @@ -75,6 +75,10 @@ const MINIMIZE_ON_COPY = new UserKeyDefinition(DESKTOP_SETTINGS_DISK, " clearOn: [], // User setting, no need to clear }); +const IN_MODAL_MODE = new KeyDefinition(DESKTOP_SETTINGS_DISK, "inModalMode", { + deserializer: (b) => b, +}); + const PREVENT_SCREENSHOTS = new KeyDefinition( DESKTOP_SETTINGS_DISK, "preventScreenshots", @@ -170,6 +174,10 @@ export class DesktopSettingsService { */ minimizeOnCopy$ = this.minimizeOnCopyState.state$.pipe(map(Boolean)); + private readonly inModalModeState = this.stateProvider.getGlobal(IN_MODAL_MODE); + + inModalMode$ = this.inModalModeState.state$.pipe(map(Boolean)); + constructor(private stateProvider: StateProvider) { this.window$ = this.windowState.state$.pipe( map((window) => @@ -178,6 +186,14 @@ export class DesktopSettingsService { ); } + /** + * This is used to clear the setting on application start to make sure we don't end up + * stuck in modal mode if the application is force-closed in modal mode. + */ + async resetInModalMode() { + await this.inModalModeState.update(() => false); + } + async setHardwareAcceleration(enabled: boolean) { await this.hwState.update(() => enabled); } @@ -286,6 +302,14 @@ export class DesktopSettingsService { await this.stateProvider.getUser(userId, MINIMIZE_ON_COPY).update(() => value); } + /** + * Sets the modal mode of the application. Setting this changes the windows-size and other properties. + * @param value `true` if the application is in modal mode, `false` if it is not. + */ + async setInModalMode(value: boolean) { + await this.inModalModeState.update(() => value); + } + /** * Sets the setting for whether or not the screenshot protection is enabled. * @param value `true` if the screenshot protection is enabled, `false` if it is not. From 5cd47ac90717a40346c057487a4e764f73a8fe7b Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com> Date: Tue, 11 Mar 2025 14:06:44 +0100 Subject: [PATCH 14/23] [PM-18243] Improve type safety in decryption (#12885) * Improve decrypt failure logging * Rename decryptcontext to decrypttrace * Improve docs * PM-16984: Improving type safety of decryption * Improving type safety of decryption --------- Co-authored-by: Bernd Schoolmann --- .../common/collections/models/collection.ts | 7 +-- .../src/platform/models/domain/domain-base.ts | 57 ++++++++++--------- .../src/platform/models/domain/enc-string.ts | 2 +- .../tools/send/models/domain/send-access.ts | 9 +-- .../src/tools/send/models/domain/send-file.ts | 8 +-- .../src/tools/send/models/domain/send-text.ts | 7 +-- .../src/tools/send/models/domain/send.ts | 10 +--- .../src/vault/models/domain/attachment.ts | 7 +-- libs/common/src/vault/models/domain/card.ts | 12 +--- libs/common/src/vault/models/domain/cipher.ts | 8 +-- .../vault/models/domain/fido2-credential.ts | 47 +++++++-------- libs/common/src/vault/models/domain/field.ts | 8 +-- libs/common/src/vault/models/domain/folder.ts | 8 +-- .../src/vault/models/domain/identity.ts | 43 +++++++------- .../src/vault/models/domain/login-uri.ts | 7 +-- libs/common/src/vault/models/domain/login.ts | 9 +-- .../src/vault/models/domain/password.ts | 7 +-- .../common/src/vault/models/domain/ssh-key.ts | 9 +-- 18 files changed, 111 insertions(+), 154 deletions(-) diff --git a/libs/admin-console/src/common/collections/models/collection.ts b/libs/admin-console/src/common/collections/models/collection.ts index f14ccb20141..5b6f1a6fb7a 100644 --- a/libs/admin-console/src/common/collections/models/collection.ts +++ b/libs/admin-console/src/common/collections/models/collection.ts @@ -39,11 +39,10 @@ export class Collection extends Domain { } decrypt(orgKey: OrgKey): Promise { - return this.decryptObj( + return this.decryptObj( + this, new CollectionView(this), - { - name: null, - }, + ["name"], this.organizationId, orgKey, ); diff --git a/libs/common/src/platform/models/domain/domain-base.ts b/libs/common/src/platform/models/domain/domain-base.ts index 5aa79946653..11d0193accc 100644 --- a/libs/common/src/platform/models/domain/domain-base.ts +++ b/libs/common/src/platform/models/domain/domain-base.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { ConditionalExcept, ConditionalKeys, Constructor } from "type-fest"; import { EncryptService } from "../../../key-management/crypto/abstractions/encrypt.service"; @@ -15,6 +13,19 @@ export type DecryptedObject< TDecryptedKeys extends EncStringKeys, > = Record & Omit; +// extracts shared keys from the domain and view types +type EncryptableKeys = (keyof D & + ConditionalKeys) & + (keyof V & ConditionalKeys); + +type DomainEncryptableKeys = { + [key in ConditionalKeys]: EncString | null; +}; + +type ViewEncryptableKeys = { + [key in ConditionalKeys]: string | null; +}; + // https://contributing.bitwarden.com/architecture/clients/data-model#domain export default class Domain { protected buildDomainModel( @@ -37,6 +48,7 @@ export default class Domain { } } } + protected buildDataModel( domain: D, dataObj: any, @@ -58,31 +70,24 @@ export default class Domain { } } - protected async decryptObj( - viewModel: T, - map: any, - orgId: string, - key: SymmetricCryptoKey = null, + protected async decryptObj( + domain: DomainEncryptableKeys, + viewModel: ViewEncryptableKeys, + props: EncryptableKeys[], + orgId: string | null, + key: SymmetricCryptoKey | null = null, objectContext: string = "No Domain Context", - ): Promise { - const self: any = this; - - for (const prop in map) { - // eslint-disable-next-line - if (!map.hasOwnProperty(prop)) { - continue; - } - - const mapProp = map[prop] || prop; - if (self[mapProp]) { - (viewModel as any)[prop] = await self[mapProp].decrypt( + ): Promise { + for (const prop of props) { + viewModel[prop] = + (await domain[prop]?.decrypt( orgId, key, - `Property: ${prop}; ObjectContext: ${objectContext}`, - ); - } + `Property: ${prop as string}; ObjectContext: ${objectContext}`, + )) ?? null; } - return viewModel; + + return viewModel as V; } /** @@ -111,7 +116,7 @@ export default class Domain { const decryptedObjects = []; for (const prop of encryptedProperties) { - const value = (this as any)[prop] as EncString; + const value = this[prop] as EncString; const decrypted = await this.decryptProperty( prop, value, @@ -138,11 +143,9 @@ export default class Domain { encryptService: EncryptService, decryptTrace: string, ) { - let decrypted: string = null; + let decrypted: string | null = null; if (value) { decrypted = await value.decryptWithKey(key, encryptService, decryptTrace); - } else { - decrypted = null; } return { [propertyKey]: decrypted, diff --git a/libs/common/src/platform/models/domain/enc-string.ts b/libs/common/src/platform/models/domain/enc-string.ts index 360cb9bab46..4ea58a6809e 100644 --- a/libs/common/src/platform/models/domain/enc-string.ts +++ b/libs/common/src/platform/models/domain/enc-string.ts @@ -160,7 +160,7 @@ export class EncString implements Encrypted { async decrypt( orgId: string | null, - key: SymmetricCryptoKey = null, + key: SymmetricCryptoKey | null = null, context?: string, ): Promise { if (this.decryptedValue != null) { diff --git a/libs/common/src/tools/send/models/domain/send-access.ts b/libs/common/src/tools/send/models/domain/send-access.ts index dcc2d3ef426..588c4e84aa1 100644 --- a/libs/common/src/tools/send/models/domain/send-access.ts +++ b/libs/common/src/tools/send/models/domain/send-access.ts @@ -54,14 +54,7 @@ export class SendAccess extends Domain { async decrypt(key: SymmetricCryptoKey): Promise { const model = new SendAccessView(this); - await this.decryptObj( - model, - { - name: null, - }, - null, - key, - ); + await this.decryptObj(this, model, ["name"], null, key); switch (this.type) { case SendType.File: diff --git a/libs/common/src/tools/send/models/domain/send-file.ts b/libs/common/src/tools/send/models/domain/send-file.ts index 90e40f3959a..b8d0a265081 100644 --- a/libs/common/src/tools/send/models/domain/send-file.ts +++ b/libs/common/src/tools/send/models/domain/send-file.ts @@ -34,15 +34,13 @@ export class SendFile extends Domain { } async decrypt(key: SymmetricCryptoKey): Promise { - const view = await this.decryptObj( + return await this.decryptObj( + this, new SendFileView(this), - { - fileName: null, - }, + ["fileName"], null, key, ); - return view; } static fromJSON(obj: Jsonify) { diff --git a/libs/common/src/tools/send/models/domain/send-text.ts b/libs/common/src/tools/send/models/domain/send-text.ts index b17e3f769fb..df33e555896 100644 --- a/libs/common/src/tools/send/models/domain/send-text.ts +++ b/libs/common/src/tools/send/models/domain/send-text.ts @@ -30,11 +30,10 @@ export class SendText extends Domain { } decrypt(key: SymmetricCryptoKey): Promise { - return this.decryptObj( + return this.decryptObj( + this, new SendTextView(this), - { - text: null, - }, + ["text"], null, key, ); diff --git a/libs/common/src/tools/send/models/domain/send.ts b/libs/common/src/tools/send/models/domain/send.ts index c2390d439e7..f12a0010fab 100644 --- a/libs/common/src/tools/send/models/domain/send.ts +++ b/libs/common/src/tools/send/models/domain/send.ts @@ -87,15 +87,7 @@ export class Send extends Domain { // TODO: error? } - await this.decryptObj( - model, - { - name: null, - notes: null, - }, - null, - model.cryptoKey, - ); + await this.decryptObj(this, model, ["name", "notes"], null, model.cryptoKey); switch (this.type) { case SendType.File: diff --git a/libs/common/src/vault/models/domain/attachment.ts b/libs/common/src/vault/models/domain/attachment.ts index 4eee0307746..16f3adbe307 100644 --- a/libs/common/src/vault/models/domain/attachment.ts +++ b/libs/common/src/vault/models/domain/attachment.ts @@ -43,11 +43,10 @@ export class Attachment extends Domain { context = "No Cipher Context", encKey?: SymmetricCryptoKey, ): Promise { - const view = await this.decryptObj( + const view = await this.decryptObj( + this, new AttachmentView(this), - { - fileName: null, - }, + ["fileName"], orgId, encKey, "DomainType: Attachment; " + context, diff --git a/libs/common/src/vault/models/domain/card.ts b/libs/common/src/vault/models/domain/card.ts index fccfe3f595b..3d73a8f527c 100644 --- a/libs/common/src/vault/models/domain/card.ts +++ b/libs/common/src/vault/models/domain/card.ts @@ -42,16 +42,10 @@ export class Card extends Domain { context = "No Cipher Context", encKey?: SymmetricCryptoKey, ): Promise { - return this.decryptObj( + return this.decryptObj( + this, new CardView(), - { - cardholderName: null, - brand: null, - number: null, - expMonth: null, - expYear: null, - code: null, - }, + ["cardholderName", "brand", "number", "expMonth", "expYear", "code"], orgId, encKey, "DomainType: Card; " + context, diff --git a/libs/common/src/vault/models/domain/cipher.ts b/libs/common/src/vault/models/domain/cipher.ts index 21538b87788..c08ec8a4ebc 100644 --- a/libs/common/src/vault/models/domain/cipher.ts +++ b/libs/common/src/vault/models/domain/cipher.ts @@ -154,12 +154,10 @@ export class Cipher extends Domain implements Decryptable { bypassValidation = false; } - await this.decryptObj( + await this.decryptObj( + this, model, - { - name: null, - notes: null, - }, + ["name", "notes"], this.organizationId, encKey, ); diff --git a/libs/common/src/vault/models/domain/fido2-credential.ts b/libs/common/src/vault/models/domain/fido2-credential.ts index 9aa2c753d7c..8b0082892e4 100644 --- a/libs/common/src/vault/models/domain/fido2-credential.ts +++ b/libs/common/src/vault/models/domain/fido2-credential.ts @@ -52,41 +52,38 @@ export class Fido2Credential extends Domain { } async decrypt(orgId: string, encKey?: SymmetricCryptoKey): Promise { - const view = await this.decryptObj( + const view = await this.decryptObj( + this, new Fido2CredentialView(), - { - credentialId: null, - keyType: null, - keyAlgorithm: null, - keyCurve: null, - keyValue: null, - rpId: null, - userHandle: null, - userName: null, - rpName: null, - userDisplayName: null, - discoverable: null, - }, + [ + "credentialId", + "keyType", + "keyAlgorithm", + "keyCurve", + "keyValue", + "rpId", + "userHandle", + "userName", + "rpName", + "userDisplayName", + ], orgId, encKey, ); - const { counter } = await this.decryptObj( - { counter: "" }, + const { counter } = await this.decryptObj< + Fido2Credential, { - counter: null, - }, - orgId, - encKey, - ); + counter: string; + } + >(this, { counter: "" }, ["counter"], orgId, encKey); // Counter will end up as NaN if this fails view.counter = parseInt(counter); - const { discoverable } = await this.decryptObj( + const { discoverable } = await this.decryptObj( + this, { discoverable: "" }, - { - discoverable: null, - }, + ["discoverable"], orgId, encKey, ); diff --git a/libs/common/src/vault/models/domain/field.ts b/libs/common/src/vault/models/domain/field.ts index f836184da6a..c0f08a38bcc 100644 --- a/libs/common/src/vault/models/domain/field.ts +++ b/libs/common/src/vault/models/domain/field.ts @@ -35,12 +35,10 @@ export class Field extends Domain { } decrypt(orgId: string, encKey?: SymmetricCryptoKey): Promise { - return this.decryptObj( + return this.decryptObj( + this, new FieldView(this), - { - name: null, - value: null, - }, + ["name", "value"], orgId, encKey, ); diff --git a/libs/common/src/vault/models/domain/folder.ts b/libs/common/src/vault/models/domain/folder.ts index 65018e3cf08..8749a92fb65 100644 --- a/libs/common/src/vault/models/domain/folder.ts +++ b/libs/common/src/vault/models/domain/folder.ts @@ -40,13 +40,7 @@ export class Folder extends Domain { } decrypt(): Promise { - return this.decryptObj( - new FolderView(this), - { - name: null, - }, - null, - ); + return this.decryptObj(this, new FolderView(this), ["name"], null); } async decryptWithKey( diff --git a/libs/common/src/vault/models/domain/identity.ts b/libs/common/src/vault/models/domain/identity.ts index 570e6c0b4d5..5d8c20ef2b3 100644 --- a/libs/common/src/vault/models/domain/identity.ts +++ b/libs/common/src/vault/models/domain/identity.ts @@ -66,28 +66,29 @@ export class Identity extends Domain { context: string = "No Cipher Context", encKey?: SymmetricCryptoKey, ): Promise { - return this.decryptObj( + return this.decryptObj( + this, new IdentityView(), - { - title: null, - firstName: null, - middleName: null, - lastName: null, - address1: null, - address2: null, - address3: null, - city: null, - state: null, - postalCode: null, - country: null, - company: null, - email: null, - phone: null, - ssn: null, - username: null, - passportNumber: null, - licenseNumber: null, - }, + [ + "title", + "firstName", + "middleName", + "lastName", + "address1", + "address2", + "address3", + "city", + "state", + "postalCode", + "country", + "company", + "email", + "phone", + "ssn", + "username", + "passportNumber", + "licenseNumber", + ], orgId, encKey, "DomainType: Identity; " + context, diff --git a/libs/common/src/vault/models/domain/login-uri.ts b/libs/common/src/vault/models/domain/login-uri.ts index 36782a81502..883f8c9a616 100644 --- a/libs/common/src/vault/models/domain/login-uri.ts +++ b/libs/common/src/vault/models/domain/login-uri.ts @@ -38,11 +38,10 @@ export class LoginUri extends Domain { context: string = "No Cipher Context", encKey?: SymmetricCryptoKey, ): Promise { - return this.decryptObj( + return this.decryptObj( + this, new LoginUriView(this), - { - uri: null, - }, + ["uri"], orgId, encKey, context, diff --git a/libs/common/src/vault/models/domain/login.ts b/libs/common/src/vault/models/domain/login.ts index f9a85cd818e..b29b42bf3de 100644 --- a/libs/common/src/vault/models/domain/login.ts +++ b/libs/common/src/vault/models/domain/login.ts @@ -58,13 +58,10 @@ export class Login extends Domain { context: string = "No Cipher Context", encKey?: SymmetricCryptoKey, ): Promise { - const view = await this.decryptObj( + const view = await this.decryptObj( + this, new LoginView(this), - { - username: null, - password: null, - totp: null, - }, + ["username", "password", "totp"], orgId, encKey, `DomainType: Login; ${context}`, diff --git a/libs/common/src/vault/models/domain/password.ts b/libs/common/src/vault/models/domain/password.ts index 48063f495f0..8573c224416 100644 --- a/libs/common/src/vault/models/domain/password.ts +++ b/libs/common/src/vault/models/domain/password.ts @@ -25,11 +25,10 @@ export class Password extends Domain { } decrypt(orgId: string, encKey?: SymmetricCryptoKey): Promise { - return this.decryptObj( + return this.decryptObj( + this, new PasswordHistoryView(this), - { - password: null, - }, + ["password"], orgId, encKey, "DomainType: PasswordHistory", diff --git a/libs/common/src/vault/models/domain/ssh-key.ts b/libs/common/src/vault/models/domain/ssh-key.ts index b4df172e543..f32a1a913ca 100644 --- a/libs/common/src/vault/models/domain/ssh-key.ts +++ b/libs/common/src/vault/models/domain/ssh-key.ts @@ -36,13 +36,10 @@ export class SshKey extends Domain { context = "No Cipher Context", encKey?: SymmetricCryptoKey, ): Promise { - return this.decryptObj( + return this.decryptObj( + this, new SshKeyView(), - { - privateKey: null, - publicKey: null, - keyFingerprint: null, - }, + ["privateKey", "publicKey", "keyFingerprint"], orgId, encKey, "DomainType: SshKey; " + context, From 9683779dbf6f3ffde6860286f96c92abe63b9bd2 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Tue, 11 Mar 2025 14:20:02 +0100 Subject: [PATCH 15/23] [PM-17984] Remove AES128CBC-HMAC encryption (#13304) * Remove AES128CBC-HMAC encryption * Increase test coverage --- .../crypto/abstractions/encrypt.service.ts | 1 - .../encrypt.service.implementation.ts | 19 ------ .../crypto/services/encrypt.service.spec.ts | 62 +++++++++---------- .../platform/enums/encryption-type.enum.ts | 7 +-- .../models/domain/enc-array-buffer.spec.ts | 39 ++++++------ .../models/domain/enc-array-buffer.ts | 1 - .../platform/models/domain/enc-string.spec.ts | 2 - .../src/platform/models/domain/enc-string.ts | 6 +- .../domain/symmetric-crypto-key.spec.ts | 15 ----- .../models/domain/symmetric-crypto-key.ts | 3 - 10 files changed, 50 insertions(+), 105 deletions(-) diff --git a/libs/common/src/key-management/crypto/abstractions/encrypt.service.ts b/libs/common/src/key-management/crypto/abstractions/encrypt.service.ts index 484327bcd27..f7f064f5251 100644 --- a/libs/common/src/key-management/crypto/abstractions/encrypt.service.ts +++ b/libs/common/src/key-management/crypto/abstractions/encrypt.service.ts @@ -36,7 +36,6 @@ export abstract class EncryptService { ): Promise; abstract rsaEncrypt(data: Uint8Array, publicKey: Uint8Array): Promise; abstract rsaDecrypt(data: EncString, privateKey: Uint8Array): Promise; - abstract resolveLegacyKey(key: SymmetricCryptoKey, encThing: Encrypted): SymmetricCryptoKey; /** * @deprecated Replaced by BulkEncryptService, remove once the feature is tested and the featureflag PM-4154-multi-worker-encryption-service is removed * @param items The items to decrypt diff --git a/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts b/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts index 8a001886837..d426340c277 100644 --- a/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts +++ b/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts @@ -78,8 +78,6 @@ export class EncryptServiceImplementation implements EncryptService { throw new Error("No key provided for decryption."); } - key = this.resolveLegacyKey(key, encString); - // DO NOT REMOVE OR MOVE. This prevents downgrade to mac-less CBC, which would compromise integrity and confidentiality. if (key.macKey != null && encString?.mac == null) { this.logService.error( @@ -145,8 +143,6 @@ export class EncryptServiceImplementation implements EncryptService { throw new Error("Nothing provided for decryption."); } - key = this.resolveLegacyKey(key, encThing); - // DO NOT REMOVE OR MOVE. This prevents downgrade to mac-less CBC, which would compromise integrity and confidentiality. if (key.macKey != null && encThing.macBytes == null) { this.logService.error( @@ -298,19 +294,4 @@ export class EncryptServiceImplementation implements EncryptService { this.logService.error(msg); } } - - /** - * Transform into new key for the old encrypt-then-mac scheme if required, otherwise return the current key unchanged - * @param encThing The encrypted object (e.g. encString or encArrayBuffer) that you want to decrypt - */ - resolveLegacyKey(key: SymmetricCryptoKey, encThing: Encrypted): SymmetricCryptoKey { - if ( - encThing.encryptionType === EncryptionType.AesCbc128_HmacSha256_B64 && - key.encType === EncryptionType.AesCbc256_B64 - ) { - return new SymmetricCryptoKey(key.key, EncryptionType.AesCbc128_HmacSha256_B64); - } - - return key; - } } diff --git a/libs/common/src/key-management/crypto/services/encrypt.service.spec.ts b/libs/common/src/key-management/crypto/services/encrypt.service.spec.ts index cff695f4829..3ce0d5883d2 100644 --- a/libs/common/src/key-management/crypto/services/encrypt.service.spec.ts +++ b/libs/common/src/key-management/crypto/services/encrypt.service.spec.ts @@ -325,6 +325,25 @@ describe("EncryptService", () => { }); }); + describe("decryptToUtf8", () => { + it("throws if no key is provided", () => { + return expect(encryptService.decryptToUtf8(null, null)).rejects.toThrow( + "No key provided for decryption.", + ); + }); + it("returns null if key is mac key but encstring has no mac", async () => { + const key = new SymmetricCryptoKey( + makeStaticByteArray(64, 0), + EncryptionType.AesCbc256_HmacSha256_B64, + ); + const encString = new EncString(EncryptionType.AesCbc256_B64, "data"); + + const actual = await encryptService.decryptToUtf8(encString, key); + expect(actual).toBeNull(); + expect(logService.error).toHaveBeenCalled(); + }); + }); + describe("rsa", () => { const data = makeStaticByteArray(10, 100); const encryptedData = makeStaticByteArray(10, 150); @@ -370,17 +389,16 @@ describe("EncryptService", () => { return expect(encryptService.rsaDecrypt(encString, null)).rejects.toThrow("No private key"); }); - it.each([ - EncryptionType.AesCbc256_B64, - EncryptionType.AesCbc128_HmacSha256_B64, - EncryptionType.AesCbc256_HmacSha256_B64, - ])("throws if encryption type is %s", async (encType) => { - encString.encryptionType = encType; + it.each([EncryptionType.AesCbc256_B64, EncryptionType.AesCbc256_HmacSha256_B64])( + "throws if encryption type is %s", + async (encType) => { + encString.encryptionType = encType; - await expect(encryptService.rsaDecrypt(encString, privateKey)).rejects.toThrow( - "Invalid encryption type", - ); - }); + await expect(encryptService.rsaDecrypt(encString, privateKey)).rejects.toThrow( + "Invalid encryption type", + ); + }, + ); it("decrypts data with provided key", async () => { cryptoFunctionService.rsaDecrypt.mockResolvedValue(data); @@ -398,30 +416,6 @@ describe("EncryptService", () => { }); }); - describe("resolveLegacyKey", () => { - it("creates a legacy key if required", async () => { - const key = new SymmetricCryptoKey(makeStaticByteArray(32), EncryptionType.AesCbc256_B64); - const encString = mock(); - encString.encryptionType = EncryptionType.AesCbc128_HmacSha256_B64; - - const actual = encryptService.resolveLegacyKey(key, encString); - - const expected = new SymmetricCryptoKey(key.key, EncryptionType.AesCbc128_HmacSha256_B64); - expect(actual).toEqual(expected); - }); - - it("does not create a legacy key if not required", async () => { - const encType = EncryptionType.AesCbc256_HmacSha256_B64; - const key = new SymmetricCryptoKey(makeStaticByteArray(64), encType); - const encString = mock(); - encString.encryptionType = encType; - - const actual = encryptService.resolveLegacyKey(key, encString); - - expect(actual).toEqual(key); - }); - }); - describe("hash", () => { it("hashes a string and returns b64", async () => { cryptoFunctionService.hash.mockResolvedValue(Uint8Array.from([1, 2, 3])); diff --git a/libs/common/src/platform/enums/encryption-type.enum.ts b/libs/common/src/platform/enums/encryption-type.enum.ts index a0ffe679279..fd484dc2fdf 100644 --- a/libs/common/src/platform/enums/encryption-type.enum.ts +++ b/libs/common/src/platform/enums/encryption-type.enum.ts @@ -1,6 +1,6 @@ export enum EncryptionType { AesCbc256_B64 = 0, - AesCbc128_HmacSha256_B64 = 1, + // Type 1 was the unused and removed AesCbc128_HmacSha256_B64 AesCbc256_HmacSha256_B64 = 2, Rsa2048_OaepSha256_B64 = 3, Rsa2048_OaepSha1_B64 = 4, @@ -17,12 +17,10 @@ export function encryptionTypeToString(encryptionType: EncryptionType): string { } /** The expected number of parts to a serialized EncString of the given encryption type. - * For example, an EncString of type AesCbc256_B64 will have 2 parts, and an EncString of type - * AesCbc128_HmacSha256_B64 will have 3 parts. + * For example, an EncString of type AesCbc256_B64 will have 2 parts * * Example of annotated serialized EncStrings: * 0.iv|data - * 1.iv|data|mac * 2.iv|data|mac * 3.data * 4.data @@ -33,7 +31,6 @@ export function encryptionTypeToString(encryptionType: EncryptionType): string { */ export const EXPECTED_NUM_PARTS_BY_ENCRYPTION_TYPE = { [EncryptionType.AesCbc256_B64]: 2, - [EncryptionType.AesCbc128_HmacSha256_B64]: 3, [EncryptionType.AesCbc256_HmacSha256_B64]: 3, [EncryptionType.Rsa2048_OaepSha256_B64]: 1, [EncryptionType.Rsa2048_OaepSha1_B64]: 1, diff --git a/libs/common/src/platform/models/domain/enc-array-buffer.spec.ts b/libs/common/src/platform/models/domain/enc-array-buffer.spec.ts index 45a45ffe087..de0fea58e36 100644 --- a/libs/common/src/platform/models/domain/enc-array-buffer.spec.ts +++ b/libs/common/src/platform/models/domain/enc-array-buffer.spec.ts @@ -5,28 +5,28 @@ import { EncArrayBuffer } from "./enc-array-buffer"; describe("encArrayBuffer", () => { describe("parses the buffer", () => { - test.each([ - [EncryptionType.AesCbc128_HmacSha256_B64, "AesCbc128_HmacSha256_B64"], - [EncryptionType.AesCbc256_HmacSha256_B64, "AesCbc256_HmacSha256_B64"], - ])("with %c%s", (encType: EncryptionType) => { - const iv = makeStaticByteArray(16, 10); - const mac = makeStaticByteArray(32, 20); - // We use the minimum data length of 1 to test the boundary of valid lengths - const data = makeStaticByteArray(1, 100); + test.each([[EncryptionType.AesCbc256_HmacSha256_B64, "AesCbc256_HmacSha256_B64"]])( + "with %c%s", + (encType: EncryptionType) => { + const iv = makeStaticByteArray(16, 10); + const mac = makeStaticByteArray(32, 20); + // We use the minimum data length of 1 to test the boundary of valid lengths + const data = makeStaticByteArray(1, 100); - const array = new Uint8Array(1 + iv.byteLength + mac.byteLength + data.byteLength); - array.set([encType]); - array.set(iv, 1); - array.set(mac, 1 + iv.byteLength); - array.set(data, 1 + iv.byteLength + mac.byteLength); + const array = new Uint8Array(1 + iv.byteLength + mac.byteLength + data.byteLength); + array.set([encType]); + array.set(iv, 1); + array.set(mac, 1 + iv.byteLength); + array.set(data, 1 + iv.byteLength + mac.byteLength); - const actual = new EncArrayBuffer(array); + const actual = new EncArrayBuffer(array); - expect(actual.encryptionType).toEqual(encType); - expect(actual.ivBytes).toEqualBuffer(iv); - expect(actual.macBytes).toEqualBuffer(mac); - expect(actual.dataBytes).toEqualBuffer(data); - }); + expect(actual.encryptionType).toEqual(encType); + expect(actual.ivBytes).toEqualBuffer(iv); + expect(actual.macBytes).toEqualBuffer(mac); + expect(actual.dataBytes).toEqualBuffer(data); + }, + ); it("with AesCbc256_B64", () => { const encType = EncryptionType.AesCbc256_B64; @@ -50,7 +50,6 @@ describe("encArrayBuffer", () => { describe("throws if the buffer has an invalid length", () => { test.each([ - [EncryptionType.AesCbc128_HmacSha256_B64, 50, "AesCbc128_HmacSha256_B64"], [EncryptionType.AesCbc256_HmacSha256_B64, 50, "AesCbc256_HmacSha256_B64"], [EncryptionType.AesCbc256_B64, 18, "AesCbc256_B64"], ])("with %c%c%s", (encType: EncryptionType, minLength: number) => { diff --git a/libs/common/src/platform/models/domain/enc-array-buffer.ts b/libs/common/src/platform/models/domain/enc-array-buffer.ts index 305504f57b7..8b69cb347ba 100644 --- a/libs/common/src/platform/models/domain/enc-array-buffer.ts +++ b/libs/common/src/platform/models/domain/enc-array-buffer.ts @@ -20,7 +20,6 @@ export class EncArrayBuffer implements Encrypted { const encType = encBytes[0]; switch (encType) { - case EncryptionType.AesCbc128_HmacSha256_B64: case EncryptionType.AesCbc256_HmacSha256_B64: { const minimumLength = ENC_TYPE_LENGTH + IV_LENGTH + MAC_LENGTH + MIN_DATA_LENGTH; if (encBytes.length < minimumLength) { diff --git a/libs/common/src/platform/models/domain/enc-string.spec.ts b/libs/common/src/platform/models/domain/enc-string.spec.ts index 3b2586fc22f..c3f257d442a 100644 --- a/libs/common/src/platform/models/domain/enc-string.spec.ts +++ b/libs/common/src/platform/models/domain/enc-string.spec.ts @@ -60,9 +60,7 @@ describe("EncString", () => { const cases = [ "aXY=|Y3Q=", // AesCbc256_B64 w/out header - "aXY=|Y3Q=|cnNhQ3Q=", // AesCbc128_HmacSha256_B64 w/out header "0.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==", // AesCbc256_B64 with header - "1.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA==", // AesCbc128_HmacSha256_B64 "2.QmFzZTY0UGFydA==|QmFzZTY0UGFydA==|QmFzZTY0UGFydA==", // AesCbc256_HmacSha256_B64 "3.QmFzZTY0UGFydA==", // Rsa2048_OaepSha256_B64 "4.QmFzZTY0UGFydA==", // Rsa2048_OaepSha1_B64 diff --git a/libs/common/src/platform/models/domain/enc-string.ts b/libs/common/src/platform/models/domain/enc-string.ts index 4ea58a6809e..b0b03e0fb3c 100644 --- a/libs/common/src/platform/models/domain/enc-string.ts +++ b/libs/common/src/platform/models/domain/enc-string.ts @@ -89,7 +89,6 @@ export class EncString implements Encrypted { } switch (encType) { - case EncryptionType.AesCbc128_HmacSha256_B64: case EncryptionType.AesCbc256_HmacSha256_B64: this.iv = encPieces[0]; this.data = encPieces[1]; @@ -132,10 +131,7 @@ export class EncString implements Encrypted { } } else { encPieces = encryptedString.split("|"); - encType = - encPieces.length === 3 - ? EncryptionType.AesCbc128_HmacSha256_B64 - : EncryptionType.AesCbc256_B64; + encType = EncryptionType.AesCbc256_B64; } return { diff --git a/libs/common/src/platform/models/domain/symmetric-crypto-key.spec.ts b/libs/common/src/platform/models/domain/symmetric-crypto-key.spec.ts index e4c43264eaf..58c902ebab6 100644 --- a/libs/common/src/platform/models/domain/symmetric-crypto-key.spec.ts +++ b/libs/common/src/platform/models/domain/symmetric-crypto-key.spec.ts @@ -27,21 +27,6 @@ describe("SymmetricCryptoKey", () => { }); }); - it("AesCbc128_HmacSha256_B64", () => { - const key = makeStaticByteArray(32); - const cryptoKey = new SymmetricCryptoKey(key, EncryptionType.AesCbc128_HmacSha256_B64); - - expect(cryptoKey).toEqual({ - encKey: key.slice(0, 16), - encKeyB64: "AAECAwQFBgcICQoLDA0ODw==", - encType: 1, - key: key, - keyB64: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=", - macKey: key.slice(16, 32), - macKeyB64: "EBESExQVFhcYGRobHB0eHw==", - }); - }); - it("AesCbc256_HmacSha256_B64", () => { const key = makeStaticByteArray(64); const cryptoKey = new SymmetricCryptoKey(key); diff --git a/libs/common/src/platform/models/domain/symmetric-crypto-key.ts b/libs/common/src/platform/models/domain/symmetric-crypto-key.ts index eab4c7b2114..372b869fd9c 100644 --- a/libs/common/src/platform/models/domain/symmetric-crypto-key.ts +++ b/libs/common/src/platform/models/domain/symmetric-crypto-key.ts @@ -38,9 +38,6 @@ export class SymmetricCryptoKey { if (encType === EncryptionType.AesCbc256_B64 && key.byteLength === 32) { this.encKey = key; this.macKey = null; - } else if (encType === EncryptionType.AesCbc128_HmacSha256_B64 && key.byteLength === 32) { - this.encKey = key.slice(0, 16); - this.macKey = key.slice(16, 32); } else if (encType === EncryptionType.AesCbc256_HmacSha256_B64 && key.byteLength === 64) { this.encKey = key.slice(0, 32); this.macKey = key.slice(32, 64); From ef06e9f03c292c7c4aaead7a9777fc9f5d7f1777 Mon Sep 17 00:00:00 2001 From: cyprain-okeke <108260115+cyprain-okeke@users.noreply.github.com> Date: Tue, 11 Mar 2025 14:42:10 +0100 Subject: [PATCH 16/23] [PM-15442]Upgrade modal additional instances (#13557) * display inline information error message * Add collection service * Refactor the code * Add a feature flag to the change * Add the modal pop for free org * Use custom error messages passed from the validator * Add the js document * Merge changes in main * Add the changes after file movement * remove these floating promises * Adding unit test and seprating the validation * fix the unit test request * Remove the conditional statment in test --- .../collection-dialog.component.html | 2 +- .../collection-dialog.component.ts | 62 +++++++++++++++ ...ree-org-collection-limit.validator.spec.ts | 78 +++++++++++++++++++ .../free-org-collection-limit.validator.ts | 44 +++++++++++ .../vault-header/vault-header.component.ts | 76 +++++++++++++++++- apps/web/src/locales/en/messages.json | 3 + 6 files changed, 261 insertions(+), 4 deletions(-) create mode 100644 apps/web/src/app/admin-console/organizations/shared/validators/free-org-collection-limit.validator.spec.ts create mode 100644 apps/web/src/app/admin-console/organizations/shared/validators/free-org-collection-limit.validator.ts diff --git a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.html b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.html index 61fc290f6fe..9188ba5ab96 100644 --- a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.html +++ b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.html @@ -124,7 +124,7 @@ buttonType="primary" [disabled]="loading || dialogReadonly" > - {{ "save" | i18n }} + {{ buttonDisplayName | i18n }}