From c154b6ad9b2db705834affcaf5c4c2ad3c4f125f Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Sun, 30 Mar 2025 12:57:05 -0400 Subject: [PATCH 1/5] Clean up remove-server-version-header feature flag (#5573) * Removed feature flag. * Linting. --- src/Core/Constants.cs | 1 - src/SharedWeb/Utilities/RequestLoggingMiddleware.cs | 10 ---------- 2 files changed, 11 deletions(-) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 3a4d3b2e13..7e39a586f4 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -163,7 +163,6 @@ public static class FeatureFlagKeys public const string EnableNewCardCombinedExpiryAutofill = "enable-new-card-combined-expiry-autofill"; public const string StorageReseedRefactor = "storage-reseed-refactor"; public const string TrialPayment = "PM-8163-trial-payment"; - public const string RemoveServerVersionHeader = "remove-server-version-header"; public const string GeneratorToolsModernization = "generator-tools-modernization"; public const string NewDeviceVerification = "new-device-verification"; public const string MacOsNativeCredentialSync = "macos-native-credential-sync"; diff --git a/src/SharedWeb/Utilities/RequestLoggingMiddleware.cs b/src/SharedWeb/Utilities/RequestLoggingMiddleware.cs index 77efdbfcf0..7f2db27eec 100644 --- a/src/SharedWeb/Utilities/RequestLoggingMiddleware.cs +++ b/src/SharedWeb/Utilities/RequestLoggingMiddleware.cs @@ -1,5 +1,4 @@ using System.Collections; -using Bit.Core; using Bit.Core.Services; using Bit.Core.Settings; using Bit.Core.Utilities; @@ -25,15 +24,6 @@ public sealed class RequestLoggingMiddleware public Task Invoke(HttpContext context, IFeatureService featureService) { - if (!featureService.IsEnabled(FeatureFlagKeys.RemoveServerVersionHeader)) - { - context.Response.OnStarting(() => - { - context.Response.Headers.Append("Server-Version", AssemblyHelpers.GetVersion()); - return Task.CompletedTask; - }); - } - using (_logger.BeginScope( new RequestLogScope(context.GetIpAddress(_globalSettings), GetHeaderValue(context, "user-agent"), From ad05e3f9e157786c201e068971f79ca7dc6079df Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Sun, 30 Mar 2025 16:03:09 -0400 Subject: [PATCH 2/5] Complete feature flag grouping by team (#5574) * Completed grouping of feature flags by team. * Completed grouping feature flags by team. * Linting * Moved flag. * Moved ssh-key-vault-item to KM. --- src/Core/Constants.cs | 129 ++++++++++++++++++++++-------------------- 1 file changed, 67 insertions(+), 62 deletions(-) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 7e39a586f4..ea360eb744 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -110,12 +110,79 @@ public static class FeatureFlagKeys public const string PolicyRequirements = "pm-14439-policy-requirements"; public const string SsoExternalIdVisibility = "pm-18630-sso-external-id-visibility"; + /* Auth Team */ + public const string PM9112DeviceApprovalPersistence = "pm-9112-device-approval-persistence"; + public const string DuoRedirect = "duo-redirect"; + public const string EmailVerification = "email-verification"; + public const string EmailVerificationDisableTimingDelays = "email-verification-disable-timing-delays"; + public const string DeviceTrustLogging = "pm-8285-device-trust-logging"; + public const string AuthenticatorTwoFactorToken = "authenticator-2fa-token"; + public const string UnauthenticatedExtensionUIRefresh = "unauth-ui-refresh"; + public const string NewDeviceVerification = "new-device-verification"; + public const string SetInitialPasswordRefactor = "pm-16117-set-initial-password-refactor"; + public const string ChangeExistingPasswordRefactor = "pm-16117-change-existing-password-refactor"; + public const string RecoveryCodeLogin = "pm-17128-recovery-code-login"; + + /* Autofill Team */ + public const string IdpAutoSubmitLogin = "idp-auto-submit-login"; + public const string UseTreeWalkerApiForPageDetailsCollection = "use-tree-walker-api-for-page-details-collection"; + public const string InlineMenuFieldQualification = "inline-menu-field-qualification"; + public const string InlineMenuPositioningImprovements = "inline-menu-positioning-improvements"; + public const string SSHAgent = "ssh-agent"; + public const string SSHVersionCheckQAOverride = "ssh-version-check-qa-override"; + public const string GenerateIdentityFillScriptRefactor = "generate-identity-fill-script-refactor"; + public const string DelayFido2PageScriptInitWithinMv2 = "delay-fido2-page-script-init-within-mv2"; + public const string NotificationBarAddLoginImprovements = "notification-bar-add-login-improvements"; + public const string BlockBrowserInjectionsByDomain = "block-browser-injections-by-domain"; + public const string NotificationRefresh = "notification-refresh"; + public const string EnableNewCardCombinedExpiryAutofill = "enable-new-card-combined-expiry-autofill"; + public const string MacOsNativeCredentialSync = "macos-native-credential-sync"; + public const string InlineMenuTotp = "inline-menu-totp"; + + /* Billing Team */ + public const string AC2101UpdateTrialInitiationEmail = "AC-2101-update-trial-initiation-email"; + public const string TrialPayment = "PM-8163-trial-payment"; + public const string ResellerManagedOrgAlert = "PM-15814-alert-owners-of-reseller-managed-orgs"; + public const string UsePricingService = "use-pricing-service"; + public const string P15179_AddExistingOrgsFromProviderPortal = "pm-15179-add-existing-orgs-from-provider-portal"; + public const string PM12276Breadcrumbing = "pm-12276-breadcrumbing-for-business-features"; + public const string PM18794_ProviderPaymentMethod = "pm-18794-provider-payment-method"; + + /* Key Management Team */ + public const string ReturnErrorOnExistingKeypair = "return-error-on-existing-keypair"; + public const string PM4154BulkEncryptionService = "PM-4154-bulk-encryption-service"; + public const string PrivateKeyRegeneration = "pm-12241-private-key-regeneration"; + public const string Argon2Default = "argon2-default"; + public const string UserkeyRotationV2 = "userkey-rotation-v2"; + public const string SSHKeyItemVaultItem = "ssh-key-vault-item"; + + /* Mobile Team */ + public const string NativeCarouselFlow = "native-carousel-flow"; + public const string NativeCreateAccountFlow = "native-create-account-flow"; + public const string AndroidImportLoginsFlow = "import-logins-flow"; + public const string AppReviewPrompt = "app-review-prompt"; + public const string EnablePasswordManagerSyncAndroid = "enable-password-manager-sync-android"; + public const string EnablePasswordManagerSynciOS = "enable-password-manager-sync-ios"; + public const string AndroidMutualTls = "mutual-tls"; + public const string SingleTapPasskeyCreation = "single-tap-passkey-creation"; + public const string SingleTapPasskeyAuthentication = "single-tap-passkey-authentication"; + public const string EnablePMAuthenticatorSync = "enable-pm-bwa-sync"; + public const string PM3503_MobileAnonAddySelfHostAlias = "anon-addy-self-host-alias"; + public const string PM3553_MobileSimpleLoginSelfHostAlias = "simple-login-self-host-alias"; + + /* Platform Team */ + public const string PersistPopupView = "persist-popup-view"; + public const string StorageReseedRefactor = "storage-reseed-refactor"; + public const string WebPush = "web-push"; + public const string RecordInstallationLastActivityDate = "installation-last-activity-date"; + /* Tools Team */ public const string ItemShare = "item-share"; public const string RiskInsightsCriticalApplication = "pm-14466-risk-insights-critical-application"; public const string EnableRiskInsightsNotifications = "enable-risk-insights-notifications"; public const string DesktopSendUIRefresh = "desktop-send-ui-refresh"; public const string ExportAttachments = "export-attachments"; + public const string GeneratorToolsModernization = "generator-tools-modernization"; /* Vault Team */ public const string PM8851_BrowserOnboardingNudge = "pm-8851-browser-onboarding-nudge"; @@ -125,69 +192,7 @@ public static class FeatureFlagKeys public const string VaultBulkManagementAction = "vault-bulk-management-action"; public const string RestrictProviderAccess = "restrict-provider-access"; public const string SecurityTasks = "security-tasks"; - - /* Auth Team */ - public const string PM9112DeviceApprovalPersistence = "pm-9112-device-approval-persistence"; - - /* Key Management Team */ - public const string SSHKeyItemVaultItem = "ssh-key-vault-item"; - public const string SSHAgent = "ssh-agent"; - public const string SSHVersionCheckQAOverride = "ssh-version-check-qa-override"; - public const string Argon2Default = "argon2-default"; - public const string PM4154BulkEncryptionService = "PM-4154-bulk-encryption-service"; - public const string PrivateKeyRegeneration = "pm-12241-private-key-regeneration"; - public const string UserkeyRotationV2 = "userkey-rotation-v2"; - - /* Unsorted */ - public const string ReturnErrorOnExistingKeypair = "return-error-on-existing-keypair"; - public const string UseTreeWalkerApiForPageDetailsCollection = "use-tree-walker-api-for-page-details-collection"; - public const string DuoRedirect = "duo-redirect"; - public const string AC2101UpdateTrialInitiationEmail = "AC-2101-update-trial-initiation-email"; - public const string EmailVerification = "email-verification"; - public const string EmailVerificationDisableTimingDelays = "email-verification-disable-timing-delays"; - public const string InlineMenuFieldQualification = "inline-menu-field-qualification"; - public const string InlineMenuPositioningImprovements = "inline-menu-positioning-improvements"; - public const string DeviceTrustLogging = "pm-8285-device-trust-logging"; - public const string AuthenticatorTwoFactorToken = "authenticator-2fa-token"; - public const string IdpAutoSubmitLogin = "idp-auto-submit-login"; - public const string UnauthenticatedExtensionUIRefresh = "unauth-ui-refresh"; - public const string GenerateIdentityFillScriptRefactor = "generate-identity-fill-script-refactor"; - public const string DelayFido2PageScriptInitWithinMv2 = "delay-fido2-page-script-init-within-mv2"; - public const string NativeCarouselFlow = "native-carousel-flow"; - public const string NativeCreateAccountFlow = "native-create-account-flow"; - public const string NotificationBarAddLoginImprovements = "notification-bar-add-login-improvements"; - public const string BlockBrowserInjectionsByDomain = "block-browser-injections-by-domain"; - public const string NotificationRefresh = "notification-refresh"; - public const string PersistPopupView = "persist-popup-view"; public const string CipherKeyEncryption = "cipher-key-encryption"; - public const string EnableNewCardCombinedExpiryAutofill = "enable-new-card-combined-expiry-autofill"; - public const string StorageReseedRefactor = "storage-reseed-refactor"; - public const string TrialPayment = "PM-8163-trial-payment"; - public const string GeneratorToolsModernization = "generator-tools-modernization"; - public const string NewDeviceVerification = "new-device-verification"; - public const string MacOsNativeCredentialSync = "macos-native-credential-sync"; - public const string InlineMenuTotp = "inline-menu-totp"; - public const string AppReviewPrompt = "app-review-prompt"; - public const string ResellerManagedOrgAlert = "PM-15814-alert-owners-of-reseller-managed-orgs"; - public const string UsePricingService = "use-pricing-service"; - public const string RecordInstallationLastActivityDate = "installation-last-activity-date"; - public const string EnablePasswordManagerSyncAndroid = "enable-password-manager-sync-android"; - public const string EnablePasswordManagerSynciOS = "enable-password-manager-sync-ios"; - public const string AccountDeprovisioningBanner = "pm-17120-account-deprovisioning-admin-console-banner"; - public const string SingleTapPasskeyCreation = "single-tap-passkey-creation"; - public const string SingleTapPasskeyAuthentication = "single-tap-passkey-authentication"; - public const string EnablePMAuthenticatorSync = "enable-pm-bwa-sync"; - public const string P15179_AddExistingOrgsFromProviderPortal = "pm-15179-add-existing-orgs-from-provider-portal"; - public const string AndroidMutualTls = "mutual-tls"; - public const string RecoveryCodeLogin = "pm-17128-recovery-code-login"; - public const string PM3503_MobileAnonAddySelfHostAlias = "anon-addy-self-host-alias"; - public const string WebPush = "web-push"; - public const string AndroidImportLoginsFlow = "import-logins-flow"; - public const string PM12276Breadcrumbing = "pm-12276-breadcrumbing-for-business-features"; - public const string PM18794_ProviderPaymentMethod = "pm-18794-provider-payment-method"; - public const string PM3553_MobileSimpleLoginSelfHostAlias = "simple-login-self-host-alias"; - public const string SetInitialPasswordRefactor = "pm-16117-set-initial-password-refactor"; - public const string ChangeExistingPasswordRefactor = "pm-16117-change-existing-password-refactor"; public static List GetAllKeys() { From f60db791cc8d7b1efdd830362b85e9a137290de0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Mon, 31 Mar 2025 12:25:36 +0100 Subject: [PATCH 3/5] [PM-19590] Add k6 load testing script for SyncController's /sync endpoint (#5508) * Add k6 load testing script for sync endpoint * Refactor sync response validation to use lowercase keys * Remove access token validation from sync.js * Update http_req_duration threshold in sync.js from 400ms to 1200ms --- perf/load/sync.js | 90 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 perf/load/sync.js diff --git a/perf/load/sync.js b/perf/load/sync.js new file mode 100644 index 0000000000..5624803e84 --- /dev/null +++ b/perf/load/sync.js @@ -0,0 +1,90 @@ +import http from "k6/http"; +import { check, fail } from "k6"; +import { authenticate } from "./helpers/auth.js"; + +const IDENTITY_URL = __ENV.IDENTITY_URL; +const API_URL = __ENV.API_URL; +const CLIENT_ID = __ENV.CLIENT_ID; +const AUTH_USERNAME = __ENV.AUTH_USER_EMAIL; +const AUTH_PASSWORD = __ENV.AUTH_USER_PASSWORD_HASH; + +export const options = { + ext: { + loadimpact: { + projectID: 3639465, + name: "Sync", + }, + }, + scenarios: { + constant_load: { + executor: "constant-arrival-rate", + rate: 30, + timeUnit: "1m", // 0.5 requests / second + duration: "10m", + preAllocatedVUs: 5, + }, + ramping_load: { + executor: "ramping-arrival-rate", + startRate: 30, + timeUnit: "1m", // 0.5 requests / second to start + stages: [ + { duration: "30s", target: 30 }, + { duration: "2m", target: 75 }, + { duration: "1m", target: 60 }, + { duration: "2m", target: 100 }, + { duration: "2m", target: 90 }, + { duration: "1m", target: 120 }, + { duration: "30s", target: 150 }, + { duration: "30s", target: 60 }, + { duration: "30s", target: 0 }, + ], + preAllocatedVUs: 20, + }, + }, + thresholds: { + http_req_failed: ["rate<0.01"], + http_req_duration: ["p(95)<1200"], + }, +}; + +export function setup() { + return authenticate(IDENTITY_URL, CLIENT_ID, AUTH_USERNAME, AUTH_PASSWORD); +} + +export default function (data) { + const params = { + headers: { + Accept: "application/json", + "Content-Type": "application/json", + Authorization: `Bearer ${data.access_token}`, + "X-ClientId": CLIENT_ID, + }, + tags: { name: "Sync" }, + }; + + const excludeDomains = Math.random() > 0.5; + + const syncRes = http.get(`${API_URL}/sync?excludeDomains=${excludeDomains}`, params); + if ( + !check(syncRes, { + "sync status is 200": (r) => r.status === 200, + }) + ) { + console.error(`Sync failed with status ${syncRes.status}: ${syncRes.body}`); + fail("sync status code was *not* 200"); + } + + if (syncRes.status === 200) { + const syncJson = syncRes.json(); + + check(syncJson, { + "sync response has profile": (j) => j.profile !== undefined, + "sync response has folders": (j) => Array.isArray(j.folders), + "sync response has collections": (j) => Array.isArray(j.collections), + "sync response has ciphers": (j) => Array.isArray(j.ciphers), + "sync response has policies": (j) => Array.isArray(j.policies), + "sync response has sends": (j) => Array.isArray(j.sends), + "sync response has correct object type": (j) => j.object === "sync" + }); + } +} From 887332b4366263abac08f72fe54f47270f8f1dc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Mon, 31 Mar 2025 15:19:55 +0200 Subject: [PATCH 4/5] [PM-15127] Remove secrets requirement from build workflow (#5546) * [PM-15127] Remove secrets requirement from build workflow * Remove unneeded check, fix target workflow * Remove IF --- .github/CODEOWNERS | 1 + .github/workflows/build.yml | 52 +++++++++++++++++------------- .github/workflows/build_target.yml | 21 ++++++++++++ 3 files changed, 52 insertions(+), 22 deletions(-) create mode 100644 .github/workflows/build_target.yml diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 11e79590f2..aa868cd1b5 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -66,6 +66,7 @@ src/Admin/Views/Tools @bitwarden/team-billing-dev # Platform team .github/workflows/build.yml @bitwarden/team-platform-dev +.github/workflows/build_target.yml @bitwarden/team-platform-dev .github/workflows/cleanup-after-pr.yml @bitwarden/team-platform-dev .github/workflows/cleanup-rc-branch.yml @bitwarden/team-platform-dev .github/workflows/repository-management.yml @bitwarden/team-platform-dev diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8f125b7811..f0df238b34 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -7,22 +7,18 @@ on: - "main" - "rc" - "hotfix-rc" - pull_request_target: + pull_request: types: [opened, synchronize] + workflow_call: + inputs: {} env: _AZ_REGISTRY: "bitwardenprod.azurecr.io" jobs: - check-run: - name: Check PR run - uses: bitwarden/gh-actions/.github/workflows/check-run.yml@main - lint: name: Lint runs-on: ubuntu-22.04 - needs: - - check-run steps: - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 @@ -40,6 +36,8 @@ jobs: runs-on: ubuntu-22.04 needs: - lint + outputs: + has_secrets: ${{ steps.check-secrets.outputs.has_secrets }} strategy: fail-fast: false matrix: @@ -75,6 +73,14 @@ jobs: base_path: ./bitwarden_license/src node: true steps: + - name: Check secrets + id: check-secrets + env: + AZURE_KV_CI_SERVICE_PRINCIPAL: ${{ secrets.AZURE_KV_CI_SERVICE_PRINCIPAL }} + run: | + has_secrets=${{ secrets.AZURE_KV_CI_SERVICE_PRINCIPAL != '' }} + echo "has_secrets=$has_secrets" >> $GITHUB_OUTPUT + - name: Check out repo uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: @@ -134,6 +140,7 @@ jobs: id-token: write needs: - build-artifacts + if: ${{ needs.build-artifacts.outputs.has_secrets == 'true' }} strategy: fail-fast: false matrix: @@ -227,7 +234,7 @@ jobs: - name: Generate Docker image tag id: tag run: | - if [[ "${GITHUB_EVENT_NAME}" == "pull_request_target" ]]; then + if [[ "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then IMAGE_TAG=$(echo "${GITHUB_HEAD_REF}" | sed "s#/#-#g") else IMAGE_TAG=$(echo "${GITHUB_REF:11}" | sed "s#/#-#g") @@ -289,11 +296,11 @@ jobs: "GH_PAT=${{ steps.retrieve-secret-pat.outputs.github-pat-bitwarden-devops-bot-repo-scope }}" - name: Install Cosign - if: github.event_name != 'pull_request_target' && github.ref == 'refs/heads/main' + if: github.event_name != 'pull_request' && github.ref == 'refs/heads/main' uses: sigstore/cosign-installer@dc72c7d5c4d10cd6bcb8cf6e3fd625a9e5e537da # v3.7.0 - name: Sign image with Cosign - if: github.event_name != 'pull_request_target' && github.ref == 'refs/heads/main' + if: github.event_name != 'pull_request' && github.ref == 'refs/heads/main' env: DIGEST: ${{ steps.build-docker.outputs.digest }} TAGS: ${{ steps.image-tags.outputs.tags }} @@ -343,7 +350,7 @@ jobs: - name: Make Docker stubs if: | - github.event_name != 'pull_request_target' + github.event_name != 'pull_request' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') run: | # Set proper setup image based on branch @@ -385,7 +392,7 @@ jobs: - name: Make Docker stub checksums if: | - github.event_name != 'pull_request_target' + github.event_name != 'pull_request' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') run: | sha256sum docker-stub-US.zip > docker-stub-US-sha256.txt @@ -393,7 +400,7 @@ jobs: - name: Upload Docker stub US artifact if: | - github.event_name != 'pull_request_target' + github.event_name != 'pull_request' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0 with: @@ -403,7 +410,7 @@ jobs: - name: Upload Docker stub EU artifact if: | - github.event_name != 'pull_request_target' + github.event_name != 'pull_request' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0 with: @@ -413,7 +420,7 @@ jobs: - name: Upload Docker stub US checksum artifact if: | - github.event_name != 'pull_request_target' + github.event_name != 'pull_request' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0 with: @@ -423,7 +430,7 @@ jobs: - name: Upload Docker stub EU checksum artifact if: | - github.event_name != 'pull_request_target' + github.event_name != 'pull_request' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0 with: @@ -552,7 +559,7 @@ jobs: self-host-build: name: Trigger self-host build if: | - github.event_name != 'pull_request_target' + github.event_name != 'pull_request' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') runs-on: ubuntu-22.04 needs: @@ -587,7 +594,7 @@ jobs: trigger-k8s-deploy: name: Trigger k8s deploy - if: github.event_name != 'pull_request_target' && github.ref == 'refs/heads/main' + if: github.event_name != 'pull_request' && github.ref == 'refs/heads/main' runs-on: ubuntu-22.04 needs: - build-docker @@ -623,7 +630,8 @@ jobs: trigger-ee-updates: name: Trigger Ephemeral Environment updates if: | - github.event_name == 'pull_request_target' + needs.build-artifacts.outputs.has_secrets == 'true' + && github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'ephemeral-environment') runs-on: ubuntu-24.04 needs: @@ -660,7 +668,8 @@ jobs: name: Trigger Ephemeral Environment Sync needs: trigger-ee-updates if: | - github.event_name == 'pull_request_target' + needs.build-artifacts.outputs.has_secrets == 'true' + && github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'ephemeral-environment') uses: bitwarden/gh-actions/.github/workflows/_ephemeral_environment_manager.yml@main with: @@ -670,7 +679,6 @@ jobs: pull_request_number: ${{ github.event.number }} secrets: inherit - check-failures: name: Check for failures if: always() @@ -686,7 +694,7 @@ jobs: steps: - name: Check if any job failed if: | - github.event_name != 'pull_request_target' + github.event_name != 'pull_request' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/rc' || github.ref == 'refs/heads/hotfix-rc') && contains(needs.*.result, 'failure') run: exit 1 diff --git a/.github/workflows/build_target.yml b/.github/workflows/build_target.yml new file mode 100644 index 0000000000..313446c949 --- /dev/null +++ b/.github/workflows/build_target.yml @@ -0,0 +1,21 @@ +name: Build on PR Target + +on: + pull_request_target: + types: [opened, synchronize] + +defaults: + run: + shell: bash + +jobs: + check-run: + name: Check PR run + uses: bitwarden/gh-actions/.github/workflows/check-run.yml@main + + run-workflow: + name: Run Build on PR Target + needs: check-run + if: ${{ github.event.pull_request.head.repo.full_name != github.repository }} + uses: ./.github/workflows/build.yml + secrets: inherit From 786b0edcebd19c08992194e46886d5c37b8a85f9 Mon Sep 17 00:00:00 2001 From: Jared McCannon Date: Mon, 31 Mar 2025 08:33:57 -0500 Subject: [PATCH 5/5] [PM-18527] - Fix allowing restored user to own multiple free orgs (#5444) * Moved RestoreUserAsync and RestoreUsersAsync to Command. * Fixing the bug. * Added test for bulk method. * Fixing sonar cube warning. * SonarQube warning fix. * Excluding org users we already have. * Fixed misspelling. Added integration test for method. * test had the misspelling as well :facepalm: * Split out interface. Added admin and confirmed constraints. * fixed queries and added xml comments and tests. --- .../Scim/Controllers/v2/UsersController.cs | 9 +- .../src/Scim/Users/PatchUserCommand.cs | 8 +- .../Scim.Test/Users/PatchUserCommandTests.cs | 7 +- .../OrganizationUsersController.cs | 10 +- .../v1/IRestoreOrganizationUserCommand.cs | 54 ++ .../v1/RestoreOrganizationUserCommand.cs | 295 ++++++++ .../Repositories/IOrganizationRepository.cs | 1 + .../Services/IOrganizationService.cs | 4 - .../Implementations/OrganizationService.cs | 144 +--- ...OrganizationServiceCollectionExtensions.cs | 3 + .../Repositories/OrganizationRepository.cs | 11 + .../Repositories/OrganizationRepository.cs | 13 + .../Organization_ReadManyByIds.sql | 67 ++ .../RestoreOrganizationUserCommandTests.cs | 693 ++++++++++++++++++ .../Services/OrganizationServiceTests.cs | 551 -------------- .../OrganizationRepositoryTests.cs | 39 + .../OrganizationRepositoryTests.cs | 33 + .../2025-03-21_00_Org_ReadManyByManyId.sql | 66 ++ 18 files changed, 1298 insertions(+), 710 deletions(-) create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/IRestoreOrganizationUserCommand.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs create mode 100644 src/Sql/dbo/Stored Procedures/Organization_ReadManyByIds.sql create mode 100644 test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs create mode 100644 util/Migrator/DbScripts/2025-03-21_00_Org_ReadManyByManyId.sql diff --git a/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs b/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs index 1323205b96..77bc62e952 100644 --- a/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs +++ b/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs @@ -1,4 +1,5 @@ using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; @@ -23,7 +24,7 @@ public class UsersController : Controller private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; private readonly IPatchUserCommand _patchUserCommand; private readonly IPostUserCommand _postUserCommand; - private readonly ILogger _logger; + private readonly IRestoreOrganizationUserCommand _restoreOrganizationUserCommand; public UsersController( IOrganizationUserRepository organizationUserRepository, @@ -32,7 +33,7 @@ public class UsersController : Controller IRemoveOrganizationUserCommand removeOrganizationUserCommand, IPatchUserCommand patchUserCommand, IPostUserCommand postUserCommand, - ILogger logger) + IRestoreOrganizationUserCommand restoreOrganizationUserCommand) { _organizationUserRepository = organizationUserRepository; _organizationService = organizationService; @@ -40,7 +41,7 @@ public class UsersController : Controller _removeOrganizationUserCommand = removeOrganizationUserCommand; _patchUserCommand = patchUserCommand; _postUserCommand = postUserCommand; - _logger = logger; + _restoreOrganizationUserCommand = restoreOrganizationUserCommand; } [HttpGet("{id}")] @@ -93,7 +94,7 @@ public class UsersController : Controller if (model.Active && orgUser.Status == OrganizationUserStatusType.Revoked) { - await _organizationService.RestoreUserAsync(orgUser, EventSystemUser.SCIM); + await _restoreOrganizationUserCommand.RestoreUserAsync(orgUser, EventSystemUser.SCIM); } else if (!model.Active && orgUser.Status != OrganizationUserStatusType.Revoked) { diff --git a/bitwarden_license/src/Scim/Users/PatchUserCommand.cs b/bitwarden_license/src/Scim/Users/PatchUserCommand.cs index f4445354ce..3d7082aacc 100644 --- a/bitwarden_license/src/Scim/Users/PatchUserCommand.cs +++ b/bitwarden_license/src/Scim/Users/PatchUserCommand.cs @@ -1,4 +1,5 @@ -using Bit.Core.Enums; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1; +using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; @@ -11,15 +12,18 @@ public class PatchUserCommand : IPatchUserCommand { private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IOrganizationService _organizationService; + private readonly IRestoreOrganizationUserCommand _restoreOrganizationUserCommand; private readonly ILogger _logger; public PatchUserCommand( IOrganizationUserRepository organizationUserRepository, IOrganizationService organizationService, + IRestoreOrganizationUserCommand restoreOrganizationUserCommand, ILogger logger) { _organizationUserRepository = organizationUserRepository; _organizationService = organizationService; + _restoreOrganizationUserCommand = restoreOrganizationUserCommand; _logger = logger; } @@ -71,7 +75,7 @@ public class PatchUserCommand : IPatchUserCommand { if (active && orgUser.Status == OrganizationUserStatusType.Revoked) { - await _organizationService.RestoreUserAsync(orgUser, EventSystemUser.SCIM); + await _restoreOrganizationUserCommand.RestoreUserAsync(orgUser, EventSystemUser.SCIM); return true; } else if (!active && orgUser.Status != OrganizationUserStatusType.Revoked) diff --git a/bitwarden_license/test/Scim.Test/Users/PatchUserCommandTests.cs b/bitwarden_license/test/Scim.Test/Users/PatchUserCommandTests.cs index 6e9c985b88..44a43d16b7 100644 --- a/bitwarden_license/test/Scim.Test/Users/PatchUserCommandTests.cs +++ b/bitwarden_license/test/Scim.Test/Users/PatchUserCommandTests.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -43,7 +44,7 @@ public class PatchUserCommandTests await sutProvider.Sut.PatchUserAsync(organizationUser.OrganizationId, organizationUser.Id, scimPatchModel); - await sutProvider.GetDependency().Received(1).RestoreUserAsync(organizationUser, EventSystemUser.SCIM); + await sutProvider.GetDependency().Received(1).RestoreUserAsync(organizationUser, EventSystemUser.SCIM); } [Theory] @@ -71,7 +72,7 @@ public class PatchUserCommandTests await sutProvider.Sut.PatchUserAsync(organizationUser.OrganizationId, organizationUser.Id, scimPatchModel); - await sutProvider.GetDependency().Received(1).RestoreUserAsync(organizationUser, EventSystemUser.SCIM); + await sutProvider.GetDependency().Received(1).RestoreUserAsync(organizationUser, EventSystemUser.SCIM); } [Theory] @@ -147,7 +148,7 @@ public class PatchUserCommandTests await sutProvider.Sut.PatchUserAsync(organizationUser.OrganizationId, organizationUser.Id, scimPatchModel); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RestoreUserAsync(default, EventSystemUser.SCIM); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RestoreUserAsync(default, EventSystemUser.SCIM); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RevokeUserAsync(default, EventSystemUser.SCIM); } diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index cfe93e87ce..5fd9109077 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -8,6 +8,7 @@ using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.OrganizationFeatures.Shared.Authorization; @@ -61,6 +62,7 @@ public class OrganizationUsersController : Controller private readonly IFeatureService _featureService; private readonly IPricingClient _pricingClient; private readonly IConfirmOrganizationUserCommand _confirmOrganizationUserCommand; + private readonly IRestoreOrganizationUserCommand _restoreOrganizationUserCommand; public OrganizationUsersController( IOrganizationRepository organizationRepository, @@ -86,7 +88,8 @@ public class OrganizationUsersController : Controller IPolicyRequirementQuery policyRequirementQuery, IFeatureService featureService, IPricingClient pricingClient, - IConfirmOrganizationUserCommand confirmOrganizationUserCommand) + IConfirmOrganizationUserCommand confirmOrganizationUserCommand, + IRestoreOrganizationUserCommand restoreOrganizationUserCommand) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; @@ -112,6 +115,7 @@ public class OrganizationUsersController : Controller _featureService = featureService; _pricingClient = pricingClient; _confirmOrganizationUserCommand = confirmOrganizationUserCommand; + _restoreOrganizationUserCommand = restoreOrganizationUserCommand; } [HttpGet("{id}")] @@ -630,14 +634,14 @@ public class OrganizationUsersController : Controller [HttpPut("{id}/restore")] public async Task RestoreAsync(Guid orgId, Guid id) { - await RestoreOrRevokeUserAsync(orgId, id, (orgUser, userId) => _organizationService.RestoreUserAsync(orgUser, userId)); + await RestoreOrRevokeUserAsync(orgId, id, (orgUser, userId) => _restoreOrganizationUserCommand.RestoreUserAsync(orgUser, userId)); } [HttpPatch("restore")] [HttpPut("restore")] public async Task> BulkRestoreAsync(Guid orgId, [FromBody] OrganizationUserBulkRequestModel model) { - return await RestoreOrRevokeUsersAsync(orgId, model, (orgId, orgUserIds, restoringUserId) => _organizationService.RestoreUsersAsync(orgId, orgUserIds, restoringUserId, _userService)); + return await RestoreOrRevokeUsersAsync(orgId, model, (orgId, orgUserIds, restoringUserId) => _restoreOrganizationUserCommand.RestoreUsersAsync(orgId, orgUserIds, restoringUserId, _userService)); } [HttpPatch("enable-secrets-manager")] diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/IRestoreOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/IRestoreOrganizationUserCommand.cs new file mode 100644 index 0000000000..e5e5bfb482 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/IRestoreOrganizationUserCommand.cs @@ -0,0 +1,54 @@ +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Services; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1; + +/// +/// Restores a user back to their previous status. +/// +public interface IRestoreOrganizationUserCommand +{ + /// + /// Validates that the requesting user can perform the action. There is also a check done to ensure the organization + /// can re-add this user based on their current occupied seats. + /// + /// Checks are performed to make sure the user is conforming to all policies enforced by the organization as well as + /// other organizations the user may belong to. + /// + /// Reference Events and Push Notifications are fired off for this as well. + /// + /// Revoked user to be restored. + /// UserId of the user performing the action. + Task RestoreUserAsync(OrganizationUser organizationUser, Guid? restoringUserId); + + /// + /// Validates that the requesting user can perform the action. There is also a check done to ensure the organization + /// can re-add this user based on their current occupied seats. + /// + /// Checks are performed to make sure the user is conforming to all policies enforced by the organization as well as + /// other organizations the user may belong to. + /// + /// Reference Events and Push Notifications are fired off for this as well. + /// + /// Revoked user to be restored. + /// System that is performing the action on behalf of the organization (Public API, SCIM, etc.) + Task RestoreUserAsync(OrganizationUser organizationUser, EventSystemUser systemUser); + + /// + /// Validates that the requesting user can perform the action. There is also a check done to ensure the organization + /// can re-add this user based on their current occupied seats. + /// + /// Checks are performed to make sure the user is conforming to all policies enforced by the organization as well as + /// other organizations the user may belong to. + /// + /// Reference Events and Push Notifications are fired off for this as well. + /// + /// Organization the users should be restored to. + /// List of organization user ids to restore to previous status. + /// UserId of the user performing the action. + /// Passed in from caller to avoid circular dependency + /// List of organization user Ids and strings. A successful restoration will have an empty string. + /// If an error occurs, the error message will be provided. + Task>> RestoreUsersAsync(Guid organizationId, IEnumerable organizationUserIds, Guid? restoringUserId, IUserService userService); +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs new file mode 100644 index 0000000000..3d4b0fba5c --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs @@ -0,0 +1,295 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Services; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; +using Bit.Core.Billing.Enums; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Platform.Push; +using Bit.Core.Repositories; +using Bit.Core.Services; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1; + +public class RestoreOrganizationUserCommand( + ICurrentContext currentContext, + IEventService eventService, + IFeatureService featureService, + IPushNotificationService pushNotificationService, + IOrganizationUserRepository organizationUserRepository, + IOrganizationRepository organizationRepository, + ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, + IPolicyService policyService, + IUserRepository userRepository, + IOrganizationService organizationService) : IRestoreOrganizationUserCommand +{ + public async Task RestoreUserAsync(OrganizationUser organizationUser, Guid? restoringUserId) + { + if (restoringUserId.HasValue && organizationUser.UserId == restoringUserId.Value) + { + throw new BadRequestException("You cannot restore yourself."); + } + + if (organizationUser.Type == OrganizationUserType.Owner && restoringUserId.HasValue && + !await currentContext.OrganizationOwner(organizationUser.OrganizationId)) + { + throw new BadRequestException("Only owners can restore other owners."); + } + + await RepositoryRestoreUserAsync(organizationUser); + await eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); + + if (featureService.IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) && + organizationUser.UserId.HasValue) + { + await pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value); + } + } + + public async Task RestoreUserAsync(OrganizationUser organizationUser, EventSystemUser systemUser) + { + await RepositoryRestoreUserAsync(organizationUser); + await eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored, + systemUser); + + if (featureService.IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) && + organizationUser.UserId.HasValue) + { + await pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value); + } + } + + private async Task RepositoryRestoreUserAsync(OrganizationUser organizationUser) + { + if (organizationUser.Status != OrganizationUserStatusType.Revoked) + { + throw new BadRequestException("Already active."); + } + + var organization = await organizationRepository.GetByIdAsync(organizationUser.OrganizationId); + var occupiedSeats = await organizationUserRepository.GetOccupiedSeatCountByOrganizationIdAsync(organization.Id); + var availableSeats = organization.Seats.GetValueOrDefault(0) - occupiedSeats; + + if (availableSeats < 1) + { + await organizationService.AutoAddSeatsAsync(organization, 1); // Hooray + } + + var userTwoFactorIsEnabled = false; + // Only check 2FA status if the user is linked to a user account + if (organizationUser.UserId.HasValue) + { + userTwoFactorIsEnabled = + (await twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync([organizationUser.UserId.Value])) + .FirstOrDefault() + .twoFactorIsEnabled; + } + + await CheckUserForOtherFreeOrganizationOwnershipAsync(organizationUser); + + await CheckPoliciesBeforeRestoreAsync(organizationUser, userTwoFactorIsEnabled); + + var status = OrganizationService.GetPriorActiveOrganizationUserStatusType(organizationUser); + + await organizationUserRepository.RestoreAsync(organizationUser.Id, status); + + organizationUser.Status = status; + } + + private async Task CheckUserForOtherFreeOrganizationOwnershipAsync(OrganizationUser organizationUser) + { + var relatedOrgUsersFromOtherOrgs = await organizationUserRepository.GetManyByUserAsync(organizationUser.UserId.Value); + var otherOrgs = await organizationRepository.GetManyByUserIdAsync(organizationUser.UserId.Value); + + var orgOrgUserDict = relatedOrgUsersFromOtherOrgs + .Where(x => x.Id != organizationUser.Id) + .ToDictionary(x => x, x => otherOrgs.FirstOrDefault(y => y.Id == x.OrganizationId)); + + CheckForOtherFreeOrganizationOwnership(organizationUser, orgOrgUserDict); + } + + private async Task> GetRelatedOrganizationUsersAndOrganizations( + IEnumerable organizationUsers) + { + var allUserIds = organizationUsers.Select(x => x.UserId.Value); + + var otherOrganizationUsers = (await organizationUserRepository.GetManyByManyUsersAsync(allUserIds)) + .Where(x => organizationUsers.Any(y => y.Id == x.Id) == false); + + var otherOrgs = await organizationRepository.GetManyByIdsAsync(otherOrganizationUsers + .Select(x => x.OrganizationId) + .Distinct()); + + return otherOrganizationUsers + .ToDictionary(x => x, x => otherOrgs.FirstOrDefault(y => y.Id == x.OrganizationId)); + } + + private static void CheckForOtherFreeOrganizationOwnership(OrganizationUser organizationUser, + Dictionary otherOrgUsersAndOrgs) + { + var ownerOrAdminList = new[] { OrganizationUserType.Owner, OrganizationUserType.Admin }; + if (otherOrgUsersAndOrgs.Any(x => + x.Key.UserId == organizationUser.UserId && + ownerOrAdminList.Any(userType => userType == x.Key.Type) && + x.Key.Status == OrganizationUserStatusType.Confirmed && + x.Value.PlanType == PlanType.Free)) + { + throw new BadRequestException( + "User is an owner/admin of another free organization. Please have them upgrade to a paid plan to restore their account."); + } + } + + public async Task>> RestoreUsersAsync(Guid organizationId, + IEnumerable organizationUserIds, Guid? restoringUserId, IUserService userService) + { + var orgUsers = await organizationUserRepository.GetManyAsync(organizationUserIds); + var filteredUsers = orgUsers.Where(u => u.OrganizationId == organizationId) + .ToList(); + + if (filteredUsers.Count == 0) + { + throw new BadRequestException("Users invalid."); + } + + var organization = await organizationRepository.GetByIdAsync(organizationId); + var occupiedSeats = await organizationUserRepository.GetOccupiedSeatCountByOrganizationIdAsync(organization.Id); + var availableSeats = organization.Seats.GetValueOrDefault(0) - occupiedSeats; + var newSeatsRequired = organizationUserIds.Count() - availableSeats; + await organizationService.AutoAddSeatsAsync(organization, newSeatsRequired); + + var deletingUserIsOwner = false; + if (restoringUserId.HasValue) + { + deletingUserIsOwner = await currentContext.OrganizationOwner(organizationId); + } + + // Query Two Factor Authentication status for all users in the organization + // This is an optimization to avoid querying the Two Factor Authentication status for each user individually + var organizationUsersTwoFactorEnabled = await twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync( + filteredUsers.Where(ou => ou.UserId.HasValue).Select(ou => ou.UserId.Value)); + + var orgUsersAndOrgs = await GetRelatedOrganizationUsersAndOrganizations(filteredUsers); + + var result = new List>(); + + foreach (var organizationUser in filteredUsers) + { + try + { + if (organizationUser.Status != OrganizationUserStatusType.Revoked) + { + throw new BadRequestException("Already active."); + } + + if (restoringUserId.HasValue && organizationUser.UserId == restoringUserId) + { + throw new BadRequestException("You cannot restore yourself."); + } + + if (organizationUser.Type == OrganizationUserType.Owner && restoringUserId.HasValue && + !deletingUserIsOwner) + { + throw new BadRequestException("Only owners can restore other owners."); + } + + var twoFactorIsEnabled = organizationUser.UserId.HasValue + && organizationUsersTwoFactorEnabled + .FirstOrDefault(ou => ou.userId == organizationUser.UserId.Value) + .twoFactorIsEnabled; + + await CheckPoliciesBeforeRestoreAsync(organizationUser, twoFactorIsEnabled); + + CheckForOtherFreeOrganizationOwnership(organizationUser, orgUsersAndOrgs); + + var status = OrganizationService.GetPriorActiveOrganizationUserStatusType(organizationUser); + + await organizationUserRepository.RestoreAsync(organizationUser.Id, status); + organizationUser.Status = status; + await eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); + if (featureService.IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) && + organizationUser.UserId.HasValue) + { + await pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value); + } + + result.Add(Tuple.Create(organizationUser, "")); + } + catch (BadRequestException e) + { + result.Add(Tuple.Create(organizationUser, e.Message)); + } + } + + return result; + } + + private async Task CheckPoliciesBeforeRestoreAsync(OrganizationUser orgUser, bool userHasTwoFactorEnabled) + { + // An invited OrganizationUser isn't linked with a user account yet, so these checks are irrelevant + // The user will be subject to the same checks when they try to accept the invite + if (OrganizationService.GetPriorActiveOrganizationUserStatusType(orgUser) == OrganizationUserStatusType.Invited) + { + return; + } + + var userId = orgUser.UserId.Value; + + // Enforce Single Organization Policy of organization user is being restored to + var allOrgUsers = await organizationUserRepository.GetManyByUserAsync(userId); + var hasOtherOrgs = allOrgUsers.Any(ou => ou.OrganizationId != orgUser.OrganizationId); + var singleOrgPoliciesApplyingToRevokedUsers = await policyService.GetPoliciesApplicableToUserAsync(userId, + PolicyType.SingleOrg, OrganizationUserStatusType.Revoked); + var singleOrgPolicyApplies = + singleOrgPoliciesApplyingToRevokedUsers.Any(p => p.OrganizationId == orgUser.OrganizationId); + + var singleOrgCompliant = true; + var belongsToOtherOrgCompliant = true; + var twoFactorCompliant = true; + + if (hasOtherOrgs && singleOrgPolicyApplies) + { + singleOrgCompliant = false; + } + + // Enforce Single Organization Policy of other organizations user is a member of + var anySingleOrgPolicies = await policyService.AnyPoliciesApplicableToUserAsync(userId, PolicyType.SingleOrg); + if (anySingleOrgPolicies) + { + belongsToOtherOrgCompliant = false; + } + + // Enforce 2FA Policy of organization user is trying to join + if (!userHasTwoFactorEnabled) + { + var invitedTwoFactorPolicies = await policyService.GetPoliciesApplicableToUserAsync(userId, + PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Revoked); + if (invitedTwoFactorPolicies.Any(p => p.OrganizationId == orgUser.OrganizationId)) + { + twoFactorCompliant = false; + } + } + + var user = await userRepository.GetByIdAsync(userId); + + if (!singleOrgCompliant && !twoFactorCompliant) + { + throw new BadRequestException(user.Email + + " is not compliant with the single organization and two-step login policy"); + } + else if (!singleOrgCompliant) + { + throw new BadRequestException(user.Email + " is not compliant with the single organization policy"); + } + else if (!belongsToOtherOrgCompliant) + { + throw new BadRequestException(user.Email + + " belongs to an organization that doesn't allow them to join multiple organizations"); + } + else if (!twoFactorCompliant) + { + throw new BadRequestException(user.Email + " is not compliant with the two-step login policy"); + } + } +} diff --git a/src/Core/AdminConsole/Repositories/IOrganizationRepository.cs b/src/Core/AdminConsole/Repositories/IOrganizationRepository.cs index 584d95ffe2..7e315ed58b 100644 --- a/src/Core/AdminConsole/Repositories/IOrganizationRepository.cs +++ b/src/Core/AdminConsole/Repositories/IOrganizationRepository.cs @@ -24,4 +24,5 @@ public interface IOrganizationRepository : IRepository /// Task> GetByVerifiedUserEmailDomainAsync(Guid userId); Task> GetAddableToProviderByUserIdAsync(Guid userId, ProviderType providerType); + Task> GetManyByIdsAsync(IEnumerable ids); } diff --git a/src/Core/AdminConsole/Services/IOrganizationService.cs b/src/Core/AdminConsole/Services/IOrganizationService.cs index 476fccb480..e0088f1f74 100644 --- a/src/Core/AdminConsole/Services/IOrganizationService.cs +++ b/src/Core/AdminConsole/Services/IOrganizationService.cs @@ -48,10 +48,6 @@ public interface IOrganizationService Task RevokeUserAsync(OrganizationUser organizationUser, EventSystemUser systemUser); Task>> RevokeUsersAsync(Guid organizationId, IEnumerable organizationUserIds, Guid? revokingUserId); - Task RestoreUserAsync(OrganizationUser organizationUser, Guid? restoringUserId); - Task RestoreUserAsync(OrganizationUser organizationUser, EventSystemUser systemUser); - Task>> RestoreUsersAsync(Guid organizationId, - IEnumerable organizationUserIds, Guid? restoringUserId, IUserService userService); Task CreatePendingOrganization(Organization organization, string ownerEmail, ClaimsPrincipal user, IUserService userService, bool salesAssistedTrialStarted); /// /// Update an Organization entry by setting the public/private keys, set it as 'Enabled' and move the Status from 'Pending' to 'Created'. diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index ab5703eaa1..64bd434327 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -19,7 +19,6 @@ using Bit.Core.Billing.Constants; using Bit.Core.Billing.Enums; using Bit.Core.Billing.Extensions; using Bit.Core.Billing.Pricing; -using Bit.Core.Billing.Services; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; @@ -75,7 +74,6 @@ public class OrganizationService : IOrganizationService private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory; private readonly IFeatureService _featureService; private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; - private readonly IOrganizationBillingService _organizationBillingService; private readonly IHasConfirmedOwnersExceptQuery _hasConfirmedOwnersExceptQuery; private readonly IPricingClient _pricingClient; private readonly IPolicyRequirementQuery _policyRequirementQuery; @@ -112,7 +110,6 @@ public class OrganizationService : IOrganizationService IProviderRepository providerRepository, IFeatureService featureService, ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, - IOrganizationBillingService organizationBillingService, IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery, IPricingClient pricingClient, IPolicyRequirementQuery policyRequirementQuery) @@ -148,7 +145,6 @@ public class OrganizationService : IOrganizationService _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory; _featureService = featureService; _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; - _organizationBillingService = organizationBillingService; _hasConfirmedOwnersExceptQuery = hasConfirmedOwnersExceptQuery; _pricingClient = pricingClient; _policyRequirementQuery = policyRequirementQuery; @@ -1891,144 +1887,6 @@ public class OrganizationService : IOrganizationService return result; } - public async Task RestoreUserAsync(OrganizationUser organizationUser, Guid? restoringUserId) - { - if (restoringUserId.HasValue && organizationUser.UserId == restoringUserId.Value) - { - throw new BadRequestException("You cannot restore yourself."); - } - - if (organizationUser.Type == OrganizationUserType.Owner && restoringUserId.HasValue && - !await _currentContext.OrganizationOwner(organizationUser.OrganizationId)) - { - throw new BadRequestException("Only owners can restore other owners."); - } - - await RepositoryRestoreUserAsync(organizationUser); - await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); - - if (_featureService.IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) && organizationUser.UserId.HasValue) - { - await _pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value); - } - } - - public async Task RestoreUserAsync(OrganizationUser organizationUser, EventSystemUser systemUser) - { - await RepositoryRestoreUserAsync(organizationUser); - await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored, systemUser); - - if (_featureService.IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) && organizationUser.UserId.HasValue) - { - await _pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value); - } - } - - private async Task RepositoryRestoreUserAsync(OrganizationUser organizationUser) - { - if (organizationUser.Status != OrganizationUserStatusType.Revoked) - { - throw new BadRequestException("Already active."); - } - - var organization = await _organizationRepository.GetByIdAsync(organizationUser.OrganizationId); - var occupiedSeats = await _organizationUserRepository.GetOccupiedSeatCountByOrganizationIdAsync(organization.Id); - var availableSeats = organization.Seats.GetValueOrDefault(0) - occupiedSeats; - if (availableSeats < 1) - { - await AutoAddSeatsAsync(organization, 1); - } - - var userTwoFactorIsEnabled = false; - // Only check Two Factor Authentication status if the user is linked to a user account - if (organizationUser.UserId.HasValue) - { - userTwoFactorIsEnabled = (await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(new[] { organizationUser.UserId.Value })).FirstOrDefault().twoFactorIsEnabled; - } - - await CheckPoliciesBeforeRestoreAsync(organizationUser, userTwoFactorIsEnabled); - - var status = GetPriorActiveOrganizationUserStatusType(organizationUser); - - await _organizationUserRepository.RestoreAsync(organizationUser.Id, status); - organizationUser.Status = status; - } - - public async Task>> RestoreUsersAsync(Guid organizationId, - IEnumerable organizationUserIds, Guid? restoringUserId, IUserService userService) - { - var orgUsers = await _organizationUserRepository.GetManyAsync(organizationUserIds); - var filteredUsers = orgUsers.Where(u => u.OrganizationId == organizationId) - .ToList(); - - if (!filteredUsers.Any()) - { - throw new BadRequestException("Users invalid."); - } - - var organization = await _organizationRepository.GetByIdAsync(organizationId); - var occupiedSeats = await _organizationUserRepository.GetOccupiedSeatCountByOrganizationIdAsync(organization.Id); - var availableSeats = organization.Seats.GetValueOrDefault(0) - occupiedSeats; - var newSeatsRequired = organizationUserIds.Count() - availableSeats; - await AutoAddSeatsAsync(organization, newSeatsRequired); - - var deletingUserIsOwner = false; - if (restoringUserId.HasValue) - { - deletingUserIsOwner = await _currentContext.OrganizationOwner(organizationId); - } - - // Query Two Factor Authentication status for all users in the organization - // This is an optimization to avoid querying the Two Factor Authentication status for each user individually - var organizationUsersTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync( - filteredUsers.Where(ou => ou.UserId.HasValue).Select(ou => ou.UserId.Value)); - - var result = new List>(); - - foreach (var organizationUser in filteredUsers) - { - try - { - if (organizationUser.Status != OrganizationUserStatusType.Revoked) - { - throw new BadRequestException("Already active."); - } - - if (restoringUserId.HasValue && organizationUser.UserId == restoringUserId) - { - throw new BadRequestException("You cannot restore yourself."); - } - - if (organizationUser.Type == OrganizationUserType.Owner && restoringUserId.HasValue && !deletingUserIsOwner) - { - throw new BadRequestException("Only owners can restore other owners."); - } - - var twoFactorIsEnabled = organizationUser.UserId.HasValue - && organizationUsersTwoFactorEnabled.FirstOrDefault(ou => ou.userId == organizationUser.UserId.Value).twoFactorIsEnabled; - await CheckPoliciesBeforeRestoreAsync(organizationUser, twoFactorIsEnabled); - - var status = GetPriorActiveOrganizationUserStatusType(organizationUser); - - await _organizationUserRepository.RestoreAsync(organizationUser.Id, status); - organizationUser.Status = status; - await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); - if (_featureService.IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) && organizationUser.UserId.HasValue) - { - await _pushNotificationService.PushSyncOrgKeysAsync(organizationUser.UserId.Value); - } - - result.Add(Tuple.Create(organizationUser, "")); - } - catch (BadRequestException e) - { - result.Add(Tuple.Create(organizationUser, e.Message)); - } - } - - return result; - } - private async Task CheckPoliciesBeforeRestoreAsync(OrganizationUser orgUser, bool userHasTwoFactorEnabled) { // An invited OrganizationUser isn't linked with a user account yet, so these checks are irrelevant @@ -2095,7 +1953,7 @@ public class OrganizationService : IOrganizationService } } - static OrganizationUserStatusType GetPriorActiveOrganizationUserStatusType(OrganizationUser organizationUser) + public static OrganizationUserStatusType GetPriorActiveOrganizationUserStatusType(OrganizationUser organizationUser) { // Determine status to revert back to var status = OrganizationUserStatusType.Invited; diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index e13a06f660..59cfdace65 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -13,6 +13,7 @@ using Bit.Core.AdminConsole.OrganizationFeatures.Organizations.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1; using Bit.Core.Models.Business.Tokenables; using Bit.Core.OrganizationFeatures.OrganizationCollections; using Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces; @@ -168,6 +169,8 @@ public static class OrganizationServiceCollectionExtensions services.AddScoped(); services.AddScoped(); + services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); diff --git a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationRepository.cs b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationRepository.cs index f624f7da28..3da8ad1a6c 100644 --- a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationRepository.cs +++ b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationRepository.cs @@ -196,4 +196,15 @@ public class OrganizationRepository : Repository, IOrganizat return result.ToList(); } } + + public async Task> GetManyByIdsAsync(IEnumerable ids) + { + await using var connection = new SqlConnection(ConnectionString); + + return (await connection.QueryAsync( + $"[{Schema}].[{Table}_ReadManyByIds]", + new { OrganizationIds = ids.ToGuidIdArrayTVP() }, + commandType: CommandType.StoredProcedure)) + .ToList(); + } } diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs index 6fc42b699d..c095b07030 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs @@ -354,6 +354,19 @@ public class OrganizationRepository : Repository> GetManyByIdsAsync(IEnumerable ids) + { + using var scope = ServiceScopeFactory.CreateScope(); + + var dbContext = GetDatabaseContext(scope); + + var query = from organization in dbContext.Organizations + where ids.Contains(organization.Id) + select organization; + + return await query.ToArrayAsync(); + } + public Task EnableCollectionEnhancements(Guid organizationId) { throw new NotImplementedException("Collection enhancements migration is not yet supported for Entity Framework."); diff --git a/src/Sql/dbo/Stored Procedures/Organization_ReadManyByIds.sql b/src/Sql/dbo/Stored Procedures/Organization_ReadManyByIds.sql new file mode 100644 index 0000000000..23f1f578d0 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/Organization_ReadManyByIds.sql @@ -0,0 +1,67 @@ +CREATE PROCEDURE [dbo].[Organization_ReadManyByIds] @OrganizationIds AS [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + + SELECT o.[Id], + o.[Identifier], + o.[Name], + o.[BusinessName], + o.[BusinessAddress1], + o.[BusinessAddress2], + o.[BusinessAddress3], + o.[BusinessCountry], + o.[BusinessTaxNumber], + o.[BillingEmail], + o.[Plan], + o.[PlanType], + o.[Seats], + o.[MaxCollections], + o.[UsePolicies], + o.[UseSso], + o.[UseGroups], + o.[UseDirectory], + o.[UseEvents], + o.[UseTotp], + o.[Use2fa], + o.[UseApi], + o.[UseResetPassword], + o.[SelfHost], + o.[UsersGetPremium], + o.[Storage], + o.[MaxStorageGb], + o.[Gateway], + o.[GatewayCustomerId], + o.[GatewaySubscriptionId], + o.[ReferenceData], + o.[Enabled], + o.[LicenseKey], + o.[PublicKey], + o.[PrivateKey], + o.[TwoFactorProviders], + o.[ExpirationDate], + o.[CreationDate], + o.[RevisionDate], + o.[OwnersNotifiedOfAutoscaling], + o.[MaxAutoscaleSeats], + o.[UseKeyConnector], + o.[UseScim], + o.[UseCustomPermissions], + o.[UseSecretsManager], + o.[Status], + o.[UsePasswordManager], + o.[SmSeats], + o.[SmServiceAccounts], + o.[MaxAutoscaleSmSeats], + o.[MaxAutoscaleSmServiceAccounts], + o.[SecretsManagerBeta], + o.[LimitCollectionCreation], + o.[LimitCollectionDeletion], + o.[LimitItemDeletion], + o.[AllowAdminAccessToAllCollectionItems], + o.[UseRiskInsights] + FROM [dbo].[OrganizationView] o + INNER JOIN @OrganizationIds ids ON o.[Id] = ids.[Id] + +END + diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs new file mode 100644 index 0000000000..726664849d --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/RestoreOrganizationUserCommandTests.cs @@ -0,0 +1,693 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1; +using Bit.Core.AdminConsole.Services; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; +using Bit.Core.Billing.Enums; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Models.Data.Organizations.OrganizationUsers; +using Bit.Core.Platform.Push; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Test.AutoFixture.OrganizationUserFixtures; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser; + +[SutProviderCustomize] +public class RestoreOrganizationUserCommandTests +{ + [Theory, BitAutoData] + public async Task RestoreUser_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, SutProvider sutProvider) + { + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + + await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); + + await sutProvider.GetDependency() + .Received(1) + .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); + } + + [Theory, BitAutoData] + public async Task RestoreUser_WithPushSyncOrgKeysOnRevokeRestoreEnabled_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, SutProvider sutProvider) + { + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) + .Returns(true); + + await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); + + await sutProvider.GetDependency() + .Received(1) + .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); + await sutProvider.GetDependency() + .Received(1) + .PushSyncOrgKeysAsync(organizationUser.UserId!.Value); + } + + [Theory, BitAutoData] + public async Task RestoreUser_WithEventSystemUser_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, EventSystemUser eventSystemUser, SutProvider sutProvider) + { + RestoreUser_Setup(organization, null, organizationUser, sutProvider); + + await sutProvider.Sut.RestoreUserAsync(organizationUser, eventSystemUser); + + await sutProvider.GetDependency() + .Received(1) + .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored, eventSystemUser); + } + + [Theory, BitAutoData] + public async Task RestoreUser_WithEventSystemUser_WithPushSyncOrgKeysOnRevokeRestoreEnabled_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, EventSystemUser eventSystemUser, SutProvider sutProvider) + { + RestoreUser_Setup(organization, null, organizationUser, sutProvider); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) + .Returns(true); + + await sutProvider.Sut.RestoreUserAsync(organizationUser, eventSystemUser); + + await sutProvider.GetDependency() + .Received(1) + .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored, eventSystemUser); + await sutProvider.GetDependency() + .Received(1) + .PushSyncOrgKeysAsync(organizationUser.UserId!.Value); + } + + [Theory, BitAutoData] + public async Task RestoreUser_RestoreThemselves_Fails(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, SutProvider sutProvider) + { + organizationUser.UserId = owner.Id; + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); + + Assert.Contains("you cannot restore yourself", exception.Message.ToLowerInvariant()); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); + } + + [Theory] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Custom)] + public async Task RestoreUser_AdminRestoreOwner_Fails(OrganizationUserType restoringUserType, + Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed)] OrganizationUser restoringUser, + [OrganizationUser(OrganizationUserStatusType.Revoked, OrganizationUserType.Owner)] OrganizationUser organizationUser, SutProvider sutProvider) + { + restoringUser.Type = restoringUserType; + RestoreUser_Setup(organization, restoringUser, organizationUser, sutProvider); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RestoreUserAsync(organizationUser, restoringUser.Id)); + + Assert.Contains("only owners can restore other owners", exception.Message.ToLowerInvariant()); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); + } + + [Theory] + [BitAutoData(OrganizationUserStatusType.Invited)] + [BitAutoData(OrganizationUserStatusType.Accepted)] + [BitAutoData(OrganizationUserStatusType.Confirmed)] + public async Task RestoreUser_WithStatusOtherThanRevoked_Fails(OrganizationUserStatusType userStatus, Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser] OrganizationUser organizationUser, SutProvider sutProvider) + { + organizationUser.Status = userStatus; + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); + + Assert.Contains("already active", exception.Message.ToLowerInvariant()); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task RestoreUser_WithOtherOrganizationSingleOrgPolicyEnabled_Fails( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, + SutProvider sutProvider) + { + organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + + sutProvider.GetDependency() + .AnyPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.SingleOrg, Arg.Any()) + .Returns(true); + + var user = new User(); + user.Email = "test@bitwarden.com"; + sutProvider.GetDependency().GetByIdAsync(organizationUser.UserId.Value).Returns(user); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); + + Assert.Contains("test@bitwarden.com belongs to an organization that doesn't allow them to join multiple organizations", exception.Message.ToLowerInvariant()); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task RestoreUser_With2FAPolicyEnabled_WithoutUser2FAConfigured_Fails( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, + SutProvider sutProvider) + { + organizationUser.Email = null; + + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (organizationUser.UserId.Value, false) }); + + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + + sutProvider.GetDependency() + .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) + .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } }); + + var user = new User(); + user.Email = "test@bitwarden.com"; + sutProvider.GetDependency().GetByIdAsync(organizationUser.UserId.Value).Returns(user); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); + + Assert.Contains("test@bitwarden.com is not compliant with the two-step login policy", exception.Message.ToLowerInvariant()); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task RestoreUser_With2FAPolicyEnabled_WithUser2FAConfigured_Success( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, + SutProvider sutProvider) + { + organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + + sutProvider.GetDependency() + .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) + .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } }); + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (organizationUser.UserId.Value, true) }); + + await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); + + await sutProvider.GetDependency() + .Received(1) + .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Confirmed); + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); + } + + [Theory, BitAutoData] + public async Task RestoreUser_WithSingleOrgPolicyEnabled_Fails( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser secondOrganizationUser, + SutProvider sutProvider) + { + organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke + secondOrganizationUser.UserId = organizationUser.UserId; + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + + sutProvider.GetDependency() + .GetManyByUserAsync(organizationUser.UserId.Value) + .Returns(new[] { organizationUser, secondOrganizationUser }); + sutProvider.GetDependency() + .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.SingleOrg, Arg.Any()) + .Returns(new[] + { + new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.SingleOrg, OrganizationUserStatus = OrganizationUserStatusType.Revoked } + }); + + var user = new User(); + user.Email = "test@bitwarden.com"; + sutProvider.GetDependency().GetByIdAsync(organizationUser.UserId.Value).Returns(user); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); + + Assert.Contains("test@bitwarden.com is not compliant with the single organization policy", exception.Message.ToLowerInvariant()); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task RestoreUser_vNext_WithOtherOrganizationSingleOrgPolicyEnabled_Fails( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser secondOrganizationUser, + SutProvider sutProvider) + { + organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke + secondOrganizationUser.UserId = organizationUser.UserId; + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)> { (organizationUser.UserId.Value, true) }); + + sutProvider.GetDependency() + .AnyPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.SingleOrg, Arg.Any()) + .Returns(true); + + var user = new User { Email = "test@bitwarden.com" }; + sutProvider.GetDependency().GetByIdAsync(organizationUser.UserId.Value).Returns(user); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); + + Assert.Contains("test@bitwarden.com belongs to an organization that doesn't allow them to join multiple organizations", exception.Message.ToLowerInvariant()); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task RestoreUser_WithSingleOrgPolicyEnabled_And_2FA_Policy_Fails( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser secondOrganizationUser, + SutProvider sutProvider) + { + organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke + secondOrganizationUser.UserId = organizationUser.UserId; + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + + sutProvider.GetDependency() + .GetManyByUserAsync(organizationUser.UserId.Value) + .Returns(new[] { organizationUser, secondOrganizationUser }); + sutProvider.GetDependency() + .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.SingleOrg, Arg.Any()) + .Returns(new[] + { + new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.SingleOrg, OrganizationUserStatus = OrganizationUserStatusType.Revoked } + }); + + sutProvider.GetDependency() + .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) + .Returns([ + new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication, OrganizationUserStatus = OrganizationUserStatusType.Revoked } + ]); + + var user = new User { Email = "test@bitwarden.com" }; + sutProvider.GetDependency().GetByIdAsync(organizationUser.UserId.Value).Returns(user); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); + + Assert.Contains("test@bitwarden.com is not compliant with the single organization and two-step login policy", exception.Message.ToLowerInvariant()); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task RestoreUser_vNext_With2FAPolicyEnabled_WithoutUser2FAConfigured_Fails( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, + SutProvider sutProvider) + { + organizationUser.Email = null; + + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + + sutProvider.GetDependency() + .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) + .Returns([new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } + ]); + + var user = new User { Email = "test@bitwarden.com" }; + sutProvider.GetDependency().GetByIdAsync(organizationUser.UserId.Value).Returns(user); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); + + Assert.Contains("test@bitwarden.com is not compliant with the two-step login policy", exception.Message.ToLowerInvariant()); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .RestoreAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushSyncOrgKeysAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task RestoreUser_vNext_With2FAPolicyEnabled_WithUser2FAConfigured_Success( + Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, + SutProvider sutProvider) + { + organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + + sutProvider.GetDependency() + .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) + .Returns([new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } + ]); + + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)> { (organizationUser.UserId.Value, true) }); + + await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); + + await sutProvider.GetDependency() + .Received(1) + .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Confirmed); + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); + } + + [Theory, BitAutoData] + public async Task RestoreUser_WhenUserOwningAnotherFreeOrganization_ThenRestoreUserFails( + Organization organization, + Organization otherOrganization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser orgUserOwnerFromDifferentOrg, + SutProvider sutProvider) + { + organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke + + orgUserOwnerFromDifferentOrg.UserId = organizationUser.UserId; + otherOrganization.Id = orgUserOwnerFromDifferentOrg.OrganizationId; + otherOrganization.PlanType = PlanType.Free; + + RestoreUser_Setup(organization, owner, organizationUser, sutProvider); + + sutProvider.GetDependency() + .GetManyByUserAsync(organizationUser.UserId.Value) + .Returns([orgUserOwnerFromDifferentOrg]); + + sutProvider.GetDependency() + .GetManyByUserIdAsync(organizationUser.UserId.Value) + .Returns([otherOrganization]); + + sutProvider.GetDependency() + .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) + .Returns([new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } + ]); + + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)> { (organizationUser.UserId.Value, true) }); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); + + Assert.Equal("User is an owner/admin of another free organization. Please have them upgrade to a paid plan to restore their account.", exception.Message); + } + + [Theory, BitAutoData] + public async Task RestoreUsers_Success(Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser1, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser2, + SutProvider sutProvider) + { + // Arrange + RestoreUser_Setup(organization, owner, orgUser1, sutProvider); + var organizationUserRepository = sutProvider.GetDependency(); + var eventService = sutProvider.GetDependency(); + var twoFactorIsEnabledQuery = sutProvider.GetDependency(); + var userService = Substitute.For(); + + orgUser1.Email = orgUser2.Email = null; // Mock that users were previously confirmed + orgUser1.OrganizationId = orgUser2.OrganizationId = organization.Id; + organizationUserRepository + .GetManyAsync(Arg.Is>(ids => ids.Contains(orgUser1.Id) && ids.Contains(orgUser2.Id))) + .Returns([orgUser1, orgUser2]); + + twoFactorIsEnabledQuery + .TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(orgUser1.UserId!.Value) && ids.Contains(orgUser2.UserId!.Value))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)> + { + (orgUser1.UserId!.Value, true), + (orgUser2.UserId!.Value, false) + }); + + // Act + var result = await sutProvider.Sut.RestoreUsersAsync(organization.Id, new[] { orgUser1.Id, orgUser2.Id }, owner.Id, userService); + + // Assert + Assert.Equal(2, result.Count); + Assert.All(result, r => Assert.Empty(r.Item2)); // No error messages + await organizationUserRepository + .Received(1) + .RestoreAsync(orgUser1.Id, OrganizationUserStatusType.Confirmed); + await organizationUserRepository + .Received(1) + .RestoreAsync(orgUser2.Id, OrganizationUserStatusType.Confirmed); + await eventService.Received(1) + .LogOrganizationUserEventAsync(orgUser1, EventType.OrganizationUser_Restored); + await eventService.Received(1) + .LogOrganizationUserEventAsync(orgUser2, EventType.OrganizationUser_Restored); + } + + [Theory, BitAutoData] + public async Task RestoreUsers_With2FAPolicy_BlocksNonCompliantUser(Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser1, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser2, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser3, + SutProvider sutProvider) + { + // Arrange + RestoreUser_Setup(organization, owner, orgUser1, sutProvider); + var organizationUserRepository = sutProvider.GetDependency(); + var userRepository = sutProvider.GetDependency(); + var policyService = sutProvider.GetDependency(); + var userService = Substitute.For(); + + orgUser1.Email = orgUser2.Email = null; + orgUser3.UserId = null; + orgUser3.Key = null; + orgUser1.OrganizationId = orgUser2.OrganizationId = orgUser3.OrganizationId = organization.Id; + organizationUserRepository + .GetManyAsync(Arg.Is>(ids => ids.Contains(orgUser1.Id) && ids.Contains(orgUser2.Id) && ids.Contains(orgUser3.Id))) + .Returns(new[] { orgUser1, orgUser2, orgUser3 }); + + userRepository.GetByIdAsync(orgUser2.UserId!.Value).Returns(new User { Email = "test@example.com" }); + + // Setup 2FA policy + policyService.GetPoliciesApplicableToUserAsync(Arg.Any(), PolicyType.TwoFactorAuthentication, Arg.Any()) + .Returns([new OrganizationUserPolicyDetails { OrganizationId = organization.Id, PolicyType = PolicyType.TwoFactorAuthentication }]); + + // User1 has 2FA, User2 doesn't + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(orgUser1.UserId!.Value) && ids.Contains(orgUser2.UserId!.Value))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)> + { + (orgUser1.UserId!.Value, true), + (orgUser2.UserId!.Value, false) + }); + + // Act + var result = await sutProvider.Sut.RestoreUsersAsync(organization.Id, [orgUser1.Id, orgUser2.Id, orgUser3.Id], owner.Id, userService); + + // Assert + Assert.Equal(3, result.Count); + Assert.Empty(result[0].Item2); // First user should succeed + Assert.Contains("two-step login", result[1].Item2); // Second user should fail + Assert.Empty(result[2].Item2); // Third user should succeed + await organizationUserRepository + .Received(1) + .RestoreAsync(orgUser1.Id, OrganizationUserStatusType.Confirmed); + await organizationUserRepository + .DidNotReceive() + .RestoreAsync(orgUser2.Id, Arg.Any()); + await organizationUserRepository + .Received(1) + .RestoreAsync(orgUser3.Id, OrganizationUserStatusType.Invited); + } + + [Theory, BitAutoData] + public async Task RestoreUsers_UserOwnsAnotherFreeOrganization_BlocksOwnerUserFromBeingRestored(Organization organization, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser1, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser2, + [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser3, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser orgUserFromOtherOrg, + Organization otherOrganization, + SutProvider sutProvider) + { + // Arrange + RestoreUser_Setup(organization, owner, orgUser1, sutProvider); + var organizationUserRepository = sutProvider.GetDependency(); + var userRepository = sutProvider.GetDependency(); + var policyService = sutProvider.GetDependency(); + var userService = Substitute.For(); + + orgUser1.Email = orgUser2.Email = null; + orgUser3.UserId = null; + orgUser3.Key = null; + orgUser1.OrganizationId = orgUser2.OrganizationId = orgUser3.OrganizationId = organization.Id; + + orgUserFromOtherOrg.UserId = orgUser1.UserId; + otherOrganization.Id = orgUserFromOtherOrg.OrganizationId; + otherOrganization.PlanType = PlanType.Free; + + organizationUserRepository + .GetManyAsync(Arg.Is>(ids => ids.Contains(orgUser1.Id) && ids.Contains(orgUser2.Id) && ids.Contains(orgUser3.Id))) + .Returns(new[] { orgUser1, orgUser2, orgUser3 }); + + userRepository.GetByIdAsync(orgUser2.UserId!.Value).Returns(new User { Email = "test@example.com" }); + + sutProvider.GetDependency() + .GetManyByManyUsersAsync(Arg.Any>()) + .Returns([orgUserFromOtherOrg]); + + sutProvider.GetDependency() + .GetManyByIdsAsync(Arg.Is>(ids => ids.Contains(orgUserFromOtherOrg.OrganizationId))) + .Returns([otherOrganization]); + + + // Setup 2FA policy + policyService.GetPoliciesApplicableToUserAsync(Arg.Any(), PolicyType.TwoFactorAuthentication, Arg.Any()) + .Returns([new OrganizationUserPolicyDetails { OrganizationId = organization.Id, PolicyType = PolicyType.TwoFactorAuthentication }]); + + // User1 has 2FA, User2 doesn't + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(orgUser1.UserId!.Value) && ids.Contains(orgUser2.UserId!.Value))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)> + { + (orgUser1.UserId!.Value, true), + (orgUser2.UserId!.Value, false) + }); + + // Act + var result = await sutProvider.Sut.RestoreUsersAsync(organization.Id, [orgUser1.Id, orgUser2.Id, orgUser3.Id], owner.Id, userService); + + // Assert + Assert.Equal(3, result.Count); + Assert.Contains("owner", result[0].Item2); // Owner should fail + await organizationUserRepository + .DidNotReceive() + .RestoreAsync(orgUser1.Id, OrganizationUserStatusType.Confirmed); + } + + private static void RestoreUser_Setup( + Organization organization, + OrganizationUser? requestingOrganizationUser, + OrganizationUser targetOrganizationUser, + SutProvider sutProvider) + { + if (requestingOrganizationUser != null) + { + requestingOrganizationUser.OrganizationId = organization.Id; + } + targetOrganizationUser.OrganizationId = organization.Id; + + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + sutProvider.GetDependency().OrganizationOwner(organization.Id).Returns(requestingOrganizationUser != null && requestingOrganizationUser.Type is OrganizationUserType.Owner); + sutProvider.GetDependency().ManageUsers(organization.Id).Returns(requestingOrganizationUser != null && (requestingOrganizationUser.Type is OrganizationUserType.Owner or OrganizationUserType.Admin)); + } +} diff --git a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs index 82dc0e2ebe..bd8ae1daaf 100644 --- a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs @@ -1,14 +1,11 @@ using System.Text.Json; using Bit.Core.AdminConsole.Entities.Provider; -using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Enums.Provider; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; -using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; -using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Billing.Enums; using Bit.Core.Billing.Pricing; using Bit.Core.Context; @@ -1233,451 +1230,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) .PushSyncOrgKeysAsync(organizationUser.UserId!.Value); } - [Theory, BitAutoData] - public async Task RestoreUser_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, SutProvider sutProvider) - { - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - - await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); - - await sutProvider.GetDependency() - .Received(1) - .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); - await sutProvider.GetDependency() - .Received(1) - .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); - } - - [Theory, BitAutoData] - public async Task RestoreUser_WithPushSyncOrgKeysOnRevokeRestoreEnabled_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, SutProvider sutProvider) - { - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) - .Returns(true); - - await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); - - await sutProvider.GetDependency() - .Received(1) - .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); - await sutProvider.GetDependency() - .Received(1) - .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); - await sutProvider.GetDependency() - .Received(1) - .PushSyncOrgKeysAsync(organizationUser.UserId!.Value); - } - - [Theory, BitAutoData] - public async Task RestoreUser_WithEventSystemUser_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, EventSystemUser eventSystemUser, SutProvider sutProvider) - { - RestoreRevokeUser_Setup(organization, null, organizationUser, sutProvider); - - await sutProvider.Sut.RestoreUserAsync(organizationUser, eventSystemUser); - - await sutProvider.GetDependency() - .Received(1) - .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); - await sutProvider.GetDependency() - .Received(1) - .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored, eventSystemUser); - } - - [Theory, BitAutoData] - public async Task RestoreUser_WithEventSystemUser_WithPushSyncOrgKeysOnRevokeRestoreEnabled_Success(Organization organization, [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, EventSystemUser eventSystemUser, SutProvider sutProvider) - { - RestoreRevokeUser_Setup(organization, null, organizationUser, sutProvider); - - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.PushSyncOrgKeysOnRevokeRestore) - .Returns(true); - - await sutProvider.Sut.RestoreUserAsync(organizationUser, eventSystemUser); - - await sutProvider.GetDependency() - .Received(1) - .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); - await sutProvider.GetDependency() - .Received(1) - .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored, eventSystemUser); - await sutProvider.GetDependency() - .Received(1) - .PushSyncOrgKeysAsync(organizationUser.UserId!.Value); - } - - [Theory, BitAutoData] - public async Task RestoreUser_RestoreThemselves_Fails(Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, SutProvider sutProvider) - { - organizationUser.UserId = owner.Id; - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); - - Assert.Contains("you cannot restore yourself", exception.Message.ToLowerInvariant()); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .RestoreAsync(Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .PushSyncOrgKeysAsync(Arg.Any()); - } - - [Theory] - [BitAutoData(OrganizationUserType.Admin)] - [BitAutoData(OrganizationUserType.Custom)] - public async Task RestoreUser_AdminRestoreOwner_Fails(OrganizationUserType restoringUserType, - Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed)] OrganizationUser restoringUser, - [OrganizationUser(OrganizationUserStatusType.Revoked, OrganizationUserType.Owner)] OrganizationUser organizationUser, SutProvider sutProvider) - { - restoringUser.Type = restoringUserType; - RestoreRevokeUser_Setup(organization, restoringUser, organizationUser, sutProvider); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, restoringUser.Id)); - - Assert.Contains("only owners can restore other owners", exception.Message.ToLowerInvariant()); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .RestoreAsync(Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .PushSyncOrgKeysAsync(Arg.Any()); - } - - [Theory] - [BitAutoData(OrganizationUserStatusType.Invited)] - [BitAutoData(OrganizationUserStatusType.Accepted)] - [BitAutoData(OrganizationUserStatusType.Confirmed)] - public async Task RestoreUser_WithStatusOtherThanRevoked_Fails(OrganizationUserStatusType userStatus, Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser] OrganizationUser organizationUser, SutProvider sutProvider) - { - organizationUser.Status = userStatus; - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); - - Assert.Contains("already active", exception.Message.ToLowerInvariant()); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .RestoreAsync(Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .PushSyncOrgKeysAsync(Arg.Any()); - } - - [Theory, BitAutoData] - public async Task RestoreUser_WithOtherOrganizationSingleOrgPolicyEnabled_Fails( - Organization organization, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, - SutProvider sutProvider) - { - organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - - sutProvider.GetDependency() - .AnyPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.SingleOrg, Arg.Any()) - .Returns(true); - - var user = new User(); - user.Email = "test@bitwarden.com"; - sutProvider.GetDependency().GetByIdAsync(organizationUser.UserId.Value).Returns(user); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); - - Assert.Contains("test@bitwarden.com belongs to an organization that doesn't allow them to join multiple organizations", exception.Message.ToLowerInvariant()); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .RestoreAsync(Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .PushSyncOrgKeysAsync(Arg.Any()); - } - - [Theory, BitAutoData] - public async Task RestoreUser_With2FAPolicyEnabled_WithoutUser2FAConfigured_Fails( - Organization organization, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, - SutProvider sutProvider) - { - organizationUser.Email = null; - - sutProvider.GetDependency() - .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) - .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (organizationUser.UserId.Value, false) }); - - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - - sutProvider.GetDependency() - .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) - .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } }); - - var user = new User(); - user.Email = "test@bitwarden.com"; - sutProvider.GetDependency().GetByIdAsync(organizationUser.UserId.Value).Returns(user); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); - - Assert.Contains("test@bitwarden.com is not compliant with the two-step login policy", exception.Message.ToLowerInvariant()); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .RestoreAsync(Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .PushSyncOrgKeysAsync(Arg.Any()); - } - - [Theory, BitAutoData] - public async Task RestoreUser_With2FAPolicyEnabled_WithUser2FAConfigured_Success( - Organization organization, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, - SutProvider sutProvider) - { - organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - - sutProvider.GetDependency() - .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) - .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } }); - sutProvider.GetDependency() - .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) - .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (organizationUser.UserId.Value, true) }); - - await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); - - await sutProvider.GetDependency() - .Received(1) - .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Confirmed); - await sutProvider.GetDependency() - .Received(1) - .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); - } - - [Theory, BitAutoData] - public async Task RestoreUser_WithSingleOrgPolicyEnabled_Fails( - Organization organization, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, - [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser secondOrganizationUser, - SutProvider sutProvider) - { - organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke - secondOrganizationUser.UserId = organizationUser.UserId; - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - - sutProvider.GetDependency() - .GetManyByUserAsync(organizationUser.UserId.Value) - .Returns(new[] { organizationUser, secondOrganizationUser }); - sutProvider.GetDependency() - .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.SingleOrg, Arg.Any()) - .Returns(new[] - { - new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.SingleOrg, OrganizationUserStatus = OrganizationUserStatusType.Revoked } - }); - - var user = new User(); - user.Email = "test@bitwarden.com"; - sutProvider.GetDependency().GetByIdAsync(organizationUser.UserId.Value).Returns(user); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); - - Assert.Contains("test@bitwarden.com is not compliant with the single organization policy", exception.Message.ToLowerInvariant()); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .RestoreAsync(Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .PushSyncOrgKeysAsync(Arg.Any()); - } - - [Theory, BitAutoData] - public async Task RestoreUser_vNext_WithOtherOrganizationSingleOrgPolicyEnabled_Fails( - Organization organization, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, - [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser secondOrganizationUser, - SutProvider sutProvider) - { - organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke - secondOrganizationUser.UserId = organizationUser.UserId; - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - - sutProvider.GetDependency() - .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) - .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (organizationUser.UserId.Value, true) }); - - sutProvider.GetDependency() - .AnyPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.SingleOrg, Arg.Any()) - .Returns(true); - - var user = new User(); - user.Email = "test@bitwarden.com"; - sutProvider.GetDependency().GetByIdAsync(organizationUser.UserId.Value).Returns(user); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); - - Assert.Contains("test@bitwarden.com belongs to an organization that doesn't allow them to join multiple organizations", exception.Message.ToLowerInvariant()); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .RestoreAsync(Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .PushSyncOrgKeysAsync(Arg.Any()); - } - - [Theory, BitAutoData] - public async Task RestoreUser_WithSingleOrgPolicyEnabled_And_2FA_Policy_Fails( - Organization organization, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, - [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser secondOrganizationUser, - SutProvider sutProvider) - { - organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke - secondOrganizationUser.UserId = organizationUser.UserId; - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - - sutProvider.GetDependency() - .GetManyByUserAsync(organizationUser.UserId.Value) - .Returns(new[] { organizationUser, secondOrganizationUser }); - sutProvider.GetDependency() - .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.SingleOrg, Arg.Any()) - .Returns(new[] - { - new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.SingleOrg, OrganizationUserStatus = OrganizationUserStatusType.Revoked } - }); - - sutProvider.GetDependency() - .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) - .Returns(new[] - { - new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication, OrganizationUserStatus = OrganizationUserStatusType.Revoked } - }); - - var user = new User(); - user.Email = "test@bitwarden.com"; - sutProvider.GetDependency().GetByIdAsync(organizationUser.UserId.Value).Returns(user); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); - - Assert.Contains("test@bitwarden.com is not compliant with the single organization and two-step login polciy", exception.Message.ToLowerInvariant()); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .RestoreAsync(Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .PushSyncOrgKeysAsync(Arg.Any()); - } - - [Theory, BitAutoData] - public async Task RestoreUser_vNext_With2FAPolicyEnabled_WithoutUser2FAConfigured_Fails( - Organization organization, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, - SutProvider sutProvider) - { - organizationUser.Email = null; - - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - - sutProvider.GetDependency() - .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) - .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } }); - - var user = new User(); - user.Email = "test@bitwarden.com"; - sutProvider.GetDependency().GetByIdAsync(organizationUser.UserId.Value).Returns(user); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); - - Assert.Contains("test@bitwarden.com is not compliant with the two-step login policy", exception.Message.ToLowerInvariant()); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .RestoreAsync(Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .PushSyncOrgKeysAsync(Arg.Any()); - } - - [Theory, BitAutoData] - public async Task RestoreUser_vNext_With2FAPolicyEnabled_WithUser2FAConfigured_Success( - Organization organization, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, - SutProvider sutProvider) - { - organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - - sutProvider.GetDependency() - .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) - .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } }); - - sutProvider.GetDependency() - .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) - .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (organizationUser.UserId.Value, true) }); - - await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); - - await sutProvider.GetDependency() - .Received(1) - .RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Confirmed); - await sutProvider.GetDependency() - .Received(1) - .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); - } - [Theory] [BitAutoData(PlanType.TeamsAnnually)] [BitAutoData(PlanType.TeamsMonthly)] @@ -1991,107 +1543,4 @@ OrganizationUserInvite invite, SutProvider sutProvider) } ); } - - [Theory, BitAutoData] - public async Task RestoreUsers_Success(Organization organization, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser1, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser2, - SutProvider sutProvider) - { - // Arrange - RestoreRevokeUser_Setup(organization, owner, orgUser1, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); - var twoFactorIsEnabledQuery = sutProvider.GetDependency(); - var userService = Substitute.For(); - - orgUser1.Email = orgUser2.Email = null; // Mock that users were previously confirmed - orgUser1.OrganizationId = orgUser2.OrganizationId = organization.Id; - organizationUserRepository - .GetManyAsync(Arg.Is>(ids => ids.Contains(orgUser1.Id) && ids.Contains(orgUser2.Id))) - .Returns(new[] { orgUser1, orgUser2 }); - - twoFactorIsEnabledQuery - .TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(orgUser1.UserId!.Value) && ids.Contains(orgUser2.UserId!.Value))) - .Returns(new List<(Guid userId, bool twoFactorIsEnabled)> - { - (orgUser1.UserId!.Value, true), - (orgUser2.UserId!.Value, false) - }); - - // Act - var result = await sutProvider.Sut.RestoreUsersAsync(organization.Id, new[] { orgUser1.Id, orgUser2.Id }, owner.Id, userService); - - // Assert - Assert.Equal(2, result.Count); - Assert.All(result, r => Assert.Empty(r.Item2)); // No error messages - await organizationUserRepository - .Received(1) - .RestoreAsync(orgUser1.Id, OrganizationUserStatusType.Confirmed); - await organizationUserRepository - .Received(1) - .RestoreAsync(orgUser2.Id, OrganizationUserStatusType.Confirmed); - await eventService.Received(1) - .LogOrganizationUserEventAsync(orgUser1, EventType.OrganizationUser_Restored); - await eventService.Received(1) - .LogOrganizationUserEventAsync(orgUser2, EventType.OrganizationUser_Restored); - } - - [Theory, BitAutoData] - public async Task RestoreUsers_With2FAPolicy_BlocksNonCompliantUser(Organization organization, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser1, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser2, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser orgUser3, - SutProvider sutProvider) - { - // Arrange - RestoreRevokeUser_Setup(organization, owner, orgUser1, sutProvider); - var organizationUserRepository = sutProvider.GetDependency(); - var userRepository = sutProvider.GetDependency(); - var policyService = sutProvider.GetDependency(); - var userService = Substitute.For(); - - orgUser1.Email = orgUser2.Email = null; - orgUser3.UserId = null; - orgUser3.Key = null; - orgUser1.OrganizationId = orgUser2.OrganizationId = orgUser3.OrganizationId = organization.Id; - organizationUserRepository - .GetManyAsync(Arg.Is>(ids => ids.Contains(orgUser1.Id) && ids.Contains(orgUser2.Id) && ids.Contains(orgUser3.Id))) - .Returns(new[] { orgUser1, orgUser2, orgUser3 }); - - userRepository.GetByIdAsync(orgUser2.UserId!.Value).Returns(new User { Email = "test@example.com" }); - - // Setup 2FA policy - policyService.GetPoliciesApplicableToUserAsync(Arg.Any(), PolicyType.TwoFactorAuthentication, Arg.Any()) - .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organization.Id, PolicyType = PolicyType.TwoFactorAuthentication } }); - - // User1 has 2FA, User2 doesn't - sutProvider.GetDependency() - .TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(orgUser1.UserId!.Value) && ids.Contains(orgUser2.UserId!.Value))) - .Returns(new List<(Guid userId, bool twoFactorIsEnabled)> - { - (orgUser1.UserId!.Value, true), - (orgUser2.UserId!.Value, false) - }); - - // Act - var result = await sutProvider.Sut.RestoreUsersAsync(organization.Id, new[] { orgUser1.Id, orgUser2.Id, orgUser3.Id }, owner.Id, userService); - - // Assert - Assert.Equal(3, result.Count); - Assert.Empty(result[0].Item2); // First user should succeed - Assert.Contains("two-step login", result[1].Item2); // Second user should fail - Assert.Empty(result[2].Item2); // Third user should succeed - await organizationUserRepository - .Received(1) - .RestoreAsync(orgUser1.Id, OrganizationUserStatusType.Confirmed); - await organizationUserRepository - .DidNotReceive() - .RestoreAsync(orgUser2.Id, Arg.Any()); - await organizationUserRepository - .Received(1) - .RestoreAsync(orgUser3.Id, OrganizationUserStatusType.Invited); - } } diff --git a/test/Infrastructure.EFIntegration.Test/AdminConsole/Repositories/OrganizationRepositoryTests.cs b/test/Infrastructure.EFIntegration.Test/AdminConsole/Repositories/OrganizationRepositoryTests.cs index f6d4227e2b..e8bafaea5b 100644 --- a/test/Infrastructure.EFIntegration.Test/AdminConsole/Repositories/OrganizationRepositoryTests.cs +++ b/test/Infrastructure.EFIntegration.Test/AdminConsole/Repositories/OrganizationRepositoryTests.cs @@ -196,4 +196,43 @@ public class OrganizationRepositoryTests Assert.Single(sqlResult); Assert.True(sqlResult.All(o => o.Name == org.Name)); } + + [CiSkippedTheory, EfOrganizationAutoData] + public async Task GetManyByIdsAsync_Works_DataMatches(List organizations, + SqlRepo.OrganizationRepository sqlOrganizationRepo, + List suts) + { + var returnedOrgs = new List(); + + foreach (var sut in suts) + { + _ = await sut.CreateMany(organizations); + sut.ClearChangeTracking(); + + var efReturnedOrgs = await sut.GetManyByIdsAsync(organizations.Select(o => o.Id).ToList()); + returnedOrgs.AddRange(efReturnedOrgs); + } + + foreach (var organization in organizations) + { + var postSqlOrg = await sqlOrganizationRepo.CreateAsync(organization); + returnedOrgs.Add(await sqlOrganizationRepo.GetByIdAsync(postSqlOrg.Id)); + } + + var orgIds = organizations.Select(o => o.Id).ToList(); + var distinctReturnedOrgIds = returnedOrgs.Select(o => o.Id).Distinct().ToList(); + + Assert.Equal(orgIds.Count, distinctReturnedOrgIds.Count); + Assert.Equivalent(orgIds, distinctReturnedOrgIds); + + // clean up + foreach (var organization in organizations) + { + await sqlOrganizationRepo.DeleteAsync(organization); + foreach (var sut in suts) + { + await sut.DeleteAsync(organization); + } + } + } } diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs index f7c61ad957..a95778b199 100644 --- a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs @@ -253,4 +253,37 @@ public class OrganizationRepositoryTests Assert.Empty(result); } + + + [DatabaseTheory, DatabaseData] + public async Task GetManyByIdsAsync_ExistingOrganizations_ReturnsOrganizations(IOrganizationRepository organizationRepository) + { + var email = "test@email.com"; + + var organization1 = await organizationRepository.CreateAsync(new Organization + { + Name = $"Test Org 1", + BillingEmail = email, + Plan = "Test", + PrivateKey = "privatekey1" + }); + + var organization2 = await organizationRepository.CreateAsync(new Organization + { + Name = $"Test Org 2", + BillingEmail = email, + Plan = "Test", + PrivateKey = "privatekey2" + }); + + var result = await organizationRepository.GetManyByIdsAsync([organization1.Id, organization2.Id]); + + Assert.Equal(2, result.Count); + Assert.Contains(result, org => org.Id == organization1.Id); + Assert.Contains(result, org => org.Id == organization2.Id); + + // Clean up + await organizationRepository.DeleteAsync(organization1); + await organizationRepository.DeleteAsync(organization2); + } } diff --git a/util/Migrator/DbScripts/2025-03-21_00_Org_ReadManyByManyId.sql b/util/Migrator/DbScripts/2025-03-21_00_Org_ReadManyByManyId.sql new file mode 100644 index 0000000000..fb58d3cff7 --- /dev/null +++ b/util/Migrator/DbScripts/2025-03-21_00_Org_ReadManyByManyId.sql @@ -0,0 +1,66 @@ +CREATE OR ALTER PROCEDURE [dbo].[Organization_ReadManyByIds] @OrganizationIds AS [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + + SELECT o.[Id], + o.[Identifier], + o.[Name], + o.[BusinessName], + o.[BusinessAddress1], + o.[BusinessAddress2], + o.[BusinessAddress3], + o.[BusinessCountry], + o.[BusinessTaxNumber], + o.[BillingEmail], + o.[Plan], + o.[PlanType], + o.[Seats], + o.[MaxCollections], + o.[UsePolicies], + o.[UseSso], + o.[UseGroups], + o.[UseDirectory], + o.[UseEvents], + o.[UseTotp], + o.[Use2fa], + o.[UseApi], + o.[UseResetPassword], + o.[SelfHost], + o.[UsersGetPremium], + o.[Storage], + o.[MaxStorageGb], + o.[Gateway], + o.[GatewayCustomerId], + o.[GatewaySubscriptionId], + o.[ReferenceData], + o.[Enabled], + o.[LicenseKey], + o.[PublicKey], + o.[PrivateKey], + o.[TwoFactorProviders], + o.[ExpirationDate], + o.[CreationDate], + o.[RevisionDate], + o.[OwnersNotifiedOfAutoscaling], + o.[MaxAutoscaleSeats], + o.[UseKeyConnector], + o.[UseScim], + o.[UseCustomPermissions], + o.[UseSecretsManager], + o.[Status], + o.[UsePasswordManager], + o.[SmSeats], + o.[SmServiceAccounts], + o.[MaxAutoscaleSmSeats], + o.[MaxAutoscaleSmServiceAccounts], + o.[SecretsManagerBeta], + o.[LimitCollectionCreation], + o.[LimitCollectionDeletion], + o.[LimitItemDeletion], + o.[AllowAdminAccessToAllCollectionItems], + o.[UseRiskInsights] + FROM [dbo].[OrganizationView] o + INNER JOIN @OrganizationIds ids ON o.[Id] = ids.[Id] +END +