From 84393e7fb11f26a9ad5e5d30472b0f9f48d5a1fe Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 24 Aug 2023 13:29:24 -0400 Subject: [PATCH 01/18] Bumped version to 2023.8.2 (#3227) Co-authored-by: bitwarden-devops-bot <106330231+bitwarden-devops-bot@users.noreply.github.com> --- Directory.Build.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Directory.Build.props b/Directory.Build.props index 62c0d4b76a..3a60d958ca 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -2,7 +2,7 @@ net6.0 - 2023.8.1 + 2023.8.2 Bit.$(MSBuildProjectName) true enable From 4c77f993e561ca37265b9b7caf3b9cc85e69504b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Fri, 25 Aug 2023 11:22:16 +0100 Subject: [PATCH 02/18] [AC-108] Enterprise policies are not removed when downgraded to teams (#3168) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [AC-1423] Add AddonProduct and BitwardenProduct properties to BillingSubscriptionItem (#3037) * [AC-1423] Add AddonProduct and BitwardenProduct properties to BillingSubscriptionItem - Add a helper method to determine the appropriate addon type based on the subscription items StripeId * [AC-1423] Add helper to StaticStore.cs to find a Plan by StripePlanId * [AC-1423] Use the helper method to set SubscriptionInfo.BitwardenProduct * Add SecretsManagerBilling feature flag to Constants * [AC 1409] Secrets Manager Subscription Stripe Integration (#3019) * Adding the Secret manager to the Plan List * Adding the unit test for the StaticStoreTests class * Fix whitespace formatting * Fix whitespace formatting * Price update * Resolving the PR comments * Resolving PR comments * Fixing the whitespace * only password manager plans are return for now * format whitespace * Resolve the test issue * Fixing the failing test * Refactoring the Plan separation * add a unit test for SingleOrDefault * Fix the whitespace format * Separate the PM and SM plans * Fixing the whitespace * Remove unnecessary directive * Fix imports ordering * Fix imports ordering * Resolve imports ordering * Fixing imports ordering * Fix response model, add MaxProjects * Fix filename * Fix format * Fix: seat price should match annual/monthly * Fix service account annual pricing * Changes for secret manager signup and upgradeplan * Changes for secrets manager signup and upgrade * refactoring the code * Format whitespace * remove unnecessary using directive * Resolve the PR comment on Subscription creation * Resolve PR comment * Add password manager to the error message * Add UseSecretsManager to the event log * Resolve PR comment on plan validation * Resolving pr comments for service account count * Resolving pr comments for service account count * Resolve the pr comments * Remove the store procedure that is no-longer needed * Rename a property properly * Resolving the PR comment * Resolve PR comments * Resolving PR comments * Resolving the Pr comments * Resolving some PR comments * Resolving the PR comments * Resolving the build identity build * Add additional Validation * Resolve the Lint issues * remove unnecessary using directive * Remove the white spaces * Adding unit test for the stripe payment * Remove the incomplete test * Fixing the failing test * Fix the failing test * Fix the fail test on organization service * Fix the failing unit test * Fix the whitespace format * Fix the failing test * Fix the whitespace format * resolve pr comments * Fix the lint message * Resolve the PR comments * resolve pr comments * Resolve pr comments * Resolve the pr comments * remove unused code * Added for sm validation test * Fix the whitespace format issues --------- Co-authored-by: Thomas Rittson Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> * SM-802: Add SecretsManagerBetaColumn SQL migration and Org table update * SM-802: Run EF Migrations for SecretsManagerBeta * SM-802: Update the two Org procs and View, and move data migration to a separate file * SM-802: Add missing comma to Organization_Create * [AC-1418] Add missing SecretsManagerPlan property to OrganizationResponseModel (#3055) * SM-802: Remove extra GO statement from data migration script * [AC 1460] Update Stripe Configuration (#3070) * change the stripeseat id * change service accountId to align with new product * make all the Id name for consistent * SM-802: Add SecretsManagerBeta to OrganizationResponseModel * SM-802: Move SecretsManagerBeta from OrganizationResponseModel to OrganizationSubscriptionResponseModel. Use sp_refreshview instead of sp_refreshsqlmodule in the migration script. * SM-802: Remove OrganizationUserOrganizationDetailsView.sql changes * [AC 1410] Secrets Manager subscription adjustment back-end changes (#3036) * Create UpgradeSecretsManagerSubscription command --------- Co-authored-by: Thomas Rittson * SM-802: Remove SecretsManagerBetaColumn migration * SM-802: Add SecretsManagerBetaColumn migration * SM-802: Remove OrganizationUserOrganizationDetailsView update * [AC-1495] Extract UpgradePlanAsync into a command (#3081) * This is a pure lift & shift with no refactors * Only register subscription commands in Api --------- Co-authored-by: cyprain-okeke * [AC-1503] Fix Stripe integration on organization upgrade (#3084) * Fix SM parameters not being passed to Stripe * Fix flaky test * Fix error message * [AC-1504] Allow SM max autoscale limits to be disabled (#3085) * [AC-1488] Changed SM Signup and Upgrade paths to set SmServiceAccounts to include the plan BaseServiceAccount (#3086) * [AC-1510] Enable access to Secrets Manager to Organization owner for new Subscription (#3089) * Revert changes to ReferenceEvent code (#3091) * Revert changes to ReferenceEvent code This will be done in AC-1481 * Revert ReferenceEventType change * Move NoopServiceAccountRepository to SM and update namespace * [AC-1462] Add secrets manager service accounts autoscaling commands (#3059) * Adding the Secret manager to the Plan List * Adding the unit test for the StaticStoreTests class * Fix whitespace formatting * Fix whitespace formatting * Price update * Resolving the PR comments * Resolving PR comments * Fixing the whitespace * only password manager plans are return for now * format whitespace * Resolve the test issue * Fixing the failing test * Refactoring the Plan separation * add a unit test for SingleOrDefault * Fix the whitespace format * Separate the PM and SM plans * Fixing the whitespace * Remove unnecessary directive * Fix imports ordering * Fix imports ordering * Resolve imports ordering * Fixing imports ordering * Fix response model, add MaxProjects * Fix filename * Fix format * Fix: seat price should match annual/monthly * Fix service account annual pricing * Changes for secret manager signup and upgradeplan * Changes for secrets manager signup and upgrade * refactoring the code * Format whitespace * remove unnecessary using directive * Changes for subscription Update * Update the seatAdjustment and update * Resolve the PR comment on Subscription creation * Resolve PR comment * Add password manager to the error message * Add UseSecretsManager to the event log * Resolve PR comment on plan validation * Resolving pr comments for service account count * Resolving pr comments for service account count * Resolve the pr comments * Remove the store procedure that is no-longer needed * Add a new class for update subscription * Modify the Update subscription for sm * Add the missing property * Rename a property properly * Resolving the PR comment * Resolve PR comments * Resolving PR comments * Resolving the Pr comments * Resolving some PR comments * Resolving the PR comments * Resolving the build identity build * Add additional Validation * Resolve the Lint issues * remove unnecessary using directive * Remove the white spaces * Adding unit test for the stripe payment * Remove the incomplete test * Fixing the failing test * Fix the failing test * Fix the fail test on organization service * Fix the failing unit test * Fix the whitespace format * Fix the failing test * Fix the whitespace format * resolve pr comments * Fix the lint message * refactor the code * Fix the failing Test * adding a new endpoint * Remove the unwanted code * Changes for Command and Queries * changes for command and queries * Fix the Lint issues * Fix imports ordering * Resolve the PR comments * resolve pr comments * Resolve pr comments * Fix the failing test on adjustSeatscommandtests * Fix the failing test * Fix the whitespaces * resolve failing test * rename a property * Resolve the pr comments * refactoring the existing implementation * Resolve the whitespaces format issue * Resolve the pr comments * [AC-1462] Created IAvailableServiceAccountsQuery along its implementation and with unit tests * [AC-1462] Renamed ICountNewServiceAccountSlotsRequiredQuery * [AC-1462] Added IAutoscaleServiceAccountsCommand and implementation * Add more unit testing * fix the whitespaces issues * [AC-1462] Added unit tests for AutoscaleServiceAccountsCommand * Add more unit test * Remove unnecessary directive * Resolve some pr comments * Adding more unit test * adding more test * add more test * Resolving some pr comments * Resolving some pr comments * Resolving some pr comments * resolve some pr comments * Resolving pr comments * remove whitespaces * remove white spaces * Resolving pr comments * resolving pr comments and fixing white spaces * resolving the lint error * Run dotnet format * resolving the pr comments * Add a missing properties to plan response model * Add the email sender for sm seat and service acct * Add the email sender for sm seat and service acct * Fix the failing test after email sender changes * Add staticstorewrapper to properly test the plans * Add more test and validate the existing test * Fix the white spaces issues * Remove staticstorewrapper and fix the test * fix a null issue on autoscaling * Suggestion: do all seat calculations in update model * Resolve some pr comments * resolving some pr comments * Return value is unnecessary * Resolve the failing test * resolve pr comments * Resolve the pr comments * Resolving admin api failure and adding more test * Resolve the issue failing admin project * Fixing the failed test * Clarify naming and add comments * Clarify naming conventions * Dotnet format * Fix the failing dependency * remove similar test * [AC-1462] Rewrote AutoscaleServiceAccountsCommand to use UpdateSecretsManagerSubscriptionCommand which has the same logic * [AC-1462] Deleted IAutoscaleServiceAccountsCommand as the logic will be moved to UpdateSecretsManagerSubscriptionCommand * [AC-1462] Created method AdjustSecretsManagerServiceAccountsAsync * [AC-1462] Changed SecretsManagerSubscriptionUpdate to only be set by its constructor * [AC-1462] Added check to CountNewServiceAccountSlotsRequiredQuery and revised unit tests * [AC-1462] Revised logic for CountNewServiceAccountSlotsRequiredQuery and fixed unit tests * [AC-1462] Changed SecretsManagerSubscriptionUpdate to receive Organization as a parameter and fixed the unit tests * [AC-1462] Renamed IUpdateSecretsManagerSubscriptionCommand methods UpdateSubscriptionAsync and AdjustServiceAccountsAsync * [AC-1462] Rewrote unit test UpdateSubscriptionAsync_ValidInput_Passes * [AC-1462] Registered CountNewServiceAccountSlotsRequiredQuery for dependency injection * [AC-1462] Added parameter names to SecretsManagerSubscriptionUpdateRequestModel * [AC-1462] Updated SecretsManagerSubscriptionUpdate logic to handle null parameters. Revised the unit tests to test null values --------- Co-authored-by: cyprain-okeke Co-authored-by: Thomas Rittson Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> * Add UsePasswordManager to sync data (#3114) * [AC-1522] Fix service account check on upgrading (#3111) * Resolved the checkmarx issues * [AC-1521] Address checkmarx security feedback (#3124) * Reinstate target attribute but add noopener noreferrer * Update date on migration script * Remove unused constant * Revert "Remove unused constant" This reverts commit 4fcb9da4d62af815c01579ab265d0ce11b47a9bb. This is required to make feature flags work on the client * [AC-1458] Add Endpoint And Service Logic for secrets manager to existing subscription (#3087) --------- Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Co-authored-by: Thomas Rittson * Remove duplicate migrations from incorrectly resolved merge * [AC-1468] Modified CountNewServiceAccountSlotsRequiredQuery to return zero if organization has SecretsManagerBeta == true (#3112) Co-authored-by: Thomas Rittson * [Ac 1563] Unable to load billing and subscription related pages for non-enterprise organizations (#3138) * Resolve the failing family plan * resolve issues * Resolve code related pr comments * Resolve test related comments * Resolving or comments * [SM-809] Add service account slot limit check (#3093) * Add service account slot limit check * Add query to DI * [AC-1462] Registered CountNewServiceAccountSlotsRequiredQuery for dependency injection * remove duplicate DI entry * Update unit tests * Remove comment * Code review updates --------- Co-authored-by: cyprain-okeke Co-authored-by: Thomas Rittson Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Co-authored-by: Rui Tome * [AC-1461] Secrets manager seat autoscaling (#3121) * Add autoscaling code to invite user, save user, and bulk enable SM flows * Add tests * Delete command for BulkEnableSecretsManager * circular dependency between OrganizationService and UpdateSecretsManagerSubscriptionCommand - fixed by temporarily duplicating ReplaceAndUpdateCache * Unresolvable dependencies in other services - fixed by temporarily registering noop services and moving around some DI code All should be resolved in PM-1880 * Refactor: improve the update object and use it to adjust values, remove excess interfaces on the command * Handle autoscaling-specific errors --------- Co-authored-by: Rui Tomé <108268980+r-tome@users.noreply.github.com> * Move bitwarden_license include reference into conditional block * [AC 1526]Show current SM seat and service account usage in Bitwarden Portal (#3142) * changes base on the tickets request * Code refactoring * Removed the unwanted method * Add implementation to the new method * Resolve some pr comments * resolve lint issue * resolve pr comments * add the new noop files * Add new noop file and resolve some pr comments * resolve pr comments * removed unused method * Fix prefill when changing plans or starting trials * [AC-1575] Don't overwrite existing org information when changing plans * [AC-1577] Prefill SM configuration section when starting trial * Clarify property names * Set SM subscription based on current usage * Fix label for Service Accounts field * Prevent subscribing to SM on an invalid plan * Edit comment * Set minimum seat count for SM * Don't use hardcoded plan values * Use plans directly in JS * Refactor to getPlan function * Remove unneeded namespace refs * Add GetPlansHelper to provide some typesafety * Use planType from form instead of model * Add @model specification * Add null coalescing * [AC-108] Updated PolicyService to use IApplicationCacheService to determine if an organization uses policies (cherry picked from commit b98b107c4b6c6c7c296295e9b55758d7ae9808b6) * [AC-108] Removed checking if Organization is enabled --------- Co-authored-by: Shane Melton Co-authored-by: Thomas Rittson Co-authored-by: cyprain-okeke <108260115+cyprain-okeke@users.noreply.github.com> Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Co-authored-by: Colton Hurst Co-authored-by: cyprain-okeke Co-authored-by: Conner Turnbull Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> --- .../Data/Organizations/OrganizationAbility.cs | 2 ++ .../Services/Implementations/PolicyService.cs | 5 ++++ .../Repositories/OrganizationRepository.cs | 3 ++- .../Organization_ReadAbilities.sql | 1 + .../Endpoints/IdentityServerSsoTests.cs | 1 + .../Endpoints/IdentityServerTests.cs | 2 +- .../2023-08-09_00_OrgAbilitiesUsePolicies.sql | 27 +++++++++++++++++++ 7 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 util/Migrator/DbScripts/2023-08-09_00_OrgAbilitiesUsePolicies.sql diff --git a/src/Core/Models/Data/Organizations/OrganizationAbility.cs b/src/Core/Models/Data/Organizations/OrganizationAbility.cs index 809f4d5d40..22bf4008eb 100644 --- a/src/Core/Models/Data/Organizations/OrganizationAbility.cs +++ b/src/Core/Models/Data/Organizations/OrganizationAbility.cs @@ -20,6 +20,7 @@ public class OrganizationAbility UseScim = organization.UseScim; UseResetPassword = organization.UseResetPassword; UseCustomPermissions = organization.UseCustomPermissions; + UsePolicies = organization.UsePolicies; } public Guid Id { get; set; } @@ -33,4 +34,5 @@ public class OrganizationAbility public bool UseScim { get; set; } public bool UseResetPassword { get; set; } public bool UseCustomPermissions { get; set; } + public bool UsePolicies { get; set; } } diff --git a/src/Core/Services/Implementations/PolicyService.cs b/src/Core/Services/Implementations/PolicyService.cs index 64da29e4dc..5f4e680df7 100644 --- a/src/Core/Services/Implementations/PolicyService.cs +++ b/src/Core/Services/Implementations/PolicyService.cs @@ -12,6 +12,7 @@ namespace Bit.Core.Services; public class PolicyService : IPolicyService { + private readonly IApplicationCacheService _applicationCacheService; private readonly IEventService _eventService; private readonly IOrganizationRepository _organizationRepository; private readonly IOrganizationUserRepository _organizationUserRepository; @@ -21,6 +22,7 @@ public class PolicyService : IPolicyService private readonly GlobalSettings _globalSettings; public PolicyService( + IApplicationCacheService applicationCacheService, IEventService eventService, IOrganizationRepository organizationRepository, IOrganizationUserRepository organizationUserRepository, @@ -29,6 +31,7 @@ public class PolicyService : IPolicyService IMailService mailService, GlobalSettings globalSettings) { + _applicationCacheService = applicationCacheService; _eventService = eventService; _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; @@ -199,7 +202,9 @@ public class PolicyService : IPolicyService { var organizationUserPolicyDetails = await _organizationUserRepository.GetByUserIdWithPolicyDetailsAsync(userId, policyType); var excludedUserTypes = GetUserTypesExcludedFromPolicy(policyType); + var orgAbilities = await _applicationCacheService.GetOrganizationAbilitiesAsync(); return organizationUserPolicyDetails.Where(o => + (!orgAbilities.ContainsKey(o.OrganizationId) || orgAbilities[o.OrganizationId].UsePolicies) && o.PolicyEnabled && !excludedUserTypes.Contains(o.OrganizationUserType) && o.OrganizationUserStatus >= minStatus && diff --git a/src/Infrastructure.EntityFramework/Repositories/OrganizationRepository.cs b/src/Infrastructure.EntityFramework/Repositories/OrganizationRepository.cs index 045ac881ee..702f9bea53 100644 --- a/src/Infrastructure.EntityFramework/Repositories/OrganizationRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/OrganizationRepository.cs @@ -87,7 +87,8 @@ public class OrganizationRepository : Repository(); diff --git a/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs b/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs index b1e74bd17d..cae6ed172c 100644 --- a/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs +++ b/test/Identity.IntegrationTest/Endpoints/IdentityServerTests.cs @@ -556,7 +556,7 @@ public class IdentityServerTests : IClassFixture var organizationUserRepository = _factory.Services.GetService(); var policyRepository = _factory.Services.GetService(); - var organization = new Bit.Core.Entities.Organization { Id = organizationId, Enabled = true, UseSso = ssoPolicyEnabled }; + var organization = new Bit.Core.Entities.Organization { Id = organizationId, Enabled = true, UseSso = ssoPolicyEnabled, UsePolicies = true }; await organizationRepository.CreateAsync(organization); var user = await userRepository.GetByEmailAsync(username); diff --git a/util/Migrator/DbScripts/2023-08-09_00_OrgAbilitiesUsePolicies.sql b/util/Migrator/DbScripts/2023-08-09_00_OrgAbilitiesUsePolicies.sql new file mode 100644 index 0000000000..f19c189fda --- /dev/null +++ b/util/Migrator/DbScripts/2023-08-09_00_OrgAbilitiesUsePolicies.sql @@ -0,0 +1,27 @@ +CREATE OR ALTER PROCEDURE [dbo].[Organization_ReadAbilities] +AS +BEGIN + SET NOCOUNT ON + + SELECT + [Id], + [UseEvents], + [Use2fa], + CASE + WHEN [Use2fa] = 1 AND [TwoFactorProviders] IS NOT NULL AND [TwoFactorProviders] != '{}' THEN + 1 + ELSE + 0 + END AS [Using2fa], + [UsersGetPremium], + [UseCustomPermissions], + [UseSso], + [UseKeyConnector], + [UseScim], + [UseResetPassword], + [UsePolicies], + [Enabled] + FROM + [dbo].[Organization] +END +GO From 4748b5b3fc53aae32b7d42d960d582cef91bf352 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Mon, 28 Aug 2023 08:05:23 +1000 Subject: [PATCH 03/18] [AC-1568] Finish refactor of update subscription interface, minor fixes (#3189) * Remove UpdateSecretsManagerSubscriptionCommand.AdjustServiceAccounts interface * Rewrite tests and use autofixture customizations * Reduce method nesting in the command, simplify parameters * Fix capitalization and wording of error messages * Add checks for existing SM beta enrolment etc --- .../OrganizationUsersController.cs | 4 +- ...tsManagerSubscriptionUpdateRequestModel.cs | 13 +- .../Controllers/ServiceAccountsController.cs | 6 +- .../SecretsManagerSubscriptionUpdate.cs | 31 ++- .../AddSecretsManagerSubscriptionCommand.cs | 11 ++ ...UpdateSecretsManagerSubscriptionCommand.cs | 4 +- ...UpdateSecretsManagerSubscriptionCommand.cs | 187 +++++++----------- .../Implementations/OrganizationService.cs | 8 +- .../ServiceAccountsControllerTests.cs | 10 +- .../AutoFixture/OrganizationFixtures.cs | 1 + ...dSecretsManagerSubscriptionCommandTests.cs | 32 +++ ...eSecretsManagerSubscriptionCommandTests.cs | 65 +++--- 12 files changed, 190 insertions(+), 182 deletions(-) diff --git a/src/Api/Controllers/OrganizationUsersController.cs b/src/Api/Controllers/OrganizationUsersController.cs index 6d1424f01e..3dbaea3ff9 100644 --- a/src/Api/Controllers/OrganizationUsersController.cs +++ b/src/Api/Controllers/OrganizationUsersController.cs @@ -447,8 +447,8 @@ public class OrganizationUsersController : Controller if (additionalSmSeatsRequired > 0) { var organization = await _organizationRepository.GetByIdAsync(orgId); - var update = new SecretsManagerSubscriptionUpdate(organization, true); - update.AdjustSeats(additionalSmSeatsRequired); + var update = new SecretsManagerSubscriptionUpdate(organization, true) + .AdjustSeats(additionalSmSeatsRequired); await _updateSecretsManagerSubscriptionCommand.UpdateSubscriptionAsync(update); } diff --git a/src/Api/Models/Request/Organizations/SecretsManagerSubscriptionUpdateRequestModel.cs b/src/Api/Models/Request/Organizations/SecretsManagerSubscriptionUpdateRequestModel.cs index a8adbe92e5..96e9603fc3 100644 --- a/src/Api/Models/Request/Organizations/SecretsManagerSubscriptionUpdateRequestModel.cs +++ b/src/Api/Models/Request/Organizations/SecretsManagerSubscriptionUpdateRequestModel.cs @@ -14,11 +14,12 @@ public class SecretsManagerSubscriptionUpdateRequestModel public virtual SecretsManagerSubscriptionUpdate ToSecretsManagerSubscriptionUpdate(Organization organization) { - var orgUpdate = new SecretsManagerSubscriptionUpdate( - organization, - seatAdjustment: SeatAdjustment, maxAutoscaleSeats: MaxAutoscaleSeats, - serviceAccountAdjustment: ServiceAccountAdjustment, maxAutoscaleServiceAccounts: MaxAutoscaleServiceAccounts); - - return orgUpdate; + return new SecretsManagerSubscriptionUpdate(organization, false) + { + MaxAutoscaleSmSeats = MaxAutoscaleSeats, + MaxAutoscaleSmServiceAccounts = MaxAutoscaleServiceAccounts + } + .AdjustSeats(SeatAdjustment) + .AdjustServiceAccounts(ServiceAccountAdjustment); } } diff --git a/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs b/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs index 0e2d59da27..bb28276b03 100644 --- a/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs +++ b/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs @@ -4,6 +4,7 @@ using Bit.Api.SecretsManager.Models.Response; using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.Models.Business; using Bit.Core.OrganizationFeatures.OrganizationSubscriptions.Interface; using Bit.Core.Repositories; using Bit.Core.SecretsManager.AuthorizationRequirements; @@ -125,8 +126,9 @@ public class ServiceAccountsController : Controller if (newServiceAccountSlotsRequired > 0) { var org = await _organizationRepository.GetByIdAsync(organizationId); - await _updateSecretsManagerSubscriptionCommand.AdjustServiceAccountsAsync(org, - newServiceAccountSlotsRequired); + var update = new SecretsManagerSubscriptionUpdate(org, true) + .AdjustServiceAccounts(newServiceAccountSlotsRequired); + await _updateSecretsManagerSubscriptionCommand.UpdateSubscriptionAsync(update); } var userId = _userService.GetProperUserId(User).Value; diff --git a/src/Core/Models/Business/SecretsManagerSubscriptionUpdate.cs b/src/Core/Models/Business/SecretsManagerSubscriptionUpdate.cs index a2e687a92e..1cb7a41df8 100644 --- a/src/Core/Models/Business/SecretsManagerSubscriptionUpdate.cs +++ b/src/Core/Models/Business/SecretsManagerSubscriptionUpdate.cs @@ -6,7 +6,7 @@ namespace Bit.Core.Models.Business; public class SecretsManagerSubscriptionUpdate { - public Organization Organization { get; set; } + public Organization Organization { get; } /// /// The total seats the organization will have after the update, including any base seats included in the plan @@ -14,8 +14,7 @@ public class SecretsManagerSubscriptionUpdate public int? SmSeats { get; set; } /// - /// The new autoscale limit for seats, expressed as a total (not an adjustment). - /// This may or may not be the same as the current autoscale limit. + /// The new autoscale limit for seats after the update /// public int? MaxAutoscaleSmSeats { get; set; } @@ -26,8 +25,7 @@ public class SecretsManagerSubscriptionUpdate public int? SmServiceAccounts { get; set; } /// - /// The new autoscale limit for service accounts, expressed as a total (not an adjustment). - /// This may or may not be the same as the current autoscale limit. + /// The new autoscale limit for service accounts after the update /// public int? MaxAutoscaleSmServiceAccounts { get; set; } @@ -39,7 +37,7 @@ public class SecretsManagerSubscriptionUpdate /// /// Whether the subscription update is a result of autoscaling /// - public bool Autoscaling { get; init; } + public bool Autoscaling { get; } /// /// The seats the organization will have after the update, excluding the base seats included in the plan @@ -57,18 +55,11 @@ public class SecretsManagerSubscriptionUpdate public bool MaxAutoscaleSmServiceAccountsChanged => MaxAutoscaleSmServiceAccounts != Organization.MaxAutoscaleSmServiceAccounts; public Plan Plan => Utilities.StaticStore.GetSecretsManagerPlan(Organization.PlanType); + public bool SmSeatAutoscaleLimitReached => SmSeats.HasValue && MaxAutoscaleSmSeats.HasValue && SmSeats == MaxAutoscaleSmSeats; - public SecretsManagerSubscriptionUpdate( - Organization organization, - int seatAdjustment, int? maxAutoscaleSeats, - int serviceAccountAdjustment, int? maxAutoscaleServiceAccounts) : this(organization, false) - { - AdjustSeats(seatAdjustment); - AdjustServiceAccounts(serviceAccountAdjustment); - - MaxAutoscaleSmSeats = maxAutoscaleSeats; - MaxAutoscaleSmServiceAccounts = maxAutoscaleServiceAccounts; - } + public bool SmServiceAccountAutoscaleLimitReached => SmServiceAccounts.HasValue && + MaxAutoscaleSmServiceAccounts.HasValue && + SmServiceAccounts == MaxAutoscaleSmServiceAccounts; public SecretsManagerSubscriptionUpdate(Organization organization, bool autoscaling) { @@ -91,13 +82,15 @@ public class SecretsManagerSubscriptionUpdate Autoscaling = autoscaling; } - public void AdjustSeats(int adjustment) + public SecretsManagerSubscriptionUpdate AdjustSeats(int adjustment) { SmSeats = SmSeats.GetValueOrDefault() + adjustment; + return this; } - public void AdjustServiceAccounts(int adjustment) + public SecretsManagerSubscriptionUpdate AdjustServiceAccounts(int adjustment) { SmServiceAccounts = SmServiceAccounts.GetValueOrDefault() + adjustment; + return this; } } diff --git a/src/Core/OrganizationFeatures/OrganizationSubscriptions/AddSecretsManagerSubscriptionCommand.cs b/src/Core/OrganizationFeatures/OrganizationSubscriptions/AddSecretsManagerSubscriptionCommand.cs index 6b514089e1..3741148af4 100644 --- a/src/Core/OrganizationFeatures/OrganizationSubscriptions/AddSecretsManagerSubscriptionCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationSubscriptions/AddSecretsManagerSubscriptionCommand.cs @@ -62,6 +62,17 @@ public class AddSecretsManagerSubscriptionCommand : IAddSecretsManagerSubscripti throw new NotFoundException(); } + if (organization.SecretsManagerBeta) + { + throw new BadRequestException("Organization is enrolled in Secrets Manager Beta. " + + "Please contact Customer Success to add Secrets Manager to your subscription."); + } + + if (organization.UseSecretsManager) + { + throw new BadRequestException("Organization already uses Secrets Manager."); + } + var plan = StaticStore.GetSecretsManagerPlan(organization.PlanType); if (string.IsNullOrWhiteSpace(organization.GatewayCustomerId) && plan.Product != ProductType.Free) { diff --git a/src/Core/OrganizationFeatures/OrganizationSubscriptions/Interface/IUpdateSecretsManagerSubscriptionCommand.cs b/src/Core/OrganizationFeatures/OrganizationSubscriptions/Interface/IUpdateSecretsManagerSubscriptionCommand.cs index 38126d17c6..036e4a1795 100644 --- a/src/Core/OrganizationFeatures/OrganizationSubscriptions/Interface/IUpdateSecretsManagerSubscriptionCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationSubscriptions/Interface/IUpdateSecretsManagerSubscriptionCommand.cs @@ -1,11 +1,9 @@ -using Bit.Core.Entities; -using Bit.Core.Models.Business; +using Bit.Core.Models.Business; namespace Bit.Core.OrganizationFeatures.OrganizationSubscriptions.Interface; public interface IUpdateSecretsManagerSubscriptionCommand { Task UpdateSubscriptionAsync(SecretsManagerSubscriptionUpdate update); - Task AdjustServiceAccountsAsync(Organization organization, int smServiceAccountsAdjustment); Task ValidateUpdate(SecretsManagerSubscriptionUpdate update); } diff --git a/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpdateSecretsManagerSubscriptionCommand.cs b/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpdateSecretsManagerSubscriptionCommand.cs index ec4b482e4b..ab96e55f70 100644 --- a/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpdateSecretsManagerSubscriptionCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationSubscriptions/UpdateSecretsManagerSubscriptionCommand.cs @@ -2,13 +2,11 @@ using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Models.Business; -using Bit.Core.Models.StaticStore; using Bit.Core.OrganizationFeatures.OrganizationSubscriptions.Interface; using Bit.Core.Repositories; using Bit.Core.SecretsManager.Repositories; using Bit.Core.Services; using Bit.Core.Settings; -using Bit.Core.Utilities; using Microsoft.Extensions.Logging; namespace Bit.Core.OrganizationFeatures.OrganizationSubscriptions; @@ -51,82 +49,46 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs { await ValidateUpdate(update); - await FinalizeSubscriptionAdjustmentAsync(update.Organization, update.Plan, update); + await FinalizeSubscriptionAdjustmentAsync(update); - await SendEmailIfAutoscaleLimitReached(update.Organization); - } - - public async Task AdjustServiceAccountsAsync(Organization organization, int smServiceAccountsAdjustment) - { - var update = new SecretsManagerSubscriptionUpdate( - organization, seatAdjustment: 0, maxAutoscaleSeats: organization?.MaxAutoscaleSmSeats, - serviceAccountAdjustment: smServiceAccountsAdjustment, maxAutoscaleServiceAccounts: organization?.MaxAutoscaleSmServiceAccounts) + if (update.SmSeatAutoscaleLimitReached) { - Autoscaling = true - }; + await SendSeatLimitEmailAsync(update.Organization); + } - await UpdateSubscriptionAsync(update); + if (update.SmServiceAccountAutoscaleLimitReached) + { + await SendServiceAccountLimitEmailAsync(update.Organization); + } } - private async Task FinalizeSubscriptionAdjustmentAsync(Organization organization, - Plan plan, SecretsManagerSubscriptionUpdate update) + private async Task FinalizeSubscriptionAdjustmentAsync(SecretsManagerSubscriptionUpdate update) { if (update.SmSeatsChanged) { - await ProcessChargesAndRaiseEventsForAdjustSeatsAsync(organization, plan, update); - organization.SmSeats = update.SmSeats; + await _paymentService.AdjustSeatsAsync(update.Organization, update.Plan, update.SmSeatsExcludingBase, update.ProrationDate); + + // TODO: call ReferenceEventService - see AC-1481 } if (update.SmServiceAccountsChanged) { - await ProcessChargesAndRaiseEventsForAdjustServiceAccountsAsync(organization, plan, update); - organization.SmServiceAccounts = update.SmServiceAccounts; + await _paymentService.AdjustServiceAccountsAsync(update.Organization, update.Plan, + update.SmServiceAccountsExcludingBase, update.ProrationDate); + + // TODO: call ReferenceEventService - see AC-1481 } - if (update.MaxAutoscaleSmSeatsChanged) - { - organization.MaxAutoscaleSmSeats = update.MaxAutoscaleSmSeats; - } - - if (update.MaxAutoscaleSmServiceAccountsChanged) - { - organization.MaxAutoscaleSmServiceAccounts = update.MaxAutoscaleSmServiceAccounts; - } + var organization = update.Organization; + organization.SmSeats = update.SmSeats; + organization.SmServiceAccounts = update.SmServiceAccounts; + organization.MaxAutoscaleSmSeats = update.MaxAutoscaleSmSeats; + organization.MaxAutoscaleSmServiceAccounts = update.MaxAutoscaleSmServiceAccounts; await ReplaceAndUpdateCacheAsync(organization); } - private async Task ProcessChargesAndRaiseEventsForAdjustSeatsAsync(Organization organization, Plan plan, - SecretsManagerSubscriptionUpdate update) - { - await _paymentService.AdjustSeatsAsync(organization, plan, update.SmSeatsExcludingBase, update.ProrationDate); - - // TODO: call ReferenceEventService - see AC-1481 - } - - private async Task ProcessChargesAndRaiseEventsForAdjustServiceAccountsAsync(Organization organization, Plan plan, - SecretsManagerSubscriptionUpdate update) - { - await _paymentService.AdjustServiceAccountsAsync(organization, plan, - update.SmServiceAccountsExcludingBase, update.ProrationDate); - - // TODO: call ReferenceEventService - see AC-1481 - } - - private async Task SendEmailIfAutoscaleLimitReached(Organization organization) - { - if (organization.SmSeats.HasValue && organization.MaxAutoscaleSmSeats.HasValue && organization.SmSeats == organization.MaxAutoscaleSmSeats) - { - await SendSeatLimitEmailAsync(organization, organization.MaxAutoscaleSmSeats.Value); - } - - if (organization.SmServiceAccounts.HasValue && organization.MaxAutoscaleSmServiceAccounts.HasValue && organization.SmServiceAccounts == organization.MaxAutoscaleSmServiceAccounts) - { - await SendServiceAccountLimitEmailAsync(organization, organization.MaxAutoscaleSmServiceAccounts.Value); - } - } - - private async Task SendSeatLimitEmailAsync(Organization organization, int MaxAutoscaleValue) + private async Task SendSeatLimitEmailAsync(Organization organization) { try { @@ -134,16 +96,16 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs OrganizationUserType.Owner)) .Select(u => u.Email).Distinct(); - await _mailService.SendSecretsManagerMaxSeatLimitReachedEmailAsync(organization, MaxAutoscaleValue, ownerEmails); + await _mailService.SendSecretsManagerMaxSeatLimitReachedEmailAsync(organization, organization.MaxAutoscaleSmSeats.Value, ownerEmails); } catch (Exception e) { - _logger.LogError(e, $"Error encountered notifying organization owners of Seats limit reached."); + _logger.LogError(e, $"Error encountered notifying organization owners of seats limit reached."); } } - private async Task SendServiceAccountLimitEmailAsync(Organization organization, int MaxAutoscaleValue) + private async Task SendServiceAccountLimitEmailAsync(Organization organization) { try { @@ -151,12 +113,12 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs OrganizationUserType.Owner)) .Select(u => u.Email).Distinct(); - await _mailService.SendSecretsManagerMaxServiceAccountLimitReachedEmailAsync(organization, MaxAutoscaleValue, ownerEmails); + await _mailService.SendSecretsManagerMaxServiceAccountLimitReachedEmailAsync(organization, organization.MaxAutoscaleSmServiceAccounts.Value, ownerEmails); } catch (Exception e) { - _logger.LogError(e, $"Error encountered notifying organization owners of Service Accounts limit reached."); + _logger.LogError(e, $"Error encountered notifying organization owners of service accounts limit reached."); } } @@ -171,46 +133,45 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs throw new BadRequestException(message); } - var organization = update.Organization; - ValidateOrganization(organization); - - var plan = GetPlanForOrganization(organization); + ValidateOrganization(update); if (update.SmSeatsChanged) { - await ValidateSmSeatsUpdateAsync(organization, update, plan); + await ValidateSmSeatsUpdateAsync(update); } if (update.SmServiceAccountsChanged) { - await ValidateSmServiceAccountsUpdateAsync(organization, update, plan); + await ValidateSmServiceAccountsUpdateAsync(update); } if (update.MaxAutoscaleSmSeatsChanged) { - ValidateMaxAutoscaleSmSeatsUpdateAsync(organization, update.MaxAutoscaleSmSeats, plan); + ValidateMaxAutoscaleSmSeatsUpdateAsync(update); } if (update.MaxAutoscaleSmServiceAccountsChanged) { - ValidateMaxAutoscaleSmServiceAccountUpdate(organization, update.MaxAutoscaleSmServiceAccounts, plan); + ValidateMaxAutoscaleSmServiceAccountUpdate(update); } } - private void ValidateOrganization(Organization organization) + private void ValidateOrganization(SecretsManagerSubscriptionUpdate update) { - if (organization == null) - { - throw new NotFoundException("Organization is not found."); - } + var organization = update.Organization; if (!organization.UseSecretsManager) { throw new BadRequestException("Organization has no access to Secrets Manager."); } - var plan = GetPlanForOrganization(organization); - if (plan.Product == ProductType.Free) + if (organization.SecretsManagerBeta) + { + throw new BadRequestException("Organization is enrolled in Secrets Manager Beta. " + + "Please contact Customer Success to add Secrets Manager to your subscription."); + } + + if (update.Plan.Product == ProductType.Free) { // No need to check the organization is set up with Stripe return; @@ -227,18 +188,11 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs } } - private Plan GetPlanForOrganization(Organization organization) + private async Task ValidateSmSeatsUpdateAsync(SecretsManagerSubscriptionUpdate update) { - var plan = StaticStore.SecretManagerPlans.FirstOrDefault(p => p.Type == organization.PlanType); - if (plan == null) - { - throw new BadRequestException("Existing plan not found."); - } - return plan; - } + var organization = update.Organization; + var plan = update.Plan; - private async Task ValidateSmSeatsUpdateAsync(Organization organization, SecretsManagerSubscriptionUpdate update, Plan plan) - { // Check if the organization has unlimited seats if (organization.SmSeats == null) { @@ -282,21 +236,24 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs // Check minimum seats currently in use by the organization if (organization.SmSeats.Value > update.SmSeats.Value) { - var currentSeats = await _organizationUserRepository.GetOccupiedSmSeatCountByOrganizationIdAsync(organization.Id); - if (currentSeats > update.SmSeats.Value) + var occupiedSeats = await _organizationUserRepository.GetOccupiedSmSeatCountByOrganizationIdAsync(organization.Id); + if (occupiedSeats > update.SmSeats.Value) { - throw new BadRequestException($"Your organization currently has {currentSeats} Secrets Manager seats. " + - $"Your plan only allows {update.SmSeats} Secrets Manager seats. Remove some Secrets Manager users."); + throw new BadRequestException($"{occupiedSeats} users are currently occupying Secrets Manager seats. " + + "You cannot decrease your subscription below your current occupied seat count."); } } } - private async Task ValidateSmServiceAccountsUpdateAsync(Organization organization, SecretsManagerSubscriptionUpdate update, Plan plan) + private async Task ValidateSmServiceAccountsUpdateAsync(SecretsManagerSubscriptionUpdate update) { + var organization = update.Organization; + var plan = update.Plan; + // Check if the organization has unlimited service accounts if (organization.SmServiceAccounts == null) { - throw new BadRequestException("Organization has no Service Accounts limit, no need to adjust Service Accounts"); + throw new BadRequestException("Organization has no service accounts limit, no need to adjust service accounts"); } if (update.Autoscaling && update.SmServiceAccounts.Value < organization.SmServiceAccounts.Value) @@ -326,13 +283,13 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs // Check minimum service accounts included with plan if (plan.BaseServiceAccount.HasValue && plan.BaseServiceAccount.Value > update.SmServiceAccounts.Value) { - throw new BadRequestException($"Plan has a minimum of {plan.BaseServiceAccount} Service Accounts."); + throw new BadRequestException($"Plan has a minimum of {plan.BaseServiceAccount} service accounts."); } // Check minimum service accounts required by business logic if (update.SmServiceAccounts.Value <= 0) { - throw new BadRequestException("You must have at least 1 Service Account."); + throw new BadRequestException("You must have at least 1 service account."); } // Check minimum service accounts currently in use by the organization @@ -341,30 +298,32 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs var currentServiceAccounts = await _serviceAccountRepository.GetServiceAccountCountByOrganizationIdAsync(organization.Id); if (currentServiceAccounts > update.SmServiceAccounts) { - throw new BadRequestException($"Your organization currently has {currentServiceAccounts} Service Accounts. " + - $"Your plan only allows {update.SmServiceAccounts} Service Accounts. Remove some Service Accounts."); + throw new BadRequestException($"Your organization currently has {currentServiceAccounts} service accounts. " + + $"You cannot decrease your subscription below your current service account usage."); } } } - private void ValidateMaxAutoscaleSmSeatsUpdateAsync(Organization organization, int? maxAutoscaleSeats, Plan plan) + private void ValidateMaxAutoscaleSmSeatsUpdateAsync(SecretsManagerSubscriptionUpdate update) { - if (!maxAutoscaleSeats.HasValue) + var plan = update.Plan; + + if (!update.MaxAutoscaleSmSeats.HasValue) { // autoscale limit has been turned off, no validation required return; } - if (organization.SmSeats.HasValue && maxAutoscaleSeats.Value < organization.SmSeats.Value) + if (update.SmSeats.HasValue && update.MaxAutoscaleSmSeats.Value < update.SmSeats.Value) { throw new BadRequestException($"Cannot set max Secrets Manager seat autoscaling below current Secrets Manager seat count."); } - if (plan.MaxUsers.HasValue && maxAutoscaleSeats.Value > plan.MaxUsers) + if (plan.MaxUsers.HasValue && update.MaxAutoscaleSmSeats.Value > plan.MaxUsers) { throw new BadRequestException(string.Concat( $"Your plan has a Secrets Manager seat limit of {plan.MaxUsers}, ", - $"but you have specified a max autoscale count of {maxAutoscaleSeats}.", + $"but you have specified a max autoscale count of {update.MaxAutoscaleSmSeats}.", "Reduce your max autoscale count.")); } @@ -374,30 +333,32 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs } } - private void ValidateMaxAutoscaleSmServiceAccountUpdate(Organization organization, int? maxAutoscaleServiceAccounts, Plan plan) + private void ValidateMaxAutoscaleSmServiceAccountUpdate(SecretsManagerSubscriptionUpdate update) { - if (!maxAutoscaleServiceAccounts.HasValue) + var plan = update.Plan; + + if (!update.MaxAutoscaleSmServiceAccounts.HasValue) { // autoscale limit has been turned off, no validation required return; } - if (organization.SmServiceAccounts.HasValue && maxAutoscaleServiceAccounts.Value < organization.SmServiceAccounts.Value) + if (update.SmServiceAccounts.HasValue && update.MaxAutoscaleSmServiceAccounts.Value < update.SmServiceAccounts.Value) { throw new BadRequestException( - $"Cannot set max Service Accounts autoscaling below current Service Accounts count."); + $"Cannot set max service accounts autoscaling below current service accounts count."); } if (!plan.AllowServiceAccountsAutoscale) { - throw new BadRequestException("Your plan does not allow Service Accounts autoscaling."); + throw new BadRequestException("Your plan does not allow service accounts autoscaling."); } - if (plan.MaxServiceAccounts.HasValue && maxAutoscaleServiceAccounts.Value > plan.MaxServiceAccounts) + if (plan.MaxServiceAccounts.HasValue && update.MaxAutoscaleSmServiceAccounts.Value > plan.MaxServiceAccounts) { throw new BadRequestException(string.Concat( - $"Your plan has a Service Accounts limit of {plan.MaxServiceAccounts}, ", - $"but you have specified a max autoscale count of {maxAutoscaleServiceAccounts}.", + $"Your plan has a service account limit of {plan.MaxServiceAccounts}, ", + $"but you have specified a max autoscale count of {update.MaxAutoscaleSmServiceAccounts}.", "Reduce your max autoscale count.")); } } diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index b27e395b0d..b7424a53e1 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -861,8 +861,8 @@ public class OrganizationService : IOrganizationService var additionalSmSeatsRequired = await _countNewSmSeatsRequiredQuery.CountNewSmSeatsRequiredAsync(organization.Id, inviteWithSmAccessCount); if (additionalSmSeatsRequired > 0) { - smSubscriptionUpdate = new SecretsManagerSubscriptionUpdate(organization, true); - smSubscriptionUpdate.AdjustSeats(additionalSmSeatsRequired); + smSubscriptionUpdate = new SecretsManagerSubscriptionUpdate(organization, true) + .AdjustSeats(additionalSmSeatsRequired); await _updateSecretsManagerSubscriptionCommand.ValidateUpdate(smSubscriptionUpdate); } @@ -1418,8 +1418,8 @@ public class OrganizationService : IOrganizationService if (additionalSmSeatsRequired > 0) { var organization = await _organizationRepository.GetByIdAsync(user.OrganizationId); - var update = new SecretsManagerSubscriptionUpdate(organization, true); - update.AdjustSeats(additionalSmSeatsRequired); + var update = new SecretsManagerSubscriptionUpdate(organization, true) + .AdjustSeats(additionalSmSeatsRequired); await _updateSecretsManagerSubscriptionCommand.UpdateSubscriptionAsync(update); } } diff --git a/test/Api.Test/SecretsManager/Controllers/ServiceAccountsControllerTests.cs b/test/Api.Test/SecretsManager/Controllers/ServiceAccountsControllerTests.cs index 6a311a4826..29dcd117c1 100644 --- a/test/Api.Test/SecretsManager/Controllers/ServiceAccountsControllerTests.cs +++ b/test/Api.Test/SecretsManager/Controllers/ServiceAccountsControllerTests.cs @@ -5,6 +5,7 @@ using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.Models.Business; using Bit.Core.OrganizationFeatures.OrganizationSubscriptions.Interface; using Bit.Core.Repositories; using Bit.Core.SecretsManager.Commands.AccessTokens.Interfaces; @@ -106,7 +107,7 @@ public class ServiceAccountsControllerTests .CreateAsync(Arg.Is(sa => sa.Name == data.Name), Arg.Any()); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() - .AdjustServiceAccountsAsync(Arg.Any(), Arg.Any()); + .UpdateSubscriptionAsync(Arg.Any()); } [Theory] @@ -124,7 +125,12 @@ public class ServiceAccountsControllerTests .CreateAsync(Arg.Is(sa => sa.Name == data.Name), Arg.Any()); await sutProvider.GetDependency().Received(1) - .AdjustServiceAccountsAsync(Arg.Is(organization), Arg.Is(newSlotsRequired)); + .UpdateSubscriptionAsync(Arg.Is(update => + update.Autoscaling == true && + update.SmServiceAccounts == organization.SmServiceAccounts + newSlotsRequired && + !update.SmSeatsChanged && + !update.MaxAutoscaleSmSeatsChanged && + !update.MaxAutoscaleSmServiceAccountsChanged)); } [Theory] diff --git a/test/Core.Test/AutoFixture/OrganizationFixtures.cs b/test/Core.Test/AutoFixture/OrganizationFixtures.cs index 8e6cfd0607..0116296d33 100644 --- a/test/Core.Test/AutoFixture/OrganizationFixtures.cs +++ b/test/Core.Test/AutoFixture/OrganizationFixtures.cs @@ -137,6 +137,7 @@ public class SecretsManagerOrganizationCustomization : ICustomization fixture.Customize(composer => composer .With(o => o.Id, organizationId) .With(o => o.UseSecretsManager, true) + .With(o => o.SecretsManagerBeta, false) .With(o => o.PlanType, planType) .With(o => o.Plan, StaticStore.GetPasswordManagerPlan(planType).Name) .With(o => o.MaxAutoscaleSmSeats, (int?)null) diff --git a/test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/AddSecretsManagerSubscriptionCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/AddSecretsManagerSubscriptionCommandTests.cs index 9d635a5afc..a09500cf67 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/AddSecretsManagerSubscriptionCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/AddSecretsManagerSubscriptionCommandTests.cs @@ -95,6 +95,38 @@ public class AddSecretsManagerSubscriptionCommandTests await VerifyDependencyNotCalledAsync(sutProvider); } + [Theory] + [BitAutoData] + public async Task SignUpAsync_ThrowsException_WhenOrganizationEnrolledInSmBeta( + SutProvider sutProvider, + Organization organization) + { + organization.UseSecretsManager = true; + organization.SecretsManagerBeta = true; + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.SignUpAsync(organization, 10, 10)); + + Assert.Contains("Organization is enrolled in Secrets Manager Beta", exception.Message); + await VerifyDependencyNotCalledAsync(sutProvider); + } + + [Theory] + [BitAutoData] + public async Task SignUpAsync_ThrowsException_WhenOrganizationAlreadyHasSecretsManager( + SutProvider sutProvider, + Organization organization) + { + organization.UseSecretsManager = true; + organization.SecretsManagerBeta = false; + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.SignUpAsync(organization, 10, 10)); + + Assert.Contains("Organization already uses Secrets Manager", exception.Message); + await VerifyDependencyNotCalledAsync(sutProvider); + } + private static async Task VerifyDependencyNotCalledAsync(SutProvider sutProvider) { await sutProvider.GetDependency().DidNotReceive() diff --git a/test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/UpdateSecretsManagerSubscriptionCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/UpdateSecretsManagerSubscriptionCommandTests.cs index f400115611..2834db335f 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/UpdateSecretsManagerSubscriptionCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/UpdateSecretsManagerSubscriptionCommandTests.cs @@ -125,8 +125,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests Organization organization, SutProvider sutProvider) { - var update = new SecretsManagerSubscriptionUpdate(organization, autoscaling); - update.AdjustSeats(2); + var update = new SecretsManagerSubscriptionUpdate(organization, autoscaling).AdjustSeats(2); sutProvider.GetDependency().SelfHosted.Returns(true); @@ -151,6 +150,23 @@ public class UpdateSecretsManagerSubscriptionCommandTests await VerifyDependencyNotCalledAsync(sutProvider); } + [Theory] + [BitAutoData] + public async Task UpdateSubscriptionAsync_OrganizationEnrolledInSmBeta_ThrowsException( + SutProvider sutProvider, + Organization organization) + { + organization.UseSecretsManager = true; + organization.SecretsManagerBeta = true; + var update = new SecretsManagerSubscriptionUpdate(organization, false); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.UpdateSubscriptionAsync(update)); + + Assert.Contains("Organization is enrolled in Secrets Manager Beta", exception.Message); + await VerifyDependencyNotCalledAsync(sutProvider); + } + [Theory] [BitAutoData(PlanType.EnterpriseAnnually)] [BitAutoData(PlanType.EnterpriseMonthly)] @@ -163,8 +179,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests { organization.PlanType = planType; organization.GatewayCustomerId = null; - var update = new SecretsManagerSubscriptionUpdate(organization, false); - update.AdjustSeats(1); + var update = new SecretsManagerSubscriptionUpdate(organization, false).AdjustSeats(1); var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSubscriptionAsync(update)); Assert.Contains("No payment method found.", exception.Message); @@ -183,8 +198,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests { organization.PlanType = planType; organization.GatewaySubscriptionId = null; - var update = new SecretsManagerSubscriptionUpdate(organization, false); - update.AdjustSeats(1); + var update = new SecretsManagerSubscriptionUpdate(organization, false).AdjustSeats(1); var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSubscriptionAsync(update)); Assert.Contains("No subscription found.", exception.Message); @@ -223,8 +237,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests var expectedSmServiceAccounts = organizationServiceAccounts + smServiceAccountsAdjustment; var expectedSmServiceAccountsExcludingBase = expectedSmServiceAccounts - plan.BaseServiceAccount.GetValueOrDefault(); - var update = new SecretsManagerSubscriptionUpdate(organization, false); - update.AdjustServiceAccounts(10); + var update = new SecretsManagerSubscriptionUpdate(organization, false).AdjustServiceAccounts(10); await sutProvider.Sut.UpdateSubscriptionAsync(update); @@ -247,7 +260,6 @@ public class UpdateSecretsManagerSubscriptionCommandTests Organization organization, SutProvider sutProvider) { - organization.SmSeats = 9; var update = new SecretsManagerSubscriptionUpdate(organization, false) { SmSeats = 10, @@ -267,8 +279,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests SutProvider sutProvider) { organization.SmSeats = null; - var update = new SecretsManagerSubscriptionUpdate(organization, false); - update.AdjustSeats(1); + var update = new SecretsManagerSubscriptionUpdate(organization, false).AdjustSeats(1); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.UpdateSubscriptionAsync(update)); @@ -283,8 +294,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests Organization organization, SutProvider sutProvider) { - var update = new SecretsManagerSubscriptionUpdate(organization, true); - update.AdjustSeats(-2); + var update = new SecretsManagerSubscriptionUpdate(organization, true).AdjustSeats(-2); var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSubscriptionAsync(update)); Assert.Contains("Cannot use autoscaling to subtract seats.", exception.Message); @@ -299,8 +309,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests SutProvider sutProvider) { organization.PlanType = planType; - var update = new SecretsManagerSubscriptionUpdate(organization, false); - update.AdjustSeats(1); + var update = new SecretsManagerSubscriptionUpdate(organization, false).AdjustSeats(1); var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSubscriptionAsync(update)); Assert.Contains("You have reached the maximum number of Secrets Manager seats (2) for this plan", @@ -317,8 +326,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests organization.SmSeats = 9; organization.MaxAutoscaleSmSeats = 10; - var update = new SecretsManagerSubscriptionUpdate(organization, true); - update.AdjustSeats(2); + var update = new SecretsManagerSubscriptionUpdate(organization, true).AdjustSeats(2); var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSubscriptionAsync(update)); Assert.Contains("Secrets Manager seat limit has been reached.", exception.Message); @@ -375,7 +383,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests sutProvider.GetDependency().GetOccupiedSmSeatCountByOrganizationIdAsync(organization.Id).Returns(8); var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSubscriptionAsync(update)); - Assert.Contains("Your organization currently has 8 Secrets Manager seats. Your plan only allows 7 Secrets Manager seats. Remove some Secrets Manager users", exception.Message); + Assert.Contains("8 users are currently occupying Secrets Manager seats. You cannot decrease your subscription below your current occupied seat count", exception.Message); await VerifyDependencyNotCalledAsync(sutProvider); } @@ -385,7 +393,6 @@ public class UpdateSecretsManagerSubscriptionCommandTests Organization organization, SutProvider sutProvider) { - organization.SmServiceAccounts = 250; var update = new SecretsManagerSubscriptionUpdate(organization, false) { SmServiceAccounts = 300, @@ -405,11 +412,10 @@ public class UpdateSecretsManagerSubscriptionCommandTests SutProvider sutProvider) { organization.SmServiceAccounts = null; - var update = new SecretsManagerSubscriptionUpdate(organization, false); - update.AdjustServiceAccounts(1); + var update = new SecretsManagerSubscriptionUpdate(organization, false).AdjustServiceAccounts(1); var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSubscriptionAsync(update)); - Assert.Contains("Organization has no Service Accounts limit, no need to adjust Service Accounts", exception.Message); + Assert.Contains("Organization has no service accounts limit, no need to adjust service accounts", exception.Message); await VerifyDependencyNotCalledAsync(sutProvider); } @@ -419,8 +425,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests Organization organization, SutProvider sutProvider) { - var update = new SecretsManagerSubscriptionUpdate(organization, true); - update.AdjustServiceAccounts(-2); + var update = new SecretsManagerSubscriptionUpdate(organization, true).AdjustServiceAccounts(-2); var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSubscriptionAsync(update)); Assert.Contains("Cannot use autoscaling to subtract service accounts.", exception.Message); @@ -435,8 +440,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests SutProvider sutProvider) { organization.PlanType = planType; - var update = new SecretsManagerSubscriptionUpdate(organization, false); - update.AdjustServiceAccounts(1); + var update = new SecretsManagerSubscriptionUpdate(organization, false).AdjustServiceAccounts(1); var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSubscriptionAsync(update)); Assert.Contains("You have reached the maximum number of service accounts (3) for this plan", @@ -453,8 +457,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests organization.SmServiceAccounts = 9; organization.MaxAutoscaleSmServiceAccounts = 10; - var update = new SecretsManagerSubscriptionUpdate(organization, true); - update.AdjustServiceAccounts(2); + var update = new SecretsManagerSubscriptionUpdate(organization, true).AdjustServiceAccounts(2); var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSubscriptionAsync(update)); Assert.Contains("Secrets Manager service account limit has been reached.", exception.Message); @@ -492,7 +495,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.UpdateSubscriptionAsync(update)); - Assert.Contains("Plan has a minimum of 200 Service Accounts", exception.Message); + Assert.Contains("Plan has a minimum of 200 service accounts", exception.Message); await VerifyDependencyNotCalledAsync(sutProvider); } @@ -516,7 +519,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests .Returns(currentServiceAccounts); var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSubscriptionAsync(update)); - Assert.Contains("Your organization currently has 301 Service Accounts. Your plan only allows 201 Service Accounts. Remove some Service Accounts", exception.Message); + Assert.Contains("Your organization currently has 301 service accounts. You cannot decrease your subscription below your current service account usage", exception.Message); await VerifyDependencyNotCalledAsync(sutProvider); } @@ -588,7 +591,7 @@ public class UpdateSecretsManagerSubscriptionCommandTests var update = new SecretsManagerSubscriptionUpdate(organization, false) { MaxAutoscaleSmServiceAccounts = 3 }; var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateSubscriptionAsync(update)); - Assert.Contains("Your plan does not allow Service Accounts autoscaling.", exception.Message); + Assert.Contains("Your plan does not allow service accounts autoscaling.", exception.Message); await VerifyDependencyNotCalledAsync(sutProvider); } From 4d59dd4a6b13ccc745f41e13497af12f2d4213f5 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Mon, 28 Aug 2023 10:20:01 +1000 Subject: [PATCH 04/18] Fix typo: CurrentContent -> CurrentContext (#3231) --- src/Core/Context/CurrentContext.cs | 36 +++++++++---------- ...ation.cs => CurrentContextOrganization.cs} | 6 ++-- ...tProvider.cs => CurrentContextProvider.cs} | 6 ++-- src/Core/Context/ICurrentContext.cs | 6 ++-- src/Core/Utilities/CoreHelpers.cs | 4 +-- .../AutoFixture/CurrentContextFixtures.cs | 2 +- test/Core.Test/Utilities/CoreHelpersTests.cs | 18 +++++----- 7 files changed, 39 insertions(+), 39 deletions(-) rename src/Core/Context/{CurrentContentOrganization.cs => CurrentContextOrganization.cs} (81%) rename src/Core/Context/{CurrentContentProvider.cs => CurrentContextProvider.cs} (77%) diff --git a/src/Core/Context/CurrentContext.cs b/src/Core/Context/CurrentContext.cs index 627e4f5845..4877a9a8f2 100644 --- a/src/Core/Context/CurrentContext.cs +++ b/src/Core/Context/CurrentContext.cs @@ -26,8 +26,8 @@ public class CurrentContext : ICurrentContext public virtual string DeviceIdentifier { get; set; } public virtual DeviceType? DeviceType { get; set; } public virtual string IpAddress { get; set; } - public virtual List Organizations { get; set; } - public virtual List Providers { get; set; } + public virtual List Organizations { get; set; } + public virtual List Providers { get; set; } public virtual Guid? InstallationId { get; set; } public virtual Guid? OrganizationId { get; set; } public virtual bool CloudflareWorkerProxied { get; set; } @@ -166,17 +166,17 @@ public class CurrentContext : ICurrentContext return Task.FromResult(0); } - private List GetOrganizations(Dictionary> claimsDict, bool orgApi) + private List GetOrganizations(Dictionary> claimsDict, bool orgApi) { var accessSecretsManager = claimsDict.ContainsKey(Claims.SecretsManagerAccess) ? claimsDict[Claims.SecretsManagerAccess].ToDictionary(s => s.Value, _ => true) : new Dictionary(); - var organizations = new List(); + var organizations = new List(); if (claimsDict.ContainsKey(Claims.OrganizationOwner)) { organizations.AddRange(claimsDict[Claims.OrganizationOwner].Select(c => - new CurrentContentOrganization + new CurrentContextOrganization { Id = new Guid(c.Value), Type = OrganizationUserType.Owner, @@ -185,7 +185,7 @@ public class CurrentContext : ICurrentContext } else if (orgApi && OrganizationId.HasValue) { - organizations.Add(new CurrentContentOrganization + organizations.Add(new CurrentContextOrganization { Id = OrganizationId.Value, Type = OrganizationUserType.Owner, @@ -195,7 +195,7 @@ public class CurrentContext : ICurrentContext if (claimsDict.ContainsKey(Claims.OrganizationAdmin)) { organizations.AddRange(claimsDict[Claims.OrganizationAdmin].Select(c => - new CurrentContentOrganization + new CurrentContextOrganization { Id = new Guid(c.Value), Type = OrganizationUserType.Admin, @@ -206,7 +206,7 @@ public class CurrentContext : ICurrentContext if (claimsDict.ContainsKey(Claims.OrganizationUser)) { organizations.AddRange(claimsDict[Claims.OrganizationUser].Select(c => - new CurrentContentOrganization + new CurrentContextOrganization { Id = new Guid(c.Value), Type = OrganizationUserType.User, @@ -217,7 +217,7 @@ public class CurrentContext : ICurrentContext if (claimsDict.ContainsKey(Claims.OrganizationManager)) { organizations.AddRange(claimsDict[Claims.OrganizationManager].Select(c => - new CurrentContentOrganization + new CurrentContextOrganization { Id = new Guid(c.Value), Type = OrganizationUserType.Manager, @@ -228,7 +228,7 @@ public class CurrentContext : ICurrentContext if (claimsDict.ContainsKey(Claims.OrganizationCustom)) { organizations.AddRange(claimsDict[Claims.OrganizationCustom].Select(c => - new CurrentContentOrganization + new CurrentContextOrganization { Id = new Guid(c.Value), Type = OrganizationUserType.Custom, @@ -240,13 +240,13 @@ public class CurrentContext : ICurrentContext return organizations; } - private List GetProviders(Dictionary> claimsDict) + private List GetProviders(Dictionary> claimsDict) { - var providers = new List(); + var providers = new List(); if (claimsDict.ContainsKey(Claims.ProviderAdmin)) { providers.AddRange(claimsDict[Claims.ProviderAdmin].Select(c => - new CurrentContentProvider + new CurrentContextProvider { Id = new Guid(c.Value), Type = ProviderUserType.ProviderAdmin @@ -256,7 +256,7 @@ public class CurrentContext : ICurrentContext if (claimsDict.ContainsKey(Claims.ProviderServiceUser)) { providers.AddRange(claimsDict[Claims.ProviderServiceUser].Select(c => - new CurrentContentProvider + new CurrentContextProvider { Id = new Guid(c.Value), Type = ProviderUserType.ServiceUser @@ -483,26 +483,26 @@ public class CurrentContext : ICurrentContext return Organizations?.Any(o => o.Id == orgId && o.AccessSecretsManager) ?? false; } - public async Task> OrganizationMembershipAsync( + public async Task> OrganizationMembershipAsync( IOrganizationUserRepository organizationUserRepository, Guid userId) { if (Organizations == null) { var userOrgs = await organizationUserRepository.GetManyDetailsByUserAsync(userId); Organizations = userOrgs.Where(ou => ou.Status == OrganizationUserStatusType.Confirmed) - .Select(ou => new CurrentContentOrganization(ou)).ToList(); + .Select(ou => new CurrentContextOrganization(ou)).ToList(); } return Organizations; } - public async Task> ProviderMembershipAsync( + public async Task> ProviderMembershipAsync( IProviderUserRepository providerUserRepository, Guid userId) { if (Providers == null) { var userProviders = await providerUserRepository.GetManyByUserAsync(userId); Providers = userProviders.Where(ou => ou.Status == ProviderUserStatusType.Confirmed) - .Select(ou => new CurrentContentProvider(ou)).ToList(); + .Select(ou => new CurrentContextProvider(ou)).ToList(); } return Providers; } diff --git a/src/Core/Context/CurrentContentOrganization.cs b/src/Core/Context/CurrentContextOrganization.cs similarity index 81% rename from src/Core/Context/CurrentContentOrganization.cs rename to src/Core/Context/CurrentContextOrganization.cs index b21598a035..cf2c8b40c7 100644 --- a/src/Core/Context/CurrentContentOrganization.cs +++ b/src/Core/Context/CurrentContextOrganization.cs @@ -5,11 +5,11 @@ using Bit.Core.Utilities; namespace Bit.Core.Context; -public class CurrentContentOrganization +public class CurrentContextOrganization { - public CurrentContentOrganization() { } + public CurrentContextOrganization() { } - public CurrentContentOrganization(OrganizationUserOrganizationDetails orgUser) + public CurrentContextOrganization(OrganizationUserOrganizationDetails orgUser) { Id = orgUser.OrganizationId; Type = orgUser.Type; diff --git a/src/Core/Context/CurrentContentProvider.cs b/src/Core/Context/CurrentContextProvider.cs similarity index 77% rename from src/Core/Context/CurrentContentProvider.cs rename to src/Core/Context/CurrentContextProvider.cs index f089be7b8a..57792840cc 100644 --- a/src/Core/Context/CurrentContentProvider.cs +++ b/src/Core/Context/CurrentContextProvider.cs @@ -5,11 +5,11 @@ using Bit.Core.Utilities; namespace Bit.Core.Context; -public class CurrentContentProvider +public class CurrentContextProvider { - public CurrentContentProvider() { } + public CurrentContextProvider() { } - public CurrentContentProvider(ProviderUser providerUser) + public CurrentContextProvider(ProviderUser providerUser) { Id = providerUser.ProviderId; Type = providerUser.Type; diff --git a/src/Core/Context/ICurrentContext.cs b/src/Core/Context/ICurrentContext.cs index 76a71e01a7..c2e362d435 100644 --- a/src/Core/Context/ICurrentContext.cs +++ b/src/Core/Context/ICurrentContext.cs @@ -16,7 +16,7 @@ public interface ICurrentContext string DeviceIdentifier { get; set; } DeviceType? DeviceType { get; set; } string IpAddress { get; set; } - List Organizations { get; set; } + List Organizations { get; set; } Guid? InstallationId { get; set; } Guid? OrganizationId { get; set; } ClientType ClientType { get; set; } @@ -64,10 +64,10 @@ public interface ICurrentContext bool AccessProviderOrganizations(Guid providerId); bool ManageProviderOrganizations(Guid providerId); - Task> OrganizationMembershipAsync( + Task> OrganizationMembershipAsync( IOrganizationUserRepository organizationUserRepository, Guid userId); - Task> ProviderMembershipAsync( + Task> ProviderMembershipAsync( IProviderUserRepository providerUserRepository, Guid userId); Task ProviderIdForOrg(Guid orgId); diff --git a/src/Core/Utilities/CoreHelpers.cs b/src/Core/Utilities/CoreHelpers.cs index f0c3d1833a..66ff08c07e 100644 --- a/src/Core/Utilities/CoreHelpers.cs +++ b/src/Core/Utilities/CoreHelpers.cs @@ -624,8 +624,8 @@ public static class CoreHelpers return configDict; } - public static List> BuildIdentityClaims(User user, ICollection orgs, - ICollection providers, bool isPremium) + public static List> BuildIdentityClaims(User user, ICollection orgs, + ICollection providers, bool isPremium) { var claims = new List>() { diff --git a/test/Core.Test/AutoFixture/CurrentContextFixtures.cs b/test/Core.Test/AutoFixture/CurrentContextFixtures.cs index 0f6c3ee950..a7b2f6d55b 100644 --- a/test/Core.Test/AutoFixture/CurrentContextFixtures.cs +++ b/test/Core.Test/AutoFixture/CurrentContextFixtures.cs @@ -32,7 +32,7 @@ internal class CurrentContextBuilder : ISpecimenBuilder } var obj = new Fixture().WithAutoNSubstitutions().Create(); - obj.Organizations = context.Create>(); + obj.Organizations = context.Create>(); return obj; } } diff --git a/test/Core.Test/Utilities/CoreHelpersTests.cs b/test/Core.Test/Utilities/CoreHelpersTests.cs index f66f9ca01a..b4a4034d93 100644 --- a/test/Core.Test/Utilities/CoreHelpersTests.cs +++ b/test/Core.Test/Utilities/CoreHelpersTests.cs @@ -273,8 +273,8 @@ public class CoreHelpersTests { "sstamp", user.SecurityStamp }, }.ToList(); - var actual = CoreHelpers.BuildIdentityClaims(user, Array.Empty(), - Array.Empty(), isPremium); + var actual = CoreHelpers.BuildIdentityClaims(user, Array.Empty(), + Array.Empty(), isPremium); foreach (var claim in expected) { @@ -289,23 +289,23 @@ public class CoreHelpersTests var fixture = new Fixture().WithAutoNSubstitutions(); foreach (var organizationUserType in Enum.GetValues().Except(new[] { OrganizationUserType.Custom })) { - var org = fixture.Create(); + var org = fixture.Create(); org.Type = organizationUserType; var expected = new KeyValuePair($"org{organizationUserType.ToString().ToLower()}", org.Id.ToString()); - var actual = CoreHelpers.BuildIdentityClaims(user, new[] { org }, Array.Empty(), false); + var actual = CoreHelpers.BuildIdentityClaims(user, new[] { org }, Array.Empty(), false); Assert.Contains(expected, actual); } } [Theory, BitAutoData, UserCustomize] - public void BuildIdentityClaims_CustomOrganizationUserClaims_Success(User user, CurrentContentOrganization org) + public void BuildIdentityClaims_CustomOrganizationUserClaims_Success(User user, CurrentContextOrganization org) { var fixture = new Fixture().WithAutoNSubstitutions(); org.Type = OrganizationUserType.Custom; - var actual = CoreHelpers.BuildIdentityClaims(user, new[] { org }, Array.Empty(), false); + var actual = CoreHelpers.BuildIdentityClaims(user, new[] { org }, Array.Empty(), false); foreach (var (permitted, claimName) in org.Permissions.ClaimsMap) { var claim = new KeyValuePair(claimName, org.Id.ToString()); @@ -325,10 +325,10 @@ public class CoreHelpersTests public void BuildIdentityClaims_ProviderClaims_Success(User user) { var fixture = new Fixture().WithAutoNSubstitutions(); - var providers = new List(); + var providers = new List(); foreach (var providerUserType in Enum.GetValues()) { - var provider = fixture.Create(); + var provider = fixture.Create(); provider.Type = providerUserType; providers.Add(provider); } @@ -357,7 +357,7 @@ public class CoreHelpersTests } } - var actual = CoreHelpers.BuildIdentityClaims(user, Array.Empty(), providers, false); + var actual = CoreHelpers.BuildIdentityClaims(user, Array.Empty(), providers, false); foreach (var claim in claims) { Assert.Contains(claim, actual); From 8de9f4d10dab36b2f1c95247701f2af5e44310ac Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Mon, 28 Aug 2023 17:01:00 +1000 Subject: [PATCH 05/18] Fix flaky OrganizationService test (#3232) --- test/Core.Test/Services/OrganizationServiceTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Core.Test/Services/OrganizationServiceTests.cs b/test/Core.Test/Services/OrganizationServiceTests.cs index 07548a16d8..01073f4d2e 100644 --- a/test/Core.Test/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/Services/OrganizationServiceTests.cs @@ -648,6 +648,7 @@ public class OrganizationServiceTests [OrganizationUser(type: OrganizationUserType.Owner, status: OrganizationUserStatusType.Confirmed)] OrganizationUser savingUser, SutProvider sutProvider) { + organization.PlanType = PlanType.EnterpriseAnnually; InviteUserHelper_ArrangeValidPermissions(organization, savingUser, sutProvider); // Set up some invites to grant access to SM From fae4d3ca1b06336b74b0c1ca71bf41e283b56803 Mon Sep 17 00:00:00 2001 From: Conner Turnbull <133619638+cturnbull-bitwarden@users.noreply.github.com> Date: Mon, 28 Aug 2023 09:22:07 -0400 Subject: [PATCH 06/18] [AC-1571] Handle null reference exception in stripe controller (#3147) * Added null checks when getting customer metadata * Added additional logging around paypal payments * Refactor region validation in StripeController * Update region retrieval method in StripeController Refactored the method GetCustomerRegionFromMetadata in StripeController. Previously, it returned null in case of nonexisting region key. Now, it checks all keys with case-insensitive comparison, and if no "region" key is found, it defaults to "US". This was done to handle cases where the region key might not be properly formatted or missing. * Updated switch expression to be switch statement * Updated new log to not log user input --- src/Billing/Controllers/StripeController.cs | 163 +++++++++++++------- 1 file changed, 106 insertions(+), 57 deletions(-) diff --git a/src/Billing/Controllers/StripeController.cs b/src/Billing/Controllers/StripeController.cs index 88f3f9e8a3..4a15541a17 100644 --- a/src/Billing/Controllers/StripeController.cs +++ b/src/Billing/Controllers/StripeController.cs @@ -10,12 +10,18 @@ using Bit.Core.Tools.Enums; using Bit.Core.Tools.Models.Business; using Bit.Core.Tools.Services; using Bit.Core.Utilities; +using Braintree; +using Braintree.Exceptions; using Microsoft.AspNetCore.Mvc; using Microsoft.Data.SqlClient; using Microsoft.Extensions.Options; using Stripe; +using Customer = Stripe.Customer; using Event = Stripe.Event; +using Subscription = Stripe.Subscription; using TaxRate = Bit.Core.Entities.TaxRate; +using Transaction = Bit.Core.Entities.Transaction; +using TransactionType = Bit.Core.Enums.TransactionType; namespace Bit.Billing.Controllers; @@ -490,60 +496,87 @@ public class StripeController : Controller /// private async Task ValidateCloudRegionAsync(Event parsedEvent) { - string customerRegion; - var serverRegion = _globalSettings.BaseServiceUri.CloudRegion; var eventType = parsedEvent.Type; + var expandOptions = new List { "customer" }; - switch (eventType) + try { - case HandledStripeWebhook.SubscriptionDeleted: - case HandledStripeWebhook.SubscriptionUpdated: - { - var subscription = await GetSubscriptionAsync(parsedEvent, true, new List { "customer" }); - customerRegion = GetCustomerRegionFromMetadata(subscription.Customer.Metadata); + Dictionary customerMetadata; + switch (eventType) + { + case HandledStripeWebhook.SubscriptionDeleted: + case HandledStripeWebhook.SubscriptionUpdated: + customerMetadata = (await GetSubscriptionAsync(parsedEvent, true, expandOptions))?.Customer + ?.Metadata; break; - } - case HandledStripeWebhook.ChargeSucceeded: - case HandledStripeWebhook.ChargeRefunded: - { - var charge = await GetChargeAsync(parsedEvent, true, new List { "customer" }); - customerRegion = GetCustomerRegionFromMetadata(charge.Customer.Metadata); + case HandledStripeWebhook.ChargeSucceeded: + case HandledStripeWebhook.ChargeRefunded: + customerMetadata = (await GetChargeAsync(parsedEvent, true, expandOptions))?.Customer?.Metadata; break; - } - case HandledStripeWebhook.UpcomingInvoice: - var eventInvoice = await GetInvoiceAsync(parsedEvent); - var customer = await GetCustomerAsync(eventInvoice.CustomerId); - customerRegion = GetCustomerRegionFromMetadata(customer.Metadata); - break; - case HandledStripeWebhook.PaymentSucceeded: - case HandledStripeWebhook.PaymentFailed: - case HandledStripeWebhook.InvoiceCreated: - { - var invoice = await GetInvoiceAsync(parsedEvent, true, new List { "customer" }); - customerRegion = GetCustomerRegionFromMetadata(invoice.Customer.Metadata); + case HandledStripeWebhook.UpcomingInvoice: + customerMetadata = (await GetInvoiceAsync(parsedEvent))?.Customer?.Metadata; break; - } - default: - { - // For all Stripe events that we're not listening to, just return 200 - return false; - } - } + case HandledStripeWebhook.PaymentSucceeded: + case HandledStripeWebhook.PaymentFailed: + case HandledStripeWebhook.InvoiceCreated: + customerMetadata = (await GetInvoiceAsync(parsedEvent, true, expandOptions))?.Customer?.Metadata; + break; + default: + customerMetadata = null; + break; + } - return customerRegion == serverRegion; + if (customerMetadata is null) + { + return false; + } + + var customerRegion = GetCustomerRegionFromMetadata(customerMetadata); + + return customerRegion == serverRegion; + } + catch (Exception e) + { + _logger.LogError(e, "Encountered unexpected error while validating cloud region"); + throw; + } } /// - /// Gets the region from the customer metadata. If no region is present, defaults to "US" + /// Gets the customer's region from the metadata. /// - /// - /// - private static string GetCustomerRegionFromMetadata(Dictionary customerMetadata) + /// The metadata of the customer. + /// The region of the customer. If the region is not specified, it returns "US", if metadata is null, + /// it returns null. It is case insensitive. + private static string GetCustomerRegionFromMetadata(IDictionary customerMetadata) { - return customerMetadata.TryGetValue("region", out var value) - ? value - : "US"; + const string defaultRegion = "US"; + + if (customerMetadata is null) + { + return null; + } + + if (customerMetadata.TryGetValue("region", out var value)) + { + return value; + } + + var miscasedRegionKey = customerMetadata.Keys + .FirstOrDefault(key => + key.Equals("region", StringComparison.OrdinalIgnoreCase)); + + if (miscasedRegionKey is null) + { + return defaultRegion; + } + + _ = customerMetadata.TryGetValue(miscasedRegionKey, out var regionValue); + + return !string.IsNullOrWhiteSpace(regionValue) + ? regionValue + : defaultRegion; } private Tuple GetIdsFromMetaData(IDictionary metaData) @@ -708,8 +741,11 @@ public class StripeController : Controller private async Task AttemptToPayInvoiceWithBraintreeAsync(Invoice invoice, Customer customer) { + _logger.LogDebug("Attempting to pay invoice with Braintree"); if (!customer?.Metadata?.ContainsKey("btCustomerId") ?? true) { + _logger.LogWarning( + "Attempted to pay invoice with Braintree but btCustomerId wasn't on Stripe customer metadata"); return false; } @@ -718,6 +754,8 @@ public class StripeController : Controller var ids = GetIdsFromMetaData(subscription?.Metadata); if (!ids.Item1.HasValue && !ids.Item2.HasValue) { + _logger.LogWarning( + "Attempted to pay invoice with Braintree but Stripe subscription metadata didn't contain either a organizationId or userId"); return false; } @@ -740,25 +778,36 @@ public class StripeController : Controller return false; } - var transactionResult = await _btGateway.Transaction.SaleAsync( - new Braintree.TransactionRequest - { - Amount = btInvoiceAmount, - CustomerId = customer.Metadata["btCustomerId"], - Options = new Braintree.TransactionOptionsRequest + Result transactionResult; + try + { + transactionResult = await _btGateway.Transaction.SaleAsync( + new Braintree.TransactionRequest { - SubmitForSettlement = true, - PayPal = new Braintree.TransactionOptionsPayPalRequest + Amount = btInvoiceAmount, + CustomerId = customer.Metadata["btCustomerId"], + Options = new Braintree.TransactionOptionsRequest { - CustomField = $"{btObjIdField}:{btObjId},region:{_globalSettings.BaseServiceUri.CloudRegion}" + SubmitForSettlement = true, + PayPal = new Braintree.TransactionOptionsPayPalRequest + { + CustomField = + $"{btObjIdField}:{btObjId},region:{_globalSettings.BaseServiceUri.CloudRegion}" + } + }, + CustomFields = new Dictionary + { + [btObjIdField] = btObjId.ToString(), + ["region"] = _globalSettings.BaseServiceUri.CloudRegion } - }, - CustomFields = new Dictionary - { - [btObjIdField] = btObjId.ToString(), - ["region"] = _globalSettings.BaseServiceUri.CloudRegion - } - }); + }); + } + catch (NotFoundException e) + { + _logger.LogError(e, + "Attempted to make a payment with Braintree, but customer did not exist for the given btCustomerId present on the Stripe metadata"); + throw; + } if (!transactionResult.IsSuccess()) { From 8eee9b330d828e0227c29f005a696102f8104ce7 Mon Sep 17 00:00:00 2001 From: Conner Turnbull <133619638+cturnbull-bitwarden@users.noreply.github.com> Date: Mon, 28 Aug 2023 09:56:50 -0400 Subject: [PATCH 07/18] [AC-1223][AC-1184] Failed Renewals (#3158) * Added null checks when getting customer metadata * Added additional logging around paypal payments * Refactor region validation in StripeController * Update region retrieval method in StripeController Refactored the method GetCustomerRegionFromMetadata in StripeController. Previously, it returned null in case of nonexisting region key. Now, it checks all keys with case-insensitive comparison, and if no "region" key is found, it defaults to "US". This was done to handle cases where the region key might not be properly formatted or missing. * Updated switch expression to be switch statement * Updated new log to not log user input * Add handling for 'payment_method.attached' webhook * Cancelling unpaid premium subscriptions * Update hardcoded Stripe status strings to constants * Updated expand string to use snake_case * Removed unnecessary comments --- src/Billing/Constants/HandledStripeWebhook.cs | 1 + src/Billing/Constants/StripeInvoiceStatus.cs | 10 + .../Constants/StripeSubscriptionStatus.cs | 13 ++ src/Billing/Controllers/StripeController.cs | 171 ++++++++++++++++-- 4 files changed, 179 insertions(+), 16 deletions(-) create mode 100644 src/Billing/Constants/StripeInvoiceStatus.cs create mode 100644 src/Billing/Constants/StripeSubscriptionStatus.cs diff --git a/src/Billing/Constants/HandledStripeWebhook.cs b/src/Billing/Constants/HandledStripeWebhook.cs index f7baa4675a..7b894a295e 100644 --- a/src/Billing/Constants/HandledStripeWebhook.cs +++ b/src/Billing/Constants/HandledStripeWebhook.cs @@ -10,4 +10,5 @@ public static class HandledStripeWebhook public const string PaymentSucceeded = "invoice.payment_succeeded"; public const string PaymentFailed = "invoice.payment_failed"; public const string InvoiceCreated = "invoice.created"; + public const string PaymentMethodAttached = "payment_method.attached"; } diff --git a/src/Billing/Constants/StripeInvoiceStatus.cs b/src/Billing/Constants/StripeInvoiceStatus.cs new file mode 100644 index 0000000000..82d286d8a2 --- /dev/null +++ b/src/Billing/Constants/StripeInvoiceStatus.cs @@ -0,0 +1,10 @@ +namespace Bit.Billing.Constants; + +public static class StripeInvoiceStatus +{ + public const string Draft = "draft"; + public const string Open = "open"; + public const string Paid = "paid"; + public const string Void = "void"; + public const string Uncollectible = "uncollectible"; +} diff --git a/src/Billing/Constants/StripeSubscriptionStatus.cs b/src/Billing/Constants/StripeSubscriptionStatus.cs new file mode 100644 index 0000000000..4589b50051 --- /dev/null +++ b/src/Billing/Constants/StripeSubscriptionStatus.cs @@ -0,0 +1,13 @@ +namespace Bit.Billing.Constants; + +public static class StripeSubscriptionStatus +{ + public const string Trialing = "trialing"; + public const string Active = "active"; + public const string Incomplete = "incomplete"; + public const string IncompleteExpired = "incomplete_expired"; + public const string PastDue = "past_due"; + public const string Canceled = "canceled"; + public const string Unpaid = "unpaid"; + public const string Paused = "paused"; +} diff --git a/src/Billing/Controllers/StripeController.cs b/src/Billing/Controllers/StripeController.cs index 4a15541a17..00a8fa5ac6 100644 --- a/src/Billing/Controllers/StripeController.cs +++ b/src/Billing/Controllers/StripeController.cs @@ -18,6 +18,7 @@ using Microsoft.Extensions.Options; using Stripe; using Customer = Stripe.Customer; using Event = Stripe.Event; +using PaymentMethod = Stripe.PaymentMethod; using Subscription = Stripe.Subscription; using TaxRate = Bit.Core.Entities.TaxRate; using Transaction = Bit.Core.Entities.Transaction; @@ -138,10 +139,10 @@ public class StripeController : Controller var ids = GetIdsFromMetaData(subscription.Metadata); var organizationId = ids.Item1 ?? Guid.Empty; var userId = ids.Item2 ?? Guid.Empty; - var subCanceled = subDeleted && subscription.Status == "canceled"; - var subUnpaid = subUpdated && subscription.Status == "unpaid"; - var subActive = subUpdated && subscription.Status == "active"; - var subIncompleteExpired = subUpdated && subscription.Status == "incomplete_expired"; + var subCanceled = subDeleted && subscription.Status == StripeSubscriptionStatus.Canceled; + var subUnpaid = subUpdated && subscription.Status == StripeSubscriptionStatus.Unpaid; + var subActive = subUpdated && subscription.Status == StripeSubscriptionStatus.Active; + var subIncompleteExpired = subUpdated && subscription.Status == StripeSubscriptionStatus.IncompleteExpired; if (subCanceled || subUnpaid || subIncompleteExpired) { @@ -153,7 +154,17 @@ public class StripeController : Controller // user else if (userId != Guid.Empty) { - await _userService.DisablePremiumAsync(userId, subscription.CurrentPeriodEnd); + if (subUnpaid && subscription.Items.Any(i => i.Price.Id is PremiumPlanId or PremiumPlanIdAppStore)) + { + await CancelSubscription(subscription.Id); + await VoidOpenInvoices(subscription.Id); + } + + var user = await _userService.GetUserByIdAsync(userId); + if (user.Premium) + { + await _userService.DisablePremiumAsync(userId, subscription.CurrentPeriodEnd); + } } } @@ -271,7 +282,7 @@ public class StripeController : Controller }); foreach (var sub in subscriptions) { - if (sub.Status != "canceled" && sub.Status != "incomplete_expired") + if (sub.Status != StripeSubscriptionStatus.Canceled && sub.Status != StripeSubscriptionStatus.IncompleteExpired) { ids = GetIdsFromMetaData(sub.Metadata); if (ids.Item1.HasValue || ids.Item2.HasValue) @@ -421,7 +432,7 @@ public class StripeController : Controller { var subscriptionService = new SubscriptionService(); var subscription = await subscriptionService.GetAsync(invoice.SubscriptionId); - if (subscription?.Status == "active") + if (subscription?.Status == StripeSubscriptionStatus.Active) { if (DateTime.UtcNow - invoice.Created < TimeSpan.FromMinutes(1)) { @@ -478,6 +489,11 @@ public class StripeController : Controller await AttemptToPayInvoiceAsync(invoice); } } + else if (parsedEvent.Type.Equals(HandledStripeWebhook.PaymentMethodAttached)) + { + var paymentMethod = await GetPaymentMethodAsync(parsedEvent); + await HandlePaymentMethodAttachedAsync(paymentMethod); + } else { _logger.LogWarning("Unsupported event received. " + parsedEvent.Type); @@ -522,6 +538,11 @@ public class StripeController : Controller case HandledStripeWebhook.InvoiceCreated: customerMetadata = (await GetInvoiceAsync(parsedEvent, true, expandOptions))?.Customer?.Metadata; break; + case HandledStripeWebhook.PaymentMethodAttached: + customerMetadata = (await GetPaymentMethodAsync(parsedEvent, true, expandOptions)) + ?.Customer + ?.Metadata; + break; default: customerMetadata = null; break; @@ -579,6 +600,77 @@ public class StripeController : Controller : defaultRegion; } + private async Task HandlePaymentMethodAttachedAsync(PaymentMethod paymentMethod) + { + if (paymentMethod is null) + { + _logger.LogWarning("Attempted to handle the event payment_method.attached but paymentMethod was null"); + return; + } + + var subscriptionService = new SubscriptionService(); + var subscriptionListOptions = new SubscriptionListOptions + { + Customer = paymentMethod.CustomerId, + Status = StripeSubscriptionStatus.Unpaid, + Expand = new List { "data.latest_invoice" } + }; + + StripeList unpaidSubscriptions; + try + { + unpaidSubscriptions = await subscriptionService.ListAsync(subscriptionListOptions); + } + catch (Exception e) + { + _logger.LogError(e, + "Attempted to get unpaid invoices for customer {CustomerId} but encountered an error while calling Stripe", + paymentMethod.CustomerId); + + return; + } + + foreach (var unpaidSubscription in unpaidSubscriptions) + { + await AttemptToPayOpenSubscriptionAsync(unpaidSubscription); + } + } + + private async Task AttemptToPayOpenSubscriptionAsync(Subscription unpaidSubscription) + { + var latestInvoice = unpaidSubscription.LatestInvoice; + + if (unpaidSubscription.LatestInvoice is null) + { + _logger.LogWarning( + "Attempted to pay unpaid subscription {SubscriptionId} but latest invoice didn't exist", + unpaidSubscription.Id); + + return; + } + + if (latestInvoice.Status != StripeInvoiceStatus.Open) + { + _logger.LogWarning( + "Attempted to pay unpaid subscription {SubscriptionId} but latest invoice wasn't \"open\"", + unpaidSubscription.Id); + + return; + } + + try + { + await AttemptToPayInvoiceAsync(latestInvoice, true); + } + catch (Exception e) + { + _logger.LogError(e, + "Attempted to pay open invoice {InvoiceId} on unpaid subscription {SubscriptionId} but encountered an error", + latestInvoice.Id, unpaidSubscription.Id); + throw; + } + } + private Tuple GetIdsFromMetaData(IDictionary metaData) { if (metaData == null || !metaData.Any()) @@ -631,7 +723,7 @@ public class StripeController : Controller } } - private async Task AttemptToPayInvoiceAsync(Invoice invoice) + private async Task AttemptToPayInvoiceAsync(Invoice invoice, bool attemptToPayWithStripe = false) { var customerService = new CustomerService(); var customer = await customerService.GetAsync(invoice.CustomerId); @@ -639,10 +731,17 @@ public class StripeController : Controller { return await AttemptToPayInvoiceWithAppleReceiptAsync(invoice, customer); } - else if (customer?.Metadata?.ContainsKey("btCustomerId") ?? false) + + if (customer?.Metadata?.ContainsKey("btCustomerId") ?? false) { return await AttemptToPayInvoiceWithBraintreeAsync(invoice, customer); } + + if (attemptToPayWithStripe) + { + return await AttemptToPayInvoiceWithStripeAsync(invoice); + } + return false; } @@ -851,6 +950,25 @@ public class StripeController : Controller return true; } + private async Task AttemptToPayInvoiceWithStripeAsync(Invoice invoice) + { + try + { + var invoiceService = new InvoiceService(); + await invoiceService.PayAsync(invoice.Id); + return true; + } + catch (Exception e) + { + _logger.LogWarning( + e, + "Exception occurred while trying to pay Stripe invoice with Id: {InvoiceId}", + invoice.Id); + + throw; + } + } + private bool UnpaidAutoChargeInvoiceForSubscriptionCycle(Invoice invoice) { return invoice.AmountDue > 0 && !invoice.Paid && invoice.CollectionMethod == "charge_automatically" && @@ -935,6 +1053,31 @@ public class StripeController : Controller return customer; } + private async Task GetPaymentMethodAsync(Event parsedEvent, bool fresh = false, + List expandOptions = null) + { + if (parsedEvent.Data.Object is not PaymentMethod eventPaymentMethod) + { + throw new Exception("Invoice is null (from parsed event). " + parsedEvent.Id); + } + + if (!fresh) + { + return eventPaymentMethod; + } + + var paymentMethodService = new PaymentMethodService(); + var paymentMethodGetOptions = new PaymentMethodGetOptions { Expand = expandOptions }; + var paymentMethod = await paymentMethodService.GetAsync(eventPaymentMethod.Id, paymentMethodGetOptions); + + if (paymentMethod == null) + { + throw new Exception($"Payment method is null. {eventPaymentMethod.Id}"); + } + + return paymentMethod; + } + private async Task VerifyCorrectTaxRateForCharge(Invoice invoice, Subscription subscription) { if (!string.IsNullOrWhiteSpace(invoice?.CustomerAddress?.Country) && !string.IsNullOrWhiteSpace(invoice?.CustomerAddress?.PostalCode)) @@ -971,12 +1114,8 @@ public class StripeController : Controller var subscriptionService = new SubscriptionService(); var subscription = await subscriptionService.GetAsync(invoice.SubscriptionId); // attempt count 4 = 11 days after initial failure - if (invoice.AttemptCount > 3 && subscription.Items.Any(i => i.Price.Id == PremiumPlanId || i.Price.Id == PremiumPlanIdAppStore)) - { - await CancelSubscription(invoice.SubscriptionId); - await VoidOpenInvoices(invoice.SubscriptionId); - } - else + if (invoice.AttemptCount <= 3 || + !subscription.Items.Any(i => i.Price.Id is PremiumPlanId or PremiumPlanIdAppStore)) { await AttemptToPayInvoiceAsync(invoice); } @@ -993,7 +1132,7 @@ public class StripeController : Controller var invoiceService = new InvoiceService(); var options = new InvoiceListOptions { - Status = "open", + Status = StripeInvoiceStatus.Open, Subscription = subscriptionId }; var invoices = invoiceService.List(options); From 640cb68d517fdea2ff542b5486c9b829d03723c9 Mon Sep 17 00:00:00 2001 From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> Date: Mon, 28 Aug 2023 10:16:50 -0500 Subject: [PATCH 08/18] [SM-863] Add endpoint for fetching multiple secrets by IDs (#3134) * Add support CanReadSecret authorization * Extract base response model for secret * Add support for SA bulk fetching event logging * secret repository bug fix * Add endpoint and request for bulk fetching secrets * Swap to original reference event * Add unit tests * Add integration tests * Add unit tests for authz handler * update authz handler tests --------- --- .../Secrets/SecretAuthorizationHandler.cs | 15 +++ .../Repositories/SecretRepository.cs | 3 +- .../SecretAuthorizationHandlerTests.cs | 63 +++++++++++ .../Controllers/SecretsController.cs | 41 +++++++ .../Models/Request/GetSecretsRequestModel.cs | 9 ++ .../Response/BaseSecretResponseModel.cs | 66 ++++++++++++ .../Models/Response/SecretResponseModel.cs | 53 +-------- .../SecretOperationRequirement.cs | 1 + src/Core/Services/IEventService.cs | 1 + .../Services/Implementations/EventService.cs | 34 ++++-- .../NoopImplementations/NoopEventService.cs | 6 ++ .../Controllers/SecretsControllerTests.cs | 63 +++++++++++ .../Controllers/SecretsControllerTests.cs | 101 ++++++++++++++++++ 13 files changed, 394 insertions(+), 62 deletions(-) create mode 100644 src/Api/SecretsManager/Models/Request/GetSecretsRequestModel.cs create mode 100644 src/Api/SecretsManager/Models/Response/BaseSecretResponseModel.cs diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandler.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandler.cs index 262d431fb3..92461e61a9 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandler.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandler.cs @@ -38,6 +38,9 @@ public class SecretAuthorizationHandler : AuthorizationHandler(s), Read = true, - Write = false, + Write = s.Projects.Any(p => + p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == userId && ap.Write)), }), _ => throw new ArgumentOutOfRangeException(nameof(accessType), accessType, null), }; diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandlerTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandlerTests.cs index d7e49fb46d..f1737e0ad4 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandlerTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandlerTests.cs @@ -232,6 +232,69 @@ public class SecretAuthorizationHandlerTests Assert.True(authzContext.HasSucceeded); } + [Theory] + [BitAutoData] + public async Task CanReadSecret_AccessToSecretsManagerFalse_DoesNotSucceed( + SutProvider sutProvider, Secret secret, + ClaimsPrincipal claimsPrincipal) + { + var requirement = SecretOperations.Read; + sutProvider.GetDependency().AccessSecretsManager(secret.OrganizationId) + .Returns(false); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task CanReadSecret_NullResource_DoesNotSucceed( + SutProvider sutProvider, Secret secret, + ClaimsPrincipal claimsPrincipal, + Guid userId) + { + var requirement = SecretOperations.Read; + SetupPermission(sutProvider, PermissionType.RunAsAdmin, secret.OrganizationId, userId); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, null); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(PermissionType.RunAsAdmin, true, true, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, false, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, true, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, false, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, true, true)] + [BitAutoData(PermissionType.RunAsServiceAccountWithPermission, false, false, false)] + [BitAutoData(PermissionType.RunAsServiceAccountWithPermission, false, true, false)] + [BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, false, true)] + [BitAutoData(PermissionType.RunAsServiceAccountWithPermission, true, true, true)] + public async Task CanReadSecret_AccessCheck(PermissionType permissionType, bool read, bool write, + bool expected, + SutProvider sutProvider, Secret secret, + ClaimsPrincipal claimsPrincipal, + Guid userId) + { + var requirement = SecretOperations.Read; + SetupPermission(sutProvider, permissionType, secret.OrganizationId, userId); + sutProvider.GetDependency() + .AccessToSecretAsync(secret.Id, userId, Arg.Any()) + .Returns((read, write)); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, secret); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.Equal(expected, authzContext.HasSucceeded); + } + [Theory] [BitAutoData] public async Task CanUpdateSecret_AccessToSecretsManagerFalse_DoesNotSucceed( diff --git a/src/Api/SecretsManager/Controllers/SecretsController.cs b/src/Api/SecretsManager/Controllers/SecretsController.cs index 99f641a172..a88a784bed 100644 --- a/src/Api/SecretsManager/Controllers/SecretsController.cs +++ b/src/Api/SecretsManager/Controllers/SecretsController.cs @@ -207,4 +207,45 @@ public class SecretsController : Controller var responses = results.Select(r => new BulkDeleteResponseModel(r.Secret.Id, r.Error)); return new ListResponseModel(responses); } + + [HttpPost("secrets/get-by-ids")] + public async Task> GetSecretsByIdsAsync( + [FromBody] GetSecretsRequestModel request) + { + var secrets = (await _secretRepository.GetManyByIds(request.Ids)).ToList(); + if (!secrets.Any() || secrets.Count != request.Ids.Count()) + { + throw new NotFoundException(); + } + + // Ensure all secrets belong to the same organization. + var organizationId = secrets.First().OrganizationId; + if (secrets.Any(secret => secret.OrganizationId != organizationId) || + !_currentContext.AccessSecretsManager(organizationId)) + { + throw new NotFoundException(); + } + + + foreach (var secret in secrets) + { + var authorizationResult = await _authorizationService.AuthorizeAsync(User, secret, SecretOperations.Read); + if (!authorizationResult.Succeeded) + { + throw new NotFoundException(); + } + } + + if (_currentContext.ClientType == ClientType.ServiceAccount) + { + var userId = _userService.GetProperUserId(User).Value; + var org = await _organizationRepository.GetByIdAsync(organizationId); + await _eventService.LogServiceAccountSecretsEventAsync(userId, secrets, EventType.Secret_Retrieved); + await _referenceEventService.RaiseEventAsync( + new ReferenceEvent(ReferenceEventType.SmServiceAccountAccessedSecret, org, _currentContext)); + } + + var responses = secrets.Select(s => new BaseSecretResponseModel(s)); + return new ListResponseModel(responses); + } } diff --git a/src/Api/SecretsManager/Models/Request/GetSecretsRequestModel.cs b/src/Api/SecretsManager/Models/Request/GetSecretsRequestModel.cs new file mode 100644 index 0000000000..42dbce5232 --- /dev/null +++ b/src/Api/SecretsManager/Models/Request/GetSecretsRequestModel.cs @@ -0,0 +1,9 @@ +using System.ComponentModel.DataAnnotations; + +namespace Bit.Api.SecretsManager.Models.Request; + +public class GetSecretsRequestModel +{ + [Required] + public IEnumerable Ids { get; set; } +} diff --git a/src/Api/SecretsManager/Models/Response/BaseSecretResponseModel.cs b/src/Api/SecretsManager/Models/Response/BaseSecretResponseModel.cs new file mode 100644 index 0000000000..0579baec07 --- /dev/null +++ b/src/Api/SecretsManager/Models/Response/BaseSecretResponseModel.cs @@ -0,0 +1,66 @@ +using Bit.Core.Models.Api; +using Bit.Core.SecretsManager.Entities; + +namespace Bit.Api.SecretsManager.Models.Response; + +public class BaseSecretResponseModel : ResponseModel +{ + private const string _objectName = "baseSecret"; + + public BaseSecretResponseModel(Secret secret, string objectName = _objectName) : base(objectName) + { + if (secret == null) + { + throw new ArgumentNullException(nameof(secret)); + } + + Id = secret.Id; + OrganizationId = secret.OrganizationId; + Key = secret.Key; + Value = secret.Value; + Note = secret.Note; + CreationDate = secret.CreationDate; + RevisionDate = secret.RevisionDate; + Projects = secret.Projects?.Select(p => new SecretResponseInnerProject(p)); + } + + public BaseSecretResponseModel(string objectName = _objectName) : base(objectName) + { + } + + public BaseSecretResponseModel() : base(_objectName) + { + } + + public Guid Id { get; set; } + + public Guid OrganizationId { get; set; } + + public string Key { get; set; } + + public string Value { get; set; } + + public string Note { get; set; } + + public DateTime CreationDate { get; set; } + + public DateTime RevisionDate { get; set; } + + public IEnumerable Projects { get; set; } + + public class SecretResponseInnerProject + { + public SecretResponseInnerProject(Project project) + { + Id = project.Id; + Name = project.Name; + } + + public SecretResponseInnerProject() + { + } + + public Guid Id { get; set; } + public string Name { get; set; } + } +} diff --git a/src/Api/SecretsManager/Models/Response/SecretResponseModel.cs b/src/Api/SecretsManager/Models/Response/SecretResponseModel.cs index 9f940a8782..c4633f78f0 100644 --- a/src/Api/SecretsManager/Models/Response/SecretResponseModel.cs +++ b/src/Api/SecretsManager/Models/Response/SecretResponseModel.cs @@ -1,28 +1,13 @@ -using Bit.Core.Models.Api; -using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Entities; namespace Bit.Api.SecretsManager.Models.Response; -public class SecretResponseModel : ResponseModel +public class SecretResponseModel : BaseSecretResponseModel { private const string _objectName = "secret"; - public SecretResponseModel(Secret secret, bool read, bool write) : base(_objectName) + public SecretResponseModel(Secret secret, bool read, bool write) : base(secret, _objectName) { - if (secret == null) - { - throw new ArgumentNullException(nameof(secret)); - } - - Id = secret.Id; - OrganizationId = secret.OrganizationId; - Key = secret.Key; - Value = secret.Value; - Note = secret.Note; - CreationDate = secret.CreationDate; - RevisionDate = secret.RevisionDate; - Projects = secret.Projects?.Select(p => new SecretResponseInnerProject(p)); - Read = read; Write = write; } @@ -31,39 +16,7 @@ public class SecretResponseModel : ResponseModel { } - public Guid Id { get; set; } - - public Guid OrganizationId { get; set; } - - public string Key { get; set; } - - public string Value { get; set; } - - public string Note { get; set; } - - public DateTime CreationDate { get; set; } - - public DateTime RevisionDate { get; set; } - - public IEnumerable Projects { get; set; } - public bool Read { get; set; } public bool Write { get; set; } - - public class SecretResponseInnerProject - { - public SecretResponseInnerProject(Project project) - { - Id = project.Id; - Name = project.Name; - } - - public SecretResponseInnerProject() - { - } - - public Guid Id { get; set; } - public string Name { get; set; } - } } diff --git a/src/Core/SecretsManager/AuthorizationRequirements/SecretOperationRequirement.cs b/src/Core/SecretsManager/AuthorizationRequirements/SecretOperationRequirement.cs index c6956ed306..e737960015 100644 --- a/src/Core/SecretsManager/AuthorizationRequirements/SecretOperationRequirement.cs +++ b/src/Core/SecretsManager/AuthorizationRequirements/SecretOperationRequirement.cs @@ -9,6 +9,7 @@ public class SecretOperationRequirement : OperationAuthorizationRequirement public static class SecretOperations { public static readonly SecretOperationRequirement Create = new() { Name = nameof(Create) }; + public static readonly SecretOperationRequirement Read = new() { Name = nameof(Read) }; public static readonly SecretOperationRequirement Update = new() { Name = nameof(Update) }; public static readonly SecretOperationRequirement Delete = new() { Name = nameof(Delete) }; } diff --git a/src/Core/Services/IEventService.cs b/src/Core/Services/IEventService.cs index 2288d1f926..10d8b6dbd0 100644 --- a/src/Core/Services/IEventService.cs +++ b/src/Core/Services/IEventService.cs @@ -29,4 +29,5 @@ public interface IEventService Task LogOrganizationDomainEventAsync(OrganizationDomain organizationDomain, EventType type, DateTime? date = null); Task LogOrganizationDomainEventAsync(OrganizationDomain organizationDomain, EventType type, EventSystemUser systemUser, DateTime? date = null); Task LogServiceAccountSecretEventAsync(Guid serviceAccountId, Secret secret, EventType type, DateTime? date = null); + Task LogServiceAccountSecretsEventAsync(Guid serviceAccountId, IEnumerable secrets, EventType type, DateTime? date = null); } diff --git a/src/Core/Services/Implementations/EventService.cs b/src/Core/Services/Implementations/EventService.cs index 96bdfe4500..d5dec0a106 100644 --- a/src/Core/Services/Implementations/EventService.cs +++ b/src/Core/Services/Implementations/EventService.cs @@ -406,22 +406,34 @@ public class EventService : IEventService } public async Task LogServiceAccountSecretEventAsync(Guid serviceAccountId, Secret secret, EventType type, DateTime? date = null) + { + await LogServiceAccountSecretsEventAsync(serviceAccountId, new[] { secret }, type, date); + } + + public async Task LogServiceAccountSecretsEventAsync(Guid serviceAccountId, IEnumerable secrets, EventType type, DateTime? date = null) { var orgAbilities = await _applicationCacheService.GetOrganizationAbilitiesAsync(); - if (!CanUseEvents(orgAbilities, secret.OrganizationId)) + var eventMessages = new List(); + + foreach (var secret in secrets) { - return; + if (!CanUseEvents(orgAbilities, secret.OrganizationId)) + { + continue; + } + + var e = new EventMessage(_currentContext) + { + OrganizationId = secret.OrganizationId, + Type = type, + SecretId = secret.Id, + ServiceAccountId = serviceAccountId, + Date = date.GetValueOrDefault(DateTime.UtcNow) + }; + eventMessages.Add(e); } - var e = new EventMessage(_currentContext) - { - OrganizationId = secret.OrganizationId, - Type = type, - SecretId = secret.Id, - ServiceAccountId = serviceAccountId, - Date = date.GetValueOrDefault(DateTime.UtcNow) - }; - await _eventWriteService.CreateAsync(e); + await _eventWriteService.CreateManyAsync(eventMessages); } private async Task GetProviderIdAsync(Guid? orgId) diff --git a/src/Core/Services/NoopImplementations/NoopEventService.cs b/src/Core/Services/NoopImplementations/NoopEventService.cs index 9eaefdab3a..068e03e35c 100644 --- a/src/Core/Services/NoopImplementations/NoopEventService.cs +++ b/src/Core/Services/NoopImplementations/NoopEventService.cs @@ -119,4 +119,10 @@ public class NoopEventService : IEventService { return Task.FromResult(0); } + + public Task LogServiceAccountSecretsEventAsync(Guid serviceAccountId, IEnumerable secrets, EventType type, + DateTime? date = null) + { + return Task.FromResult(0); + } } diff --git a/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs b/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs index 3f847d5f2c..0d937d3433 100644 --- a/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs +++ b/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs @@ -709,6 +709,69 @@ public class SecretsControllerTests : IClassFixture, IAsy Assert.Empty(secrets); } + [Theory] + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(false, true)] + public async Task GetSecretsByIds_SmNotEnabled_NotFound(bool useSecrets, bool accessSecrets) + { + var (org, _) = await _organizationHelper.Initialize(useSecrets, accessSecrets); + await LoginAsync(_email); + + var secret = await _secretRepository.CreateAsync(new Secret + { + OrganizationId = org.Id, + Key = _mockEncryptedString, + Value = _mockEncryptedString, + Note = _mockEncryptedString, + }); + + var request = new GetSecretsRequestModel { Ids = new[] { secret.Id } }; + + var response = await _client.PostAsJsonAsync("/secrets/get-by-ids", request); + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Theory] + [InlineData(PermissionType.RunAsAdmin)] + [InlineData(PermissionType.RunAsUserWithPermission)] + public async Task GetSecretsByIds_Success(PermissionType permissionType) + { + var (org, _) = await _organizationHelper.Initialize(true, true); + await LoginAsync(_email); + + var (project, secretIds) = await CreateSecretsAsync(org.Id); + + if (permissionType == PermissionType.RunAsUserWithPermission) + { + var (email, orgUser) = await _organizationHelper.CreateNewUser(OrganizationUserType.User, true); + await LoginAsync(email); + + var accessPolicies = new List + { + new UserProjectAccessPolicy + { + GrantedProjectId = project.Id, OrganizationUserId = orgUser.Id, Read = true, Write = true, + }, + }; + await _accessPolicyRepository.CreateManyAsync(accessPolicies); + } + else + { + var (email, _) = await _organizationHelper.CreateNewUser(OrganizationUserType.Admin, true); + await LoginAsync(email); + } + + var request = new GetSecretsRequestModel { Ids = secretIds }; + + var response = await _client.PostAsJsonAsync("/secrets/get-by-ids", request); + response.EnsureSuccessStatusCode(); + var result = await response.Content.ReadFromJsonAsync>(); + Assert.NotNull(result); + Assert.NotEmpty(result!.Data); + Assert.Equal(secretIds.Count, result!.Data.Count()); + } + private async Task<(Project Project, List secretIds)> CreateSecretsAsync(Guid orgId, int numberToCreate = 3) { var project = await _projectRepository.CreateAsync(new Project diff --git a/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs b/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs index 8afa2000a4..373bf8b183 100644 --- a/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs +++ b/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs @@ -346,4 +346,105 @@ public class SecretsControllerTests Assert.Null(result.Error); } } + + [Theory] + [BitAutoData] + public async void GetSecretsByIds_NoSecretsFound_ThrowsNotFound(SutProvider sutProvider, + List data) + { + var (ids, request) = BuildGetSecretsRequestModel(data); + sutProvider.GetDependency().GetManyByIds(Arg.Is(ids)).ReturnsForAnyArgs(new List()); + await Assert.ThrowsAsync(() => sutProvider.Sut.GetSecretsByIdsAsync(request)); + } + + [Theory] + [BitAutoData] + public async void GetSecretsByIds_SecretsFoundMisMatch_ThrowsNotFound(SutProvider sutProvider, + List data, Secret mockSecret) + { + var (ids, request) = BuildGetSecretsRequestModel(data); + ids.Add(mockSecret.Id); + sutProvider.GetDependency().GetManyByIds(Arg.Is(ids)) + .ReturnsForAnyArgs(new List { mockSecret }); + await Assert.ThrowsAsync(() => sutProvider.Sut.GetSecretsByIdsAsync(request)); + } + + [Theory] + [BitAutoData] + public async void GetSecretsByIds_OrganizationMisMatch_ThrowsNotFound(SutProvider sutProvider, + List data) + { + var (ids, request) = BuildGetSecretsRequestModel(data); + sutProvider.GetDependency().GetManyByIds(Arg.Is(ids)).ReturnsForAnyArgs(data); + await Assert.ThrowsAsync(() => sutProvider.Sut.GetSecretsByIdsAsync(request)); + } + + [Theory] + [BitAutoData] + public async void GetSecretsByIds_NoAccessToSecretsManager_ThrowsNotFound( + SutProvider sutProvider, List data) + { + var (ids, request) = BuildGetSecretsRequestModel(data); + var organizationId = SetOrganizations(ref data); + + sutProvider.GetDependency().AccessSecretsManager(Arg.Is(organizationId)) + .ReturnsForAnyArgs(false); + sutProvider.GetDependency().GetManyByIds(Arg.Is(ids)).ReturnsForAnyArgs(data); + await Assert.ThrowsAsync(() => sutProvider.Sut.GetSecretsByIdsAsync(request)); + } + + [Theory] + [BitAutoData] + public async void GetSecretsByIds_AccessDenied_ThrowsNotFound(SutProvider sutProvider, + List data) + { + var (ids, request) = BuildGetSecretsRequestModel(data); + var organizationId = SetOrganizations(ref data); + + sutProvider.GetDependency().GetManyByIds(Arg.Is(ids)).ReturnsForAnyArgs(data); + sutProvider.GetDependency().AccessSecretsManager(Arg.Is(organizationId)) + .ReturnsForAnyArgs(true); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), data.First(), + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Failed()); + + await Assert.ThrowsAsync(() => sutProvider.Sut.GetSecretsByIdsAsync(request)); + } + + [Theory] + [BitAutoData] + public async void GetSecretsByIds_Success(SutProvider sutProvider, List data) + { + var (ids, request) = BuildGetSecretsRequestModel(data); + var organizationId = SetOrganizations(ref data); + + sutProvider.GetDependency().GetManyByIds(Arg.Is(ids)).ReturnsForAnyArgs(data); + sutProvider.GetDependency().AccessSecretsManager(Arg.Is(organizationId)) + .ReturnsForAnyArgs(true); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), data.First(), + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); + + var results = await sutProvider.Sut.GetSecretsByIdsAsync(request); + Assert.Equal(data.Count, results.Data.Count()); + } + + private static (List Ids, GetSecretsRequestModel request) BuildGetSecretsRequestModel( + IEnumerable data) + { + var ids = data.Select(s => s.Id).ToList(); + var request = new GetSecretsRequestModel { Ids = ids }; + return (ids, request); + } + + private static Guid SetOrganizations(ref List data) + { + var organizationId = data.First().OrganizationId; + foreach (var s in data) + { + s.OrganizationId = organizationId; + } + + return organizationId; + } } From 7cac93ea906c82865d1020a7716431187077ec5e Mon Sep 17 00:00:00 2001 From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> Date: Mon, 28 Aug 2023 11:10:32 -0500 Subject: [PATCH 09/18] Increase secret value max limit (#3193) --- .../SecretsManager/Models/Request/SecretCreateRequestModel.cs | 2 +- .../SecretsManager/Models/Request/SecretUpdateRequestModel.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Api/SecretsManager/Models/Request/SecretCreateRequestModel.cs b/src/Api/SecretsManager/Models/Request/SecretCreateRequestModel.cs index 9d6f8c9aa1..fd895594a4 100644 --- a/src/Api/SecretsManager/Models/Request/SecretCreateRequestModel.cs +++ b/src/Api/SecretsManager/Models/Request/SecretCreateRequestModel.cs @@ -13,7 +13,7 @@ public class SecretCreateRequestModel : IValidatableObject [Required] [EncryptedString] - [EncryptedStringLength(5000)] + [EncryptedStringLength(35000)] public string Value { get; set; } [Required] diff --git a/src/Api/SecretsManager/Models/Request/SecretUpdateRequestModel.cs b/src/Api/SecretsManager/Models/Request/SecretUpdateRequestModel.cs index d5cd320fff..a08ed90c3c 100644 --- a/src/Api/SecretsManager/Models/Request/SecretUpdateRequestModel.cs +++ b/src/Api/SecretsManager/Models/Request/SecretUpdateRequestModel.cs @@ -13,7 +13,7 @@ public class SecretUpdateRequestModel : IValidatableObject [Required] [EncryptedString] - [EncryptedStringLength(5000)] + [EncryptedStringLength(35000)] public string Value { get; set; } [Required] From a1d227c121890d7513dea19e22d002129e377631 Mon Sep 17 00:00:00 2001 From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> Date: Mon, 28 Aug 2023 12:34:37 -0500 Subject: [PATCH 10/18] [SM-895] Enforce project maximums (#3214) * Add ProjectLimitQuery * Add query to DI * Add unit tests * Add query to controller * Add controller unit tests * add integration tests * rename query and variables * More renaming --- .../Queries/Projects/MaxProjectsQuery.cs | 45 +++++++++ .../SecretsManagerCollectionExtensions.cs | 3 + .../Queries/Projects/MaxProjectsQueryTests.cs | 97 +++++++++++++++++++ .../Controllers/ProjectsController.cs | 11 +++ .../Projects/Interfaces/IMaxProjectsQuery.cs | 6 ++ .../Controllers/ProjectsControllerTests.cs | 13 +++ .../Controllers/ProjectsControllerTests.cs | 19 ++++ 7 files changed, 194 insertions(+) create mode 100644 bitwarden_license/src/Commercial.Core/SecretsManager/Queries/Projects/MaxProjectsQuery.cs create mode 100644 bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/Projects/MaxProjectsQueryTests.cs create mode 100644 src/Core/SecretsManager/Queries/Projects/Interfaces/IMaxProjectsQuery.cs diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/Queries/Projects/MaxProjectsQuery.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/Queries/Projects/MaxProjectsQuery.cs new file mode 100644 index 0000000000..7cbb37f18c --- /dev/null +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/Queries/Projects/MaxProjectsQuery.cs @@ -0,0 +1,45 @@ +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Repositories; +using Bit.Core.SecretsManager.Queries.Projects.Interfaces; +using Bit.Core.SecretsManager.Repositories; +using Bit.Core.Utilities; + +namespace Bit.Commercial.Core.SecretsManager.Queries.Projects; + +public class MaxProjectsQuery : IMaxProjectsQuery +{ + private readonly IOrganizationRepository _organizationRepository; + private readonly IProjectRepository _projectRepository; + + public MaxProjectsQuery( + IOrganizationRepository organizationRepository, + IProjectRepository projectRepository) + { + _organizationRepository = organizationRepository; + _projectRepository = projectRepository; + } + + public async Task<(short? max, bool? atMax)> GetByOrgIdAsync(Guid organizationId) + { + var org = await _organizationRepository.GetByIdAsync(organizationId); + if (org == null) + { + throw new NotFoundException(); + } + + var plan = StaticStore.GetSecretsManagerPlan(org.PlanType); + if (plan == null) + { + throw new BadRequestException("Existing plan not found."); + } + + if (plan.Type == PlanType.Free) + { + var projects = await _projectRepository.GetProjectCountByOrganizationIdAsync(organizationId); + return projects >= plan.MaxProjects ? (plan.MaxProjects, true) : (plan.MaxProjects, false); + } + + return (null, null); + } +} diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs index be9534cfc8..47547eb0b1 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs @@ -10,6 +10,7 @@ using Bit.Commercial.Core.SecretsManager.Commands.Secrets; using Bit.Commercial.Core.SecretsManager.Commands.ServiceAccounts; using Bit.Commercial.Core.SecretsManager.Commands.Trash; using Bit.Commercial.Core.SecretsManager.Queries; +using Bit.Commercial.Core.SecretsManager.Queries.Projects; using Bit.Commercial.Core.SecretsManager.Queries.ServiceAccounts; using Bit.Core.SecretsManager.Commands.AccessPolicies.Interfaces; using Bit.Core.SecretsManager.Commands.AccessTokens.Interfaces; @@ -19,6 +20,7 @@ using Bit.Core.SecretsManager.Commands.Secrets.Interfaces; using Bit.Core.SecretsManager.Commands.ServiceAccounts.Interfaces; using Bit.Core.SecretsManager.Commands.Trash.Interfaces; using Bit.Core.SecretsManager.Queries.Interfaces; +using Bit.Core.SecretsManager.Queries.Projects.Interfaces; using Bit.Core.SecretsManager.Queries.ServiceAccounts.Interfaces; using Microsoft.AspNetCore.Authorization; using Microsoft.Extensions.DependencyInjection; @@ -34,6 +36,7 @@ public static class SecretsManagerCollectionExtensions services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); services.AddScoped(); diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/Projects/MaxProjectsQueryTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/Projects/MaxProjectsQueryTests.cs new file mode 100644 index 0000000000..6706e01657 --- /dev/null +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Queries/Projects/MaxProjectsQueryTests.cs @@ -0,0 +1,97 @@ +using Bit.Commercial.Core.SecretsManager.Queries.Projects; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Repositories; +using Bit.Core.SecretsManager.Repositories; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using NSubstitute.ReturnsExtensions; +using Xunit; + +namespace Bit.Commercial.Core.Test.SecretsManager.Queries.Projects; + +[SutProviderCustomize] +public class MaxProjectsQueryTests +{ + [Theory] + [BitAutoData] + public async Task GetByOrgIdAsync_OrganizationIsNull_ThrowsNotFound(SutProvider sutProvider, + Guid organizationId) + { + sutProvider.GetDependency().GetByIdAsync(default).ReturnsNull(); + + await Assert.ThrowsAsync(async () => await sutProvider.Sut.GetByOrgIdAsync(organizationId)); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .GetProjectCountByOrganizationIdAsync(organizationId); + } + + [Theory] + [BitAutoData(PlanType.FamiliesAnnually2019)] + [BitAutoData(PlanType.TeamsMonthly2019)] + [BitAutoData(PlanType.TeamsAnnually2019)] + [BitAutoData(PlanType.EnterpriseMonthly2019)] + [BitAutoData(PlanType.EnterpriseAnnually2019)] + [BitAutoData(PlanType.Custom)] + [BitAutoData(PlanType.FamiliesAnnually)] + public async Task GetByOrgIdAsync_SmPlanIsNull_ThrowsBadRequest(PlanType planType, + SutProvider sutProvider, Organization organization) + { + organization.PlanType = planType; + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + + await Assert.ThrowsAsync( + async () => await sutProvider.Sut.GetByOrgIdAsync(organization.Id)); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .GetProjectCountByOrganizationIdAsync(organization.Id); + } + + [Theory] + [BitAutoData(PlanType.TeamsMonthly)] + [BitAutoData(PlanType.TeamsAnnually)] + [BitAutoData(PlanType.EnterpriseMonthly)] + [BitAutoData(PlanType.EnterpriseAnnually)] + public async Task GetByOrgIdAsync_SmNoneFreePlans_ReturnsNull(PlanType planType, + SutProvider sutProvider, Organization organization) + { + organization.PlanType = planType; + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + + var (limit, overLimit) = await sutProvider.Sut.GetByOrgIdAsync(organization.Id); + + Assert.Null(limit); + Assert.Null(overLimit); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .GetProjectCountByOrganizationIdAsync(organization.Id); + } + + [Theory] + [BitAutoData(PlanType.Free, 0, false)] + [BitAutoData(PlanType.Free, 1, false)] + [BitAutoData(PlanType.Free, 2, false)] + [BitAutoData(PlanType.Free, 3, true)] + [BitAutoData(PlanType.Free, 4, true)] + [BitAutoData(PlanType.Free, 40, true)] + public async Task GetByOrgIdAsync_SmFreePlan_Success(PlanType planType, int projects, bool shouldBeAtMax, + SutProvider sutProvider, Organization organization) + { + organization.PlanType = planType; + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + sutProvider.GetDependency().GetProjectCountByOrganizationIdAsync(organization.Id) + .Returns(projects); + + var (max, atMax) = await sutProvider.Sut.GetByOrgIdAsync(organization.Id); + + Assert.NotNull(max); + Assert.NotNull(atMax); + Assert.Equal(3, max.Value); + Assert.Equal(shouldBeAtMax, atMax); + + await sutProvider.GetDependency().Received(1) + .GetProjectCountByOrganizationIdAsync(organization.Id); + } +} diff --git a/src/Api/SecretsManager/Controllers/ProjectsController.cs b/src/Api/SecretsManager/Controllers/ProjectsController.cs index 7ed428ad54..4f3815ca78 100644 --- a/src/Api/SecretsManager/Controllers/ProjectsController.cs +++ b/src/Api/SecretsManager/Controllers/ProjectsController.cs @@ -7,6 +7,7 @@ using Bit.Core.Exceptions; using Bit.Core.SecretsManager.AuthorizationRequirements; using Bit.Core.SecretsManager.Commands.Projects.Interfaces; using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Queries.Projects.Interfaces; using Bit.Core.SecretsManager.Repositories; using Bit.Core.Services; using Bit.Core.Utilities; @@ -22,6 +23,7 @@ public class ProjectsController : Controller private readonly ICurrentContext _currentContext; private readonly IUserService _userService; private readonly IProjectRepository _projectRepository; + private readonly IMaxProjectsQuery _maxProjectsQuery; private readonly ICreateProjectCommand _createProjectCommand; private readonly IUpdateProjectCommand _updateProjectCommand; private readonly IDeleteProjectCommand _deleteProjectCommand; @@ -31,6 +33,7 @@ public class ProjectsController : Controller ICurrentContext currentContext, IUserService userService, IProjectRepository projectRepository, + IMaxProjectsQuery maxProjectsQuery, ICreateProjectCommand createProjectCommand, IUpdateProjectCommand updateProjectCommand, IDeleteProjectCommand deleteProjectCommand, @@ -39,6 +42,7 @@ public class ProjectsController : Controller _currentContext = currentContext; _userService = userService; _projectRepository = projectRepository; + _maxProjectsQuery = maxProjectsQuery; _createProjectCommand = createProjectCommand; _updateProjectCommand = updateProjectCommand; _deleteProjectCommand = deleteProjectCommand; @@ -74,6 +78,13 @@ public class ProjectsController : Controller { throw new NotFoundException(); } + + var (max, atMax) = await _maxProjectsQuery.GetByOrgIdAsync(organizationId); + if (atMax != null && atMax.Value) + { + throw new BadRequestException($"You have reached the maximum number of projects ({max}) for this plan."); + } + var userId = _userService.GetProperUserId(User).Value; var result = await _createProjectCommand.CreateAsync(project, userId, _currentContext.ClientType); diff --git a/src/Core/SecretsManager/Queries/Projects/Interfaces/IMaxProjectsQuery.cs b/src/Core/SecretsManager/Queries/Projects/Interfaces/IMaxProjectsQuery.cs new file mode 100644 index 0000000000..e00f5ed674 --- /dev/null +++ b/src/Core/SecretsManager/Queries/Projects/Interfaces/IMaxProjectsQuery.cs @@ -0,0 +1,6 @@ +namespace Bit.Core.SecretsManager.Queries.Projects.Interfaces; + +public interface IMaxProjectsQuery +{ + Task<(short? max, bool? atMax)> GetByOrgIdAsync(Guid organizationId); +} diff --git a/test/Api.IntegrationTest/SecretsManager/Controllers/ProjectsControllerTests.cs b/test/Api.IntegrationTest/SecretsManager/Controllers/ProjectsControllerTests.cs index 0ef31085f2..fa88a44b83 100644 --- a/test/Api.IntegrationTest/SecretsManager/Controllers/ProjectsControllerTests.cs +++ b/test/Api.IntegrationTest/SecretsManager/Controllers/ProjectsControllerTests.cs @@ -116,6 +116,19 @@ public class ProjectsControllerTests : IClassFixture, IAs Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); } + [Theory] + [InlineData(PermissionType.RunAsAdmin)] + [InlineData(PermissionType.RunAsUserWithPermission)] + public async Task Create_AtMaxProjects_BadRequest(PermissionType permissionType) + { + var (_, organization) = await SetupProjectsWithAccessAsync(permissionType, 3); + var request = new ProjectCreateRequestModel { Name = _mockEncryptedString }; + + var response = await _client.PostAsJsonAsync($"/organizations/{organization.Id}/projects", request); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + [Theory] [InlineData(PermissionType.RunAsAdmin)] [InlineData(PermissionType.RunAsUserWithPermission)] diff --git a/test/Api.Test/SecretsManager/Controllers/ProjectsControllerTests.cs b/test/Api.Test/SecretsManager/Controllers/ProjectsControllerTests.cs index 30287eb953..32239159a6 100644 --- a/test/Api.Test/SecretsManager/Controllers/ProjectsControllerTests.cs +++ b/test/Api.Test/SecretsManager/Controllers/ProjectsControllerTests.cs @@ -8,6 +8,7 @@ using Bit.Core.Exceptions; using Bit.Core.SecretsManager.Commands.Projects.Interfaces; using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Models.Data; +using Bit.Core.SecretsManager.Queries.Projects.Interfaces; using Bit.Core.SecretsManager.Repositories; using Bit.Core.Services; using Bit.Core.Test.SecretsManager.AutoFixture.ProjectsFixture; @@ -122,6 +123,24 @@ public class ProjectsControllerTests .CreateAsync(Arg.Any(), Arg.Any(), sutProvider.GetDependency().ClientType); } + [Theory] + [BitAutoData] + public async void Create_AtMaxProjects_Throws(SutProvider sutProvider, + Guid orgId, ProjectCreateRequestModel data) + { + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), data.ToProject(orgId), + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); + sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(Guid.NewGuid()); + sutProvider.GetDependency().GetByOrgIdAsync(orgId).Returns(((short)3, true)); + + + await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAsync(orgId, data)); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .CreateAsync(Arg.Any(), Arg.Any(), sutProvider.GetDependency().ClientType); + } + [Theory] [BitAutoData] public async void Create_Success(SutProvider sutProvider, From 27ba3069b22cae524dab55d4694710b052ac51bc Mon Sep 17 00:00:00 2001 From: Jake Fink Date: Mon, 28 Aug 2023 14:51:23 -0400 Subject: [PATCH 11/18] [PM-3504] Return early for SAML auths (#3215) * return early if scheme doesn't match * Revert "return early if scheme doesn't match" This reverts commit 5c07d66774971927589542b2abea7c483e20a5b5. * extend saml2handler for extra validation * add comment * fix file encoding * add comment --- .../DynamicAuthenticationSchemeProvider.cs | 2 +- .../src/Sso/Utilities/Saml2BitHandler.cs | 205 ++++++++++++++++++ 2 files changed, 206 insertions(+), 1 deletion(-) create mode 100644 bitwarden_license/src/Sso/Utilities/Saml2BitHandler.cs diff --git a/bitwarden_license/src/Sso/Utilities/DynamicAuthenticationSchemeProvider.cs b/bitwarden_license/src/Sso/Utilities/DynamicAuthenticationSchemeProvider.cs index aeef3f679b..1487697279 100644 --- a/bitwarden_license/src/Sso/Utilities/DynamicAuthenticationSchemeProvider.cs +++ b/bitwarden_license/src/Sso/Utilities/DynamicAuthenticationSchemeProvider.cs @@ -415,7 +415,7 @@ public class DynamicAuthenticationSchemeProvider : AuthenticationSchemeProvider }; options.IdentityProviders.Add(idp); - return new DynamicAuthenticationScheme(name, name, typeof(Saml2Handler), options, SsoType.Saml2); + return new DynamicAuthenticationScheme(name, name, typeof(Saml2BitHandler), options, SsoType.Saml2); } private NameIdFormat GetNameIdFormat(Saml2NameIdFormat format) diff --git a/bitwarden_license/src/Sso/Utilities/Saml2BitHandler.cs b/bitwarden_license/src/Sso/Utilities/Saml2BitHandler.cs new file mode 100644 index 0000000000..6e5a37fb96 --- /dev/null +++ b/bitwarden_license/src/Sso/Utilities/Saml2BitHandler.cs @@ -0,0 +1,205 @@ +using System.Text; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication.Cookies; +using Microsoft.AspNetCore.DataProtection; +using Microsoft.Extensions.Options; +using Sustainsys.Saml2.AspNetCore2; +using Sustainsys.Saml2.WebSso; + +namespace Bit.Sso.Utilities; + +// Temporary handler for validating Saml2 requests +// Most of this is taken from Sustainsys.Saml2.AspNetCore2.Saml2Handler +// TODO: PM-3641 - Remove this handler once there is a proper solution +public class Saml2BitHandler : IAuthenticationRequestHandler +{ + private readonly Saml2Handler _saml2Handler; + private string _scheme; + + private readonly IOptionsMonitorCache _optionsCache; + private Saml2Options _options; + private HttpContext _context; + private readonly IDataProtector _dataProtector; + private readonly IOptionsFactory _optionsFactory; + private bool _emitSameSiteNone; + + public Saml2BitHandler( + IOptionsMonitorCache optionsCache, + IDataProtectionProvider dataProtectorProvider, + IOptionsFactory optionsFactory) + { + if (dataProtectorProvider == null) + { + throw new ArgumentNullException(nameof(dataProtectorProvider)); + } + + _optionsFactory = optionsFactory; + _optionsCache = optionsCache; + + _saml2Handler = new Saml2Handler(optionsCache, dataProtectorProvider, optionsFactory); + _dataProtector = dataProtectorProvider.CreateProtector(_saml2Handler.GetType().FullName); + } + + public Task InitializeAsync(AuthenticationScheme scheme, HttpContext context) + { + _context = context ?? throw new ArgumentNullException(nameof(context)); + _options = _optionsCache.GetOrAdd(scheme.Name, () => _optionsFactory.Create(scheme.Name)); + _emitSameSiteNone = _options.Notifications.EmitSameSiteNone(context.Request.GetUserAgent()); + _scheme = scheme.Name; + + return _saml2Handler.InitializeAsync(scheme, context); + } + + + public async Task HandleRequestAsync() + { + if (!_context.Request.Path.StartsWithSegments(_options.SPOptions.ModulePath, StringComparison.Ordinal)) + { + return false; + } + + var commandName = _context.Request.Path.Value.Substring( + _options.SPOptions.ModulePath.Length).TrimStart('/'); + + var commandResult = CommandFactory.GetCommand(commandName).Run( + _context.ToHttpRequestData(_options.CookieManager, _dataProtector.Unprotect), _options); + + // Scheme is the organization ID since we use dynamic handlers for authentication schemes. + // We need to compare this to the scheme returned in the RelayData to ensure this value hasn't been + // tampered with + if (commandResult.RelayData["scheme"] != _scheme) + { + return false; + } + + await commandResult.Apply( + _context, _dataProtector, _options.CookieManager, _options.SignInScheme, _options.SignOutScheme, _emitSameSiteNone); + + return true; + } + + public Task AuthenticateAsync() => _saml2Handler.AuthenticateAsync(); + + public Task ChallengeAsync(AuthenticationProperties properties) => _saml2Handler.ChallengeAsync(properties); + + public Task ForbidAsync(AuthenticationProperties properties) => _saml2Handler.ForbidAsync(properties); +} + + +static class HttpRequestExtensions +{ + public static HttpRequestData ToHttpRequestData( + this HttpContext httpContext, + ICookieManager cookieManager, + Func cookieDecryptor) + { + var request = httpContext.Request; + + var uri = new Uri( + request.Scheme + + "://" + + request.Host + + request.Path + + request.QueryString); + + var pathBase = httpContext.Request.PathBase.Value; + pathBase = string.IsNullOrEmpty(pathBase) ? "/" : pathBase; + IEnumerable>> formData = null; + if (httpContext.Request.Method == "POST" && httpContext.Request.HasFormContentType) + { + formData = request.Form.Select( + f => new KeyValuePair>(f.Key, f.Value)); + } + + return new HttpRequestData( + httpContext.Request.Method, + uri, + pathBase, + formData, + cookieName => cookieManager.GetRequestCookie(httpContext, cookieName), + cookieDecryptor, + httpContext.User); + } + + public static string GetUserAgent(this HttpRequest request) + { + return request.Headers["user-agent"].FirstOrDefault() ?? ""; + } +} + +static class CommandResultExtensions +{ + public static async Task Apply( + this CommandResult commandResult, + HttpContext httpContext, + IDataProtector dataProtector, + ICookieManager cookieManager, + string signInScheme, + string signOutScheme, + bool emitSameSiteNone) + { + httpContext.Response.StatusCode = (int)commandResult.HttpStatusCode; + + if (commandResult.Location != null) + { + httpContext.Response.Headers["Location"] = commandResult.Location.OriginalString; + } + + if (!string.IsNullOrEmpty(commandResult.SetCookieName)) + { + var cookieData = HttpRequestData.ConvertBinaryData( + dataProtector.Protect(commandResult.GetSerializedRequestState())); + + cookieManager.AppendResponseCookie( + httpContext, + commandResult.SetCookieName, + cookieData, + new CookieOptions() + { + HttpOnly = true, + Secure = commandResult.SetCookieSecureFlag, + // We are expecting a different site to POST back to us, + // so the ASP.Net Core default of Lax is not appropriate in this case + SameSite = emitSameSiteNone ? SameSiteMode.None : (SameSiteMode)(-1), + IsEssential = true + }); + } + + foreach (var h in commandResult.Headers) + { + httpContext.Response.Headers.Add(h.Key, h.Value); + } + + if (!string.IsNullOrEmpty(commandResult.ClearCookieName)) + { + cookieManager.DeleteCookie( + httpContext, + commandResult.ClearCookieName, + new CookieOptions + { + Secure = commandResult.SetCookieSecureFlag + }); + } + + if (!string.IsNullOrEmpty(commandResult.Content)) + { + var buffer = Encoding.UTF8.GetBytes(commandResult.Content); + httpContext.Response.ContentType = commandResult.ContentType; + await httpContext.Response.Body.WriteAsync(buffer, 0, buffer.Length); + } + + if (commandResult.Principal != null) + { + var authProps = new AuthenticationProperties(commandResult.RelayData) + { + RedirectUri = commandResult.Location.OriginalString + }; + await httpContext.SignInAsync(signInScheme, commandResult.Principal, authProps); + } + + if (commandResult.TerminateLocalSession) + { + await httpContext.SignOutAsync(signOutScheme ?? signInScheme); + } + } +} From 626f7977d147ab44be099c4a9df2d37af0f95fb7 Mon Sep 17 00:00:00 2001 From: Jake Fink Date: Mon, 28 Aug 2023 14:54:01 -0400 Subject: [PATCH 12/18] Revert "[PM-3504] Return early for SAML auths (#3215)" (#3234) This reverts commit 27ba3069b22cae524dab55d4694710b052ac51bc. --- .../DynamicAuthenticationSchemeProvider.cs | 2 +- .../src/Sso/Utilities/Saml2BitHandler.cs | 205 ------------------ 2 files changed, 1 insertion(+), 206 deletions(-) delete mode 100644 bitwarden_license/src/Sso/Utilities/Saml2BitHandler.cs diff --git a/bitwarden_license/src/Sso/Utilities/DynamicAuthenticationSchemeProvider.cs b/bitwarden_license/src/Sso/Utilities/DynamicAuthenticationSchemeProvider.cs index 1487697279..aeef3f679b 100644 --- a/bitwarden_license/src/Sso/Utilities/DynamicAuthenticationSchemeProvider.cs +++ b/bitwarden_license/src/Sso/Utilities/DynamicAuthenticationSchemeProvider.cs @@ -415,7 +415,7 @@ public class DynamicAuthenticationSchemeProvider : AuthenticationSchemeProvider }; options.IdentityProviders.Add(idp); - return new DynamicAuthenticationScheme(name, name, typeof(Saml2BitHandler), options, SsoType.Saml2); + return new DynamicAuthenticationScheme(name, name, typeof(Saml2Handler), options, SsoType.Saml2); } private NameIdFormat GetNameIdFormat(Saml2NameIdFormat format) diff --git a/bitwarden_license/src/Sso/Utilities/Saml2BitHandler.cs b/bitwarden_license/src/Sso/Utilities/Saml2BitHandler.cs deleted file mode 100644 index 6e5a37fb96..0000000000 --- a/bitwarden_license/src/Sso/Utilities/Saml2BitHandler.cs +++ /dev/null @@ -1,205 +0,0 @@ -using System.Text; -using Microsoft.AspNetCore.Authentication; -using Microsoft.AspNetCore.Authentication.Cookies; -using Microsoft.AspNetCore.DataProtection; -using Microsoft.Extensions.Options; -using Sustainsys.Saml2.AspNetCore2; -using Sustainsys.Saml2.WebSso; - -namespace Bit.Sso.Utilities; - -// Temporary handler for validating Saml2 requests -// Most of this is taken from Sustainsys.Saml2.AspNetCore2.Saml2Handler -// TODO: PM-3641 - Remove this handler once there is a proper solution -public class Saml2BitHandler : IAuthenticationRequestHandler -{ - private readonly Saml2Handler _saml2Handler; - private string _scheme; - - private readonly IOptionsMonitorCache _optionsCache; - private Saml2Options _options; - private HttpContext _context; - private readonly IDataProtector _dataProtector; - private readonly IOptionsFactory _optionsFactory; - private bool _emitSameSiteNone; - - public Saml2BitHandler( - IOptionsMonitorCache optionsCache, - IDataProtectionProvider dataProtectorProvider, - IOptionsFactory optionsFactory) - { - if (dataProtectorProvider == null) - { - throw new ArgumentNullException(nameof(dataProtectorProvider)); - } - - _optionsFactory = optionsFactory; - _optionsCache = optionsCache; - - _saml2Handler = new Saml2Handler(optionsCache, dataProtectorProvider, optionsFactory); - _dataProtector = dataProtectorProvider.CreateProtector(_saml2Handler.GetType().FullName); - } - - public Task InitializeAsync(AuthenticationScheme scheme, HttpContext context) - { - _context = context ?? throw new ArgumentNullException(nameof(context)); - _options = _optionsCache.GetOrAdd(scheme.Name, () => _optionsFactory.Create(scheme.Name)); - _emitSameSiteNone = _options.Notifications.EmitSameSiteNone(context.Request.GetUserAgent()); - _scheme = scheme.Name; - - return _saml2Handler.InitializeAsync(scheme, context); - } - - - public async Task HandleRequestAsync() - { - if (!_context.Request.Path.StartsWithSegments(_options.SPOptions.ModulePath, StringComparison.Ordinal)) - { - return false; - } - - var commandName = _context.Request.Path.Value.Substring( - _options.SPOptions.ModulePath.Length).TrimStart('/'); - - var commandResult = CommandFactory.GetCommand(commandName).Run( - _context.ToHttpRequestData(_options.CookieManager, _dataProtector.Unprotect), _options); - - // Scheme is the organization ID since we use dynamic handlers for authentication schemes. - // We need to compare this to the scheme returned in the RelayData to ensure this value hasn't been - // tampered with - if (commandResult.RelayData["scheme"] != _scheme) - { - return false; - } - - await commandResult.Apply( - _context, _dataProtector, _options.CookieManager, _options.SignInScheme, _options.SignOutScheme, _emitSameSiteNone); - - return true; - } - - public Task AuthenticateAsync() => _saml2Handler.AuthenticateAsync(); - - public Task ChallengeAsync(AuthenticationProperties properties) => _saml2Handler.ChallengeAsync(properties); - - public Task ForbidAsync(AuthenticationProperties properties) => _saml2Handler.ForbidAsync(properties); -} - - -static class HttpRequestExtensions -{ - public static HttpRequestData ToHttpRequestData( - this HttpContext httpContext, - ICookieManager cookieManager, - Func cookieDecryptor) - { - var request = httpContext.Request; - - var uri = new Uri( - request.Scheme - + "://" - + request.Host - + request.Path - + request.QueryString); - - var pathBase = httpContext.Request.PathBase.Value; - pathBase = string.IsNullOrEmpty(pathBase) ? "/" : pathBase; - IEnumerable>> formData = null; - if (httpContext.Request.Method == "POST" && httpContext.Request.HasFormContentType) - { - formData = request.Form.Select( - f => new KeyValuePair>(f.Key, f.Value)); - } - - return new HttpRequestData( - httpContext.Request.Method, - uri, - pathBase, - formData, - cookieName => cookieManager.GetRequestCookie(httpContext, cookieName), - cookieDecryptor, - httpContext.User); - } - - public static string GetUserAgent(this HttpRequest request) - { - return request.Headers["user-agent"].FirstOrDefault() ?? ""; - } -} - -static class CommandResultExtensions -{ - public static async Task Apply( - this CommandResult commandResult, - HttpContext httpContext, - IDataProtector dataProtector, - ICookieManager cookieManager, - string signInScheme, - string signOutScheme, - bool emitSameSiteNone) - { - httpContext.Response.StatusCode = (int)commandResult.HttpStatusCode; - - if (commandResult.Location != null) - { - httpContext.Response.Headers["Location"] = commandResult.Location.OriginalString; - } - - if (!string.IsNullOrEmpty(commandResult.SetCookieName)) - { - var cookieData = HttpRequestData.ConvertBinaryData( - dataProtector.Protect(commandResult.GetSerializedRequestState())); - - cookieManager.AppendResponseCookie( - httpContext, - commandResult.SetCookieName, - cookieData, - new CookieOptions() - { - HttpOnly = true, - Secure = commandResult.SetCookieSecureFlag, - // We are expecting a different site to POST back to us, - // so the ASP.Net Core default of Lax is not appropriate in this case - SameSite = emitSameSiteNone ? SameSiteMode.None : (SameSiteMode)(-1), - IsEssential = true - }); - } - - foreach (var h in commandResult.Headers) - { - httpContext.Response.Headers.Add(h.Key, h.Value); - } - - if (!string.IsNullOrEmpty(commandResult.ClearCookieName)) - { - cookieManager.DeleteCookie( - httpContext, - commandResult.ClearCookieName, - new CookieOptions - { - Secure = commandResult.SetCookieSecureFlag - }); - } - - if (!string.IsNullOrEmpty(commandResult.Content)) - { - var buffer = Encoding.UTF8.GetBytes(commandResult.Content); - httpContext.Response.ContentType = commandResult.ContentType; - await httpContext.Response.Body.WriteAsync(buffer, 0, buffer.Length); - } - - if (commandResult.Principal != null) - { - var authProps = new AuthenticationProperties(commandResult.RelayData) - { - RedirectUri = commandResult.Location.OriginalString - }; - await httpContext.SignInAsync(signInScheme, commandResult.Principal, authProps); - } - - if (commandResult.TerminateLocalSession) - { - await httpContext.SignOutAsync(signOutScheme ?? signInScheme); - } - } -} From 776e454b79ae93a8badd7ce1d404c2ba50928bd4 Mon Sep 17 00:00:00 2001 From: Alexey Zilber <110793805+alex8bitw@users.noreply.github.com> Date: Tue, 29 Aug 2023 21:25:47 +0800 Subject: [PATCH 13/18] Changing CF-Connecting-IP -> X-Connecting-IP. Also renaming CloudFlareConnectingIp. (#3236) --- src/Api/Controllers/InfoController.cs | 2 +- src/Api/appsettings.json | 2 +- src/Core/Utilities/CoreHelpers.cs | 6 +++--- src/Identity/appsettings.json | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Api/Controllers/InfoController.cs b/src/Api/Controllers/InfoController.cs index 9c6b7b8866..fcd41540d0 100644 --- a/src/Api/Controllers/InfoController.cs +++ b/src/Api/Controllers/InfoController.cs @@ -21,7 +21,7 @@ public class InfoController : Controller [HttpGet("~/ip")] public JsonResult Ip() { - var headerSet = new HashSet { "x-forwarded-for", "cf-connecting-ip", "client-ip" }; + var headerSet = new HashSet { "x-forwarded-for", "x-connecting-ip", "cf-connecting-ip", "client-ip", "true-client-ip" }; var headers = HttpContext.Request?.Headers .Where(h => headerSet.Contains(h.Key.ToLower())) .ToDictionary(h => h.Key); diff --git a/src/Api/appsettings.json b/src/Api/appsettings.json index fde1db479e..e49491857f 100644 --- a/src/Api/appsettings.json +++ b/src/Api/appsettings.json @@ -79,7 +79,7 @@ "IpRateLimitOptions": { "EnableEndpointRateLimiting": true, "StackBlockedRequests": false, - "RealIpHeader": "CF-Connecting-IP", + "RealIpHeader": "X-Connecting-IP", "ClientIdHeader": "X-ClientId", "HttpStatusCode": 429, "IpWhitelist": [], diff --git a/src/Core/Utilities/CoreHelpers.cs b/src/Core/Utilities/CoreHelpers.cs index 66ff08c07e..addc213390 100644 --- a/src/Core/Utilities/CoreHelpers.cs +++ b/src/Core/Utilities/CoreHelpers.cs @@ -29,7 +29,7 @@ public static class CoreHelpers private static readonly DateTime _epoc = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc); private static readonly DateTime _max = new DateTime(9999, 1, 1, 0, 0, 0, DateTimeKind.Utc); private static readonly Random _random = new Random(); - private static readonly string CloudFlareConnectingIp = "CF-Connecting-IP"; + private static readonly string RealConnectingIp = "X-Connecting-IP"; /// /// Generate sequential Guid for Sql Server. @@ -557,9 +557,9 @@ public static class CoreHelpers return null; } - if (!globalSettings.SelfHosted && httpContext.Request.Headers.ContainsKey(CloudFlareConnectingIp)) + if (!globalSettings.SelfHosted && httpContext.Request.Headers.ContainsKey(RealConnectingIp)) { - return httpContext.Request.Headers[CloudFlareConnectingIp].ToString(); + return httpContext.Request.Headers[RealConnectingIp].ToString(); } return httpContext.Connection?.RemoteIpAddress?.ToString(); diff --git a/src/Identity/appsettings.json b/src/Identity/appsettings.json index 609a5004aa..e3626b4e16 100644 --- a/src/Identity/appsettings.json +++ b/src/Identity/appsettings.json @@ -69,7 +69,7 @@ "IpRateLimitOptions": { "EnableEndpointRateLimiting": true, "StackBlockedRequests": false, - "RealIpHeader": "CF-Connecting-IP", + "RealIpHeader": "X-Connecting-IP", "ClientIdHeader": "X-ClientId", "HttpStatusCode": 429, "IpWhitelist": [], From b1725115e368adc800d64ba1a700e754f7ba15bf Mon Sep 17 00:00:00 2001 From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> Date: Tue, 29 Aug 2023 17:15:07 -0500 Subject: [PATCH 14/18] [SM-823] ApiKey table follow up (#3183) * dbo_future -> dbo * DbScripts_future -> DbScripts * Remove deprecated property * Move data_migration -> DbScripts --- .../Models/Data/ApiKeyDetails.cs | 2 - src/Identity/IdentityServer/ClientStore.cs | 5 --- .../ApiKey/ApiKey_Create.sql | 15 ++----- src/Sql/SecretsManager/dbo/Tables/ApiKey.sql | 1 - .../Stored Procedures/ApiKey_Create.sql | 42 ------------------- src/Sql/dbo_future/Tables/ApiKey.sql | 18 -------- ...8-10_00_ClientSecretHashDataMigration.sql} | 6 +-- .../2023-08-10_01_RemoveClientSecret.sql} | 4 +- 8 files changed, 8 insertions(+), 85 deletions(-) delete mode 100644 src/Sql/dbo_future/Stored Procedures/ApiKey_Create.sql delete mode 100644 src/Sql/dbo_future/Tables/ApiKey.sql rename util/Migrator/{DbScripts_data_migration/2023-05-16_00_ClientSecretHashDataMigration.sql => DbScripts/2023-08-10_00_ClientSecretHashDataMigration.sql} (88%) rename util/Migrator/{DbScripts_future/2023-06-FutureMigration.sql => DbScripts/2023-08-10_01_RemoveClientSecret.sql} (96%) diff --git a/src/Core/SecretsManager/Models/Data/ApiKeyDetails.cs b/src/Core/SecretsManager/Models/Data/ApiKeyDetails.cs index 2c8c3d4692..f8945e9610 100644 --- a/src/Core/SecretsManager/Models/Data/ApiKeyDetails.cs +++ b/src/Core/SecretsManager/Models/Data/ApiKeyDetails.cs @@ -4,8 +4,6 @@ namespace Bit.Core.SecretsManager.Models.Data; public class ApiKeyDetails : ApiKey { - public string ClientSecret { get; set; } // Deprecated as of 2023-05-17 - protected ApiKeyDetails() { } protected ApiKeyDetails(ApiKey apiKey) diff --git a/src/Identity/IdentityServer/ClientStore.cs b/src/Identity/IdentityServer/ClientStore.cs index f7987b9baf..e2fd33c9db 100644 --- a/src/Identity/IdentityServer/ClientStore.cs +++ b/src/Identity/IdentityServer/ClientStore.cs @@ -107,11 +107,6 @@ public class ClientStore : IClientStore break; } - if (string.IsNullOrEmpty(apiKey.ClientSecretHash)) - { - apiKey.ClientSecretHash = apiKey.ClientSecret.Sha256(); - } - var client = new Client { ClientId = clientId, diff --git a/src/Sql/SecretsManager/dbo/Stored Procedures/ApiKey/ApiKey_Create.sql b/src/Sql/SecretsManager/dbo/Stored Procedures/ApiKey/ApiKey_Create.sql index e72316ef49..e9440026a9 100644 --- a/src/Sql/SecretsManager/dbo/Stored Procedures/ApiKey/ApiKey_Create.sql +++ b/src/Sql/SecretsManager/dbo/Stored Procedures/ApiKey/ApiKey_Create.sql @@ -2,8 +2,7 @@ CREATE PROCEDURE [dbo].[ApiKey_Create] @Id UNIQUEIDENTIFIER OUTPUT, @ServiceAccountId UNIQUEIDENTIFIER, @Name VARCHAR(200), - @ClientSecret VARCHAR(30) = 'migrated', -- Deprecated as of 2023-05-17 - @ClientSecretHash VARCHAR(128) = NULL, + @ClientSecretHash VARCHAR(128), @Scope NVARCHAR(4000), @EncryptedPayload NVARCHAR(4000), @Key VARCHAR(MAX), @@ -14,18 +13,11 @@ AS BEGIN SET NOCOUNT ON - IF (@ClientSecretHash IS NULL) - BEGIN - DECLARE @hb VARBINARY(128) = HASHBYTES('SHA2_256', @ClientSecret); - SET @ClientSecretHash = CAST(N'' as xml).value('xs:base64Binary(sql:variable("@hb"))', 'VARCHAR(128)'); - END - - INSERT INTO [dbo].[ApiKey] + INSERT INTO [dbo].[ApiKey] ( [Id], [ServiceAccountId], [Name], - [ClientSecret], [ClientSecretHash], [Scope], [EncryptedPayload], @@ -34,12 +26,11 @@ BEGIN [CreationDate], [RevisionDate] ) - VALUES + VALUES ( @Id, @ServiceAccountId, @Name, - @ClientSecret, @ClientSecretHash, @Scope, @EncryptedPayload, diff --git a/src/Sql/SecretsManager/dbo/Tables/ApiKey.sql b/src/Sql/SecretsManager/dbo/Tables/ApiKey.sql index 5761d45e48..051339f3ad 100644 --- a/src/Sql/SecretsManager/dbo/Tables/ApiKey.sql +++ b/src/Sql/SecretsManager/dbo/Tables/ApiKey.sql @@ -2,7 +2,6 @@ [Id] UNIQUEIDENTIFIER, [ServiceAccountId] UNIQUEIDENTIFIER NULL, [Name] VARCHAR(200) NOT NULL, - [ClientSecret] VARCHAR(30) NOT NULL, [ClientSecretHash] VARCHAR(128) NULL, [Scope] NVARCHAR (4000) NOT NULL, [EncryptedPayload] NVARCHAR (4000) NOT NULL, diff --git a/src/Sql/dbo_future/Stored Procedures/ApiKey_Create.sql b/src/Sql/dbo_future/Stored Procedures/ApiKey_Create.sql deleted file mode 100644 index 241881efdd..0000000000 --- a/src/Sql/dbo_future/Stored Procedures/ApiKey_Create.sql +++ /dev/null @@ -1,42 +0,0 @@ -CREATE PROCEDURE [dbo].[ApiKey_Create] - @Id UNIQUEIDENTIFIER OUTPUT, - @ServiceAccountId UNIQUEIDENTIFIER, - @Name VARCHAR(200), - @ClientSecretHash VARCHAR(128), - @Scope NVARCHAR(4000), - @EncryptedPayload NVARCHAR(4000), - @Key VARCHAR(MAX), - @ExpireAt DATETIME2(7), - @CreationDate DATETIME2(7), - @RevisionDate DATETIME2(7) -AS -BEGIN - SET NOCOUNT ON - - INSERT INTO [dbo].[ApiKey] - ( - [Id], - [ServiceAccountId], - [Name], - [ClientSecretHash], - [Scope], - [EncryptedPayload], - [Key], - [ExpireAt], - [CreationDate], - [RevisionDate] - ) - VALUES - ( - @Id, - @ServiceAccountId, - @Name, - @ClientSecretHash, - @Scope, - @EncryptedPayload, - @Key, - @ExpireAt, - @CreationDate, - @RevisionDate - ) -END diff --git a/src/Sql/dbo_future/Tables/ApiKey.sql b/src/Sql/dbo_future/Tables/ApiKey.sql deleted file mode 100644 index 70400d5ed4..0000000000 --- a/src/Sql/dbo_future/Tables/ApiKey.sql +++ /dev/null @@ -1,18 +0,0 @@ -CREATE TABLE [dbo].[ApiKey] ( - [Id] UNIQUEIDENTIFIER, - [ServiceAccountId] UNIQUEIDENTIFIER NULL, - [Name] VARCHAR(200) NOT NULL, - [ClientSecretHash] VARCHAR(128) NULL, - [Scope] NVARCHAR (4000) NOT NULL, - [EncryptedPayload] NVARCHAR (4000) NOT NULL, - [Key] VARCHAR (MAX) NOT NULL, - [ExpireAt] DATETIME2(7) NULL, - [CreationDate] DATETIME2(7) NOT NULL, - [RevisionDate] DATETIME2(7) NOT NULL, - CONSTRAINT [PK_ApiKey] PRIMARY KEY CLUSTERED ([Id] ASC), - CONSTRAINT [FK_ApiKey_ServiceAccountId] FOREIGN KEY ([ServiceAccountId]) REFERENCES [dbo].[ServiceAccount] ([Id]) -); - -GO -CREATE NONCLUSTERED INDEX [IX_ApiKey_ServiceAccountId] - ON [dbo].[ApiKey]([ServiceAccountId] ASC); diff --git a/util/Migrator/DbScripts_data_migration/2023-05-16_00_ClientSecretHashDataMigration.sql b/util/Migrator/DbScripts/2023-08-10_00_ClientSecretHashDataMigration.sql similarity index 88% rename from util/Migrator/DbScripts_data_migration/2023-05-16_00_ClientSecretHashDataMigration.sql rename to util/Migrator/DbScripts/2023-08-10_00_ClientSecretHashDataMigration.sql index 5d8261f930..66869d48b7 100644 --- a/util/Migrator/DbScripts_data_migration/2023-05-16_00_ClientSecretHashDataMigration.sql +++ b/util/Migrator/DbScripts/2023-08-10_00_ClientSecretHashDataMigration.sql @@ -1,7 +1,7 @@ /* This is the data migration script for the client secret hash updates. The initial migration util/Migrator/DbScripts/2023-05-16_00_ClientSecretHash.sql should be run prior. -The final migration is in util/Migrator/DbScripts_future/2023-06-FutureMigration.sql. +The final migration is in util/Migrator/DbScripts/2023-08-10_01_RemoveClientSecret */ IF COL_LENGTH('[dbo].[ApiKey]', 'ClientSecretHash') IS NOT NULL AND COL_LENGTH('[dbo].[ApiKey]', 'ClientSecret') IS NOT NULL BEGIN @@ -9,7 +9,7 @@ BEGIN -- Add index IF NOT EXISTS(SELECT name FROM sys.indexes WHERE name = 'IX_ApiKey_ClientSecretHash') BEGIN - CREATE NONCLUSTERED INDEX [IX_ApiKey_ClientSecretHash] + CREATE NONCLUSTERED INDEX [IX_ApiKey_ClientSecretHash] ON [dbo].[ApiKey]([ClientSecretHash] ASC) WITH (ONLINE = ON) END @@ -30,7 +30,7 @@ BEGIN WHERE [ClientSecretHash] IS NULL SET @BatchSize = @@ROWCOUNT - + COMMIT TRANSACTION Migrate_ClientSecretHash END diff --git a/util/Migrator/DbScripts_future/2023-06-FutureMigration.sql b/util/Migrator/DbScripts/2023-08-10_01_RemoveClientSecret.sql similarity index 96% rename from util/Migrator/DbScripts_future/2023-06-FutureMigration.sql rename to util/Migrator/DbScripts/2023-08-10_01_RemoveClientSecret.sql index 273c4625f1..f9d4b82185 100644 --- a/util/Migrator/DbScripts_future/2023-06-FutureMigration.sql +++ b/util/Migrator/DbScripts/2023-08-10_01_RemoveClientSecret.sql @@ -36,7 +36,7 @@ AS BEGIN SET NOCOUNT ON - INSERT INTO [dbo].[ApiKey] + INSERT INTO [dbo].[ApiKey] ( [Id], [ServiceAccountId], @@ -49,7 +49,7 @@ BEGIN [CreationDate], [RevisionDate] ) - VALUES + VALUES ( @Id, @ServiceAccountId, From 6d078851dc2cc353919afdb89f7c021b57941043 Mon Sep 17 00:00:00 2001 From: Joseph Flinn <58369717+joseph-flinn@users.noreply.github.com> Date: Tue, 29 Aug 2023 15:22:09 -0700 Subject: [PATCH 15/18] Devops 1539 optimize server build (#3237) * Switch server build back to only triggering the Unified and relying on Slack notifications to notify of a failure * Run actionlint over workflow files * pin actions/github-script hash --- .github/workflows/build.yml | 38 ++++++++++++++++++++++++----- .github/workflows/protect-files.yml | 2 +- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7562dd354a..22e5fad21e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -514,13 +514,39 @@ jobs: path: util/MsSqlMigratorUtility/obj/build-output/publish/MsSqlMigratorUtility if-no-files-found: error + self-host-build: - name: Self-host build - needs: build-docker - uses: bitwarden/self-host/.github/workflows/build-unified.yml@master - with: - server_branch: ${{ github.ref_name }} - secrets: inherit + name: Trigger self-host build + runs-on: ubuntu-22.04 + needs: + - build-docker + steps: + - name: Login to Azure - CI Subscription + uses: Azure/login@1f63701bf3e6892515f1b7ce2d2bf1708b46beaf # v1.4.3 + with: + creds: ${{ secrets.AZURE_KV_CI_SERVICE_PRINCIPAL }} + + - name: Retrieve github PAT secrets + id: retrieve-secret-pat + uses: bitwarden/gh-actions/get-keyvault-secrets@f096207b7a2f31723165aee6ad03e91716686e78 + with: + keyvault: "bitwarden-ci" + secrets: "github-pat-bitwarden-devops-bot-repo-scope" + + - name: Trigger self-host build + uses: actions/github-script@d7906e4ad0b1822421a7e6a35d5ca353c962f410 # v6.4.1 + with: + github-token: ${{ steps.retrieve-secret-pat.outputs.github-pat-bitwarden-devops-bot-repo-scope }} + script: | + await github.rest.actions.createWorkflowDispatch({ + owner: 'bitwarden', + repo: 'self-host', + workflow_id: 'build-unified.yml', + ref: 'master', + inputs: { + server_branch: '${{ github.ref }}' + } + }) check-failures: name: Check for failures diff --git a/.github/workflows/protect-files.yml b/.github/workflows/protect-files.yml index 25a019c76d..22f8bc57c0 100644 --- a/.github/workflows/protect-files.yml +++ b/.github/workflows/protect-files.yml @@ -49,7 +49,7 @@ jobs: done - name: Add label to pull request - if: contains(steps.check-changes.outputs.changes_detected, true) + if: contains(steps.check-changes.outputs.changes_detected, 'true') uses: andymckay/labeler@e6c4322d0397f3240f0e7e30a33b5c5df2d39e90 # 1.0.4 with: add-labels: ${{ matrix.label }} From e679d3127abfd976e3af3d02783d869da77fb590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Wed, 30 Aug 2023 07:23:45 +0100 Subject: [PATCH 16/18] [AC-1585] Automatically verify managed members on an organization with a verified domain (#3207) --- .../src/Sso/Controllers/AccountController.cs | 15 ++++++++++- src/Core/Utilities/CoreHelpers.cs | 25 +++++++++++++++---- test/Core.Test/Utilities/CoreHelpersTests.cs | 21 ++++++++++++++++ 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/bitwarden_license/src/Sso/Controllers/AccountController.cs b/bitwarden_license/src/Sso/Controllers/AccountController.cs index f9bb77837d..40d4d1f42a 100644 --- a/bitwarden_license/src/Sso/Controllers/AccountController.cs +++ b/bitwarden_license/src/Sso/Controllers/AccountController.cs @@ -47,6 +47,7 @@ public class AccountController : Controller private readonly IGlobalSettings _globalSettings; private readonly Core.Services.IEventService _eventService; private readonly IDataProtectorTokenFactory _dataProtector; + private readonly IOrganizationDomainRepository _organizationDomainRepository; public AccountController( IAuthenticationSchemeProvider schemeProvider, @@ -65,7 +66,8 @@ public class AccountController : Controller UserManager userManager, IGlobalSettings globalSettings, Core.Services.IEventService eventService, - IDataProtectorTokenFactory dataProtector) + IDataProtectorTokenFactory dataProtector, + IOrganizationDomainRepository organizationDomainRepository) { _schemeProvider = schemeProvider; _clientStore = clientStore; @@ -84,6 +86,7 @@ public class AccountController : Controller _eventService = eventService; _globalSettings = globalSettings; _dataProtector = dataProtector; + _organizationDomainRepository = organizationDomainRepository; } [HttpGet] @@ -513,11 +516,21 @@ public class AccountController : Controller } } + // If the email domain is verified, we can mark the email as verified + var emailVerified = false; + var emailDomain = CoreHelpers.GetEmailDomain(email); + if (!string.IsNullOrWhiteSpace(emailDomain)) + { + var organizationDomain = await _organizationDomainRepository.GetDomainByOrgIdAndDomainNameAsync(orgId, emailDomain); + emailVerified = organizationDomain?.VerifiedDate.HasValue ?? false; + } + // Create user record - all existing user flows are handled above var user = new User { Name = name, Email = email, + EmailVerified = emailVerified, ApiKey = CoreHelpers.SecureRandomString(30) }; await _userService.RegisterUserAsync(user); diff --git a/src/Core/Utilities/CoreHelpers.cs b/src/Core/Utilities/CoreHelpers.cs index addc213390..c128fa8e46 100644 --- a/src/Core/Utilities/CoreHelpers.cs +++ b/src/Core/Utilities/CoreHelpers.cs @@ -50,20 +50,20 @@ public static class CoreHelpers { var guidArray = startingGuid.ToByteArray(); - // Get the days and milliseconds which will be used to build the byte string + // Get the days and milliseconds which will be used to build the byte string var days = new TimeSpan(time.Ticks - _baseDateTicks); var msecs = time.TimeOfDay; - // Convert to a byte array - // Note that SQL Server is accurate to 1/300th of a millisecond so we divide by 3.333333 + // Convert to a byte array + // Note that SQL Server is accurate to 1/300th of a millisecond so we divide by 3.333333 var daysArray = BitConverter.GetBytes(days.Days); var msecsArray = BitConverter.GetBytes((long)(msecs.TotalMilliseconds / 3.333333)); - // Reverse the bytes to match SQL Servers ordering + // Reverse the bytes to match SQL Servers ordering Array.Reverse(daysArray); Array.Reverse(msecsArray); - // Copy the bytes into the guid + // Copy the bytes into the guid Array.Copy(daysArray, daysArray.Length - 2, guidArray, guidArray.Length - 6, 2); Array.Copy(msecsArray, msecsArray.Length - 4, guidArray, guidArray.Length - 4, 4); @@ -817,4 +817,19 @@ public static class CoreHelpers .ToString(); } + + public static string GetEmailDomain(string email) + { + if (!string.IsNullOrWhiteSpace(email)) + { + var emailParts = email.Split('@', StringSplitOptions.RemoveEmptyEntries); + + if (emailParts.Length == 2) + { + return emailParts[1].Trim(); + } + } + + return null; + } } diff --git a/test/Core.Test/Utilities/CoreHelpersTests.cs b/test/Core.Test/Utilities/CoreHelpersTests.cs index b4a4034d93..4def694042 100644 --- a/test/Core.Test/Utilities/CoreHelpersTests.cs +++ b/test/Core.Test/Utilities/CoreHelpersTests.cs @@ -416,4 +416,25 @@ public class CoreHelpersTests { Assert.Equal(expected, CoreHelpers.ObfuscateEmail(input)); } + + [Theory] + [InlineData("user@example.com")] + [InlineData("user@example.com ")] + [InlineData("user.name@example.com")] + public void GetEmailDomain_Success(string email) + { + Assert.Equal("example.com", CoreHelpers.GetEmailDomain(email)); + } + + [Theory] + [InlineData("")] + [InlineData(null)] + [InlineData("userexample.com")] + [InlineData("user@")] + [InlineData("@example.com")] + [InlineData("user@ex@ample.com")] + public void GetEmailDomain_ReturnsNull(string wrongEmail) + { + Assert.Null(CoreHelpers.GetEmailDomain(wrongEmail)); + } } From 257efe3f9aa00139beeff912d93a86598376d570 Mon Sep 17 00:00:00 2001 From: Kyle Spearrin Date: Wed, 30 Aug 2023 12:11:33 -0400 Subject: [PATCH 17/18] [PM-3563] Prevent org name from injecting HTML into FD notes (#3219) * prevent org name from injecting HTML into FD notes * htmlencode --- src/Billing/Controllers/FreshdeskController.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Billing/Controllers/FreshdeskController.cs b/src/Billing/Controllers/FreshdeskController.cs index c45407b53d..1b6ddea429 100644 --- a/src/Billing/Controllers/FreshdeskController.cs +++ b/src/Billing/Controllers/FreshdeskController.cs @@ -1,6 +1,7 @@ using System.ComponentModel.DataAnnotations; using System.Reflection; using System.Text; +using System.Web; using Bit.Billing.Models; using Bit.Core.Repositories; using Bit.Core.Settings; @@ -77,7 +78,9 @@ public class FreshdeskController : Controller foreach (var org in orgs) { - var orgNote = $"{org.Name} ({org.Seats.GetValueOrDefault()}): " + + // Prevent org names from injecting any additional HTML + var orgName = HttpUtility.HtmlEncode(org.Name); + var orgNote = $"{orgName} ({org.Seats.GetValueOrDefault()}): " + $"{_globalSettings.BaseServiceUri.Admin}/organizations/edit/{org.Id}"; note += $"
  • Org, {orgNote}
    • "; if (!customFields.Any(kvp => kvp.Key == _billingSettings.FreshDesk.OrgFieldName)) From ba53208c93be6e99f6fba7b2b48ee8479157653a Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Wed, 30 Aug 2023 13:39:37 -0400 Subject: [PATCH 18/18] Add feature flag for enabling v2 of Autofill. (#3049) Co-authored-by: Cesar Gonzalez --- src/Core/Constants.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 5cd0349291..8db721d17e 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -37,6 +37,7 @@ public static class FeatureFlagKeys public const string DisplayLowKdfIterationWarning = "display-kdf-iteration-warning"; public const string TrustedDeviceEncryption = "trusted-device-encryption"; public const string SecretsManagerBilling = "sm-ga-billing"; + public const string AutofillV2 = "autofill-v2"; public static List GetAllKeys() {