From e2d2a2ba90ea8716e14dc5d30e452acb0c1d88bd Mon Sep 17 00:00:00 2001 From: Addison Beck Date: Mon, 1 Jul 2024 11:52:58 -0400 Subject: [PATCH] Add a master password hash check to account recovery enrollment (#4154) --- .../OrganizationUsersController.cs | 5 ++++ .../OrganizationUserRequestModels.cs | 3 +- .../OrganizationUsersControllerTests.cs | 29 +++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index 0b93839d2..cc5b76a24 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -456,6 +456,11 @@ public class OrganizationUsersController : Controller throw new UnauthorizedAccessException(); } + if (!string.IsNullOrWhiteSpace(model.ResetPasswordKey) && !await _userService.VerifySecretAsync(user, model.Secret)) + { + throw new BadRequestException("Incorrect password"); + } + var callingUserId = user.Id; await _organizationService.UpdateUserResetPasswordEnrollmentAsync( orgId, userId, model.ResetPasswordKey, callingUserId); diff --git a/src/Api/AdminConsole/Models/Request/Organizations/OrganizationUserRequestModels.cs b/src/Api/AdminConsole/Models/Request/Organizations/OrganizationUserRequestModels.cs index 0313d8579..44e9853bb 100644 --- a/src/Api/AdminConsole/Models/Request/Organizations/OrganizationUserRequestModels.cs +++ b/src/Api/AdminConsole/Models/Request/Organizations/OrganizationUserRequestModels.cs @@ -1,4 +1,5 @@ using System.ComponentModel.DataAnnotations; +using Bit.Api.Auth.Models.Request.Accounts; using Bit.Api.Models.Request; using Bit.Core.Entities; using Bit.Core.Enums; @@ -98,7 +99,7 @@ public class OrganizationUserUpdateRequestModel } } -public class OrganizationUserResetPasswordEnrollmentRequestModel +public class OrganizationUserResetPasswordEnrollmentRequestModel : SecretVerificationRequestModel { public string ResetPasswordKey { get; set; } } diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs index 5fef2885e..80c458d69 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs @@ -40,6 +40,7 @@ public class OrganizationUsersControllerTests { orgUser.Status = Core.Enums.OrganizationUserStatusType.Invited; sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user); + sutProvider.GetDependency().VerifySecretAsync(default, default).ReturnsForAnyArgs(true); sutProvider.GetDependency().GetByOrganizationAsync(default, default).ReturnsForAnyArgs(orgUser); await sutProvider.Sut.PutResetPasswordEnrollment(orgId, userId, model); @@ -54,6 +55,7 @@ public class OrganizationUsersControllerTests { orgUser.Status = Core.Enums.OrganizationUserStatusType.Confirmed; sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user); + sutProvider.GetDependency().VerifySecretAsync(default, default).ReturnsForAnyArgs(true); sutProvider.GetDependency().GetByOrganizationAsync(default, default).ReturnsForAnyArgs(orgUser); await sutProvider.Sut.PutResetPasswordEnrollment(orgId, userId, model); @@ -61,6 +63,33 @@ public class OrganizationUsersControllerTests await sutProvider.GetDependency().Received(0).AcceptOrgUserByOrgIdAsync(orgId, user, sutProvider.GetDependency()); } + [Theory] + [BitAutoData] + public async Task PutResetPasswordEnrollment_PasswordValidationFails_Throws(Guid orgId, Guid userId, OrganizationUserResetPasswordEnrollmentRequestModel model, + User user, SutProvider sutProvider) + { + model.MasterPasswordHash = "NotThePassword"; + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user); + await Assert.ThrowsAsync(async () => await sutProvider.Sut.PutResetPasswordEnrollment(orgId, userId, model)); + } + + [Theory] + [BitAutoData] + public async Task PutResetPasswordEnrollment_PasswordValidationPasses_Continues(Guid orgId, Guid userId, OrganizationUserResetPasswordEnrollmentRequestModel model, + User user, OrganizationUser orgUser, SutProvider sutProvider) + { + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user); + sutProvider.GetDependency().VerifySecretAsync(user, model.Secret).Returns(true); + sutProvider.GetDependency().GetByOrganizationAsync(default, default).ReturnsForAnyArgs(orgUser); + await sutProvider.Sut.PutResetPasswordEnrollment(orgId, userId, model); + await sutProvider.GetDependency().Received(1).UpdateUserResetPasswordEnrollmentAsync( + orgId, + userId, + model.ResetPasswordKey, + user.Id + ); + } + [Theory] [BitAutoData] public async Task Accept_RequiresKnownUser(Guid orgId, Guid orgUserId, OrganizationUserAcceptRequestModel model,