From 6db02e2e5c1a49c24a053780c1b6f9ce9120764a Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Thu, 31 Aug 2023 11:25:23 -0700 Subject: [PATCH 1/4] Make WebAuthn a Free Method (#3217) * make webauthn method free * flip premium params * remove premium checks --- src/Api/Auth/Controllers/TwoFactorController.cs | 8 ++++---- src/Core/Auth/Identity/WebAuthnTokenProvider.cs | 10 +--------- src/Core/Auth/Models/TwoFactorProvider.cs | 1 - 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/Api/Auth/Controllers/TwoFactorController.cs b/src/Api/Auth/Controllers/TwoFactorController.cs index 0c8822219a..884f2939ba 100644 --- a/src/Api/Auth/Controllers/TwoFactorController.cs +++ b/src/Api/Auth/Controllers/TwoFactorController.cs @@ -236,7 +236,7 @@ public class TwoFactorController : Controller [HttpPost("get-webauthn")] public async Task GetWebAuthn([FromBody] SecretVerificationRequestModel model) { - var user = await CheckAsync(model, true); + var user = await CheckAsync(model, false); var response = new TwoFactorWebAuthnResponseModel(user); return response; } @@ -245,7 +245,7 @@ public class TwoFactorController : Controller [ApiExplorerSettings(IgnoreApi = true)] // Disable Swagger due to CredentialCreateOptions not converting properly public async Task GetWebAuthnChallenge([FromBody] SecretVerificationRequestModel model) { - var user = await CheckAsync(model, true); + var user = await CheckAsync(model, false); var reg = await _userService.StartWebAuthnRegistrationAsync(user); return reg; } @@ -254,7 +254,7 @@ public class TwoFactorController : Controller [HttpPost("webauthn")] public async Task PutWebAuthn([FromBody] TwoFactorWebAuthnRequestModel model) { - var user = await CheckAsync(model, true); + var user = await CheckAsync(model, false); var success = await _userService.CompleteWebAuthRegistrationAsync( user, model.Id.Value, model.Name, model.DeviceResponse); @@ -271,7 +271,7 @@ public class TwoFactorController : Controller public async Task DeleteWebAuthn( [FromBody] TwoFactorWebAuthnDeleteRequestModel model) { - var user = await CheckAsync(model, true); + var user = await CheckAsync(model, false); await _userService.DeleteWebAuthnKeyAsync(user, model.Id.Value); var response = new TwoFactorWebAuthnResponseModel(user); return response; diff --git a/src/Core/Auth/Identity/WebAuthnTokenProvider.cs b/src/Core/Auth/Identity/WebAuthnTokenProvider.cs index 8ada74b697..ef6535de74 100644 --- a/src/Core/Auth/Identity/WebAuthnTokenProvider.cs +++ b/src/Core/Auth/Identity/WebAuthnTokenProvider.cs @@ -28,10 +28,6 @@ public class WebAuthnTokenProvider : IUserTwoFactorTokenProvider public async Task CanGenerateTwoFactorTokenAsync(UserManager manager, User user) { var userService = _serviceProvider.GetRequiredService(); - if (!(await userService.CanAccessPremium(user))) - { - return false; - } var webAuthnProvider = user.GetTwoFactorProvider(TwoFactorProviderType.WebAuthn); if (!HasProperMetaData(webAuthnProvider)) @@ -45,10 +41,6 @@ public class WebAuthnTokenProvider : IUserTwoFactorTokenProvider public async Task GenerateAsync(string purpose, UserManager manager, User user) { var userService = _serviceProvider.GetRequiredService(); - if (!(await userService.CanAccessPremium(user))) - { - return null; - } var provider = user.GetTwoFactorProvider(TwoFactorProviderType.WebAuthn); var keys = LoadKeys(provider); @@ -81,7 +73,7 @@ public class WebAuthnTokenProvider : IUserTwoFactorTokenProvider public async Task ValidateAsync(string purpose, string token, UserManager manager, User user) { var userService = _serviceProvider.GetRequiredService(); - if (!(await userService.CanAccessPremium(user)) || string.IsNullOrWhiteSpace(token)) + if (string.IsNullOrWhiteSpace(token)) { return false; } diff --git a/src/Core/Auth/Models/TwoFactorProvider.cs b/src/Core/Auth/Models/TwoFactorProvider.cs index 9cd7a98b07..498a70cb09 100644 --- a/src/Core/Auth/Models/TwoFactorProvider.cs +++ b/src/Core/Auth/Models/TwoFactorProvider.cs @@ -57,7 +57,6 @@ public class TwoFactorProvider case TwoFactorProviderType.Duo: case TwoFactorProviderType.YubiKey: case TwoFactorProviderType.U2f: // Keep to ensure old U2f keys are considered premium - case TwoFactorProviderType.WebAuthn: return true; default: return false; From 1f0251a34ed6d64bd200588e7f0b307451bd426f Mon Sep 17 00:00:00 2001 From: Kyle Spearrin Date: Fri, 1 Sep 2023 04:04:32 -0400 Subject: [PATCH 2/4] Remove org name from confirmation message (#3218) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Rui Tomé <108268980+r-tome@users.noreply.github.com> --- src/Admin/Views/Providers/Organizations.cshtml | 2 +- src/Admin/Views/Providers/_ProviderScripts.cshtml | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Admin/Views/Providers/Organizations.cshtml b/src/Admin/Views/Providers/Organizations.cshtml index a6cc20a1c8..06fc490bcc 100644 --- a/src/Admin/Views/Providers/Organizations.cshtml +++ b/src/Admin/Views/Providers/Organizations.cshtml @@ -44,7 +44,7 @@
@if (org.Status == OrganizationStatusType.Pending) { - + } diff --git a/src/Admin/Views/Providers/_ProviderScripts.cshtml b/src/Admin/Views/Providers/_ProviderScripts.cshtml index d74065d562..3ff2d9ae2c 100644 --- a/src/Admin/Views/Providers/_ProviderScripts.cshtml +++ b/src/Admin/Views/Providers/_ProviderScripts.cshtml @@ -1,14 +1,14 @@ \ No newline at end of file + From c271e2dae214583cb449f58aa67340a135432305 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Fri, 1 Sep 2023 09:10:02 +0100 Subject: [PATCH 3/4] [PM-3177] Extract IOrganizationService.UpdateUserGroupsAsync to a command (#3131) 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) * Create new query and add to save orgUser flow * Add tests * Resolved the checkmarx issues * [AC-1521] Address checkmarx security feedback (#3124) * Reinstate target attribute but add noopener noreferrer * Make same updates to service account adjustment code * Wire up to BulkEnableSecretsManager, delete separate command * Register new query * WIP: autoscaling in invite user flow * Resolve dependency issues * 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 * fix using refs * 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 * Fix tests * Refactor: fix the update object and use it to adjust values * [PM-3177] Created OrganizationUserCommand and UpdateOrganizationUserGroupsCommand * [PM-3177] Added unit tests for UpdateOrganizationUserGroupsCommand * [PM-3177] Replaced IOrganizationService.UpdateUserGroupsAsync with IUpdateOrganizationUserGroupsCommand * [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 * Handle autoscaling-specific errors * Revert name change to method * Fix typo * Exclude beta users from seat limits * Fix inaccurate comment * Update nullable properties to use .Value accessor * Fix tests * Add missing awaits * Move early return up * Revert based on currentOrganization * Remove duplicate migrations from incorrectly resolved merge * Add tests * [PM-3177] Remove Admin project referencing Commercial.Infrastructure.EntityFramework * [PM-3177] Removed abstract class OrganizationUserCommand. Added ValidateOrganizationUserUpdatePermissions to IOrganizationService --------- 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 --- .../ScimServiceCollectionExtensions.cs | 5 +- .../OrganizationUsersController.cs | 7 +- .../Public/Controllers/MembersController.cs | 8 ++- ...OrganizationServiceCollectionExtensions.cs | 7 ++ .../IUpdateOrganizationUserGroupsCommand.cs | 8 +++ .../UpdateOrganizationUserGroupsCommand.cs | 34 +++++++++ src/Core/Services/IOrganizationService.cs | 3 +- .../Implementations/OrganizationService.cs | 13 +--- ...pdateOrganizationUserGroupsCommandTests.cs | 50 +++++++++++++ .../Services/OrganizationServiceTests.cs | 70 +++++++++++++++++++ 10 files changed, 184 insertions(+), 21 deletions(-) create mode 100644 src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserGroupsCommand.cs create mode 100644 src/Core/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommand.cs create mode 100644 test/Core.Test/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommandTests.cs diff --git a/bitwarden_license/src/Scim/Utilities/ScimServiceCollectionExtensions.cs b/bitwarden_license/src/Scim/Utilities/ScimServiceCollectionExtensions.cs index fff70760d4..75b60a71fc 100644 --- a/bitwarden_license/src/Scim/Utilities/ScimServiceCollectionExtensions.cs +++ b/bitwarden_license/src/Scim/Utilities/ScimServiceCollectionExtensions.cs @@ -1,6 +1,4 @@ -using Bit.Core.OrganizationFeatures.OrganizationUsers; -using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; -using Bit.Scim.Groups; +using Bit.Scim.Groups; using Bit.Scim.Groups.Interfaces; using Bit.Scim.Users; using Bit.Scim.Users.Interfaces; @@ -23,7 +21,6 @@ public static class ScimServiceCollectionExtensions public static void AddScimUserCommands(this IServiceCollection services) { - services.AddScoped(); services.AddScoped(); services.AddScoped(); } diff --git a/src/Api/Controllers/OrganizationUsersController.cs b/src/Api/Controllers/OrganizationUsersController.cs index 3dbaea3ff9..d1c4ebacd6 100644 --- a/src/Api/Controllers/OrganizationUsersController.cs +++ b/src/Api/Controllers/OrganizationUsersController.cs @@ -30,6 +30,7 @@ public class OrganizationUsersController : Controller private readonly ICurrentContext _currentContext; private readonly ICountNewSmSeatsRequiredQuery _countNewSmSeatsRequiredQuery; private readonly IUpdateSecretsManagerSubscriptionCommand _updateSecretsManagerSubscriptionCommand; + private readonly IUpdateOrganizationUserGroupsCommand _updateOrganizationUserGroupsCommand; public OrganizationUsersController( IOrganizationRepository organizationRepository, @@ -41,7 +42,8 @@ public class OrganizationUsersController : Controller IPolicyRepository policyRepository, ICurrentContext currentContext, ICountNewSmSeatsRequiredQuery countNewSmSeatsRequiredQuery, - IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand) + IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand, + IUpdateOrganizationUserGroupsCommand updateOrganizationUserGroupsCommand) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; @@ -53,6 +55,7 @@ public class OrganizationUsersController : Controller _currentContext = currentContext; _countNewSmSeatsRequiredQuery = countNewSmSeatsRequiredQuery; _updateSecretsManagerSubscriptionCommand = updateSecretsManagerSubscriptionCommand; + _updateOrganizationUserGroupsCommand = updateOrganizationUserGroupsCommand; } [HttpGet("{id}")] @@ -308,7 +311,7 @@ public class OrganizationUsersController : Controller } var loggedInUserId = _userService.GetProperUserId(User); - await _organizationService.UpdateUserGroupsAsync(organizationUser, model.GroupIds.Select(g => new Guid(g)), loggedInUserId); + await _updateOrganizationUserGroupsCommand.UpdateUserGroupsAsync(organizationUser, model.GroupIds.Select(g => new Guid(g)), loggedInUserId); } [HttpPut("{userId}/reset-password-enrollment")] diff --git a/src/Api/Public/Controllers/MembersController.cs b/src/Api/Public/Controllers/MembersController.cs index 9d7ca86d3b..74dd4b83e1 100644 --- a/src/Api/Public/Controllers/MembersController.cs +++ b/src/Api/Public/Controllers/MembersController.cs @@ -3,6 +3,7 @@ using Bit.Api.Models.Public.Request; using Bit.Api.Models.Public.Response; using Bit.Core.Context; using Bit.Core.Models.Business; +using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; using Microsoft.AspNetCore.Authorization; @@ -19,19 +20,22 @@ public class MembersController : Controller private readonly IOrganizationService _organizationService; private readonly IUserService _userService; private readonly ICurrentContext _currentContext; + private readonly IUpdateOrganizationUserGroupsCommand _updateOrganizationUserGroupsCommand; public MembersController( IOrganizationUserRepository organizationUserRepository, IGroupRepository groupRepository, IOrganizationService organizationService, IUserService userService, - ICurrentContext currentContext) + ICurrentContext currentContext, + IUpdateOrganizationUserGroupsCommand updateOrganizationUserGroupsCommand) { _organizationUserRepository = organizationUserRepository; _groupRepository = groupRepository; _organizationService = organizationService; _userService = userService; _currentContext = currentContext; + _updateOrganizationUserGroupsCommand = updateOrganizationUserGroupsCommand; } /// @@ -183,7 +187,7 @@ public class MembersController : Controller { return new NotFoundResult(); } - await _organizationService.UpdateUserGroupsAsync(existingUser, model.GroupIds, null); + await _updateOrganizationUserGroupsCommand.UpdateUserGroupsAsync(existingUser, model.GroupIds, null); return new OkResult(); } diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index a534ed5bea..1756149236 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -45,6 +45,7 @@ public static class OrganizationServiceCollectionExtensions services.AddOrganizationLicenseCommandsQueries(); services.AddOrganizationDomainCommandsQueries(); services.AddOrganizationAuthCommands(); + services.AddOrganizationUserCommands(); services.AddOrganizationUserCommandsQueries(); services.AddBaseOrganizationSubscriptionCommandsQueries(); } @@ -81,6 +82,12 @@ public static class OrganizationServiceCollectionExtensions } } + private static void AddOrganizationUserCommands(this IServiceCollection services) + { + services.AddScoped(); + services.AddScoped(); + } + private static void AddOrganizationApiKeyCommandsQueries(this IServiceCollection services) { services.AddScoped(); diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserGroupsCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserGroupsCommand.cs new file mode 100644 index 0000000000..9dae932334 --- /dev/null +++ b/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserGroupsCommand.cs @@ -0,0 +1,8 @@ +using Bit.Core.Entities; + +namespace Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; + +public interface IUpdateOrganizationUserGroupsCommand +{ + Task UpdateUserGroupsAsync(OrganizationUser organizationUser, IEnumerable groupIds, Guid? loggedInUserId); +} diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommand.cs new file mode 100644 index 0000000000..7bb5814d6e --- /dev/null +++ b/src/Core/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommand.cs @@ -0,0 +1,34 @@ +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.Repositories; +using Bit.Core.Services; + +namespace Bit.Core.OrganizationFeatures.OrganizationUsers; + +public class UpdateOrganizationUserGroupsCommand : IUpdateOrganizationUserGroupsCommand +{ + private readonly IEventService _eventService; + private readonly IOrganizationService _organizationService; + private readonly IOrganizationUserRepository _organizationUserRepository; + + public UpdateOrganizationUserGroupsCommand( + IEventService eventService, + IOrganizationService organizationService, + IOrganizationUserRepository organizationUserRepository) + { + _eventService = eventService; + _organizationService = organizationService; + _organizationUserRepository = organizationUserRepository; + } + + public async Task UpdateUserGroupsAsync(OrganizationUser organizationUser, IEnumerable groupIds, Guid? loggedInUserId) + { + if (loggedInUserId.HasValue) + { + await _organizationService.ValidateOrganizationUserUpdatePermissions(organizationUser.OrganizationId, organizationUser.Type, null, organizationUser.GetPermissions()); + } + await _organizationUserRepository.UpdateGroupsAsync(organizationUser.Id, groupIds); + await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_UpdatedGroups); + } +} diff --git a/src/Core/Services/IOrganizationService.cs b/src/Core/Services/IOrganizationService.cs index 832aa9de4c..e085d04531 100644 --- a/src/Core/Services/IOrganizationService.cs +++ b/src/Core/Services/IOrganizationService.cs @@ -54,7 +54,6 @@ public interface IOrganizationService Task DeleteUserAsync(Guid organizationId, Guid userId); Task>> DeleteUsersAsync(Guid organizationId, IEnumerable organizationUserIds, Guid? deletingUserId); - Task UpdateUserGroupsAsync(OrganizationUser organizationUser, IEnumerable groupIds, Guid? loggedInUserId); Task UpdateUserResetPasswordEnrollmentAsync(Guid organizationId, Guid userId, string resetPasswordKey, Guid? callingUserId); Task ImportAsync(Guid organizationId, Guid? importingUserId, IEnumerable groups, IEnumerable newUsers, IEnumerable removeUserExternalIds, @@ -82,4 +81,6 @@ public interface IOrganizationService void ValidatePasswordManagerPlan(Models.StaticStore.Plan plan, OrganizationUpgrade upgrade); void ValidateSecretsManagerPlan(Models.StaticStore.Plan plan, OrganizationUpgrade upgrade); + Task ValidateOrganizationUserUpdatePermissions(Guid organizationId, OrganizationUserType newType, + OrganizationUserType? oldType, Permissions permissions); } diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index b7424a53e1..d9d12d1f9f 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -1582,17 +1582,6 @@ public class OrganizationService : IOrganizationService return hasOtherOwner; } - public async Task UpdateUserGroupsAsync(OrganizationUser organizationUser, IEnumerable groupIds, Guid? loggedInUserId) - { - if (loggedInUserId.HasValue) - { - await ValidateOrganizationUserUpdatePermissions(organizationUser.OrganizationId, organizationUser.Type, null, organizationUser.GetPermissions()); - } - await _organizationUserRepository.UpdateGroupsAsync(organizationUser.Id, groupIds); - await _eventService.LogOrganizationUserEventAsync(organizationUser, - EventType.OrganizationUser_UpdatedGroups); - } - public async Task UpdateUserResetPasswordEnrollmentAsync(Guid organizationId, Guid userId, string resetPasswordKey, Guid? callingUserId) { // Org User must be the same as the calling user and the organization ID associated with the user must match passed org ID @@ -2032,7 +2021,7 @@ public class OrganizationService : IOrganizationService } } - private async Task ValidateOrganizationUserUpdatePermissions(Guid organizationId, OrganizationUserType newType, OrganizationUserType? oldType, Permissions permissions) + public async Task ValidateOrganizationUserUpdatePermissions(Guid organizationId, OrganizationUserType newType, OrganizationUserType? oldType, Permissions permissions) { if (await _currentContext.OrganizationOwner(organizationId)) { diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommandTests.cs new file mode 100644 index 0000000000..542220560c --- /dev/null +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommandTests.cs @@ -0,0 +1,50 @@ +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.OrganizationFeatures.OrganizationUsers; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.OrganizationFeatures.OrganizationUsers; + +[SutProviderCustomize] +public class UpdateOrganizationUserGroupsCommandTests +{ + [Theory, BitAutoData] + public async Task UpdateUserGroups_Passes( + OrganizationUser organizationUser, + IEnumerable groupIds, + SutProvider sutProvider) + { + await sutProvider.Sut.UpdateUserGroupsAsync(organizationUser, groupIds, null); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .ValidateOrganizationUserUpdatePermissions(default, default, default, default); + await sutProvider.GetDependency().Received(1) + .UpdateGroupsAsync(organizationUser.Id, groupIds); + await sutProvider.GetDependency().Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_UpdatedGroups); + } + + [Theory, BitAutoData] + public async Task UpdateUserGroups_WithSavingUserId_Passes( + OrganizationUser organizationUser, + IEnumerable groupIds, + Guid savingUserId, + SutProvider sutProvider) + { + organizationUser.Permissions = null; + + await sutProvider.Sut.UpdateUserGroupsAsync(organizationUser, groupIds, savingUserId); + + await sutProvider.GetDependency().Received(1) + .ValidateOrganizationUserUpdatePermissions(organizationUser.OrganizationId, organizationUser.Type, null, organizationUser.GetPermissions()); + await sutProvider.GetDependency().Received(1) + .UpdateGroupsAsync(organizationUser.Id, groupIds); + await sutProvider.GetDependency().Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_UpdatedGroups); + } +} diff --git a/test/Core.Test/Services/OrganizationServiceTests.cs b/test/Core.Test/Services/OrganizationServiceTests.cs index 01073f4d2e..0389407da1 100644 --- a/test/Core.Test/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/Services/OrganizationServiceTests.cs @@ -1832,4 +1832,74 @@ public class OrganizationServiceTests sutProvider.Sut.ValidateSecretsManagerPlan(plan, signup); } + + [Theory] + [OrganizationInviteCustomize( + InviteeUserType = OrganizationUserType.Owner, + InvitorUserType = OrganizationUserType.Admin + ), BitAutoData] + public async Task ValidateOrganizationUserUpdatePermissions_WithAdminAddingOwner_Throws( + Guid organizationId, + OrganizationUserInvite organizationUserInvite, + SutProvider sutProvider) + { + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.ValidateOrganizationUserUpdatePermissions(organizationId, organizationUserInvite.Type.Value, null, organizationUserInvite.Permissions)); + + Assert.Contains("only an owner can configure another owner's account.", exception.Message.ToLowerInvariant()); + } + + [Theory] + [OrganizationInviteCustomize( + InviteeUserType = OrganizationUserType.Admin, + InvitorUserType = OrganizationUserType.Owner + ), BitAutoData] + public async Task ValidateOrganizationUserUpdatePermissions_WithoutManageUsersPermission_Throws( + Guid organizationId, + OrganizationUserInvite organizationUserInvite, + SutProvider sutProvider) + { + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.ValidateOrganizationUserUpdatePermissions(organizationId, organizationUserInvite.Type.Value, null, organizationUserInvite.Permissions)); + + Assert.Contains("your account does not have permission to manage users.", exception.Message.ToLowerInvariant()); + } + + [Theory] + [OrganizationInviteCustomize( + InviteeUserType = OrganizationUserType.Admin, + InvitorUserType = OrganizationUserType.Custom + ), BitAutoData] + public async Task ValidateOrganizationUserUpdatePermissions_WithCustomAddingAdmin_Throws( + Guid organizationId, + OrganizationUserInvite organizationUserInvite, + SutProvider sutProvider) + { + sutProvider.GetDependency().ManageUsers(organizationId).Returns(true); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.ValidateOrganizationUserUpdatePermissions(organizationId, organizationUserInvite.Type.Value, null, organizationUserInvite.Permissions)); + + Assert.Contains("custom users can not manage admins or owners.", exception.Message.ToLowerInvariant()); + } + + [Theory] + [OrganizationInviteCustomize( + InviteeUserType = OrganizationUserType.Custom, + InvitorUserType = OrganizationUserType.Custom + ), BitAutoData] + public async Task ValidateOrganizationUserUpdatePermissions_WithCustomAddingUser_WithoutPermissions_Throws( + Guid organizationId, + OrganizationUserInvite organizationUserInvite, + SutProvider sutProvider) + { + var invitePermissions = new Permissions { AccessReports = true }; + sutProvider.GetDependency().ManageUsers(organizationId).Returns(true); + sutProvider.GetDependency().AccessReports(organizationId).Returns(false); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.ValidateOrganizationUserUpdatePermissions(organizationId, organizationUserInvite.Type.Value, null, invitePermissions)); + + Assert.Contains("custom users can only grant the same custom permissions that they have.", exception.Message.ToLowerInvariant()); + } } From d0cf8204c79c26233f187dfe483057ed3f0ba5fb Mon Sep 17 00:00:00 2001 From: Matt Bishop Date: Fri, 1 Sep 2023 07:06:21 -0400 Subject: [PATCH 4/4] [PM-3708] Allow local overrides for flag values (#3243) * Allow local overrides for flag values * Rename helper method --- src/Core/Constants.cs | 9 ++++ .../LaunchDarklyFeatureService.cs | 46 +++++++++++-------- .../LaunchDarklyFeatureServiceTests.cs | 8 ---- 3 files changed, 37 insertions(+), 26 deletions(-) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 8db721d17e..132b1db447 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -46,4 +46,13 @@ public static class FeatureFlagKeys .Select(x => (string)x.GetRawConstantValue()) .ToList(); } + + public static Dictionary GetLocalOverrideFlagValues() + { + // place overriding values when needed locally (offline), or return null + return new Dictionary() + { + { TrustedDeviceEncryption, "true" } + }; + } } diff --git a/src/Core/Services/Implementations/LaunchDarklyFeatureService.cs b/src/Core/Services/Implementations/LaunchDarklyFeatureService.cs index adf2a6e96a..501db21d82 100644 --- a/src/Core/Services/Implementations/LaunchDarklyFeatureService.cs +++ b/src/Core/Services/Implementations/LaunchDarklyFeatureService.cs @@ -29,24 +29,12 @@ public class LaunchDarklyFeatureService : IFeatureService, IDisposable // support configuration directly from settings else if (globalSettings.LaunchDarkly?.FlagValues?.Any() is true) { - var source = TestData.DataSource(); - foreach (var kvp in globalSettings.LaunchDarkly.FlagValues) - { - if (bool.TryParse(kvp.Value, out bool boolValue)) - { - source.Update(source.Flag(kvp.Key).ValueForAll(LaunchDarkly.Sdk.LdValue.Of(boolValue))); - } - else if (int.TryParse(kvp.Value, out int intValue)) - { - source.Update(source.Flag(kvp.Key).ValueForAll(LaunchDarkly.Sdk.LdValue.Of(intValue))); - } - else - { - source.Update(source.Flag(kvp.Key).ValueForAll(LaunchDarkly.Sdk.LdValue.Of(kvp.Value))); - } - } - - ldConfig.DataSource(source); + ldConfig.DataSource(BuildDataSource(globalSettings.LaunchDarkly.FlagValues)); + } + // support local overrides + else if (FeatureFlagKeys.GetLocalOverrideFlagValues()?.Any() is true) + { + ldConfig.DataSource(BuildDataSource(FeatureFlagKeys.GetLocalOverrideFlagValues())); } else { @@ -187,4 +175,26 @@ public class LaunchDarklyFeatureService : IFeatureService, IDisposable return builder.Build(); } + + private TestData BuildDataSource(Dictionary values) + { + var source = TestData.DataSource(); + foreach (var kvp in values) + { + if (bool.TryParse(kvp.Value, out bool boolValue)) + { + source.Update(source.Flag(kvp.Key).ValueForAll(LaunchDarkly.Sdk.LdValue.Of(boolValue))); + } + else if (int.TryParse(kvp.Value, out int intValue)) + { + source.Update(source.Flag(kvp.Key).ValueForAll(LaunchDarkly.Sdk.LdValue.Of(intValue))); + } + else + { + source.Update(source.Flag(kvp.Key).ValueForAll(LaunchDarkly.Sdk.LdValue.Of(kvp.Value))); + } + } + + return source; + } } diff --git a/test/Core.Test/Services/LaunchDarklyFeatureServiceTests.cs b/test/Core.Test/Services/LaunchDarklyFeatureServiceTests.cs index 3644dc805b..37ced167be 100644 --- a/test/Core.Test/Services/LaunchDarklyFeatureServiceTests.cs +++ b/test/Core.Test/Services/LaunchDarklyFeatureServiceTests.cs @@ -20,14 +20,6 @@ public class LaunchDarklyFeatureServiceTests .Create(); } - [Fact] - public void Offline_WhenSelfHost() - { - var sutProvider = GetSutProvider(new Core.Settings.GlobalSettings() { SelfHosted = true }); - - Assert.False(sutProvider.Sut.IsOnline()); - } - [Theory, BitAutoData] public void DefaultFeatureValue_WhenSelfHost(string key) {