diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 99efec2fbbb..d1266a174e4 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -75,6 +75,7 @@ bitwarden_license/bit-cli/src/admin-console @bitwarden/team-admin-console-dev libs/angular/src/admin-console @bitwarden/team-admin-console-dev libs/common/src/admin-console @bitwarden/team-admin-console-dev libs/admin-console @bitwarden/team-admin-console-dev +libs/auto-confirm @bitwarden/team-admin-console-dev ## Billing team files ## apps/browser/src/billing @bitwarden/team-billing-dev diff --git a/.github/renovate.json5 b/.github/renovate.json5 index b402d01e209..1b6522c94dd 100644 --- a/.github/renovate.json5 +++ b/.github/renovate.json5 @@ -298,6 +298,7 @@ "oidc-client-ts", "papaparse", "utf-8-validate", + "verifysign", "zxcvbn", ], description: "Tools owned dependencies", diff --git a/.github/workflows/build-browser.yml b/.github/workflows/build-browser.yml index b5859516eaa..7614fdba396 100644 --- a/.github/workflows/build-browser.yml +++ b/.github/workflows/build-browser.yml @@ -193,7 +193,7 @@ jobs: zip -r browser-source.zip browser-source - name: Upload browser source - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: ${{matrix.license_type.archive_name_prefix}}browser-source-${{ env._BUILD_NUMBER }}.zip path: browser-source.zip @@ -272,7 +272,7 @@ jobs: npm --version - name: Download browser source - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: name: ${{matrix.license_type.source_archive_name_prefix}}browser-source-${{ env._BUILD_NUMBER }}.zip @@ -336,7 +336,7 @@ jobs: working-directory: browser-source/apps/browser - name: Upload extension artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: ${{ matrix.license_type.artifact_prefix }}${{ matrix.browser.artifact_name }}-${{ env._BUILD_NUMBER }}.zip path: browser-source/apps/browser/dist/${{matrix.license_type.archive_name_prefix}}${{ matrix.browser.archive_name }} @@ -349,7 +349,7 @@ jobs: - name: Upload dev extension artifact if: ${{ matrix.browser.archive_name_dev != '' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: ${{ matrix.license_type.artifact_prefix }}${{ matrix.browser.artifact_name_dev }}-${{ env._BUILD_NUMBER }}.zip path: browser-source/apps/browser/dist/${{matrix.license_type.archive_name_prefix}}${{ matrix.browser.archive_name_dev }} @@ -523,7 +523,7 @@ jobs: ls -la - name: Upload Safari artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: ${{matrix.license_type.archive_name_prefix}}dist-safari-${{ env._BUILD_NUMBER }}.zip path: apps/browser/dist/${{matrix.license_type.archive_name_prefix}}dist-safari.zip diff --git a/.github/workflows/build-cli.yml b/.github/workflows/build-cli.yml index 704a9810b27..d0abe8e12e7 100644 --- a/.github/workflows/build-cli.yml +++ b/.github/workflows/build-cli.yml @@ -268,7 +268,7 @@ jobs: fi - name: Upload unix zip asset - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bw${{ matrix.license_type.artifact_prefix }}-${{ env.LOWER_RUNNER_OS }}${{ matrix.os.target_suffix }}-${{ env._PACKAGE_VERSION }}.zip path: apps/cli/dist/bw${{ matrix.license_type.artifact_prefix }}-${{ env.LOWER_RUNNER_OS }}${{ matrix.os.target_suffix }}-${{ env._PACKAGE_VERSION }}.zip @@ -482,7 +482,7 @@ jobs: } - name: Upload windows zip asset - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bw${{ matrix.license_type.artifact_prefix }}-windows-${{ env._PACKAGE_VERSION }}.zip path: apps/cli/dist/bw${{ matrix.license_type.artifact_prefix }}-windows-${{ env._PACKAGE_VERSION }}.zip @@ -490,7 +490,7 @@ jobs: - name: Upload Chocolatey asset if: matrix.license_type.build_prefix == 'bit' - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bitwarden-cli.${{ env._PACKAGE_VERSION }}.nupkg path: apps/cli/dist/chocolatey/bitwarden-cli.${{ env._PACKAGE_VERSION }}.nupkg @@ -503,7 +503,7 @@ jobs: - name: Upload NPM Build Directory asset if: matrix.license_type.build_prefix == 'bit' - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bitwarden-cli-${{ env._PACKAGE_VERSION }}-npm-build.zip path: apps/cli/bitwarden-cli-${{ env._PACKAGE_VERSION }}-npm-build.zip @@ -535,7 +535,7 @@ jobs: echo "BW Package Version: $_PACKAGE_VERSION" - name: Get bw linux cli - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: name: bw-linux-${{ env._PACKAGE_VERSION }}.zip path: apps/cli/dist/snap @@ -572,7 +572,7 @@ jobs: run: sudo snap remove bw - name: Upload snap asset - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bw_${{ env._PACKAGE_VERSION }}_amd64.snap path: apps/cli/dist/snap/bw_${{ env._PACKAGE_VERSION }}_amd64.snap diff --git a/.github/workflows/build-desktop.yml b/.github/workflows/build-desktop.yml index 40185f5b700..6b652149d8d 100644 --- a/.github/workflows/build-desktop.yml +++ b/.github/workflows/build-desktop.yml @@ -261,42 +261,42 @@ jobs: run: npm run dist:lin - name: Upload tar.gz artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bitwarden_${{ env._PACKAGE_VERSION }}_x64.tar.gz path: apps/desktop/dist/bitwarden_desktop_x64.tar.gz if-no-files-found: error - name: Upload .deb artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-${{ env._PACKAGE_VERSION }}-amd64.deb path: apps/desktop/dist/Bitwarden-${{ env._PACKAGE_VERSION }}-amd64.deb if-no-files-found: error - name: Upload .rpm artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-${{ env._PACKAGE_VERSION }}-x86_64.rpm path: apps/desktop/dist/Bitwarden-${{ env._PACKAGE_VERSION }}-x86_64.rpm if-no-files-found: error - name: Upload .snap artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bitwarden_${{ env._PACKAGE_VERSION }}_amd64.snap path: apps/desktop/dist/bitwarden_${{ env._PACKAGE_VERSION }}_amd64.snap if-no-files-found: error - name: Upload .AppImage artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-${{ env._PACKAGE_VERSION }}-x86_64.AppImage path: apps/desktop/dist/Bitwarden-${{ env._PACKAGE_VERSION }}-x86_64.AppImage if-no-files-found: error - name: Upload auto-update artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: ${{ needs.setup.outputs.release_channel }}-linux.yml path: apps/desktop/dist/${{ needs.setup.outputs.release_channel }}-linux.yml @@ -309,7 +309,7 @@ jobs: sudo npm run pack:lin:flatpak - name: Upload flatpak artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: com.bitwarden.desktop.flatpak path: apps/desktop/dist/com.bitwarden.desktop.flatpak @@ -437,14 +437,14 @@ jobs: run: npm run dist:lin:arm64 - name: Upload .snap artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bitwarden_${{ env._PACKAGE_VERSION }}_arm64.snap path: apps/desktop/dist/bitwarden_${{ env._PACKAGE_VERSION }}_arm64.snap if-no-files-found: error - name: Upload tar.gz artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bitwarden_${{ env._PACKAGE_VERSION }}_arm64.tar.gz path: apps/desktop/dist/bitwarden_desktop_arm64.tar.gz @@ -457,7 +457,7 @@ jobs: sudo npm run pack:lin:flatpak - name: Upload flatpak artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: com.bitwarden.desktop-arm64.flatpak path: apps/desktop/dist/com.bitwarden.desktop.flatpak @@ -630,7 +630,7 @@ jobs: -NewName bitwarden-$env:_PACKAGE_VERSION-arm64.nsis.7z - name: Upload portable exe artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-Portable-${{ env._PACKAGE_VERSION }}.exe path: apps/desktop/dist/Bitwarden-Portable-${{ env._PACKAGE_VERSION }}.exe @@ -638,7 +638,7 @@ jobs: - name: Upload installer exe artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-Installer-${{ env._PACKAGE_VERSION }}.exe path: apps/desktop/dist/nsis-web/Bitwarden-Installer-${{ env._PACKAGE_VERSION }}.exe @@ -646,7 +646,7 @@ jobs: - name: Upload appx ia32 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-${{ env._PACKAGE_VERSION }}-ia32.appx path: apps/desktop/dist/Bitwarden-${{ env._PACKAGE_VERSION }}-ia32.appx @@ -654,7 +654,7 @@ jobs: - name: Upload store appx ia32 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-${{ env._PACKAGE_VERSION }}-ia32-store.appx path: apps/desktop/dist/Bitwarden-${{ env._PACKAGE_VERSION }}-ia32-store.appx @@ -662,7 +662,7 @@ jobs: - name: Upload NSIS ia32 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bitwarden-${{ env._PACKAGE_VERSION }}-ia32.nsis.7z path: apps/desktop/dist/nsis-web/bitwarden-${{ env._PACKAGE_VERSION }}-ia32.nsis.7z @@ -670,7 +670,7 @@ jobs: - name: Upload appx x64 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-${{ env._PACKAGE_VERSION }}-x64.appx path: apps/desktop/dist/Bitwarden-${{ env._PACKAGE_VERSION }}-x64.appx @@ -678,7 +678,7 @@ jobs: - name: Upload store appx x64 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-${{ env._PACKAGE_VERSION }}-x64-store.appx path: apps/desktop/dist/Bitwarden-${{ env._PACKAGE_VERSION }}-x64-store.appx @@ -686,7 +686,7 @@ jobs: - name: Upload NSIS x64 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bitwarden-${{ env._PACKAGE_VERSION }}-x64.nsis.7z path: apps/desktop/dist/nsis-web/bitwarden-${{ env._PACKAGE_VERSION }}-x64.nsis.7z @@ -694,7 +694,7 @@ jobs: - name: Upload appx ARM64 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-${{ env._PACKAGE_VERSION }}-arm64.appx path: apps/desktop/dist/Bitwarden-${{ env._PACKAGE_VERSION }}-arm64.appx @@ -702,7 +702,7 @@ jobs: - name: Upload store appx ARM64 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-${{ env._PACKAGE_VERSION }}-arm64-store.appx path: apps/desktop/dist/Bitwarden-${{ env._PACKAGE_VERSION }}-arm64-store.appx @@ -710,7 +710,7 @@ jobs: - name: Upload NSIS ARM64 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bitwarden-${{ env._PACKAGE_VERSION }}-arm64.nsis.7z path: apps/desktop/dist/nsis-web/bitwarden-${{ env._PACKAGE_VERSION }}-arm64.nsis.7z @@ -718,7 +718,7 @@ jobs: - name: Upload nupkg artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bitwarden.${{ env._PACKAGE_VERSION }}.nupkg path: apps/desktop/dist/chocolatey/bitwarden.${{ env._PACKAGE_VERSION }}.nupkg @@ -726,7 +726,7 @@ jobs: - name: Upload auto-update artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: ${{ needs.setup.outputs.release_channel }}.yml path: apps/desktop/dist/nsis-web/${{ needs.setup.outputs.release_channel }}.yml @@ -883,7 +883,7 @@ jobs: -NewName latest-beta.yml - name: Upload portable exe artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-Beta-Portable-${{ env._PACKAGE_VERSION }}.exe path: apps/desktop/dist/Bitwarden-Beta-Portable-${{ env._PACKAGE_VERSION }}.exe @@ -891,7 +891,7 @@ jobs: - name: Upload installer exe artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-Beta-Installer-${{ env._PACKAGE_VERSION }}.exe path: apps/desktop/dist/nsis-web/Bitwarden-Beta-Installer-${{ env._PACKAGE_VERSION }}.exe @@ -899,7 +899,7 @@ jobs: - name: Upload appx ia32 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-Beta-${{ env._PACKAGE_VERSION }}-ia32.appx path: apps/desktop/dist/Bitwarden-Beta-${{ env._PACKAGE_VERSION }}-ia32.appx @@ -907,7 +907,7 @@ jobs: - name: Upload store appx ia32 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-Beta-${{ env._PACKAGE_VERSION }}-ia32-store.appx path: apps/desktop/dist/Bitwarden-Beta-${{ env._PACKAGE_VERSION }}-ia32-store.appx @@ -915,7 +915,7 @@ jobs: - name: Upload NSIS ia32 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bitwarden-beta-${{ env._PACKAGE_VERSION }}-ia32.nsis.7z path: apps/desktop/dist/nsis-web/bitwarden-beta-${{ env._PACKAGE_VERSION }}-ia32.nsis.7z @@ -923,7 +923,7 @@ jobs: - name: Upload appx x64 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-Beta-${{ env._PACKAGE_VERSION }}-x64.appx path: apps/desktop/dist/Bitwarden-Beta-${{ env._PACKAGE_VERSION }}-x64.appx @@ -931,7 +931,7 @@ jobs: - name: Upload store appx x64 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-Beta-${{ env._PACKAGE_VERSION }}-x64-store.appx path: apps/desktop/dist/Bitwarden-Beta-${{ env._PACKAGE_VERSION }}-x64-store.appx @@ -939,7 +939,7 @@ jobs: - name: Upload NSIS x64 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bitwarden-beta-${{ env._PACKAGE_VERSION }}-x64.nsis.7z path: apps/desktop/dist/nsis-web/bitwarden-beta-${{ env._PACKAGE_VERSION }}-x64.nsis.7z @@ -947,7 +947,7 @@ jobs: - name: Upload appx ARM64 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-Beta-${{ env._PACKAGE_VERSION }}-arm64.appx path: apps/desktop/dist/Bitwarden-Beta-${{ env._PACKAGE_VERSION }}-arm64.appx @@ -955,7 +955,7 @@ jobs: - name: Upload store appx ARM64 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-Beta-${{ env._PACKAGE_VERSION }}-arm64-store.appx path: apps/desktop/dist/Bitwarden-Beta-${{ env._PACKAGE_VERSION }}-arm64-store.appx @@ -963,7 +963,7 @@ jobs: - name: Upload NSIS ARM64 artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: bitwarden-beta-${{ env._PACKAGE_VERSION }}-arm64.nsis.7z path: apps/desktop/dist/nsis-web/bitwarden-beta-${{ env._PACKAGE_VERSION }}-arm64.nsis.7z @@ -971,7 +971,7 @@ jobs: - name: Upload auto-update artifact if: ${{ needs.setup.outputs.has_secrets == 'true' }} - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: latest-beta.yml path: apps/desktop/dist/nsis-web/latest-beta.yml @@ -1429,7 +1429,7 @@ jobs: run: npm run build - name: Download Browser artifact - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: path: ${{ github.workspace }}/browser-build-artifacts @@ -1462,28 +1462,28 @@ jobs: run: npm run pack:mac - name: Upload .zip artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-${{ env._PACKAGE_VERSION }}-universal-mac.zip path: apps/desktop/dist/Bitwarden-${{ env._PACKAGE_VERSION }}-universal-mac.zip if-no-files-found: error - name: Upload .dmg artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-${{ env._PACKAGE_VERSION }}-universal.dmg path: apps/desktop/dist/Bitwarden-${{ env._PACKAGE_VERSION }}-universal.dmg if-no-files-found: error - name: Upload .dmg blockmap artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-${{ env._PACKAGE_VERSION }}-universal.dmg.blockmap path: apps/desktop/dist/Bitwarden-${{ env._PACKAGE_VERSION }}-universal.dmg.blockmap if-no-files-found: error - name: Upload auto-update artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: ${{ needs.setup.outputs.release_channel }}-mac.yml path: apps/desktop/dist/${{ needs.setup.outputs.release_channel }}-mac.yml @@ -1712,7 +1712,7 @@ jobs: run: npm run build - name: Download Browser artifact - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: path: ${{ github.workspace }}/browser-build-artifacts @@ -1755,14 +1755,14 @@ jobs: $buildInfo | ConvertTo-Json | Set-Content -Path dist/macos-build-number.json - name: Upload MacOS App Store build number artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: macos-build-number.json path: apps/desktop/dist/macos-build-number.json if-no-files-found: error - name: Upload .pkg artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: Bitwarden-${{ env._PACKAGE_VERSION }}-universal.pkg path: apps/desktop/dist/mas-universal/Bitwarden-${{ env._PACKAGE_VERSION }}-universal.pkg diff --git a/.github/workflows/build-web.yml b/.github/workflows/build-web.yml index 7d302fb453b..24a8df084a2 100644 --- a/.github/workflows/build-web.yml +++ b/.github/workflows/build-web.yml @@ -307,7 +307,7 @@ jobs: zip -r web-$_VERSION-${{ matrix.artifact_name }}.zip build - name: Upload ${{ matrix.artifact_name }} artifact - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: web-${{ env._VERSION }}-${{ matrix.artifact_name }}.zip path: apps/web/web-${{ env._VERSION }}-${{ matrix.artifact_name }}.zip diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index e8dd654d8ec..ea6894dab8e 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -115,7 +115,7 @@ jobs: run: rustup --version - name: Cache cargo registry - uses: Swatinem/rust-cache@f0deed1e0edfc6a9be95417288c0e1099b1eeec3 # v2.7.7 + uses: Swatinem/rust-cache@f13886b937689c021905a6b90929199931d60db1 # v2.8.1 - name: Run cargo fmt working-directory: ./apps/desktop/desktop_native diff --git a/.github/workflows/nx.yml b/.github/workflows/nx.yml index 1e23c31b033..3a7431c07f0 100644 --- a/.github/workflows/nx.yml +++ b/.github/workflows/nx.yml @@ -36,7 +36,7 @@ jobs: run: npm ci - name: Set Nx SHAs for affected detection - uses: nrwl/nx-set-shas@826660b82addbef3abff5fa871492ebad618c9e1 # v4.3.3 + uses: nrwl/nx-set-shas@3e9ad7370203c1e93d109be57f3b72eb0eb511b1 # v4.4.0 - name: Run Nx affected tasks continue-on-error: true diff --git a/.github/workflows/repository-management.yml b/.github/workflows/repository-management.yml index b2edf0171db..79f3335313e 100644 --- a/.github/workflows/repository-management.yml +++ b/.github/workflows/repository-management.yml @@ -71,6 +71,8 @@ jobs: version_web: ${{ steps.set-final-version-output.outputs.version_web }} permissions: id-token: write + contents: write + pull-requests: write steps: - name: Validate version input format @@ -93,6 +95,13 @@ jobs: keyvault: gh-org-bitwarden secrets: "BW-GHAPP-ID,BW-GHAPP-KEY" + - name: Retrieve GPG secrets + id: retrieve-gpg-secrets + uses: bitwarden/gh-actions/get-keyvault-secrets@main + with: + keyvault: "bitwarden-ci" + secrets: "github-gpg-private-key, github-gpg-private-key-passphrase" + - name: Log out from Azure uses: bitwarden/gh-actions/azure-logout@main @@ -102,7 +111,8 @@ jobs: with: app-id: ${{ steps.get-kv-secrets.outputs.BW-GHAPP-ID }} private-key: ${{ steps.get-kv-secrets.outputs.BW-GHAPP-KEY }} - permission-contents: write # for committing and pushing to current branch + permission-contents: write # for creating, committing to, and pushing new branches + permission-pull-requests: write # for generating pull requests - name: Check out branch uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 @@ -113,8 +123,20 @@ jobs: - name: Configure Git run: | - git config --local user.email "actions@github.com" - git config --local user.name "Github Actions" + git config --local user.email "106330231+bitwarden-devops-bot@users.noreply.github.com" + git config --local user.name "bitwarden-devops-bot" + + - name: Setup GPG signing + env: + GPG_PRIVATE_KEY: ${{ steps.retrieve-gpg-secrets.outputs.github-gpg-private-key }} + GPG_PASSPHRASE: ${{ steps.retrieve-gpg-secrets.outputs.github-gpg-private-key-passphrase }} + run: | + echo "$GPG_PRIVATE_KEY" | gpg --import --batch + GPG_KEY_ID=$(gpg --list-secret-keys --keyid-format=long | grep -o "rsa[0-9]\+/[A-F0-9]\+" | head -n1 | cut -d'/' -f2) + git config --local user.signingkey "$GPG_KEY_ID" + git config --local commit.gpgsign true + export GPG_TTY=$(tty) + echo "test" | gpg --clearsign --pinentry-mode=loopback --passphrase "$GPG_PASSPHRASE" > /dev/null 2>&1 ######################## # VERSION BUMP SECTION # @@ -426,13 +448,53 @@ jobs: echo "No changes to commit!"; fi - - name: Commit files + - name: Create version bump branch if: ${{ steps.version-changed.outputs.changes_to_commit == 'TRUE' }} - run: git commit -m "Bumped client version(s)" -a + run: | + BRANCH_NAME="version-bump-$(date +%s)" + git checkout -b "$BRANCH_NAME" + echo "BRANCH_NAME=$BRANCH_NAME" >> $GITHUB_ENV - - name: Push changes + - name: Commit version bumps with GPG signature if: ${{ steps.version-changed.outputs.changes_to_commit == 'TRUE' }} - run: git push + run: | + git commit -m "Bumped client version(s)" -a + + - name: Push version bump branch + if: ${{ steps.version-changed.outputs.changes_to_commit == 'TRUE' }} + run: | + git push --set-upstream origin "$BRANCH_NAME" + + - name: Create Pull Request for version bump + if: ${{ steps.version-changed.outputs.changes_to_commit == 'TRUE' }} + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + env: + VERSION_BROWSER: ${{ steps.set-final-version-output.outputs.version_browser }} + VERSION_CLI: ${{ steps.set-final-version-output.outputs.version_cli }} + VERSION_DESKTOP: ${{ steps.set-final-version-output.outputs.version_desktop }} + VERSION_WEB: ${{ steps.set-final-version-output.outputs.version_web }} + with: + github-token: ${{ steps.app-token.outputs.token }} + script: | + const versions = []; + if (process.env.VERSION_BROWSER) versions.push(`- Browser: ${process.env.VERSION_BROWSER}`); + if (process.env.VERSION_CLI) versions.push(`- CLI: ${process.env.VERSION_CLI}`); + if (process.env.VERSION_DESKTOP) versions.push(`- Desktop: ${process.env.VERSION_DESKTOP}`); + if (process.env.VERSION_WEB) versions.push(`- Web: ${process.env.VERSION_WEB}`); + + const body = versions.length > 0 + ? `Automated version bump:\n\n${versions.join('\n')}` + : 'Automated version bump'; + + const { data: pr } = await github.rest.pulls.create({ + owner: context.repo.owner, + repo: context.repo.repo, + title: 'Bumped client version(s)', + body: body, + head: process.env.BRANCH_NAME, + base: context.ref.replace('refs/heads/', '') + }); + console.log(`Created PR #${pr.number}: ${pr.html_url}`); cut_branch: name: Cut branch diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e9738fe6175..cf7251b259a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -62,7 +62,7 @@ jobs: run: npm test -- --coverage --maxWorkers=3 - name: Report test results - uses: dorny/test-reporter@dc3a92680fcc15842eef52e8c4606ea7ce6bd3f3 # v2.1.1 + uses: dorny/test-reporter@7b7927aa7da8b82e81e755810cb51f39941a2cc7 # v2.2.0 if: ${{ github.event.pull_request.head.repo.full_name == github.repository && !cancelled() }} with: name: Test Results @@ -74,7 +74,7 @@ jobs: uses: codecov/test-results-action@47f89e9acb64b76debcd5ea40642d25a4adced9f # v1.1.1 - name: Upload test coverage - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: jest-coverage path: ./coverage/lcov.info @@ -160,7 +160,7 @@ jobs: run: cargo llvm-cov --all-features --lcov --output-path lcov.info --workspace --no-cfg-coverage - name: Upload test coverage - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0 with: name: rust-coverage path: ./apps/desktop/desktop_native/lcov.info @@ -178,13 +178,13 @@ jobs: persist-credentials: false - name: Download jest coverage - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: name: jest-coverage path: ./ - name: Download rust coverage - uses: actions/download-artifact@018cc2cf5baa6db3ef3c5f8a56943fffe632ef53 # v6.0.0 + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: name: rust-coverage path: ./apps/desktop/desktop_native diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index ca9dde99a95..29b39863bc6 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -4811,6 +4811,24 @@ "adminConsole": { "message": "Admin Console" }, + "admin" :{ + "message": "Admin" + }, + "automaticUserConfirmation": { + "message": "Automatic user confirmation" + }, + "automaticUserConfirmationHint": { + "message": "Automatically confirm pending users while this device is unlocked" + }, + "autoConfirmOnboardingCallout":{ + "message": "Save time with automatic user confirmation" + }, + "autoConfirmWarning": { + "message": "This could impact your organization’s data security. " + }, + "autoConfirmWarningLink": { + "message": "Learn about the risks" + }, "accountSecurity": { "message": "Account security" }, diff --git a/apps/browser/src/auth/popup/settings/account-security.component.spec.ts b/apps/browser/src/auth/popup/settings/account-security.component.spec.ts index ebabbadf71c..d1380f5eae0 100644 --- a/apps/browser/src/auth/popup/settings/account-security.component.spec.ts +++ b/apps/browser/src/auth/popup/settings/account-security.component.spec.ts @@ -8,6 +8,7 @@ import { firstValueFrom, of, BehaviorSubject } from "rxjs"; import { CollectionService } from "@bitwarden/admin-console/common"; import { NudgesService } from "@bitwarden/angular/vault"; import { LockService } from "@bitwarden/auth/common"; +import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; @@ -124,6 +125,12 @@ describe("AccountSecurityComponent", () => { { provide: ToastService, useValue: mock() }, { provide: UserVerificationService, useValue: mock() }, { provide: ValidationService, useValue: validationService }, + { provide: LockService, useValue: lockService }, + { + provide: AutomaticUserConfirmationService, + useValue: mock(), + }, + { provide: ConfigService, useValue: configService }, { provide: VaultTimeoutSettingsService, useValue: vaultTimeoutSettingsService }, ], }) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 09ae9deb8f1..b9b41943b04 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -506,7 +506,6 @@ export default class MainBackground { // DIRT private phishingDataService: PhishingDataService; private phishingDetectionSettingsService: PhishingDetectionSettingsServiceAbstraction; - private phishingDetectionCleanup: (() => void) | null = null; constructor() { const logoutCallback = async (logoutReason: LogoutReason, userId?: UserId) => @@ -1516,12 +1515,7 @@ export default class MainBackground { this.stateProvider, ); - // Call cleanup from previous initialization if it exists (service worker restart scenario) - if (this.phishingDetectionCleanup) { - this.phishingDetectionCleanup(); - } - - this.phishingDetectionCleanup = PhishingDetectionService.initialize( + PhishingDetectionService.initialize( this.logService, this.phishingDataService, this.phishingDetectionSettingsService, @@ -1680,32 +1674,6 @@ export default class MainBackground { } } - /** - * Triggers a phishing cache update in the background. - * Called on extension install/update to pre-populate the cache - * so it's ready when a premium user logs in. - * - * Creates a temporary subscription to ensure the update executes even if - * there are no other subscribers (install/update scenario). The subscription - * is automatically cleaned up after the update completes or errors. - */ - triggerPhishingCacheUpdate(): void { - // Create a temporary subscription to ensure the update executes - // since update$ uses shareReplay with refCount: true, which requires at least one subscriber - const tempSub = this.phishingDataService.update$.subscribe({ - next: () => { - this.logService.debug("[MainBackground] Phishing cache pre-population completed"); - tempSub.unsubscribe(); - }, - error: (err: unknown) => { - this.logService.error("[MainBackground] Phishing cache pre-population failed", err); - tempSub.unsubscribe(); - }, - }); - // Trigger the update after subscription is created - this.phishingDataService.triggerUpdateIfNeeded(); - } - /** * Switch accounts to indicated userId -- null is no active user */ diff --git a/apps/browser/src/background/runtime.background.ts b/apps/browser/src/background/runtime.background.ts index e4d3c428802..eba6b01fe90 100644 --- a/apps/browser/src/background/runtime.background.ts +++ b/apps/browser/src/background/runtime.background.ts @@ -433,15 +433,6 @@ export default class RuntimeBackground { void this.autofillService.loadAutofillScriptsOnInstall(); if (this.onInstalledReason != null) { - // Pre-populate phishing cache on install/update so it's ready when premium user logs in - // This runs in background and doesn't block the user - if (this.onInstalledReason === "install" || this.onInstalledReason === "update") { - this.logService.debug( - `[RuntimeBackground] Extension ${this.onInstalledReason}: triggering phishing cache pre-population`, - ); - this.main.triggerPhishingCacheUpdate(); - } - if ( this.onInstalledReason === "install" && !(await firstValueFrom(this.browserInitialInstallService.extensionInstalled$)) diff --git a/apps/browser/src/dirt/phishing-detection/phishing-resources.ts b/apps/browser/src/dirt/phishing-detection/phishing-resources.ts index 4cd155c8ae3..262d6cf833b 100644 --- a/apps/browser/src/dirt/phishing-detection/phishing-resources.ts +++ b/apps/browser/src/dirt/phishing-detection/phishing-resources.ts @@ -18,7 +18,8 @@ export const PHISHING_RESOURCES: Record { expect(result!.applicationVersion).toBe("2.0.0"); }); - it("returns null if checksum matches (no update needed)", async () => { + it("only updates timestamp if checksum matches", async () => { const prev: PhishingData = { webAddresses: ["a.com"], timestamp: Date.now() - 60000, @@ -122,8 +122,9 @@ describe("PhishingDataService", () => { }; fetchChecksumSpy.mockResolvedValue("abc"); const result = await service.getNextWebAddresses(prev); - // When checksum matches, return null to signal "skip state update" - expect(result).toBeNull(); + expect(result!.webAddresses).toEqual(prev.webAddresses); + expect(result!.checksum).toBe("abc"); + expect(result!.timestamp).not.toBe(prev.timestamp); }); it("patches daily domains if cache is fresh", async () => { diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts index 6f5e6dc63f3..21fe74f1873 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-data.service.ts @@ -1,17 +1,13 @@ import { catchError, - distinctUntilChanged, EMPTY, - filter, - finalize, first, firstValueFrom, - from, - of, + map, retry, - shareReplay, + share, + startWith, Subject, - Subscription, switchMap, tap, timer, @@ -22,12 +18,7 @@ import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { ScheduledTaskNames, TaskSchedulerService } from "@bitwarden/common/platform/scheduling"; import { LogService } from "@bitwarden/logging"; -import { - GlobalState, - GlobalStateProvider, - KeyDefinition, - PHISHING_DETECTION_DISK, -} from "@bitwarden/state"; +import { GlobalStateProvider, KeyDefinition, PHISHING_DETECTION_DISK } from "@bitwarden/state"; import { getPhishingResources, PhishingResourceType } from "../phishing-resources"; @@ -47,31 +38,70 @@ export const PHISHING_DOMAINS_KEY = new KeyDefinition( PHISHING_DETECTION_DISK, "phishingDomains", { - deserializer: (value: PhishingData) => { - return value ?? { webAddresses: [], timestamp: 0, checksum: "", applicationVersion: "" }; - }, + deserializer: (value: PhishingData) => + value ?? { webAddresses: [], timestamp: 0, checksum: "", applicationVersion: "" }, }, ); /** Coordinates fetching, caching, and patching of known phishing web addresses */ export class PhishingDataService { - // Static tracking to prevent interval accumulation across service instances (reload scenario) - private static _intervalSubscription: Subscription | null = null; - private _testWebAddresses = this.getTestWebAddresses(); - private _cachedPhishingDataStateInstance: GlobalState | null = null; + private _cachedState = this.globalStateProvider.get(PHISHING_DOMAINS_KEY); + private _webAddresses$ = this._cachedState.state$.pipe( + map( + (state) => + new Set( + (state?.webAddresses?.filter((line) => line.trim().length > 0) ?? []).concat( + this._testWebAddresses, + "phishing.testcategory.com", // Included for QA to test in prod + ), + ), + ), + ); - /** - * Lazy getter for cached phishing data state. Only accesses storage when phishing detection is actually used. - * This prevents blocking service worker initialization on extension reload for non-premium users. - */ - private get _cachedPhishingDataState() { - if (this._cachedPhishingDataStateInstance === null) { - this.logService.debug("[PhishingDataService] Lazy-loading state from storage (first access)"); - this._cachedPhishingDataStateInstance = this.globalStateProvider.get(PHISHING_DOMAINS_KEY); - } - return this._cachedPhishingDataStateInstance; - } + // How often are new web addresses added to the remote? + readonly UPDATE_INTERVAL_DURATION = 24 * 60 * 60 * 1000; // 24 hours + + private _triggerUpdate$ = new Subject(); + update$ = this._triggerUpdate$.pipe( + startWith(undefined), // Always emit once + tap(() => this.logService.info(`[PhishingDataService] Update triggered...`)), + switchMap(() => + this._cachedState.state$.pipe( + first(), // Only take the first value to avoid an infinite loop when updating the cache below + switchMap(async (cachedState) => { + const next = await this.getNextWebAddresses(cachedState); + if (next) { + await this._cachedState.update(() => next); + this.logService.info(`[PhishingDataService] cache updated`); + } + }), + retry({ + count: 3, + delay: (err, count) => { + this.logService.error( + `[PhishingDataService] Unable to update web addresses. Attempt ${count}.`, + err, + ); + return timer(5 * 60 * 1000); // 5 minutes + }, + resetOnSuccess: true, + }), + catchError( + ( + err: unknown /** Eslint actually crashed if you remove this type: https://github.com/cartant/eslint-plugin-rxjs/issues/122 */, + ) => { + this.logService.error( + "[PhishingDataService] Retries unsuccessful. Unable to update web addresses.", + err, + ); + return EMPTY; + }, + ), + ), + ), + share(), + ); constructor( private apiService: ApiService, @@ -84,182 +114,12 @@ export class PhishingDataService { this.taskSchedulerService.registerTaskHandler(ScheduledTaskNames.phishingDomainUpdate, () => { this._triggerUpdate$.next(); }); - - // Clean up previous interval if it exists (prevents accumulation on service recreation) - if (PhishingDataService._intervalSubscription) { - PhishingDataService._intervalSubscription.unsubscribe(); - PhishingDataService._intervalSubscription = null; - } - // Store interval subscription statically to prevent accumulation on reload - PhishingDataService._intervalSubscription = this.taskSchedulerService.setInterval( + this.taskSchedulerService.setInterval( ScheduledTaskNames.phishingDomainUpdate, this.UPDATE_INTERVAL_DURATION, ); } - // In-memory cache to avoid expensive Set rebuilds and state rewrites - private _cachedWebAddressesSet: Set | null = null; - private _cachedSetChecksum: string = ""; - private _lastCheckTime: number = 0; // Track check time in memory, not state - - // Lazy observable: only subscribes to state$ when actually needed (first URL check) - // This prevents blocking service worker initialization on extension reload - // Using a getter with caching to defer access to _cachedPhishingDataState until actually subscribed - private _webAddresses$Instance: ReturnType | null = null; - private get _webAddresses$() { - if (this._webAddresses$Instance === null) { - this._webAddresses$Instance = this.createWebAddresses$(); - } - return this._webAddresses$Instance; - } - - private createWebAddresses$() { - return this._cachedPhishingDataState.state$.pipe( - // Only rebuild Set when checksum changes (actual data change) - distinctUntilChanged((prev, curr) => prev?.checksum === curr?.checksum), - switchMap((state) => { - // Return cached Set if checksum matches - if (this._cachedWebAddressesSet && state?.checksum === this._cachedSetChecksum) { - this.logService.debug( - `[PhishingDataService] Using cached Set (${this._cachedWebAddressesSet.size} entries, checksum: ${state?.checksum.substring(0, 8)}...)`, - ); - return of(this._cachedWebAddressesSet); - } - // Build Set in chunks to avoid blocking UI - this.logService.debug( - `[PhishingDataService] Building Set from ${state?.webAddresses?.length ?? 0} entries`, - ); - return from(this.buildSetInChunks(state?.webAddresses ?? [], state?.checksum ?? "")); - }), - shareReplay({ bufferSize: 1, refCount: true }), - ); - } - - // How often are new web addresses added to the remote? - readonly UPDATE_INTERVAL_DURATION = 24 * 60 * 60 * 1000; // 24 hours - - // Minimum time between updates when triggered by account switch (5 minutes) - private readonly MIN_UPDATE_INTERVAL = 5 * 60 * 1000; - - private _triggerUpdate$ = new Subject(); - private _updateInProgress = false; - - /** - * Observable that handles phishing data updates. - * - * Updates are triggered explicitly via triggerUpdateIfNeeded() or the 24-hour scheduler. - * The observable includes safeguards to prevent redundant updates: - * - Skips if an update is already in progress - * - Skips if cache was updated within MIN_UPDATE_INTERVAL (5 min) - * - * Lazy getter with caching: Only accesses _cachedPhishingDataState when actually subscribed to prevent storage read on reload. - */ - private _update$Instance: ReturnType | null = null; - get update$() { - if (this._update$Instance === null) { - this._update$Instance = this.createUpdate$(); - } - return this._update$Instance; - } - - private createUpdate$() { - return this._triggerUpdate$.pipe( - // Don't use startWith - initial update is handled by triggerUpdateIfNeeded() - filter(() => { - if (this._updateInProgress) { - this.logService.debug("[PhishingDataService] Update already in progress, skipping"); - return false; - } - return true; - }), - tap(() => { - this._updateInProgress = true; - }), - switchMap(async () => { - // Get current state directly without subscribing to state$ observable - // This avoids creating a subscription that stays active - const cachedState = await firstValueFrom( - this._cachedPhishingDataState.state$.pipe(first()), - ); - - // Early exit if we checked recently (using in-memory tracking) - const timeSinceLastCheck = Date.now() - this._lastCheckTime; - if (timeSinceLastCheck < this.MIN_UPDATE_INTERVAL) { - this.logService.debug( - `[PhishingDataService] Checked ${Math.round(timeSinceLastCheck / 1000)}s ago, skipping`, - ); - return; - } - - // Update last check time in memory (not state - avoids expensive write) - this._lastCheckTime = Date.now(); - - try { - const result = await this.getNextWebAddresses(cachedState); - - // result is null when checksum matched - skip state update entirely - if (result === null) { - this.logService.debug("[PhishingDataService] Checksum matched, skipping state update"); - return; - } - - if (result) { - // Yield to event loop before state update - await new Promise((resolve) => setTimeout(resolve, 0)); - await this._cachedPhishingDataState.update(() => result); - this.logService.info( - `[PhishingDataService] State updated with ${result.webAddresses?.length ?? 0} entries`, - ); - } - } catch (err) { - this.logService.error("[PhishingDataService] Unable to update web addresses.", err); - // Retry logic removed - let the 24-hour scheduler handle retries - throw err; - } - }), - retry({ - count: 3, - delay: (err, count) => { - this.logService.error( - `[PhishingDataService] Unable to update web addresses. Attempt ${count}.`, - err, - ); - return timer(5 * 60 * 1000); // 5 minutes - }, - resetOnSuccess: true, - }), - catchError( - ( - err: unknown /** Eslint actually crashed if you remove this type: https://github.com/cartant/eslint-plugin-rxjs/issues/122 */, - ) => { - this.logService.error( - "[PhishingDataService] Retries unsuccessful. Unable to update web addresses.", - err, - ); - return EMPTY; - }, - ), - // Use finalize() to ensure _updateInProgress is reset on success, error, OR completion - // Per ADR: "Use finalize() operator to ensure cleanup code always runs" - finalize(() => { - this._updateInProgress = false; - }), - shareReplay({ bufferSize: 1, refCount: true }), - ); - } - - /** - * Triggers an update if the cache is stale or empty. - * Should be called when phishing detection is enabled for an account or on install/update. - * - * The lazy loading of _cachedPhishingDataState ensures that storage is only accessed - * when the update$ observable chain actually executes (i.e., when there are subscribers). - * If there are no subscribers, the chain doesn't execute and no storage access occurs. - */ - triggerUpdateIfNeeded(): void { - this._triggerUpdate$.next(); - } - /** * Checks if the given URL is a known phishing web address * @@ -267,16 +127,13 @@ export class PhishingDataService { * @returns True if the URL is a known phishing web address, false otherwise */ async isPhishingWebAddress(url: URL): Promise { - // Lazy load: Only now do we subscribe to _webAddresses$ and trigger storage read + Set build - // This ensures we don't block service worker initialization on extension reload - this.logService.debug(`[PhishingDataService] Checking URL: ${url.href}`); + // Use domain (hostname) matching for domain resources, and link matching for links resources const entries = await firstValueFrom(this._webAddresses$); const resource = getPhishingResources(this.resourceType); if (resource && resource.match) { for (const entry of entries) { if (resource.match(url, entry)) { - this.logService.info(`[PhishingDataService] Match: ${url.href} matched entry: ${entry}`); return true; } } @@ -287,72 +144,44 @@ export class PhishingDataService { return entries.has(url.hostname); } - /** - * Determines if the phishing data needs to be updated and fetches new data if necessary. - * - * The CHECKSUM is an MD5 hash of the phishing list file, hosted at: - * For full url see: clients/apps/browser/src/dirt/phishing-detection/phishing-resources.ts - * - Links: https://raw.githubusercontent.com/Phishing-Database/checksums/.../phishing-links-ACTIVE.txt.md5 - * - Domains: https://raw.githubusercontent.com/Phishing-Database/checksums/.../phishing-domains-ACTIVE.txt.md5 - * - * PURPOSE: The checksum allows us to quickly check if the list has changed without - * downloading the entire file (~63MB uncompressed). If checksums match, data is identical. - * - * FLOW: - * 1. Fetch remote checksum (~62 bytes) - fast - * 2. Compare to local cached checksum - * 3. If match: return null (skip expensive state update) - * 4. If different: fetch new data and update state - * - * @returns PhishingData if data changed, null if checksum matched (no update needed) - */ async getNextWebAddresses(prev: PhishingData | null): Promise { prev = prev ?? { webAddresses: [], timestamp: 0, checksum: "", applicationVersion: "" }; const timestamp = Date.now(); const prevAge = timestamp - prev.timestamp; - - this.logService.debug( - `[PhishingDataService] Cache: ${prev.webAddresses?.length ?? 0} entries, age ${Math.round(prevAge / 1000 / 60)}min`, - ); + this.logService.info(`[PhishingDataService] Cache age: ${prevAge}`); const applicationVersion = await this.platformUtilsService.getApplicationVersion(); - // STEP 1: Fetch the remote checksum (tiny file, ~32 bytes) + // If checksum matches, return existing data with new timestamp & version const remoteChecksum = await this.fetchPhishingChecksum(this.resourceType); - - // STEP 2: Compare checksums if (remoteChecksum && prev.checksum === remoteChecksum) { - this.logService.debug("[PhishingDataService] Checksum match, no update needed"); - return null; // Signal to skip state update - no UI blocking! - } - - // STEP 3: Checksum different - data needs to be updated - this.logService.info("[PhishingDataService] Checksum mismatch, fetching new data"); - - // Approach 1: Fetch only today's new entries (if cache is less than 24h old) - const isOneDayOldMax = prevAge <= this.UPDATE_INTERVAL_DURATION; - if ( - isOneDayOldMax && - applicationVersion === prev.applicationVersion && - (prev.webAddresses?.length ?? 0) > 0 - ) { - const webAddressesTodayUrl = getPhishingResources(this.resourceType)!.todayUrl; - const dailyWebAddresses = await this.fetchPhishingWebAddresses(webAddressesTodayUrl); this.logService.info( - `[PhishingDataService] Daily update: +${dailyWebAddresses.length} entries`, + `[PhishingDataService] Remote checksum matches local checksum, updating timestamp only.`, + ); + return { ...prev, timestamp, applicationVersion }; + } + // Checksum is different, data needs to be updated. + + // Approach 1: Fetch only new web addresses and append + const isOneDayOldMax = prevAge <= this.UPDATE_INTERVAL_DURATION; + if (isOneDayOldMax && applicationVersion === prev.applicationVersion) { + const webAddressesTodayUrl = getPhishingResources(this.resourceType)!.todayUrl; + const dailyWebAddresses: string[] = + await this.fetchPhishingWebAddresses(webAddressesTodayUrl); + this.logService.info( + `[PhishingDataService] ${dailyWebAddresses.length} new phishing web addresses added`, ); return { - webAddresses: (prev.webAddresses ?? []).concat(dailyWebAddresses), + webAddresses: prev.webAddresses.concat(dailyWebAddresses), checksum: remoteChecksum, timestamp, applicationVersion, }; } - // Approach 2: Fetch entire list (cache is stale or empty) + // Approach 2: Fetch all web addresses const remoteUrl = getPhishingResources(this.resourceType)!.remoteUrl; const remoteWebAddresses = await this.fetchPhishingWebAddresses(remoteUrl); - this.logService.info(`[PhishingDataService] Full update: ${remoteWebAddresses.length} entries`); return { webAddresses: remoteWebAddresses, timestamp, @@ -361,136 +190,23 @@ export class PhishingDataService { }; } - /** - * Fetches the MD5 checksum of the phishing list from GitHub. - * The checksum file is tiny (~32 bytes) and fast to fetch. - * Used to detect if the phishing list has changed without downloading the full list. - */ private async fetchPhishingChecksum(type: PhishingResourceType = PhishingResourceType.Domains) { const checksumUrl = getPhishingResources(type)!.checksumUrl; - this.logService.debug(`[PhishingDataService] Fetching checksum from: ${checksumUrl}`); const response = await this.apiService.nativeFetch(new Request(checksumUrl)); if (!response.ok) { throw new Error(`[PhishingDataService] Failed to fetch checksum: ${response.status}`); } - const checksum = await response.text(); - return checksum.trim(); // MD5 checksums are 32 hex characters + return response.text(); } - /** - * Fetches phishing web addresses from the given URL. - * Uses streaming to avoid loading the entire file into memory at once, - * which can cause Firefox to freeze due to memory pressure. - */ - private async fetchPhishingWebAddresses(url: string): Promise { + private async fetchPhishingWebAddresses(url: string) { const response = await this.apiService.nativeFetch(new Request(url)); if (!response.ok) { throw new Error(`[PhishingDataService] Failed to fetch web addresses: ${response.status}`); } - // Stream the response to avoid loading entire file into memory at once - // This prevents Firefox from freezing on large phishing lists (~63MB uncompressed) - const reader = response.body?.getReader(); - if (!reader) { - // Fallback for environments without streaming support - this.logService.warning( - "[PhishingDataService] Streaming not available, falling back to full load", - ); - const text = await response.text(); - return text - .split(/\r?\n/) - .map((l) => l.trim()) - .filter((l) => l.length > 0); - } - - const decoder = new TextDecoder(); - const addresses: string[] = []; - let buffer = ""; - - try { - while (true) { - const { done, value } = await reader.read(); - if (done) { - break; - } - - buffer += decoder.decode(value, { stream: true }); - - // Process complete lines from buffer - const lines = buffer.split(/\r?\n/); - buffer = lines.pop() || ""; // Keep incomplete last line in buffer - - for (let i = 0; i < lines.length; i++) { - const trimmed = lines[i].trim(); - if (trimmed.length > 0) { - addresses.push(trimmed); - } - } - // Yield after processing each network chunk to keep service worker responsive - // This allows popup messages to be handled between chunks - await new Promise((resolve) => setTimeout(resolve, 0)); - } - - // Process any remaining buffer content - const remaining = buffer.trim(); - if (remaining.length > 0) { - addresses.push(remaining); - } - } finally { - // Ensure reader is released even if an error occurs - reader.releaseLock(); - } - - this.logService.debug(`[PhishingDataService] Streamed ${addresses.length} addresses`); - return addresses; - } - - /** - * Builds a Set from an array of web addresses in chunks to avoid blocking the UI. - * Yields to the event loop every CHUNK_SIZE entries, keeping the UI responsive - * even when processing 700K+ entries. - * - * @param addresses Array of web addresses to add to the Set - * @param checksum The checksum to associate with this cached Set - * @returns Promise that resolves to the built Set - */ - private async buildSetInChunks(addresses: string[], checksum: string): Promise> { - const CHUNK_SIZE = 50000; // Process 50K entries per chunk (fast, fewer iterations) - const startTime = Date.now(); - const set = new Set(); - - this.logService.debug(`[PhishingDataService] Building Set (${addresses.length} entries)`); - - for (let i = 0; i < addresses.length; i += CHUNK_SIZE) { - const chunk = addresses.slice(i, Math.min(i + CHUNK_SIZE, addresses.length)); - for (const addr of chunk) { - if (addr) { - // Skip empty strings - set.add(addr); - } - } - - // Yield to event loop after each chunk - if (i + CHUNK_SIZE < addresses.length) { - await new Promise((resolve) => setTimeout(resolve, 0)); - } - } - - // Add test addresses - this._testWebAddresses.forEach((addr) => set.add(addr)); - set.add("phishing.testcategory.com"); // For QA testing - - // Cache for future use - this._cachedWebAddressesSet = set; - this._cachedSetChecksum = checksum; - - const buildTime = Date.now() - startTime; - this.logService.debug( - `[PhishingDataService] Set built: ${set.size} entries in ${buildTime}ms (checksum: ${checksum.substring(0, 8)}...)`, - ); - - return set; + return response.text().then((text) => text.split("\n")); } private getTestWebAddresses() { @@ -502,7 +218,7 @@ export class PhishingDataService { const webAddresses = devFlagValue("testPhishingUrls") as unknown[]; if (webAddresses && webAddresses instanceof Array) { this.logService.debug( - "[PhishingDataService] Dev flag enabled for testing phishing detection. Adding test phishing web addresses:", + "[PhishingDetectionService] Dev flag enabled for testing phishing detection. Adding test phishing web addresses:", webAddresses, ); return webAddresses as string[]; diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.spec.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.spec.ts index cebdd4c9c73..06a37f12faa 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.spec.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.spec.ts @@ -1,5 +1,5 @@ import { mock, MockProxy } from "jest-mock-extended"; -import { EMPTY, Observable, of } from "rxjs"; +import { Observable, of } from "rxjs"; import { PhishingDetectionSettingsServiceAbstraction } from "@bitwarden/common/dirt/services/abstractions/phishing-detection-settings.service.abstraction"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -16,9 +16,7 @@ describe("PhishingDetectionService", () => { beforeEach(() => { logService = { info: jest.fn(), debug: jest.fn(), warning: jest.fn(), error: jest.fn() } as any; - phishingDataService = mock({ - update$: EMPTY, - }); + phishingDataService = mock(); messageListener = mock({ messages$(_commandDefinition) { return new Observable(); diff --git a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts index 744540f9ec8..d90e872eef8 100644 --- a/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts +++ b/apps/browser/src/dirt/phishing-detection/services/phishing-detection.service.ts @@ -1,14 +1,11 @@ import { concatMap, - delay, distinctUntilChanged, EMPTY, filter, map, merge, - of, Subject, - Subscription, switchMap, tap, } from "rxjs"; @@ -46,8 +43,6 @@ export class PhishingDetectionService { private static _tabUpdated$ = new Subject(); private static _ignoredHostnames = new Set(); private static _didInit = false; - private static _triggerUpdateSub: Subscription | null = null; - private static _boundTabHandler: ((...args: readonly unknown[]) => unknown) | null = null; static initialize( logService: LogService, @@ -55,34 +50,18 @@ export class PhishingDetectionService { phishingDetectionSettingsService: PhishingDetectionSettingsServiceAbstraction, messageListener: MessageListener, ) { - // If already initialized, clean up first to prevent memory leaks on service worker restart if (this._didInit) { - logService.debug( - "[PhishingDetectionService] Initialize already called. Cleaning up previous instance first.", - ); - // Clean up previous state - if (this._triggerUpdateSub) { - this._triggerUpdateSub.unsubscribe(); - this._triggerUpdateSub = null; - } - if (this._boundTabHandler) { - BrowserApi.removeListener(chrome.tabs.onUpdated, this._boundTabHandler); - this._boundTabHandler = null; - } - // Clear accumulated state - this._ignoredHostnames.clear(); - // Reset flag to allow re-initialization - this._didInit = false; + logService.debug("[PhishingDetectionService] Initialize already called. Aborting."); + return; } - this._boundTabHandler = this._handleTabUpdated.bind(this) as ( - ...args: readonly unknown[] - ) => unknown; - BrowserApi.addListener(chrome.tabs.onUpdated, this._boundTabHandler); + logService.debug("[PhishingDetectionService] Initialize called. Checking prerequisites..."); + + BrowserApi.addListener(chrome.tabs.onUpdated, this._handleTabUpdated.bind(this)); const onContinueCommand$ = messageListener.messages$(PHISHING_DETECTION_CONTINUE_COMMAND).pipe( tap((message) => - logService.debug(`[PhishingDetectionService] User selected continue for ${message.url}`), + logService.debug(`[PhishingDetectionService] user selected continue for ${message.url}`), ), concatMap(async (message) => { const url = new URL(message.url); @@ -108,9 +87,7 @@ export class PhishingDetectionService { prev.tabId === curr.tabId && prev.ignored === curr.ignored, ), - tap((event) => - logService.debug(`[PhishingDetectionService] Processing navigation event:`, event), - ), + tap((event) => logService.debug(`[PhishingDetectionService] processing event:`, event)), concatMap(async ({ tabId, url, ignored }) => { if (ignored) { // The next time this host is visited, block again @@ -136,58 +113,23 @@ export class PhishingDetectionService { const phishingDetectionActive$ = phishingDetectionSettingsService.on$; - // CRITICAL: Only subscribe to update$ if phishing detection is available - // This prevents storage access for non-premium users on extension reload - // The subscription is created lazily when phishing detection becomes active - let updateSub: Subscription | null = null; - const initSub = phishingDetectionActive$ .pipe( distinctUntilChanged(), switchMap((activeUserHasAccess) => { - // Clean up previous trigger subscription if it exists - // This prevents memory leaks when account access changes (switch, lock/unlock) - if (this._triggerUpdateSub) { - this._triggerUpdateSub.unsubscribe(); - this._triggerUpdateSub = null; - } - if (!activeUserHasAccess) { logService.debug( "[PhishingDetectionService] User does not have access to phishing detection service.", ); - // Unsubscribe from update$ if user loses access (e.g., account switch to non-premium) - if (updateSub) { - updateSub.unsubscribe(); - updateSub = null; - } return EMPTY; } else { logService.debug("[PhishingDetectionService] Enabling phishing detection service"); - // Lazy subscription: Only subscribe to update$ when phishing detection becomes active - // This prevents storage access for non-premium users on extension reload - if (!updateSub) { - updateSub = phishingDataService.update$.subscribe({ - next: () => { - logService.debug("[PhishingDetectionService] Update completed"); - }, - error: (err: unknown) => { - logService.error("[PhishingDetectionService] Update error", err); - }, - complete: () => { - logService.debug("[PhishingDetectionService] Update subscription completed"); - }, - }); - } - // Trigger cache update asynchronously using RxJS delay(0) - // This defers to the next event loop tick, preventing UI blocking during account switch - // CRITICAL: Store subscription to prevent memory leaks on account switches - this._triggerUpdateSub = of(null) - .pipe(delay(0)) - .subscribe(() => phishingDataService.triggerUpdateIfNeeded()); - // update$ removed from merge - popup no longer blocks waiting for update - // The actual update runs via updateSub above - return merge(onContinueCommand$, onTabUpdated$, onCancelCommand$); + return merge( + phishingDataService.update$, + onContinueCommand$, + onTabUpdated$, + onCancelCommand$, + ); } }), ) @@ -195,26 +137,16 @@ export class PhishingDetectionService { this._didInit = true; return () => { - logService.debug("[PhishingDetectionService] Cleanup function called"); - if (updateSub) { - updateSub.unsubscribe(); - updateSub = null; - } initSub.unsubscribe(); - // Clean up trigger subscription to prevent memory leaks - if (this._triggerUpdateSub) { - this._triggerUpdateSub.unsubscribe(); - this._triggerUpdateSub = null; - } this._didInit = false; - if (this._boundTabHandler) { - BrowserApi.removeListener(chrome.tabs.onUpdated, this._boundTabHandler); - this._boundTabHandler = null; - } - - // Clear accumulated state to prevent memory leaks - this._ignoredHostnames.clear(); + // Manually type cast to satisfy the listener signature due to the mixture + // of static and instance methods in this class. To be fixed when refactoring + // this class to be instance-based while providing a singleton instance in usage + BrowserApi.removeListener( + chrome.tabs.onUpdated, + PhishingDetectionService._handleTabUpdated as (...args: readonly unknown[]) => unknown, + ); }; } diff --git a/apps/browser/src/popup/app-routing.module.ts b/apps/browser/src/popup/app-routing.module.ts index 12e1288e806..6838d4940ab 100644 --- a/apps/browser/src/popup/app-routing.module.ts +++ b/apps/browser/src/popup/app-routing.module.ts @@ -42,6 +42,7 @@ import { TwoFactorAuthComponent, TwoFactorAuthGuard, } from "@bitwarden/auth/angular"; +import { canAccessAutoConfirmSettings } from "@bitwarden/auto-confirm"; import { AnonLayoutWrapperComponent, AnonLayoutWrapperData } from "@bitwarden/components"; import { LockComponent, @@ -90,6 +91,7 @@ import { } from "../vault/popup/guards/at-risk-passwords.guard"; import { clearVaultStateGuard } from "../vault/popup/guards/clear-vault-state.guard"; import { IntroCarouselGuard } from "../vault/popup/guards/intro-carousel.guard"; +import { AdminSettingsComponent } from "../vault/popup/settings/admin-settings.component"; import { AppearanceV2Component } from "../vault/popup/settings/appearance-v2.component"; import { ArchiveComponent } from "../vault/popup/settings/archive.component"; import { DownloadBitwardenComponent } from "../vault/popup/settings/download-bitwarden.component"; @@ -332,6 +334,12 @@ const routes: Routes = [ canActivate: [authGuard], data: { elevation: 1 } satisfies RouteDataProperties, }, + { + path: "admin", + component: AdminSettingsComponent, + canActivate: [authGuard, canAccessAutoConfirmSettings], + data: { elevation: 1 } satisfies RouteDataProperties, + }, { path: "clone-cipher", component: AddEditV2Component, diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index cb6ee51f98c..c462e798a42 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -3,7 +3,11 @@ import { APP_INITIALIZER, NgModule, NgZone } from "@angular/core"; import { merge, of, Subject } from "rxjs"; -import { CollectionService } from "@bitwarden/admin-console/common"; +import { + CollectionService, + OrganizationUserApiService, + OrganizationUserService, +} from "@bitwarden/admin-console/common"; import { DeviceManagementComponentServiceAbstraction } from "@bitwarden/angular/auth/device-management/device-management-component.service.abstraction"; import { ChangePasswordService } from "@bitwarden/angular/auth/password-management/change-password"; import { AngularThemingService } from "@bitwarden/angular/platform/services/theming/angular-theming.service"; @@ -40,11 +44,18 @@ import { LogoutService, UserDecryptionOptionsServiceAbstraction, } from "@bitwarden/auth/common"; +import { + AutomaticUserConfirmationService, + DefaultAutomaticUserConfirmationService, +} from "@bitwarden/auto-confirm"; import { ExtensionAuthRequestAnsweringService } from "@bitwarden/browser/auth/services/auth-request-answering/extension-auth-request-answering.service"; import { ExtensionNewDeviceVerificationComponentService } from "@bitwarden/browser/auth/services/new-device-verification/extension-new-device-verification-component.service"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { EventCollectionService as EventCollectionServiceAbstraction } from "@bitwarden/common/abstractions/event/event-collection.service"; -import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { + InternalOrganizationServiceAbstraction, + OrganizationService, +} from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AccountService, @@ -745,6 +756,19 @@ const safeProviders: SafeProvider[] = [ useClass: ExtensionNewDeviceVerificationComponentService, deps: [], }), + safeProvider({ + provide: AutomaticUserConfirmationService, + useClass: DefaultAutomaticUserConfirmationService, + deps: [ + ConfigService, + ApiService, + OrganizationUserService, + StateProvider, + InternalOrganizationServiceAbstraction, + OrganizationUserApiService, + PolicyService, + ], + }), safeProvider({ provide: SessionTimeoutTypeService, useClass: BrowserSessionTimeoutTypeService, diff --git a/apps/browser/src/tools/popup/settings/settings-v2.component.html b/apps/browser/src/tools/popup/settings/settings-v2.component.html index 06c89e15f59..c6f1c9dbc3b 100644 --- a/apps/browser/src/tools/popup/settings/settings-v2.component.html +++ b/apps/browser/src/tools/popup/settings/settings-v2.component.html @@ -82,6 +82,24 @@ + + @if (showAdminSettingsLink$ | async) { + + + +
+

{{ "admin" | i18n }}

+ @if (showAdminBadge$ | async) { + 1 + } +
+ +
+
+ } + diff --git a/apps/browser/src/tools/popup/settings/settings-v2.component.spec.ts b/apps/browser/src/tools/popup/settings/settings-v2.component.spec.ts index 4cc3ed0149c..a05fa45753e 100644 --- a/apps/browser/src/tools/popup/settings/settings-v2.component.spec.ts +++ b/apps/browser/src/tools/popup/settings/settings-v2.component.spec.ts @@ -6,6 +6,7 @@ import { BehaviorSubject, firstValueFrom, of, Subject } from "rxjs"; import { PremiumUpgradeDialogComponent } from "@bitwarden/angular/billing/components"; import { NudgesService, NudgeType } from "@bitwarden/angular/vault"; +import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; import { AutofillBrowserSettingsService } from "@bitwarden/browser/autofill/services/autofill-browser-settings.service"; import { BrowserApi } from "@bitwarden/browser/platform/browser/browser-api"; import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; @@ -42,6 +43,9 @@ describe("SettingsV2Component", () => { defaultBrowserAutofillDisabled$: Subject; isBrowserAutofillSettingOverridden: jest.Mock>; }; + let mockAutoConfirmService: { + canManageAutoConfirm$: jest.Mock; + }; let dialogService: MockProxy; let openSpy: jest.SpyInstance; @@ -66,6 +70,10 @@ describe("SettingsV2Component", () => { isBrowserAutofillSettingOverridden: jest.fn().mockResolvedValue(false), }; + mockAutoConfirmService = { + canManageAutoConfirm$: jest.fn().mockReturnValue(of(false)), + }; + jest.spyOn(BrowserApi, "getBrowserClientVendor").mockReturnValue("Chrome"); const cfg = TestBed.configureTestingModule({ @@ -75,6 +83,7 @@ describe("SettingsV2Component", () => { { provide: BillingAccountProfileStateService, useValue: mockBillingState }, { provide: NudgesService, useValue: mockNudges }, { provide: AutofillBrowserSettingsService, useValue: mockAutofillSettings }, + { provide: AutomaticUserConfirmationService, useValue: mockAutoConfirmService }, { provide: DialogService, useValue: dialogService }, { provide: I18nService, useValue: { t: jest.fn((key: string) => key) } }, { provide: GlobalStateProvider, useValue: new FakeGlobalStateProvider() }, diff --git a/apps/browser/src/tools/popup/settings/settings-v2.component.ts b/apps/browser/src/tools/popup/settings/settings-v2.component.ts index e10d41b9445..2c9f893c99c 100644 --- a/apps/browser/src/tools/popup/settings/settings-v2.component.ts +++ b/apps/browser/src/tools/popup/settings/settings-v2.component.ts @@ -7,7 +7,9 @@ import { PremiumUpgradeDialogComponent } from "@bitwarden/angular/billing/compon import { JslibModule } from "@bitwarden/angular/jslib.module"; import { NudgesService, NudgeType } from "@bitwarden/angular/vault"; import { SpotlightComponent } from "@bitwarden/angular/vault/components/spotlight/spotlight.component"; +import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; import { UserId } from "@bitwarden/common/types/guid"; import { @@ -65,13 +67,25 @@ export class SettingsV2Component { ), ); + showAdminBadge$: Observable = this.authenticatedAccount$.pipe( + switchMap((account) => + this.nudgesService.showNudgeBadge$(NudgeType.AutoConfirmNudge, account.id), + ), + ); + showAutofillBadge$: Observable = this.authenticatedAccount$.pipe( switchMap((account) => this.nudgesService.showNudgeBadge$(NudgeType.AutofillNudge, account.id)), ); + showAdminSettingsLink$: Observable = this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.autoConfirmService.canManageAutoConfirm$(userId)), + ); + constructor( private readonly nudgesService: NudgesService, private readonly accountService: AccountService, + private readonly autoConfirmService: AutomaticUserConfirmationService, private readonly accountProfileStateService: BillingAccountProfileStateService, private readonly dialogService: DialogService, ) {} diff --git a/apps/browser/src/vault/popup/settings/admin-settings.component.html b/apps/browser/src/vault/popup/settings/admin-settings.component.html new file mode 100644 index 00000000000..5e67750278f --- /dev/null +++ b/apps/browser/src/vault/popup/settings/admin-settings.component.html @@ -0,0 +1,41 @@ + + + + + + + +
+ @if (showAutoConfirmSpotlight$ | async) { + +
+ + {{ "autoConfirmOnboardingCallout" | i18n }} + + + +
+
+ } + +
+ + + + + {{ "automaticUserConfirmation" | i18n }} + + + {{ "automaticUserConfirmationHint" | i18n }} + + +
+
+
diff --git a/apps/browser/src/vault/popup/settings/admin-settings.component.spec.ts b/apps/browser/src/vault/popup/settings/admin-settings.component.spec.ts new file mode 100644 index 00000000000..f7b4e7b473a --- /dev/null +++ b/apps/browser/src/vault/popup/settings/admin-settings.component.spec.ts @@ -0,0 +1,199 @@ +import { ChangeDetectionStrategy, Component, input } from "@angular/core"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { provideNoopAnimations } from "@angular/platform-browser/animations"; +import { mock, MockProxy } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { NudgesService, NudgeType } from "@bitwarden/angular/vault"; +import { AutoConfirmState, AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; +import { PopOutComponent } from "@bitwarden/browser/platform/popup/components/pop-out.component"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { mockAccountServiceWith } from "@bitwarden/common/spec"; +import { UserId } from "@bitwarden/common/types/guid"; +import { DialogService } from "@bitwarden/components"; + +import { PopupHeaderComponent } from "../../../platform/popup/layout/popup-header.component"; +import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.component"; + +import { AdminSettingsComponent } from "./admin-settings.component"; + +@Component({ + selector: "popup-header", + template: ``, + changeDetection: ChangeDetectionStrategy.OnPush, +}) +class MockPopupHeaderComponent { + readonly pageTitle = input(); + readonly backAction = input<() => void>(); +} + +@Component({ + selector: "popup-page", + template: ``, + changeDetection: ChangeDetectionStrategy.OnPush, +}) +class MockPopupPageComponent { + readonly loading = input(); +} + +@Component({ + selector: "app-pop-out", + template: ``, + changeDetection: ChangeDetectionStrategy.OnPush, +}) +class MockPopOutComponent { + readonly show = input(true); +} + +describe("AdminSettingsComponent", () => { + let component: AdminSettingsComponent; + let fixture: ComponentFixture; + let autoConfirmService: MockProxy; + let nudgesService: MockProxy; + let mockDialogService: MockProxy; + + const userId = "test-user-id" as UserId; + const mockAutoConfirmState: AutoConfirmState = { + enabled: false, + showSetupDialog: true, + showBrowserNotification: false, + }; + + beforeEach(async () => { + autoConfirmService = mock(); + nudgesService = mock(); + mockDialogService = mock(); + + autoConfirmService.configuration$.mockReturnValue(of(mockAutoConfirmState)); + autoConfirmService.upsert.mockResolvedValue(undefined); + nudgesService.showNudgeSpotlight$.mockReturnValue(of(false)); + + await TestBed.configureTestingModule({ + imports: [AdminSettingsComponent], + providers: [ + provideNoopAnimations(), + { provide: AccountService, useValue: mockAccountServiceWith(userId) }, + { provide: AutomaticUserConfirmationService, useValue: autoConfirmService }, + { provide: DialogService, useValue: mockDialogService }, + { provide: NudgesService, useValue: nudgesService }, + { provide: I18nService, useValue: { t: (key: string) => key } }, + ], + }) + .overrideComponent(AdminSettingsComponent, { + remove: { + imports: [PopupHeaderComponent, PopupPageComponent, PopOutComponent], + }, + add: { + imports: [MockPopupHeaderComponent, MockPopupPageComponent, MockPopOutComponent], + }, + }) + .compileComponents(); + + fixture = TestBed.createComponent(AdminSettingsComponent); + component = fixture.componentInstance; + }); + + describe("initialization", () => { + it("should populate form with current auto-confirm state", async () => { + const mockState: AutoConfirmState = { + enabled: true, + showSetupDialog: false, + showBrowserNotification: true, + }; + autoConfirmService.configuration$.mockReturnValue(of(mockState)); + + await component.ngOnInit(); + fixture.detectChanges(); + await fixture.whenStable(); + + expect(component["adminForm"].value).toEqual({ + autoConfirm: true, + }); + }); + + it("should populate form with disabled auto-confirm state", async () => { + await component.ngOnInit(); + fixture.detectChanges(); + await fixture.whenStable(); + + expect(component["adminForm"].value).toEqual({ + autoConfirm: false, + }); + }); + }); + + describe("spotlight", () => { + beforeEach(async () => { + await component.ngOnInit(); + fixture.detectChanges(); + }); + + it("should expose showAutoConfirmSpotlight$ observable", (done) => { + nudgesService.showNudgeSpotlight$.mockReturnValue(of(true)); + + const newFixture = TestBed.createComponent(AdminSettingsComponent); + const newComponent = newFixture.componentInstance; + + newComponent["showAutoConfirmSpotlight$"].subscribe((show) => { + expect(show).toBe(true); + expect(nudgesService.showNudgeSpotlight$).toHaveBeenCalledWith( + NudgeType.AutoConfirmNudge, + userId, + ); + done(); + }); + }); + + it("should dismiss spotlight and update state", async () => { + autoConfirmService.upsert.mockResolvedValue(); + + await component.dismissSpotlight(); + + expect(autoConfirmService.upsert).toHaveBeenCalledWith(userId, { + ...mockAutoConfirmState, + showBrowserNotification: false, + }); + }); + + it("should use current userId when dismissing spotlight", async () => { + autoConfirmService.upsert.mockResolvedValue(); + + await component.dismissSpotlight(); + + expect(autoConfirmService.upsert).toHaveBeenCalledWith(userId, expect.any(Object)); + }); + + it("should preserve existing state when dismissing spotlight", async () => { + const customState: AutoConfirmState = { + enabled: true, + showSetupDialog: false, + showBrowserNotification: true, + }; + autoConfirmService.configuration$.mockReturnValue(of(customState)); + autoConfirmService.upsert.mockResolvedValue(); + + await component.dismissSpotlight(); + + expect(autoConfirmService.upsert).toHaveBeenCalledWith(userId, { + ...customState, + showBrowserNotification: false, + }); + }); + }); + + describe("form validation", () => { + beforeEach(async () => { + await component.ngOnInit(); + fixture.detectChanges(); + }); + + it("should have a valid form", () => { + expect(component["adminForm"].valid).toBe(true); + }); + + it("should have autoConfirm control", () => { + expect(component["adminForm"].controls.autoConfirm).toBeDefined(); + }); + }); +}); diff --git a/apps/browser/src/vault/popup/settings/admin-settings.component.ts b/apps/browser/src/vault/popup/settings/admin-settings.component.ts new file mode 100644 index 00000000000..e4b676525ed --- /dev/null +++ b/apps/browser/src/vault/popup/settings/admin-settings.component.ts @@ -0,0 +1,121 @@ +import { CommonModule } from "@angular/common"; +import { + ChangeDetectionStrategy, + Component, + DestroyRef, + OnInit, + signal, + WritableSignal, +} from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; +import { FormBuilder, ReactiveFormsModule } from "@angular/forms"; +import { firstValueFrom, map, Observable, of, switchMap, tap, withLatestFrom } from "rxjs"; + +import { NudgesService, NudgeType } from "@bitwarden/angular/vault"; +import { SpotlightComponent } from "@bitwarden/angular/vault/components/spotlight/spotlight.component"; +import { + AutoConfirmWarningDialogComponent, + AutomaticUserConfirmationService, +} from "@bitwarden/auto-confirm"; +import { PopOutComponent } from "@bitwarden/browser/platform/popup/components/pop-out.component"; +import { PopupHeaderComponent } from "@bitwarden/browser/platform/popup/layout/popup-header.component"; +import { PopupPageComponent } from "@bitwarden/browser/platform/popup/layout/popup-page.component"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; +import { + BitIconButtonComponent, + CardComponent, + DialogService, + FormFieldModule, + SwitchComponent, +} from "@bitwarden/components"; +import { I18nPipe } from "@bitwarden/ui-common"; +import { UserId } from "@bitwarden/user-core"; + +@Component({ + templateUrl: "./admin-settings.component.html", + changeDetection: ChangeDetectionStrategy.OnPush, + imports: [ + CommonModule, + PopupPageComponent, + PopupHeaderComponent, + PopOutComponent, + FormFieldModule, + ReactiveFormsModule, + SwitchComponent, + CardComponent, + SpotlightComponent, + BitIconButtonComponent, + I18nPipe, + ], +}) +export class AdminSettingsComponent implements OnInit { + private userId$: Observable = this.accountService.activeAccount$.pipe(getUserId); + + protected readonly formLoading: WritableSignal = signal(true); + protected adminForm = this.formBuilder.group({ + autoConfirm: false, + }); + protected showAutoConfirmSpotlight$: Observable = this.userId$.pipe( + switchMap((userId) => + this.nudgesService.showNudgeSpotlight$(NudgeType.AutoConfirmNudge, userId), + ), + ); + + constructor( + private formBuilder: FormBuilder, + private accountService: AccountService, + private autoConfirmService: AutomaticUserConfirmationService, + private destroyRef: DestroyRef, + private dialogService: DialogService, + private nudgesService: NudgesService, + ) {} + + async ngOnInit() { + const userId = await firstValueFrom(this.userId$); + const autoConfirmEnabled = ( + await firstValueFrom(this.autoConfirmService.configuration$(userId)) + ).enabled; + this.adminForm.setValue({ autoConfirm: autoConfirmEnabled }); + + this.formLoading.set(false); + + this.adminForm.controls.autoConfirm.valueChanges + .pipe( + switchMap((newValue) => { + if (newValue) { + return this.confirm(); + } + return of(false); + }), + withLatestFrom(this.autoConfirmService.configuration$(userId)), + switchMap(([newValue, existingState]) => + this.autoConfirmService.upsert(userId, { + ...existingState, + enabled: newValue, + showBrowserNotification: false, + }), + ), + takeUntilDestroyed(this.destroyRef), + ) + .subscribe(); + } + + private confirm(): Observable { + return AutoConfirmWarningDialogComponent.open(this.dialogService).closed.pipe( + map((result) => result ?? false), + tap((result) => { + if (!result) { + this.adminForm.setValue({ autoConfirm: false }, { emitEvent: false }); + } + }), + ); + } + + async dismissSpotlight() { + const userId = await firstValueFrom(this.userId$); + const state = await firstValueFrom(this.autoConfirmService.configuration$(userId)); + + await this.autoConfirmService.upsert(userId, { ...state, showBrowserNotification: false }); + } +} diff --git a/apps/desktop/desktop_native/autotype/src/lib.rs b/apps/desktop/desktop_native/autotype/src/lib.rs index 4b9e65180e6..917f0f797b6 100644 --- a/apps/desktop/desktop_native/autotype/src/lib.rs +++ b/apps/desktop/desktop_native/autotype/src/lib.rs @@ -1,5 +1,11 @@ use anyhow::Result; +#[cfg(target_os = "windows")] +mod modifier_keys; + +#[cfg(target_os = "windows")] +pub(crate) use modifier_keys::*; + #[cfg_attr(target_os = "linux", path = "linux.rs")] #[cfg_attr(target_os = "macos", path = "macos.rs")] #[cfg_attr(target_os = "windows", path = "windows/mod.rs")] diff --git a/apps/desktop/desktop_native/autotype/src/modifier_keys.rs b/apps/desktop/desktop_native/autotype/src/modifier_keys.rs new file mode 100644 index 00000000000..c451a3b25e4 --- /dev/null +++ b/apps/desktop/desktop_native/autotype/src/modifier_keys.rs @@ -0,0 +1,45 @@ +// Electron modifier keys +// +pub(crate) const CONTROL_KEY_STR: &str = "Control"; +pub(crate) const ALT_KEY_STR: &str = "Alt"; +pub(crate) const SUPER_KEY_STR: &str = "Super"; + +// numeric values for modifier keys +pub(crate) const CONTROL_KEY: u16 = 0x11; +pub(crate) const ALT_KEY: u16 = 0x12; +pub(crate) const SUPER_KEY: u16 = 0x5B; + +/// A mapping of to +static MODIFIER_KEYS: [(&str, u16); 3] = [ + (CONTROL_KEY_STR, CONTROL_KEY), + (ALT_KEY_STR, ALT_KEY), + (SUPER_KEY_STR, SUPER_KEY), +]; + +/// Provides a mapping of the valid modifier keys' electron +/// string representation to the numeric representation. +pub(crate) fn get_numeric_modifier_key(modifier: &str) -> Option { + for (modifier_str, modifier_num) in MODIFIER_KEYS { + if modifier_str == modifier { + return Some(modifier_num); + } + } + None +} + +#[cfg(test)] +mod test { + use super::get_numeric_modifier_key; + + #[test] + fn valid_modifier_keys() { + assert_eq!(get_numeric_modifier_key("Control").unwrap(), 0x11); + assert_eq!(get_numeric_modifier_key("Alt").unwrap(), 0x12); + assert_eq!(get_numeric_modifier_key("Super").unwrap(), 0x5B); + } + + #[test] + fn does_not_contain_invalid_modifier_keys() { + assert!(get_numeric_modifier_key("Shift").is_none()); + } +} diff --git a/apps/desktop/desktop_native/autotype/src/windows/mod.rs b/apps/desktop/desktop_native/autotype/src/windows/mod.rs index 9cd9bc0cbe5..ed985749303 100644 --- a/apps/desktop/desktop_native/autotype/src/windows/mod.rs +++ b/apps/desktop/desktop_native/autotype/src/windows/mod.rs @@ -44,7 +44,6 @@ pub fn get_foreground_window_title() -> Result { /// - Control /// - Alt /// - Super -/// - Shift /// - \[a-z\]\[A-Z\] struct KeyboardShortcutInput(INPUT); diff --git a/apps/desktop/desktop_native/autotype/src/windows/type_input.rs b/apps/desktop/desktop_native/autotype/src/windows/type_input.rs index b62dd7290d1..f7879e676bf 100644 --- a/apps/desktop/desktop_native/autotype/src/windows/type_input.rs +++ b/apps/desktop/desktop_native/autotype/src/windows/type_input.rs @@ -6,11 +6,7 @@ use windows::Win32::UI::Input::KeyboardAndMouse::{ }; use super::{ErrorOperations, KeyboardShortcutInput, Win32ErrorOperations}; - -const SHIFT_KEY_STR: &str = "Shift"; -const CONTROL_KEY_STR: &str = "Control"; -const ALT_KEY_STR: &str = "Alt"; -const LEFT_WINDOWS_KEY_STR: &str = "Super"; +use crate::get_numeric_modifier_key; const IS_VIRTUAL_KEY: bool = true; const IS_REAL_KEY: bool = false; @@ -88,22 +84,19 @@ impl TryFrom<&str> for KeyboardShortcutInput { type Error = anyhow::Error; fn try_from(key: &str) -> std::result::Result { - const SHIFT_KEY: u16 = 0x10; - const CONTROL_KEY: u16 = 0x11; - const ALT_KEY: u16 = 0x12; - const LEFT_WINDOWS_KEY: u16 = 0x5B; - + // not modifier key + if key.len() == 1 { + let input = build_unicode_input(InputKeyPress::Up, get_alphabetic_hotkey(key)?); + return Ok(KeyboardShortcutInput(input)); + } // the modifier keys are using the Up keypress variant because the user has already // pressed those keys in order to trigger the feature. - let input = match key { - SHIFT_KEY_STR => build_virtual_key_input(InputKeyPress::Up, SHIFT_KEY), - CONTROL_KEY_STR => build_virtual_key_input(InputKeyPress::Up, CONTROL_KEY), - ALT_KEY_STR => build_virtual_key_input(InputKeyPress::Up, ALT_KEY), - LEFT_WINDOWS_KEY_STR => build_virtual_key_input(InputKeyPress::Up, LEFT_WINDOWS_KEY), - _ => build_unicode_input(InputKeyPress::Up, get_alphabetic_hotkey(key)?), - }; - - Ok(KeyboardShortcutInput(input)) + if let Some(numeric_modifier_key) = get_numeric_modifier_key(key) { + let input = build_virtual_key_input(InputKeyPress::Up, numeric_modifier_key); + Ok(KeyboardShortcutInput(input)) + } else { + Err(anyhow!("Unsupported modifier key: {key}")) + } } } @@ -278,7 +271,7 @@ mod tests { #[test] #[serial] fn keyboard_shortcut_conversion_succeeds() { - let keyboard_shortcut = [CONTROL_KEY_STR, SHIFT_KEY_STR, "B"]; + let keyboard_shortcut = ["Control", "Alt", "B"]; let _: Vec = keyboard_shortcut .iter() .map(|s| KeyboardShortcutInput::try_from(*s)) @@ -290,7 +283,19 @@ mod tests { #[serial] #[should_panic = "Letter is not ASCII Alphabetic ([a-z][A-Z]): '1'"] fn keyboard_shortcut_conversion_fails_invalid_key() { - let keyboard_shortcut = [CONTROL_KEY_STR, SHIFT_KEY_STR, "1"]; + let keyboard_shortcut = ["Control", "Alt", "1"]; + let _: Vec = keyboard_shortcut + .iter() + .map(|s| KeyboardShortcutInput::try_from(*s)) + .try_collect() + .unwrap(); + } + + #[test] + #[serial] + #[should_panic(expected = "Unsupported modifier key: Shift")] + fn keyboard_shortcut_conversion_fails_with_shift() { + let keyboard_shortcut = ["Control", "Shift", "B"]; let _: Vec = keyboard_shortcut .iter() .map(|s| KeyboardShortcutInput::try_from(*s)) diff --git a/apps/desktop/src/app/accounts/settings.component.spec.ts b/apps/desktop/src/app/accounts/settings.component.spec.ts index d518ac29aa4..bffa06d2654 100644 --- a/apps/desktop/src/app/accounts/settings.component.spec.ts +++ b/apps/desktop/src/app/accounts/settings.component.spec.ts @@ -188,7 +188,7 @@ describe("SettingsComponent", () => { pinServiceAbstraction.isPinSet.mockResolvedValue(false); policyService.policiesByType$.mockReturnValue(of([null])); desktopAutotypeService.autotypeEnabledUserSetting$ = of(false); - desktopAutotypeService.autotypeKeyboardShortcut$ = of(["Control", "Shift", "B"]); + desktopAutotypeService.autotypeKeyboardShortcut$ = of(["Control", "Alt", "B"]); billingAccountProfileStateService.hasPremiumFromAnySource$.mockReturnValue(of(false)); configService.getFeatureFlag$.mockReturnValue(of(false)); }); diff --git a/apps/desktop/src/autofill/components/autotype-shortcut.component.html b/apps/desktop/src/autofill/components/autotype-shortcut.component.html index 6f73d4006ac..feb1f507c97 100644 --- a/apps/desktop/src/autofill/components/autotype-shortcut.component.html +++ b/apps/desktop/src/autofill/components/autotype-shortcut.component.html @@ -5,7 +5,7 @@

- {{ "editAutotypeShortcutDescription" | i18n }} + {{ "editAutotypeKeyboardModifiersDescription" | i18n }}

{{ "typeShortcut" | i18n }} diff --git a/apps/desktop/src/autofill/components/autotype-shortcut.component.spec.ts b/apps/desktop/src/autofill/components/autotype-shortcut.component.spec.ts index 90aa493c596..ea394274600 100644 --- a/apps/desktop/src/autofill/components/autotype-shortcut.component.spec.ts +++ b/apps/desktop/src/autofill/components/autotype-shortcut.component.spec.ts @@ -30,11 +30,9 @@ describe("AutotypeShortcutComponent", () => { const validShortcuts = [ "Control+A", "Alt+B", - "Shift+C", "Win+D", "control+e", // case insensitive "ALT+F", - "SHIFT+G", "WIN+H", ]; @@ -46,14 +44,7 @@ describe("AutotypeShortcutComponent", () => { }); it("should accept two modifiers with letter", () => { - const validShortcuts = [ - "Control+Alt+A", - "Control+Shift+B", - "Control+Win+C", - "Alt+Shift+D", - "Alt+Win+E", - "Shift+Win+F", - ]; + const validShortcuts = ["Control+Alt+A", "Control+Win+C", "Alt+Win+D", "Alt+Win+E"]; validShortcuts.forEach((shortcut) => { const control = createControl(shortcut); @@ -63,7 +54,7 @@ describe("AutotypeShortcutComponent", () => { }); it("should accept modifiers in different orders", () => { - const validShortcuts = ["Alt+Control+A", "Shift+Control+B", "Win+Alt+C"]; + const validShortcuts = ["Alt+Control+A", "Win+Control+B", "Win+Alt+C"]; validShortcuts.forEach((shortcut) => { const control = createControl(shortcut); @@ -88,15 +79,14 @@ describe("AutotypeShortcutComponent", () => { const invalidShortcuts = [ "Control+1", "Alt+2", - "Shift+3", "Win+4", "Control+!", "Alt+@", - "Shift+#", + "Alt+#", "Win+$", "Control+Space", "Alt+Enter", - "Shift+Tab", + "Control+Tab", "Win+Escape", ]; @@ -111,12 +101,10 @@ describe("AutotypeShortcutComponent", () => { const invalidShortcuts = [ "Control", "Alt", - "Shift", "Win", "Control+Alt", - "Control+Shift", - "Alt+Shift", - "Control+Alt+Shift", + "Control+Win", + "Control+Alt+Win", ]; invalidShortcuts.forEach((shortcut) => { @@ -127,7 +115,7 @@ describe("AutotypeShortcutComponent", () => { }); it("should reject shortcuts with invalid modifier names", () => { - const invalidShortcuts = ["Ctrl+A", "Command+A", "Super+A", "Meta+A", "Cmd+A", "Invalid+A"]; + const invalidShortcuts = ["Ctrl+A", "Command+A", "Meta+A", "Cmd+A", "Invalid+A"]; invalidShortcuts.forEach((shortcut) => { const control = createControl(shortcut); @@ -137,7 +125,7 @@ describe("AutotypeShortcutComponent", () => { }); it("should reject shortcuts with multiple base keys", () => { - const invalidShortcuts = ["Control+A+B", "Alt+Ctrl+Shift"]; + const invalidShortcuts = ["Control+A+B", "Alt+Ctrl+Win"]; invalidShortcuts.forEach((shortcut) => { const control = createControl(shortcut); @@ -148,11 +136,10 @@ describe("AutotypeShortcutComponent", () => { it("should reject shortcuts with more than two modifiers", () => { const invalidShortcuts = [ - "Control+Alt+Shift+A", + "Control+Alt+Win+A", "Control+Alt+Win+B", - "Control+Shift+Win+C", - "Alt+Shift+Win+D", - "Control+Alt+Shift+Win+E", + "Control+Alt+Win+C", + "Alt+Control+Win+D", ]; invalidShortcuts.forEach((shortcut) => { @@ -221,7 +208,7 @@ describe("AutotypeShortcutComponent", () => { }); it("should handle very long strings", () => { - const longString = "Control+Alt+Shift+Win+A".repeat(100); + const longString = "Control+Alt+Win+A".repeat(100); const control = createControl(longString); const result = validator(control); expect(result).toEqual({ invalidShortcut: { message: "Invalid shortcut" } }); @@ -230,7 +217,7 @@ describe("AutotypeShortcutComponent", () => { describe("modifier combinations", () => { it("should accept all possible single modifier combinations", () => { - const modifiers = ["Control", "Alt", "Shift", "Win"]; + const modifiers = ["Control", "Alt", "Win"]; modifiers.forEach((modifier) => { const control = createControl(`${modifier}+A`); @@ -240,14 +227,7 @@ describe("AutotypeShortcutComponent", () => { }); it("should accept all possible two-modifier combinations", () => { - const combinations = [ - "Control+Alt+A", - "Control+Shift+A", - "Control+Win+A", - "Alt+Shift+A", - "Alt+Win+A", - "Shift+Win+A", - ]; + const combinations = ["Control+Alt+A", "Control+Win+A", "Alt+Win+A"]; combinations.forEach((shortcut) => { const control = createControl(shortcut); @@ -257,12 +237,7 @@ describe("AutotypeShortcutComponent", () => { }); it("should reject all three-modifier combinations", () => { - const combinations = [ - "Control+Alt+Shift+A", - "Control+Alt+Win+A", - "Control+Shift+Win+A", - "Alt+Shift+Win+A", - ]; + const combinations = ["Control+Alt+Win+A", "Alt+Control+Win+A", "Win+Alt+Control+A"]; combinations.forEach((shortcut) => { const control = createControl(shortcut); @@ -270,12 +245,6 @@ describe("AutotypeShortcutComponent", () => { expect(result).toEqual({ invalidShortcut: { message: "Invalid shortcut" } }); }); }); - - it("should reject all four modifiers combination", () => { - const control = createControl("Control+Alt+Shift+Win+A"); - const result = validator(control); - expect(result).toEqual({ invalidShortcut: { message: "Invalid shortcut" } }); - }); }); }); }); diff --git a/apps/desktop/src/autofill/components/autotype-shortcut.component.ts b/apps/desktop/src/autofill/components/autotype-shortcut.component.ts index 3c82d8297a1..4e1a0c2108c 100644 --- a/apps/desktop/src/autofill/components/autotype-shortcut.component.ts +++ b/apps/desktop/src/autofill/components/autotype-shortcut.component.ts @@ -77,25 +77,31 @@ export class AutotypeShortcutComponent { } } + // private buildShortcutFromEvent(event: KeyboardEvent): string | null { const hasCtrl = event.ctrlKey; const hasAlt = event.altKey; const hasShift = event.shiftKey; - const hasMeta = event.metaKey; // Windows key on Windows, Command on macOS + const hasSuper = event.metaKey; // Windows key on Windows, Command on macOS - // Require at least one modifier (Control, Alt, Shift, or Super) - if (!hasCtrl && !hasAlt && !hasShift && !hasMeta) { + // Require at least one valid modifier (Control, Alt, Super) + if (!hasCtrl && !hasAlt && !hasSuper) { return null; } const key = event.key; - // Ignore pure modifier keys themselves - if (key === "Control" || key === "Alt" || key === "Shift" || key === "Meta") { + // disallow pure modifier keys themselves + if (key === "Control" || key === "Alt" || key === "Meta") { return null; } - // Accept a single alphabetical letter as the base key + // disallow shift modifier + if (hasShift) { + return null; + } + + // require a single alphabetical letter as the base key const isAlphabetical = typeof key === "string" && /^[a-z]$/i.test(key); if (!isAlphabetical) { return null; @@ -108,10 +114,7 @@ export class AutotypeShortcutComponent { if (hasAlt) { parts.push("Alt"); } - if (hasShift) { - parts.push("Shift"); - } - if (hasMeta) { + if (hasSuper) { parts.push("Super"); } parts.push(key.toUpperCase()); @@ -129,10 +132,9 @@ export class AutotypeShortcutComponent { } // Must include exactly 1-2 modifiers and end with a single letter - // Valid examples: Ctrl+A, Shift+Z, Ctrl+Shift+X, Alt+Shift+Q + // Valid examples: Ctrl+A, Alt+B, Ctrl+Alt+X, Alt+Control+Q, Win+B, Ctrl+Win+A // Allow modifiers in any order, but only 1-2 modifiers total - const pattern = - /^(?=.*\b(Control|Alt|Shift|Win)\b)(?:Control\+|Alt\+|Shift\+|Win\+){1,2}[A-Z]$/i; + const pattern = /^(?=.*\b(Control|Alt|Win)\b)(?:Control\+|Alt\+|Win\+){1,2}[A-Z]$/i; return pattern.test(value) ? null : { invalidShortcut: { message: this.i18nService.t("invalidShortcut") } }; diff --git a/apps/desktop/src/autofill/models/main-autotype-keyboard-shortcut.ts b/apps/desktop/src/autofill/models/main-autotype-keyboard-shortcut.ts index b26be92585e..8b241ade032 100644 --- a/apps/desktop/src/autofill/models/main-autotype-keyboard-shortcut.ts +++ b/apps/desktop/src/autofill/models/main-autotype-keyboard-shortcut.ts @@ -1,4 +1,14 @@ -import { defaultWindowsAutotypeKeyboardShortcut } from "../services/desktop-autotype.service"; +/** + Electron's representation of modifier keys + +*/ +export const CONTROL_KEY_STR = "Control"; +export const ALT_KEY_STR = "Alt"; +export const SUPER_KEY_STR = "Super"; + +export const VALID_SHORTCUT_MODIFIER_KEYS: string[] = [CONTROL_KEY_STR, ALT_KEY_STR, SUPER_KEY_STR]; + +export const DEFAULT_KEYBOARD_SHORTCUT: string[] = [CONTROL_KEY_STR, ALT_KEY_STR, "B"]; /* This class provides the following: @@ -13,7 +23,7 @@ export class AutotypeKeyboardShortcut { private autotypeKeyboardShortcut: string[]; constructor() { - this.autotypeKeyboardShortcut = defaultWindowsAutotypeKeyboardShortcut; + this.autotypeKeyboardShortcut = DEFAULT_KEYBOARD_SHORTCUT; } /* @@ -51,14 +61,16 @@ export class AutotypeKeyboardShortcut { This private function validates the strArray input to make sure the array contains valid, currently accepted shortcut keys for Windows. - Valid windows shortcut keys: Control, Alt, Super, Shift, letters A - Z - Valid macOS shortcut keys: Control, Alt, Command, Shift, letters A - Z (not yet supported) + Valid shortcut keys: Control, Alt, Super, letters A - Z + Platform specifics: + - On Windows, Super maps to the Windows key. + - On MacOS, Super maps to the Command key. + - On MacOS, Alt maps to the Option key. See Electron keyboard shorcut docs for more info: https://www.electronjs.org/docs/latest/tutorial/keyboard-shortcuts */ #keyboardShortcutIsValid(strArray: string[]) { - const VALID_SHORTCUT_CONTROL_KEYS: string[] = ["Control", "Alt", "Super", "Shift"]; const UNICODE_LOWER_BOUND = 65; // unicode 'A' const UNICODE_UPPER_BOUND = 90; // unicode 'Z' const MIN_LENGTH: number = 2; @@ -77,7 +89,7 @@ export class AutotypeKeyboardShortcut { // Ensure strArray is all modifier keys, and that the last key is a letter for (let i = 0; i < strArray.length; i++) { if (i < strArray.length - 1) { - if (!VALID_SHORTCUT_CONTROL_KEYS.includes(strArray[i])) { + if (!VALID_SHORTCUT_MODIFIER_KEYS.includes(strArray[i])) { return false; } } else { diff --git a/apps/desktop/src/autofill/services/desktop-autotype.service.ts b/apps/desktop/src/autofill/services/desktop-autotype.service.ts index 46fec662d7a..d108577567d 100644 --- a/apps/desktop/src/autofill/services/desktop-autotype.service.ts +++ b/apps/desktop/src/autofill/services/desktop-autotype.service.ts @@ -33,11 +33,10 @@ import { UserId } from "@bitwarden/user-core"; import { AutotypeConfig } from "../models/autotype-config"; import { AutotypeVaultData } from "../models/autotype-vault-data"; +import { DEFAULT_KEYBOARD_SHORTCUT } from "../models/main-autotype-keyboard-shortcut"; import { DesktopAutotypeDefaultSettingPolicy } from "./desktop-autotype-policy.service"; -export const defaultWindowsAutotypeKeyboardShortcut: string[] = ["Control", "Shift", "B"]; - export const AUTOTYPE_ENABLED = new KeyDefinition( AUTOTYPE_SETTINGS_DISK, "autotypeEnabled", @@ -72,10 +71,9 @@ export class DesktopAutotypeService implements OnDestroy { private readonly isPremiumAccount$: Observable; // The enabled/disabled state from the user settings menu - autotypeEnabledUserSetting$: Observable; + autotypeEnabledUserSetting$: Observable = of(false); - // The keyboard shortcut from the user settings menu - autotypeKeyboardShortcut$: Observable = of(defaultWindowsAutotypeKeyboardShortcut); + autotypeKeyboardShortcut$: Observable = of(DEFAULT_KEYBOARD_SHORTCUT); private destroy$ = new Subject(); @@ -106,7 +104,7 @@ export class DesktopAutotypeService implements OnDestroy { ); this.autotypeKeyboardShortcut$ = this.autotypeKeyboardShortcut.state$.pipe( - map((shortcut) => shortcut ?? defaultWindowsAutotypeKeyboardShortcut), + map((shortcut) => shortcut ?? DEFAULT_KEYBOARD_SHORTCUT), takeUntil(this.destroy$), ); } diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index c47817f3ee4..b00233457ec 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -4267,8 +4267,8 @@ "typeShortcut": { "message": "Type shortcut" }, - "editAutotypeShortcutDescription": { - "message": "Include one or two of the following modifiers: Ctrl, Alt, Win, or Shift, and a letter." + "editAutotypeKeyboardModifiersDescription": { + "message": "Include one or two of the following modifiers: Ctrl, Alt, Win, and a letter." }, "invalidShortcut": { "message": "Invalid shortcut" diff --git a/apps/web/src/app/admin-console/organizations/members/index.ts b/apps/web/src/app/admin-console/organizations/members/index.ts index 95bd8baf7c7..7026b9bb6ac 100644 --- a/apps/web/src/app/admin-console/organizations/members/index.ts +++ b/apps/web/src/app/admin-console/organizations/members/index.ts @@ -1 +1,2 @@ export * from "./members.module"; +export * from "./pipes"; diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.html b/apps/web/src/app/admin-console/organizations/members/members.component.html index 84e5c33d20d..921004e315d 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.html +++ b/apps/web/src/app/admin-console/organizations/members/members.component.html @@ -102,15 +102,25 @@ {{ (organization.useGroups ? "groups" : "collections") | i18n }} {{ "role" | i18n }} {{ "policies" | i18n }} - - + +
+ + +
@@ -352,13 +362,16 @@ - +
+
+ +
diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.ts b/apps/web/src/app/admin-console/organizations/members/members.component.ts index 51a2a6dafc0..e57cf54c180 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.component.ts @@ -35,6 +35,7 @@ import { OrganizationMetadataServiceAbstraction } from "@bitwarden/common/billin import { OrganizationBillingMetadataResponse } from "@bitwarden/common/billing/models/response/organization-billing-metadata.response"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; +import { FileDownloadService } from "@bitwarden/common/platform/abstractions/file-download/file-download.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; @@ -55,7 +56,11 @@ import { OrganizationUserView } from "../core/views/organization-user.view"; import { AccountRecoveryDialogResultType } from "./components/account-recovery/account-recovery-dialog.component"; import { MemberDialogResult, MemberDialogTab } from "./components/member-dialog"; -import { MemberDialogManagerService, OrganizationMembersService } from "./services"; +import { + MemberDialogManagerService, + MemberExportService, + OrganizationMembersService, +} from "./services"; import { DeleteManagedMemberWarningService } from "./services/delete-managed-member/delete-managed-member-warning.service"; import { MemberActionsService, @@ -119,6 +124,8 @@ export class MembersComponent extends BaseMembersComponent private policyService: PolicyService, private policyApiService: PolicyApiServiceAbstraction, private organizationMetadataService: OrganizationMetadataServiceAbstraction, + private memberExportService: MemberExportService, + private fileDownloadService: FileDownloadService, private configService: ConfigService, private environmentService: EnvironmentService, ) { @@ -593,4 +600,36 @@ export class MembersComponent extends BaseMembersComponent .getCheckedUsers() .every((member) => member.managedByOrganization && validStatuses.includes(member.status)); } + + exportMembers = async (): Promise => { + try { + const members = this.dataSource.data; + if (!members || members.length === 0) { + this.toastService.showToast({ + variant: "error", + title: this.i18nService.t("errorOccurred"), + message: this.i18nService.t("noMembersToExport"), + }); + return; + } + + const csvData = this.memberExportService.getMemberExport(members); + const fileName = this.memberExportService.getFileName("org-members"); + + this.fileDownloadService.download({ + fileName: fileName, + blobData: csvData, + blobOptions: { type: "text/plain" }, + }); + + this.toastService.showToast({ + variant: "success", + title: undefined, + message: this.i18nService.t("dataExportSuccess"), + }); + } catch (e) { + this.validationService.showError(e); + this.logService.error(`Failed to export members: ${e}`); + } + }; } diff --git a/apps/web/src/app/admin-console/organizations/members/members.module.ts b/apps/web/src/app/admin-console/organizations/members/members.module.ts index 3b233932ed3..65625cfd247 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.module.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.module.ts @@ -19,10 +19,12 @@ import { BulkStatusComponent } from "./components/bulk/bulk-status.component"; import { UserDialogModule } from "./components/member-dialog"; import { MembersRoutingModule } from "./members-routing.module"; import { MembersComponent } from "./members.component"; +import { UserStatusPipe } from "./pipes"; import { OrganizationMembersService, MemberActionsService, MemberDialogManagerService, + MemberExportService, } from "./services"; @NgModule({ @@ -45,12 +47,15 @@ import { BulkStatusComponent, MembersComponent, BulkDeleteDialogComponent, + UserStatusPipe, ], providers: [ OrganizationMembersService, MemberActionsService, BillingConstraintService, MemberDialogManagerService, + MemberExportService, + UserStatusPipe, ], }) export class MembersModule {} diff --git a/apps/web/src/app/admin-console/organizations/members/pipes/index.ts b/apps/web/src/app/admin-console/organizations/members/pipes/index.ts new file mode 100644 index 00000000000..67c485ed361 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/pipes/index.ts @@ -0,0 +1 @@ +export * from "./user-status.pipe"; diff --git a/apps/web/src/app/admin-console/organizations/members/pipes/user-status.pipe.spec.ts b/apps/web/src/app/admin-console/organizations/members/pipes/user-status.pipe.spec.ts new file mode 100644 index 00000000000..3fd05c8a2e8 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/pipes/user-status.pipe.spec.ts @@ -0,0 +1,47 @@ +import { MockProxy, mock } from "jest-mock-extended"; + +import { OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; + +import { UserStatusPipe } from "./user-status.pipe"; + +describe("UserStatusPipe", () => { + let pipe: UserStatusPipe; + let i18nService: MockProxy; + + beforeEach(() => { + i18nService = mock(); + i18nService.t.mockImplementation((key: string) => key); + pipe = new UserStatusPipe(i18nService); + }); + + it("transforms OrganizationUserStatusType.Invited to 'invited'", () => { + expect(pipe.transform(OrganizationUserStatusType.Invited)).toBe("invited"); + expect(i18nService.t).toHaveBeenCalledWith("invited"); + }); + + it("transforms OrganizationUserStatusType.Accepted to 'accepted'", () => { + expect(pipe.transform(OrganizationUserStatusType.Accepted)).toBe("accepted"); + expect(i18nService.t).toHaveBeenCalledWith("accepted"); + }); + + it("transforms OrganizationUserStatusType.Confirmed to 'confirmed'", () => { + expect(pipe.transform(OrganizationUserStatusType.Confirmed)).toBe("confirmed"); + expect(i18nService.t).toHaveBeenCalledWith("confirmed"); + }); + + it("transforms OrganizationUserStatusType.Revoked to 'revoked'", () => { + expect(pipe.transform(OrganizationUserStatusType.Revoked)).toBe("revoked"); + expect(i18nService.t).toHaveBeenCalledWith("revoked"); + }); + + it("transforms null to 'unknown'", () => { + expect(pipe.transform(null)).toBe("unknown"); + expect(i18nService.t).toHaveBeenCalledWith("unknown"); + }); + + it("transforms undefined to 'unknown'", () => { + expect(pipe.transform(undefined)).toBe("unknown"); + expect(i18nService.t).toHaveBeenCalledWith("unknown"); + }); +}); diff --git a/apps/web/src/app/admin-console/organizations/members/pipes/user-status.pipe.ts b/apps/web/src/app/admin-console/organizations/members/pipes/user-status.pipe.ts new file mode 100644 index 00000000000..81590616027 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/pipes/user-status.pipe.ts @@ -0,0 +1,30 @@ +import { Pipe, PipeTransform } from "@angular/core"; + +import { OrganizationUserStatusType } from "@bitwarden/common/admin-console/enums"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; + +@Pipe({ + name: "userStatus", + standalone: false, +}) +export class UserStatusPipe implements PipeTransform { + constructor(private i18nService: I18nService) {} + + transform(value?: OrganizationUserStatusType): string { + if (value == null) { + return this.i18nService.t("unknown"); + } + switch (value) { + case OrganizationUserStatusType.Invited: + return this.i18nService.t("invited"); + case OrganizationUserStatusType.Accepted: + return this.i18nService.t("accepted"); + case OrganizationUserStatusType.Confirmed: + return this.i18nService.t("confirmed"); + case OrganizationUserStatusType.Revoked: + return this.i18nService.t("revoked"); + default: + return this.i18nService.t("unknown"); + } + } +} diff --git a/apps/web/src/app/admin-console/organizations/members/services/index.ts b/apps/web/src/app/admin-console/organizations/members/services/index.ts index baaa33eeae9..fd6b5816513 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/index.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/index.ts @@ -1,4 +1,5 @@ export { OrganizationMembersService } from "./organization-members-service/organization-members.service"; export { MemberActionsService } from "./member-actions/member-actions.service"; export { MemberDialogManagerService } from "./member-dialog-manager/member-dialog-manager.service"; +export { MemberExportService } from "./member-export"; export { DeleteManagedMemberWarningService } from "./delete-managed-member/delete-managed-member-warning.service"; diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-export/index.ts b/apps/web/src/app/admin-console/organizations/members/services/member-export/index.ts new file mode 100644 index 00000000000..acd36a91683 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/services/member-export/index.ts @@ -0,0 +1,2 @@ +export * from "./member.export"; +export * from "./member-export.service"; diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-export/member-export.service.spec.ts b/apps/web/src/app/admin-console/organizations/members/services/member-export/member-export.service.spec.ts new file mode 100644 index 00000000000..1e229b95d24 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/services/member-export/member-export.service.spec.ts @@ -0,0 +1,151 @@ +import { TestBed } from "@angular/core/testing"; +import { MockProxy, mock } from "jest-mock-extended"; + +import { UserTypePipe } from "@bitwarden/angular/pipes/user-type.pipe"; +import { + OrganizationUserStatusType, + OrganizationUserType, +} from "@bitwarden/common/admin-console/enums"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; + +import { OrganizationUserView } from "../../../core"; +import { UserStatusPipe } from "../../pipes"; + +import { MemberExportService } from "./member-export.service"; + +describe("MemberExportService", () => { + let service: MemberExportService; + let i18nService: MockProxy; + + beforeEach(() => { + i18nService = mock(); + + // Setup common i18n translations + i18nService.t.mockImplementation((key: string) => { + const translations: Record = { + // Column headers + email: "Email", + name: "Name", + status: "Status", + role: "Role", + twoStepLogin: "Two-step Login", + accountRecovery: "Account Recovery", + secretsManager: "Secrets Manager", + groups: "Groups", + // Status values + invited: "Invited", + accepted: "Accepted", + confirmed: "Confirmed", + revoked: "Revoked", + // Role values + owner: "Owner", + admin: "Admin", + user: "User", + custom: "Custom", + // Boolean states + enabled: "Enabled", + disabled: "Disabled", + enrolled: "Enrolled", + notEnrolled: "Not Enrolled", + }; + return translations[key] || key; + }); + + TestBed.configureTestingModule({ + providers: [ + MemberExportService, + { provide: I18nService, useValue: i18nService }, + UserTypePipe, + UserStatusPipe, + ], + }); + + service = TestBed.inject(MemberExportService); + }); + + describe("getMemberExport", () => { + it("should export members with all fields populated", () => { + const members: OrganizationUserView[] = [ + { + email: "user1@example.com", + name: "User One", + status: OrganizationUserStatusType.Confirmed, + type: OrganizationUserType.Admin, + twoFactorEnabled: true, + resetPasswordEnrolled: true, + accessSecretsManager: true, + groupNames: ["Group A", "Group B"], + } as OrganizationUserView, + { + email: "user2@example.com", + name: "User Two", + status: OrganizationUserStatusType.Invited, + type: OrganizationUserType.User, + twoFactorEnabled: false, + resetPasswordEnrolled: false, + accessSecretsManager: false, + groupNames: ["Group C"], + } as OrganizationUserView, + ]; + + const csvData = service.getMemberExport(members); + + expect(csvData).toContain("Email,Name,Status,Role,Two-step Login,Account Recovery"); + expect(csvData).toContain("user1@example.com"); + expect(csvData).toContain("User One"); + expect(csvData).toContain("Confirmed"); + expect(csvData).toContain("Admin"); + expect(csvData).toContain("user2@example.com"); + expect(csvData).toContain("User Two"); + expect(csvData).toContain("Invited"); + }); + + it("should handle members with null name", () => { + const members: OrganizationUserView[] = [ + { + email: "user@example.com", + name: null, + status: OrganizationUserStatusType.Confirmed, + type: OrganizationUserType.User, + twoFactorEnabled: false, + resetPasswordEnrolled: false, + accessSecretsManager: false, + groupNames: [], + } as OrganizationUserView, + ]; + + const csvData = service.getMemberExport(members); + + expect(csvData).toContain("user@example.com"); + // Empty name is represented as an empty field in CSV + expect(csvData).toContain("user@example.com,,Confirmed"); + }); + + it("should handle members with no groups", () => { + const members: OrganizationUserView[] = [ + { + email: "user@example.com", + name: "User", + status: OrganizationUserStatusType.Confirmed, + type: OrganizationUserType.User, + twoFactorEnabled: false, + resetPasswordEnrolled: false, + accessSecretsManager: false, + groupNames: null, + } as OrganizationUserView, + ]; + + const csvData = service.getMemberExport(members); + + expect(csvData).toContain("user@example.com"); + expect(csvData).toBeDefined(); + }); + + it("should handle empty members array", () => { + const csvData = service.getMemberExport([]); + + // When array is empty, papaparse returns an empty string + expect(csvData).toBe(""); + }); + }); +}); diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-export/member-export.service.ts b/apps/web/src/app/admin-console/organizations/members/services/member-export/member-export.service.ts new file mode 100644 index 00000000000..c00881617a4 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/services/member-export/member-export.service.ts @@ -0,0 +1,49 @@ +import { inject, Injectable } from "@angular/core"; +import * as papa from "papaparse"; + +import { UserTypePipe } from "@bitwarden/angular/pipes/user-type.pipe"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { ExportHelper } from "@bitwarden/vault-export-core"; + +import { OrganizationUserView } from "../../../core"; +import { UserStatusPipe } from "../../pipes"; + +import { MemberExport } from "./member.export"; + +@Injectable() +export class MemberExportService { + private i18nService = inject(I18nService); + private userTypePipe = inject(UserTypePipe); + private userStatusPipe = inject(UserStatusPipe); + + getMemberExport(members: OrganizationUserView[]): string { + const exportData = members.map((m) => + MemberExport.fromOrganizationUserView( + this.i18nService, + this.userTypePipe, + this.userStatusPipe, + m, + ), + ); + + const headers: string[] = [ + this.i18nService.t("email"), + this.i18nService.t("name"), + this.i18nService.t("status"), + this.i18nService.t("role"), + this.i18nService.t("twoStepLogin"), + this.i18nService.t("accountRecovery"), + this.i18nService.t("secretsManager"), + this.i18nService.t("groups"), + ]; + + return papa.unparse(exportData, { + columns: headers, + header: true, + }); + } + + getFileName(prefix: string | null = null, extension = "csv"): string { + return ExportHelper.getFileName(prefix ?? "", extension); + } +} diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-export/member.export.ts b/apps/web/src/app/admin-console/organizations/members/services/member-export/member.export.ts new file mode 100644 index 00000000000..262e8ebd9fb --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/members/services/member-export/member.export.ts @@ -0,0 +1,43 @@ +import { UserTypePipe } from "@bitwarden/angular/pipes/user-type.pipe"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; + +import { OrganizationUserView } from "../../../core"; +import { UserStatusPipe } from "../../pipes"; + +export class MemberExport { + /** + * @param user Organization user to export + * @returns a Record of each column header key, value + * All property members must be a string for export purposes. Null and undefined will appear as + * "null" in a .csv export, therefore an empty string is preferable to a nullish type. + */ + static fromOrganizationUserView( + i18nService: I18nService, + userTypePipe: UserTypePipe, + userStatusPipe: UserStatusPipe, + user: OrganizationUserView, + ): Record { + const result = { + [i18nService.t("email")]: user.email, + [i18nService.t("name")]: user.name ?? "", + [i18nService.t("status")]: userStatusPipe.transform(user.status), + [i18nService.t("role")]: userTypePipe.transform(user.type), + + [i18nService.t("twoStepLogin")]: user.twoFactorEnabled + ? i18nService.t("optionEnabled") + : i18nService.t("disabled"), + + [i18nService.t("accountRecovery")]: user.resetPasswordEnrolled + ? i18nService.t("enrolled") + : i18nService.t("notEnrolled"), + + [i18nService.t("secretsManager")]: user.accessSecretsManager + ? i18nService.t("optionEnabled") + : i18nService.t("disabled"), + + [i18nService.t("groups")]: user.groupNames?.join(", ") ?? "", + }; + + return result; + } +} diff --git a/apps/web/src/app/admin-console/organizations/policies/auto-confirm-edit-policy-dialog.component.ts b/apps/web/src/app/admin-console/organizations/policies/auto-confirm-edit-policy-dialog.component.ts index 63a8a4341d6..9dfb8ebb7e7 100644 --- a/apps/web/src/app/admin-console/organizations/policies/auto-confirm-edit-policy-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/auto-confirm-edit-policy-dialog.component.ts @@ -22,7 +22,7 @@ import { tap, } from "rxjs"; -import { AutomaticUserConfirmationService } from "@bitwarden/admin-console/common"; +import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; 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"; diff --git a/apps/web/src/app/admin-console/organizations/policies/policies.component.spec.ts b/apps/web/src/app/admin-console/organizations/policies/policies.component.spec.ts index 0e025a9d52a..125876ce05a 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policies.component.spec.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policies.component.spec.ts @@ -188,7 +188,7 @@ describe("PoliciesComponent", () => { }); describe("orgPolicies$", () => { - it("should fetch policies from API for current organization", async () => { + describe("with multiple policies", () => { const mockPolicyResponsesData = [ { id: newGuid(), @@ -206,39 +206,63 @@ describe("PoliciesComponent", () => { }, ]; - const listResponse = new ListResponse( - { Data: mockPolicyResponsesData, ContinuationToken: null }, - PolicyResponse, - ); + beforeEach(async () => { + const listResponse = new ListResponse( + { Data: mockPolicyResponsesData, ContinuationToken: null }, + PolicyResponse, + ); - mockPolicyApiService.getPolicies.mockResolvedValue(listResponse); + mockPolicyApiService.getPolicies.mockResolvedValue(listResponse); - const policies = await firstValueFrom(component["orgPolicies$"]); - expect(policies).toEqual(listResponse.data); - expect(mockPolicyApiService.getPolicies).toHaveBeenCalledWith(mockOrgId); + fixture = TestBed.createComponent(PoliciesComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it("should fetch policies from API for current organization", async () => { + const policies = await firstValueFrom(component["orgPolicies$"]); + expect(policies.length).toBe(2); + expect(mockPolicyApiService.getPolicies).toHaveBeenCalledWith(mockOrgId); + }); }); - it("should return empty array when API returns no data", async () => { - mockPolicyApiService.getPolicies.mockResolvedValue( - new ListResponse({ Data: [], ContinuationToken: null }, PolicyResponse), - ); + describe("with no policies", () => { + beforeEach(async () => { + mockPolicyApiService.getPolicies.mockResolvedValue( + new ListResponse({ Data: [], ContinuationToken: null }, PolicyResponse), + ); - const policies = await firstValueFrom(component["orgPolicies$"]); - expect(policies).toEqual([]); + fixture = TestBed.createComponent(PoliciesComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it("should return empty array when API returns no data", async () => { + const policies = await firstValueFrom(component["orgPolicies$"]); + expect(policies).toEqual([]); + }); }); - it("should return empty array when API returns null data", async () => { - mockPolicyApiService.getPolicies.mockResolvedValue( - new ListResponse({ Data: null, ContinuationToken: null }, PolicyResponse), - ); + describe("with null data", () => { + beforeEach(async () => { + mockPolicyApiService.getPolicies.mockResolvedValue( + new ListResponse({ Data: null, ContinuationToken: null }, PolicyResponse), + ); - const policies = await firstValueFrom(component["orgPolicies$"]); - expect(policies).toEqual([]); + fixture = TestBed.createComponent(PoliciesComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it("should return empty array when API returns null data", async () => { + const policies = await firstValueFrom(component["orgPolicies$"]); + expect(policies).toEqual([]); + }); }); }); describe("policiesEnabledMap$", () => { - it("should create a map of policy types to their enabled status", async () => { + describe("with multiple policies", () => { const mockPolicyResponsesData = [ { id: "policy-1", @@ -263,27 +287,43 @@ describe("PoliciesComponent", () => { }, ]; - mockPolicyApiService.getPolicies.mockResolvedValue( - new ListResponse( - { Data: mockPolicyResponsesData, ContinuationToken: null }, - PolicyResponse, - ), - ); + beforeEach(async () => { + mockPolicyApiService.getPolicies.mockResolvedValue( + new ListResponse( + { Data: mockPolicyResponsesData, ContinuationToken: null }, + PolicyResponse, + ), + ); - const map = await firstValueFrom(component.policiesEnabledMap$); - expect(map.size).toBe(3); - expect(map.get(PolicyType.TwoFactorAuthentication)).toBe(true); - expect(map.get(PolicyType.RequireSso)).toBe(false); - expect(map.get(PolicyType.SingleOrg)).toBe(true); + fixture = TestBed.createComponent(PoliciesComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it("should create a map of policy types to their enabled status", async () => { + const map = await firstValueFrom(component.policiesEnabledMap$); + expect(map.size).toBe(3); + expect(map.get(PolicyType.TwoFactorAuthentication)).toBe(true); + expect(map.get(PolicyType.RequireSso)).toBe(false); + expect(map.get(PolicyType.SingleOrg)).toBe(true); + }); }); - it("should create empty map when no policies exist", async () => { - mockPolicyApiService.getPolicies.mockResolvedValue( - new ListResponse({ Data: [], ContinuationToken: null }, PolicyResponse), - ); + describe("with no policies", () => { + beforeEach(async () => { + mockPolicyApiService.getPolicies.mockResolvedValue( + new ListResponse({ Data: [], ContinuationToken: null }, PolicyResponse), + ); - const map = await firstValueFrom(component.policiesEnabledMap$); - expect(map.size).toBe(0); + fixture = TestBed.createComponent(PoliciesComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it("should create empty map when no policies exist", async () => { + const map = await firstValueFrom(component.policiesEnabledMap$); + expect(map.size).toBe(0); + }); }); }); @@ -292,31 +332,36 @@ describe("PoliciesComponent", () => { expect(mockPolicyService.policies$).toHaveBeenCalledWith(mockUserId); }); - it("should refresh policies when policyService emits", async () => { - const policiesSubject = new BehaviorSubject([]); - mockPolicyService.policies$.mockReturnValue(policiesSubject.asObservable()); + describe("when policyService emits", () => { + let policiesSubject: BehaviorSubject; + let callCount: number; - let callCount = 0; - mockPolicyApiService.getPolicies.mockImplementation(() => { - callCount++; - return of(new ListResponse({ Data: [], ContinuationToken: null }, PolicyResponse)); + beforeEach(async () => { + policiesSubject = new BehaviorSubject([]); + mockPolicyService.policies$.mockReturnValue(policiesSubject.asObservable()); + + callCount = 0; + mockPolicyApiService.getPolicies.mockImplementation(() => { + callCount++; + return of(new ListResponse({ Data: [], ContinuationToken: null }, PolicyResponse)); + }); + + fixture = TestBed.createComponent(PoliciesComponent); + fixture.detectChanges(); }); - const newFixture = TestBed.createComponent(PoliciesComponent); - newFixture.detectChanges(); + it("should refresh policies when policyService emits", () => { + const initialCallCount = callCount; - const initialCallCount = callCount; + policiesSubject.next([{ type: PolicyType.TwoFactorAuthentication }]); - policiesSubject.next([{ type: PolicyType.TwoFactorAuthentication }]); - - expect(callCount).toBeGreaterThan(initialCallCount); - - newFixture.destroy(); + expect(callCount).toBeGreaterThan(initialCallCount); + }); }); }); describe("handleLaunchEvent", () => { - it("should open policy dialog when policyId is in query params", async () => { + describe("when policyId is in query params", () => { const mockPolicyId = newGuid(); const mockPolicy: BasePolicyEditDefinition = { name: "Test Policy", @@ -335,54 +380,59 @@ describe("PoliciesComponent", () => { data: null, }; - queryParamsSubject.next({ policyId: mockPolicyId }); + let dialogOpenSpy: jest.SpyInstance; - mockPolicyApiService.getPolicies.mockReturnValue( - of( - new ListResponse( - { Data: [mockPolicyResponseData], ContinuationToken: null }, - PolicyResponse, + beforeEach(async () => { + queryParamsSubject.next({ policyId: mockPolicyId }); + + mockPolicyApiService.getPolicies.mockReturnValue( + of( + new ListResponse( + { Data: [mockPolicyResponseData], ContinuationToken: null }, + PolicyResponse, + ), ), - ), - ); + ); - const dialogOpenSpy = jest - .spyOn(PolicyEditDialogComponent, "open") - .mockReturnValue({ close: jest.fn() } as any); + dialogOpenSpy = jest + .spyOn(PolicyEditDialogComponent, "open") + .mockReturnValue({ close: jest.fn() } as any); - TestBed.resetTestingModule(); - await TestBed.configureTestingModule({ - imports: [PoliciesComponent], - providers: [ - { provide: ActivatedRoute, useValue: mockActivatedRoute }, - { provide: OrganizationService, useValue: mockOrganizationService }, - { provide: AccountService, useValue: mockAccountService }, - { provide: PolicyApiServiceAbstraction, useValue: mockPolicyApiService }, - { provide: PolicyListService, useValue: mockPolicyListService }, - { provide: DialogService, useValue: mockDialogService }, - { provide: PolicyService, useValue: mockPolicyService }, - { provide: ConfigService, useValue: mockConfigService }, - { provide: I18nService, useValue: mockI18nService }, - { provide: PlatformUtilsService, useValue: mockPlatformUtilsService }, - { provide: POLICY_EDIT_REGISTER, useValue: [mockPolicy] }, - ], - schemas: [NO_ERRORS_SCHEMA], - }) - .overrideComponent(PoliciesComponent, { - remove: { imports: [] }, - add: { template: "
" }, + TestBed.resetTestingModule(); + await TestBed.configureTestingModule({ + imports: [PoliciesComponent], + providers: [ + { provide: ActivatedRoute, useValue: mockActivatedRoute }, + { provide: OrganizationService, useValue: mockOrganizationService }, + { provide: AccountService, useValue: mockAccountService }, + { provide: PolicyApiServiceAbstraction, useValue: mockPolicyApiService }, + { provide: PolicyListService, useValue: mockPolicyListService }, + { provide: DialogService, useValue: mockDialogService }, + { provide: PolicyService, useValue: mockPolicyService }, + { provide: ConfigService, useValue: mockConfigService }, + { provide: I18nService, useValue: mockI18nService }, + { provide: PlatformUtilsService, useValue: mockPlatformUtilsService }, + { provide: POLICY_EDIT_REGISTER, useValue: [mockPolicy] }, + ], + schemas: [NO_ERRORS_SCHEMA], }) - .compileComponents(); + .overrideComponent(PoliciesComponent, { + remove: { imports: [] }, + add: { template: "
" }, + }) + .compileComponents(); - const newFixture = TestBed.createComponent(PoliciesComponent); - newFixture.detectChanges(); + fixture = TestBed.createComponent(PoliciesComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); - expect(dialogOpenSpy).toHaveBeenCalled(); - const callArgs = dialogOpenSpy.mock.calls[0][1]; - expect(callArgs.data?.policy.type).toBe(mockPolicy.type); - expect(callArgs.data?.organizationId).toBe(mockOrgId); - - newFixture.destroy(); + it("should open policy dialog when policyId is in query params", () => { + expect(dialogOpenSpy).toHaveBeenCalled(); + const callArgs = dialogOpenSpy.mock.calls[0][1]; + expect(callArgs.data?.policy.type).toBe(mockPolicy.type); + expect(callArgs.data?.organizationId).toBe(mockOrgId); + }); }); it("should not open dialog when policyId is not in query params", async () => { diff --git a/apps/web/src/app/admin-console/organizations/policies/policies.component.ts b/apps/web/src/app/admin-console/organizations/policies/policies.component.ts index 70daf55f662..1f9a8deaa85 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policies.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policies.component.ts @@ -1,7 +1,7 @@ import { ChangeDetectionStrategy, Component, DestroyRef } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { ActivatedRoute } from "@angular/router"; -import { combineLatest, Observable, of, switchMap, first, map } from "rxjs"; +import { combineLatest, Observable, of, switchMap, first, map, shareReplay } from "rxjs"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; @@ -70,6 +70,7 @@ export class PoliciesComponent { switchMap(() => this.organizationId$), switchMap((organizationId) => this.policyApiService.getPolicies(organizationId)), map((response) => (response.data != null && response.data.length > 0 ? response.data : [])), + shareReplay({ bufferSize: 1, refCount: true }), ); protected policiesEnabledMap$: Observable> = this.orgPolicies$.pipe( diff --git a/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.html b/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.html index 860f80eb346..4858deabec6 100644 --- a/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.html +++ b/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.html @@ -40,21 +40,27 @@ {{ i.amount | currency: "$" }} - + {{ "freeForOneYear" | i18n }}
- {{ i.quantity * i.amount | currency: "$" }} /{{ i.interval | i18n }} + {{ i.quantity * i.amount | currency: "$" }} / + {{ i.interval | i18n }} {{ - calculateTotalAppliedDiscount(i.quantity * i.amount) | currency: "$" - }} - / {{ "year" | i18n }}{{ i.quantity * i.originalAmount | currency: "$" }} / + {{ "year" | i18n }}
diff --git a/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts b/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts index de5d71cce5e..323a190fe1c 100644 --- a/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts +++ b/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts @@ -19,11 +19,9 @@ import { import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; -import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions"; import { PlanType, ProductTierType } from "@bitwarden/common/billing/enums"; import { OrganizationSubscriptionResponse } from "@bitwarden/common/billing/models/response/organization-subscription.response"; import { BillingSubscriptionItemResponse } from "@bitwarden/common/billing/models/response/subscription.response"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { DialogService, ToastService } from "@bitwarden/components"; @@ -82,9 +80,7 @@ export class OrganizationSubscriptionCloudComponent implements OnInit, OnDestroy private organizationApiService: OrganizationApiServiceAbstraction, private route: ActivatedRoute, private dialogService: DialogService, - private configService: ConfigService, private toastService: ToastService, - private billingApiService: BillingApiServiceAbstraction, private organizationUserApiService: OrganizationUserApiService, ) {} @@ -218,6 +214,7 @@ export class OrganizationSubscriptionCloudComponent implements OnInit, OnDestroy get subscriptionLineItems() { return this.lineItems.map((lineItem: BillingSubscriptionItemResponse) => ({ name: lineItem.name, + originalAmount: lineItem.amount, amount: this.discountPrice(lineItem.amount, lineItem.productId), quantity: lineItem.quantity, interval: lineItem.interval, @@ -406,12 +403,16 @@ export class OrganizationSubscriptionCloudComponent implements OnInit, OnDestroy const isSmStandalone = this.sub?.customerDiscount?.id === "sm-standalone"; const appliesToProduct = this.sub?.subscription?.items?.some((item) => - this.sub?.customerDiscount?.appliesTo?.includes(item.productId), + this.discountAppliesToProduct(item.productId), ) ?? false; return isSmStandalone && appliesToProduct; } + discountAppliesToProduct(productId: string): boolean { + return this.sub?.customerDiscount?.appliesTo?.includes(productId) ?? false; + } + closeChangePlan() { this.showChangePlan = false; } @@ -438,10 +439,6 @@ export class OrganizationSubscriptionCloudComponent implements OnInit, OnDestroy await this.load(); } - calculateTotalAppliedDiscount(total: number) { - return total / (1 - this.customerDiscount?.percentOff / 100); - } - adjustStorage = (add: boolean) => { return async () => { const dialogRef = AdjustStorageDialogComponent.open(this.dialogService, { diff --git a/apps/web/src/app/core/core.module.ts b/apps/web/src/app/core/core.module.ts index e436e194e9e..661d14502fe 100644 --- a/apps/web/src/app/core/core.module.ts +++ b/apps/web/src/app/core/core.module.ts @@ -9,8 +9,6 @@ import { DefaultCollectionAdminService, OrganizationUserApiService, CollectionService, - AutomaticUserConfirmationService, - DefaultAutomaticUserConfirmationService, OrganizationUserService, DefaultOrganizationUserService, } from "@bitwarden/admin-console/common"; @@ -46,6 +44,10 @@ import { InternalUserDecryptionOptionsServiceAbstraction, LoginEmailService, } from "@bitwarden/auth/common"; +import { + AutomaticUserConfirmationService, + DefaultAutomaticUserConfirmationService, +} from "@bitwarden/auto-confirm"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization-api.service.abstraction"; import { @@ -376,6 +378,7 @@ const safeProviders: SafeProvider[] = [ StateProvider, InternalOrganizationServiceAbstraction, OrganizationUserApiService, + PolicyService, ], }), safeProvider({ diff --git a/apps/web/src/app/layouts/user-layout.component.ts b/apps/web/src/app/layouts/user-layout.component.ts index 3af514466b7..90207f59ad4 100644 --- a/apps/web/src/app/layouts/user-layout.component.ts +++ b/apps/web/src/app/layouts/user-layout.component.ts @@ -4,12 +4,12 @@ import { CommonModule } from "@angular/common"; import { Component, OnInit, Signal } from "@angular/core"; import { toSignal } from "@angular/core/rxjs-interop"; import { RouterModule } from "@angular/router"; -import { combineLatest, map, Observable, switchMap } from "rxjs"; +import { Observable, switchMap } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { PasswordManagerLogo } from "@bitwarden/assets/svg"; +import { canAccessEmergencyAccess } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; -import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; @@ -58,21 +58,11 @@ export class UserLayoutComponent implements OnInit { ); this.showEmergencyAccess = toSignal( - combineLatest([ - this.configService.getFeatureFlag$(FeatureFlag.AutoConfirm), - this.accountService.activeAccount$.pipe( - getUserId, - switchMap((userId) => - this.policyService.policyAppliesToUser$(PolicyType.AutoConfirm, userId), - ), + this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => + canAccessEmergencyAccess(userId, this.configService, this.policyService), ), - ]).pipe( - map(([enabled, policyAppliesToUser]) => { - if (!enabled || !policyAppliesToUser) { - return true; - } - return false; - }), ), ); diff --git a/apps/web/src/app/oss-routing.module.ts b/apps/web/src/app/oss-routing.module.ts index f4fd55bd1e6..932d0b8119b 100644 --- a/apps/web/src/app/oss-routing.module.ts +++ b/apps/web/src/app/oss-routing.module.ts @@ -1,6 +1,7 @@ import { NgModule } from "@angular/core"; import { Route, RouterModule, Routes } from "@angular/router"; +import { organizationPolicyGuard } from "@bitwarden/angular/admin-console/guards"; import { AuthenticationTimeoutComponent } from "@bitwarden/angular/auth/components/authentication-timeout.component"; import { AuthRoute } from "@bitwarden/angular/auth/constants"; import { @@ -56,7 +57,6 @@ import { premiumInterestRedirectGuard } from "@bitwarden/web-vault/app/vault/gua import { flagEnabled, Flags } from "../utils/flags"; -import { organizationPolicyGuard } from "./admin-console/organizations/guards/org-policy.guard"; import { VerifyRecoverDeleteOrgComponent } from "./admin-console/organizations/manage/verify-recover-delete-org.component"; import { AcceptFamilySponsorshipComponent } from "./admin-console/organizations/sponsorships/accept-family-sponsorship.component"; import { FamiliesForEnterpriseSetupComponent } from "./admin-console/organizations/sponsorships/families-for-enterprise-setup.component"; diff --git a/apps/web/src/app/tools/event-export/event-export.service.ts b/apps/web/src/app/tools/event-export/event-export.service.ts index f39b786b6d1..d888af51edf 100644 --- a/apps/web/src/app/tools/event-export/event-export.service.ts +++ b/apps/web/src/app/tools/event-export/event-export.service.ts @@ -4,6 +4,7 @@ import { Injectable } from "@angular/core"; import * as papa from "papaparse"; import { EventView } from "@bitwarden/common/models/view/event.view"; +import { ExportHelper } from "@bitwarden/vault-export-core"; import { EventExport } from "./event.export"; @@ -16,25 +17,6 @@ export class EventExportService { } getFileName(prefix: string = null, extension = "csv"): string { - const now = new Date(); - const dateString = - now.getFullYear() + - "" + - this.padNumber(now.getMonth() + 1, 2) + - "" + - this.padNumber(now.getDate(), 2) + - this.padNumber(now.getHours(), 2) + - "" + - this.padNumber(now.getMinutes(), 2) + - this.padNumber(now.getSeconds(), 2); - - return "bitwarden" + (prefix ? "_" + prefix : "") + "_export_" + dateString + "." + extension; - } - - private padNumber(num: number, width: number, padCharacter = "0"): string { - const numString = num.toString(); - return numString.length >= width - ? numString - : new Array(width - numString.length + 1).join(padCharacter) + numString; + return ExportHelper.getFileName(prefix ?? "", extension); } } diff --git a/apps/web/src/app/tools/send/send-access/access.component.html b/apps/web/src/app/tools/send/send-access/access.component.html index aec6e2a10b9..b86933410b8 100644 --- a/apps/web/src/app/tools/send/send-access/access.component.html +++ b/apps/web/src/app/tools/send/send-access/access.component.html @@ -1,52 +1,14 @@ -
- - {{ "viewSendHiddenEmailWarning" | i18n }} - {{ - "learnMore" | i18n - }}. - - - -
-

{{ "sendAccessUnavailable" | i18n }}

-
-
-

{{ "unexpectedErrorSend" | i18n }}

-
-
-

- {{ send.name }} -

-
- - - - - - - - -

- Expires: {{ expirationDate | date: "medium" }} -

-
-
- -
- - {{ "loading" | i18n }} -
-
-
+@switch (viewState) { + @case ("auth") { + + } + @case ("view") { + + } +} diff --git a/apps/web/src/app/tools/send/send-access/access.component.ts b/apps/web/src/app/tools/send/send-access/access.component.ts index 273f1c8c979..4ea469a0b1c 100644 --- a/apps/web/src/app/tools/send/send-access/access.component.ts +++ b/apps/web/src/app/tools/send/send-access/access.component.ts @@ -1,161 +1,60 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { Component, OnInit } from "@angular/core"; -import { FormBuilder } from "@angular/forms"; import { ActivatedRoute } from "@angular/router"; -import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; -import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; -import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; -import { SendType } from "@bitwarden/common/tools/send/enums/send-type"; -import { SendAccess } from "@bitwarden/common/tools/send/models/domain/send-access"; import { SendAccessRequest } from "@bitwarden/common/tools/send/models/request/send-access.request"; import { SendAccessResponse } from "@bitwarden/common/tools/send/models/response/send-access.response"; -import { SendAccessView } from "@bitwarden/common/tools/send/models/view/send-access.view"; -import { SEND_KDF_ITERATIONS } from "@bitwarden/common/tools/send/send-kdf"; -import { SendApiService } from "@bitwarden/common/tools/send/services/send-api.service.abstraction"; -import { AnonLayoutWrapperDataService, ToastService } from "@bitwarden/components"; -import { KeyService } from "@bitwarden/key-management"; import { SharedModule } from "../../../shared"; -import { SendAccessFileComponent } from "./send-access-file.component"; -import { SendAccessPasswordComponent } from "./send-access-password.component"; -import { SendAccessTextComponent } from "./send-access-text.component"; +import { SendAuthComponent } from "./send-auth.component"; +import { SendViewComponent } from "./send-view.component"; + +const SendViewState = Object.freeze({ + View: "view", + Auth: "auth", +} as const); +type SendViewState = (typeof SendViewState)[keyof typeof SendViewState]; // FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush // eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ selector: "app-send-access", templateUrl: "access.component.html", - imports: [ - SendAccessFileComponent, - SendAccessTextComponent, - SendAccessPasswordComponent, - SharedModule, - ], + imports: [SendAuthComponent, SendViewComponent, SharedModule], }) export class AccessComponent implements OnInit { - protected send: SendAccessView; - protected sendType = SendType; - protected loading = true; - protected passwordRequired = false; - protected formPromise: Promise; - protected password: string; - protected unavailable = false; - protected error = false; - protected hideEmail = false; - protected decKey: SymmetricCryptoKey; - protected accessRequest: SendAccessRequest; + viewState: SendViewState = SendViewState.View; + id: string; + key: string; - protected formGroup = this.formBuilder.group({}); + sendAccessResponse: SendAccessResponse | null = null; + sendAccessRequest: SendAccessRequest = new SendAccessRequest(); - private id: string; - private key: string; - - constructor( - private cryptoFunctionService: CryptoFunctionService, - private route: ActivatedRoute, - private keyService: KeyService, - private sendApiService: SendApiService, - private toastService: ToastService, - private i18nService: I18nService, - private layoutWrapperDataService: AnonLayoutWrapperDataService, - protected formBuilder: FormBuilder, - ) {} - - protected get expirationDate() { - if (this.send == null || this.send.expirationDate == null) { - return null; - } - return this.send.expirationDate; - } - - protected get creatorIdentifier() { - if (this.send == null || this.send.creatorIdentifier == null) { - return null; - } - return this.send.creatorIdentifier; - } + constructor(private route: ActivatedRoute) {} async ngOnInit() { // eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe this.route.params.subscribe(async (params) => { this.id = params.sendId; this.key = params.key; - if (this.key == null || this.id == null) { - return; + + if (this.id && this.key) { + this.viewState = SendViewState.View; + this.sendAccessResponse = null; + this.sendAccessRequest = new SendAccessRequest(); } - await this.load(); }); } - protected load = async () => { - this.unavailable = false; - this.error = false; - this.hideEmail = false; - try { - const keyArray = Utils.fromUrlB64ToArray(this.key); - this.accessRequest = new SendAccessRequest(); - if (this.password != null) { - const passwordHash = await this.cryptoFunctionService.pbkdf2( - this.password, - keyArray, - "sha256", - SEND_KDF_ITERATIONS, - ); - this.accessRequest.password = Utils.fromBufferToB64(passwordHash); - } - let sendResponse: SendAccessResponse = null; - if (this.loading) { - sendResponse = await this.sendApiService.postSendAccess(this.id, this.accessRequest); - } else { - this.formPromise = this.sendApiService.postSendAccess(this.id, this.accessRequest); - sendResponse = await this.formPromise; - } - this.passwordRequired = false; - const sendAccess = new SendAccess(sendResponse); - this.decKey = await this.keyService.makeSendKey(keyArray); - this.send = await sendAccess.decrypt(this.decKey); - } catch (e) { - if (e instanceof ErrorResponse) { - if (e.statusCode === 401) { - this.passwordRequired = true; - } else if (e.statusCode === 404) { - this.unavailable = true; - } else if (e.statusCode === 400) { - this.toastService.showToast({ - variant: "error", - title: this.i18nService.t("errorOccurred"), - message: e.message, - }); - } else { - this.error = true; - } - } else { - this.error = true; - } - } - this.loading = false; - this.hideEmail = - this.creatorIdentifier == null && - !this.passwordRequired && - !this.loading && - !this.unavailable; + onAuthRequired() { + this.viewState = SendViewState.Auth; + } - if (this.creatorIdentifier != null) { - this.layoutWrapperDataService.setAnonLayoutWrapperData({ - pageSubtitle: { - key: "sendAccessCreatorIdentifier", - placeholders: [this.creatorIdentifier], - }, - }); - } - }; - - protected setPassword(password: string) { - this.password = password; + onAccessGranted(event: { response: SendAccessResponse; request: SendAccessRequest }) { + this.sendAccessResponse = event.response; + this.sendAccessRequest = event.request; + this.viewState = SendViewState.View; } } diff --git a/apps/web/src/app/tools/send/send-access/send-auth.component.html b/apps/web/src/app/tools/send/send-access/send-auth.component.html new file mode 100644 index 00000000000..21a6de50ba8 --- /dev/null +++ b/apps/web/src/app/tools/send/send-access/send-auth.component.html @@ -0,0 +1,14 @@ +
+
+

{{ "sendAccessUnavailable" | i18n }}

+
+
+

{{ "unexpectedErrorSend" | i18n }}

+
+ + +
diff --git a/apps/web/src/app/tools/send/send-access/send-auth.component.ts b/apps/web/src/app/tools/send/send-access/send-auth.component.ts new file mode 100644 index 00000000000..b360044a8b6 --- /dev/null +++ b/apps/web/src/app/tools/send/send-access/send-auth.component.ts @@ -0,0 +1,86 @@ +import { ChangeDetectionStrategy, Component, input, output } from "@angular/core"; + +import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; +import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { SendAccessRequest } from "@bitwarden/common/tools/send/models/request/send-access.request"; +import { SendAccessResponse } from "@bitwarden/common/tools/send/models/response/send-access.response"; +import { SEND_KDF_ITERATIONS } from "@bitwarden/common/tools/send/send-kdf"; +import { SendApiService } from "@bitwarden/common/tools/send/services/send-api.service.abstraction"; +import { ToastService } from "@bitwarden/components"; + +import { SharedModule } from "../../../shared"; + +import { SendAccessPasswordComponent } from "./send-access-password.component"; + +@Component({ + selector: "app-send-auth", + templateUrl: "send-auth.component.html", + imports: [SendAccessPasswordComponent, SharedModule], + changeDetection: ChangeDetectionStrategy.OnPush, +}) +export class SendAuthComponent { + readonly id = input.required(); + readonly key = input.required(); + + accessGranted = output<{ + response: SendAccessResponse; + request: SendAccessRequest; + }>(); + + loading = false; + error = false; + unavailable = false; + password?: string; + + private accessRequest!: SendAccessRequest; + + constructor( + private cryptoFunctionService: CryptoFunctionService, + private sendApiService: SendApiService, + private toastService: ToastService, + private i18nService: I18nService, + ) {} + + async onSubmit(password: string) { + this.password = password; + this.loading = true; + this.error = false; + this.unavailable = false; + + try { + const keyArray = Utils.fromUrlB64ToArray(this.key()); + this.accessRequest = new SendAccessRequest(); + + const passwordHash = await this.cryptoFunctionService.pbkdf2( + this.password, + keyArray, + "sha256", + SEND_KDF_ITERATIONS, + ); + this.accessRequest.password = Utils.fromBufferToB64(passwordHash); + + const sendResponse = await this.sendApiService.postSendAccess(this.id(), this.accessRequest); + this.accessGranted.emit({ response: sendResponse, request: this.accessRequest }); + } catch (e) { + if (e instanceof ErrorResponse) { + if (e.statusCode === 404) { + this.unavailable = true; + } else if (e.statusCode === 400) { + this.toastService.showToast({ + variant: "error", + title: this.i18nService.t("errorOccurred"), + message: e.message, + }); + } else { + this.error = true; + } + } else { + this.error = true; + } + } finally { + this.loading = false; + } + } +} diff --git a/apps/web/src/app/tools/send/send-access/send-view.component.html b/apps/web/src/app/tools/send/send-access/send-view.component.html new file mode 100644 index 00000000000..dd0b770b261 --- /dev/null +++ b/apps/web/src/app/tools/send/send-access/send-view.component.html @@ -0,0 +1,47 @@ + + {{ "viewSendHiddenEmailWarning" | i18n }} + {{ + "learnMore" | i18n + }}. + + + +
+

{{ "sendAccessUnavailable" | i18n }}

+
+
+

{{ "unexpectedErrorSend" | i18n }}

+
+
+

+ {{ send.name }} +

+
+ + + + + + + + +

+ Expires: {{ expirationDate | date: "medium" }} +

+
+
+ +
+ + {{ "loading" | i18n }} +
+
diff --git a/apps/web/src/app/tools/send/send-access/send-view.component.ts b/apps/web/src/app/tools/send/send-access/send-view.component.ts new file mode 100644 index 00000000000..0397575f021 --- /dev/null +++ b/apps/web/src/app/tools/send/send-access/send-view.component.ts @@ -0,0 +1,131 @@ +import { + ChangeDetectionStrategy, + ChangeDetectorRef, + Component, + input, + OnInit, + output, +} from "@angular/core"; + +import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; +import { SendType } from "@bitwarden/common/tools/send/enums/send-type"; +import { SendAccess } from "@bitwarden/common/tools/send/models/domain/send-access"; +import { SendAccessRequest } from "@bitwarden/common/tools/send/models/request/send-access.request"; +import { SendAccessResponse } from "@bitwarden/common/tools/send/models/response/send-access.response"; +import { SendAccessView } from "@bitwarden/common/tools/send/models/view/send-access.view"; +import { SendApiService } from "@bitwarden/common/tools/send/services/send-api.service.abstraction"; +import { AnonLayoutWrapperDataService, ToastService } from "@bitwarden/components"; +import { KeyService } from "@bitwarden/key-management"; + +import { SharedModule } from "../../../shared"; + +import { SendAccessFileComponent } from "./send-access-file.component"; +import { SendAccessTextComponent } from "./send-access-text.component"; + +@Component({ + selector: "app-send-view", + templateUrl: "send-view.component.html", + imports: [SendAccessFileComponent, SendAccessTextComponent, SharedModule], + changeDetection: ChangeDetectionStrategy.OnPush, +}) +export class SendViewComponent implements OnInit { + readonly id = input.required(); + readonly key = input.required(); + readonly sendResponse = input(null); + readonly accessRequest = input(new SendAccessRequest()); + + authRequired = output(); + + send: SendAccessView | null = null; + sendType = SendType; + loading = true; + unavailable = false; + error = false; + hideEmail = false; + decKey!: SymmetricCryptoKey; + + constructor( + private keyService: KeyService, + private sendApiService: SendApiService, + private toastService: ToastService, + private i18nService: I18nService, + private layoutWrapperDataService: AnonLayoutWrapperDataService, + private cdRef: ChangeDetectorRef, + ) {} + + get expirationDate() { + if (this.send == null || this.send.expirationDate == null) { + return null; + } + return this.send.expirationDate; + } + + get creatorIdentifier() { + if (this.send == null || this.send.creatorIdentifier == null) { + return null; + } + return this.send.creatorIdentifier; + } + + async ngOnInit() { + await this.load(); + } + + private async load() { + this.unavailable = false; + this.error = false; + this.hideEmail = false; + this.loading = true; + + let response = this.sendResponse(); + + try { + if (!response) { + response = await this.sendApiService.postSendAccess(this.id(), this.accessRequest()); + } + + const keyArray = Utils.fromUrlB64ToArray(this.key()); + const sendAccess = new SendAccess(response); + this.decKey = await this.keyService.makeSendKey(keyArray); + this.send = await sendAccess.decrypt(this.decKey); + } catch (e) { + if (e instanceof ErrorResponse) { + if (e.statusCode === 401) { + this.authRequired.emit(); + } else if (e.statusCode === 404) { + this.unavailable = true; + } else if (e.statusCode === 400) { + this.toastService.showToast({ + variant: "error", + title: this.i18nService.t("errorOccurred"), + message: e.message, + }); + } else { + this.error = true; + } + } else { + this.error = true; + } + } + + this.loading = false; + this.hideEmail = + this.creatorIdentifier == null && !this.loading && !this.unavailable && !response; + + this.hideEmail = this.send != null && this.creatorIdentifier == null; + + if (this.creatorIdentifier != null) { + this.layoutWrapperDataService.setAnonLayoutWrapperData({ + pageSubtitle: { + key: "sendAccessCreatorIdentifier", + placeholders: [this.creatorIdentifier], + }, + }); + } + + this.cdRef.markForCheck(); + } +} diff --git a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts index a723f1e942b..e437537b1cc 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts @@ -144,8 +144,9 @@ export class VaultCipherRowComponent implements OnInit } } + // Archive button will not show in Admin Console protected get showArchiveButton() { - if (!this.archiveEnabled()) { + if (!this.archiveEnabled() || this.viewingOrgVault) { return false; } diff --git a/apps/web/src/app/vault/individual-vault/vault.component.ts b/apps/web/src/app/vault/individual-vault/vault.component.ts index a5121831304..aa238922eea 100644 --- a/apps/web/src/app/vault/individual-vault/vault.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault.component.ts @@ -26,7 +26,6 @@ import { } from "rxjs/operators"; import { - AutomaticUserConfirmationService, CollectionData, CollectionDetailsResponse, CollectionService, @@ -42,6 +41,7 @@ import { ItemTypes, Icon, } from "@bitwarden/assets/svg"; +import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; import { diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index db30a9d1153..8024de21e56 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -1749,6 +1749,9 @@ "noMembersInList": { "message": "There are no members to list." }, + "noMembersToExport": { + "message": "There are no members to export." + }, "noEventsInList": { "message": "There are no events to list." }, @@ -2537,6 +2540,9 @@ "enabled": { "message": "Turned on" }, + "optionEnabled": { + "message": "Enabled" + }, "restoreAccess": { "message": "Restore access" }, @@ -5649,6 +5655,9 @@ "revoked": { "message": "Revoked" }, + "accepted": { + "message": "Accepted" + }, "sendLink": { "message": "Send link", "description": "'Send' is a noun and the name of a feature called 'Bitwarden Send'. It should not be translated." @@ -6307,6 +6316,12 @@ "enrolledAccountRecovery": { "message": "Enrolled in account recovery" }, + "enrolled": { + "message": "Enrolled" + }, + "notEnrolled": { + "message": "Not enrolled" + }, "withdrawAccountRecovery": { "message": "Withdraw from account recovery" }, diff --git a/bitwarden_license/bit-common/src/dirt/organization-integrations/services/README.md b/bitwarden_license/bit-common/src/dirt/organization-integrations/services/README.md new file mode 100644 index 00000000000..1796b0db071 --- /dev/null +++ b/bitwarden_license/bit-common/src/dirt/organization-integrations/services/README.md @@ -0,0 +1,358 @@ +# Adding a New Integration Configuration and Template + +This guide explains how to add a new integration type (e.g., Datadog, Splunk HEC) to the organization integrations system. + +## Step 1: Define the Configuration Class + +Create a new configuration class that implements `OrgIntegrationConfiguration`: + +```typescript +export class MyServiceConfiguration implements OrgIntegrationConfiguration { + // Required: Specify which service this configuration is for + bw_serviceName: OrganizationIntegrationServiceName; + + // Add service-specific properties (e.g., uri, apiKey, token) + uri: string; + apiKey: string; + + constructor(uri: string, apiKey: string, bw_serviceName: OrganizationIntegrationServiceName) { + this.uri = uri; + this.apiKey = apiKey; + this.bw_serviceName = bw_serviceName; + } + + // Required: Serialize configuration to JSON string for API transmission + // Property names should match PascalCase for backend compatibility + // Example: "Uri", "ApiKey" - the backend expects PascalCase keys + toString(): string { + return JSON.stringify({ + Uri: this.uri, + ApiKey: this.apiKey, + bw_serviceName: this.bw_serviceName, + }); + } +} +``` + +**Required Interface Properties:** + +- `bw_serviceName: OrganizationIntegrationServiceName` - Identifies the external service +- `toString(): string` - Serializes configuration for API storage + +## Step 2: Define the Template Class + +Create a template class that implements `OrgIntegrationTemplate`: + +```typescript +export class MyServiceTemplate implements OrgIntegrationTemplate { + // Required: Specify which service this template is for + bw_serviceName: OrganizationIntegrationServiceName; + + // Add template-specific properties with placeholders (e.g., #CipherId#, #UserEmail#) + // These placeholders will be replaced with actual values at runtime + + constructor(service: OrganizationIntegrationServiceName) { + this.bw_serviceName = service; + } + + // Required: Serialize template to JSON string + // Define the structure of data that will be sent to the external service + toString(): string { + return JSON.stringify({ + bw_serviceName: this.bw_serviceName, + event: { + type: "#Type#", + userId: "#UserId#", + // ... other placeholders + }, + }); + } +} +``` + +**Required Interface Properties:** + +- `bw_serviceName: OrganizationIntegrationServiceName` - Identifies the external service +- `toString(): string` - Serializes template structure with placeholders + +## Step 3: Update OrganizationIntegrationType + +Add your new integration type to the enum: + +```typescript +export const OrganizationIntegrationType = Object.freeze({ + // ... existing types + MyService: 7, +} as const); +``` + +## Step 4: Extend OrgIntegrationBuilder + +The `OrgIntegrationBuilder` is the central factory for creating and deserializing integration configurations and templates. +It provides a consistent API for the `OrganizationIntegrationService` to work with different integration types. + +Add four methods to `OrgIntegrationBuilder`: + +### 4a. Add a static factory method for configuration: + +```typescript +static buildMyServiceConfiguration( + uri: string, + apiKey: string, + bw_serviceName: OrganizationIntegrationServiceName +): OrgIntegrationConfiguration { + return new MyServiceConfiguration(uri, apiKey, bw_serviceName); +} +``` + +### 4b. Add a static factory method for template: + +```typescript +static buildMyServiceTemplate( + bw_serviceName: OrganizationIntegrationServiceName +): OrgIntegrationTemplate { + return new MyServiceTemplate(bw_serviceName); +} +``` + +### 4c. Add a case to `buildConfiguration()` switch statement: + +```typescript +case OrganizationIntegrationType.MyService: { + const config = this.convertToJson(configuration); + return this.buildMyServiceConfiguration(config.uri, config.apiKey, config.bw_serviceName); +} +``` + +This allows deserialization of JSON configuration strings from the API into typed objects. + +### 4d. Add a case to `buildTemplate()` switch statement: + +```typescript +case OrganizationIntegrationType.MyService: { + const template = this.convertToJson(template); + return this.buildMyServiceTemplate(template.bw_serviceName); +} +``` + +This allows deserialization of JSON template strings from the API into typed objects. + +## How This Facilitates OrganizationIntegrationService + +The `OrgIntegrationBuilder` acts as an abstraction layer that enables the `OrganizationIntegrationService` to: + +1. **Save/Update Operations**: Accept strongly-typed configuration and template objects, serialize them via `toString()`, + and send to the API as JSON strings. + +2. **Load Operations**: Receive JSON strings from the API, use `buildConfiguration()` and `buildTemplate()` to + deserialize them into strongly-typed objects through the builder's factory methods. + +3. **Type Safety**: Work with typed domain models (`OrgIntegrationConfiguration`, `OrgIntegrationTemplate`) without + knowing the specific implementation details of each integration type. + +4. **Extensibility**: Add new integration types without modifying the service layer logic. The service only needs to + call the builder's methods, which internally route to the correct implementation based on `OrganizationIntegrationType`. + +5. **Property Normalization**: The builder's `normalizePropertyCase()` method handles conversion between PascalCase + (backend) and camelCase (frontend), ensuring seamless data flow regardless of API naming conventions. + +The service uses these capabilities in methods like `save()`, `update()`, and `mapResponsesToOrganizationIntegration()` +to manage the complete lifecycle of integration configurations and templates. + +## Step 5: Add Service Name to OrganizationIntegrationServiceName + +If you're adding a new external service (not just a new integration type for an existing service), +add it to the `OrganizationIntegrationServiceName` enum in `organization-integration-service-type.ts`: + +```typescript +export const OrganizationIntegrationServiceName = Object.freeze({ + CrowdStrike: "CrowdStrike", + Datadog: "Datadog", + MyService: "MyService", // Add your new service +} as const); +``` + +This identifies the external service your integration connects to. The `bw_serviceName` property in your +configuration and template classes should use a value from this enum. + +## Step 6: File Organization + +Place your new files in the following directories: + +- **Configuration classes**: `models/configuration/` + - Example: `models/configuration/myservice-configuration.ts` +- **Template classes**: `models/integration-configuration-config/configuration-template/` + - Example: `models/integration-configuration-config/configuration-template/myservice-template.ts` + +This organization keeps related files grouped and maintains consistency with existing integrations. + +## Important Conventions + +### Template Placeholders + +Templates support standardized placeholders that are replaced with actual values at runtime. +Use the following format with hashtags: + +**Common placeholders**: + +- `#EventMessage#` - Full event message +- `#Type#` - Event type +- `#CipherId#` - Cipher/item identifier +- `#CollectionId#` - Collection identifier +- `#GroupId#` - Group identifier +- `#PolicyId#` - Policy identifier +- `#UserId#` - User identifier +- `#ActingUserId#` - User performing the action +- `#UserName#` - User's name +- `#UserEmail#` - User's email +- `#ActingUserName#` - Acting user's name +- `#ActingUserEmail#` - Acting user's email +- `#DateIso8601#` - ISO 8601 formatted date +- `#DeviceType#` - Device type +- `#IpAddress#` - IP address +- `#SecretId#` - Secret identifier +- `#ProjectId#` - Project identifier +- `#ServiceAccountId#` - Service account identifier + +These placeholders are processed server-side when events are sent to the external service. +**_Also, these placeholders are determined by the server-side implementation, so ensure your template matches the expected format._** + +## Step 7: Add Tests + +Add comprehensive tests for your new integration in three test files: + +### 7a. Integration Service Tests + +Add tests in `organization-integration-service.spec.ts`: + +```typescript +describe("MyService integration", () => { + it("should save a new MyService integration successfully", async () => { + const config = OrgIntegrationBuilder.buildMyServiceConfiguration( + "https://test.myservice.com", + "test-api-key", + OrganizationIntegrationServiceName.MyService, + ); + const template = OrgIntegrationBuilder.buildMyServiceTemplate( + OrganizationIntegrationServiceName.MyService, + ); + // ... test implementation + }); +}); +``` + +The implementation should cover save, update, delete, and load operations. +This is all that is required to make a new integration type functional within the service. + +--- + +## Understanding the Architecture + +**Workflow**: + +1. Call `setOrganizationId(orgId)` to load integrations for an organization +2. Subscribe to `integrations$` to receive the loaded integrations +3. Any save/update/delete operations automatically update `integrations$` + +The service uses `BehaviorSubject` internally to manage state and emit updates to all subscribers. + +### Error Handling Pattern + +All modification operations (`save()`, `update()`, `delete()`) return `IntegrationModificationResult`: + +```typescript +type IntegrationModificationResult = { + success: boolean; // Operation succeeded + mustBeOwner: boolean; // If false, permission denied (404) - user must be organization owner +}; +``` + +This pattern allows the UI to provide specific feedback when users lack necessary permissions. + +### Configuration vs Template + +Understanding the distinction between these two concepts is crucial: + +**Configuration (`OrgIntegrationConfiguration`)**: + +- Contains authentication and connection details +- Example: API URLs, tokens, API keys, authentication schemes +- Stored in the `Integration` record +- Usually contains sensitive data +- One per integration + +**Template (`OrgIntegrationTemplate`)**: + +- Defines the structure and format of event data +- Contains placeholders like `#UserId#`, `#EventMessage#` +- Stored in the `IntegrationConfiguration` record +- No sensitive data +- Specifies how Bitwarden events map to external service format +- One per integration (current implementation) + +When an event occurs, the system: + +1. Uses the **Configuration** to know where and how to send data +2. Uses the **Template** to format the event data for that specific service + +## Example: Complete Integration + +Here's a minimal example showing all pieces working together: + +```typescript +// 1. Configuration +export class ExampleConfiguration implements OrgIntegrationConfiguration { + uri: string; + apiKey: string; + bw_serviceName: OrganizationIntegrationServiceName; + + constructor(uri: string, apiKey: string, bw_serviceName: OrganizationIntegrationServiceName) { + this.uri = uri; + this.apiKey = apiKey; + this.bw_serviceName = bw_serviceName; + } + + toString(): string { + return JSON.stringify({ + Uri: this.uri, + ApiKey: this.apiKey, + bw_serviceName: this.bw_serviceName, + }); + } +} + +// 2. Template +export class ExampleTemplate implements OrgIntegrationTemplate { + bw_serviceName: OrganizationIntegrationServiceName; + + constructor(bw_serviceName: OrganizationIntegrationServiceName) { + this.bw_serviceName = bw_serviceName; + } + + toString(): string { + return JSON.stringify({ + bw_serviceName: this.bw_serviceName, + event: { + type: "#Type#", + user: "#UserEmail#", + timestamp: "#DateIso8601#", + }, + }); + } +} + +// 3. Usage in OrganizationIntegrationService +const config = OrgIntegrationBuilder.buildExampleConfiguration( + "https://api.example.com", + "secret-key", + OrganizationIntegrationServiceName.Example, +); + +const template = OrgIntegrationBuilder.buildExampleTemplate( + OrganizationIntegrationServiceName.Example, +); + +await service.save(orgId, OrganizationIntegrationType.Example, config, template); +``` + +This creates a complete integration that can authenticate with the external service and format event data appropriately. diff --git a/jest.config.js b/jest.config.js index 37d15eb8f92..bfe447f7a53 100644 --- a/jest.config.js +++ b/jest.config.js @@ -59,6 +59,7 @@ module.exports = { "/libs/tools/send/send-ui/jest.config.js", "/libs/user-core/jest.config.js", "/libs/vault/jest.config.js", + "/libs/auto-confirm/jest.config.js", "/libs/subscription/jest.config.js", ], diff --git a/libs/admin-console/src/common/index.ts b/libs/admin-console/src/common/index.ts index 37f79d56256..5178805cec5 100644 --- a/libs/admin-console/src/common/index.ts +++ b/libs/admin-console/src/common/index.ts @@ -1,3 +1,2 @@ -export * from "./auto-confirm"; export * from "./collections"; export * from "./organization-user"; diff --git a/libs/angular/src/admin-console/guards/index.ts b/libs/angular/src/admin-console/guards/index.ts new file mode 100644 index 00000000000..71f34285761 --- /dev/null +++ b/libs/angular/src/admin-console/guards/index.ts @@ -0,0 +1 @@ +export * from "./org-policy.guard"; diff --git a/apps/web/src/app/admin-console/organizations/guards/org-policy.guard.ts b/libs/angular/src/admin-console/guards/org-policy.guard.ts similarity index 100% rename from apps/web/src/app/admin-console/organizations/guards/org-policy.guard.ts rename to libs/angular/src/admin-console/guards/org-policy.guard.ts diff --git a/libs/angular/src/vault/services/custom-nudges-services/auto-confirm-nudge.service.spec.ts b/libs/angular/src/vault/services/custom-nudges-services/auto-confirm-nudge.service.spec.ts new file mode 100644 index 00000000000..4e8d1ed3d1a --- /dev/null +++ b/libs/angular/src/vault/services/custom-nudges-services/auto-confirm-nudge.service.spec.ts @@ -0,0 +1,226 @@ +import { TestBed } from "@angular/core/testing"; +import { mock, MockProxy } from "jest-mock-extended"; +import { BehaviorSubject, firstValueFrom } from "rxjs"; + +import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; +import { StateProvider } from "@bitwarden/common/platform/state"; +import { UserId } from "@bitwarden/user-core"; + +import { FakeStateProvider, mockAccountServiceWith } from "../../../../../../libs/common/spec"; +import { NUDGE_DISMISSED_DISK_KEY, NudgeType } from "../nudges.service"; + +import { AutoConfirmNudgeService } from "./auto-confirm-nudge.service"; + +describe("AutoConfirmNudgeService", () => { + let service: AutoConfirmNudgeService; + let autoConfirmService: MockProxy; + let fakeStateProvider: FakeStateProvider; + const userId = "user-id" as UserId; + + const mockAutoConfirmState = { + enabled: true, + showSetupDialog: false, + showBrowserNotification: true, + }; + + beforeEach(() => { + fakeStateProvider = new FakeStateProvider(mockAccountServiceWith(userId)); + autoConfirmService = mock(); + + TestBed.configureTestingModule({ + providers: [ + AutoConfirmNudgeService, + { + provide: StateProvider, + useValue: fakeStateProvider, + }, + { + provide: AutomaticUserConfirmationService, + useValue: autoConfirmService, + }, + ], + }); + + service = TestBed.inject(AutoConfirmNudgeService); + }); + + describe("nudgeStatus$", () => { + it("should return all dismissed when user cannot manage auto-confirm", async () => { + autoConfirmService.configuration$.mockReturnValue(new BehaviorSubject(mockAutoConfirmState)); + autoConfirmService.canManageAutoConfirm$.mockReturnValue(new BehaviorSubject(false)); + + const result = await firstValueFrom(service.nudgeStatus$(NudgeType.AutoConfirmNudge, userId)); + + expect(result).toEqual({ + hasBadgeDismissed: true, + hasSpotlightDismissed: true, + }); + }); + + it("should return all dismissed when showBrowserNotification is false", async () => { + autoConfirmService.configuration$.mockReturnValue( + new BehaviorSubject({ + ...mockAutoConfirmState, + showBrowserNotification: false, + }), + ); + autoConfirmService.canManageAutoConfirm$.mockReturnValue(new BehaviorSubject(true)); + + const result = await firstValueFrom(service.nudgeStatus$(NudgeType.AutoConfirmNudge, userId)); + + expect(result).toEqual({ + hasBadgeDismissed: true, + hasSpotlightDismissed: true, + }); + }); + + it("should return not dismissed when showBrowserNotification is true and user can manage", async () => { + autoConfirmService.configuration$.mockReturnValue( + new BehaviorSubject({ + ...mockAutoConfirmState, + showBrowserNotification: true, + }), + ); + autoConfirmService.canManageAutoConfirm$.mockReturnValue(new BehaviorSubject(true)); + + const result = await firstValueFrom(service.nudgeStatus$(NudgeType.AutoConfirmNudge, userId)); + + expect(result).toEqual({ + hasBadgeDismissed: false, + hasSpotlightDismissed: false, + }); + }); + + it("should return not dismissed when showBrowserNotification is undefined and user can manage", async () => { + autoConfirmService.configuration$.mockReturnValue( + new BehaviorSubject({ + ...mockAutoConfirmState, + showBrowserNotification: undefined, + }), + ); + autoConfirmService.canManageAutoConfirm$.mockReturnValue(new BehaviorSubject(true)); + + const result = await firstValueFrom(service.nudgeStatus$(NudgeType.AutoConfirmNudge, userId)); + + expect(result).toEqual({ + hasBadgeDismissed: false, + hasSpotlightDismissed: false, + }); + }); + + it("should return stored nudge status when badge is already dismissed", async () => { + await fakeStateProvider.getUser(userId, NUDGE_DISMISSED_DISK_KEY).update(() => ({ + [NudgeType.AutoConfirmNudge]: { + hasBadgeDismissed: true, + hasSpotlightDismissed: false, + }, + })); + + autoConfirmService.configuration$.mockReturnValue( + new BehaviorSubject({ + ...mockAutoConfirmState, + showBrowserNotification: true, + }), + ); + autoConfirmService.canManageAutoConfirm$.mockReturnValue(new BehaviorSubject(true)); + + const result = await firstValueFrom(service.nudgeStatus$(NudgeType.AutoConfirmNudge, userId)); + + expect(result).toEqual({ + hasBadgeDismissed: true, + hasSpotlightDismissed: false, + }); + }); + + it("should return stored nudge status when spotlight is already dismissed", async () => { + await fakeStateProvider.getUser(userId, NUDGE_DISMISSED_DISK_KEY).update(() => ({ + [NudgeType.AutoConfirmNudge]: { + hasBadgeDismissed: false, + hasSpotlightDismissed: true, + }, + })); + + autoConfirmService.configuration$.mockReturnValue( + new BehaviorSubject({ + ...mockAutoConfirmState, + showBrowserNotification: true, + }), + ); + autoConfirmService.canManageAutoConfirm$.mockReturnValue(new BehaviorSubject(true)); + + const result = await firstValueFrom(service.nudgeStatus$(NudgeType.AutoConfirmNudge, userId)); + + expect(result).toEqual({ + hasBadgeDismissed: false, + hasSpotlightDismissed: true, + }); + }); + + it("should return stored nudge status when both badge and spotlight are already dismissed", async () => { + await fakeStateProvider.getUser(userId, NUDGE_DISMISSED_DISK_KEY).update(() => ({ + [NudgeType.AutoConfirmNudge]: { + hasBadgeDismissed: true, + hasSpotlightDismissed: true, + }, + })); + + autoConfirmService.configuration$.mockReturnValue( + new BehaviorSubject({ + ...mockAutoConfirmState, + showBrowserNotification: true, + }), + ); + autoConfirmService.canManageAutoConfirm$.mockReturnValue(new BehaviorSubject(true)); + + const result = await firstValueFrom(service.nudgeStatus$(NudgeType.AutoConfirmNudge, userId)); + + expect(result).toEqual({ + hasBadgeDismissed: true, + hasSpotlightDismissed: true, + }); + }); + + it("should prioritize user permissions over showBrowserNotification setting", async () => { + await fakeStateProvider.getUser(userId, NUDGE_DISMISSED_DISK_KEY).update(() => ({ + [NudgeType.AutoConfirmNudge]: { + hasBadgeDismissed: false, + hasSpotlightDismissed: false, + }, + })); + + autoConfirmService.configuration$.mockReturnValue( + new BehaviorSubject({ + ...mockAutoConfirmState, + showBrowserNotification: true, + }), + ); + autoConfirmService.canManageAutoConfirm$.mockReturnValue(new BehaviorSubject(false)); + + const result = await firstValueFrom(service.nudgeStatus$(NudgeType.AutoConfirmNudge, userId)); + + expect(result).toEqual({ + hasBadgeDismissed: true, + hasSpotlightDismissed: true, + }); + }); + + it("should respect stored dismissal even when user cannot manage auto-confirm", async () => { + await fakeStateProvider.getUser(userId, NUDGE_DISMISSED_DISK_KEY).update(() => ({ + [NudgeType.AutoConfirmNudge]: { + hasBadgeDismissed: true, + hasSpotlightDismissed: false, + }, + })); + + autoConfirmService.configuration$.mockReturnValue(new BehaviorSubject(mockAutoConfirmState)); + autoConfirmService.canManageAutoConfirm$.mockReturnValue(new BehaviorSubject(false)); + + const result = await firstValueFrom(service.nudgeStatus$(NudgeType.AutoConfirmNudge, userId)); + + expect(result).toEqual({ + hasBadgeDismissed: true, + hasSpotlightDismissed: true, + }); + }); + }); +}); diff --git a/libs/angular/src/vault/services/custom-nudges-services/auto-confirm-nudge.service.ts b/libs/angular/src/vault/services/custom-nudges-services/auto-confirm-nudge.service.ts new file mode 100644 index 00000000000..52fc87d7604 --- /dev/null +++ b/libs/angular/src/vault/services/custom-nudges-services/auto-confirm-nudge.service.ts @@ -0,0 +1,41 @@ +import { inject, Injectable } from "@angular/core"; +import { combineLatest, map, Observable } from "rxjs"; + +import { AutomaticUserConfirmationService } from "@bitwarden/auto-confirm"; +import { UserId } from "@bitwarden/user-core"; + +import { DefaultSingleNudgeService } from "../default-single-nudge.service"; +import { NudgeType, NudgeStatus } from "../nudges.service"; + +@Injectable({ providedIn: "root" }) +export class AutoConfirmNudgeService extends DefaultSingleNudgeService { + autoConfirmService = inject(AutomaticUserConfirmationService); + + nudgeStatus$(nudgeType: NudgeType, userId: UserId): Observable { + return combineLatest([ + this.getNudgeStatus$(nudgeType, userId), + this.autoConfirmService.configuration$(userId), + this.autoConfirmService.canManageAutoConfirm$(userId), + ]).pipe( + map(([nudgeStatus, autoConfirmState, canManageAutoConfirm]) => { + if (!canManageAutoConfirm) { + return { + hasBadgeDismissed: true, + hasSpotlightDismissed: true, + }; + } + + if (nudgeStatus.hasBadgeDismissed || nudgeStatus.hasSpotlightDismissed) { + return nudgeStatus; + } + + const dismissed = autoConfirmState.showBrowserNotification === false; + + return { + hasBadgeDismissed: dismissed, + hasSpotlightDismissed: dismissed, + }; + }), + ); + } +} diff --git a/libs/angular/src/vault/services/custom-nudges-services/index.ts b/libs/angular/src/vault/services/custom-nudges-services/index.ts index d4bfe80a525..030a46c10b2 100644 --- a/libs/angular/src/vault/services/custom-nudges-services/index.ts +++ b/libs/angular/src/vault/services/custom-nudges-services/index.ts @@ -1,4 +1,5 @@ export * from "./account-security-nudge.service"; +export * from "./auto-confirm-nudge.service"; export * from "./has-items-nudge.service"; export * from "./empty-vault-nudge.service"; export * from "./vault-settings-import-nudge.service"; diff --git a/libs/angular/src/vault/services/nudges.service.spec.ts b/libs/angular/src/vault/services/nudges.service.spec.ts index cba973bd894..346b22bf122 100644 --- a/libs/angular/src/vault/services/nudges.service.spec.ts +++ b/libs/angular/src/vault/services/nudges.service.spec.ts @@ -23,6 +23,7 @@ import { AccountSecurityNudgeService, VaultSettingsImportNudgeService, } from "./custom-nudges-services"; +import { AutoConfirmNudgeService } from "./custom-nudges-services/auto-confirm-nudge.service"; import { DefaultSingleNudgeService } from "./default-single-nudge.service"; import { NudgesService, NudgeType } from "./nudges.service"; @@ -35,6 +36,7 @@ describe("Vault Nudges Service", () => { EmptyVaultNudgeService, NewAccountNudgeService, AccountSecurityNudgeService, + AutoConfirmNudgeService, ]; beforeEach(async () => { @@ -73,6 +75,10 @@ describe("Vault Nudges Service", () => { provide: VaultSettingsImportNudgeService, useValue: mock(), }, + { + provide: AutoConfirmNudgeService, + useValue: mock(), + }, { provide: ApiService, useValue: mock(), diff --git a/libs/angular/src/vault/services/nudges.service.ts b/libs/angular/src/vault/services/nudges.service.ts index 19acf690d32..afd0d184d6e 100644 --- a/libs/angular/src/vault/services/nudges.service.ts +++ b/libs/angular/src/vault/services/nudges.service.ts @@ -12,6 +12,7 @@ import { NewItemNudgeService, AccountSecurityNudgeService, VaultSettingsImportNudgeService, + AutoConfirmNudgeService, NoOpNudgeService, } from "./custom-nudges-services"; import { DefaultSingleNudgeService, SingleNudgeService } from "./default-single-nudge.service"; @@ -39,6 +40,7 @@ export const NudgeType = { NewNoteItemStatus: "new-note-item-status", NewSshItemStatus: "new-ssh-item-status", GeneratorNudgeStatus: "generator-nudge-status", + AutoConfirmNudge: "auto-confirm-nudge", PremiumUpgrade: "premium-upgrade", } as const; @@ -82,6 +84,7 @@ export class NudgesService { [NudgeType.NewIdentityItemStatus]: this.newItemNudgeService, [NudgeType.NewNoteItemStatus]: this.newItemNudgeService, [NudgeType.NewSshItemStatus]: this.newItemNudgeService, + [NudgeType.AutoConfirmNudge]: inject(AutoConfirmNudgeService), }; /** @@ -148,6 +151,7 @@ export class NudgesService { NudgeType.EmptyVaultNudge, NudgeType.DownloadBitwarden, NudgeType.AutofillNudge, + NudgeType.AutoConfirmNudge, ]; const nudgeTypesWithBadge$ = nudgeTypes.map((nudge) => { diff --git a/libs/auto-confirm/README.md b/libs/auto-confirm/README.md new file mode 100644 index 00000000000..15779018b90 --- /dev/null +++ b/libs/auto-confirm/README.md @@ -0,0 +1,18 @@ +# Automatic User Confirmation + +Owned by: admin-console + +The automatic user confirmation (auto confirm) feature enables an organization to confirm users to an organization without manual intervention +from any user as long as an administrator's device is unlocked. The feature is enabled via the following: + +1. an organization plan feature in the Bitwarden Portal (enabled by an internal team) +2. the automatic user confirmation policy in the Admin Console (enabled by an organization admin) +3. a toggle switch in the extension's admin settings page (enabled on the admin's local device) + +Once these three toggles are enabled, auto confirm will be enabled and users will be auto confirmed as long as an admin is logged in. Note that the setting in +the browser extension is not synced across clients, therefore it will not be enabled if the same admin logs into another browser until it is enabled in that +browser. This is an intentional security measure to ensure that the server cannot enable the feature unilaterally. + +Once enabled, the AutomaticUserConfirmationService runs in the background on admins' devices and reacts to push notifications from the server containing organization members who need confirmation. + +For more information about security goals and the push notification system, see [README in server repo](https://github.com/bitwarden/server/tree/main/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser). diff --git a/libs/auto-confirm/eslint.config.mjs b/libs/auto-confirm/eslint.config.mjs new file mode 100644 index 00000000000..9c37d10e3ff --- /dev/null +++ b/libs/auto-confirm/eslint.config.mjs @@ -0,0 +1,3 @@ +import baseConfig from "../../eslint.config.mjs"; + +export default [...baseConfig]; diff --git a/libs/auto-confirm/jest.config.js b/libs/auto-confirm/jest.config.js new file mode 100644 index 00000000000..461c4ef5602 --- /dev/null +++ b/libs/auto-confirm/jest.config.js @@ -0,0 +1,18 @@ +const { pathsToModuleNameMapper } = require("ts-jest"); + +const { compilerOptions } = require("../../tsconfig.base"); + +const sharedConfig = require("../../libs/shared/jest.config.angular"); + +module.exports = { + ...sharedConfig, + displayName: "auto-confirm", + setupFilesAfterEnv: ["/test.setup.ts"], + coverageDirectory: "../../coverage/libs/auto-confirm", + moduleNameMapper: pathsToModuleNameMapper( + { "@bitwarden/common/spec": ["libs/common/spec"], ...(compilerOptions?.paths ?? {}) }, + { + prefix: "/../../", + }, + ), +}; diff --git a/libs/auto-confirm/package.json b/libs/auto-confirm/package.json new file mode 100644 index 00000000000..6bb4a334d6a --- /dev/null +++ b/libs/auto-confirm/package.json @@ -0,0 +1,11 @@ +{ + "name": "@bitwarden/auto-confirm", + "version": "0.0.1", + "description": "auto confirm", + "private": true, + "type": "commonjs", + "main": "index.js", + "types": "index.d.ts", + "license": "GPL-3.0", + "author": "admin-console" +} diff --git a/libs/auto-confirm/project.json b/libs/auto-confirm/project.json new file mode 100644 index 00000000000..81efa0c77ca --- /dev/null +++ b/libs/auto-confirm/project.json @@ -0,0 +1,34 @@ +{ + "name": "auto-confirm", + "$schema": "../../node_modules/nx/schemas/project-schema.json", + "sourceRoot": "libs/auto-confirm/src", + "projectType": "library", + "tags": [], + "targets": { + "build": { + "executor": "@nx/js:tsc", + "outputs": ["{options.outputPath}"], + "options": { + "outputPath": "dist/libs/auto-confirm", + "main": "libs/auto-confirm/src/index.ts", + "tsConfig": "libs/auto-confirm/tsconfig.lib.json", + "assets": ["libs/auto-confirm/*.md"], + "rootDir": "libs/auto-confirm/src" + } + }, + "lint": { + "executor": "@nx/eslint:lint", + "outputs": ["{options.outputFile}"], + "options": { + "lintFilePatterns": ["libs/auto-confirm/**/*.ts"] + } + }, + "test": { + "executor": "@nx/jest:jest", + "outputs": ["{workspaceRoot}/coverage/{projectRoot}"], + "options": { + "jestConfig": "libs/auto-confirm/jest.config.js" + } + } + } +} diff --git a/libs/admin-console/src/common/auto-confirm/abstractions/auto-confirm.service.abstraction.ts b/libs/auto-confirm/src/abstractions/auto-confirm.service.abstraction.ts similarity index 90% rename from libs/admin-console/src/common/auto-confirm/abstractions/auto-confirm.service.abstraction.ts rename to libs/auto-confirm/src/abstractions/auto-confirm.service.abstraction.ts index e753184273e..9ce6cb9c1a4 100644 --- a/libs/admin-console/src/common/auto-confirm/abstractions/auto-confirm.service.abstraction.ts +++ b/libs/auto-confirm/src/abstractions/auto-confirm.service.abstraction.ts @@ -1,7 +1,6 @@ import { Observable } from "rxjs"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; -import { OrganizationId } from "@bitwarden/common/types/guid"; import { UserId } from "@bitwarden/user-core"; import { AutoConfirmState } from "../models/auto-confirm-state.model"; @@ -24,10 +23,7 @@ export abstract class AutomaticUserConfirmationService { * @param userId * @returns Observable an observable with a boolean telling us if the provided user may confgure the auto confirm feature. **/ - abstract canManageAutoConfirm$( - userId: UserId, - organizationId: OrganizationId, - ): Observable; + abstract canManageAutoConfirm$(userId: UserId): Observable; /** * Calls the API endpoint to initiate automatic user confirmation. * @param userId The userId of the logged in admin performing auto confirmation. This is neccesary to perform the key exchange and for permissions checks. diff --git a/libs/admin-console/src/common/auto-confirm/abstractions/index.ts b/libs/auto-confirm/src/abstractions/index.ts similarity index 100% rename from libs/admin-console/src/common/auto-confirm/abstractions/index.ts rename to libs/auto-confirm/src/abstractions/index.ts diff --git a/libs/auto-confirm/src/components/auto-confirm-warning-dialog.component.html b/libs/auto-confirm/src/components/auto-confirm-warning-dialog.component.html new file mode 100644 index 00000000000..d1697c1968d --- /dev/null +++ b/libs/auto-confirm/src/components/auto-confirm-warning-dialog.component.html @@ -0,0 +1,25 @@ + + + {{ "warningCapitalized" | i18n }} + + + {{ "autoConfirmWarning" | i18n }} + + {{ "autoConfirmWarningLink" | i18n }} + + + + + + + + diff --git a/libs/auto-confirm/src/components/auto-confirm-warning-dialog.component.ts b/libs/auto-confirm/src/components/auto-confirm-warning-dialog.component.ts new file mode 100644 index 00000000000..f126ce3b92c --- /dev/null +++ b/libs/auto-confirm/src/components/auto-confirm-warning-dialog.component.ts @@ -0,0 +1,19 @@ +import { DialogRef } from "@angular/cdk/dialog"; +import { CommonModule } from "@angular/common"; +import { ChangeDetectionStrategy, Component } from "@angular/core"; + +import { ButtonModule, DialogModule, DialogService } from "@bitwarden/components"; +import { I18nPipe } from "@bitwarden/ui-common"; + +@Component({ + changeDetection: ChangeDetectionStrategy.OnPush, + templateUrl: "./auto-confirm-warning-dialog.component.html", + imports: [ButtonModule, DialogModule, CommonModule, I18nPipe], +}) +export class AutoConfirmWarningDialogComponent { + constructor(public dialogRef: DialogRef) {} + + static open(dialogService: DialogService) { + return dialogService.open(AutoConfirmWarningDialogComponent); + } +} diff --git a/libs/auto-confirm/src/components/index.ts b/libs/auto-confirm/src/components/index.ts new file mode 100644 index 00000000000..a0310e805c6 --- /dev/null +++ b/libs/auto-confirm/src/components/index.ts @@ -0,0 +1 @@ +export * from "./auto-confirm-warning-dialog.component"; diff --git a/libs/auto-confirm/src/guards/automatic-user-confirmation-settings.guard.spec.ts b/libs/auto-confirm/src/guards/automatic-user-confirmation-settings.guard.spec.ts new file mode 100644 index 00000000000..aca51edb8dc --- /dev/null +++ b/libs/auto-confirm/src/guards/automatic-user-confirmation-settings.guard.spec.ts @@ -0,0 +1,93 @@ +import { TestBed } from "@angular/core/testing"; +import { Router, UrlTree } from "@angular/router"; +import { mock, MockProxy } from "jest-mock-extended"; +import { BehaviorSubject, firstValueFrom, Observable, of } from "rxjs"; + +import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { UserId } from "@bitwarden/common/types/guid"; +import { ToastService } from "@bitwarden/components"; +import { newGuid } from "@bitwarden/guid"; + +import { AutomaticUserConfirmationService } from "../abstractions"; + +import { canAccessAutoConfirmSettings } from "./automatic-user-confirmation-settings.guard"; + +describe("canAccessAutoConfirmSettings", () => { + let accountService: MockProxy; + let autoConfirmService: MockProxy; + let toastService: MockProxy; + let i18nService: MockProxy; + let router: MockProxy; + + const mockUserId = newGuid() as UserId; + const mockAccount: Account = { + id: mockUserId, + email: "test@example.com", + emailVerified: true, + name: "Test User", + creationDate: undefined, + }; + let activeAccount$: BehaviorSubject; + + const runGuard = () => { + return TestBed.runInInjectionContext(() => { + return canAccessAutoConfirmSettings(null as any, null as any) as Observable< + boolean | UrlTree + >; + }); + }; + + beforeEach(() => { + accountService = mock(); + autoConfirmService = mock(); + toastService = mock(); + i18nService = mock(); + router = mock(); + + activeAccount$ = new BehaviorSubject(mockAccount); + accountService.activeAccount$ = activeAccount$; + + TestBed.configureTestingModule({ + providers: [ + { provide: AccountService, useValue: accountService }, + { provide: AutomaticUserConfirmationService, useValue: autoConfirmService }, + { provide: ToastService, useValue: toastService }, + { provide: I18nService, useValue: i18nService }, + { provide: Router, useValue: router }, + ], + }); + }); + + it("should allow access when user has permission", async () => { + autoConfirmService.canManageAutoConfirm$.mockReturnValue(of(true)); + + const result = await firstValueFrom(runGuard()); + + expect(result).toBe(true); + }); + + it("should redirect to vault when user lacks permission", async () => { + autoConfirmService.canManageAutoConfirm$.mockReturnValue(of(false)); + const mockUrlTree = {} as UrlTree; + router.createUrlTree.mockReturnValue(mockUrlTree); + + const result = await firstValueFrom(runGuard()); + + expect(result).toBe(mockUrlTree); + expect(router.createUrlTree).toHaveBeenCalledWith(["/tabs/vault"]); + }); + + it("should not emit when active account is null", async () => { + activeAccount$.next(null); + autoConfirmService.canManageAutoConfirm$.mockReturnValue(of(true)); + + let guardEmitted = false; + const subscription = runGuard().subscribe(() => { + guardEmitted = true; + }); + + expect(guardEmitted).toBe(false); + subscription.unsubscribe(); + }); +}); diff --git a/libs/auto-confirm/src/guards/automatic-user-confirmation-settings.guard.ts b/libs/auto-confirm/src/guards/automatic-user-confirmation-settings.guard.ts new file mode 100644 index 00000000000..77f01ba2801 --- /dev/null +++ b/libs/auto-confirm/src/guards/automatic-user-confirmation-settings.guard.ts @@ -0,0 +1,35 @@ +import { inject } from "@angular/core"; +import { CanActivateFn, Router } from "@angular/router"; +import { map, switchMap } from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { filterOutNullish } from "@bitwarden/common/vault/utils/observable-utilities"; +import { ToastService } from "@bitwarden/components"; + +import { AutomaticUserConfirmationService } from "../abstractions"; + +export const canAccessAutoConfirmSettings: CanActivateFn = () => { + const accountService = inject(AccountService); + const autoConfirmService = inject(AutomaticUserConfirmationService); + const toastService = inject(ToastService); + const i18nService = inject(I18nService); + const router = inject(Router); + + return accountService.activeAccount$.pipe( + filterOutNullish(), + switchMap((user) => autoConfirmService.canManageAutoConfirm$(user.id)), + map((canManageAutoConfirm) => { + if (!canManageAutoConfirm) { + toastService.showToast({ + variant: "error", + title: "", + message: i18nService.t("noPermissionsViewPage"), + }); + + return router.createUrlTree(["/tabs/vault"]); + } + return true; + }), + ); +}; diff --git a/libs/auto-confirm/src/guards/index.ts b/libs/auto-confirm/src/guards/index.ts new file mode 100644 index 00000000000..fa635bcb9e1 --- /dev/null +++ b/libs/auto-confirm/src/guards/index.ts @@ -0,0 +1 @@ +export * from "./automatic-user-confirmation-settings.guard"; diff --git a/libs/admin-console/src/common/auto-confirm/index.ts b/libs/auto-confirm/src/index.ts similarity index 60% rename from libs/admin-console/src/common/auto-confirm/index.ts rename to libs/auto-confirm/src/index.ts index 9187ccd39cf..56b9d0b0285 100644 --- a/libs/admin-console/src/common/auto-confirm/index.ts +++ b/libs/auto-confirm/src/index.ts @@ -1,3 +1,5 @@ export * from "./abstractions"; +export * from "./components"; +export * from "./guards"; export * from "./models"; export * from "./services"; diff --git a/libs/admin-console/src/common/auto-confirm/models/auto-confirm-state.model.ts b/libs/auto-confirm/src/models/auto-confirm-state.model.ts similarity index 100% rename from libs/admin-console/src/common/auto-confirm/models/auto-confirm-state.model.ts rename to libs/auto-confirm/src/models/auto-confirm-state.model.ts diff --git a/libs/admin-console/src/common/auto-confirm/models/index.ts b/libs/auto-confirm/src/models/index.ts similarity index 100% rename from libs/admin-console/src/common/auto-confirm/models/index.ts rename to libs/auto-confirm/src/models/index.ts diff --git a/libs/admin-console/src/common/auto-confirm/services/default-auto-confirm.service.spec.ts b/libs/auto-confirm/src/services/default-auto-confirm.service.spec.ts similarity index 72% rename from libs/admin-console/src/common/auto-confirm/services/default-auto-confirm.service.spec.ts rename to libs/auto-confirm/src/services/default-auto-confirm.service.spec.ts index 133dac758b4..1d37378b96c 100644 --- a/libs/admin-console/src/common/auto-confirm/services/default-auto-confirm.service.spec.ts +++ b/libs/auto-confirm/src/services/default-auto-confirm.service.spec.ts @@ -1,62 +1,55 @@ import { TestBed } from "@angular/core/testing"; +import { mock, MockProxy } from "jest-mock-extended"; import { BehaviorSubject, firstValueFrom, of, throwError } from "rxjs"; -import { ApiService } from "@bitwarden/common/abstractions/api.service"; -import { InternalOrganizationServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; -import { PermissionsApi } from "@bitwarden/common/admin-console/models/api/permissions.api"; -import { OrganizationData } from "@bitwarden/common/admin-console/models/data/organization.data"; -import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { FakeStateProvider, mockAccountServiceWith } from "@bitwarden/common/spec"; -import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; - import { DefaultOrganizationUserService, OrganizationUserApiService, OrganizationUserConfirmRequest, -} from "../../organization-user"; +} from "@bitwarden/admin-console/common"; +import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { InternalOrganizationServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { PolicyType } from "@bitwarden/common/admin-console/enums"; +import { PermissionsApi } from "@bitwarden/common/admin-console/models/api/permissions.api"; +import { OrganizationData } from "@bitwarden/common/admin-console/models/data/organization.data"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { ProfileOrganizationResponse } from "@bitwarden/common/admin-console/models/response/profile-organization.response"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { UserKeyResponse } from "@bitwarden/common/models/response/user-key.response"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { FakeStateProvider, mockAccountServiceWith } from "@bitwarden/common/spec"; +import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; +import { newGuid } from "@bitwarden/guid"; + import { AUTO_CONFIRM_STATE, AutoConfirmState } from "../models/auto-confirm-state.model"; import { DefaultAutomaticUserConfirmationService } from "./default-auto-confirm.service"; describe("DefaultAutomaticUserConfirmationService", () => { let service: DefaultAutomaticUserConfirmationService; - let configService: jest.Mocked; - let apiService: jest.Mocked; - let organizationUserService: jest.Mocked; + let configService: MockProxy; + let apiService: MockProxy; + let organizationUserService: MockProxy; let stateProvider: FakeStateProvider; - let organizationService: jest.Mocked; - let organizationUserApiService: jest.Mocked; + let organizationService: MockProxy; + let organizationUserApiService: MockProxy; + let policyService: MockProxy; - const mockUserId = Utils.newGuid() as UserId; - const mockConfirmingUserId = Utils.newGuid() as UserId; - const mockOrganizationId = Utils.newGuid() as OrganizationId; + const mockUserId = newGuid() as UserId; + const mockConfirmingUserId = newGuid() as UserId; + const mockOrganizationId = newGuid() as OrganizationId; let mockOrganization: Organization; beforeEach(() => { - configService = { - getFeatureFlag$: jest.fn(), - } as any; - - apiService = { - getUserPublicKey: jest.fn(), - } as any; - - organizationUserService = { - buildConfirmRequest: jest.fn(), - } as any; - + configService = mock(); + apiService = mock(); + organizationUserService = mock(); stateProvider = new FakeStateProvider(mockAccountServiceWith(mockUserId)); - - organizationService = { - organizations$: jest.fn(), - } as any; - - organizationUserApiService = { - postOrganizationUserConfirm: jest.fn(), - } as any; + organizationService = mock(); + organizationUserApiService = mock(); + policyService = mock(); TestBed.configureTestingModule({ providers: [ @@ -70,6 +63,7 @@ describe("DefaultAutomaticUserConfirmationService", () => { useValue: organizationService, }, { provide: OrganizationUserApiService, useValue: organizationUserApiService }, + { provide: PolicyService, useValue: policyService }, ], }); @@ -80,9 +74,13 @@ describe("DefaultAutomaticUserConfirmationService", () => { stateProvider, organizationService, organizationUserApiService, + policyService, ); - const mockOrgData = new OrganizationData({} as any, {} as any); + const mockOrgData = new OrganizationData({} as ProfileOrganizationResponse, { + isMember: true, + isProviderUser: false, + }); mockOrgData.id = mockOrganizationId; mockOrgData.useAutomaticUserConfirmation = true; @@ -180,7 +178,7 @@ describe("DefaultAutomaticUserConfirmationService", () => { }); it("should preserve other user configurations when updating", async () => { - const otherUserId = Utils.newGuid() as UserId; + const otherUserId = newGuid() as UserId; const otherConfig = new AutoConfirmState(); otherConfig.enabled = true; @@ -209,12 +207,13 @@ describe("DefaultAutomaticUserConfirmationService", () => { beforeEach(() => { const organizations$ = new BehaviorSubject([mockOrganization]); organizationService.organizations$.mockReturnValue(organizations$); + policyService.policyAppliesToUser$.mockReturnValue(of(true)); }); it("should return true when feature flag is enabled and organization allows management", async () => { configService.getFeatureFlag$.mockReturnValue(of(true)); - const canManage$ = service.canManageAutoConfirm$(mockUserId, mockOrganizationId); + const canManage$ = service.canManageAutoConfirm$(mockUserId); const canManage = await firstValueFrom(canManage$); expect(canManage).toBe(true); @@ -223,7 +222,7 @@ describe("DefaultAutomaticUserConfirmationService", () => { it("should return false when feature flag is disabled", async () => { configService.getFeatureFlag$.mockReturnValue(of(false)); - const canManage$ = service.canManageAutoConfirm$(mockUserId, mockOrganizationId); + const canManage$ = service.canManageAutoConfirm$(mockUserId); const canManage = await firstValueFrom(canManage$); expect(canManage).toBe(false); @@ -233,7 +232,10 @@ describe("DefaultAutomaticUserConfirmationService", () => { configService.getFeatureFlag$.mockReturnValue(of(true)); // Create organization without manageUsers permission - const mockOrgData = new OrganizationData({} as any, {} as any); + const mockOrgData = new OrganizationData({} as ProfileOrganizationResponse, { + isMember: true, + isProviderUser: false, + }); mockOrgData.id = mockOrganizationId; mockOrgData.useAutomaticUserConfirmation = true; const permissions = new PermissionsApi(); @@ -244,7 +246,7 @@ describe("DefaultAutomaticUserConfirmationService", () => { const organizations$ = new BehaviorSubject([orgWithoutManageUsers]); organizationService.organizations$.mockReturnValue(organizations$); - const canManage$ = service.canManageAutoConfirm$(mockUserId, mockOrganizationId); + const canManage$ = service.canManageAutoConfirm$(mockUserId); const canManage = await firstValueFrom(canManage$); expect(canManage).toBe(false); @@ -254,7 +256,10 @@ describe("DefaultAutomaticUserConfirmationService", () => { configService.getFeatureFlag$.mockReturnValue(of(true)); // Create organization without useAutomaticUserConfirmation - const mockOrgData = new OrganizationData({} as any, {} as any); + const mockOrgData = new OrganizationData({} as ProfileOrganizationResponse, { + isMember: true, + isProviderUser: false, + }); mockOrgData.id = mockOrganizationId; mockOrgData.useAutomaticUserConfirmation = false; const permissions = new PermissionsApi(); @@ -265,7 +270,7 @@ describe("DefaultAutomaticUserConfirmationService", () => { const organizations$ = new BehaviorSubject([orgWithoutAutoConfirm]); organizationService.organizations$.mockReturnValue(organizations$); - const canManage$ = service.canManageAutoConfirm$(mockUserId, mockOrganizationId); + const canManage$ = service.canManageAutoConfirm$(mockUserId); const canManage = await firstValueFrom(canManage$); expect(canManage).toBe(false); @@ -277,7 +282,31 @@ describe("DefaultAutomaticUserConfirmationService", () => { const organizations$ = new BehaviorSubject([]); organizationService.organizations$.mockReturnValue(organizations$); - const canManage$ = service.canManageAutoConfirm$(mockUserId, mockOrganizationId); + const canManage$ = service.canManageAutoConfirm$(mockUserId); + const canManage = await firstValueFrom(canManage$); + + expect(canManage).toBe(false); + }); + + it("should return false when the user is not a member of any organizations", async () => { + configService.getFeatureFlag$.mockReturnValue(of(true)); + + // Create organization where user is not a member + const mockOrgData = new OrganizationData({} as ProfileOrganizationResponse, { + isMember: false, + isProviderUser: false, + }); + mockOrgData.id = mockOrganizationId; + mockOrgData.useAutomaticUserConfirmation = true; + const permissions = new PermissionsApi(); + permissions.manageUsers = true; + mockOrgData.permissions = permissions; + const orgWhereNotMember = new Organization(mockOrgData); + + const organizations$ = new BehaviorSubject([orgWhereNotMember]); + organizationService.organizations$.mockReturnValue(organizations$); + + const canManage$ = service.canManageAutoConfirm$(mockUserId); const canManage = await firstValueFrom(canManage$); expect(canManage).toBe(false); @@ -286,11 +315,58 @@ describe("DefaultAutomaticUserConfirmationService", () => { it("should use the correct feature flag", async () => { configService.getFeatureFlag$.mockReturnValue(of(true)); - const canManage$ = service.canManageAutoConfirm$(mockUserId, mockOrganizationId); + const canManage$ = service.canManageAutoConfirm$(mockUserId); await firstValueFrom(canManage$); expect(configService.getFeatureFlag$).toHaveBeenCalledWith(FeatureFlag.AutoConfirm); }); + + it("should return false when policy does not apply to user", async () => { + configService.getFeatureFlag$.mockReturnValue(of(true)); + policyService.policyAppliesToUser$.mockReturnValue(of(false)); + + const canManage$ = service.canManageAutoConfirm$(mockUserId); + const canManage = await firstValueFrom(canManage$); + + expect(canManage).toBe(false); + }); + + it("should return true when policy applies to user", async () => { + configService.getFeatureFlag$.mockReturnValue(of(true)); + policyService.policyAppliesToUser$.mockReturnValue(of(true)); + + const canManage$ = service.canManageAutoConfirm$(mockUserId); + const canManage = await firstValueFrom(canManage$); + + expect(canManage).toBe(true); + }); + + it("should check policy with correct PolicyType and userId", async () => { + configService.getFeatureFlag$.mockReturnValue(of(true)); + policyService.policyAppliesToUser$.mockReturnValue(of(true)); + + const canManage$ = service.canManageAutoConfirm$(mockUserId); + await firstValueFrom(canManage$); + + expect(policyService.policyAppliesToUser$).toHaveBeenCalledWith( + PolicyType.AutoConfirm, + mockUserId, + ); + }); + + it("should return false when feature flag is enabled but policy does not apply", async () => { + configService.getFeatureFlag$.mockReturnValue(of(true)); + policyService.policyAppliesToUser$.mockReturnValue(of(false)); + + const canManage$ = service.canManageAutoConfirm$(mockUserId); + const canManage = await firstValueFrom(canManage$); + + expect(canManage).toBe(false); + expect(policyService.policyAppliesToUser$).toHaveBeenCalledWith( + PolicyType.AutoConfirm, + mockUserId, + ); + }); }); describe("autoConfirmUser", () => { @@ -305,8 +381,11 @@ describe("DefaultAutomaticUserConfirmationService", () => { const organizations$ = new BehaviorSubject([mockOrganization]); organizationService.organizations$.mockReturnValue(organizations$); configService.getFeatureFlag$.mockReturnValue(of(true)); + policyService.policyAppliesToUser$.mockReturnValue(of(true)); - apiService.getUserPublicKey.mockResolvedValue({ publicKey: mockPublicKey } as any); + apiService.getUserPublicKey.mockResolvedValue({ + publicKey: mockPublicKey, + } as UserKeyResponse); jest.spyOn(Utils, "fromB64ToArray").mockReturnValue(mockPublicKeyArray); organizationUserService.buildConfirmRequest.mockReturnValue(of(mockConfirmRequest)); organizationUserApiService.postOrganizationUserConfirm.mockResolvedValue(undefined); diff --git a/libs/admin-console/src/common/auto-confirm/services/default-auto-confirm.service.ts b/libs/auto-confirm/src/services/default-auto-confirm.service.ts similarity index 75% rename from libs/admin-console/src/common/auto-confirm/services/default-auto-confirm.service.ts rename to libs/auto-confirm/src/services/default-auto-confirm.service.ts index d6c435b84a3..109ccb6c9db 100644 --- a/libs/admin-console/src/common/auto-confirm/services/default-auto-confirm.service.ts +++ b/libs/auto-confirm/src/services/default-auto-confirm.service.ts @@ -1,17 +1,20 @@ import { combineLatest, firstValueFrom, map, Observable, switchMap } from "rxjs"; +import { + OrganizationUserApiService, + OrganizationUserService, +} from "@bitwarden/admin-console/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { InternalOrganizationServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; -import { getById } from "@bitwarden/common/platform/misc"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { OrganizationId } from "@bitwarden/common/types/guid"; import { StateProvider } from "@bitwarden/state"; import { UserId } from "@bitwarden/user-core"; -import { OrganizationUserApiService, OrganizationUserService } from "../../organization-user"; import { AutomaticUserConfirmationService } from "../abstractions/auto-confirm.service.abstraction"; import { AUTO_CONFIRM_STATE, AutoConfirmState } from "../models/auto-confirm-state.model"; @@ -23,6 +26,7 @@ export class DefaultAutomaticUserConfirmationService implements AutomaticUserCon private stateProvider: StateProvider, private organizationService: InternalOrganizationServiceAbstraction, private organizationUserApiService: OrganizationUserApiService, + private policyService: PolicyService, ) {} private autoConfirmState(userId: UserId) { return this.stateProvider.getUser(userId, AUTO_CONFIRM_STATE); @@ -43,15 +47,19 @@ export class DefaultAutomaticUserConfirmationService implements AutomaticUserCon }); } - canManageAutoConfirm$(userId: UserId, organizationId: OrganizationId): Observable { + canManageAutoConfirm$(userId: UserId): Observable { return combineLatest([ this.configService.getFeatureFlag$(FeatureFlag.AutoConfirm), - this.organizationService.organizations$(userId).pipe(getById(organizationId)), + this.organizationService + .organizations$(userId) + // auto-confirm does not allow the user to be part of any other organization (even if admin or owner) + // so we can assume that the first organization is the relevant one. + .pipe(map((organizations) => organizations[0])), + this.policyService.policyAppliesToUser$(PolicyType.AutoConfirm, userId), ]).pipe( map( - ([enabled, organization]) => - (enabled && organization?.canManageUsers && organization?.useAutomaticUserConfirmation) ?? - false, + ([enabled, organization, policyEnabled]) => + enabled && policyEnabled && (organization?.canManageAutoConfirm ?? false), ), ); } @@ -62,7 +70,7 @@ export class DefaultAutomaticUserConfirmationService implements AutomaticUserCon organization: Organization, ): Promise { await firstValueFrom( - this.canManageAutoConfirm$(userId, organization.id).pipe( + this.canManageAutoConfirm$(userId).pipe( map((canManage) => { if (!canManage) { throw new Error("Cannot automatically confirm user (insufficient permissions)"); diff --git a/libs/admin-console/src/common/auto-confirm/services/index.ts b/libs/auto-confirm/src/services/index.ts similarity index 100% rename from libs/admin-console/src/common/auto-confirm/services/index.ts rename to libs/auto-confirm/src/services/index.ts diff --git a/libs/auto-confirm/test.setup.ts b/libs/auto-confirm/test.setup.ts new file mode 100644 index 00000000000..5c248668a6d --- /dev/null +++ b/libs/auto-confirm/test.setup.ts @@ -0,0 +1,23 @@ +import "@bitwarden/ui-common/setup-jest"; + +Object.defineProperty(window, "CSS", { value: null }); +Object.defineProperty(window, "getComputedStyle", { + value: () => { + return { + display: "none", + appearance: ["-webkit-appearance"], + }; + }, +}); + +Object.defineProperty(document, "doctype", { + value: "", +}); +Object.defineProperty(document.body.style, "transform", { + value: () => { + return { + enumerable: true, + configurable: true, + }; + }, +}); diff --git a/libs/auto-confirm/tsconfig.eslint.json b/libs/auto-confirm/tsconfig.eslint.json new file mode 100644 index 00000000000..3daf120441a --- /dev/null +++ b/libs/auto-confirm/tsconfig.eslint.json @@ -0,0 +1,6 @@ +{ + "extends": "../../tsconfig.base.json", + "files": [], + "include": ["src/**/*.ts", "src/**/*.js"], + "exclude": ["**/build", "**/dist"] +} diff --git a/libs/auto-confirm/tsconfig.json b/libs/auto-confirm/tsconfig.json new file mode 100644 index 00000000000..62ebbd94647 --- /dev/null +++ b/libs/auto-confirm/tsconfig.json @@ -0,0 +1,13 @@ +{ + "extends": "../../tsconfig.base.json", + "files": [], + "include": [], + "references": [ + { + "path": "./tsconfig.lib.json" + }, + { + "path": "./tsconfig.spec.json" + } + ] +} diff --git a/libs/auto-confirm/tsconfig.lib.json b/libs/auto-confirm/tsconfig.lib.json new file mode 100644 index 00000000000..9cbf6736007 --- /dev/null +++ b/libs/auto-confirm/tsconfig.lib.json @@ -0,0 +1,10 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "outDir": "../../dist/out-tsc", + "declaration": true, + "types": ["node"] + }, + "include": ["src/**/*.ts"], + "exclude": ["jest.config.js", "src/**/*.spec.ts"] +} diff --git a/libs/auto-confirm/tsconfig.spec.json b/libs/auto-confirm/tsconfig.spec.json new file mode 100644 index 00000000000..1275f148a18 --- /dev/null +++ b/libs/auto-confirm/tsconfig.spec.json @@ -0,0 +1,10 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "outDir": "../../dist/out-tsc", + "module": "commonjs", + "moduleResolution": "node10", + "types": ["jest", "node"] + }, + "include": ["jest.config.ts", "src/**/*.test.ts", "src/**/*.spec.ts", "src/**/*.d.ts"] +} diff --git a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts index 54d2f93ac03..d1181343549 100644 --- a/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/organization/organization.service.abstraction.ts @@ -75,8 +75,8 @@ export function canAccessEmergencyAccess( ) { return combineLatest([ configService.getFeatureFlag$(FeatureFlag.AutoConfirm), - policyService.policiesByType$(PolicyType.AutoConfirm, userId), - ]).pipe(map(([enabled, policies]) => !enabled || !policies.some((p) => p.enabled))); + policyService.policyAppliesToUser$(PolicyType.AutoConfirm, userId), + ]).pipe(map(([enabled, policyAppliesToUser]) => !(enabled && policyAppliesToUser))); } /** diff --git a/libs/common/src/admin-console/models/domain/organization.ts b/libs/common/src/admin-console/models/domain/organization.ts index 13c7a48e6c4..2991ffb7caa 100644 --- a/libs/common/src/admin-console/models/domain/organization.ts +++ b/libs/common/src/admin-console/models/domain/organization.ts @@ -383,6 +383,13 @@ export class Organization { return this.familySponsorshipAvailable || this.familySponsorshipFriendlyName !== null; } + /** + * Do not call this function to perform business logic, use the function in @link AutomaticUserConfirmationService instead. + **/ + get canManageAutoConfirm() { + return this.isMember && this.canManageUsers && this.useAutomaticUserConfirmation; + } + static fromJSON(json: Jsonify) { if (json == null) { return null; diff --git a/libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.spec.ts b/libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.spec.ts index f9cb93d05b8..e6363b490cb 100644 --- a/libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.spec.ts +++ b/libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.spec.ts @@ -1,6 +1,5 @@ import { mock, MockProxy } from "jest-mock-extended"; import { BehaviorSubject, firstValueFrom, Subject } from "rxjs"; -import { filter } from "rxjs/operators"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; @@ -98,32 +97,19 @@ describe("PhishingDetectionSettingsService", () => { describe("enabled$", () => { it("should default to true if an account is logged in", async () => { activeAccountSubject.next(account); - featureFlagSubject.next(true); - premiumStatusSubject.next(true); - organizationsSubject.next([]); const result = await firstValueFrom(service.enabled$); expect(result).toBe(true); }); it("should return the stored value", async () => { activeAccountSubject.next(account); - featureFlagSubject.next(true); - premiumStatusSubject.next(true); - organizationsSubject.next([]); - - // Wait for initial emission (startWith(true)) - await firstValueFrom(service.enabled$); await service.setEnabled(mockUserId, false); - // Wait for the next emission after state update - const resultDisabled = await firstValueFrom( - service.enabled$.pipe(filter((v) => v === false)), - ); + const resultDisabled = await firstValueFrom(service.enabled$); expect(resultDisabled).toBe(false); await service.setEnabled(mockUserId, true); - // Wait for the next emission after state update - const resultEnabled = await firstValueFrom(service.enabled$.pipe(filter((v) => v === true))); + const resultEnabled = await firstValueFrom(service.enabled$); expect(resultEnabled).toBe(true); }); }); @@ -131,21 +117,12 @@ describe("PhishingDetectionSettingsService", () => { describe("setEnabled", () => { it("should update the stored value", async () => { activeAccountSubject.next(account); - featureFlagSubject.next(true); - premiumStatusSubject.next(true); - organizationsSubject.next([]); - - // Wait for initial emission (startWith(true)) - await firstValueFrom(service.enabled$); - await service.setEnabled(mockUserId, false); - // Wait for the next emission after state update - let result = await firstValueFrom(service.enabled$.pipe(filter((v) => v === false))); + let result = await firstValueFrom(service.enabled$); expect(result).toBe(false); await service.setEnabled(mockUserId, true); - // Wait for the next emission after state update - result = await firstValueFrom(service.enabled$.pipe(filter((v) => v === true))); + result = await firstValueFrom(service.enabled$); expect(result).toBe(true); }); }); diff --git a/libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.ts b/libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.ts index 9927e099f24..e30592b2f68 100644 --- a/libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.ts +++ b/libs/common/src/dirt/services/phishing-detection/phishing-detection-settings.service.ts @@ -1,5 +1,5 @@ import { combineLatest, Observable, of, switchMap } from "rxjs"; -import { catchError, distinctUntilChanged, map, shareReplay, startWith } from "rxjs/operators"; +import { catchError, distinctUntilChanged, map, shareReplay } from "rxjs/operators"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; @@ -18,9 +18,7 @@ const ENABLE_PHISHING_DETECTION = new UserKeyDefinition( PHISHING_DETECTION_DISK, "enablePhishingDetection", { - deserializer: (value: boolean) => { - return value ?? true; - }, // Default: enabled + deserializer: (value: boolean) => value ?? true, // Default: enabled clearOn: [], }, ); @@ -99,11 +97,9 @@ export class PhishingDetectionSettingsService implements PhishingDetectionSettin if (!account) { return of(false); } - return this.stateProvider.getUserState$(ENABLE_PHISHING_DETECTION, account.id).pipe( - startWith(true), // Default: enabled (matches deserializer default) - map((enabled) => enabled ?? true), - ); + return this.stateProvider.getUserState$(ENABLE_PHISHING_DETECTION, account.id); }), + map((enabled) => enabled ?? true), ); } diff --git a/libs/key-management/src/abstractions/key.service.ts b/libs/key-management/src/abstractions/key.service.ts index feb4a38ac27..6cf44544422 100644 --- a/libs/key-management/src/abstractions/key.service.ts +++ b/libs/key-management/src/abstractions/key.service.ts @@ -129,11 +129,11 @@ export abstract class KeyService { /** * Generates a new user key * @deprecated Interacting with the master key directly is prohibited. Use {@link makeUserKeyV1} instead. - * @throws Error when master key is null and there is no active user - * @param masterKey The user's master key. When null, grabs master key from active user. + * @throws Error when master key is null or undefined. + * @param masterKey The user's master key. * @returns A new user key and the master key protected version of it */ - abstract makeUserKey(masterKey: MasterKey | null): Promise<[UserKey, EncString]>; + abstract makeUserKey(masterKey: MasterKey): Promise<[UserKey, EncString]>; /** * Generates a new user key for a V1 user * Note: This will be replaced by a higher level function to initialize a whole users cryptographic state in the near future. diff --git a/libs/key-management/src/key.service.spec.ts b/libs/key-management/src/key.service.spec.ts index c0af62fe6e9..c0a0ab62347 100644 --- a/libs/key-management/src/key.service.spec.ts +++ b/libs/key-management/src/key.service.spec.ts @@ -177,6 +177,32 @@ describe("keyService", () => { }); }); + describe("makeUserKey", () => { + test.each([null as unknown as MasterKey, undefined as unknown as MasterKey])( + "throws when the provided masterKey is %s", + async (masterKey) => { + await expect(keyService.makeUserKey(masterKey)).rejects.toThrow("MasterKey is required"); + }, + ); + + it("encrypts the user key with the master key", async () => { + const mockUserKey = makeSymmetricCryptoKey(64); + const mockEncryptedUserKey = makeEncString("encryptedUserKey"); + + keyGenerationService.createKey.mockResolvedValue(mockUserKey); + encryptService.wrapSymmetricKey.mockResolvedValue(mockEncryptedUserKey); + const stretchedMasterKey = new SymmetricCryptoKey(new Uint8Array(64)); + keyGenerationService.stretchKey.mockResolvedValue(stretchedMasterKey); + + const result = await keyService.makeUserKey(makeSymmetricCryptoKey(32)); + + expect(encryptService.wrapSymmetricKey).toHaveBeenCalledWith(mockUserKey, stretchedMasterKey); + expect(keyGenerationService.createKey).toHaveBeenCalledWith(512); + expect(result[0]).toBe(mockUserKey); + expect(result[1]).toBe(mockEncryptedUserKey); + }); + }); + describe("everHadUserKey$", () => { let everHadUserKeyState: FakeSingleUserState; diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts index 621a8135d1e..8cb072a4c2a 100644 --- a/libs/key-management/src/key.service.ts +++ b/libs/key-management/src/key.service.ts @@ -204,17 +204,9 @@ export class DefaultKeyService implements KeyServiceAbstraction { return (await firstValueFrom(this.stateProvider.getUserState$(USER_KEY, userId))) != null; } - async makeUserKey(masterKey: MasterKey | null): Promise<[UserKey, EncString]> { - if (masterKey == null) { - const userId = await firstValueFrom(this.stateProvider.activeUserId$); - if (userId == null) { - throw new Error("No active user id found."); - } - - masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId)); - } - if (masterKey == null) { - throw new Error("No Master Key found."); + async makeUserKey(masterKey: MasterKey): Promise<[UserKey, EncString]> { + if (!masterKey) { + throw new Error("MasterKey is required"); } const newUserKey = await this.keyGenerationService.createKey(512); diff --git a/package-lock.json b/package-lock.json index 78b9dce23db..eec3487b6d4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -513,6 +513,11 @@ "version": "0.0.0", "license": "GPL-3.0" }, + "libs/auto-confirm": { + "name": "@bitwarden/auto-confirm", + "version": "0.0.1", + "license": "GPL-3.0" + }, "libs/billing": { "name": "@bitwarden/billing", "version": "0.0.0", @@ -4956,6 +4961,10 @@ "resolved": "libs/auth", "link": true }, + "node_modules/@bitwarden/auto-confirm": { + "resolved": "libs/auto-confirm", + "link": true + }, "node_modules/@bitwarden/billing": { "resolved": "libs/billing", "link": true diff --git a/tsconfig.base.json b/tsconfig.base.json index d91e8cb9890..68498cfae01 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -24,6 +24,7 @@ "@bitwarden/assets/svg": ["./libs/assets/src/svg/index.ts"], "@bitwarden/auth/angular": ["./libs/auth/src/angular"], "@bitwarden/auth/common": ["./libs/auth/src/common"], + "@bitwarden/auto-confirm": ["libs/auto-confirm/src/index.ts"], "@bitwarden/billing": ["./libs/billing/src"], "@bitwarden/bit-common/*": ["./bitwarden_license/bit-common/src/*"], "@bitwarden/browser/*": ["./apps/browser/src/*"],