diff --git a/src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs b/src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs index ed628105e0..233dc138a6 100644 --- a/src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs +++ b/src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs @@ -13,9 +13,10 @@ public static class AuthorizationHandlerCollectionExtensions services.TryAddEnumerable([ ServiceDescriptor.Scoped(), - ServiceDescriptor.Scoped(), - ServiceDescriptor.Scoped(), - ServiceDescriptor.Scoped(), - ]); + ServiceDescriptor.Scoped(), + ServiceDescriptor.Scoped(), + ServiceDescriptor.Scoped(), + ServiceDescriptor.Scoped(), + ]); } } diff --git a/src/Api/AdminConsole/Authorization/RecoverAccountAuthorizationHandler.cs b/src/Api/AdminConsole/Authorization/RecoverAccountAuthorizationHandler.cs new file mode 100644 index 0000000000..239148ab25 --- /dev/null +++ b/src/Api/AdminConsole/Authorization/RecoverAccountAuthorizationHandler.cs @@ -0,0 +1,110 @@ +using System.Security.Claims; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Microsoft.AspNetCore.Authorization; + +namespace Bit.Api.AdminConsole.Authorization; + +/// +/// An authorization requirement for recovering an organization member's account. +/// +/// +/// Note: this is different to simply being able to manage account recovery. The user must be recovering +/// a member who has equal or lesser permissions than them. +/// +public class RecoverAccountAuthorizationRequirement : IAuthorizationRequirement; + +/// +/// Authorizes members and providers to recover a target OrganizationUser's account. +/// +/// +/// This prevents privilege escalation by ensuring that a user cannot recover the account of +/// another user with a higher role or with provider membership. +/// +public class RecoverAccountAuthorizationHandler( + IOrganizationContext organizationContext, + ICurrentContext currentContext, + IProviderUserRepository providerUserRepository) + : AuthorizationHandler +{ + public const string FailureReason = "You are not permitted to recover this user's account."; + public const string ProviderFailureReason = "You are not permitted to recover a Provider member's account."; + + protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, + RecoverAccountAuthorizationRequirement requirement, + OrganizationUser targetOrganizationUser) + { + // Step 1: check that the User has permissions with respect to the organization. + // This may come from their role in the organization or their provider relationship. + var canRecoverOrganizationMember = + AuthorizeMember(context.User, targetOrganizationUser) || + await AuthorizeProviderAsync(context.User, targetOrganizationUser); + + if (!canRecoverOrganizationMember) + { + context.Fail(new AuthorizationFailureReason(this, FailureReason)); + return; + } + + // Step 2: check that the User has permissions with respect to any provider the target user is a member of. + // This prevents an organization admin performing privilege escalation into an unrelated provider. + var canRecoverProviderMember = await CanRecoverProviderAsync(targetOrganizationUser); + if (!canRecoverProviderMember) + { + context.Fail(new AuthorizationFailureReason(this, ProviderFailureReason)); + return; + } + + context.Succeed(requirement); + } + + private async Task AuthorizeProviderAsync(ClaimsPrincipal currentUser, OrganizationUser targetOrganizationUser) + { + return await organizationContext.IsProviderUserForOrganization(currentUser, targetOrganizationUser.OrganizationId); + } + + private bool AuthorizeMember(ClaimsPrincipal currentUser, OrganizationUser targetOrganizationUser) + { + var currentContextOrganization = organizationContext.GetOrganizationClaims(currentUser, targetOrganizationUser.OrganizationId); + if (currentContextOrganization == null) + { + return false; + } + + // Current user must have equal or greater permissions than the user account being recovered + var authorized = targetOrganizationUser.Type switch + { + OrganizationUserType.Owner => currentContextOrganization.Type is OrganizationUserType.Owner, + OrganizationUserType.Admin => currentContextOrganization.Type is OrganizationUserType.Owner or OrganizationUserType.Admin, + _ => currentContextOrganization is + { Type: OrganizationUserType.Owner or OrganizationUserType.Admin } + or { Type: OrganizationUserType.Custom, Permissions.ManageResetPassword: true } + }; + + return authorized; + } + + private async Task CanRecoverProviderAsync(OrganizationUser targetOrganizationUser) + { + if (!targetOrganizationUser.UserId.HasValue) + { + // If an OrganizationUser is not linked to a User then it can't be linked to a Provider either. + // This is invalid but does not pose a privilege escalation risk. Return early and let the command + // handle the invalid input. + return true; + } + + var targetUserProviderUsers = + await providerUserRepository.GetManyByUserAsync(targetOrganizationUser.UserId.Value); + + // If the target user belongs to any provider that the current user is not a member of, + // deny the action to prevent privilege escalation from organization to provider. + // Note: we do not expect that a user is a member of more than 1 provider, but there is also no guarantee + // against it; this returns a sequence, so we handle the possibility. + var authorized = targetUserProviderUsers.All(providerUser => currentContext.ProviderUser(providerUser.ProviderId)); + return authorized; + } +} + diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index 74ac9b1255..4b9f7e5d71 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -1,4 +1,5 @@ // FIXME: Update this file to be null safe and then delete the line below +// NOTE: This file is partially migrated to nullable reference types. Remove inline #nullable directives when addressing the FIXME. #nullable disable using Bit.Api.AdminConsole.Authorization; @@ -11,6 +12,7 @@ using Bit.Api.Vault.AuthorizationHandlers.Collections; using Bit.Core; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.AccountRecovery; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccount; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers; @@ -70,6 +72,7 @@ public class OrganizationUsersController : Controller private readonly IRestoreOrganizationUserCommand _restoreOrganizationUserCommand; private readonly IInitPendingOrganizationCommand _initPendingOrganizationCommand; private readonly IRevokeOrganizationUserCommand _revokeOrganizationUserCommand; + private readonly IAdminRecoverAccountCommand _adminRecoverAccountCommand; public OrganizationUsersController(IOrganizationRepository organizationRepository, IOrganizationUserRepository organizationUserRepository, @@ -97,7 +100,8 @@ public class OrganizationUsersController : Controller IRestoreOrganizationUserCommand restoreOrganizationUserCommand, IInitPendingOrganizationCommand initPendingOrganizationCommand, IRevokeOrganizationUserCommand revokeOrganizationUserCommand, - IResendOrganizationInviteCommand resendOrganizationInviteCommand) + IResendOrganizationInviteCommand resendOrganizationInviteCommand, + IAdminRecoverAccountCommand adminRecoverAccountCommand) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; @@ -126,6 +130,7 @@ public class OrganizationUsersController : Controller _restoreOrganizationUserCommand = restoreOrganizationUserCommand; _initPendingOrganizationCommand = initPendingOrganizationCommand; _revokeOrganizationUserCommand = revokeOrganizationUserCommand; + _adminRecoverAccountCommand = adminRecoverAccountCommand; } [HttpGet("{id}")] @@ -474,21 +479,27 @@ public class OrganizationUsersController : Controller [HttpPut("{id}/reset-password")] [Authorize] - public async Task PutResetPassword(Guid orgId, Guid id, [FromBody] OrganizationUserResetPasswordRequestModel model) + public async Task PutResetPassword(Guid orgId, Guid id, [FromBody] OrganizationUserResetPasswordRequestModel model) { + if (_featureService.IsEnabled(FeatureFlagKeys.AccountRecoveryCommand)) + { + // TODO: remove legacy implementation after feature flag is enabled. + return await PutResetPasswordNew(orgId, id, model); + } + // Get the users role, since provider users aren't a member of the organization we use the owner check var orgUserType = await _currentContext.OrganizationOwner(orgId) ? OrganizationUserType.Owner : _currentContext.Organizations?.FirstOrDefault(o => o.Id == orgId)?.Type; if (orgUserType == null) { - throw new NotFoundException(); + return TypedResults.NotFound(); } var result = await _userService.AdminResetPasswordAsync(orgUserType.Value, orgId, id, model.NewMasterPasswordHash, model.Key); if (result.Succeeded) { - return; + return TypedResults.Ok(); } foreach (var error in result.Errors) @@ -497,9 +508,45 @@ public class OrganizationUsersController : Controller } await Task.Delay(2000); - throw new BadRequestException(ModelState); + return TypedResults.BadRequest(ModelState); } +#nullable enable + // TODO: make sure the route and authorize attributes are maintained when the legacy implementation is removed. + private async Task PutResetPasswordNew(Guid orgId, Guid id, [FromBody] OrganizationUserResetPasswordRequestModel model) + { + var targetOrganizationUser = await _organizationUserRepository.GetByIdAsync(id); + if (targetOrganizationUser == null || targetOrganizationUser.OrganizationId != orgId) + { + return TypedResults.NotFound(); + } + + var authorizationResult = await _authorizationService.AuthorizeAsync(User, targetOrganizationUser, new RecoverAccountAuthorizationRequirement()); + if (!authorizationResult.Succeeded) + { + // Return an informative error to show in the UI. + // The Authorize attribute already prevents enumeration by users outside the organization, so this can be specific. + var failureReason = authorizationResult.Failure?.FailureReasons.FirstOrDefault()?.Message ?? RecoverAccountAuthorizationHandler.FailureReason; + // This should be a 403 Forbidden, but that causes a logout on our client apps so we're using 400 Bad Request instead + return TypedResults.BadRequest(new ErrorResponseModel(failureReason)); + } + + var result = await _adminRecoverAccountCommand.RecoverAccountAsync(orgId, targetOrganizationUser, model.NewMasterPasswordHash, model.Key); + if (result.Succeeded) + { + return TypedResults.Ok(); + } + + foreach (var error in result.Errors) + { + ModelState.AddModelError(string.Empty, error.Description); + } + + await Task.Delay(2000); + return TypedResults.BadRequest(ModelState); + } +#nullable disable + [HttpDelete("{id}")] [Authorize] public async Task Remove(Guid orgId, Guid id) diff --git a/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/AdminRecoverAccountCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/AdminRecoverAccountCommand.cs new file mode 100644 index 0000000000..5783301a0b --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/AdminRecoverAccountCommand.cs @@ -0,0 +1,79 @@ +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Platform.Push; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Microsoft.AspNetCore.Identity; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.AccountRecovery; + +public class AdminRecoverAccountCommand(IOrganizationRepository organizationRepository, + IPolicyRepository policyRepository, + IUserRepository userRepository, + IMailService mailService, + IEventService eventService, + IPushNotificationService pushNotificationService, + IUserService userService, + TimeProvider timeProvider) : IAdminRecoverAccountCommand +{ + public async Task RecoverAccountAsync(Guid orgId, + OrganizationUser organizationUser, string newMasterPassword, string key) + { + // Org must be able to use reset password + var org = await organizationRepository.GetByIdAsync(orgId); + if (org == null || !org.UseResetPassword) + { + throw new BadRequestException("Organization does not allow password reset."); + } + + // Enterprise policy must be enabled + var resetPasswordPolicy = + await policyRepository.GetByOrganizationIdTypeAsync(orgId, PolicyType.ResetPassword); + if (resetPasswordPolicy == null || !resetPasswordPolicy.Enabled) + { + throw new BadRequestException("Organization does not have the password reset policy enabled."); + } + + // Org User must be confirmed and have a ResetPasswordKey + if (organizationUser == null || + organizationUser.Status != OrganizationUserStatusType.Confirmed || + organizationUser.OrganizationId != orgId || + string.IsNullOrEmpty(organizationUser.ResetPasswordKey) || + !organizationUser.UserId.HasValue) + { + throw new BadRequestException("Organization User not valid"); + } + + var user = await userService.GetUserByIdAsync(organizationUser.UserId.Value); + if (user == null) + { + throw new NotFoundException(); + } + + if (user.UsesKeyConnector) + { + throw new BadRequestException("Cannot reset password of a user with Key Connector."); + } + + var result = await userService.UpdatePasswordHash(user, newMasterPassword); + if (!result.Succeeded) + { + return result; + } + + user.RevisionDate = user.AccountRevisionDate = timeProvider.GetUtcNow().UtcDateTime; + user.LastPasswordChangeDate = user.RevisionDate; + user.ForcePasswordReset = true; + user.Key = key; + + await userRepository.ReplaceAsync(user); + await mailService.SendAdminResetPasswordEmailAsync(user.Email, user.Name, org.DisplayName()); + await eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_AdminResetPassword); + await pushNotificationService.PushLogOutAsync(user.Id); + + return IdentityResult.Success; + } +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/IAdminRecoverAccountCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/IAdminRecoverAccountCommand.cs new file mode 100644 index 0000000000..75babc643e --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/IAdminRecoverAccountCommand.cs @@ -0,0 +1,24 @@ +using Bit.Core.Entities; +using Bit.Core.Exceptions; +using Microsoft.AspNetCore.Identity; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.AccountRecovery; + +/// +/// A command used to recover an organization user's account by an organization admin. +/// +public interface IAdminRecoverAccountCommand +{ + /// + /// Recovers an organization user's account by resetting their master password. + /// + /// The organization the user belongs to. + /// The organization user being recovered. + /// The user's new master password hash. + /// The user's new master-password-sealed user key. + /// An IdentityResult indicating success or failure. + /// When organization settings, policy, or user state is invalid. + /// When the user does not exist. + Task RecoverAccountAsync(Guid orgId, OrganizationUser organizationUser, + string newMasterPassword, string key); +} diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index d147f3908b..fead9947a0 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -142,6 +142,7 @@ public static class FeatureFlagKeys public const string CreateDefaultLocation = "pm-19467-create-default-location"; public const string AutomaticConfirmUsers = "pm-19934-auto-confirm-organization-users"; public const string PM23845_VNextApplicationCache = "pm-24957-refactor-memory-application-cache"; + public const string AccountRecoveryCommand = "pm-24192-account-recovery-command"; /* Auth Team */ public const string TwoFactorExtensionDataPersistence = "pm-9115-two-factor-extension-data-persistence"; diff --git a/src/Core/Context/ICurrentContext.cs b/src/Core/Context/ICurrentContext.cs index 417e220ba2..f62a048070 100644 --- a/src/Core/Context/ICurrentContext.cs +++ b/src/Core/Context/ICurrentContext.cs @@ -1,6 +1,4 @@ -#nullable enable - -using System.Security.Claims; +using System.Security.Claims; using Bit.Core.AdminConsole.Context; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Auth.Identity; @@ -12,6 +10,14 @@ using Microsoft.AspNetCore.Http; namespace Bit.Core.Context; +/// +/// Provides information about the current HTTP request and the currently authenticated user (if any). +/// This is often (but not exclusively) parsed from the JWT in the current request. +/// +/// +/// This interface suffers from having too much responsibility; consider whether any new code can go in a more +/// specific class rather than adding it here. +/// public interface ICurrentContext { HttpContext HttpContext { get; set; } @@ -59,8 +65,20 @@ public interface ICurrentContext Task EditSubscription(Guid orgId); Task EditPaymentMethods(Guid orgId); Task ViewBillingHistory(Guid orgId); + /// + /// Returns true if the current user is a member of a provider that manages the specified organization. + /// This generally gives the user administrative privileges for the organization. + /// + /// + /// Task ProviderUserForOrgAsync(Guid orgId); + /// + /// Returns true if the current user is a Provider Admin of the specified provider. + /// bool ProviderProviderAdmin(Guid providerId); + /// + /// Returns true if the current user is a member of the specified provider (with any role). + /// bool ProviderUser(Guid providerId); bool ProviderManageUsers(Guid providerId); bool ProviderAccessEventLogs(Guid providerId); diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index da05bc929c..8cfd0a8df1 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -1,5 +1,6 @@ using Bit.Core.AdminConsole.OrganizationAuth; using Bit.Core.AdminConsole.OrganizationAuth.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.AccountRecovery; using Bit.Core.AdminConsole.OrganizationFeatures.Groups; using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.Import; @@ -133,6 +134,7 @@ public static class OrganizationServiceCollectionExtensions services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); diff --git a/src/Core/Services/IMailService.cs b/src/Core/Services/IMailService.cs index 5a3428c25a..91bbde949b 100644 --- a/src/Core/Services/IMailService.cs +++ b/src/Core/Services/IMailService.cs @@ -1,6 +1,4 @@ -#nullable enable - -using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Entities.Provider; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; @@ -92,7 +90,7 @@ public interface IMailService Task SendEmergencyAccessRecoveryReminder(EmergencyAccess emergencyAccess, string initiatingName, string email); Task SendEmergencyAccessRecoveryTimedOut(EmergencyAccess ea, string initiatingName, string email); Task SendEnqueuedMailMessageAsync(IMailQueueMessage queueMessage); - Task SendAdminResetPasswordEmailAsync(string email, string userName, string orgName); + Task SendAdminResetPasswordEmailAsync(string email, string? userName, string orgName); Task SendProviderSetupInviteEmailAsync(Provider provider, string token, string email); Task SendBusinessUnitConversionInviteAsync(Organization organization, string token, string email); Task SendProviderInviteEmailAsync(string providerName, ProviderUser providerUser, string token, string email); diff --git a/src/Core/Services/Implementations/HandlebarsMailService.cs b/src/Core/Services/Implementations/HandlebarsMailService.cs index 19705766ed..e8707d13e8 100644 --- a/src/Core/Services/Implementations/HandlebarsMailService.cs +++ b/src/Core/Services/Implementations/HandlebarsMailService.cs @@ -674,7 +674,7 @@ public class HandlebarsMailService : IMailService await _mailDeliveryService.SendEmailAsync(message); } - public async Task SendAdminResetPasswordEmailAsync(string email, string userName, string orgName) + public async Task SendAdminResetPasswordEmailAsync(string email, string? userName, string orgName) { var message = CreateDefaultMessage("Your admin has initiated account recovery", email); var model = new AdminResetPasswordViewModel() diff --git a/src/Core/Services/NoopImplementations/NoopMailService.cs b/src/Core/Services/NoopImplementations/NoopMailService.cs index 1459fab966..5e7c67bd61 100644 --- a/src/Core/Services/NoopImplementations/NoopMailService.cs +++ b/src/Core/Services/NoopImplementations/NoopMailService.cs @@ -221,7 +221,7 @@ public class NoopMailService : IMailService return Task.FromResult(0); } - public Task SendAdminResetPasswordEmailAsync(string email, string userName, string orgName) + public Task SendAdminResetPasswordEmailAsync(string email, string? userName, string orgName) { return Task.FromResult(0); } diff --git a/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationUsersControllerPutResetPasswordTests.cs b/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationUsersControllerPutResetPasswordTests.cs new file mode 100644 index 0000000000..cf842d1568 --- /dev/null +++ b/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationUsersControllerPutResetPasswordTests.cs @@ -0,0 +1,197 @@ +using System.Net; +using Bit.Api.AdminConsole.Authorization; +using Bit.Api.IntegrationTest.Factories; +using Bit.Api.IntegrationTest.Helpers; +using Bit.Api.Models.Request.Organizations; +using Bit.Core; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Entities.Provider; +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Enums.Provider; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Billing.Enums; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Models.Api; +using Bit.Core.Repositories; +using Bit.Core.Services; +using NSubstitute; +using Xunit; + +namespace Bit.Api.IntegrationTest.AdminConsole.Controllers; + +public class OrganizationUsersControllerPutResetPasswordTests : IClassFixture, IAsyncLifetime +{ + private readonly HttpClient _client; + private readonly ApiApplicationFactory _factory; + private readonly LoginHelper _loginHelper; + + private Organization _organization = null!; + private string _ownerEmail = null!; + + public OrganizationUsersControllerPutResetPasswordTests(ApiApplicationFactory apiFactory) + { + _factory = apiFactory; + _factory.SubstituteService(featureService => + { + featureService + .IsEnabled(FeatureFlagKeys.AccountRecoveryCommand) + .Returns(true); + }); + _client = _factory.CreateClient(); + _loginHelper = new LoginHelper(_factory, _client); + } + + public async Task InitializeAsync() + { + _ownerEmail = $"reset-password-test-{Guid.NewGuid()}@example.com"; + await _factory.LoginWithNewAccount(_ownerEmail); + + (_organization, _) = await OrganizationTestHelpers.SignUpAsync(_factory, plan: PlanType.EnterpriseAnnually2023, + ownerEmail: _ownerEmail, passwordManagerSeats: 5, paymentMethod: PaymentMethodType.Card); + + // Enable reset password and policies for the organization + var organizationRepository = _factory.GetService(); + _organization.UseResetPassword = true; + _organization.UsePolicies = true; + await organizationRepository.ReplaceAsync(_organization); + + // Enable the ResetPassword policy + var policyRepository = _factory.GetService(); + await policyRepository.CreateAsync(new Policy + { + OrganizationId = _organization.Id, + Type = PolicyType.ResetPassword, + Enabled = true, + Data = "{}" + }); + } + + public Task DisposeAsync() + { + _client.Dispose(); + return Task.CompletedTask; + } + + /// + /// Helper method to set the ResetPasswordKey on an organization user, which is required for account recovery + /// + private async Task SetResetPasswordKeyAsync(OrganizationUser orgUser) + { + var organizationUserRepository = _factory.GetService(); + orgUser.ResetPasswordKey = "encrypted-reset-password-key"; + await organizationUserRepository.ReplaceAsync(orgUser); + } + + [Fact] + public async Task PutResetPassword_AsHigherRole_CanRecoverLowerRole() + { + // Arrange + var (ownerEmail, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory, + _organization.Id, OrganizationUserType.Owner); + await _loginHelper.LoginAsync(ownerEmail); + + var (_, targetOrgUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync( + _factory, _organization.Id, OrganizationUserType.User); + await SetResetPasswordKeyAsync(targetOrgUser); + + var resetPasswordRequest = new OrganizationUserResetPasswordRequestModel + { + NewMasterPasswordHash = "new-master-password-hash", + Key = "encrypted-recovery-key" + }; + + // Act + var response = await _client.PutAsJsonAsync( + $"organizations/{_organization.Id}/users/{targetOrgUser.Id}/reset-password", + resetPasswordRequest); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + [Fact] + public async Task PutResetPassword_AsLowerRole_CannotRecoverHigherRole() + { + // Arrange + var (adminEmail, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory, + _organization.Id, OrganizationUserType.Admin); + await _loginHelper.LoginAsync(adminEmail); + + var (_, targetOwnerOrgUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync( + _factory, _organization.Id, OrganizationUserType.Owner); + await SetResetPasswordKeyAsync(targetOwnerOrgUser); + + var resetPasswordRequest = new OrganizationUserResetPasswordRequestModel + { + NewMasterPasswordHash = "new-master-password-hash", + Key = "encrypted-recovery-key" + }; + + // Act + var response = await _client.PutAsJsonAsync( + $"organizations/{_organization.Id}/users/{targetOwnerOrgUser.Id}/reset-password", + resetPasswordRequest); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var model = await response.Content.ReadFromJsonAsync(); + Assert.Contains(RecoverAccountAuthorizationHandler.FailureReason, model.Message); + } + + [Fact] + public async Task PutResetPassword_CannotRecoverProviderAccount() + { + // Arrange - Create owner who will try to recover the provider account + var (ownerEmail, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory, + _organization.Id, OrganizationUserType.Owner); + await _loginHelper.LoginAsync(ownerEmail); + + // Create a user who is also a provider user + var (targetUserEmail, targetOrgUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync( + _factory, _organization.Id, OrganizationUserType.User); + await SetResetPasswordKeyAsync(targetOrgUser); + + // Add the target user as a provider user to a different provider + var providerRepository = _factory.GetService(); + var providerUserRepository = _factory.GetService(); + var userRepository = _factory.GetService(); + + var provider = await providerRepository.CreateAsync(new Provider + { + Name = "Test Provider", + BusinessName = "Test Provider Business", + BillingEmail = "provider@example.com", + Type = ProviderType.Msp, + Status = ProviderStatusType.Created, + Enabled = true + }); + + var targetUser = await userRepository.GetByEmailAsync(targetUserEmail); + Assert.NotNull(targetUser); + + await providerUserRepository.CreateAsync(new ProviderUser + { + ProviderId = provider.Id, + UserId = targetUser.Id, + Status = ProviderUserStatusType.Confirmed, + Type = ProviderUserType.ProviderAdmin + }); + + var resetPasswordRequest = new OrganizationUserResetPasswordRequestModel + { + NewMasterPasswordHash = "new-master-password-hash", + Key = "encrypted-recovery-key" + }; + + // Act + var response = await _client.PutAsJsonAsync( + $"organizations/{_organization.Id}/users/{targetOrgUser.Id}/reset-password", + resetPasswordRequest); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var model = await response.Content.ReadFromJsonAsync(); + Assert.Equal(RecoverAccountAuthorizationHandler.ProviderFailureReason, model.Message); + } +} diff --git a/test/Api.Test/AdminConsole/Authorization/RecoverAccountAuthorizationHandlerTests.cs b/test/Api.Test/AdminConsole/Authorization/RecoverAccountAuthorizationHandlerTests.cs new file mode 100644 index 0000000000..92efb641f1 --- /dev/null +++ b/test/Api.Test/AdminConsole/Authorization/RecoverAccountAuthorizationHandlerTests.cs @@ -0,0 +1,296 @@ +using System.Security.Claims; +using Bit.Api.AdminConsole.Authorization; +using Bit.Core.AdminConsole.Entities.Provider; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Models.Data; +using Bit.Core.Test.AutoFixture.OrganizationUserFixtures; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Authorization; +using NSubstitute; +using Xunit; + +namespace Bit.Api.Test.AdminConsole.Authorization; + +[SutProviderCustomize] +public class RecoverAccountAuthorizationHandlerTests +{ + [Theory, BitAutoData] + public async Task HandleRequirementAsync_CurrentUserIsProvider_TargetUserNotProvider_Authorized( + SutProvider sutProvider, + [OrganizationUser] OrganizationUser targetOrganizationUser, + ClaimsPrincipal claimsPrincipal) + { + // Arrange + var context = new AuthorizationHandlerContext( + [new RecoverAccountAuthorizationRequirement()], + claimsPrincipal, + targetOrganizationUser); + + MockOrganizationClaims(sutProvider, claimsPrincipal, targetOrganizationUser, null); + MockCurrentUserIsProvider(sutProvider, claimsPrincipal, targetOrganizationUser); + + // Act + await sutProvider.Sut.HandleAsync(context); + + // Assert + Assert.True(context.HasSucceeded); + } + + [Theory, BitAutoData] + public async Task HandleRequirementAsync_CurrentUserIsNotMemberOrProvider_NotAuthorized( + SutProvider sutProvider, + [OrganizationUser] OrganizationUser targetOrganizationUser, + ClaimsPrincipal claimsPrincipal) + { + // Arrange + var context = new AuthorizationHandlerContext( + [new RecoverAccountAuthorizationRequirement()], + claimsPrincipal, + targetOrganizationUser); + + MockOrganizationClaims(sutProvider, claimsPrincipal, targetOrganizationUser, null); + + // Act + await sutProvider.Sut.HandleAsync(context); + + // Assert + AssertFailed(context, RecoverAccountAuthorizationHandler.FailureReason); + } + + // Pairing of CurrentContextOrganization (current user permissions) and target user role + // Read this as: a ___ can recover the account for a ___ + public static IEnumerable AuthorizedRoleCombinations => new object[][] + { + [new CurrentContextOrganization { Type = OrganizationUserType.Owner }, OrganizationUserType.Owner], + [new CurrentContextOrganization { Type = OrganizationUserType.Owner }, OrganizationUserType.Admin], + [new CurrentContextOrganization { Type = OrganizationUserType.Owner }, OrganizationUserType.Custom], + [new CurrentContextOrganization { Type = OrganizationUserType.Owner }, OrganizationUserType.User], + [new CurrentContextOrganization { Type = OrganizationUserType.Admin }, OrganizationUserType.Admin], + [new CurrentContextOrganization { Type = OrganizationUserType.Admin }, OrganizationUserType.Custom], + [new CurrentContextOrganization { Type = OrganizationUserType.Admin }, OrganizationUserType.User], + [new CurrentContextOrganization { Type = OrganizationUserType.Custom, Permissions = new Permissions { ManageResetPassword = true}}, OrganizationUserType.Custom], + [new CurrentContextOrganization { Type = OrganizationUserType.Custom, Permissions = new Permissions { ManageResetPassword = true}}, OrganizationUserType.User], + }; + + [Theory, BitMemberAutoData(nameof(AuthorizedRoleCombinations))] + public async Task AuthorizeMemberAsync_RecoverEqualOrLesserRoles_TargetUserNotProvider_Authorized( + CurrentContextOrganization currentContextOrganization, + OrganizationUserType targetOrganizationUserType, + SutProvider sutProvider, + [OrganizationUser] OrganizationUser targetOrganizationUser, + ClaimsPrincipal claimsPrincipal) + { + // Arrange + targetOrganizationUser.Type = targetOrganizationUserType; + currentContextOrganization.Id = targetOrganizationUser.OrganizationId; + + var context = new AuthorizationHandlerContext( + [new RecoverAccountAuthorizationRequirement()], + claimsPrincipal, + targetOrganizationUser); + + MockOrganizationClaims(sutProvider, claimsPrincipal, targetOrganizationUser, currentContextOrganization); + + // Act + await sutProvider.Sut.HandleAsync(context); + + // Assert + Assert.True(context.HasSucceeded); + } + + // Pairing of CurrentContextOrganization (current user permissions) and target user role + // Read this as: a ___ cannot recover the account for a ___ + public static IEnumerable UnauthorizedRoleCombinations => new object[][] + { + // These roles should fail because you cannot recover a greater role + [new CurrentContextOrganization { Type = OrganizationUserType.Admin }, OrganizationUserType.Owner], + [new CurrentContextOrganization { Type = OrganizationUserType.Custom, Permissions = new Permissions { ManageResetPassword = true}}, OrganizationUserType.Owner], + [new CurrentContextOrganization { Type = OrganizationUserType.Custom, Permissions = new Permissions { ManageResetPassword = true} }, OrganizationUserType.Admin], + + // These roles are never authorized to recover any account + [new CurrentContextOrganization { Type = OrganizationUserType.User }, OrganizationUserType.Owner], + [new CurrentContextOrganization { Type = OrganizationUserType.User }, OrganizationUserType.Admin], + [new CurrentContextOrganization { Type = OrganizationUserType.User }, OrganizationUserType.Custom], + [new CurrentContextOrganization { Type = OrganizationUserType.User }, OrganizationUserType.User], + [new CurrentContextOrganization { Type = OrganizationUserType.Custom }, OrganizationUserType.Owner], + [new CurrentContextOrganization { Type = OrganizationUserType.Custom }, OrganizationUserType.Admin], + [new CurrentContextOrganization { Type = OrganizationUserType.Custom }, OrganizationUserType.Custom], + [new CurrentContextOrganization { Type = OrganizationUserType.Custom }, OrganizationUserType.User], + }; + + [Theory, BitMemberAutoData(nameof(UnauthorizedRoleCombinations))] + public async Task AuthorizeMemberAsync_InvalidRoles_TargetUserNotProvider_Unauthorized( + CurrentContextOrganization currentContextOrganization, + OrganizationUserType targetOrganizationUserType, + SutProvider sutProvider, + [OrganizationUser] OrganizationUser targetOrganizationUser, + ClaimsPrincipal claimsPrincipal) + { + // Arrange + targetOrganizationUser.Type = targetOrganizationUserType; + currentContextOrganization.Id = targetOrganizationUser.OrganizationId; + + var context = new AuthorizationHandlerContext( + [new RecoverAccountAuthorizationRequirement()], + claimsPrincipal, + targetOrganizationUser); + + MockOrganizationClaims(sutProvider, claimsPrincipal, targetOrganizationUser, currentContextOrganization); + + // Act + await sutProvider.Sut.HandleAsync(context); + + // Assert + AssertFailed(context, RecoverAccountAuthorizationHandler.FailureReason); + } + + [Theory, BitAutoData] + public async Task HandleRequirementAsync_TargetUserIdIsNull_DoesNotBlock( + SutProvider sutProvider, + OrganizationUser targetOrganizationUser, + ClaimsPrincipal claimsPrincipal) + { + // Arrange + targetOrganizationUser.UserId = null; + MockCurrentUserIsOwner(sutProvider, claimsPrincipal, targetOrganizationUser); + + var context = new AuthorizationHandlerContext( + [new RecoverAccountAuthorizationRequirement()], + claimsPrincipal, + targetOrganizationUser); + + // Act + await sutProvider.Sut.HandleAsync(context); + + // Assert + Assert.True(context.HasSucceeded); + // This should shortcut the provider escalation check + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .GetManyByUserAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task HandleRequirementAsync_CurrentUserIsMemberOfAllTargetUserProviders_DoesNotBlock( + SutProvider sutProvider, + [OrganizationUser] OrganizationUser targetOrganizationUser, + ClaimsPrincipal claimsPrincipal, + Guid providerId1, + Guid providerId2) + { + // Arrange + var targetUserProviders = new List + { + new() { ProviderId = providerId1, UserId = targetOrganizationUser.UserId }, + new() { ProviderId = providerId2, UserId = targetOrganizationUser.UserId } + }; + + var context = new AuthorizationHandlerContext( + [new RecoverAccountAuthorizationRequirement()], + claimsPrincipal, + targetOrganizationUser); + + MockCurrentUserIsProvider(sutProvider, claimsPrincipal, targetOrganizationUser); + + sutProvider.GetDependency() + .GetManyByUserAsync(targetOrganizationUser.UserId!.Value) + .Returns(targetUserProviders); + + sutProvider.GetDependency() + .ProviderUser(providerId1) + .Returns(true); + + sutProvider.GetDependency() + .ProviderUser(providerId2) + .Returns(true); + + // Act + await sutProvider.Sut.HandleAsync(context); + + // Assert + Assert.True(context.HasSucceeded); + } + + [Theory, BitAutoData] + public async Task HandleRequirementAsync_CurrentUserMissingProviderMembership_Blocks( + SutProvider sutProvider, + [OrganizationUser] OrganizationUser targetOrganizationUser, + ClaimsPrincipal claimsPrincipal, + Guid providerId1, + Guid providerId2) + { + // Arrange + var targetUserProviders = new List + { + new() { ProviderId = providerId1, UserId = targetOrganizationUser.UserId }, + new() { ProviderId = providerId2, UserId = targetOrganizationUser.UserId } + }; + + var context = new AuthorizationHandlerContext( + [new RecoverAccountAuthorizationRequirement()], + claimsPrincipal, + targetOrganizationUser); + + MockCurrentUserIsOwner(sutProvider, claimsPrincipal, targetOrganizationUser); + + sutProvider.GetDependency() + .GetManyByUserAsync(targetOrganizationUser.UserId!.Value) + .Returns(targetUserProviders); + + sutProvider.GetDependency() + .ProviderUser(providerId1) + .Returns(true); + + // Not a member of this provider + sutProvider.GetDependency() + .ProviderUser(providerId2) + .Returns(false); + + // Act + await sutProvider.Sut.HandleAsync(context); + + // Assert + AssertFailed(context, RecoverAccountAuthorizationHandler.ProviderFailureReason); + } + + private static void MockOrganizationClaims(SutProvider sutProvider, + ClaimsPrincipal currentUser, OrganizationUser targetOrganizationUser, + CurrentContextOrganization? currentContextOrganization) + { + sutProvider.GetDependency() + .GetOrganizationClaims(currentUser, targetOrganizationUser.OrganizationId) + .Returns(currentContextOrganization); + } + + private static void MockCurrentUserIsProvider(SutProvider sutProvider, + ClaimsPrincipal currentUser, OrganizationUser targetOrganizationUser) + { + sutProvider.GetDependency() + .IsProviderUserForOrganization(currentUser, targetOrganizationUser.OrganizationId) + .Returns(true); + } + + private static void MockCurrentUserIsOwner(SutProvider sutProvider, + ClaimsPrincipal currentUser, OrganizationUser targetOrganizationUser) + { + var currentContextOrganization = new CurrentContextOrganization + { + Id = targetOrganizationUser.OrganizationId, + Type = OrganizationUserType.Owner + }; + + sutProvider.GetDependency() + .GetOrganizationClaims(currentUser, targetOrganizationUser.OrganizationId) + .Returns(currentContextOrganization); + } + + private static void AssertFailed(AuthorizationHandlerContext context, string expectedMessage) + { + Assert.True(context.HasFailed); + var failureReason = Assert.Single(context.FailureReasons); + Assert.Equal(expectedMessage, failureReason.Message); + } +} diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs index e5aa03f067..5875cda05a 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs @@ -1,11 +1,14 @@ using System.Security.Claims; +using Bit.Api.AdminConsole.Authorization; using Bit.Api.AdminConsole.Controllers; using Bit.Api.AdminConsole.Models.Request.Organizations; +using Bit.Api.Models.Request.Organizations; using Bit.Api.Vault.AuthorizationHandlers.Collections; using Bit.Core; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.AccountRecovery; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; @@ -16,6 +19,7 @@ using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.Models.Api; using Bit.Core.Models.Business; using Bit.Core.Models.Data; using Bit.Core.Models.Data.Organizations; @@ -30,6 +34,7 @@ using Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http.HttpResults; +using Microsoft.AspNetCore.Mvc.ModelBinding; using NSubstitute; using Xunit; @@ -440,4 +445,153 @@ public class OrganizationUsersControllerTests Assert.Equal("Master Password reset is required, but not provided.", exception.Message); } + + [Theory] + [BitAutoData] + public async Task PutResetPassword_WithFeatureFlagDisabled_CallsLegacyPath( + Guid orgId, Guid orgUserId, OrganizationUserResetPasswordRequestModel model, + SutProvider sutProvider) + { + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(false); + sutProvider.GetDependency().OrganizationOwner(orgId).Returns(true); + sutProvider.GetDependency().AdminResetPasswordAsync(Arg.Any(), orgId, orgUserId, model.NewMasterPasswordHash, model.Key) + .Returns(Microsoft.AspNetCore.Identity.IdentityResult.Success); + + var result = await sutProvider.Sut.PutResetPassword(orgId, orgUserId, model); + + Assert.IsType(result); + await sutProvider.GetDependency().Received(1) + .AdminResetPasswordAsync(OrganizationUserType.Owner, orgId, orgUserId, model.NewMasterPasswordHash, model.Key); + } + + [Theory] + [BitAutoData] + public async Task PutResetPassword_WithFeatureFlagDisabled_WhenOrgUserTypeIsNull_ReturnsNotFound( + Guid orgId, Guid orgUserId, OrganizationUserResetPasswordRequestModel model, + SutProvider sutProvider) + { + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(false); + sutProvider.GetDependency().OrganizationOwner(orgId).Returns(false); + sutProvider.GetDependency().Organizations.Returns(new List()); + + var result = await sutProvider.Sut.PutResetPassword(orgId, orgUserId, model); + + Assert.IsType(result); + } + + [Theory] + [BitAutoData] + public async Task PutResetPassword_WithFeatureFlagDisabled_WhenAdminResetPasswordFails_ReturnsBadRequest( + Guid orgId, Guid orgUserId, OrganizationUserResetPasswordRequestModel model, + SutProvider sutProvider) + { + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(false); + sutProvider.GetDependency().OrganizationOwner(orgId).Returns(true); + sutProvider.GetDependency().AdminResetPasswordAsync(Arg.Any(), orgId, orgUserId, model.NewMasterPasswordHash, model.Key) + .Returns(Microsoft.AspNetCore.Identity.IdentityResult.Failed(new Microsoft.AspNetCore.Identity.IdentityError { Description = "Error 1" })); + + var result = await sutProvider.Sut.PutResetPassword(orgId, orgUserId, model); + + Assert.IsType>(result); + } + + [Theory] + [BitAutoData] + public async Task PutResetPassword_WithFeatureFlagEnabled_WhenOrganizationUserNotFound_ReturnsNotFound( + Guid orgId, Guid orgUserId, OrganizationUserResetPasswordRequestModel model, + SutProvider sutProvider) + { + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(true); + sutProvider.GetDependency().GetByIdAsync(orgUserId).Returns((OrganizationUser)null); + + var result = await sutProvider.Sut.PutResetPassword(orgId, orgUserId, model); + + Assert.IsType(result); + } + + [Theory] + [BitAutoData] + public async Task PutResetPassword_WithFeatureFlagEnabled_WhenOrganizationIdMismatch_ReturnsNotFound( + Guid orgId, Guid orgUserId, OrganizationUserResetPasswordRequestModel model, OrganizationUser organizationUser, + SutProvider sutProvider) + { + organizationUser.OrganizationId = Guid.NewGuid(); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(true); + sutProvider.GetDependency().GetByIdAsync(orgUserId).Returns(organizationUser); + + var result = await sutProvider.Sut.PutResetPassword(orgId, orgUserId, model); + + Assert.IsType(result); + } + + [Theory] + [BitAutoData] + public async Task PutResetPassword_WithFeatureFlagEnabled_WhenAuthorizationFails_ReturnsBadRequest( + Guid orgId, Guid orgUserId, OrganizationUserResetPasswordRequestModel model, OrganizationUser organizationUser, + SutProvider sutProvider) + { + organizationUser.OrganizationId = orgId; + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(true); + sutProvider.GetDependency().GetByIdAsync(orgUserId).Returns(organizationUser); + sutProvider.GetDependency() + .AuthorizeAsync( + Arg.Any(), + organizationUser, + Arg.Is>(x => x.SingleOrDefault() is RecoverAccountAuthorizationRequirement)) + .Returns(AuthorizationResult.Failed()); + + var result = await sutProvider.Sut.PutResetPassword(orgId, orgUserId, model); + + Assert.IsType>(result); + } + + [Theory] + [BitAutoData] + public async Task PutResetPassword_WithFeatureFlagEnabled_WhenRecoverAccountSucceeds_ReturnsOk( + Guid orgId, Guid orgUserId, OrganizationUserResetPasswordRequestModel model, OrganizationUser organizationUser, + SutProvider sutProvider) + { + organizationUser.OrganizationId = orgId; + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(true); + sutProvider.GetDependency().GetByIdAsync(orgUserId).Returns(organizationUser); + sutProvider.GetDependency() + .AuthorizeAsync( + Arg.Any(), + organizationUser, + Arg.Is>(x => x.SingleOrDefault() is RecoverAccountAuthorizationRequirement)) + .Returns(AuthorizationResult.Success()); + sutProvider.GetDependency() + .RecoverAccountAsync(orgId, organizationUser, model.NewMasterPasswordHash, model.Key) + .Returns(Microsoft.AspNetCore.Identity.IdentityResult.Success); + + var result = await sutProvider.Sut.PutResetPassword(orgId, orgUserId, model); + + Assert.IsType(result); + await sutProvider.GetDependency().Received(1) + .RecoverAccountAsync(orgId, organizationUser, model.NewMasterPasswordHash, model.Key); + } + + [Theory] + [BitAutoData] + public async Task PutResetPassword_WithFeatureFlagEnabled_WhenRecoverAccountFails_ReturnsBadRequest( + Guid orgId, Guid orgUserId, OrganizationUserResetPasswordRequestModel model, OrganizationUser organizationUser, + SutProvider sutProvider) + { + organizationUser.OrganizationId = orgId; + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(true); + sutProvider.GetDependency().GetByIdAsync(orgUserId).Returns(organizationUser); + sutProvider.GetDependency() + .AuthorizeAsync( + Arg.Any(), + organizationUser, + Arg.Is>(x => x.SingleOrDefault() is RecoverAccountAuthorizationRequirement)) + .Returns(AuthorizationResult.Success()); + sutProvider.GetDependency() + .RecoverAccountAsync(orgId, organizationUser, model.NewMasterPasswordHash, model.Key) + .Returns(Microsoft.AspNetCore.Identity.IdentityResult.Failed(new Microsoft.AspNetCore.Identity.IdentityError { Description = "Error message" })); + + var result = await sutProvider.Sut.PutResetPassword(orgId, orgUserId, model); + + Assert.IsType>(result); + } } diff --git a/test/Core.Test/AdminConsole/AutoFixture/CurrentContextOrganizationFixtures.cs b/test/Core.Test/AdminConsole/AutoFixture/CurrentContextOrganizationFixtures.cs index 080b8ec62e..1c809f604d 100644 --- a/test/Core.Test/AdminConsole/AutoFixture/CurrentContextOrganizationFixtures.cs +++ b/test/Core.Test/AdminConsole/AutoFixture/CurrentContextOrganizationFixtures.cs @@ -1,4 +1,6 @@ -using AutoFixture; +using System.Reflection; +using AutoFixture; +using AutoFixture.Xunit2; using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.Models.Data; @@ -23,6 +25,7 @@ public class CurrentContextOrganizationCustomization : ICustomization } } +[AttributeUsage(AttributeTargets.Method)] public class CurrentContextOrganizationCustomizeAttribute : BitCustomizeAttribute { public Guid Id { get; set; } @@ -38,3 +41,19 @@ public class CurrentContextOrganizationCustomizeAttribute : BitCustomizeAttribut AccessSecretsManager = AccessSecretsManager }; } + +public class CurrentContextOrganizationAttribute : CustomizeAttribute +{ + public Guid Id { get; set; } + public OrganizationUserType Type { get; set; } = OrganizationUserType.User; + public Permissions Permissions { get; set; } = new(); + public bool AccessSecretsManager { get; set; } = false; + + public override ICustomization GetCustomization(ParameterInfo _) => new CurrentContextOrganizationCustomization + { + Id = Id, + Type = Type, + Permissions = Permissions, + AccessSecretsManager = AccessSecretsManager + }; +} diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/AccountRecovery/AdminRecoverAccountCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/AccountRecovery/AdminRecoverAccountCommandTests.cs new file mode 100644 index 0000000000..88025301b6 --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/AccountRecovery/AdminRecoverAccountCommandTests.cs @@ -0,0 +1,296 @@ +using AutoFixture; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.OrganizationFeatures.AccountRecovery; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Platform.Push; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Test.AutoFixture.OrganizationUserFixtures; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Identity; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.AccountRecovery; + +[SutProviderCustomize] +public class AdminRecoverAccountCommandTests +{ + [Theory] + [BitAutoData] + public async Task RecoverAccountAsync_Success( + string newMasterPassword, + string key, + Organization organization, + OrganizationUser organizationUser, + User user, + SutProvider sutProvider) + { + // Arrange + SetupValidOrganization(sutProvider, organization); + SetupValidPolicy(sutProvider, organization); + SetupValidOrganizationUser(organizationUser, organization.Id); + SetupValidUser(sutProvider, user, organizationUser); + SetupSuccessfulPasswordUpdate(sutProvider, user, newMasterPassword); + + // Act + var result = await sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, newMasterPassword, key); + + // Assert + Assert.True(result.Succeeded); + await AssertSuccessAsync(sutProvider, user, key, organization, organizationUser); + } + + [Theory] + [BitAutoData] + public async Task RecoverAccountAsync_OrganizationDoesNotExist_ThrowsBadRequest( + [OrganizationUser] OrganizationUser organizationUser, + string newMasterPassword, + string key, + SutProvider sutProvider) + { + // Arrange + var orgId = Guid.NewGuid(); + sutProvider.GetDependency() + .GetByIdAsync(orgId) + .Returns((Organization)null); + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.RecoverAccountAsync(orgId, organizationUser, newMasterPassword, key)); + Assert.Equal("Organization does not allow password reset.", exception.Message); + } + + [Theory] + [BitAutoData] + public async Task RecoverAccountAsync_OrganizationDoesNotAllowResetPassword_ThrowsBadRequest( + string newMasterPassword, + string key, + Organization organization, + [OrganizationUser] OrganizationUser organizationUser, + SutProvider sutProvider) + { + // Arrange + organization.UseResetPassword = false; + sutProvider.GetDependency() + .GetByIdAsync(organization.Id) + .Returns(organization); + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, newMasterPassword, key)); + Assert.Equal("Organization does not allow password reset.", exception.Message); + } + + public static IEnumerable InvalidPolicies => new object[][] + { + [new Policy { Type = PolicyType.ResetPassword, Enabled = false }], [null] + }; + + [Theory] + [BitMemberAutoData(nameof(InvalidPolicies))] + public async Task RecoverAccountAsync_InvalidPolicy_ThrowsBadRequest( + Policy resetPasswordPolicy, + string newMasterPassword, + string key, + Organization organization, + SutProvider sutProvider) + { + // Arrange + SetupValidOrganization(sutProvider, organization); + sutProvider.GetDependency() + .GetByOrganizationIdTypeAsync(organization.Id, PolicyType.ResetPassword) + .Returns(resetPasswordPolicy); + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.RecoverAccountAsync(organization.Id, new OrganizationUser { Id = Guid.NewGuid() }, + newMasterPassword, key)); + Assert.Equal("Organization does not have the password reset policy enabled.", exception.Message); + } + + public static IEnumerable InvalidOrganizationUsers() + { + // Make an organization so we can use its Id + var organization = new Fixture().Create(); + + var nonConfirmed = new OrganizationUser + { + Id = Guid.NewGuid(), + OrganizationId = organization.Id, + Status = OrganizationUserStatusType.Invited + }; + yield return [nonConfirmed, organization]; + + var wrongOrganization = new OrganizationUser + { + Status = OrganizationUserStatusType.Confirmed, + OrganizationId = Guid.NewGuid(), // Different org + ResetPasswordKey = "test-key", + UserId = Guid.NewGuid(), + }; + yield return [wrongOrganization, organization]; + + var nullResetPasswordKey = new OrganizationUser + { + Status = OrganizationUserStatusType.Confirmed, + OrganizationId = organization.Id, + ResetPasswordKey = null, + UserId = Guid.NewGuid(), + }; + yield return [nullResetPasswordKey, organization]; + + var emptyResetPasswordKey = new OrganizationUser + { + Status = OrganizationUserStatusType.Confirmed, + OrganizationId = organization.Id, + ResetPasswordKey = "", + UserId = Guid.NewGuid(), + }; + yield return [emptyResetPasswordKey, organization]; + + var nullUserId = new OrganizationUser + { + Status = OrganizationUserStatusType.Confirmed, + OrganizationId = organization.Id, + ResetPasswordKey = "test-key", + UserId = null, + }; + yield return [nullUserId, organization]; + } + + [Theory] + [BitMemberAutoData(nameof(InvalidOrganizationUsers))] + public async Task RecoverAccountAsync_OrganizationUserIsInvalid_ThrowsBadRequest( + OrganizationUser organizationUser, + Organization organization, + string newMasterPassword, + string key, + SutProvider sutProvider) + { + // Arrange + SetupValidOrganization(sutProvider, organization); + SetupValidPolicy(sutProvider, organization); + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, newMasterPassword, key)); + Assert.Equal("Organization User not valid", exception.Message); + } + + [Theory] + [BitAutoData] + public async Task RecoverAccountAsync_UserDoesNotExist_ThrowsNotFoundException( + string newMasterPassword, + string key, + Organization organization, + OrganizationUser organizationUser, + SutProvider sutProvider) + { + // Arrange + SetupValidOrganization(sutProvider, organization); + SetupValidPolicy(sutProvider, organization); + SetupValidOrganizationUser(organizationUser, organization.Id); + sutProvider.GetDependency() + .GetUserByIdAsync(organizationUser.UserId!.Value) + .Returns((User)null); + + // Act & Assert + await Assert.ThrowsAsync(() => + sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, newMasterPassword, key)); + } + + [Theory] + [BitAutoData] + public async Task RecoverAccountAsync_UserUsesKeyConnector_ThrowsBadRequest( + string newMasterPassword, + string key, + Organization organization, + OrganizationUser organizationUser, + User user, + SutProvider sutProvider) + { + // Arrange + SetupValidOrganization(sutProvider, organization); + SetupValidPolicy(sutProvider, organization); + SetupValidOrganizationUser(organizationUser, organization.Id); + user.UsesKeyConnector = true; + sutProvider.GetDependency() + .GetUserByIdAsync(organizationUser.UserId!.Value) + .Returns(user); + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, newMasterPassword, key)); + Assert.Equal("Cannot reset password of a user with Key Connector.", exception.Message); + } + + private static void SetupValidOrganization(SutProvider sutProvider, Organization organization) + { + organization.UseResetPassword = true; + sutProvider.GetDependency() + .GetByIdAsync(organization.Id) + .Returns(organization); + } + + private static void SetupValidPolicy(SutProvider sutProvider, Organization organization) + { + var policy = new Policy { Type = PolicyType.ResetPassword, Enabled = true }; + sutProvider.GetDependency() + .GetByOrganizationIdTypeAsync(organization.Id, PolicyType.ResetPassword) + .Returns(policy); + } + + private static void SetupValidOrganizationUser(OrganizationUser organizationUser, Guid orgId) + { + organizationUser.Status = OrganizationUserStatusType.Confirmed; + organizationUser.OrganizationId = orgId; + organizationUser.ResetPasswordKey = "test-key"; + organizationUser.Type = OrganizationUserType.User; + } + + private static void SetupValidUser(SutProvider sutProvider, User user, OrganizationUser organizationUser) + { + user.Id = organizationUser.UserId!.Value; + user.UsesKeyConnector = false; + sutProvider.GetDependency() + .GetUserByIdAsync(user.Id) + .Returns(user); + } + + private static void SetupSuccessfulPasswordUpdate(SutProvider sutProvider, User user, string newMasterPassword) + { + sutProvider.GetDependency() + .UpdatePasswordHash(user, newMasterPassword) + .Returns(IdentityResult.Success); + } + + private static async Task AssertSuccessAsync(SutProvider sutProvider, User user, string key, + Organization organization, OrganizationUser organizationUser) + { + await sutProvider.GetDependency().Received(1).ReplaceAsync( + Arg.Is(u => + u.Id == user.Id && + u.Key == key && + u.ForcePasswordReset == true && + u.RevisionDate == u.AccountRevisionDate && + u.LastPasswordChangeDate == u.RevisionDate)); + + await sutProvider.GetDependency().Received(1).SendAdminResetPasswordEmailAsync( + Arg.Is(user.Email), + Arg.Is(user.Name), + Arg.Is(organization.DisplayName())); + + await sutProvider.GetDependency().Received(1).LogOrganizationUserEventAsync( + Arg.Is(organizationUser), + Arg.Is(EventType.OrganizationUser_AdminResetPassword)); + + await sutProvider.GetDependency().Received(1).PushLogOutAsync( + Arg.Is(user.Id)); + } +}