From ea860b2f35e55da35bc4b99b75c975a2f6cda89c Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Mon, 7 Apr 2025 12:55:53 -0400 Subject: [PATCH 01/11] [PM-17933] Rename github-action group to no longer be minor-only (#13799) * Renamed groups and added consistent periods * Fixed punctuation Co-authored-by: Matt Bishop --------- Co-authored-by: Matt Bishop --- .github/renovate.json5 | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/.github/renovate.json5 b/.github/renovate.json5 index fe5ae09deaa..973ba700349 100644 --- a/.github/renovate.json5 +++ b/.github/renovate.json5 @@ -4,10 +4,10 @@ enabledManagers: ["cargo", "github-actions", "npm"], packageRules: [ { - // Group all build/test/lint workflows for GitHub Actions together for Platform - // Since they are code owners we don't need to assign a review team in Renovate - // Any changes here should also be reflected in CODEOWNERS - groupName: "github-action minor", + // Group all build/test/lint workflows for GitHub Actions together for Platform. + // Since they are code owners we don't need to assign a review team in Renovate. + // Any changes here should also be reflected in CODEOWNERS. + groupName: "github-action", matchManagers: ["github-actions"], matchFileNames: [ "./github/workflows/automatic-issue-responses.yml", @@ -30,10 +30,10 @@ commitMessagePrefix: "[deps] Platform:", }, { - // Group all release-related workflows for GitHub Actions together for BRE - // Since they are code owners we don't need to assign a review team in Renovate - // Any changes here should also be reflected in CODEOWNERS - groupName: "github-action minor", + // Group all release-related workflows for GitHub Actions together for BRE. + // Since they are code owners we don't need to assign a review team in Renovate. + // Any changes here should also be reflected in CODEOWNERS. + groupName: "github-action", matchManagers: ["github-actions"], matchFileNames: [ "./github/workflows/brew-bump-desktop.yml", @@ -51,7 +51,7 @@ commitMessagePrefix: "[deps] BRE:", }, { - // Disable major and minor updates for TypeScript and Zone.js because they are managed by Angular + // Disable major and minor updates for TypeScript and Zone.js because they are managed by Angular. matchPackageNames: ["typescript", "zone.js"], matchUpdateTypes: ["major", "minor"], description: "Determined by Angular", @@ -72,27 +72,27 @@ enabled: false, }, { - // Renovate should manage patch updates for TypeScript and Zone.js, despite ignoring major and minor + // Renovate should manage patch updates for TypeScript and Zone.js, despite ignoring major and minor. matchPackageNames: ["typescript", "zone.js"], matchUpdateTypes: "patch", }, { - // We want to update all the Jest-related packages together, to reduce PR noise + // We want to update all the Jest-related packages together, to reduce PR noise. groupName: "jest", matchPackageNames: ["@types/jest", "jest", "ts-jest", "jest-preset-angular"], }, { - // We need to group all napi-related packages together to avoid build errors caused by version incompatibilities + // We need to group all napi-related packages together to avoid build errors caused by version incompatibilities. groupName: "napi", matchPackageNames: ["napi", "napi-build", "napi-derive"], }, { - // We need to group all macOS/iOS binding-related packages together to avoid build errors caused by version incompatibilities + // We need to group all macOS/iOS binding-related packages together to avoid build errors caused by version incompatibilities. groupName: "macOS/iOS bindings", matchPackageNames: ["core-foundation", "security-framework", "security-framework-sys"], }, { - // We need to group all zbus-related packages together to avoid build errors caused by version incompatibilities + // We need to group all zbus-related packages together to avoid build errors caused by version incompatibilities. groupName: "zbus", matchPackageNames: ["zbus", "zbus_polkit"], }, From 7f58cee41b33714449117434b995ca4f755fa909 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lison=20Fernandes?= Date: Mon, 7 Apr 2025 19:20:29 +0100 Subject: [PATCH 02/11] [PM-18897] Add workflow to auto-reply / auto-close GitHub Discussions (#13682) --- .../DISCUSSION_TEMPLATE/password-manager.yml | 11 ++ .../DISCUSSION_TEMPLATE/secrets-manager.yml | 11 ++ .github/workflows/auto-reply-discussions.yml | 112 ++++++++++++++++++ 3 files changed, 134 insertions(+) create mode 100644 .github/workflows/auto-reply-discussions.yml diff --git a/.github/DISCUSSION_TEMPLATE/password-manager.yml b/.github/DISCUSSION_TEMPLATE/password-manager.yml index bf2d0900db9..bc3938c1962 100644 --- a/.github/DISCUSSION_TEMPLATE/password-manager.yml +++ b/.github/DISCUSSION_TEMPLATE/password-manager.yml @@ -1,8 +1,19 @@ +labels: ["discussions-new"] body: - type: markdown attributes: value: | If you would like to contribute code to the Bitwarden codebase for consideration, please review [https://contributing.bitwarden.com/](https://contributing.bitwarden.com/) before posting. To keep discussion on topic, posts that do not include a proposal for a code contribution you wish to develop will be removed. For feature requests and community discussion, please visit https://community.bitwarden.com/ + - type: dropdown + attributes: + label: Select Topic Area + description: "What would you like to discuss? :warning: For feature requests and product feedback, please visit https://community.bitwarden.com/" + options: + - "✅ Code Contribution Proposal" + - "🚫 Product Feedback" + - "🚫 Feature Request" + validations: + required: true - type: textarea attributes: label: Code Contribution Proposal diff --git a/.github/DISCUSSION_TEMPLATE/secrets-manager.yml b/.github/DISCUSSION_TEMPLATE/secrets-manager.yml index bf2d0900db9..bc3938c1962 100644 --- a/.github/DISCUSSION_TEMPLATE/secrets-manager.yml +++ b/.github/DISCUSSION_TEMPLATE/secrets-manager.yml @@ -1,8 +1,19 @@ +labels: ["discussions-new"] body: - type: markdown attributes: value: | If you would like to contribute code to the Bitwarden codebase for consideration, please review [https://contributing.bitwarden.com/](https://contributing.bitwarden.com/) before posting. To keep discussion on topic, posts that do not include a proposal for a code contribution you wish to develop will be removed. For feature requests and community discussion, please visit https://community.bitwarden.com/ + - type: dropdown + attributes: + label: Select Topic Area + description: "What would you like to discuss? :warning: For feature requests and product feedback, please visit https://community.bitwarden.com/" + options: + - "✅ Code Contribution Proposal" + - "🚫 Product Feedback" + - "🚫 Feature Request" + validations: + required: true - type: textarea attributes: label: Code Contribution Proposal diff --git a/.github/workflows/auto-reply-discussions.yml b/.github/workflows/auto-reply-discussions.yml new file mode 100644 index 00000000000..8becc7471c5 --- /dev/null +++ b/.github/workflows/auto-reply-discussions.yml @@ -0,0 +1,112 @@ +name: Auto-reply to Discussions + +on: + discussion: + types: created + +jobs: + reply: + name: Auto-reply + runs-on: ubuntu-22.04 + + steps: + - name: Get discussion label and template name + id: discussion-label + uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 + with: + script: | + const discussion = context.payload.discussion; + const template_name = discussion.category.slug; + const label = discussion.labels?.[0]?.name ?? ''; + console.log('Discussion label:', label); + console.log('Discussion category slug:', template_name); + + core.setOutput('label', label); + core.setOutput('template_name', template_name); + + - name: Get selected topic + id: get_selected_topic + uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 + with: + result-encoding: string + script: | + try { + const body = context.payload.discussion.body; + const match = body.match(/### Select Topic Area\n\n(.*?)\n\n/); + console.log('Match:', match); + console.log('Match1:', match[1]); + return match ? match[1].trim() : ""; + } catch (error) { + console.error('Error getting selected topic:', error); + return ""; + } + + - name: Reply or close Discussion + uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 + env: + TEMPLATE_NAME: ${{ steps.discussion-label.outputs.template_name }} + TOPIC: ${{ steps.get_selected_topic.outputs.result }} + with: + script: | + async function addComment(discussionId, body) { + await github.graphql(` + mutation AddDiscussionComment($discussionId: ID!, $body: String!) { + addDiscussionComment(input: {discussionId: $discussionId, body: $body}) { + comment { + id + } + } + } + `, { + discussionId, + body + }); + } + + async function closeDiscussion(discussionId) { + await github.graphql(` + mutation { + closeDiscussion(input: {discussionId: "${discussionId}"}) { + discussion { + id + } + } + } + `); + console.log('♻️ Closing discussion'); + } + + const discussion = context.payload.discussion; + const isFeatureRequest = process.env.TEMPLATE_NAME === 'feature-request'; + const isTopicEmpty = !process.env.TOPIC || process.env.TOPIC.trim() === ''; // topic step may have failed + const isCodeTopic = process.env.TOPIC.includes('Code Contribution Proposal'); + const shouldClose = isFeatureRequest || (!isTopicEmpty && !isCodeTopic); + + console.log('Template name:', process.env.TEMPLATE_NAME); + console.log('Topic:', process.env.TOPIC); + console.log('isTopicEmpty:', isTopicEmpty); + console.log('isCodeTopic:', isCodeTopic); + console.log('shouldClose:', shouldClose); + + if (shouldClose) { + const closeMessage = + "Thank you for your contribution! GitHub Discussions is specifically for [proposing code](https://contributing.bitwarden.com/) that you would like to write for the Bitwarden codebase. Since this post does not appear to include a proposal, it will be closed. If you believe this was done in error or have any questions, please feel free to reach out to us!\n\n" + + "- :bulb: For feature requests and general discussions, please visit the [Bitwarden Community Forums](https://community.bitwarden.com/).\n" + + "- :information_source: For questions and support, visit the [Bitwarden Help Center](https://bitwarden.com/help/).\n" + + "- :bug: To report a potential bug, please visit the appropriate repository: [Server](https://github.com/bitwarden/server/issues) | [Clients](https://github.com/bitwarden/clients/issues) | [iOS](https://github.com/bitwarden/ios/issues) | [Android](https://github.com/bitwarden/android/issues)."; + + await addComment(discussion.node_id, closeMessage); + await closeDiscussion(discussion.node_id); + return; + } + + const replyMessage = + ":sparkles: Thank you for your code contribution proposal! While the Bitwarden team reviews your submission, we encourage you to check out our [contribution guidelines](https://contributing.bitwarden.com/).\n\n" + + "Please ensure that your code contribution includes a detailed description of what you would like to contribute, along with any relevant screenshots and links to existing feature requests. This information helps us gather feedback from the community and Bitwarden team members before you start writing code.\n\n" + + "To keep discussions focused, posts that do not include a proposal for a code contribution will be removed.\n\n" + + "- :bulb: For feature requests and general discussion, please visit the [Bitwarden Community Forums](https://community.bitwarden.com/).\n" + + "- :information_source: For questions and support, visit the [Bitwarden Help Center](https://bitwarden.com/help/).\n" + + "- :bug: To report a potential bug, please visit the corresponding repo. [Server](https://github.com/bitwarden/server/issues) | [Clients](https://github.com/bitwarden/clients/issues) | [iOS](https://github.com/bitwarden/ios/issues) | [Android](https://github.com/bitwarden/android/issues)\n\n" + + "Thank you for contributing to Bitwarden!"; + + await addComment(discussion.node_id, replyMessage); From 22678768605b5554c0b2b22043c508261d9a8b47 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Mon, 7 Apr 2025 11:58:50 -0700 Subject: [PATCH 03/11] refactor(set-change-password): [Auth/PM-18206] Update InputPasswordComponent to handle multiple flows (#13745) Updates the InputPasswordComponent so that it can eventually be used in multiple set/change password scenarios. Most importantly, this PR adds an InputPasswordFlow enum and @Input so that parent components can dictate which UI elements to show. --- apps/browser/src/_locales/en/messages.json | 3 + apps/desktop/src/locales/en/messages.json | 3 + .../web-registration-finish.service.spec.ts | 14 +- .../complete-trial-initiation.component.html | 6 +- .../complete-trial-initiation.component.ts | 10 +- apps/web/src/locales/en/messages.json | 3 + .../input-password.component.html | 76 +++++++-- .../input-password.component.ts | 156 ++++++++++++++---- .../angular/input-password/input-password.mdx | 90 +++++++--- .../input-password/input-password.stories.ts | 91 +++++++++- .../input-password/password-input-result.ts | 12 +- ...efault-registration-finish.service.spec.ts | 6 +- .../default-registration-finish.service.ts | 2 +- .../registration-finish.component.html | 3 +- .../registration-finish.component.ts | 9 +- .../default-set-password-jit.service.spec.ts | 6 +- .../default-set-password-jit.service.ts | 6 +- .../set-password-jit.component.html | 3 +- .../set-password-jit.component.ts | 6 +- .../set-password-jit.service.abstraction.ts | 2 +- 20 files changed, 394 insertions(+), 113 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 54024b83f98..586e7e1f2cf 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -2297,6 +2297,9 @@ "privacyPolicy": { "message": "Privacy Policy" }, + "yourNewPasswordCannotBeTheSameAsYourCurrentPassword": { + "message": "Your new password cannot be the same as your current password." + }, "hintEqualsPassword": { "message": "Your password hint cannot be the same as your password." }, diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index f1c63780793..8850cbe5a3f 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -2072,6 +2072,9 @@ "personalOwnershipSubmitError": { "message": "Due to an enterprise policy, you are restricted from saving items to your individual vault. Change the ownership option to an organization and choose from available collections." }, + "yourNewPasswordCannotBeTheSameAsYourCurrentPassword": { + "message": "Your new password cannot be the same as your current password." + }, "hintEqualsPassword": { "message": "Your password hint cannot be the same as your password." }, diff --git a/apps/web/src/app/auth/core/services/registration/web-registration-finish.service.spec.ts b/apps/web/src/app/auth/core/services/registration/web-registration-finish.service.spec.ts index 48b74dc5e2e..aa02e28b3b3 100644 --- a/apps/web/src/app/auth/core/services/registration/web-registration-finish.service.spec.ts +++ b/apps/web/src/app/auth/core/services/registration/web-registration-finish.service.spec.ts @@ -187,11 +187,11 @@ describe("WebRegistrationFinishService", () => { masterKey = new SymmetricCryptoKey(new Uint8Array(64).buffer as CsprngArray) as MasterKey; passwordInputResult = { masterKey: masterKey, - masterKeyHash: "masterKeyHash", + serverMasterKeyHash: "serverMasterKeyHash", localMasterKeyHash: "localMasterKeyHash", kdfConfig: DEFAULT_KDF_CONFIG, hint: "hint", - password: "password", + newPassword: "newPassword", }; userKey = new SymmetricCryptoKey(new Uint8Array(64).buffer as CsprngArray) as UserKey; @@ -239,7 +239,7 @@ describe("WebRegistrationFinishService", () => { expect.objectContaining({ email, emailVerificationToken: emailVerificationToken, - masterPasswordHash: passwordInputResult.masterKeyHash, + masterPasswordHash: passwordInputResult.serverMasterKeyHash, masterPasswordHint: passwordInputResult.hint, userSymmetricKey: userKeyEncString.encryptedString, userAsymmetricKeys: { @@ -277,7 +277,7 @@ describe("WebRegistrationFinishService", () => { expect.objectContaining({ email, emailVerificationToken: undefined, - masterPasswordHash: passwordInputResult.masterKeyHash, + masterPasswordHash: passwordInputResult.serverMasterKeyHash, masterPasswordHint: passwordInputResult.hint, userSymmetricKey: userKeyEncString.encryptedString, userAsymmetricKeys: { @@ -320,7 +320,7 @@ describe("WebRegistrationFinishService", () => { expect.objectContaining({ email, emailVerificationToken: undefined, - masterPasswordHash: passwordInputResult.masterKeyHash, + masterPasswordHash: passwordInputResult.serverMasterKeyHash, masterPasswordHint: passwordInputResult.hint, userSymmetricKey: userKeyEncString.encryptedString, userAsymmetricKeys: { @@ -365,7 +365,7 @@ describe("WebRegistrationFinishService", () => { expect.objectContaining({ email, emailVerificationToken: undefined, - masterPasswordHash: passwordInputResult.masterKeyHash, + masterPasswordHash: passwordInputResult.serverMasterKeyHash, masterPasswordHint: passwordInputResult.hint, userSymmetricKey: userKeyEncString.encryptedString, userAsymmetricKeys: { @@ -412,7 +412,7 @@ describe("WebRegistrationFinishService", () => { expect.objectContaining({ email, emailVerificationToken: undefined, - masterPasswordHash: passwordInputResult.masterKeyHash, + masterPasswordHash: passwordInputResult.serverMasterKeyHash, masterPasswordHint: passwordInputResult.hint, userSymmetricKey: userKeyEncString.encryptedString, userAsymmetricKeys: { diff --git a/apps/web/src/app/billing/trial-initiation/complete-trial-initiation/complete-trial-initiation.component.html b/apps/web/src/app/billing/trial-initiation/complete-trial-initiation/complete-trial-initiation.component.html index 416d4004260..8118b1e512d 100644 --- a/apps/web/src/app/billing/trial-initiation/complete-trial-initiation/complete-trial-initiation.component.html +++ b/apps/web/src/app/billing/trial-initiation/complete-trial-initiation/complete-trial-initiation.component.html @@ -1,19 +1,21 @@
diff --git a/apps/web/src/app/billing/trial-initiation/complete-trial-initiation/complete-trial-initiation.component.ts b/apps/web/src/app/billing/trial-initiation/complete-trial-initiation/complete-trial-initiation.component.ts index 27ce4dc9f5d..e2a4149a58d 100644 --- a/apps/web/src/app/billing/trial-initiation/complete-trial-initiation/complete-trial-initiation.component.ts +++ b/apps/web/src/app/billing/trial-initiation/complete-trial-initiation/complete-trial-initiation.component.ts @@ -6,7 +6,11 @@ import { FormBuilder, Validators } from "@angular/forms"; import { ActivatedRoute, Router } from "@angular/router"; import { firstValueFrom, Subject, switchMap, takeUntil } from "rxjs"; -import { PasswordInputResult, RegistrationFinishService } from "@bitwarden/auth/angular"; +import { + InputPasswordFlow, + PasswordInputResult, + RegistrationFinishService, +} from "@bitwarden/auth/angular"; import { LoginStrategyServiceAbstraction, PasswordLoginCredentials } from "@bitwarden/auth/common"; import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; @@ -47,6 +51,8 @@ export type InitiationPath = export class CompleteTrialInitiationComponent implements OnInit, OnDestroy { @ViewChild("stepper", { static: false }) verticalStepper: VerticalStepperComponent; + InputPasswordFlow = InputPasswordFlow; + /** Password Manager or Secrets Manager */ product: ProductType; /** The tier of product being subscribed to */ @@ -363,7 +369,7 @@ export class CompleteTrialInitiationComponent implements OnInit, OnDestroy { return; } - await this.logIn(passwordInputResult.password, captchaToken); + await this.logIn(passwordInputResult.newPassword, captchaToken); this.submitting = false; diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 6c730338fea..3300f73eb3b 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -5707,6 +5707,9 @@ "webAuthnSuccess": { "message": "WebAuthn verified successfully! You may close this tab." }, + "yourNewPasswordCannotBeTheSameAsYourCurrentPassword": { + "message": "Your new password cannot be the same as your current password." + }, "hintEqualsPassword": { "message": "Your password hint cannot be the same as your password." }, diff --git a/libs/auth/src/angular/input-password/input-password.component.html b/libs/auth/src/angular/input-password/input-password.component.html index 114d9b8fb8d..39995f9f44f 100644 --- a/libs/auth/src/angular/input-password/input-password.component.html +++ b/libs/auth/src/angular/input-password/input-password.component.html @@ -4,14 +4,36 @@ [policy]="masterPasswordPolicyOptions" > + + {{ "currentMasterPass" | i18n }} + + + +
- {{ "masterPassword" | i18n }} + {{ "newMasterPass" | i18n }}
@@ -38,10 +60,10 @@ {{ "confirmMasterPassword" | i18n }} + + + {{ "rotateAccountEncKey" | i18n }} + + + + + + +
+ + + +
diff --git a/libs/auth/src/angular/input-password/input-password.component.ts b/libs/auth/src/angular/input-password/input-password.component.ts index c613cf5f533..2f8e5d5b01d 100644 --- a/libs/auth/src/angular/input-password/input-password.component.ts +++ b/libs/auth/src/angular/input-password/input-password.component.ts @@ -1,7 +1,5 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore -import { Component, EventEmitter, Input, Output } from "@angular/core"; -import { ReactiveFormsModule, FormBuilder, Validators } from "@angular/forms"; +import { Component, EventEmitter, Input, OnInit, Output } from "@angular/core"; +import { ReactiveFormsModule, FormBuilder, Validators, FormGroup } from "@angular/forms"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { @@ -23,6 +21,7 @@ import { IconButtonModule, InputModule, ToastService, + Translation, } from "@bitwarden/components"; import { DEFAULT_KDF_CONFIG, KeyService } from "@bitwarden/key-management"; @@ -36,6 +35,29 @@ import { PasswordCalloutComponent } from "../password-callout/password-callout.c import { PasswordInputResult } from "./password-input-result"; +/** + * Determines which form input elements will be displayed in the UI. + */ +export enum InputPasswordFlow { + /** + * - Input: New password + * - Input: Confirm new password + * - Input: Hint + * - Checkbox: Check for breaches + */ + SetInitialPassword, + /** + * Everything above, plus: + * - Input: Current password (as the first element in the UI) + */ + ChangePassword, + /** + * Everything above, plus: + * - Checkbox: Rotate account encryption key (as the last element in the UI) + */ + ChangePasswordWithOptionalUserKeyRotation, +} + @Component({ standalone: true, selector: "auth-input-password", @@ -54,44 +76,58 @@ import { PasswordInputResult } from "./password-input-result"; JslibModule, ], }) -export class InputPasswordComponent { +export class InputPasswordComponent implements OnInit { @Output() onPasswordFormSubmit = new EventEmitter(); + @Output() onSecondaryButtonClick = new EventEmitter(); - @Input({ required: true }) email: string; - @Input() buttonText: string; + @Input({ required: true }) inputPasswordFlow!: InputPasswordFlow; + @Input({ required: true }) email!: string; + + @Input() loading = false; @Input() masterPasswordPolicyOptions: MasterPasswordPolicyOptions | null = null; - @Input() loading: boolean = false; - @Input() btnBlock: boolean = true; + @Input() inlineButtons = false; + @Input() primaryButtonText?: Translation; + protected primaryButtonTextStr: string = ""; + @Input() secondaryButtonText?: Translation; + protected secondaryButtonTextStr: string = ""; + + protected InputPasswordFlow = InputPasswordFlow; private minHintLength = 0; protected maxHintLength = 50; protected minPasswordLength = Utils.minimumPasswordLength; protected minPasswordMsg = ""; - protected passwordStrengthScore: PasswordStrengthScore; + protected passwordStrengthScore: PasswordStrengthScore = 0; protected showErrorSummary = false; protected showPassword = false; - protected formGroup = this.formBuilder.group( + protected formGroup = this.formBuilder.nonNullable.group( { - password: ["", [Validators.required, Validators.minLength(this.minPasswordLength)]], - confirmedPassword: ["", Validators.required], + newPassword: ["", [Validators.required, Validators.minLength(this.minPasswordLength)]], + confirmNewPassword: ["", Validators.required], hint: [ "", // must be string (not null) because we check length in validation [Validators.minLength(this.minHintLength), Validators.maxLength(this.maxHintLength)], ], - checkForBreaches: true, + checkForBreaches: [true], }, { validators: [ + InputsFieldMatch.compareInputs( + "doNotMatch", + "currentPassword", + "newPassword", + this.i18nService.t("yourNewPasswordCannotBeTheSameAsYourCurrentPassword"), + ), InputsFieldMatch.compareInputs( "match", - "password", - "confirmedPassword", + "newPassword", + "confirmNewPassword", this.i18nService.t("masterPassDoesntMatch"), ), InputsFieldMatch.compareInputs( "doNotMatch", - "password", + "newPassword", "hint", this.i18nService.t("hintEqualsPassword"), ), @@ -109,6 +145,41 @@ export class InputPasswordComponent { private toastService: ToastService, ) {} + ngOnInit(): void { + if ( + this.inputPasswordFlow === InputPasswordFlow.ChangePassword || + this.inputPasswordFlow === InputPasswordFlow.ChangePasswordWithOptionalUserKeyRotation + ) { + // https://github.com/angular/angular/issues/48794 + (this.formGroup as FormGroup).addControl( + "currentPassword", + this.formBuilder.control("", Validators.required), + ); + } + + if (this.inputPasswordFlow === InputPasswordFlow.ChangePasswordWithOptionalUserKeyRotation) { + // https://github.com/angular/angular/issues/48794 + (this.formGroup as FormGroup).addControl( + "rotateUserKey", + this.formBuilder.control(false), + ); + } + + if (this.primaryButtonText) { + this.primaryButtonTextStr = this.i18nService.t( + this.primaryButtonText.key, + ...(this.primaryButtonText?.placeholders ?? []), + ); + } + + if (this.secondaryButtonText) { + this.secondaryButtonTextStr = this.i18nService.t( + this.secondaryButtonText.key, + ...(this.secondaryButtonText?.placeholders ?? []), + ); + } + } + get minPasswordLengthMsg() { if ( this.masterPasswordPolicyOptions != null && @@ -132,10 +203,10 @@ export class InputPasswordComponent { return; } - const password = this.formGroup.controls.password.value; + const newPassword = this.formGroup.controls.newPassword.value; - const passwordEvaluatedSuccessfully = await this.evaluatePassword( - password, + const passwordEvaluatedSuccessfully = await this.evaluateNewPassword( + newPassword, this.passwordStrengthScore, this.formGroup.controls.checkForBreaches.value, ); @@ -152,38 +223,55 @@ export class InputPasswordComponent { } const masterKey = await this.keyService.makeMasterKey( - password, + newPassword, this.email.trim().toLowerCase(), kdfConfig, ); - const masterKeyHash = await this.keyService.hashMasterKey(password, masterKey); + const serverMasterKeyHash = await this.keyService.hashMasterKey( + newPassword, + masterKey, + HashPurpose.ServerAuthorization, + ); const localMasterKeyHash = await this.keyService.hashMasterKey( - password, + newPassword, masterKey, HashPurpose.LocalAuthorization, ); - this.onPasswordFormSubmit.emit({ - masterKey, - masterKeyHash, - localMasterKeyHash, - kdfConfig, + const passwordInputResult: PasswordInputResult = { + newPassword, hint: this.formGroup.controls.hint.value, - password, - }); + kdfConfig, + masterKey, + serverMasterKeyHash, + localMasterKeyHash, + }; + + if ( + this.inputPasswordFlow === InputPasswordFlow.ChangePassword || + this.inputPasswordFlow === InputPasswordFlow.ChangePasswordWithOptionalUserKeyRotation + ) { + passwordInputResult.currentPassword = this.formGroup.get("currentPassword")?.value; + } + + if (this.inputPasswordFlow === InputPasswordFlow.ChangePasswordWithOptionalUserKeyRotation) { + passwordInputResult.rotateUserKey = this.formGroup.get("rotateUserKey")?.value; + } + + this.onPasswordFormSubmit.emit(passwordInputResult); }; // Returns true if the password passes all checks, false otherwise - private async evaluatePassword( - password: string, + private async evaluateNewPassword( + newPassword: string, passwordStrengthScore: PasswordStrengthScore, checkForBreaches: boolean, ) { // Check if the password is breached, weak, or both const passwordIsBreached = - checkForBreaches && (await this.auditService.passwordLeaked(password)); + checkForBreaches && (await this.auditService.passwordLeaked(newPassword)); const passwordWeak = passwordStrengthScore != null && passwordStrengthScore < 3; @@ -224,7 +312,7 @@ export class InputPasswordComponent { this.masterPasswordPolicyOptions != null && !this.policyService.evaluateMasterPassword( this.passwordStrengthScore, - password, + newPassword, this.masterPasswordPolicyOptions, ) ) { diff --git a/libs/auth/src/angular/input-password/input-password.mdx b/libs/auth/src/angular/input-password/input-password.mdx index 5110e2b3130..c1edcc254a7 100644 --- a/libs/auth/src/angular/input-password/input-password.mdx +++ b/libs/auth/src/angular/input-password/input-password.mdx @@ -6,9 +6,9 @@ import * as stories from "./input-password.stories.ts"; # InputPassword Component -The `InputPasswordComponent` allows a user to enter a master password and hint. On submission it -creates a master key, master key hash, and emits those values to the parent (along with the hint and -default kdfConfig). +The `InputPasswordComponent` allows a user to enter master password related credentials. On +submission it creates a master key, master key hash, and emits those values to the parent (along +with the other values found in `PasswordInputResult`). The component is intended for re-use in different scenarios throughout the application. Therefore it is mostly presentational and simply emits values rather than acting on them itself. It is the job of @@ -18,26 +18,66 @@ the parent component to act on those values as needed. ## `@Input()`'s -- `email` (**required**) - the parent component must provide an email so that the - `InputPasswordComponent` can create a master key. -- `buttonText` (optional) - an `i18n` translated string that can be used as button text (default - text is "Set master password"). -- `masterPasswordPolicyOptions` (optional) - used to display and enforce master password policy - requirements. +**Required** + +- `inputPasswordFlow` - the parent component must provide the correct flow, which is used to + determine which form input elements will be displayed in the UI. +- `email` - the parent component must provide an email so that the `InputPasswordComponent` can + create a master key. + +**Optional** + +- `loading` - a boolean used to indicate that the parent component is performing some + long-running/async operation and that the form should be disabled until the operation is complete. + The primary button will also show a spinner if `loading` true. +- `masterPasswordPolicyOptions` - used to display and enforce master password policy requirements. +- `inlineButtons` - takes a boolean that determines if the button(s) should be displayed inline (as + opposed to full-width) +- `primaryButtonText` - takes a `Translation` object that can be used as button text +- `secondaryButtonText` - takes a `Translation` object that can be used as button text + +## `@Output()`'s + +- `onPasswordFormSubmit` - on form submit, emits a `PasswordInputResult` object +- `onSecondaryButtonClick` - on click, emits a notice that the secondary button has been clicked. + The parent component can listen for this event and take some custom action as needed (go back, + cancel, logout, etc.)
## Form Input Fields -The `InputPasswordComponent` allows a user to enter: +The `InputPasswordComponent` can handle up to 6 different form input fields, depending on the +`InputPasswordFlow` provided by the parent component. -1. Master password -2. Master password confirmation -3. Hint (optional) -4. Chooses whether to check for password breaches (checkbox) +**InputPasswordFlow.SetInitialPassword** -Validation ensures that the master password and confirmed master password are the same, and that the -master password and hint values are not the same. +- Input: New password +- Input: Confirm new password +- Input: Hint +- Checkbox: Check for breaches + +**InputPasswordFlow.ChangePassword** + +Includes everything above, plus: + +- Input: Current password (as the first element in the UI) + +**InputPasswordFlow.ChangePasswordWithOptionalUserKeyRotation** + +Includes everything above, plus: + +- Checkbox: Rotate account encryption key (as the last element in the UI) + +
+ +## Validation + +Validation ensures that: + +- The current password and new password are NOT the same +- The new password and confirmed new password are the same +- The new password and password hint are NOT the same
@@ -57,19 +97,23 @@ When the form is submitted, the `InputPasswordComponent` does the following in o ```typescript export interface PasswordInputResult { - masterKey: MasterKey; - masterKeyHash: string; - kdfConfig: PBKDF2KdfConfig; + newPassword: string; hint: string; + kdfConfig: PBKDF2KdfConfig; + masterKey: MasterKey; + serverMasterKeyHash: string; + localMasterKeyHash: string; + currentPassword?: string; // included if the flow is ChangePassword or ChangePasswordWithOptionalUserKeyRotation + rotateUserKey?: boolean; // included if the flow is ChangePasswordWithOptionalUserKeyRotation } ``` -# Default Example +# Example - InputPasswordFlow.SetInitialPassword - +
-# With Policy Requrements +# Example - With Policy Requrements - + diff --git a/libs/auth/src/angular/input-password/input-password.stories.ts b/libs/auth/src/angular/input-password/input-password.stories.ts index 41577328f87..beda97a2c16 100644 --- a/libs/auth/src/angular/input-password/input-password.stories.ts +++ b/libs/auth/src/angular/input-password/input-password.stories.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { importProvidersFrom } from "@angular/core"; import { BrowserAnimationsModule } from "@angular/platform-browser/animations"; import { action } from "@storybook/addon-actions"; @@ -18,7 +16,7 @@ import { KeyService } from "@bitwarden/key-management"; // eslint-disable-next-line import/no-restricted-paths, no-restricted-imports import { PreloadedEnglishI18nModule } from "../../../../../apps/web/src/app/core/tests"; -import { InputPasswordComponent } from "./input-password.component"; +import { InputPasswordComponent, InputPasswordFlow } from "./input-password.component"; export default { title: "Auth/Input Password", @@ -62,7 +60,7 @@ export default { provide: PasswordStrengthServiceAbstraction, useValue: { getPasswordStrength: (password) => { - let score = 0; + let score: number | null = null; if (password.length === 0) { score = null; } else if (password.length <= 4) { @@ -88,6 +86,12 @@ export default { }), ], args: { + InputPasswordFlow: { + SetInitialPassword: InputPasswordFlow.SetInitialPassword, + ChangePassword: InputPasswordFlow.ChangePassword, + ChangePasswordWithOptionalUserKeyRotation: + InputPasswordFlow.ChangePasswordWithOptionalUserKeyRotation, + }, masterPasswordPolicyOptions: { minComplexity: 4, minLength: 14, @@ -96,25 +100,77 @@ export default { requireNumbers: true, requireSpecial: true, } as MasterPasswordPolicyOptions, + argTypes: { + onSecondaryButtonClick: { action: "onSecondaryButtonClick" }, + }, }, } as Meta; type Story = StoryObj; -export const Default: Story = { +export const SetInitialPassword: Story = { render: (args) => ({ props: args, template: ` - + `, }), }; -export const WithPolicy: Story = { +export const ChangePassword: Story = { render: (args) => ({ props: args, template: ` - + + `, + }), +}; + +export const ChangePasswordWithOptionalUserKeyRotation: Story = { + render: (args) => ({ + props: args, + template: ` + + `, + }), +}; + +export const WithPolicies: Story = { + render: (args) => ({ + props: args, + template: ` + + `, + }), +}; + +export const SecondaryButton: Story = { + render: (args) => ({ + props: args, + template: ` + + `, + }), +}; + +export const SecondaryButtonWithPlaceHolderText: Story = { + render: (args) => ({ + props: args, + template: ` + `, }), }; @@ -123,7 +179,24 @@ export const InlineButton: Story = { render: (args) => ({ props: args, template: ` - + + `, + }), +}; + +export const InlineButtons: Story = { + render: (args) => ({ + props: args, + template: ` + `, }), }; diff --git a/libs/auth/src/angular/input-password/password-input-result.ts b/libs/auth/src/angular/input-password/password-input-result.ts index 07157aaf4ca..c64225e2eb5 100644 --- a/libs/auth/src/angular/input-password/password-input-result.ts +++ b/libs/auth/src/angular/input-password/password-input-result.ts @@ -2,10 +2,12 @@ import { MasterKey } from "@bitwarden/common/types/key"; import { PBKDF2KdfConfig } from "@bitwarden/key-management"; export interface PasswordInputResult { - masterKey: MasterKey; - masterKeyHash: string; - localMasterKeyHash: string; - kdfConfig: PBKDF2KdfConfig; + newPassword: string; hint: string; - password: string; + kdfConfig: PBKDF2KdfConfig; + masterKey: MasterKey; + serverMasterKeyHash: string; + localMasterKeyHash: string; + currentPassword?: string; + rotateUserKey?: boolean; } diff --git a/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.spec.ts b/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.spec.ts index 14600cebf1d..c288f30d812 100644 --- a/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.spec.ts +++ b/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.spec.ts @@ -60,11 +60,11 @@ describe("DefaultRegistrationFinishService", () => { masterKey = new SymmetricCryptoKey(new Uint8Array(64).buffer as CsprngArray) as MasterKey; passwordInputResult = { masterKey: masterKey, - masterKeyHash: "masterKeyHash", + serverMasterKeyHash: "serverMasterKeyHash", localMasterKeyHash: "localMasterKeyHash", kdfConfig: DEFAULT_KDF_CONFIG, hint: "hint", - password: "password", + newPassword: "password", }; userKey = new SymmetricCryptoKey(new Uint8Array(64).buffer as CsprngArray) as UserKey; @@ -101,7 +101,7 @@ describe("DefaultRegistrationFinishService", () => { expect.objectContaining({ email, emailVerificationToken: emailVerificationToken, - masterPasswordHash: passwordInputResult.masterKeyHash, + masterPasswordHash: passwordInputResult.serverMasterKeyHash, masterPasswordHint: passwordInputResult.hint, userSymmetricKey: userKeyEncString.encryptedString, userAsymmetricKeys: { diff --git a/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.ts b/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.ts index 72e26720134..7d844ce8cb0 100644 --- a/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.ts +++ b/libs/auth/src/angular/registration/registration-finish/default-registration-finish.service.ts @@ -81,7 +81,7 @@ export class DefaultRegistrationFinishService implements RegistrationFinishServi const registerFinishRequest = new RegisterFinishRequest( email, - passwordInputResult.masterKeyHash, + passwordInputResult.serverMasterKeyHash, passwordInputResult.hint, encryptedUserKey, userAsymmetricKeysRequest, diff --git a/libs/auth/src/angular/registration/registration-finish/registration-finish.component.html b/libs/auth/src/angular/registration/registration-finish/registration-finish.component.html index e42ed91166a..ccc502bd7b6 100644 --- a/libs/auth/src/angular/registration/registration-finish/registration-finish.component.html +++ b/libs/auth/src/angular/registration/registration-finish/registration-finish.component.html @@ -5,8 +5,9 @@ diff --git a/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts b/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts index c419e1f427f..506b7475db1 100644 --- a/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts +++ b/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts @@ -22,7 +22,10 @@ import { PasswordLoginCredentials, } from "../../../common"; import { AnonLayoutWrapperDataService } from "../../anon-layout/anon-layout-wrapper-data.service"; -import { InputPasswordComponent } from "../../input-password/input-password.component"; +import { + InputPasswordComponent, + InputPasswordFlow, +} from "../../input-password/input-password.component"; import { PasswordInputResult } from "../../input-password/password-input-result"; import { RegistrationFinishService } from "./registration-finish.service"; @@ -36,6 +39,8 @@ import { RegistrationFinishService } from "./registration-finish.service"; export class RegistrationFinishComponent implements OnInit, OnDestroy { private destroy$ = new Subject(); + InputPasswordFlow = InputPasswordFlow; + loading = true; submitting = false; email: string; @@ -176,7 +181,7 @@ export class RegistrationFinishComponent implements OnInit, OnDestroy { try { const credentials = new PasswordLoginCredentials( this.email, - passwordInputResult.password, + passwordInputResult.newPassword, captchaBypassToken, null, ); diff --git a/libs/auth/src/angular/set-password-jit/default-set-password-jit.service.spec.ts b/libs/auth/src/angular/set-password-jit/default-set-password-jit.service.spec.ts index bd62092a4b6..cbcebd14526 100644 --- a/libs/auth/src/angular/set-password-jit/default-set-password-jit.service.spec.ts +++ b/libs/auth/src/angular/set-password-jit/default-set-password-jit.service.spec.ts @@ -112,11 +112,11 @@ describe("DefaultSetPasswordJitService", () => { passwordInputResult = { masterKey: masterKey, - masterKeyHash: "masterKeyHash", + serverMasterKeyHash: "serverMasterKeyHash", localMasterKeyHash: "localMasterKeyHash", hint: "hint", kdfConfig: DEFAULT_KDF_CONFIG, - password: "password", + newPassword: "password", }; credentials = { @@ -131,7 +131,7 @@ describe("DefaultSetPasswordJitService", () => { userDecryptionOptionsService.userDecryptionOptions$ = userDecryptionOptionsSubject; setPasswordRequest = new SetPasswordRequest( - passwordInputResult.masterKeyHash, + passwordInputResult.serverMasterKeyHash, protectedUserKey[1].encryptedString, passwordInputResult.hint, orgSsoIdentifier, diff --git a/libs/auth/src/angular/set-password-jit/default-set-password-jit.service.ts b/libs/auth/src/angular/set-password-jit/default-set-password-jit.service.ts index 42d964f3de0..174760aae21 100644 --- a/libs/auth/src/angular/set-password-jit/default-set-password-jit.service.ts +++ b/libs/auth/src/angular/set-password-jit/default-set-password-jit.service.ts @@ -44,7 +44,7 @@ export class DefaultSetPasswordJitService implements SetPasswordJitService { async setPassword(credentials: SetPasswordCredentials): Promise { const { masterKey, - masterKeyHash, + serverMasterKeyHash, localMasterKeyHash, hint, kdfConfig, @@ -70,7 +70,7 @@ export class DefaultSetPasswordJitService implements SetPasswordJitService { const [keyPair, keysRequest] = await this.makeKeyPairAndRequest(protectedUserKey); const request = new SetPasswordRequest( - masterKeyHash, + serverMasterKeyHash, protectedUserKey[1].encryptedString, hint, orgSsoIdentifier, @@ -92,7 +92,7 @@ export class DefaultSetPasswordJitService implements SetPasswordJitService { await this.masterPasswordService.setMasterKeyHash(localMasterKeyHash, userId); if (resetPasswordAutoEnroll) { - await this.handleResetPasswordAutoEnroll(masterKeyHash, orgId, userId); + await this.handleResetPasswordAutoEnroll(serverMasterKeyHash, orgId, userId); } } diff --git a/libs/auth/src/angular/set-password-jit/set-password-jit.component.html b/libs/auth/src/angular/set-password-jit/set-password-jit.component.html index aa6a1229937..3a4956ef7e9 100644 --- a/libs/auth/src/angular/set-password-jit/set-password-jit.component.html +++ b/libs/auth/src/angular/set-password-jit/set-password-jit.component.html @@ -13,7 +13,8 @@ Date: Mon, 7 Apr 2025 22:00:31 +0100 Subject: [PATCH 04/11] Fix the bug for free bitwarden Families menu (#14155) --- .../src/app/billing/services/free-families-policy.service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/web/src/app/billing/services/free-families-policy.service.ts b/apps/web/src/app/billing/services/free-families-policy.service.ts index 8d4e89d40a0..81cb970cdbe 100644 --- a/apps/web/src/app/billing/services/free-families-policy.service.ts +++ b/apps/web/src/app/billing/services/free-families-policy.service.ts @@ -59,8 +59,8 @@ export class FreeFamiliesPolicyService { return false; } const { belongToOneEnterpriseOrgs, isFreeFamilyPolicyEnabled } = orgStatus; - const canManageSponsorships = organizations.filter((org) => org.canManageSponsorships); - return canManageSponsorships && !(belongToOneEnterpriseOrgs && isFreeFamilyPolicyEnabled); + const hasSponsorshipOrgs = organizations.some((org) => org.canManageSponsorships); + return hasSponsorshipOrgs && !(belongToOneEnterpriseOrgs && isFreeFamilyPolicyEnabled); } checkEnterpriseOrganizationsAndFetchPolicy(): Observable { From 19d59212a893de1e1296eec7d9d8e79e74d4e009 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Mon, 7 Apr 2025 17:20:45 -0400 Subject: [PATCH 05/11] Add save-exact=true (#14169) --- .npmrc | 1 + 1 file changed, 1 insertion(+) create mode 100644 .npmrc diff --git a/.npmrc b/.npmrc new file mode 100644 index 00000000000..cffe8cdef13 --- /dev/null +++ b/.npmrc @@ -0,0 +1 @@ +save-exact=true From ecb4b2d0b72ce7150571755e7021a1bcd9ca318a Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Mon, 7 Apr 2025 15:04:24 -0700 Subject: [PATCH 06/11] refactor(set-change-password): [Auth/PM-17649] Move and test compareInputs validator (#13553) - Move the `compareInputs` validator to `libs/auth` - Add tests for the `compareInputs` validator - Delete the deprecated `InputsFieldMatch` class (inputs-field-match.validator.ts) --- .../inputs-field-match.validator.ts | 167 -------------- libs/auth/src/angular/index.ts | 3 + .../input-password.component.ts | 22 +- .../compare-inputs.validator.spec.ts | 218 ++++++++++++++++++ .../validators/compare-inputs.validator.ts | 134 +++++++++++ 5 files changed, 365 insertions(+), 179 deletions(-) delete mode 100644 libs/angular/src/auth/validators/inputs-field-match.validator.ts create mode 100644 libs/auth/src/angular/validators/compare-inputs.validator.spec.ts create mode 100644 libs/auth/src/angular/validators/compare-inputs.validator.ts diff --git a/libs/angular/src/auth/validators/inputs-field-match.validator.ts b/libs/angular/src/auth/validators/inputs-field-match.validator.ts deleted file mode 100644 index 5f3acd73bc6..00000000000 --- a/libs/angular/src/auth/validators/inputs-field-match.validator.ts +++ /dev/null @@ -1,167 +0,0 @@ -import { AbstractControl, UntypedFormGroup, ValidationErrors, ValidatorFn } from "@angular/forms"; - -import { FormGroupControls } from "../../platform/abstractions/form-validation-errors.service"; - -export class InputsFieldMatch { - /** - * Check to ensure two fields do not have the same value - * - * @deprecated Use compareInputs() instead - */ - static validateInputsDoesntMatch(matchTo: string, errorMessage: string): ValidatorFn { - return (control: AbstractControl) => { - if (control.parent && control.parent.controls) { - return control?.value === (control?.parent?.controls as FormGroupControls)[matchTo].value - ? { - inputsMatchError: { - message: errorMessage, - }, - } - : null; - } - - return null; - }; - } - - //check to ensure two fields have the same value - static validateInputsMatch(matchTo: string, errorMessage: string): ValidatorFn { - return (control: AbstractControl) => { - if (control.parent && control.parent.controls) { - return control?.value === (control?.parent?.controls as FormGroupControls)[matchTo].value - ? null - : { - inputsDoesntMatchError: { - message: errorMessage, - }, - }; - } - - return null; - }; - } - - /** - * Checks the formGroup if two fields have the same value and validation is controlled from either field - * - * @deprecated - * Use compareInputs() instead. - * - * For more info on deprecation - * - Do not use untyped `options` object in formBuilder.group() {@link https://angular.dev/api/forms/UntypedFormBuilder} - * - Use formBuilder.group() overload with AbstractControlOptions type instead {@link https://angular.dev/api/forms/AbstractControlOptions} - * - * Remove this method after deprecated instances are replaced - */ - static validateFormInputsMatch(field: string, fieldMatchTo: string, errorMessage: string) { - return (formGroup: UntypedFormGroup) => { - const fieldCtrl = formGroup.controls[field]; - const fieldMatchToCtrl = formGroup.controls[fieldMatchTo]; - - if (fieldCtrl.value !== fieldMatchToCtrl.value) { - fieldMatchToCtrl.setErrors({ - inputsDoesntMatchError: { - message: errorMessage, - }, - }); - } else { - fieldMatchToCtrl.setErrors(null); - } - }; - } - - /** - * Checks whether two form controls do or do not have the same input value (except for empty string values). - * - * - Validation is controlled from either form control. - * - The error message is displayed under controlB by default, but can be set to controlA. - * - * @param validationGoal Whether you want to verify that the form control input values match or do not match - * @param controlNameA The name of the first form control to compare. - * @param controlNameB The name of the second form control to compare. - * @param errorMessage The error message to display if there is an error. This will probably - * be an i18n translated string. - * @param showErrorOn The control under which you want to display the error (default is controlB). - */ - static compareInputs( - validationGoal: "match" | "doNotMatch", - controlNameA: string, - controlNameB: string, - errorMessage: string, - showErrorOn: "controlA" | "controlB" = "controlB", - ): ValidatorFn { - return (control: AbstractControl): ValidationErrors | null => { - const controlA = control.get(controlNameA); - const controlB = control.get(controlNameB); - - if (!controlA || !controlB) { - return null; - } - - const controlThatShowsError = showErrorOn === "controlA" ? controlA : controlB; - - // Don't compare empty strings - if (controlA.value === "" && controlB.value === "") { - return pass(); - } - - const controlValuesMatch = controlA.value === controlB.value; - - if (validationGoal === "match") { - if (controlValuesMatch) { - return pass(); - } else { - return fail(); - } - } - - if (validationGoal === "doNotMatch") { - if (!controlValuesMatch) { - return pass(); - } else { - return fail(); - } - } - - return null; // default return - - function fail() { - controlThatShowsError.setErrors({ - // Preserve any pre-existing errors - ...controlThatShowsError.errors, - // Add new inputMatchError - inputMatchError: { - message: errorMessage, - }, - }); - - return { - inputMatchError: { - message: errorMessage, - }, - }; - } - - function pass(): null { - // Get the current errors object - const errorsObj = controlThatShowsError?.errors; - - if (errorsObj != null) { - // Remove any inputMatchError if it exists, since that is the sole error we are targeting with this validator - if (errorsObj?.inputMatchError) { - delete errorsObj.inputMatchError; - } - - // Check if the errorsObj is now empty - const isEmptyObj = Object.keys(errorsObj).length === 0; - - // If the errorsObj is empty, set errors to null, otherwise set the errors to an object of pre-existing errors (other than inputMatchError) - controlThatShowsError.setErrors(isEmptyObj ? null : errorsObj); - } - - // Return null for this validator - return null; - } - }; - } -} diff --git a/libs/auth/src/angular/index.ts b/libs/auth/src/angular/index.ts index bb2956b7569..91d34a34838 100644 --- a/libs/auth/src/angular/index.ts +++ b/libs/auth/src/angular/index.ts @@ -77,3 +77,6 @@ export * from "./two-factor-auth"; // device verification export * from "./new-device-verification/new-device-verification.component"; + +// validators +export * from "./validators/compare-inputs.validator"; diff --git a/libs/auth/src/angular/input-password/input-password.component.ts b/libs/auth/src/angular/input-password/input-password.component.ts index 2f8e5d5b01d..bc7f4121fbe 100644 --- a/libs/auth/src/angular/input-password/input-password.component.ts +++ b/libs/auth/src/angular/input-password/input-password.component.ts @@ -25,13 +25,11 @@ import { } from "@bitwarden/components"; import { DEFAULT_KDF_CONFIG, KeyService } from "@bitwarden/key-management"; -// FIXME: remove `src` and fix import -// eslint-disable-next-line no-restricted-imports -import { InputsFieldMatch } from "../../../../angular/src/auth/validators/inputs-field-match.validator"; // FIXME: remove `src` and fix import // eslint-disable-next-line no-restricted-imports import { SharedModule } from "../../../../components/src/shared"; import { PasswordCalloutComponent } from "../password-callout/password-callout.component"; +import { compareInputs, ValidationGoal } from "../validators/compare-inputs.validator"; import { PasswordInputResult } from "./password-input-result"; @@ -113,21 +111,21 @@ export class InputPasswordComponent implements OnInit { }, { validators: [ - InputsFieldMatch.compareInputs( - "doNotMatch", + compareInputs( + ValidationGoal.InputsShouldNotMatch, "currentPassword", "newPassword", this.i18nService.t("yourNewPasswordCannotBeTheSameAsYourCurrentPassword"), ), - InputsFieldMatch.compareInputs( - "match", - "newPassword", - "confirmNewPassword", + compareInputs( + ValidationGoal.InputsShouldMatch, + "password", + "confirmedPassword", this.i18nService.t("masterPassDoesntMatch"), ), - InputsFieldMatch.compareInputs( - "doNotMatch", - "newPassword", + compareInputs( + ValidationGoal.InputsShouldNotMatch, + "password", "hint", this.i18nService.t("hintEqualsPassword"), ), diff --git a/libs/auth/src/angular/validators/compare-inputs.validator.spec.ts b/libs/auth/src/angular/validators/compare-inputs.validator.spec.ts new file mode 100644 index 00000000000..b9ce8c1590a --- /dev/null +++ b/libs/auth/src/angular/validators/compare-inputs.validator.spec.ts @@ -0,0 +1,218 @@ +import { FormControl, FormGroup, ValidationErrors } from "@angular/forms"; + +import { compareInputs, ValidationGoal } from "./compare-inputs.validator"; + +describe("compareInputs", () => { + let validationErrorsObj: ValidationErrors; + + beforeEach(() => { + // Use a fresh object for each test so that a mutation in one test doesn't affect another test + validationErrorsObj = { + compareInputsError: { + message: "Custom error message", + }, + }; + }); + + it("should throw an error if compareInputs is not being applied to a FormGroup", () => { + const notAFormGroup = new FormControl("form-control"); + + const validatorFn = compareInputs( + ValidationGoal.InputsShouldMatch, + "ctrlA", + "ctrlB", + "Custom error message", + ); + + // Assert + expect(() => validatorFn(notAFormGroup)).toThrow( + "compareInputs only supports validation at the FormGroup level", + ); + }); + + it("should return null if either control is not found", () => { + // Arrange + const formGroup = new FormGroup({ + ctrlA: new FormControl("content"), + }); + + // Act + const validatorFn = compareInputs( + ValidationGoal.InputsShouldMatch, + "ctrlA", + "ctrlB", // ctrlB is missing above + "Custom error message", + ); + + const result = validatorFn(formGroup); + + // Assert + expect(result).toBeNull(); + }); + + it("should return null if both controls have empty string values", () => { + // Arrange + const formGroup = new FormGroup({ + ctrlA: new FormControl(""), + ctrlB: new FormControl(""), + }); + + // Act + const validatorFn = compareInputs( + ValidationGoal.InputsShouldMatch, + "ctrlA", + "ctrlB", + "Custom error message", + ); + + const result = validatorFn(formGroup); + + // Assert + expect(result).toBeNull(); + }); + + it("should call setErrors() on ctrlB if validation fails", () => { + // Arrange + const formGroup = new FormGroup({ + ctrlA: new FormControl("apple"), + ctrlB: new FormControl("banana"), + }); + + const ctrlBSetErrorsSpy = jest.spyOn(formGroup.controls.ctrlB, "setErrors"); + + // Act + const validatorFn = compareInputs( + ValidationGoal.InputsShouldMatch, + "ctrlA", + "ctrlB", + "Custom error message", + ); + + validatorFn(formGroup); + + // Assert + expect(ctrlBSetErrorsSpy).toHaveBeenCalledWith(validationErrorsObj); + }); + + it("should call setErrors() on ctrlA if validation fails and 'showErrorOn' is set to 'controlA'", () => { + // Arrange + const formGroup = new FormGroup({ + ctrlA: new FormControl("apple"), + ctrlB: new FormControl("banana"), + }); + + const ctrlASetErrorsSpy = jest.spyOn(formGroup.controls.ctrlA, "setErrors"); + const ctrlBSetErrorsSpy = jest.spyOn(formGroup.controls.ctrlB, "setErrors"); + + // Act + const validatorFn = compareInputs( + ValidationGoal.InputsShouldMatch, + "ctrlA", + "ctrlB", + "Custom error message", + "controlA", + ); + + validatorFn(formGroup); + + // Assert + expect(ctrlASetErrorsSpy).toHaveBeenCalledWith(validationErrorsObj); + expect(ctrlBSetErrorsSpy).not.toHaveBeenCalled(); + }); + + it("should not call setErrors() on ctrlB if validation passes and there is not a pre-existing error on ctrlB", () => { + // Arrange + const formGroup = new FormGroup({ + ctrlA: new FormControl("apple"), + ctrlB: new FormControl("apple"), + }); + + const ctrlBSetErrorsSpy = jest.spyOn(formGroup.controls.ctrlB, "setErrors"); + + // Act + const validatorFn = compareInputs( + ValidationGoal.InputsShouldMatch, + "ctrlA", + "ctrlB", + "Custom error message", + ); + + validatorFn(formGroup); + + // Assert + expect(ctrlBSetErrorsSpy).not.toHaveBeenCalled(); + }); + + it("should call setErrors(null) on ctrlB if validation passes and there is a pre-existing error on ctrlB", () => { + // Arrange + const formGroup = new FormGroup({ + ctrlA: new FormControl("apple"), + ctrlB: new FormControl("apple"), + }); + + const ctrlBSetErrorsSpy = jest.spyOn(formGroup.controls.ctrlB, "setErrors"); + + formGroup.controls.ctrlB.setErrors(validationErrorsObj); // the pre-existing error + + // Act + const validatorFn = compareInputs( + ValidationGoal.InputsShouldMatch, + "ctrlA", + "ctrlB", + "Custom error message", + ); + + validatorFn(formGroup); + + // Assert + expect(ctrlBSetErrorsSpy).toHaveBeenCalledWith(null); + }); + + const cases = [ + { + expected: null, + goal: ValidationGoal.InputsShouldMatch, + matchStatus: "match", + values: { ctrlA: "apple", ctrlB: "apple" }, + }, + { + expected: "a ValidationErrors object", + goal: ValidationGoal.InputsShouldMatch, + matchStatus: "do not match", + values: { ctrlA: "apple", ctrlB: "banana" }, + }, + { + expected: null, + goal: ValidationGoal.InputsShouldNotMatch, + matchStatus: "do not match", + values: { ctrlA: "apple", ctrlB: "banana" }, + }, + { + expected: "a ValidationErrors object", + goal: ValidationGoal.InputsShouldNotMatch, + matchStatus: "match", + values: { ctrlA: "apple", ctrlB: "apple" }, + }, + ]; + + cases.forEach(({ goal, expected, matchStatus, values }) => { + const goalString = + goal === ValidationGoal.InputsShouldMatch ? "InputsShouldMatch" : "InputsShouldNotMatch"; + + it(`should return ${expected} if the goal is ${goalString} and the inputs ${matchStatus}`, () => { + // Arrange + const formGroup = new FormGroup({ + ctrlA: new FormControl(values.ctrlA), + ctrlB: new FormControl(values.ctrlB), + }); + + // Act + const validatorFn = compareInputs(goal, "ctrlA", "ctrlB", "Custom error message"); + + const result = validatorFn(formGroup); + + // Assert + expect(result).toEqual(expected === null ? null : validationErrorsObj); + }); + }); +}); diff --git a/libs/auth/src/angular/validators/compare-inputs.validator.ts b/libs/auth/src/angular/validators/compare-inputs.validator.ts new file mode 100644 index 00000000000..cb31ac664f5 --- /dev/null +++ b/libs/auth/src/angular/validators/compare-inputs.validator.ts @@ -0,0 +1,134 @@ +import { AbstractControl, FormGroup, ValidationErrors, ValidatorFn } from "@angular/forms"; + +export enum ValidationGoal { + InputsShouldMatch, + InputsShouldNotMatch, +} + +/** + * A cross-field validator that evaluates whether two form controls do or do + * not have the same input value (except for empty string values). This validator + * gets added to the entire FormGroup, not to an individual FormControl, like so: + * + * > ``` + * > formGroup = new FormGroup({ + * > password: new FormControl(), + * > confirmPassword: new FormControl(), + * > }, + * > { + * > validators: compareInputs(...), + * > }); + * > ``` + * + * Notes: + * - Validation is controlled from either form control. + * - The error message is displayed under controlB by default, but can be set to controlA. + * - For more info on custom validators and cross-field validation: + * - https://v18.angular.dev/guide/forms/form-validation#defining-custom-validators + * - https://v18.angular.dev/guide/forms/form-validation#cross-field-validation + * + * @param validationGoal Whether you want to verify that the form controls do or do not have matching input values. + * @param controlNameA The name of the first form control to compare. + * @param controlNameB The name of the second form control to compare. + * @param errorMessage The error message to display if there is an error. This will probably + * be an i18n translated string. + * @param showErrorOn The control under which you want to display the error (default is controlB). + * + * @returns A validator function that can be used on a FormGroup. + */ +export function compareInputs( + validationGoal: ValidationGoal, + controlNameA: string, + controlNameB: string, + errorMessage: string, + showErrorOn: "controlA" | "controlB" = "controlB", +): ValidatorFn { + /** + * Documentation for the inner ValidatorFn that gets returned: + * + * @param formGroup The AbstractControl that we want to perform validation on. In this case we + * perform validation on the FormGroup, which is a subclass of AbstractControl. + * The reason we validate at the FormGroup level and not at the FormControl level + * is because we want to compare two child FormControls in a single validator, so + * we use the FormGroup as the common ancestor. + * + * @returns A ValidationErrors object if the validation fails, or null if the validation passes. + */ + return (formGroup: AbstractControl): ValidationErrors | null => { + if (!(formGroup instanceof FormGroup)) { + throw new Error("compareInputs only supports validation at the FormGroup level"); + } + + const controlA = formGroup.get(controlNameA); + const controlB = formGroup.get(controlNameB); + + if (!controlA || !controlB) { + return null; + } + + const controlThatShowsError = showErrorOn === "controlA" ? controlA : controlB; + + // Don't compare empty strings + if (controlA.value === "" && controlB.value === "") { + return pass(); + } + + const controlValuesMatch = controlA.value === controlB.value; + + if (validationGoal === ValidationGoal.InputsShouldMatch) { + if (controlValuesMatch) { + return pass(); + } else { + return fail(); + } + } + + if (validationGoal === ValidationGoal.InputsShouldNotMatch) { + if (!controlValuesMatch) { + return pass(); + } else { + return fail(); + } + } + + return null; // default return + + function fail() { + controlThatShowsError.setErrors({ + // Preserve any pre-existing errors + ...(controlThatShowsError.errors || {}), + // Add new compareInputsError + compareInputsError: { + message: errorMessage, + }, + }); + + return { + compareInputsError: { + message: errorMessage, + }, + }; + } + + function pass(): null { + // Get the current errors object + const errorsObj = controlThatShowsError?.errors; + + if (errorsObj != null) { + // Remove any compareInputsError if it exists, since that is the sole error we are targeting with this validator + if (errorsObj?.compareInputsError) { + delete errorsObj.compareInputsError; + } + + // Check if the errorsObj is now empty + const isEmptyObj = Object.keys(errorsObj).length === 0; + + // If the errorsObj is empty, set errors to null, otherwise set the errors to an object of pre-existing errors (other than compareInputsError) + controlThatShowsError.setErrors(isEmptyObj ? null : errorsObj); + } + + // Return null for this validator + return null; + } + }; +} From 81350a2ee10d297a5cbd9faf9a56e8b202b7b84c Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Mon, 7 Apr 2025 18:34:35 -0700 Subject: [PATCH 07/11] =?UTF-8?q?Revert=20"refactor(set-change-password):?= =?UTF-8?q?=20[Auth/PM-17649]=20Move=20and=20test=20compareI=E2=80=A6"=20(?= =?UTF-8?q?#14171)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit ecb4b2d0b72ce7150571755e7021a1bcd9ca318a. --- .../inputs-field-match.validator.ts | 167 ++++++++++++++ libs/auth/src/angular/index.ts | 3 - .../input-password.component.ts | 22 +- .../compare-inputs.validator.spec.ts | 218 ------------------ .../validators/compare-inputs.validator.ts | 134 ----------- 5 files changed, 179 insertions(+), 365 deletions(-) create mode 100644 libs/angular/src/auth/validators/inputs-field-match.validator.ts delete mode 100644 libs/auth/src/angular/validators/compare-inputs.validator.spec.ts delete mode 100644 libs/auth/src/angular/validators/compare-inputs.validator.ts diff --git a/libs/angular/src/auth/validators/inputs-field-match.validator.ts b/libs/angular/src/auth/validators/inputs-field-match.validator.ts new file mode 100644 index 00000000000..5f3acd73bc6 --- /dev/null +++ b/libs/angular/src/auth/validators/inputs-field-match.validator.ts @@ -0,0 +1,167 @@ +import { AbstractControl, UntypedFormGroup, ValidationErrors, ValidatorFn } from "@angular/forms"; + +import { FormGroupControls } from "../../platform/abstractions/form-validation-errors.service"; + +export class InputsFieldMatch { + /** + * Check to ensure two fields do not have the same value + * + * @deprecated Use compareInputs() instead + */ + static validateInputsDoesntMatch(matchTo: string, errorMessage: string): ValidatorFn { + return (control: AbstractControl) => { + if (control.parent && control.parent.controls) { + return control?.value === (control?.parent?.controls as FormGroupControls)[matchTo].value + ? { + inputsMatchError: { + message: errorMessage, + }, + } + : null; + } + + return null; + }; + } + + //check to ensure two fields have the same value + static validateInputsMatch(matchTo: string, errorMessage: string): ValidatorFn { + return (control: AbstractControl) => { + if (control.parent && control.parent.controls) { + return control?.value === (control?.parent?.controls as FormGroupControls)[matchTo].value + ? null + : { + inputsDoesntMatchError: { + message: errorMessage, + }, + }; + } + + return null; + }; + } + + /** + * Checks the formGroup if two fields have the same value and validation is controlled from either field + * + * @deprecated + * Use compareInputs() instead. + * + * For more info on deprecation + * - Do not use untyped `options` object in formBuilder.group() {@link https://angular.dev/api/forms/UntypedFormBuilder} + * - Use formBuilder.group() overload with AbstractControlOptions type instead {@link https://angular.dev/api/forms/AbstractControlOptions} + * + * Remove this method after deprecated instances are replaced + */ + static validateFormInputsMatch(field: string, fieldMatchTo: string, errorMessage: string) { + return (formGroup: UntypedFormGroup) => { + const fieldCtrl = formGroup.controls[field]; + const fieldMatchToCtrl = formGroup.controls[fieldMatchTo]; + + if (fieldCtrl.value !== fieldMatchToCtrl.value) { + fieldMatchToCtrl.setErrors({ + inputsDoesntMatchError: { + message: errorMessage, + }, + }); + } else { + fieldMatchToCtrl.setErrors(null); + } + }; + } + + /** + * Checks whether two form controls do or do not have the same input value (except for empty string values). + * + * - Validation is controlled from either form control. + * - The error message is displayed under controlB by default, but can be set to controlA. + * + * @param validationGoal Whether you want to verify that the form control input values match or do not match + * @param controlNameA The name of the first form control to compare. + * @param controlNameB The name of the second form control to compare. + * @param errorMessage The error message to display if there is an error. This will probably + * be an i18n translated string. + * @param showErrorOn The control under which you want to display the error (default is controlB). + */ + static compareInputs( + validationGoal: "match" | "doNotMatch", + controlNameA: string, + controlNameB: string, + errorMessage: string, + showErrorOn: "controlA" | "controlB" = "controlB", + ): ValidatorFn { + return (control: AbstractControl): ValidationErrors | null => { + const controlA = control.get(controlNameA); + const controlB = control.get(controlNameB); + + if (!controlA || !controlB) { + return null; + } + + const controlThatShowsError = showErrorOn === "controlA" ? controlA : controlB; + + // Don't compare empty strings + if (controlA.value === "" && controlB.value === "") { + return pass(); + } + + const controlValuesMatch = controlA.value === controlB.value; + + if (validationGoal === "match") { + if (controlValuesMatch) { + return pass(); + } else { + return fail(); + } + } + + if (validationGoal === "doNotMatch") { + if (!controlValuesMatch) { + return pass(); + } else { + return fail(); + } + } + + return null; // default return + + function fail() { + controlThatShowsError.setErrors({ + // Preserve any pre-existing errors + ...controlThatShowsError.errors, + // Add new inputMatchError + inputMatchError: { + message: errorMessage, + }, + }); + + return { + inputMatchError: { + message: errorMessage, + }, + }; + } + + function pass(): null { + // Get the current errors object + const errorsObj = controlThatShowsError?.errors; + + if (errorsObj != null) { + // Remove any inputMatchError if it exists, since that is the sole error we are targeting with this validator + if (errorsObj?.inputMatchError) { + delete errorsObj.inputMatchError; + } + + // Check if the errorsObj is now empty + const isEmptyObj = Object.keys(errorsObj).length === 0; + + // If the errorsObj is empty, set errors to null, otherwise set the errors to an object of pre-existing errors (other than inputMatchError) + controlThatShowsError.setErrors(isEmptyObj ? null : errorsObj); + } + + // Return null for this validator + return null; + } + }; + } +} diff --git a/libs/auth/src/angular/index.ts b/libs/auth/src/angular/index.ts index 91d34a34838..bb2956b7569 100644 --- a/libs/auth/src/angular/index.ts +++ b/libs/auth/src/angular/index.ts @@ -77,6 +77,3 @@ export * from "./two-factor-auth"; // device verification export * from "./new-device-verification/new-device-verification.component"; - -// validators -export * from "./validators/compare-inputs.validator"; diff --git a/libs/auth/src/angular/input-password/input-password.component.ts b/libs/auth/src/angular/input-password/input-password.component.ts index bc7f4121fbe..2f8e5d5b01d 100644 --- a/libs/auth/src/angular/input-password/input-password.component.ts +++ b/libs/auth/src/angular/input-password/input-password.component.ts @@ -25,11 +25,13 @@ import { } from "@bitwarden/components"; import { DEFAULT_KDF_CONFIG, KeyService } from "@bitwarden/key-management"; +// FIXME: remove `src` and fix import +// eslint-disable-next-line no-restricted-imports +import { InputsFieldMatch } from "../../../../angular/src/auth/validators/inputs-field-match.validator"; // FIXME: remove `src` and fix import // eslint-disable-next-line no-restricted-imports import { SharedModule } from "../../../../components/src/shared"; import { PasswordCalloutComponent } from "../password-callout/password-callout.component"; -import { compareInputs, ValidationGoal } from "../validators/compare-inputs.validator"; import { PasswordInputResult } from "./password-input-result"; @@ -111,21 +113,21 @@ export class InputPasswordComponent implements OnInit { }, { validators: [ - compareInputs( - ValidationGoal.InputsShouldNotMatch, + InputsFieldMatch.compareInputs( + "doNotMatch", "currentPassword", "newPassword", this.i18nService.t("yourNewPasswordCannotBeTheSameAsYourCurrentPassword"), ), - compareInputs( - ValidationGoal.InputsShouldMatch, - "password", - "confirmedPassword", + InputsFieldMatch.compareInputs( + "match", + "newPassword", + "confirmNewPassword", this.i18nService.t("masterPassDoesntMatch"), ), - compareInputs( - ValidationGoal.InputsShouldNotMatch, - "password", + InputsFieldMatch.compareInputs( + "doNotMatch", + "newPassword", "hint", this.i18nService.t("hintEqualsPassword"), ), diff --git a/libs/auth/src/angular/validators/compare-inputs.validator.spec.ts b/libs/auth/src/angular/validators/compare-inputs.validator.spec.ts deleted file mode 100644 index b9ce8c1590a..00000000000 --- a/libs/auth/src/angular/validators/compare-inputs.validator.spec.ts +++ /dev/null @@ -1,218 +0,0 @@ -import { FormControl, FormGroup, ValidationErrors } from "@angular/forms"; - -import { compareInputs, ValidationGoal } from "./compare-inputs.validator"; - -describe("compareInputs", () => { - let validationErrorsObj: ValidationErrors; - - beforeEach(() => { - // Use a fresh object for each test so that a mutation in one test doesn't affect another test - validationErrorsObj = { - compareInputsError: { - message: "Custom error message", - }, - }; - }); - - it("should throw an error if compareInputs is not being applied to a FormGroup", () => { - const notAFormGroup = new FormControl("form-control"); - - const validatorFn = compareInputs( - ValidationGoal.InputsShouldMatch, - "ctrlA", - "ctrlB", - "Custom error message", - ); - - // Assert - expect(() => validatorFn(notAFormGroup)).toThrow( - "compareInputs only supports validation at the FormGroup level", - ); - }); - - it("should return null if either control is not found", () => { - // Arrange - const formGroup = new FormGroup({ - ctrlA: new FormControl("content"), - }); - - // Act - const validatorFn = compareInputs( - ValidationGoal.InputsShouldMatch, - "ctrlA", - "ctrlB", // ctrlB is missing above - "Custom error message", - ); - - const result = validatorFn(formGroup); - - // Assert - expect(result).toBeNull(); - }); - - it("should return null if both controls have empty string values", () => { - // Arrange - const formGroup = new FormGroup({ - ctrlA: new FormControl(""), - ctrlB: new FormControl(""), - }); - - // Act - const validatorFn = compareInputs( - ValidationGoal.InputsShouldMatch, - "ctrlA", - "ctrlB", - "Custom error message", - ); - - const result = validatorFn(formGroup); - - // Assert - expect(result).toBeNull(); - }); - - it("should call setErrors() on ctrlB if validation fails", () => { - // Arrange - const formGroup = new FormGroup({ - ctrlA: new FormControl("apple"), - ctrlB: new FormControl("banana"), - }); - - const ctrlBSetErrorsSpy = jest.spyOn(formGroup.controls.ctrlB, "setErrors"); - - // Act - const validatorFn = compareInputs( - ValidationGoal.InputsShouldMatch, - "ctrlA", - "ctrlB", - "Custom error message", - ); - - validatorFn(formGroup); - - // Assert - expect(ctrlBSetErrorsSpy).toHaveBeenCalledWith(validationErrorsObj); - }); - - it("should call setErrors() on ctrlA if validation fails and 'showErrorOn' is set to 'controlA'", () => { - // Arrange - const formGroup = new FormGroup({ - ctrlA: new FormControl("apple"), - ctrlB: new FormControl("banana"), - }); - - const ctrlASetErrorsSpy = jest.spyOn(formGroup.controls.ctrlA, "setErrors"); - const ctrlBSetErrorsSpy = jest.spyOn(formGroup.controls.ctrlB, "setErrors"); - - // Act - const validatorFn = compareInputs( - ValidationGoal.InputsShouldMatch, - "ctrlA", - "ctrlB", - "Custom error message", - "controlA", - ); - - validatorFn(formGroup); - - // Assert - expect(ctrlASetErrorsSpy).toHaveBeenCalledWith(validationErrorsObj); - expect(ctrlBSetErrorsSpy).not.toHaveBeenCalled(); - }); - - it("should not call setErrors() on ctrlB if validation passes and there is not a pre-existing error on ctrlB", () => { - // Arrange - const formGroup = new FormGroup({ - ctrlA: new FormControl("apple"), - ctrlB: new FormControl("apple"), - }); - - const ctrlBSetErrorsSpy = jest.spyOn(formGroup.controls.ctrlB, "setErrors"); - - // Act - const validatorFn = compareInputs( - ValidationGoal.InputsShouldMatch, - "ctrlA", - "ctrlB", - "Custom error message", - ); - - validatorFn(formGroup); - - // Assert - expect(ctrlBSetErrorsSpy).not.toHaveBeenCalled(); - }); - - it("should call setErrors(null) on ctrlB if validation passes and there is a pre-existing error on ctrlB", () => { - // Arrange - const formGroup = new FormGroup({ - ctrlA: new FormControl("apple"), - ctrlB: new FormControl("apple"), - }); - - const ctrlBSetErrorsSpy = jest.spyOn(formGroup.controls.ctrlB, "setErrors"); - - formGroup.controls.ctrlB.setErrors(validationErrorsObj); // the pre-existing error - - // Act - const validatorFn = compareInputs( - ValidationGoal.InputsShouldMatch, - "ctrlA", - "ctrlB", - "Custom error message", - ); - - validatorFn(formGroup); - - // Assert - expect(ctrlBSetErrorsSpy).toHaveBeenCalledWith(null); - }); - - const cases = [ - { - expected: null, - goal: ValidationGoal.InputsShouldMatch, - matchStatus: "match", - values: { ctrlA: "apple", ctrlB: "apple" }, - }, - { - expected: "a ValidationErrors object", - goal: ValidationGoal.InputsShouldMatch, - matchStatus: "do not match", - values: { ctrlA: "apple", ctrlB: "banana" }, - }, - { - expected: null, - goal: ValidationGoal.InputsShouldNotMatch, - matchStatus: "do not match", - values: { ctrlA: "apple", ctrlB: "banana" }, - }, - { - expected: "a ValidationErrors object", - goal: ValidationGoal.InputsShouldNotMatch, - matchStatus: "match", - values: { ctrlA: "apple", ctrlB: "apple" }, - }, - ]; - - cases.forEach(({ goal, expected, matchStatus, values }) => { - const goalString = - goal === ValidationGoal.InputsShouldMatch ? "InputsShouldMatch" : "InputsShouldNotMatch"; - - it(`should return ${expected} if the goal is ${goalString} and the inputs ${matchStatus}`, () => { - // Arrange - const formGroup = new FormGroup({ - ctrlA: new FormControl(values.ctrlA), - ctrlB: new FormControl(values.ctrlB), - }); - - // Act - const validatorFn = compareInputs(goal, "ctrlA", "ctrlB", "Custom error message"); - - const result = validatorFn(formGroup); - - // Assert - expect(result).toEqual(expected === null ? null : validationErrorsObj); - }); - }); -}); diff --git a/libs/auth/src/angular/validators/compare-inputs.validator.ts b/libs/auth/src/angular/validators/compare-inputs.validator.ts deleted file mode 100644 index cb31ac664f5..00000000000 --- a/libs/auth/src/angular/validators/compare-inputs.validator.ts +++ /dev/null @@ -1,134 +0,0 @@ -import { AbstractControl, FormGroup, ValidationErrors, ValidatorFn } from "@angular/forms"; - -export enum ValidationGoal { - InputsShouldMatch, - InputsShouldNotMatch, -} - -/** - * A cross-field validator that evaluates whether two form controls do or do - * not have the same input value (except for empty string values). This validator - * gets added to the entire FormGroup, not to an individual FormControl, like so: - * - * > ``` - * > formGroup = new FormGroup({ - * > password: new FormControl(), - * > confirmPassword: new FormControl(), - * > }, - * > { - * > validators: compareInputs(...), - * > }); - * > ``` - * - * Notes: - * - Validation is controlled from either form control. - * - The error message is displayed under controlB by default, but can be set to controlA. - * - For more info on custom validators and cross-field validation: - * - https://v18.angular.dev/guide/forms/form-validation#defining-custom-validators - * - https://v18.angular.dev/guide/forms/form-validation#cross-field-validation - * - * @param validationGoal Whether you want to verify that the form controls do or do not have matching input values. - * @param controlNameA The name of the first form control to compare. - * @param controlNameB The name of the second form control to compare. - * @param errorMessage The error message to display if there is an error. This will probably - * be an i18n translated string. - * @param showErrorOn The control under which you want to display the error (default is controlB). - * - * @returns A validator function that can be used on a FormGroup. - */ -export function compareInputs( - validationGoal: ValidationGoal, - controlNameA: string, - controlNameB: string, - errorMessage: string, - showErrorOn: "controlA" | "controlB" = "controlB", -): ValidatorFn { - /** - * Documentation for the inner ValidatorFn that gets returned: - * - * @param formGroup The AbstractControl that we want to perform validation on. In this case we - * perform validation on the FormGroup, which is a subclass of AbstractControl. - * The reason we validate at the FormGroup level and not at the FormControl level - * is because we want to compare two child FormControls in a single validator, so - * we use the FormGroup as the common ancestor. - * - * @returns A ValidationErrors object if the validation fails, or null if the validation passes. - */ - return (formGroup: AbstractControl): ValidationErrors | null => { - if (!(formGroup instanceof FormGroup)) { - throw new Error("compareInputs only supports validation at the FormGroup level"); - } - - const controlA = formGroup.get(controlNameA); - const controlB = formGroup.get(controlNameB); - - if (!controlA || !controlB) { - return null; - } - - const controlThatShowsError = showErrorOn === "controlA" ? controlA : controlB; - - // Don't compare empty strings - if (controlA.value === "" && controlB.value === "") { - return pass(); - } - - const controlValuesMatch = controlA.value === controlB.value; - - if (validationGoal === ValidationGoal.InputsShouldMatch) { - if (controlValuesMatch) { - return pass(); - } else { - return fail(); - } - } - - if (validationGoal === ValidationGoal.InputsShouldNotMatch) { - if (!controlValuesMatch) { - return pass(); - } else { - return fail(); - } - } - - return null; // default return - - function fail() { - controlThatShowsError.setErrors({ - // Preserve any pre-existing errors - ...(controlThatShowsError.errors || {}), - // Add new compareInputsError - compareInputsError: { - message: errorMessage, - }, - }); - - return { - compareInputsError: { - message: errorMessage, - }, - }; - } - - function pass(): null { - // Get the current errors object - const errorsObj = controlThatShowsError?.errors; - - if (errorsObj != null) { - // Remove any compareInputsError if it exists, since that is the sole error we are targeting with this validator - if (errorsObj?.compareInputsError) { - delete errorsObj.compareInputsError; - } - - // Check if the errorsObj is now empty - const isEmptyObj = Object.keys(errorsObj).length === 0; - - // If the errorsObj is empty, set errors to null, otherwise set the errors to an object of pre-existing errors (other than compareInputsError) - controlThatShowsError.setErrors(isEmptyObj ? null : errorsObj); - } - - // Return null for this validator - return null; - } - }; -} From cf0e693caa94dee4c36601e05c530cf8dd262d45 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Tue, 8 Apr 2025 12:42:42 +0200 Subject: [PATCH 08/11] [PM-18697] Add new symmetric key runtime representation and move encrypt service to it (#13578) * Remove AES128CBC-HMAC encryption * Increase test coverage * Refactor symmetric keys and increase test coverage * Re-add type 0 encryption * Fix ts strict warning * Re-add support for encrypt hmac-less aes * Add comment about inner() * Update comment * Deduplicate encryption type check * Undo test changes * Lift out encryption type check to before splitting by encryption type * Change null to undefined * Fix test --- .../encrypt.service.implementation.ts | 246 ++++++++++-------- .../crypto/services/encrypt.service.spec.ts | 21 +- .../models/domain/encrypted-object.ts | 2 - .../domain/symmetric-crypto-key.spec.ts | 48 +++- .../models/domain/symmetric-crypto-key.ts | 115 ++++++-- 5 files changed, 283 insertions(+), 149 deletions(-) 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 7c5bc15a511..901e42d8f89 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 @@ -13,7 +13,11 @@ import { Utils } from "@bitwarden/common/platform/misc/utils"; import { EncArrayBuffer } from "@bitwarden/common/platform/models/domain/enc-array-buffer"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { EncryptedObject } from "@bitwarden/common/platform/models/domain/encrypted-object"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; +import { + Aes256CbcHmacKey, + Aes256CbcKey, + SymmetricCryptoKey, +} from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { ServerConfig } from "../../../platform/abstractions/config/server-config"; import { EncryptService } from "../abstractions/encrypt.service"; @@ -46,11 +50,19 @@ export class EncryptServiceImplementation implements EncryptService { plainBuf = plainValue; } - const encObj = await this.aesEncrypt(plainBuf, key); - const iv = Utils.fromBufferToB64(encObj.iv); - const data = Utils.fromBufferToB64(encObj.data); - const mac = encObj.mac != null ? Utils.fromBufferToB64(encObj.mac) : null; - return new EncString(encObj.key.encType, data, iv, mac); + const innerKey = key.inner(); + if (innerKey.type === EncryptionType.AesCbc256_HmacSha256_B64) { + const encObj = await this.aesEncrypt(plainBuf, innerKey); + const iv = Utils.fromBufferToB64(encObj.iv); + const data = Utils.fromBufferToB64(encObj.data); + const mac = Utils.fromBufferToB64(encObj.mac); + return new EncString(innerKey.type, data, iv, mac); + } else if (innerKey.type === EncryptionType.AesCbc256_B64) { + const encObj = await this.aesEncryptLegacy(plainBuf, innerKey); + const iv = Utils.fromBufferToB64(encObj.iv); + const data = Utils.fromBufferToB64(encObj.data); + return new EncString(innerKey.type, data, iv); + } } async encryptToBytes(plainValue: Uint8Array, key: SymmetricCryptoKey): Promise { @@ -58,21 +70,26 @@ export class EncryptServiceImplementation implements EncryptService { throw new Error("No encryption key provided."); } - const encValue = await this.aesEncrypt(plainValue, key); - let macLen = 0; - if (encValue.mac != null) { - macLen = encValue.mac.byteLength; - } - - const encBytes = new Uint8Array(1 + encValue.iv.byteLength + macLen + encValue.data.byteLength); - encBytes.set([encValue.key.encType]); - encBytes.set(new Uint8Array(encValue.iv), 1); - if (encValue.mac != null) { + const innerKey = key.inner(); + if (innerKey.type === EncryptionType.AesCbc256_HmacSha256_B64) { + const encValue = await this.aesEncrypt(plainValue, innerKey); + const macLen = encValue.mac.length; + const encBytes = new Uint8Array( + 1 + encValue.iv.byteLength + macLen + encValue.data.byteLength, + ); + encBytes.set([innerKey.type]); + encBytes.set(new Uint8Array(encValue.iv), 1); encBytes.set(new Uint8Array(encValue.mac), 1 + encValue.iv.byteLength); + encBytes.set(new Uint8Array(encValue.data), 1 + encValue.iv.byteLength + macLen); + return new EncArrayBuffer(encBytes); + } else if (innerKey.type === EncryptionType.AesCbc256_B64) { + const encValue = await this.aesEncryptLegacy(plainValue, innerKey); + const encBytes = new Uint8Array(1 + encValue.iv.byteLength + encValue.data.byteLength); + encBytes.set([innerKey.type]); + encBytes.set(new Uint8Array(encValue.iv), 1); + encBytes.set(new Uint8Array(encValue.data), 1 + encValue.iv.byteLength); + return new EncArrayBuffer(encBytes); } - - encBytes.set(new Uint8Array(encValue.data), 1 + encValue.iv.byteLength + macLen); - return new EncArrayBuffer(encBytes); } async decryptToUtf8( @@ -84,36 +101,25 @@ export class EncryptServiceImplementation implements EncryptService { throw new Error("No key provided for decryption."); } - // 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( - "[Encrypt service] Key has mac key but payload is missing mac bytes. Key type " + - encryptionTypeName(key.encType) + - "Payload type " + - encryptionTypeName(encString.encryptionType), - "Decrypt context: " + decryptContext, + const innerKey = key.inner(); + if (encString.encryptionType !== innerKey.type) { + this.logDecryptError( + "Key encryption type does not match payload encryption type", + key.encType, + encString.encryptionType, + decryptContext, ); return null; } - if (key.encType !== encString.encryptionType) { - this.logService.error( - "[Encrypt service] Key encryption type does not match payload encryption type. Key type " + - encryptionTypeName(key.encType) + - "Payload type " + - encryptionTypeName(encString.encryptionType), - "Decrypt context: " + decryptContext, + if (innerKey.type === EncryptionType.AesCbc256_HmacSha256_B64) { + const fastParams = this.cryptoFunctionService.aesDecryptFastParameters( + encString.data, + encString.iv, + encString.mac, + key, ); - return null; - } - const fastParams = this.cryptoFunctionService.aesDecryptFastParameters( - encString.data, - encString.iv, - encString.mac, - key, - ); - if (fastParams.macKey != null && fastParams.mac != null) { const computedMac = await this.cryptoFunctionService.hmacFast( fastParams.macData, fastParams.macKey, @@ -122,18 +128,31 @@ export class EncryptServiceImplementation implements EncryptService { const macsEqual = await this.cryptoFunctionService.compareFast(fastParams.mac, computedMac); if (!macsEqual) { this.logMacFailed( - "[Encrypt service] decryptToUtf8 MAC comparison failed. Key or payload has changed. Key type " + - encryptionTypeName(key.encType) + - "Payload type " + - encryptionTypeName(encString.encryptionType) + - " Decrypt context: " + - decryptContext, + "decryptToUtf8 MAC comparison failed. Key or payload has changed.", + key.encType, + encString.encryptionType, + decryptContext, ); return null; } + return await this.cryptoFunctionService.aesDecryptFast({ + mode: "cbc", + parameters: fastParams, + }); + } else if (innerKey.type === EncryptionType.AesCbc256_B64) { + const fastParams = this.cryptoFunctionService.aesDecryptFastParameters( + encString.data, + encString.iv, + undefined, + key, + ); + return await this.cryptoFunctionService.aesDecryptFast({ + mode: "cbc", + parameters: fastParams, + }); + } else { + throw new Error(`Unsupported encryption type`); } - - return await this.cryptoFunctionService.aesDecryptFast({ mode: "cbc", parameters: fastParams }); } async decryptToBytes( @@ -149,72 +168,52 @@ export class EncryptServiceImplementation implements EncryptService { throw new Error("Nothing provided for decryption."); } - // 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( - "[Encrypt service] Key has mac key but payload is missing mac bytes. Key type " + - encryptionTypeName(key.encType) + - " Payload type " + - encryptionTypeName(encThing.encryptionType) + - " Decrypt context: " + - decryptContext, + const inner = key.inner(); + if (encThing.encryptionType !== inner.type) { + this.logDecryptError( + "Encryption key type mismatch", + key.encType, + encThing.encryptionType, + decryptContext, ); return null; } - if (key.encType !== encThing.encryptionType) { - this.logService.error( - "[Encrypt service] Key encryption type does not match payload encryption type. Key type " + - encryptionTypeName(key.encType) + - " Payload type " + - encryptionTypeName(encThing.encryptionType) + - " Decrypt context: " + - decryptContext, - ); - return null; - } + if (inner.type === EncryptionType.AesCbc256_HmacSha256_B64) { + if (encThing.macBytes == null) { + this.logDecryptError("Mac missing", key.encType, encThing.encryptionType, decryptContext); + return null; + } - if (key.macKey != null && encThing.macBytes != null) { const macData = new Uint8Array(encThing.ivBytes.byteLength + encThing.dataBytes.byteLength); macData.set(new Uint8Array(encThing.ivBytes), 0); macData.set(new Uint8Array(encThing.dataBytes), encThing.ivBytes.byteLength); const computedMac = await this.cryptoFunctionService.hmac(macData, key.macKey, "sha256"); - if (computedMac === null) { - this.logMacFailed( - "[Encrypt service#decryptToBytes] Failed to compute MAC." + - " Key type " + - encryptionTypeName(key.encType) + - " Payload type " + - encryptionTypeName(encThing.encryptionType) + - " Decrypt context: " + - decryptContext, - ); - return null; - } - const macsMatch = await this.cryptoFunctionService.compare(encThing.macBytes, computedMac); if (!macsMatch) { this.logMacFailed( - "[Encrypt service#decryptToBytes]: MAC comparison failed. Key or payload has changed." + - " Key type " + - encryptionTypeName(key.encType) + - " Payload type " + - encryptionTypeName(encThing.encryptionType) + - " Decrypt context: " + - decryptContext, + "MAC comparison failed. Key or payload has changed.", + key.encType, + encThing.encryptionType, + decryptContext, ); return null; } + + return await this.cryptoFunctionService.aesDecrypt( + encThing.dataBytes, + encThing.ivBytes, + key.encKey, + "cbc", + ); + } else if (inner.type === EncryptionType.AesCbc256_B64) { + return await this.cryptoFunctionService.aesDecrypt( + encThing.dataBytes, + encThing.ivBytes, + key.encKey, + "cbc", + ); } - - const result = await this.cryptoFunctionService.aesDecrypt( - encThing.dataBytes, - encThing.ivBytes, - key.encKey, - "cbc", - ); - - return result ?? null; } async rsaEncrypt(data: Uint8Array, publicKey: Uint8Array): Promise { @@ -279,25 +278,48 @@ export class EncryptServiceImplementation implements EncryptService { return Utils.fromBufferToB64(hashArray); } - private async aesEncrypt(data: Uint8Array, key: SymmetricCryptoKey): Promise { + private async aesEncrypt(data: Uint8Array, key: Aes256CbcHmacKey): Promise { const obj = new EncryptedObject(); - obj.key = key; obj.iv = await this.cryptoFunctionService.randomBytes(16); - obj.data = await this.cryptoFunctionService.aesEncrypt(data, obj.iv, obj.key.encKey); + obj.data = await this.cryptoFunctionService.aesEncrypt(data, obj.iv, key.encryptionKey); - if (obj.key.macKey != null) { - const macData = new Uint8Array(obj.iv.byteLength + obj.data.byteLength); - macData.set(new Uint8Array(obj.iv), 0); - macData.set(new Uint8Array(obj.data), obj.iv.byteLength); - obj.mac = await this.cryptoFunctionService.hmac(macData, obj.key.macKey, "sha256"); - } + const macData = new Uint8Array(obj.iv.byteLength + obj.data.byteLength); + macData.set(new Uint8Array(obj.iv), 0); + macData.set(new Uint8Array(obj.data), obj.iv.byteLength); + obj.mac = await this.cryptoFunctionService.hmac(macData, key.authenticationKey, "sha256"); return obj; } - private logMacFailed(msg: string) { + /** + * @deprecated Removed once AesCbc256_B64 support is removed + */ + private async aesEncryptLegacy(data: Uint8Array, key: Aes256CbcKey): Promise { + const obj = new EncryptedObject(); + obj.iv = await this.cryptoFunctionService.randomBytes(16); + obj.data = await this.cryptoFunctionService.aesEncrypt(data, obj.iv, key.encryptionKey); + return obj; + } + + private logDecryptError( + msg: string, + keyEncType: EncryptionType, + dataEncType: EncryptionType, + decryptContext: string, + ) { + this.logService.error( + `[Encrypt service] ${msg} Key type ${encryptionTypeName(keyEncType)} Payload type ${encryptionTypeName(dataEncType)} Decrypt context: ${decryptContext}`, + ); + } + + private logMacFailed( + msg: string, + keyEncType: EncryptionType, + dataEncType: EncryptionType, + decryptContext: string, + ) { if (this.logMacFailures) { - this.logService.error(msg); + this.logDecryptError(msg, keyEncType, dataEncType, decryptContext); } } } 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 3ce0d5883d2..7b173ab3eb2 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 @@ -150,7 +150,7 @@ describe("EncryptService", () => { ); }); - it("decrypts data with provided key for Aes256Cbc", async () => { + it("decrypts data with provided key for Aes256CbcHmac", async () => { const decryptedBytes = makeStaticByteArray(10, 200); cryptoFunctionService.hmac.mockResolvedValue(makeStaticByteArray(1)); @@ -257,7 +257,7 @@ describe("EncryptService", () => { ); }); - it("decrypts data with provided key for Aes256Cbc_HmacSha256", async () => { + it("decrypts data with provided key for AesCbc256_HmacSha256", async () => { const key = new SymmetricCryptoKey(makeStaticByteArray(64, 0)); const encString = new EncString(EncryptionType.AesCbc256_HmacSha256_B64, "data", "iv", "mac"); cryptoFunctionService.aesDecryptFastParameters.mockReturnValue({ @@ -277,10 +277,14 @@ describe("EncryptService", () => { ); }); - it("decrypts data with provided key for Aes256Cbc", async () => { + it("decrypts data with provided key for AesCbc256", async () => { const key = new SymmetricCryptoKey(makeStaticByteArray(32, 0)); const encString = new EncString(EncryptionType.AesCbc256_B64, "data"); - cryptoFunctionService.aesDecryptFastParameters.mockReturnValue({} as any); + cryptoFunctionService.aesDecryptFastParameters.mockReturnValue({ + macData: makeStaticByteArray(32, 0), + macKey: makeStaticByteArray(32, 0), + mac: makeStaticByteArray(32, 0), + } as any); cryptoFunctionService.hmacFast.mockResolvedValue(makeStaticByteArray(32, 0)); cryptoFunctionService.compareFast.mockResolvedValue(true); cryptoFunctionService.aesDecryptFast.mockResolvedValue("data"); @@ -290,7 +294,7 @@ describe("EncryptService", () => { expect(cryptoFunctionService.compareFast).not.toHaveBeenCalled(); }); - it("returns null if key is Aes256Cbc_HmacSha256 but EncString is Aes256Cbc", async () => { + it("returns null if key is AesCbc256_HMAC but encstring is AesCbc256", async () => { const key = new SymmetricCryptoKey(makeStaticByteArray(64, 0)); const encString = new EncString(EncryptionType.AesCbc256_B64, "data"); @@ -299,7 +303,7 @@ describe("EncryptService", () => { expect(logService.error).toHaveBeenCalled(); }); - it("returns null if key is Aes256Cbc but encstring is AesCbc256_HmacSha256", async () => { + it("returns null if key is AesCbc256 but encstring is AesCbc256_HMAC", async () => { const key = new SymmetricCryptoKey(makeStaticByteArray(32, 0)); const encString = new EncString(EncryptionType.AesCbc256_HmacSha256_B64, "data", "iv", "mac"); @@ -332,10 +336,7 @@ describe("EncryptService", () => { ); }); 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 key = new SymmetricCryptoKey(makeStaticByteArray(64, 0)); const encString = new EncString(EncryptionType.AesCbc256_B64, "data"); const actual = await encryptService.decryptToUtf8(encString, key); diff --git a/libs/common/src/platform/models/domain/encrypted-object.ts b/libs/common/src/platform/models/domain/encrypted-object.ts index 92153b27636..3caa7ae68d1 100644 --- a/libs/common/src/platform/models/domain/encrypted-object.ts +++ b/libs/common/src/platform/models/domain/encrypted-object.ts @@ -1,10 +1,8 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; export class EncryptedObject { iv: Uint8Array; data: Uint8Array; mac: Uint8Array; - key: SymmetricCryptoKey; } 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 58c902ebab6..cce99b847bb 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 @@ -1,5 +1,6 @@ import { makeStaticByteArray } from "../../../../spec"; import { EncryptionType } from "../../enums"; +import { Utils } from "../../misc/utils"; import { SymmetricCryptoKey } from "./symmetric-crypto-key"; @@ -24,6 +25,11 @@ describe("SymmetricCryptoKey", () => { key: key, keyB64: "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=", macKey: null, + macKeyB64: undefined, + innerKey: { + type: EncryptionType.AesCbc256_B64, + encryptionKey: key, + }, }); }); @@ -40,6 +46,11 @@ describe("SymmetricCryptoKey", () => { "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4vMDEyMzQ1Njc4OTo7PD0+Pw==", macKey: key.slice(32, 64), macKeyB64: "ICEiIyQlJicoKSorLC0uLzAxMjM0NTY3ODk6Ozw9Pj8=", + innerKey: { + type: EncryptionType.AesCbc256_HmacSha256_B64, + encryptionKey: key.slice(0, 32), + authenticationKey: key.slice(32), + }, }); }); @@ -48,7 +59,7 @@ describe("SymmetricCryptoKey", () => { new SymmetricCryptoKey(makeStaticByteArray(30)); }; - expect(t).toThrowError("Unable to determine encType."); + expect(t).toThrowError(`Unsupported encType/key length 30`); }); }); @@ -69,6 +80,41 @@ describe("SymmetricCryptoKey", () => { expect(actual).toBeInstanceOf(SymmetricCryptoKey); }); + it("inner returns inner key", () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(64)); + const actual = key.inner(); + + expect(actual).toEqual({ + type: EncryptionType.AesCbc256_HmacSha256_B64, + encryptionKey: key.encKey, + authenticationKey: key.macKey, + }); + }); + + it("toEncoded returns encoded key for AesCbc256_B64", () => { + const key = new SymmetricCryptoKey(makeStaticByteArray(32)); + const actual = key.toEncoded(); + + expect(actual).toEqual(key.encKey); + }); + + it("toEncoded returns encoded key for AesCbc256_HmacSha256_B64", () => { + const keyBytes = makeStaticByteArray(64); + const key = new SymmetricCryptoKey(keyBytes); + const actual = key.toEncoded(); + + expect(actual).toEqual(keyBytes); + }); + + it("toBase64 returns base64 encoded key", () => { + const keyBytes = makeStaticByteArray(64); + const keyB64 = Utils.fromBufferToB64(keyBytes); + const key = new SymmetricCryptoKey(keyBytes); + const actual = key.toBase64(); + + expect(actual).toEqual(keyB64); + }); + describe("fromString", () => { it("null string returns null", () => { const actual = SymmetricCryptoKey.fromString(null); 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 372b869fd9c..45e15c1f602 100644 --- a/libs/common/src/platform/models/domain/symmetric-crypto-key.ts +++ b/libs/common/src/platform/models/domain/symmetric-crypto-key.ts @@ -5,7 +5,25 @@ import { Jsonify } from "type-fest"; import { Utils } from "../../../platform/misc/utils"; import { EncryptionType } from "../../enums"; +export type Aes256CbcHmacKey = { + type: EncryptionType.AesCbc256_HmacSha256_B64; + encryptionKey: Uint8Array; + authenticationKey: Uint8Array; +}; + +export type Aes256CbcKey = { + type: EncryptionType.AesCbc256_B64; + encryptionKey: Uint8Array; +}; + +/** + * A symmetric crypto key represents a symmetric key usable for symmetric encryption and decryption operations. + * The specific algorithm used is private to the key, and should only be exposed to encrypt service implementations. + * This can be done via `inner()`. + */ export class SymmetricCryptoKey { + private innerKey: Aes256CbcHmacKey | Aes256CbcKey; + key: Uint8Array; encKey: Uint8Array; macKey?: Uint8Array; @@ -17,38 +35,45 @@ export class SymmetricCryptoKey { meta: any; - constructor(key: Uint8Array, encType?: EncryptionType) { + /** + * @param key The key in one of the permitted serialization formats + */ + constructor(key: Uint8Array) { if (key == null) { throw new Error("Must provide key"); } - if (encType == null) { - if (key.byteLength === 32) { - encType = EncryptionType.AesCbc256_B64; - } else if (key.byteLength === 64) { - encType = EncryptionType.AesCbc256_HmacSha256_B64; - } else { - throw new Error("Unable to determine encType."); - } - } + if (key.byteLength === 32) { + this.innerKey = { + type: EncryptionType.AesCbc256_B64, + encryptionKey: key, + }; + this.encType = EncryptionType.AesCbc256_B64; + this.key = key; + this.keyB64 = Utils.fromBufferToB64(this.key); - this.key = key; - this.encType = encType; - - if (encType === EncryptionType.AesCbc256_B64 && key.byteLength === 32) { this.encKey = key; - this.macKey = null; - } else if (encType === EncryptionType.AesCbc256_HmacSha256_B64 && key.byteLength === 64) { - this.encKey = key.slice(0, 32); - this.macKey = key.slice(32, 64); - } else { - throw new Error("Unsupported encType/key length."); - } + this.encKeyB64 = Utils.fromBufferToB64(this.encKey); - this.keyB64 = Utils.fromBufferToB64(this.key); - this.encKeyB64 = Utils.fromBufferToB64(this.encKey); - if (this.macKey != null) { + this.macKey = null; + this.macKeyB64 = undefined; + } else if (key.byteLength === 64) { + this.innerKey = { + type: EncryptionType.AesCbc256_HmacSha256_B64, + encryptionKey: key.slice(0, 32), + authenticationKey: key.slice(32), + }; + this.encType = EncryptionType.AesCbc256_HmacSha256_B64; + this.key = key; + this.keyB64 = Utils.fromBufferToB64(this.key); + + this.encKey = key.slice(0, 32); + this.encKeyB64 = Utils.fromBufferToB64(this.encKey); + + this.macKey = key.slice(32); this.macKeyB64 = Utils.fromBufferToB64(this.macKey); + } else { + throw new Error(`Unsupported encType/key length ${key.byteLength}`); } } @@ -57,6 +82,48 @@ export class SymmetricCryptoKey { return { keyB64: this.keyB64 }; } + /** + * It is preferred not to work with the raw key where possible. + * Only use this method if absolutely necessary. + * + * @returns The inner key instance that can be directly used for encryption primitives + */ + inner(): Aes256CbcHmacKey | Aes256CbcKey { + return this.innerKey; + } + + /** + * @returns The serialized key in base64 format + */ + toBase64(): string { + return Utils.fromBufferToB64(this.toEncoded()); + } + + /** + * Serializes the key to a format that can be written to state or shared + * The currently permitted format is: + * - AesCbc256_B64: 32 bytes (the raw key) + * - AesCbc256_HmacSha256_B64: 64 bytes (32 bytes encryption key, 32 bytes authentication key, concatenated) + * + * @returns The serialized key that can be written to state or encrypted and then written to state / shared + */ + toEncoded(): Uint8Array { + if (this.innerKey.type === EncryptionType.AesCbc256_B64) { + return this.innerKey.encryptionKey; + } else if (this.innerKey.type === EncryptionType.AesCbc256_HmacSha256_B64) { + const encodedKey = new Uint8Array(64); + encodedKey.set(this.innerKey.encryptionKey, 0); + encodedKey.set(this.innerKey.authenticationKey, 32); + return encodedKey; + } else { + throw new Error("Unsupported encryption type."); + } + } + + /** + * @param s The serialized key in base64 format + * @returns A SymmetricCryptoKey instance + */ static fromString(s: string): SymmetricCryptoKey { if (s == null) { return null; From fa0ae2435a1b247571f101123bdf29653ab36efc Mon Sep 17 00:00:00 2001 From: Jared McCannon Date: Tue, 8 Apr 2025 07:17:53 -0500 Subject: [PATCH 09/11] Removed bootstrap classes from setup.component.html (#14166) --- .../admin-console/providers/setup/setup.component.html | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.html b/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.html index 74aa468c42e..38b4c3bc9de 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.html +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.html @@ -1,18 +1,18 @@ {{ "loading" | i18n }} -
-
+
+

{{ "setupProvider" | i18n }}

{{ "setupProviderDesc" | i18n }}

-

{{ "generalInformation" | i18n }}

+

{{ "generalInformation" | i18n }}

From b48825372275c1d33f4ebe60afa8a596f77c6f30 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 8 Apr 2025 14:24:04 +0200 Subject: [PATCH 10/11] [deps]: Update actions/create-github-app-token action to v1.12.0 (#13879) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/crowdin-pull.yml | 2 +- .github/workflows/repository-management.yml | 6 +++--- .github/workflows/version-auto-bump.yml | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/crowdin-pull.yml b/.github/workflows/crowdin-pull.yml index 027a2f11e55..32907699747 100644 --- a/.github/workflows/crowdin-pull.yml +++ b/.github/workflows/crowdin-pull.yml @@ -22,7 +22,7 @@ jobs: crowdin_project_id: "308189" steps: - name: Generate GH App token - uses: actions/create-github-app-token@c1a285145b9d317df6ced56c09f525b5c2b6f755 # v1.11.1 + uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1.12.0 id: app-token with: app-id: ${{ secrets.BW_GHAPP_ID }} diff --git a/.github/workflows/repository-management.yml b/.github/workflows/repository-management.yml index ac2733e765b..06daf70d7c9 100644 --- a/.github/workflows/repository-management.yml +++ b/.github/workflows/repository-management.yml @@ -66,7 +66,7 @@ jobs: runs-on: ubuntu-24.04 steps: - name: Generate GH App token - uses: actions/create-github-app-token@c1a285145b9d317df6ced56c09f525b5c2b6f755 # v1.11.1 + uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1.12.0 id: app-token with: app-id: ${{ secrets.BW_GHAPP_ID }} @@ -115,7 +115,7 @@ jobs: version: ${{ inputs.version_number_override }} - name: Generate GH App token - uses: actions/create-github-app-token@c1a285145b9d317df6ced56c09f525b5c2b6f755 # v1.11.1 + uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1.12.0 id: app-token with: app-id: ${{ secrets.BW_GHAPP_ID }} @@ -452,7 +452,7 @@ jobs: - setup steps: - name: Generate GH App token - uses: actions/create-github-app-token@c1a285145b9d317df6ced56c09f525b5c2b6f755 # v1.11.1 + uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1.12.0 id: app-token with: app-id: ${{ secrets.BW_GHAPP_ID }} diff --git a/.github/workflows/version-auto-bump.yml b/.github/workflows/version-auto-bump.yml index ef46dbc867d..9431eab37c5 100644 --- a/.github/workflows/version-auto-bump.yml +++ b/.github/workflows/version-auto-bump.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-24.04 steps: - name: Generate GH App token - uses: actions/create-github-app-token@c1a285145b9d317df6ced56c09f525b5c2b6f755 # v1.11.1 + uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1.12.0 id: app-token with: app-id: ${{ secrets.BW_GHAPP_ID }} From 772b42f5b53f483a956b3e4fec31b83e54742418 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Tue, 8 Apr 2025 15:06:39 +0200 Subject: [PATCH 11/11] [PM-18039] Add initial verison of IpcServices to client (#13373) * feat: add foreground ipc service * refactor: create abstract ipc service in libs * wip: remove IPC service complexity The code was making some wrong assumptions about how IPC is going to work. I'm removing everything and starting the content-script instead * feat: working message sending from page to background * refactor: move into common * feat: somewhat complete web <-> browser link * wip: ping command from web * fix: import path * fix: wip urls * wip: add console log * feat: successfull message sending (not receiving) * feat: implement IPC using new refactored framework * wip: add some console logs * wip: almost working ping/pong * feat: working ping/pong * chore: clean-up ping/pong and some console logs * chore: remove unused file * fix: override lint rule * chore: remove unused ping message * feat: add tests for message queue * fix: adapt to name changes and modifications to SDK branch * fix: missing import * fix: remove content script from manifest The feature is not ready for prodution code yet. We will add dynamic injection with feature-flag support in a follow-up PR * fix: remove fileless lp * fix: make same changes to manifest v2 * fix: initialization functions Add missing error handling, wait for the SDK to load and properly depend on the log service * feat: use named id field * chore: update sdk version to include IPC changes * fix: remove messages$ buffer * fix: forgot to commit package-lock * feat: add additional destination check * feat: only import type in ipc-message * fix: typing issues * feat: check message origin --- .../browser/src/background/main.background.ts | 7 +++ .../ipc/background-communication-backend.ts | 41 +++++++++++++++ .../ipc/content/ipc-content-script.ts | 51 +++++++++++++++++++ .../platform/ipc/ipc-background.service.ts | 26 ++++++++++ apps/browser/webpack.config.js | 1 + apps/web/src/app/core/core.module.ts | 9 +++- apps/web/src/app/core/init.service.ts | 3 ++ .../ipc/web-communication-provider.ts | 41 +++++++++++++++ .../src/app/platform/ipc/web-ipc.service.ts | 25 +++++++++ libs/common/src/platform/ipc/index.ts | 2 + libs/common/src/platform/ipc/ipc-message.ts | 10 ++++ libs/common/src/platform/ipc/ipc.service.ts | 51 +++++++++++++++++++ .../src/platform/ipc/message-queue.spec.ts | 48 +++++++++++++++++ libs/common/src/platform/ipc/message-queue.ts | 20 ++++++++ package-lock.json | 9 ++-- package.json | 2 +- 16 files changed, 340 insertions(+), 6 deletions(-) create mode 100644 apps/browser/src/platform/ipc/background-communication-backend.ts create mode 100644 apps/browser/src/platform/ipc/content/ipc-content-script.ts create mode 100644 apps/browser/src/platform/ipc/ipc-background.service.ts create mode 100644 apps/web/src/app/platform/ipc/web-communication-provider.ts create mode 100644 apps/web/src/app/platform/ipc/web-ipc.service.ts create mode 100644 libs/common/src/platform/ipc/index.ts create mode 100644 libs/common/src/platform/ipc/ipc-message.ts create mode 100644 libs/common/src/platform/ipc/ipc.service.ts create mode 100644 libs/common/src/platform/ipc/message-queue.spec.ts create mode 100644 libs/common/src/platform/ipc/message-queue.ts diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 663fc863f5e..aeee58c6fdf 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -111,6 +111,7 @@ import { } from "@bitwarden/common/platform/abstractions/storage.service"; import { SystemService as SystemServiceAbstraction } from "@bitwarden/common/platform/abstractions/system.service"; import { StateFactory } from "@bitwarden/common/platform/factories/state-factory"; +import { IpcService } from "@bitwarden/common/platform/ipc"; import { Message, MessageListener, MessageSender } from "@bitwarden/common/platform/messaging"; // eslint-disable-next-line no-restricted-imports -- Used for dependency creation import { SubjectMessageSender } from "@bitwarden/common/platform/messaging/internal"; @@ -259,6 +260,7 @@ import { BackgroundBrowserBiometricsService } from "../key-management/biometrics import VaultTimeoutService from "../key-management/vault-timeout/vault-timeout.service"; import { BrowserApi } from "../platform/browser/browser-api"; import { flagEnabled } from "../platform/flags"; +import { IpcBackgroundService } from "../platform/ipc/ipc-background.service"; import { UpdateBadge } from "../platform/listeners/update-badge"; /* eslint-disable no-restricted-imports */ import { ChromeMessageSender } from "../platform/messaging/chrome-message.sender"; @@ -403,6 +405,8 @@ export default class MainBackground { inlineMenuFieldQualificationService: InlineMenuFieldQualificationService; taskService: TaskService; + ipcService: IpcService; + onUpdatedRan: boolean; onReplacedRan: boolean; loginToAutoFill: CipherView = null; @@ -1309,6 +1313,8 @@ export default class MainBackground { ); this.inlineMenuFieldQualificationService = new InlineMenuFieldQualificationService(); + + this.ipcService = new IpcBackgroundService(this.logService); } async bootstrap() { @@ -1382,6 +1388,7 @@ export default class MainBackground { } await this.initOverlayAndTabsBackground(); + await this.ipcService.init(); return new Promise((resolve) => { setTimeout(async () => { diff --git a/apps/browser/src/platform/ipc/background-communication-backend.ts b/apps/browser/src/platform/ipc/background-communication-backend.ts new file mode 100644 index 00000000000..6c5b374dd56 --- /dev/null +++ b/apps/browser/src/platform/ipc/background-communication-backend.ts @@ -0,0 +1,41 @@ +import { IpcMessage, isIpcMessage } from "@bitwarden/common/platform/ipc"; +import { MessageQueue } from "@bitwarden/common/platform/ipc/message-queue"; +import { CommunicationBackend, IncomingMessage, OutgoingMessage } from "@bitwarden/sdk-internal"; + +import { BrowserApi } from "../browser/browser-api"; + +export class BackgroundCommunicationBackend implements CommunicationBackend { + private queue = new MessageQueue(); + + constructor() { + BrowserApi.messageListener("platform.ipc", (message, sender) => { + if (!isIpcMessage(message)) { + return; + } + + if (sender.tab?.id === undefined || sender.tab.id === chrome.tabs.TAB_ID_NONE) { + // Ignore messages from non-tab sources + return; + } + + void this.queue.enqueue({ ...message.message, source: { Web: { id: sender.tab.id } } }); + }); + } + + async send(message: OutgoingMessage): Promise { + if (typeof message.destination === "object" && "Web" in message.destination) { + await BrowserApi.tabSendMessage( + { id: message.destination.Web.id } as chrome.tabs.Tab, + { type: "bitwarden-ipc-message", message } satisfies IpcMessage, + { frameId: 0 }, + ); + return; + } + + throw new Error("Destination not supported."); + } + + async receive(): Promise { + return this.queue.dequeue(); + } +} diff --git a/apps/browser/src/platform/ipc/content/ipc-content-script.ts b/apps/browser/src/platform/ipc/content/ipc-content-script.ts new file mode 100644 index 00000000000..a21ad07be4b --- /dev/null +++ b/apps/browser/src/platform/ipc/content/ipc-content-script.ts @@ -0,0 +1,51 @@ +// TODO: This content script should be dynamically reloaded when the extension is updated, +// to avoid "Extension context invalidated." errors. + +import { isIpcMessage } from "@bitwarden/common/platform/ipc/ipc-message"; + +// Web -> Background +export function sendExtensionMessage(message: unknown) { + if ( + typeof browser !== "undefined" && + typeof browser.runtime !== "undefined" && + typeof browser.runtime.sendMessage !== "undefined" + ) { + void browser.runtime.sendMessage(message); + return; + } + + void chrome.runtime.sendMessage(message); +} + +window.addEventListener("message", (event) => { + if (event.origin !== window.origin) { + return; + } + + if (isIpcMessage(event.data)) { + sendExtensionMessage(event.data); + } +}); + +// Background -> Web +function setupMessageListener() { + function listener(message: unknown) { + if (isIpcMessage(message)) { + void window.postMessage(message); + } + } + + if ( + typeof browser !== "undefined" && + typeof browser.runtime !== "undefined" && + typeof browser.runtime.onMessage !== "undefined" + ) { + browser.runtime.onMessage.addListener(listener); + return; + } + + // eslint-disable-next-line no-restricted-syntax -- This doesn't run in the popup but in the content script + chrome.runtime.onMessage.addListener(listener); +} + +setupMessageListener(); diff --git a/apps/browser/src/platform/ipc/ipc-background.service.ts b/apps/browser/src/platform/ipc/ipc-background.service.ts new file mode 100644 index 00000000000..a87f6bb4acb --- /dev/null +++ b/apps/browser/src/platform/ipc/ipc-background.service.ts @@ -0,0 +1,26 @@ +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-load.service"; +import { IpcService } from "@bitwarden/common/platform/ipc"; +import { IpcClient } from "@bitwarden/sdk-internal"; + +import { BackgroundCommunicationBackend } from "./background-communication-backend"; + +export class IpcBackgroundService extends IpcService { + private communicationProvider?: BackgroundCommunicationBackend; + + constructor(private logService: LogService) { + super(); + } + + override async init() { + try { + // This function uses classes and functions defined in the SDK, so we need to wait for the SDK to load. + await SdkLoadService.Ready; + this.communicationProvider = new BackgroundCommunicationBackend(); + + await super.initWithClient(new IpcClient(this.communicationProvider)); + } catch (e) { + this.logService.error("[IPC] Initialization failed", e); + } + } +} diff --git a/apps/browser/webpack.config.js b/apps/browser/webpack.config.js index f386a3167ec..09d1133a4df 100644 --- a/apps/browser/webpack.config.js +++ b/apps/browser/webpack.config.js @@ -205,6 +205,7 @@ const mainConfig = { "content/content-message-handler": "./src/autofill/content/content-message-handler.ts", "content/fido2-content-script": "./src/autofill/fido2/content/fido2-content-script.ts", "content/fido2-page-script": "./src/autofill/fido2/content/fido2-page-script.ts", + "content/ipc-content-script": "./src/platform/ipc/content/ipc-content-script.ts", "notification/bar": "./src/autofill/notification/bar.ts", "overlay/menu-button": "./src/autofill/overlay/inline-menu/pages/button/bootstrap-autofill-inline-menu-button.ts", diff --git a/apps/web/src/app/core/core.module.ts b/apps/web/src/app/core/core.module.ts index 9e6f88d18d6..1fa4fd80e52 100644 --- a/apps/web/src/app/core/core.module.ts +++ b/apps/web/src/app/core/core.module.ts @@ -75,6 +75,7 @@ import { SdkClientFactory } from "@bitwarden/common/platform/abstractions/sdk/sd import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-load.service"; import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service"; import { ThemeTypes } from "@bitwarden/common/platform/enums"; +import { IpcService } from "@bitwarden/common/platform/ipc"; // eslint-disable-next-line no-restricted-imports -- Needed for DI import { UnsupportedWebPushConnectionService, @@ -122,9 +123,11 @@ import { WebSsoComponentService } from "../auth/core/services/login/web-sso-comp import { AcceptOrganizationInviteService } from "../auth/organization-invite/accept-organization.service"; import { HtmlStorageService } from "../core/html-storage.service"; import { I18nService } from "../core/i18n.service"; +import { WebFileDownloadService } from "../core/web-file-download.service"; import { WebLockComponentService } from "../key-management/lock/services/web-lock-component.service"; import { WebProcessReloadService } from "../key-management/services/web-process-reload.service"; import { WebBiometricsService } from "../key-management/web-biometric.service"; +import { WebIpcService } from "../platform/ipc/web-ipc.service"; import { WebEnvironmentService } from "../platform/web-environment.service"; import { WebMigrationRunner } from "../platform/web-migration-runner"; import { WebSdkLoadService } from "../platform/web-sdk-load.service"; @@ -135,7 +138,6 @@ import { InitService } from "./init.service"; import { ENV_URLS } from "./injection-tokens"; import { ModalService } from "./modal.service"; import { RouterService } from "./router.service"; -import { WebFileDownloadService } from "./web-file-download.service"; import { WebPlatformUtilsService } from "./web-platform-utils.service"; /** @@ -368,6 +370,11 @@ const safeProviders: SafeProvider[] = [ useClass: WebLoginDecryptionOptionsService, deps: [MessagingService, RouterService, AcceptOrganizationInviteService], }), + safeProvider({ + provide: IpcService, + useClass: WebIpcService, + deps: [], + }), safeProvider({ provide: SshImportPromptService, useClass: DefaultSshImportPromptService, diff --git a/apps/web/src/app/core/init.service.ts b/apps/web/src/app/core/init.service.ts index 24b5631abcc..43547ff5d57 100644 --- a/apps/web/src/app/core/init.service.ts +++ b/apps/web/src/app/core/init.service.ts @@ -14,6 +14,7 @@ import { ConfigService } from "@bitwarden/common/platform/abstractions/config/co import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platform/abstractions/i18n.service"; import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-load.service"; import { StateService as StateServiceAbstraction } from "@bitwarden/common/platform/abstractions/state.service"; +import { IpcService } from "@bitwarden/common/platform/ipc"; import { NotificationsService } from "@bitwarden/common/platform/notifications"; import { ContainerService } from "@bitwarden/common/platform/services/container.service"; import { UserAutoUnlockKeyService } from "@bitwarden/common/platform/services/user-auto-unlock-key.service"; @@ -38,6 +39,7 @@ export class InitService { private userAutoUnlockKeyService: UserAutoUnlockKeyService, private accountService: AccountService, private versionService: VersionService, + private ipcService: IpcService, private sdkLoadService: SdkLoadService, private configService: ConfigService, private bulkEncryptService: BulkEncryptService, @@ -72,6 +74,7 @@ export class InitService { htmlEl.classList.add("locale_" + this.i18nService.translationLocale); this.themingService.applyThemeChangesTo(this.document); this.versionService.applyVersionToWindow(); + void this.ipcService.init(); const containerService = new ContainerService(this.keyService, this.encryptService); containerService.attachToGlobal(this.win); diff --git a/apps/web/src/app/platform/ipc/web-communication-provider.ts b/apps/web/src/app/platform/ipc/web-communication-provider.ts new file mode 100644 index 00000000000..85353ab77af --- /dev/null +++ b/apps/web/src/app/platform/ipc/web-communication-provider.ts @@ -0,0 +1,41 @@ +import { Injectable } from "@angular/core"; + +import { IpcMessage, isIpcMessage } from "@bitwarden/common/platform/ipc"; +import { MessageQueue } from "@bitwarden/common/platform/ipc/message-queue"; +import { CommunicationBackend, IncomingMessage, OutgoingMessage } from "@bitwarden/sdk-internal"; + +@Injectable({ providedIn: "root" }) +export class WebCommunicationProvider implements CommunicationBackend { + private queue = new MessageQueue(); + + constructor() { + window.addEventListener("message", async (event: MessageEvent) => { + if (event.origin !== window.origin) { + return; + } + + const message = event.data; + if (!isIpcMessage(message)) { + return; + } + + await this.queue.enqueue({ ...message.message, source: "BrowserBackground" }); + }); + } + + async send(message: OutgoingMessage): Promise { + if (message.destination === "BrowserBackground") { + window.postMessage( + { type: "bitwarden-ipc-message", message } satisfies IpcMessage, + window.location.origin, + ); + return; + } + + throw new Error(`Destination not supported: ${message.destination}`); + } + + receive(): Promise { + return this.queue.dequeue(); + } +} diff --git a/apps/web/src/app/platform/ipc/web-ipc.service.ts b/apps/web/src/app/platform/ipc/web-ipc.service.ts new file mode 100644 index 00000000000..59a715f31f5 --- /dev/null +++ b/apps/web/src/app/platform/ipc/web-ipc.service.ts @@ -0,0 +1,25 @@ +import { inject } from "@angular/core"; + +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-load.service"; +import { IpcService } from "@bitwarden/common/platform/ipc"; +import { IpcClient } from "@bitwarden/sdk-internal"; + +import { WebCommunicationProvider } from "./web-communication-provider"; + +export class WebIpcService extends IpcService { + private logService = inject(LogService); + private communicationProvider?: WebCommunicationProvider; + + override async init() { + try { + // This function uses classes and functions defined in the SDK, so we need to wait for the SDK to load. + await SdkLoadService.Ready; + this.communicationProvider = new WebCommunicationProvider(); + + await super.initWithClient(new IpcClient(this.communicationProvider)); + } catch (e) { + this.logService.error("[IPC] Initialization failed", e); + } + } +} diff --git a/libs/common/src/platform/ipc/index.ts b/libs/common/src/platform/ipc/index.ts new file mode 100644 index 00000000000..f1acccdddbf --- /dev/null +++ b/libs/common/src/platform/ipc/index.ts @@ -0,0 +1,2 @@ +export * from "./ipc-message"; +export * from "./ipc.service"; diff --git a/libs/common/src/platform/ipc/ipc-message.ts b/libs/common/src/platform/ipc/ipc-message.ts new file mode 100644 index 00000000000..78cce011f49 --- /dev/null +++ b/libs/common/src/platform/ipc/ipc-message.ts @@ -0,0 +1,10 @@ +import type { OutgoingMessage } from "@bitwarden/sdk-internal"; + +export interface IpcMessage { + type: "bitwarden-ipc-message"; + message: OutgoingMessage; +} + +export function isIpcMessage(message: any): message is IpcMessage { + return message.type === "bitwarden-ipc-message"; +} diff --git a/libs/common/src/platform/ipc/ipc.service.ts b/libs/common/src/platform/ipc/ipc.service.ts new file mode 100644 index 00000000000..7d2c7479be7 --- /dev/null +++ b/libs/common/src/platform/ipc/ipc.service.ts @@ -0,0 +1,51 @@ +import { Observable, shareReplay } from "rxjs"; + +import { IpcClient, IncomingMessage, OutgoingMessage } from "@bitwarden/sdk-internal"; + +export abstract class IpcService { + private _client?: IpcClient; + protected get client(): IpcClient { + if (!this._client) { + throw new Error("IpcService not initialized"); + } + return this._client; + } + + private _messages$?: Observable; + protected get messages$(): Observable { + if (!this._messages$) { + throw new Error("IpcService not initialized"); + } + return this._messages$; + } + + abstract init(): Promise; + + protected async initWithClient(client: IpcClient): Promise { + this._client = client; + this._messages$ = new Observable((subscriber) => { + let isSubscribed = true; + + const receiveLoop = async () => { + while (isSubscribed) { + try { + const message = await this.client.receive(); + subscriber.next(message); + } catch (error) { + subscriber.error(error); + break; + } + } + }; + void receiveLoop(); + + return () => { + isSubscribed = false; + }; + }).pipe(shareReplay({ bufferSize: 0, refCount: true })); + } + + async send(message: OutgoingMessage) { + await this.client.send(message); + } +} diff --git a/libs/common/src/platform/ipc/message-queue.spec.ts b/libs/common/src/platform/ipc/message-queue.spec.ts new file mode 100644 index 00000000000..9a9ebec1227 --- /dev/null +++ b/libs/common/src/platform/ipc/message-queue.spec.ts @@ -0,0 +1,48 @@ +import { MessageQueue } from "./message-queue"; + +type Message = symbol; + +describe("MessageQueue", () => { + let messageQueue!: MessageQueue; + + beforeEach(() => { + messageQueue = new MessageQueue(); + }); + + it("waits for a new message when queue is empty", async () => { + const message = createMessage(); + + // Start a promise to dequeue a message + let dequeuedValue: Message | undefined; + void messageQueue.dequeue().then((value) => { + dequeuedValue = value; + }); + + // No message is enqueued yet + expect(dequeuedValue).toBeUndefined(); + + // Enqueue a message + await messageQueue.enqueue(message); + + // Expect the message to be dequeued + await new Promise(process.nextTick); + expect(dequeuedValue).toBe(message); + }); + + it("returns existing message when queue is not empty", async () => { + const message = createMessage(); + + // Enqueue a message + await messageQueue.enqueue(message); + + // Dequeue the message + const dequeuedValue = await messageQueue.dequeue(); + + // Expect the message to be dequeued + expect(dequeuedValue).toBe(message); + }); +}); + +function createMessage(name?: string): symbol { + return Symbol(name); +} diff --git a/libs/common/src/platform/ipc/message-queue.ts b/libs/common/src/platform/ipc/message-queue.ts new file mode 100644 index 00000000000..8ab484141b0 --- /dev/null +++ b/libs/common/src/platform/ipc/message-queue.ts @@ -0,0 +1,20 @@ +import { firstValueFrom, Subject } from "rxjs"; + +export class MessageQueue { + private queue: T[] = []; + private messageAvailable$ = new Subject(); + + async enqueue(message: T): Promise { + this.queue.push(message); + this.messageAvailable$.next(); + } + + async dequeue(): Promise { + if (this.queue.length > 0) { + return this.queue.shift() as T; + } + + await firstValueFrom(this.messageAvailable$); + return this.queue.shift() as T; + } +} diff --git a/package-lock.json b/package-lock.json index 7d5d56b46bd..c880e5f60d7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24,7 +24,7 @@ "@angular/platform-browser": "18.2.13", "@angular/platform-browser-dynamic": "18.2.13", "@angular/router": "18.2.13", - "@bitwarden/sdk-internal": "0.2.0-main.124", + "@bitwarden/sdk-internal": "0.2.0-main.133", "@electron/fuses": "1.8.0", "@emotion/css": "11.13.5", "@koa/multer": "3.0.2", @@ -4699,9 +4699,10 @@ "link": true }, "node_modules/@bitwarden/sdk-internal": { - "version": "0.2.0-main.124", - "resolved": "https://registry.npmjs.org/@bitwarden/sdk-internal/-/sdk-internal-0.2.0-main.124.tgz", - "integrity": "sha512-7F+DlPFng/thT4EVIQk2tRC7kff6G2B7alHAIxBdioJc9vE64Z5R5pviUyMZzqLnA5e9y8EnQdtWsQzUkHxisQ==" + "version": "0.2.0-main.133", + "resolved": "https://registry.npmjs.org/@bitwarden/sdk-internal/-/sdk-internal-0.2.0-main.133.tgz", + "integrity": "sha512-KzKJGf9cKlcQzfRmqkAwVGBN1kDpcRFkTMm7nrphZSrjfaWJWI1lBEJ0DhnkbMMHJXhQavGyoVk5TIn/Y8ylmw==", + "license": "GPL-3.0" }, "node_modules/@bitwarden/send-ui": { "resolved": "libs/tools/send/send-ui", diff --git a/package.json b/package.json index 051ff4f7251..f24588c4c82 100644 --- a/package.json +++ b/package.json @@ -155,7 +155,7 @@ "@angular/platform-browser": "18.2.13", "@angular/platform-browser-dynamic": "18.2.13", "@angular/router": "18.2.13", - "@bitwarden/sdk-internal": "0.2.0-main.124", + "@bitwarden/sdk-internal": "0.2.0-main.133", "@electron/fuses": "1.8.0", "@emotion/css": "11.13.5", "@koa/multer": "3.0.2",