From 9321515eca750c8f9bc84126912827ed11d36aa7 Mon Sep 17 00:00:00 2001 From: Conner Turnbull <133619638+cturnbull-bitwarden@users.noreply.github.com> Date: Mon, 16 Dec 2024 08:04:05 -0500 Subject: [PATCH 01/11] [PM-10873] Updated errors thrown when creating organization on selfhost to be more specific (#5007) * Updated errors thrown when creating organization on selfhost to be more specific * Added additional validation to ensure that the license type is accurate --------- Co-authored-by: Matt Bishop --- .../Implementations/OrganizationService.cs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index 49f339cc9ab4..1cf22b23adbc 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -519,17 +519,29 @@ private async Task ValidateSignUpPoliciesAsync(Guid ownerId) OrganizationLicense license, User owner, string ownerKey, string collectionName, string publicKey, string privateKey) { + if (license.LicenseType != LicenseType.Organization) + { + throw new BadRequestException("Premium licenses cannot be applied to an organization. " + + "Upload this license from your personal account settings page."); + } + var claimsPrincipal = _licensingService.GetClaimsPrincipalFromLicense(license); var canUse = license.CanUse(_globalSettings, _licensingService, claimsPrincipal, out var exception); + if (!canUse) { throw new BadRequestException(exception); } - if (license.PlanType != PlanType.Custom && - StaticStore.Plans.FirstOrDefault(p => p.Type == license.PlanType && !p.Disabled) == null) + var plan = StaticStore.Plans.FirstOrDefault(p => p.Type == license.PlanType); + if (plan is null) { - throw new BadRequestException("Plan not found."); + throw new BadRequestException($"Server must be updated to support {license.Plan}."); + } + + if (license.PlanType != PlanType.Custom && plan.Disabled) + { + throw new BadRequestException($"Plan {plan.Name} is disabled."); } var enabledOrgs = await _organizationRepository.GetManyByEnabledAsync(); From d0c72a34f12baadede2c191a814315f153872b4a Mon Sep 17 00:00:00 2001 From: Opeyemi Date: Mon, 16 Dec 2024 14:21:05 +0000 Subject: [PATCH 02/11] Update SH Unified Build trigger (#5154) * Update SH Unified Build trigger * make value a boolean --- .github/workflows/build.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 420b9b63750a..eb644fe8b5e2 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -580,6 +580,7 @@ jobs: ref: 'main', inputs: { server_branch: process.env.GITHUB_REF + is_workflow_call: true } }); From 8994d1d7dd019e857d68e8ae46262f8daea82058 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 16 Dec 2024 15:11:56 +0000 Subject: [PATCH 03/11] [deps] Tools: Update aws-sdk-net monorepo (#5126) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Daniel James Smith <2670567+djsmith85@users.noreply.github.com> --- src/Core/Core.csproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/Core.csproj b/src/Core/Core.csproj index fd4d8cc7e124..9049f94dcfb9 100644 --- a/src/Core/Core.csproj +++ b/src/Core/Core.csproj @@ -21,8 +21,8 @@ - - + + From c446ac86fead55e6d7b762ca44ea814d9d53e80b Mon Sep 17 00:00:00 2001 From: Ike <137194738+ike-kottlowski@users.noreply.github.com> Date: Mon, 16 Dec 2024 07:57:56 -0800 Subject: [PATCH 04/11] [PM-12512] Add Endpoint to allow users to request a new device otp (#5146) feat(NewDeviceVerification): Added a resend new device OTP endpoint and method for the IUserService as well as wrote test for new methods for the user service. --- .../Auth/Controllers/AccountsController.cs | 8 + ...henticatedSecretVerificatioRequestModel.cs | 12 ++ src/Core/Services/IUserService.cs | 2 +- .../Services/Implementations/UserService.cs | 16 +- test/Core.Test/Services/UserServiceTests.cs | 171 ++++++++++++++---- 5 files changed, 171 insertions(+), 38 deletions(-) create mode 100644 src/Api/Auth/Models/Request/Accounts/UnauthenticatedSecretVerificatioRequestModel.cs diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index a94e170cbbab..1c08ce4f7394 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -961,6 +961,14 @@ public async Task VerifyOTP([FromBody] VerifyOTPRequestModel model) } } + [RequireFeature(FeatureFlagKeys.NewDeviceVerification)] + [AllowAnonymous] + [HttpPost("resend-new-device-otp")] + public async Task ResendNewDeviceOtpAsync([FromBody] UnauthenticatedSecretVerificatioRequestModel request) + { + await _userService.ResendNewDeviceVerificationEmail(request.Email, request.Secret); + } + private async Task> GetOrganizationIdsManagingUserAsync(Guid userId) { var organizationManagingUser = await _userService.GetOrganizationsManagingUserAsync(userId); diff --git a/src/Api/Auth/Models/Request/Accounts/UnauthenticatedSecretVerificatioRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/UnauthenticatedSecretVerificatioRequestModel.cs new file mode 100644 index 000000000000..629896b8c4aa --- /dev/null +++ b/src/Api/Auth/Models/Request/Accounts/UnauthenticatedSecretVerificatioRequestModel.cs @@ -0,0 +1,12 @@ +using System.ComponentModel.DataAnnotations; +using Bit.Core.Utilities; + +namespace Bit.Api.Auth.Models.Request.Accounts; + +public class UnauthenticatedSecretVerificatioRequestModel : SecretVerificationRequestModel +{ + [Required] + [StrictEmailAddress] + [StringLength(256)] + public string Email { get; set; } +} diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index 65bec5ea9f78..f0ba5352662e 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -76,7 +76,7 @@ Task UpdatePasswordHash(User user, string newPassword, Task SendOTPAsync(User user); Task VerifyOTPAsync(User user, string token); Task VerifySecretAsync(User user, string secret, bool isSettingMFA = false); - + Task ResendNewDeviceVerificationEmail(string email, string secret); void SetTwoFactorProvider(User user, TwoFactorProviderType type, bool setEnabled = true); diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 2bc81959b9b3..cb17d6e26b45 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -1407,7 +1407,7 @@ public async Task RotateApiKeyAsync(User user) public async Task SendOTPAsync(User user) { - if (user.Email == null) + if (string.IsNullOrEmpty(user.Email)) { throw new BadRequestException("No user email."); } @@ -1450,6 +1450,20 @@ public async Task VerifySecretAsync(User user, string secret, bool isSetti return isVerified; } + public async Task ResendNewDeviceVerificationEmail(string email, string secret) + { + var user = await _userRepository.GetByEmailAsync(email); + if (user == null) + { + return; + } + + if (await VerifySecretAsync(user, secret)) + { + await SendOTPAsync(user); + } + } + private async Task SendAppropriateWelcomeEmailAsync(User user, string initiationPath) { var isFromMarketingWebsite = initiationPath.Contains("Secrets Manager trial"); diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index de2a5187171c..e44609c6d64c 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -13,6 +13,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.Models.Data.Organizations; using Bit.Core.Models.Data.Organizations.OrganizationUsers; @@ -240,42 +241,7 @@ public async Task VerifySecretAsync_Works( }); // HACK: SutProvider is being weird about not injecting the IPasswordHasher that I configured - var sut = new UserService( - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency>(), - sutProvider.GetDependency>(), - sutProvider.GetDependency>(), - sutProvider.GetDependency>>(), - sutProvider.GetDependency>>(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency>>(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - new FakeDataProtectorTokenFactory(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency() - ); + var sut = RebuildSut(sutProvider); var actualIsVerified = await sut.VerifySecretAsync(user, secret); @@ -522,6 +488,99 @@ await sutProvider.GetDependency() .SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(organization2.DisplayName(), user.Email); } + [Theory, BitAutoData] + public async Task ResendNewDeviceVerificationEmail_UserNull_SendOTPAsyncNotCalled( + SutProvider sutProvider, string email, string secret) + { + sutProvider.GetDependency() + .GetByEmailAsync(email) + .Returns(null as User); + + await sutProvider.Sut.ResendNewDeviceVerificationEmail(email, secret); + + await sutProvider.GetDependency() + .DidNotReceive() + .SendOTPEmailAsync(Arg.Any(), Arg.Any()); + } + + [Theory, BitAutoData] + public async Task ResendNewDeviceVerificationEmail_SecretNotValid_SendOTPAsyncNotCalled( + SutProvider sutProvider, string email, string secret) + { + sutProvider.GetDependency() + .GetByEmailAsync(email) + .Returns(null as User); + + await sutProvider.Sut.ResendNewDeviceVerificationEmail(email, secret); + + await sutProvider.GetDependency() + .DidNotReceive() + .SendOTPEmailAsync(Arg.Any(), Arg.Any()); + } + + [Theory, BitAutoData] + public async Task ResendNewDeviceVerificationEmail_SendsToken_Success( + SutProvider sutProvider, User user) + { + // Arrange + var testPassword = "test_password"; + var tokenProvider = SetupFakeTokenProvider(sutProvider, user); + SetupUserAndDevice(user, true); + + // Setup the fake password verification + var substitutedUserPasswordStore = Substitute.For>(); + substitutedUserPasswordStore + .GetPasswordHashAsync(user, Arg.Any()) + .Returns((ci) => + { + return Task.FromResult("hashed_test_password"); + }); + + sutProvider.SetDependency>(substitutedUserPasswordStore, "store"); + + sutProvider.GetDependency>("passwordHasher") + .VerifyHashedPassword(user, "hashed_test_password", testPassword) + .Returns((ci) => + { + return PasswordVerificationResult.Success; + }); + + sutProvider.GetDependency() + .GetByEmailAsync(user.Email) + .Returns(user); + + // HACK: SutProvider is being weird about not injecting the IPasswordHasher that I configured + var sut = RebuildSut(sutProvider); + + await sut.ResendNewDeviceVerificationEmail(user.Email, testPassword); + + await sutProvider.GetDependency() + .Received(1) + .SendOTPEmailAsync(user.Email, Arg.Any()); + } + + [Theory] + [BitAutoData("")] + [BitAutoData("null")] + public async Task SendOTPAsync_UserEmailNull_ThrowsBadRequest( + string email, + SutProvider sutProvider, User user) + { + user.Email = email == "null" ? null : ""; + var expectedMessage = "No user email."; + try + { + await sutProvider.Sut.SendOTPAsync(user); + } + catch (BadRequestException ex) + { + Assert.Equal(ex.Message, expectedMessage); + await sutProvider.GetDependency() + .DidNotReceive() + .SendOTPEmailAsync(Arg.Any(), Arg.Any()); + } + } + private static void SetupUserAndDevice(User user, bool shouldHavePassword) { @@ -573,4 +632,44 @@ private static IUserTwoFactorTokenProvider SetupFakeTokenProvider(SutProvi return fakeUserTwoFactorProvider; } + + private IUserService RebuildSut(SutProvider sutProvider) + { + return new UserService( + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency>(), + sutProvider.GetDependency>(), + sutProvider.GetDependency>(), + sutProvider.GetDependency>>(), + sutProvider.GetDependency>>(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency>>(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + new FakeDataProtectorTokenFactory(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency(), + sutProvider.GetDependency() + ); + } } From d88a103fbc8baa60147eaf377d05eeeb87b1eb7a Mon Sep 17 00:00:00 2001 From: Daniel James Smith <2670567+djsmith85@users.noreply.github.com> Date: Mon, 16 Dec 2024 17:11:37 +0100 Subject: [PATCH 05/11] Move CSVHelper under billing ownership (#5156) Co-authored-by: Daniel James Smith --- .github/renovate.json | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/renovate.json b/.github/renovate.json index 5779b28edb41..4ae3cc19d8b4 100644 --- a/.github/renovate.json +++ b/.github/renovate.json @@ -63,6 +63,7 @@ "BitPay.Light", "Braintree", "coverlet.collector", + "CsvHelper", "FluentAssertions", "Kralizek.AutoFixture.Extensions.MockHttp", "Microsoft.AspNetCore.Mvc.Testing", From 7637cbe12ac67006db73573f3eb9a1010a7cb153 Mon Sep 17 00:00:00 2001 From: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> Date: Mon, 16 Dec 2024 12:01:09 -0600 Subject: [PATCH 06/11] [PM-13362] Add private key regeneration endpoint (#4929) * Add new RegenerateUserAsymmetricKeysCommand * add new command tests * Add regen controller * Add regen controller tests * add feature flag * Add push notification to sync new asymmetric keys to other devices --- .../AccountsKeyManagementController.cs | 50 +++++ .../Requests/KeyRegenerationRequestModel.cs | 23 ++ src/Core/Constants.cs | 1 + .../IRegenerateUserAsymmetricKeysCommand.cs | 13 ++ .../RegenerateUserAsymmetricKeysCommand.cs | 71 +++++++ ...eyManagementServiceCollectionExtensions.cs | 18 ++ .../Utilities/ServiceCollectionExtensions.cs | 2 + .../Helpers/LoginHelper.cs | 6 + .../AccountsKeyManagementControllerTests.cs | 164 +++++++++++++++ .../AccountsKeyManagementControllerTests.cs | 96 +++++++++ ...egenerateUserAsymmetricKeysCommandTests.cs | 197 ++++++++++++++++++ 11 files changed, 641 insertions(+) create mode 100644 src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs create mode 100644 src/Api/KeyManagement/Models/Requests/KeyRegenerationRequestModel.cs create mode 100644 src/Core/KeyManagement/Commands/Interfaces/IRegenerateUserAsymmetricKeysCommand.cs create mode 100644 src/Core/KeyManagement/Commands/RegenerateUserAsymmetricKeysCommand.cs create mode 100644 src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs create mode 100644 test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs create mode 100644 test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs create mode 100644 test/Core.Test/KeyManagement/Commands/RegenerateUserAsymmetricKeysCommandTests.cs diff --git a/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs b/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs new file mode 100644 index 000000000000..b8d5e3094931 --- /dev/null +++ b/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs @@ -0,0 +1,50 @@ +#nullable enable +using Bit.Api.KeyManagement.Models.Requests; +using Bit.Core; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Commands.Interfaces; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Mvc; + +namespace Bit.Api.KeyManagement.Controllers; + +[Route("accounts/key-management")] +[Authorize("Application")] +public class AccountsKeyManagementController : Controller +{ + private readonly IEmergencyAccessRepository _emergencyAccessRepository; + private readonly IFeatureService _featureService; + private readonly IOrganizationUserRepository _organizationUserRepository; + private readonly IRegenerateUserAsymmetricKeysCommand _regenerateUserAsymmetricKeysCommand; + private readonly IUserService _userService; + + public AccountsKeyManagementController(IUserService userService, + IFeatureService featureService, + IOrganizationUserRepository organizationUserRepository, + IEmergencyAccessRepository emergencyAccessRepository, + IRegenerateUserAsymmetricKeysCommand regenerateUserAsymmetricKeysCommand) + { + _userService = userService; + _featureService = featureService; + _regenerateUserAsymmetricKeysCommand = regenerateUserAsymmetricKeysCommand; + _organizationUserRepository = organizationUserRepository; + _emergencyAccessRepository = emergencyAccessRepository; + } + + [HttpPost("regenerate-keys")] + public async Task RegenerateKeysAsync([FromBody] KeyRegenerationRequestModel request) + { + if (!_featureService.IsEnabled(FeatureFlagKeys.PrivateKeyRegeneration)) + { + throw new NotFoundException(); + } + + var user = await _userService.GetUserByPrincipalAsync(User) ?? throw new UnauthorizedAccessException(); + var usersOrganizationAccounts = await _organizationUserRepository.GetManyByUserAsync(user.Id); + var designatedEmergencyAccess = await _emergencyAccessRepository.GetManyDetailsByGranteeIdAsync(user.Id); + await _regenerateUserAsymmetricKeysCommand.RegenerateKeysAsync(request.ToUserAsymmetricKeys(user.Id), + usersOrganizationAccounts, designatedEmergencyAccess); + } +} diff --git a/src/Api/KeyManagement/Models/Requests/KeyRegenerationRequestModel.cs b/src/Api/KeyManagement/Models/Requests/KeyRegenerationRequestModel.cs new file mode 100644 index 000000000000..495d13cccd95 --- /dev/null +++ b/src/Api/KeyManagement/Models/Requests/KeyRegenerationRequestModel.cs @@ -0,0 +1,23 @@ +#nullable enable +using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.Utilities; + +namespace Bit.Api.KeyManagement.Models.Requests; + +public class KeyRegenerationRequestModel +{ + public required string UserPublicKey { get; set; } + + [EncryptedString] + public required string UserKeyEncryptedUserPrivateKey { get; set; } + + public UserAsymmetricKeys ToUserAsymmetricKeys(Guid userId) + { + return new UserAsymmetricKeys + { + UserId = userId, + PublicKey = UserPublicKey, + UserKeyEncryptedPrivateKey = UserKeyEncryptedUserPrivateKey, + }; + } +} diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index df0abfb4b999..2c315b257848 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -160,6 +160,7 @@ public static class FeatureFlagKeys public const string PM12443RemovePagingLogic = "pm-12443-remove-paging-logic"; public const string SelfHostLicenseRefactor = "pm-11516-self-host-license-refactor"; public const string PromoteProviderServiceUserTool = "pm-15128-promote-provider-service-user-tool"; + public const string PrivateKeyRegeneration = "pm-12241-private-key-regeneration"; public static List GetAllKeys() { diff --git a/src/Core/KeyManagement/Commands/Interfaces/IRegenerateUserAsymmetricKeysCommand.cs b/src/Core/KeyManagement/Commands/Interfaces/IRegenerateUserAsymmetricKeysCommand.cs new file mode 100644 index 000000000000..d7ad7e3959eb --- /dev/null +++ b/src/Core/KeyManagement/Commands/Interfaces/IRegenerateUserAsymmetricKeysCommand.cs @@ -0,0 +1,13 @@ +#nullable enable +using Bit.Core.Auth.Models.Data; +using Bit.Core.Entities; +using Bit.Core.KeyManagement.Models.Data; + +namespace Bit.Core.KeyManagement.Commands.Interfaces; + +public interface IRegenerateUserAsymmetricKeysCommand +{ + Task RegenerateKeysAsync(UserAsymmetricKeys userAsymmetricKeys, + ICollection usersOrganizationAccounts, + ICollection designatedEmergencyAccess); +} diff --git a/src/Core/KeyManagement/Commands/RegenerateUserAsymmetricKeysCommand.cs b/src/Core/KeyManagement/Commands/RegenerateUserAsymmetricKeysCommand.cs new file mode 100644 index 000000000000..a54223f685b2 --- /dev/null +++ b/src/Core/KeyManagement/Commands/RegenerateUserAsymmetricKeysCommand.cs @@ -0,0 +1,71 @@ +#nullable enable +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Models.Data; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Commands.Interfaces; +using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.KeyManagement.Repositories; +using Bit.Core.Services; +using Microsoft.Extensions.Logging; + +namespace Bit.Core.KeyManagement.Commands; + +public class RegenerateUserAsymmetricKeysCommand : IRegenerateUserAsymmetricKeysCommand +{ + private readonly ICurrentContext _currentContext; + private readonly ILogger _logger; + private readonly IUserAsymmetricKeysRepository _userAsymmetricKeysRepository; + private readonly IPushNotificationService _pushService; + + public RegenerateUserAsymmetricKeysCommand( + ICurrentContext currentContext, + IUserAsymmetricKeysRepository userAsymmetricKeysRepository, + IPushNotificationService pushService, + ILogger logger) + { + _currentContext = currentContext; + _logger = logger; + _userAsymmetricKeysRepository = userAsymmetricKeysRepository; + _pushService = pushService; + } + + public async Task RegenerateKeysAsync(UserAsymmetricKeys userAsymmetricKeys, + ICollection usersOrganizationAccounts, + ICollection designatedEmergencyAccess) + { + var userId = _currentContext.UserId; + if (!userId.HasValue || + userAsymmetricKeys.UserId != userId.Value || + usersOrganizationAccounts.Any(ou => ou.UserId != userId) || + designatedEmergencyAccess.Any(dea => dea.GranteeId != userId)) + { + throw new NotFoundException(); + } + + var inOrganizations = usersOrganizationAccounts.Any(ou => + ou.Status is OrganizationUserStatusType.Confirmed or OrganizationUserStatusType.Revoked); + var hasDesignatedEmergencyAccess = designatedEmergencyAccess.Any(x => + x.Status is EmergencyAccessStatusType.Confirmed or EmergencyAccessStatusType.RecoveryApproved + or EmergencyAccessStatusType.RecoveryInitiated); + + _logger.LogInformation( + "User asymmetric keys regeneration requested. UserId: {userId} OrganizationMembership: {inOrganizations} DesignatedEmergencyAccess: {hasDesignatedEmergencyAccess} DeviceType: {deviceType}", + userAsymmetricKeys.UserId, inOrganizations, hasDesignatedEmergencyAccess, _currentContext.DeviceType); + + // For now, don't regenerate asymmetric keys for user's with organization membership and designated emergency access. + if (inOrganizations || hasDesignatedEmergencyAccess) + { + throw new BadRequestException("Key regeneration not supported for this user."); + } + + await _userAsymmetricKeysRepository.RegenerateUserAsymmetricKeysAsync(userAsymmetricKeys); + _logger.LogInformation( + "User's asymmetric keys regenerated. UserId: {userId} OrganizationMembership: {inOrganizations} DesignatedEmergencyAccess: {hasDesignatedEmergencyAccess} DeviceType: {deviceType}", + userAsymmetricKeys.UserId, inOrganizations, hasDesignatedEmergencyAccess, _currentContext.DeviceType); + + await _pushService.PushSyncSettingsAsync(userId.Value); + } +} diff --git a/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs b/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs new file mode 100644 index 000000000000..102630c7e6b7 --- /dev/null +++ b/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs @@ -0,0 +1,18 @@ +using Bit.Core.KeyManagement.Commands; +using Bit.Core.KeyManagement.Commands.Interfaces; +using Microsoft.Extensions.DependencyInjection; + +namespace Bit.Core.KeyManagement; + +public static class KeyManagementServiceCollectionExtensions +{ + public static void AddKeyManagementServices(this IServiceCollection services) + { + services.AddKeyManagementCommands(); + } + + private static void AddKeyManagementCommands(this IServiceCollection services) + { + services.AddScoped(); + } +} diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index 7585739d82ca..c757f163e92d 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -26,6 +26,7 @@ using Bit.Core.HostedServices; using Bit.Core.Identity; using Bit.Core.IdentityServer; +using Bit.Core.KeyManagement; using Bit.Core.NotificationHub; using Bit.Core.OrganizationFeatures; using Bit.Core.Repositories; @@ -120,6 +121,7 @@ public static void AddBaseServices(this IServiceCollection services, IGlobalSett services.AddScoped(); services.AddVaultServices(); services.AddReportingServices(); + services.AddKeyManagementServices(); } public static void AddTokenizers(this IServiceCollection services) diff --git a/test/Api.IntegrationTest/Helpers/LoginHelper.cs b/test/Api.IntegrationTest/Helpers/LoginHelper.cs index d6ce911bd0fe..1f5eb725d956 100644 --- a/test/Api.IntegrationTest/Helpers/LoginHelper.cs +++ b/test/Api.IntegrationTest/Helpers/LoginHelper.cs @@ -16,6 +16,12 @@ public LoginHelper(ApiApplicationFactory factory, HttpClient client) _client = client; } + public async Task LoginAsync(string email) + { + var tokens = await _factory.LoginAsync(email); + _client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", tokens.Token); + } + public async Task LoginWithOrganizationApiKeyAsync(Guid organizationId) { var (clientId, apiKey) = await GetOrganizationApiKey(_factory, organizationId); diff --git a/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs b/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs new file mode 100644 index 000000000000..ec7ca3746010 --- /dev/null +++ b/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs @@ -0,0 +1,164 @@ +using System.Net; +using Bit.Api.IntegrationTest.Factories; +using Bit.Api.IntegrationTest.Helpers; +using Bit.Api.KeyManagement.Models.Requests; +using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Enums; +using Bit.Core.Billing.Enums; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using Bit.Test.Common.AutoFixture.Attributes; +using Xunit; + +namespace Bit.Api.IntegrationTest.KeyManagement.Controllers; + +public class AccountsKeyManagementControllerTests : IClassFixture, IAsyncLifetime +{ + private static readonly string _mockEncryptedString = + "2.AOs41Hd8OQiCPXjyJKCiDA==|O6OHgt2U2hJGBSNGnimJmg==|iD33s8B69C8JhYYhSa4V1tArjvLr8eEaGqOV7BRo5Jk="; + + private readonly HttpClient _client; + private readonly IEmergencyAccessRepository _emergencyAccessRepository; + private readonly IOrganizationUserRepository _organizationUserRepository; + private readonly ApiApplicationFactory _factory; + private readonly LoginHelper _loginHelper; + private readonly IUserRepository _userRepository; + private string _ownerEmail = null!; + + public AccountsKeyManagementControllerTests(ApiApplicationFactory factory) + { + _factory = factory; + _factory.UpdateConfiguration("globalSettings:launchDarkly:flagValues:pm-12241-private-key-regeneration", + "true"); + _client = factory.CreateClient(); + _loginHelper = new LoginHelper(_factory, _client); + _userRepository = _factory.GetService(); + _emergencyAccessRepository = _factory.GetService(); + _organizationUserRepository = _factory.GetService(); + } + + public async Task InitializeAsync() + { + _ownerEmail = $"integration-test{Guid.NewGuid()}@bitwarden.com"; + await _factory.LoginWithNewAccount(_ownerEmail); + } + + public Task DisposeAsync() + { + _client.Dispose(); + return Task.CompletedTask; + } + + [Theory] + [BitAutoData] + public async Task RegenerateKeysAsync_FeatureFlagTurnedOff_NotFound(KeyRegenerationRequestModel request) + { + // Localize factory to inject a false value for the feature flag. + var localFactory = new ApiApplicationFactory(); + localFactory.UpdateConfiguration("globalSettings:launchDarkly:flagValues:pm-12241-private-key-regeneration", + "false"); + var localClient = localFactory.CreateClient(); + var localEmail = $"integration-test{Guid.NewGuid()}@bitwarden.com"; + var localLoginHelper = new LoginHelper(localFactory, localClient); + await localFactory.LoginWithNewAccount(localEmail); + await localLoginHelper.LoginAsync(localEmail); + + request.UserKeyEncryptedUserPrivateKey = _mockEncryptedString; + + var response = await localClient.PostAsJsonAsync("/accounts/key-management/regenerate-keys", request); + + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Theory] + [BitAutoData] + public async Task RegenerateKeysAsync_NotLoggedIn_Unauthorized(KeyRegenerationRequestModel request) + { + request.UserKeyEncryptedUserPrivateKey = _mockEncryptedString; + + var response = await _client.PostAsJsonAsync("/accounts/key-management/regenerate-keys", request); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + [Theory] + [BitAutoData(OrganizationUserStatusType.Confirmed, EmergencyAccessStatusType.Confirmed)] + [BitAutoData(OrganizationUserStatusType.Confirmed, EmergencyAccessStatusType.RecoveryApproved)] + [BitAutoData(OrganizationUserStatusType.Confirmed, EmergencyAccessStatusType.RecoveryInitiated)] + [BitAutoData(OrganizationUserStatusType.Revoked, EmergencyAccessStatusType.Confirmed)] + [BitAutoData(OrganizationUserStatusType.Revoked, EmergencyAccessStatusType.RecoveryApproved)] + [BitAutoData(OrganizationUserStatusType.Revoked, EmergencyAccessStatusType.RecoveryInitiated)] + [BitAutoData(OrganizationUserStatusType.Confirmed, null)] + [BitAutoData(OrganizationUserStatusType.Revoked, null)] + [BitAutoData(OrganizationUserStatusType.Invited, EmergencyAccessStatusType.Confirmed)] + [BitAutoData(OrganizationUserStatusType.Invited, EmergencyAccessStatusType.RecoveryApproved)] + [BitAutoData(OrganizationUserStatusType.Invited, EmergencyAccessStatusType.RecoveryInitiated)] + public async Task RegenerateKeysAsync_UserInOrgOrHasDesignatedEmergencyAccess_ThrowsBadRequest( + OrganizationUserStatusType organizationUserStatus, + EmergencyAccessStatusType? emergencyAccessStatus, + KeyRegenerationRequestModel request) + { + if (organizationUserStatus is OrganizationUserStatusType.Confirmed or OrganizationUserStatusType.Revoked) + { + await CreateOrganizationUserAsync(organizationUserStatus); + } + + if (emergencyAccessStatus != null) + { + await CreateDesignatedEmergencyAccessAsync(emergencyAccessStatus.Value); + } + + await _loginHelper.LoginAsync(_ownerEmail); + request.UserKeyEncryptedUserPrivateKey = _mockEncryptedString; + + var response = await _client.PostAsJsonAsync("/accounts/key-management/regenerate-keys", request); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Theory] + [BitAutoData] + public async Task RegenerateKeysAsync_Success(KeyRegenerationRequestModel request) + { + await _loginHelper.LoginAsync(_ownerEmail); + request.UserKeyEncryptedUserPrivateKey = _mockEncryptedString; + + var response = await _client.PostAsJsonAsync("/accounts/key-management/regenerate-keys", request); + response.EnsureSuccessStatusCode(); + + var user = await _userRepository.GetByEmailAsync(_ownerEmail); + Assert.NotNull(user); + Assert.Equal(request.UserPublicKey, user.PublicKey); + Assert.Equal(request.UserKeyEncryptedUserPrivateKey, user.PrivateKey); + } + + private async Task CreateOrganizationUserAsync(OrganizationUserStatusType organizationUserStatus) + { + var (_, organizationUser) = await OrganizationTestHelpers.SignUpAsync(_factory, + PlanType.EnterpriseAnnually, _ownerEmail, passwordManagerSeats: 10, + paymentMethod: PaymentMethodType.Card); + organizationUser.Status = organizationUserStatus; + await _organizationUserRepository.ReplaceAsync(organizationUser); + } + + private async Task CreateDesignatedEmergencyAccessAsync(EmergencyAccessStatusType emergencyAccessStatus) + { + var tempEmail = $"integration-test{Guid.NewGuid()}@bitwarden.com"; + await _factory.LoginWithNewAccount(tempEmail); + + var tempUser = await _userRepository.GetByEmailAsync(tempEmail); + var user = await _userRepository.GetByEmailAsync(_ownerEmail); + var emergencyAccess = new EmergencyAccess + { + GrantorId = tempUser!.Id, + GranteeId = user!.Id, + KeyEncrypted = _mockEncryptedString, + Status = emergencyAccessStatus, + Type = EmergencyAccessType.View, + WaitTimeDays = 10, + CreationDate = DateTime.UtcNow, + RevisionDate = DateTime.UtcNow + }; + await _emergencyAccessRepository.CreateAsync(emergencyAccess); + } +} diff --git a/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs new file mode 100644 index 000000000000..2615697ad374 --- /dev/null +++ b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs @@ -0,0 +1,96 @@ +#nullable enable +using System.Security.Claims; +using Bit.Api.KeyManagement.Controllers; +using Bit.Api.KeyManagement.Models.Requests; +using Bit.Core; +using Bit.Core.Auth.Models.Data; +using Bit.Core.Entities; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Commands.Interfaces; +using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using NSubstitute.ReturnsExtensions; +using Xunit; + +namespace Bit.Api.Test.KeyManagement.Controllers; + +[ControllerCustomize(typeof(AccountsKeyManagementController))] +[SutProviderCustomize] +[JsonDocumentCustomize] +public class AccountsKeyManagementControllerTests +{ + [Theory] + [BitAutoData] + public async Task RegenerateKeysAsync_FeatureFlagOff_Throws( + SutProvider sutProvider, + KeyRegenerationRequestModel data) + { + sutProvider.GetDependency().IsEnabled(Arg.Is(FeatureFlagKeys.PrivateKeyRegeneration)) + .Returns(false); + sutProvider.GetDependency().GetUserByPrincipalAsync(Arg.Any()).ReturnsNull(); + + await Assert.ThrowsAsync(() => sutProvider.Sut.RegenerateKeysAsync(data)); + + await sutProvider.GetDependency().ReceivedWithAnyArgs(0) + .GetManyByUserAsync(Arg.Any()); + await sutProvider.GetDependency().ReceivedWithAnyArgs(0) + .GetManyDetailsByGranteeIdAsync(Arg.Any()); + await sutProvider.GetDependency().ReceivedWithAnyArgs(0) + .RegenerateKeysAsync(Arg.Any(), + Arg.Any>(), + Arg.Any>()); + } + + [Theory] + [BitAutoData] + public async Task RegenerateKeysAsync_UserNull_Throws(SutProvider sutProvider, + KeyRegenerationRequestModel data) + { + sutProvider.GetDependency().IsEnabled(Arg.Is(FeatureFlagKeys.PrivateKeyRegeneration)) + .Returns(true); + sutProvider.GetDependency().GetUserByPrincipalAsync(Arg.Any()).ReturnsNull(); + + await Assert.ThrowsAsync(() => sutProvider.Sut.RegenerateKeysAsync(data)); + + await sutProvider.GetDependency().ReceivedWithAnyArgs(0) + .GetManyByUserAsync(Arg.Any()); + await sutProvider.GetDependency().ReceivedWithAnyArgs(0) + .GetManyDetailsByGranteeIdAsync(Arg.Any()); + await sutProvider.GetDependency().ReceivedWithAnyArgs(0) + .RegenerateKeysAsync(Arg.Any(), + Arg.Any>(), + Arg.Any>()); + } + + [Theory] + [BitAutoData] + public async Task RegenerateKeysAsync_Success(SutProvider sutProvider, + KeyRegenerationRequestModel data, User user, ICollection orgUsers, + ICollection accessDetails) + { + sutProvider.GetDependency().IsEnabled(Arg.Is(FeatureFlagKeys.PrivateKeyRegeneration)) + .Returns(true); + sutProvider.GetDependency().GetUserByPrincipalAsync(Arg.Any()).Returns(user); + sutProvider.GetDependency().GetManyByUserAsync(Arg.Is(user.Id)).Returns(orgUsers); + sutProvider.GetDependency().GetManyDetailsByGranteeIdAsync(Arg.Is(user.Id)) + .Returns(accessDetails); + + await sutProvider.Sut.RegenerateKeysAsync(data); + + await sutProvider.GetDependency().Received(1) + .GetManyByUserAsync(Arg.Is(user.Id)); + await sutProvider.GetDependency().Received(1) + .GetManyDetailsByGranteeIdAsync(Arg.Is(user.Id)); + await sutProvider.GetDependency().Received(1) + .RegenerateKeysAsync( + Arg.Is(u => + u.UserId == user.Id && u.PublicKey == data.UserPublicKey && + u.UserKeyEncryptedPrivateKey == data.UserKeyEncryptedUserPrivateKey), + Arg.Is(orgUsers), + Arg.Is(accessDetails)); + } +} diff --git a/test/Core.Test/KeyManagement/Commands/RegenerateUserAsymmetricKeysCommandTests.cs b/test/Core.Test/KeyManagement/Commands/RegenerateUserAsymmetricKeysCommandTests.cs new file mode 100644 index 000000000000..3388956156e7 --- /dev/null +++ b/test/Core.Test/KeyManagement/Commands/RegenerateUserAsymmetricKeysCommandTests.cs @@ -0,0 +1,197 @@ +#nullable enable +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Models.Data; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Commands; +using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.KeyManagement.Repositories; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using NSubstitute.ReturnsExtensions; +using Xunit; + +namespace Bit.Core.Test.KeyManagement.Commands; + +[SutProviderCustomize] +public class RegenerateUserAsymmetricKeysCommandTests +{ + [Theory] + [BitAutoData] + public async Task RegenerateKeysAsync_NoCurrentContext_NotFoundException( + SutProvider sutProvider, + UserAsymmetricKeys userAsymmetricKeys) + { + sutProvider.GetDependency().UserId.ReturnsNullForAnyArgs(); + var usersOrganizationAccounts = new List(); + var designatedEmergencyAccess = new List(); + + await Assert.ThrowsAsync(() => sutProvider.Sut.RegenerateKeysAsync(userAsymmetricKeys, + usersOrganizationAccounts, designatedEmergencyAccess)); + } + + [Theory] + [BitAutoData] + public async Task RegenerateKeysAsync_UserHasNoSharedAccess_Success( + SutProvider sutProvider, + UserAsymmetricKeys userAsymmetricKeys) + { + sutProvider.GetDependency().UserId.ReturnsForAnyArgs(userAsymmetricKeys.UserId); + var usersOrganizationAccounts = new List(); + var designatedEmergencyAccess = new List(); + + await sutProvider.Sut.RegenerateKeysAsync(userAsymmetricKeys, + usersOrganizationAccounts, designatedEmergencyAccess); + + await sutProvider.GetDependency() + .Received(1) + .RegenerateUserAsymmetricKeysAsync(Arg.Is(userAsymmetricKeys)); + await sutProvider.GetDependency() + .Received(1) + .PushSyncSettingsAsync(Arg.Is(userAsymmetricKeys.UserId)); + } + + [Theory] + [BitAutoData(false, false, true)] + [BitAutoData(false, true, false)] + [BitAutoData(false, true, true)] + [BitAutoData(true, false, false)] + [BitAutoData(true, false, true)] + [BitAutoData(true, true, false)] + [BitAutoData(true, true, true)] + public async Task RegenerateKeysAsync_UserIdMisMatch_NotFoundException( + bool userAsymmetricKeysMismatch, + bool orgMismatch, + bool emergencyAccessMismatch, + SutProvider sutProvider, + UserAsymmetricKeys userAsymmetricKeys, + ICollection usersOrganizationAccounts, + ICollection designatedEmergencyAccess) + { + sutProvider.GetDependency().UserId + .ReturnsForAnyArgs(userAsymmetricKeysMismatch ? new Guid() : userAsymmetricKeys.UserId); + + if (!orgMismatch) + { + usersOrganizationAccounts = + SetupOrganizationUserAccounts(userAsymmetricKeys.UserId, usersOrganizationAccounts); + } + + if (!emergencyAccessMismatch) + { + designatedEmergencyAccess = SetupEmergencyAccess(userAsymmetricKeys.UserId, designatedEmergencyAccess); + } + + await Assert.ThrowsAsync(() => sutProvider.Sut.RegenerateKeysAsync(userAsymmetricKeys, + usersOrganizationAccounts, designatedEmergencyAccess)); + + await sutProvider.GetDependency() + .ReceivedWithAnyArgs(0) + .RegenerateUserAsymmetricKeysAsync(Arg.Any()); + await sutProvider.GetDependency() + .ReceivedWithAnyArgs(0) + .PushSyncSettingsAsync(Arg.Any()); + } + + [Theory] + [BitAutoData(OrganizationUserStatusType.Confirmed)] + [BitAutoData(OrganizationUserStatusType.Revoked)] + public async Task RegenerateKeysAsync_UserInOrganizations_BadRequestException( + OrganizationUserStatusType organizationUserStatus, + SutProvider sutProvider, + UserAsymmetricKeys userAsymmetricKeys, + ICollection usersOrganizationAccounts) + { + sutProvider.GetDependency().UserId.ReturnsForAnyArgs(userAsymmetricKeys.UserId); + usersOrganizationAccounts = CreateInOrganizationAccounts(userAsymmetricKeys.UserId, organizationUserStatus, + usersOrganizationAccounts); + var designatedEmergencyAccess = new List(); + + await Assert.ThrowsAsync(() => sutProvider.Sut.RegenerateKeysAsync(userAsymmetricKeys, + usersOrganizationAccounts, designatedEmergencyAccess)); + + await sutProvider.GetDependency() + .ReceivedWithAnyArgs(0) + .RegenerateUserAsymmetricKeysAsync(Arg.Any()); + await sutProvider.GetDependency() + .ReceivedWithAnyArgs(0) + .PushSyncSettingsAsync(Arg.Any()); + } + + [Theory] + [BitAutoData(EmergencyAccessStatusType.Confirmed)] + [BitAutoData(EmergencyAccessStatusType.RecoveryApproved)] + [BitAutoData(EmergencyAccessStatusType.RecoveryInitiated)] + public async Task RegenerateKeysAsync_UserHasDesignatedEmergencyAccess_BadRequestException( + EmergencyAccessStatusType statusType, + SutProvider sutProvider, + UserAsymmetricKeys userAsymmetricKeys, + ICollection designatedEmergencyAccess) + { + sutProvider.GetDependency().UserId.ReturnsForAnyArgs(userAsymmetricKeys.UserId); + designatedEmergencyAccess = + CreateDesignatedEmergencyAccess(userAsymmetricKeys.UserId, statusType, designatedEmergencyAccess); + var usersOrganizationAccounts = new List(); + + + await Assert.ThrowsAsync(() => sutProvider.Sut.RegenerateKeysAsync(userAsymmetricKeys, + usersOrganizationAccounts, designatedEmergencyAccess)); + + await sutProvider.GetDependency() + .ReceivedWithAnyArgs(0) + .RegenerateUserAsymmetricKeysAsync(Arg.Any()); + await sutProvider.GetDependency() + .ReceivedWithAnyArgs(0) + .PushSyncSettingsAsync(Arg.Any()); + } + + private static ICollection CreateInOrganizationAccounts(Guid userId, + OrganizationUserStatusType organizationUserStatus, ICollection organizationUserAccounts) + { + foreach (var organizationUserAccount in organizationUserAccounts) + { + organizationUserAccount.UserId = userId; + organizationUserAccount.Status = organizationUserStatus; + } + + return organizationUserAccounts; + } + + private static ICollection CreateDesignatedEmergencyAccess(Guid userId, + EmergencyAccessStatusType status, ICollection designatedEmergencyAccess) + { + foreach (var designated in designatedEmergencyAccess) + { + designated.GranteeId = userId; + designated.Status = status; + } + + return designatedEmergencyAccess; + } + + private static ICollection SetupOrganizationUserAccounts(Guid userId, + ICollection organizationUserAccounts) + { + foreach (var organizationUserAccount in organizationUserAccounts) + { + organizationUserAccount.UserId = userId; + } + + return organizationUserAccounts; + } + + private static ICollection SetupEmergencyAccess(Guid userId, + ICollection emergencyAccessDetails) + { + foreach (var emergencyAccessDetail in emergencyAccessDetails) + { + emergencyAccessDetail.GranteeId = userId; + } + + return emergencyAccessDetails; + } +} From b907935edaf0542cc2d8915b076d99e5b398cf62 Mon Sep 17 00:00:00 2001 From: Robyn MacCallum Date: Mon, 16 Dec 2024 16:18:33 -0500 Subject: [PATCH 07/11] Add Authenticator sync flags (#5159) * Add Authenticator sync flags * Fix whitespace --- src/Core/Constants.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 2c315b257848..86e49fa6cf72 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -161,6 +161,8 @@ public static class FeatureFlagKeys public const string SelfHostLicenseRefactor = "pm-11516-self-host-license-refactor"; public const string PromoteProviderServiceUserTool = "pm-15128-promote-provider-service-user-tool"; public const string PrivateKeyRegeneration = "pm-12241-private-key-regeneration"; + public const string AuthenticatorSynciOS = "enable-authenticator-sync-ios"; + public const string AuthenticatorSyncAndroid = "enable-authenticator-sync-android"; public static List GetAllKeys() { From ecbfc056830e41822e4567721be0a8bc51fc0436 Mon Sep 17 00:00:00 2001 From: aj-bw <81774843+aj-bw@users.noreply.github.com> Date: Tue, 17 Dec 2024 08:32:37 -0500 Subject: [PATCH 08/11] QA-689/BEEEP-public-api-GET-subscription-details (#5041) * added GET operation to org subscription endpoint * adding back removed using statement * addressing unused import and lint warnings * whitespace lint fix * successful local format * add NotSelfHostOnly attribute * add endpoint summary and return details --- .../Controllers/OrganizationController.cs | 44 +++++++++++++++++++ ...anizationSubscriptionUpdateRequestModel.cs | 0 ...izationSubscriptionDetailsResponseModel.cs | 32 ++++++++++++++ 3 files changed, 76 insertions(+) rename src/Api/Billing/Public/Models/{ => Request}/OrganizationSubscriptionUpdateRequestModel.cs (100%) create mode 100644 src/Api/Billing/Public/Models/Response/OrganizationSubscriptionDetailsResponseModel.cs diff --git a/src/Api/Billing/Public/Controllers/OrganizationController.cs b/src/Api/Billing/Public/Controllers/OrganizationController.cs index c696f2af5065..7fcd94acd38d 100644 --- a/src/Api/Billing/Public/Controllers/OrganizationController.cs +++ b/src/Api/Billing/Public/Controllers/OrganizationController.cs @@ -1,4 +1,5 @@ using System.Net; +using Bit.Api.Billing.Public.Models; using Bit.Api.Models.Public.Response; using Bit.Core.Context; using Bit.Core.OrganizationFeatures.OrganizationSubscriptions.Interface; @@ -35,6 +36,49 @@ public OrganizationController( _logger = logger; } + /// + /// Retrieves the subscription details for the current organization. + /// + /// + /// Returns an object containing the subscription details if successful. + /// + [HttpGet("subscription")] + [SelfHosted(NotSelfHostedOnly = true)] + [ProducesResponseType(typeof(OrganizationSubscriptionDetailsResponseModel), (int)HttpStatusCode.OK)] + [ProducesResponseType(typeof(ErrorResponseModel), (int)HttpStatusCode.NotFound)] + public async Task GetSubscriptionAsync() + { + try + { + var organizationId = _currentContext.OrganizationId.Value; + var organization = await _organizationRepository.GetByIdAsync(organizationId); + + var subscriptionDetails = new OrganizationSubscriptionDetailsResponseModel + { + PasswordManager = new PasswordManagerSubscriptionDetails + { + Seats = organization.Seats, + MaxAutoScaleSeats = organization.MaxAutoscaleSeats, + Storage = organization.MaxStorageGb + }, + SecretsManager = new SecretsManagerSubscriptionDetails + { + Seats = organization.SmSeats, + MaxAutoScaleSeats = organization.MaxAutoscaleSmSeats, + ServiceAccounts = organization.SmServiceAccounts, + MaxAutoScaleServiceAccounts = organization.MaxAutoscaleSmServiceAccounts + } + }; + + return Ok(subscriptionDetails); + } + catch (Exception ex) + { + _logger.LogError(ex, "Unhandled error while retrieving the subscription details"); + return StatusCode(500, new { Message = "An error occurred while retrieving the subscription details." }); + } + } + /// /// Update the organization's current subscription for Password Manager and/or Secrets Manager. /// diff --git a/src/Api/Billing/Public/Models/OrganizationSubscriptionUpdateRequestModel.cs b/src/Api/Billing/Public/Models/Request/OrganizationSubscriptionUpdateRequestModel.cs similarity index 100% rename from src/Api/Billing/Public/Models/OrganizationSubscriptionUpdateRequestModel.cs rename to src/Api/Billing/Public/Models/Request/OrganizationSubscriptionUpdateRequestModel.cs diff --git a/src/Api/Billing/Public/Models/Response/OrganizationSubscriptionDetailsResponseModel.cs b/src/Api/Billing/Public/Models/Response/OrganizationSubscriptionDetailsResponseModel.cs new file mode 100644 index 000000000000..09aa7decc17d --- /dev/null +++ b/src/Api/Billing/Public/Models/Response/OrganizationSubscriptionDetailsResponseModel.cs @@ -0,0 +1,32 @@ +using System.ComponentModel.DataAnnotations; + +namespace Bit.Api.Billing.Public.Models; + +public class OrganizationSubscriptionDetailsResponseModel : IValidatableObject +{ + public PasswordManagerSubscriptionDetails PasswordManager { get; set; } + public SecretsManagerSubscriptionDetails SecretsManager { get; set; } + public IEnumerable Validate(ValidationContext validationContext) + { + if (PasswordManager == null && SecretsManager == null) + { + yield return new ValidationResult("At least one of PasswordManager or SecretsManager must be provided."); + } + + yield return ValidationResult.Success; + } +} +public class PasswordManagerSubscriptionDetails +{ + public int? Seats { get; set; } + public int? MaxAutoScaleSeats { get; set; } + public short? Storage { get; set; } +} + +public class SecretsManagerSubscriptionDetails +{ + public int? Seats { get; set; } + public int? MaxAutoScaleSeats { get; set; } + public int? ServiceAccounts { get; set; } + public int? MaxAutoScaleServiceAccounts { get; set; } +} From 16488091d24f275885cab4777b9764f5847298c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ch=C4=99ci=C5=84ski?= Date: Tue, 17 Dec 2024 16:45:02 +0100 Subject: [PATCH 09/11] Remove is_workflow_call input from build workflow (#5161) --- .github/workflows/build.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index eb644fe8b5e2..420b9b63750a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -580,7 +580,6 @@ jobs: ref: 'main', inputs: { server_branch: process.env.GITHUB_REF - is_workflow_call: true } }); From b75c63c2c6ac335747958c0e72e2d4143a3520c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Tue, 17 Dec 2024 15:57:31 +0000 Subject: [PATCH 10/11] [PM-15957] Fix: Domain Claim fails to enable Single Organization Policy, sends no emails and Revokes all users (#5147) * Add JSON-based stored procedure for updating account revision dates and modify existing procedure to use it * Refactor SingleOrgPolicyValidator to revoke only non-compliant organization users and update related tests --- .../SingleOrgPolicyValidator.cs | 11 +++- ...OrganizationUser_SetStatusForUsersById.sql | 2 +- ...tRevisionDateByOrganizationUserIdsJson.sql | 33 ++++++++++ .../SingleOrgPolicyValidatorTests.cs | 38 +++++++++-- ...2-11-00_BumpAccountRevisionDateJsonIds.sql | 64 +++++++++++++++++++ 5 files changed, 140 insertions(+), 8 deletions(-) create mode 100644 src/Sql/dbo/Stored Procedures/User_BumpAccountRevisionDateByOrganizationUserIdsJson.sql create mode 100644 util/Migrator/DbScripts/2024-12-11-00_BumpAccountRevisionDateJsonIds.sql diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidator.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidator.cs index 050949ee7f91..a37deef3eb78 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidator.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidator.cs @@ -97,15 +97,22 @@ private async Task RevokeNonCompliantUsersAsync(Guid organizationId, IActingUser return; } + var allRevocableUserOrgs = await _organizationUserRepository.GetManyByManyUsersAsync( + currentActiveRevocableOrganizationUsers.Select(ou => ou.UserId!.Value)); + var usersToRevoke = currentActiveRevocableOrganizationUsers.Where(ou => + allRevocableUserOrgs.Any(uo => uo.UserId == ou.UserId && + uo.OrganizationId != organizationId && + uo.Status != OrganizationUserStatusType.Invited)).ToList(); + var commandResult = await _revokeNonCompliantOrganizationUserCommand.RevokeNonCompliantOrganizationUsersAsync( - new RevokeOrganizationUsersRequest(organizationId, currentActiveRevocableOrganizationUsers, performedBy)); + new RevokeOrganizationUsersRequest(organizationId, usersToRevoke, performedBy)); if (commandResult.HasErrors) { throw new BadRequestException(string.Join(", ", commandResult.ErrorMessages)); } - await Task.WhenAll(currentActiveRevocableOrganizationUsers.Select(x => + await Task.WhenAll(usersToRevoke.Select(x => _mailService.SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(organization.DisplayName(), x.Email))); } diff --git a/src/Sql/dbo/Stored Procedures/OrganizationUser_SetStatusForUsersById.sql b/src/Sql/dbo/Stored Procedures/OrganizationUser_SetStatusForUsersById.sql index 95ed5a3155d0..18b876775e2c 100644 --- a/src/Sql/dbo/Stored Procedures/OrganizationUser_SetStatusForUsersById.sql +++ b/src/Sql/dbo/Stored Procedures/OrganizationUser_SetStatusForUsersById.sql @@ -24,6 +24,6 @@ BEGIN SET [Status] = @Status WHERE [Id] IN (SELECT Id from @ParsedIds) - EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationUserIds] @OrganizationUserIds + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationUserIdsJson] @OrganizationUserIds END diff --git a/src/Sql/dbo/Stored Procedures/User_BumpAccountRevisionDateByOrganizationUserIdsJson.sql b/src/Sql/dbo/Stored Procedures/User_BumpAccountRevisionDateByOrganizationUserIdsJson.sql new file mode 100644 index 000000000000..6e4119d86447 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/User_BumpAccountRevisionDateByOrganizationUserIdsJson.sql @@ -0,0 +1,33 @@ +CREATE PROCEDURE [dbo].[User_BumpAccountRevisionDateByOrganizationUserIdsJson] + @OrganizationUserIds NVARCHAR(MAX) +AS +BEGIN + SET NOCOUNT ON + + CREATE TABLE #UserIds + ( + UserId UNIQUEIDENTIFIER NOT NULL + ); + + INSERT INTO #UserIds (UserId) + SELECT + OU.UserId + FROM + [dbo].[OrganizationUser] OU + INNER JOIN + (SELECT [value] as Id FROM OPENJSON(@OrganizationUserIds)) AS OUIds + ON OUIds.Id = OU.Id + WHERE + OU.[Status] = 2 -- Confirmed + + UPDATE + U + SET + U.[AccountRevisionDate] = GETUTCDATE() + FROM + [dbo].[User] U + INNER JOIN + #UserIds ON U.[Id] = #UserIds.[UserId] + + DROP TABLE #UserIds +END diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidatorTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidatorTests.cs index 0731920757a6..d2809102aae2 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidatorTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidatorTests.cs @@ -75,6 +75,7 @@ public async Task OnSaveSideEffectsAsync_RevokesNonCompliantUsers( var compliantUser1 = new OrganizationUserUserDetails { + Id = Guid.NewGuid(), OrganizationId = organization.Id, Type = OrganizationUserType.User, Status = OrganizationUserStatusType.Confirmed, @@ -84,6 +85,7 @@ public async Task OnSaveSideEffectsAsync_RevokesNonCompliantUsers( var compliantUser2 = new OrganizationUserUserDetails { + Id = Guid.NewGuid(), OrganizationId = organization.Id, Type = OrganizationUserType.User, Status = OrganizationUserStatusType.Confirmed, @@ -93,6 +95,7 @@ public async Task OnSaveSideEffectsAsync_RevokesNonCompliantUsers( var nonCompliantUser = new OrganizationUserUserDetails { + Id = Guid.NewGuid(), OrganizationId = organization.Id, Type = OrganizationUserType.User, Status = OrganizationUserStatusType.Confirmed, @@ -106,6 +109,7 @@ public async Task OnSaveSideEffectsAsync_RevokesNonCompliantUsers( var otherOrganizationUser = new OrganizationUser { + Id = Guid.NewGuid(), OrganizationId = new Guid(), UserId = nonCompliantUserId, Status = OrganizationUserStatusType.Confirmed @@ -129,11 +133,20 @@ public async Task OnSaveSideEffectsAsync_RevokesNonCompliantUsers( await sutProvider.GetDependency() .Received(1) - .RevokeNonCompliantOrganizationUsersAsync(Arg.Any()); + .RevokeNonCompliantOrganizationUsersAsync( + Arg.Is(r => + r.OrganizationId == organization.Id && + r.OrganizationUsers.Count() == 1 && + r.OrganizationUsers.First().Id == nonCompliantUser.Id)); + await sutProvider.GetDependency() + .DidNotReceive() + .SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(organization.DisplayName(), compliantUser1.Email); + await sutProvider.GetDependency() + .DidNotReceive() + .SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(organization.DisplayName(), compliantUser2.Email); await sutProvider.GetDependency() .Received(1) - .SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(organization.DisplayName(), - "user3@example.com"); + .SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(organization.DisplayName(), nonCompliantUser.Email); } [Theory, BitAutoData] @@ -148,6 +161,7 @@ public async Task OnSaveSideEffectsAsync_RemovesNonCompliantUsers( var compliantUser1 = new OrganizationUserUserDetails { + Id = Guid.NewGuid(), OrganizationId = organization.Id, Type = OrganizationUserType.User, Status = OrganizationUserStatusType.Confirmed, @@ -157,6 +171,7 @@ public async Task OnSaveSideEffectsAsync_RemovesNonCompliantUsers( var compliantUser2 = new OrganizationUserUserDetails { + Id = Guid.NewGuid(), OrganizationId = organization.Id, Type = OrganizationUserType.User, Status = OrganizationUserStatusType.Confirmed, @@ -166,6 +181,7 @@ public async Task OnSaveSideEffectsAsync_RemovesNonCompliantUsers( var nonCompliantUser = new OrganizationUserUserDetails { + Id = Guid.NewGuid(), OrganizationId = organization.Id, Type = OrganizationUserType.User, Status = OrganizationUserStatusType.Confirmed, @@ -179,6 +195,7 @@ public async Task OnSaveSideEffectsAsync_RemovesNonCompliantUsers( var otherOrganizationUser = new OrganizationUser { + Id = Guid.NewGuid(), OrganizationId = new Guid(), UserId = nonCompliantUserId, Status = OrganizationUserStatusType.Confirmed @@ -200,13 +217,24 @@ public async Task OnSaveSideEffectsAsync_RemovesNonCompliantUsers( await sutProvider.Sut.OnSaveSideEffectsAsync(policyUpdate, policy); + await sutProvider.GetDependency() + .DidNotReceive() + .RemoveUserAsync(policyUpdate.OrganizationId, compliantUser1.Id, savingUserId); + await sutProvider.GetDependency() + .DidNotReceive() + .RemoveUserAsync(policyUpdate.OrganizationId, compliantUser2.Id, savingUserId); await sutProvider.GetDependency() .Received(1) .RemoveUserAsync(policyUpdate.OrganizationId, nonCompliantUser.Id, savingUserId); + await sutProvider.GetDependency() + .DidNotReceive() + .SendOrganizationUserRemovedForPolicySingleOrgEmailAsync(organization.DisplayName(), compliantUser1.Email); + await sutProvider.GetDependency() + .DidNotReceive() + .SendOrganizationUserRemovedForPolicySingleOrgEmailAsync(organization.DisplayName(), compliantUser2.Email); await sutProvider.GetDependency() .Received(1) - .SendOrganizationUserRemovedForPolicySingleOrgEmailAsync(organization.DisplayName(), - "user3@example.com"); + .SendOrganizationUserRemovedForPolicySingleOrgEmailAsync(organization.DisplayName(), nonCompliantUser.Email); } [Theory, BitAutoData] diff --git a/util/Migrator/DbScripts/2024-12-11-00_BumpAccountRevisionDateJsonIds.sql b/util/Migrator/DbScripts/2024-12-11-00_BumpAccountRevisionDateJsonIds.sql new file mode 100644 index 000000000000..11d1d75a31c2 --- /dev/null +++ b/util/Migrator/DbScripts/2024-12-11-00_BumpAccountRevisionDateJsonIds.sql @@ -0,0 +1,64 @@ +CREATE OR ALTER PROCEDURE [dbo].[User_BumpAccountRevisionDateByOrganizationUserIdsJson] + @OrganizationUserIds NVARCHAR(MAX) +AS +BEGIN + SET NOCOUNT ON + + CREATE TABLE #UserIds + ( + UserId UNIQUEIDENTIFIER NOT NULL + ); + + INSERT INTO #UserIds (UserId) + SELECT + OU.UserId + FROM + [dbo].[OrganizationUser] OU + INNER JOIN + (SELECT [value] as Id FROM OPENJSON(@OrganizationUserIds)) AS OUIds + ON OUIds.Id = OU.Id + WHERE + OU.[Status] = 2 -- Confirmed + + UPDATE + U + SET + U.[AccountRevisionDate] = GETUTCDATE() + FROM + [dbo].[User] U + INNER JOIN + #UserIds ON U.[Id] = #UserIds.[UserId] + + DROP TABLE #UserIds +END +GO + +CREATE OR ALTER PROCEDURE [dbo].[OrganizationUser_SetStatusForUsersById] + @OrganizationUserIds AS NVARCHAR(MAX), + @Status SMALLINT +AS +BEGIN + SET NOCOUNT ON + + -- Declare a table variable to hold the parsed JSON data + DECLARE @ParsedIds TABLE (Id UNIQUEIDENTIFIER); + + -- Parse the JSON input into the table variable + INSERT INTO @ParsedIds (Id) + SELECT value + FROM OPENJSON(@OrganizationUserIds); + + -- Check if the input table is empty + IF (SELECT COUNT(1) FROM @ParsedIds) < 1 + BEGIN + RETURN(-1); + END + + UPDATE + [dbo].[OrganizationUser] + SET [Status] = @Status + WHERE [Id] IN (SELECT Id from @ParsedIds) + + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationUserIdsJson] @OrganizationUserIds +END +GO From 2e8f2df9428edf19a272141a8d36dc7ddad20ed4 Mon Sep 17 00:00:00 2001 From: Ike <137194738+ike-kottlowski@users.noreply.github.com> Date: Tue, 17 Dec 2024 08:59:39 -0800 Subject: [PATCH 11/11] feat(NewDeviceVerification) : (#5153) feat(NewDeviceVerification) : Added constat for the cache key in Bit.Core because the cache key format needs to be shared between the Identity Server and the MVC Admin project. Updated DeviceValidator class to handle checking cache for user information to allow pass through. Updated and Added tests to handle new flow. --- src/Core/Constants.cs | 2 +- .../RequestValidators/DeviceValidator.cs | 18 ++++++++- .../IdentityServer/DeviceValidatorTests.cs | 38 ++++++++++++++++++- 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 86e49fa6cf72..9b51b12d6296 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -57,7 +57,7 @@ public static class AuthConstants public static readonly RangeConstant ARGON2_ITERATIONS = new(2, 10, 3); public static readonly RangeConstant ARGON2_MEMORY = new(15, 1024, 64); public static readonly RangeConstant ARGON2_PARALLELISM = new(1, 16, 4); - + public static readonly string NewDeviceVerificationExceptionCacheKeyFormat = "NewDeviceVerificationException_{0}"; } public class RangeConstant diff --git a/src/Identity/IdentityServer/RequestValidators/DeviceValidator.cs b/src/Identity/IdentityServer/RequestValidators/DeviceValidator.cs index 2a048bcb2aab..d59417bfa72d 100644 --- a/src/Identity/IdentityServer/RequestValidators/DeviceValidator.cs +++ b/src/Identity/IdentityServer/RequestValidators/DeviceValidator.cs @@ -10,6 +10,7 @@ using Bit.Core.Settings; using Bit.Identity.IdentityServer.Enums; using Duende.IdentityServer.Validation; +using Microsoft.Extensions.Caching.Distributed; namespace Bit.Identity.IdentityServer.RequestValidators; @@ -20,6 +21,8 @@ public class DeviceValidator( IMailService mailService, ICurrentContext currentContext, IUserService userService, + IDistributedCache distributedCache, + ILogger logger, IFeatureService featureService) : IDeviceValidator { private readonly IDeviceService _deviceService = deviceService; @@ -28,6 +31,8 @@ public class DeviceValidator( private readonly IMailService _mailService = mailService; private readonly ICurrentContext _currentContext = currentContext; private readonly IUserService _userService = userService; + private readonly IDistributedCache distributedCache = distributedCache; + private readonly ILogger _logger = logger; private readonly IFeatureService _featureService = featureService; public async Task ValidateRequestDeviceAsync(ValidatedTokenRequest request, CustomValidatorRequestContext context) @@ -67,7 +72,6 @@ public async Task ValidateRequestDeviceAsync(ValidatedTokenRequest request !context.SsoRequired && _globalSettings.EnableNewDeviceVerification) { - // We only want to return early if the device is invalid or there is an error var validationResult = await HandleNewDeviceVerificationAsync(context.User, request); if (validationResult != DeviceValidationResultType.Success) { @@ -121,6 +125,18 @@ private async Task HandleNewDeviceVerificationAsync( return DeviceValidationResultType.InvalidUser; } + // CS exception flow + // Check cache for user information + var cacheKey = string.Format(AuthConstants.NewDeviceVerificationExceptionCacheKeyFormat, user.Id.ToString()); + var cacheValue = await distributedCache.GetAsync(cacheKey); + if (cacheValue != null) + { + // if found in cache return success result and remove from cache + await distributedCache.RemoveAsync(cacheKey); + _logger.LogInformation("New device verification exception for user {UserId} found in cache", user.Id); + return DeviceValidationResultType.Success; + } + // parse request for NewDeviceOtp to validate var newDeviceOtp = request.Raw["NewDeviceOtp"]?.ToString(); // we only check null here since an empty OTP will be considered an incorrect OTP diff --git a/test/Identity.Test/IdentityServer/DeviceValidatorTests.cs b/test/Identity.Test/IdentityServer/DeviceValidatorTests.cs index 304715b68cb1..105267ea305d 100644 --- a/test/Identity.Test/IdentityServer/DeviceValidatorTests.cs +++ b/test/Identity.Test/IdentityServer/DeviceValidatorTests.cs @@ -10,6 +10,8 @@ using Bit.Identity.IdentityServer.RequestValidators; using Bit.Test.Common.AutoFixture.Attributes; using Duende.IdentityServer.Validation; +using Microsoft.Extensions.Caching.Distributed; +using Microsoft.Extensions.Logging; using NSubstitute; using Xunit; using AuthFixtures = Bit.Identity.Test.AutoFixture; @@ -24,6 +26,8 @@ public class DeviceValidatorTests private readonly IMailService _mailService; private readonly ICurrentContext _currentContext; private readonly IUserService _userService; + private readonly IDistributedCache _distributedCache; + private readonly Logger _logger; private readonly IFeatureService _featureService; private readonly DeviceValidator _sut; @@ -35,6 +39,8 @@ public DeviceValidatorTests() _mailService = Substitute.For(); _currentContext = Substitute.For(); _userService = Substitute.For(); + _distributedCache = Substitute.For(); + _logger = new Logger(Substitute.For()); _featureService = Substitute.For(); _sut = new DeviceValidator( _deviceService, @@ -43,6 +49,8 @@ public DeviceValidatorTests() _mailService, _currentContext, _userService, + _distributedCache, + _logger, _featureService); } @@ -51,7 +59,7 @@ public async void GetKnownDeviceAsync_UserNull_ReturnsFalse( Device device) { // Arrange - // AutoData arrages + // AutoData arranges // Act var result = await _sut.GetKnownDeviceAsync(null, device); @@ -421,6 +429,30 @@ public async void HandleNewDeviceVerificationAsync_UserNull_ContextModified_Retu Assert.Equal(expectedErrorMessage, actualResponse.Message); } + [Theory, BitAutoData] + public async void HandleNewDeviceVerificationAsync_UserHasCacheValue_ReturnsSuccess( + CustomValidatorRequestContext context, + [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest request) + { + // Arrange + ArrangeForHandleNewDeviceVerificationTest(context, request); + _featureService.IsEnabled(FeatureFlagKeys.NewDeviceVerification).Returns(true); + _globalSettings.EnableNewDeviceVerification = true; + _distributedCache.GetAsync(Arg.Any()).Returns([1]); + + // Act + var result = await _sut.ValidateRequestDeviceAsync(request, context); + + // Assert + await _userService.Received(0).SendOTPAsync(context.User); + await _deviceService.Received(1).SaveAsync(Arg.Any()); + + Assert.True(result); + Assert.False(context.CustomResponse.ContainsKey("ErrorModel")); + Assert.Equal(context.User.Id, context.Device.UserId); + Assert.NotNull(context.Device); + } + [Theory, BitAutoData] public async void HandleNewDeviceVerificationAsync_NewDeviceOtpValid_ReturnsSuccess( CustomValidatorRequestContext context, @@ -430,6 +462,7 @@ public async void HandleNewDeviceVerificationAsync_NewDeviceOtpValid_ReturnsSucc ArrangeForHandleNewDeviceVerificationTest(context, request); _featureService.IsEnabled(FeatureFlagKeys.NewDeviceVerification).Returns(true); _globalSettings.EnableNewDeviceVerification = true; + _distributedCache.GetAsync(Arg.Any()).Returns(null as byte[]); var newDeviceOtp = "123456"; request.Raw.Add("NewDeviceOtp", newDeviceOtp); @@ -461,6 +494,7 @@ public async void HandleNewDeviceVerificationAsync_NewDeviceOtpInvalid_ReturnsIn ArrangeForHandleNewDeviceVerificationTest(context, request); _featureService.IsEnabled(FeatureFlagKeys.NewDeviceVerification).Returns(true); _globalSettings.EnableNewDeviceVerification = true; + _distributedCache.GetAsync(Arg.Any()).Returns(null as byte[]); request.Raw.Add("NewDeviceOtp", newDeviceOtp); @@ -489,6 +523,7 @@ public async void HandleNewDeviceVerificationAsync_UserHasNoDevices_ReturnsSucce ArrangeForHandleNewDeviceVerificationTest(context, request); _featureService.IsEnabled(FeatureFlagKeys.NewDeviceVerification).Returns(true); _globalSettings.EnableNewDeviceVerification = true; + _distributedCache.GetAsync(Arg.Any()).Returns([1]); _deviceRepository.GetManyByUserIdAsync(context.User.Id).Returns([]); // Act @@ -515,6 +550,7 @@ public async void HandleNewDeviceVerificationAsync_NewDeviceOtpEmpty_UserHasDevi _featureService.IsEnabled(FeatureFlagKeys.NewDeviceVerification).Returns(true); _globalSettings.EnableNewDeviceVerification = true; _deviceRepository.GetManyByUserIdAsync(context.User.Id).Returns([new Device()]); + _distributedCache.GetAsync(Arg.Any()).Returns(null as byte[]); // Act var result = await _sut.ValidateRequestDeviceAsync(request, context);