From 2c70b3a9035a3c9bcd54ac5fb08ea4e0e2725589 Mon Sep 17 00:00:00 2001 From: jrmccannon Date: Mon, 17 Mar 2025 08:57:08 -0500 Subject: [PATCH] Consolidated account recovery into 1 handler. Both use the same method. --- .../OrganizationUsersController.cs | 11 ++- ...untRecoveryDetailsAuthorizationHandler.cs} | 12 +-- ...onUsersAccountRecoveryDetailsOperations.cs | 11 +++ ...OrganizationServiceCollectionExtensions.cs | 4 +- .../OrganizationUsersControllerTests.cs | 17 +++- ...ecoveryDetailsAuthorizationHandlerTests.cs | 87 +++++++++++++++++++ ...asswordDetailsAuthorizationHandlerTests.cs | 51 ----------- 7 files changed, 129 insertions(+), 64 deletions(-) rename src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/{OrganizationUsersResetPasswordDetails/OrganizationUserResetPasswordDetailsAuthorizationHandler.cs => OrganizationUserAccountRecoveryDetails/OrganizationUserAccountRecoveryDetailsAuthorizationHandler.cs} (50%) create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserAccountRecoveryDetails/OrganizationUsersAccountRecoveryDetailsOperations.cs create mode 100644 test/Core.Test/AdminConsole/Authorization/OrganizationUserAccountRecoveryDetailsAuthorizationHandlerTests.cs delete mode 100644 test/Core.Test/AdminConsole/Authorization/OrganizationUserResetPasswordDetailsAuthorizationHandlerTests.cs diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index d8485e78b5..5b17932e93 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -7,9 +7,9 @@ using Bit.Core; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization.OrganizationUserAccountRecoveryDetails; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization.OrganizationUserDetails; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization.OrganizationUserGroups; -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization.OrganizationUsersResetPasswordDetails; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.Shared.Authorization; using Bit.Core.AdminConsole.Repositories; @@ -214,7 +214,7 @@ public class OrganizationUsersController : Controller { var authResult = await _authorizationService.AuthorizeAsync(User, new OrganizationScope(orgId), - [OrganizationUsersResetPasswordDetailsOperations.Read]); + [OrganizationUsersAccountRecoveryDetailsOperations.Read]); if (authResult.Succeeded is false) { @@ -248,8 +248,11 @@ public class OrganizationUsersController : Controller [HttpPost("account-recovery-details")] public async Task> GetAccountRecoveryDetails(Guid orgId, [FromBody] OrganizationUserBulkRequestModel model) { - // Make sure the calling user can reset passwords for this org - if (!await _currentContext.ManageResetPassword(orgId)) + var authResult = await _authorizationService.AuthorizeAsync(User, + new OrganizationScope(orgId), + [OrganizationUsersAccountRecoveryDetailsOperations.ReadAll]); + + if (authResult.Succeeded is false) { throw new NotFoundException(); } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUsersResetPasswordDetails/OrganizationUserResetPasswordDetailsAuthorizationHandler.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserAccountRecoveryDetails/OrganizationUserAccountRecoveryDetailsAuthorizationHandler.cs similarity index 50% rename from src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUsersResetPasswordDetails/OrganizationUserResetPasswordDetailsAuthorizationHandler.cs rename to src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserAccountRecoveryDetails/OrganizationUserAccountRecoveryDetailsAuthorizationHandler.cs index 54bdfc980b..cc5dde3a30 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUsersResetPasswordDetails/OrganizationUserResetPasswordDetailsAuthorizationHandler.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserAccountRecoveryDetails/OrganizationUserAccountRecoveryDetailsAuthorizationHandler.cs @@ -2,17 +2,19 @@ using Bit.Core.Context; using Microsoft.AspNetCore.Authorization; -namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization.OrganizationUsersResetPasswordDetails; +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization. + OrganizationUserAccountRecoveryDetails; -public class OrganizationUserResetPasswordDetailsAuthorizationHandler(ICurrentContext currentContext) - : AuthorizationHandler +public class OrganizationUserAccountRecoveryDetailsAuthorizationHandler(ICurrentContext currentContext) + : AuthorizationHandler { protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, - OrganizationUsersResetPasswordDetailsOperationRequirement requirement, OrganizationScope resource) + OrganizationUsersAccountRecoveryDetailsOperationRequirement requirement, OrganizationScope resource) { var authorized = requirement switch { - not null when requirement.Name == nameof(OrganizationUsersResetPasswordDetailsOperations.Read) => + not null when requirement.Name is nameof(OrganizationUsersAccountRecoveryDetailsOperations.Read) or + nameof(OrganizationUsersAccountRecoveryDetailsOperations.ReadAll) => await currentContext.ManageResetPassword(resource), _ => false }; diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserAccountRecoveryDetails/OrganizationUsersAccountRecoveryDetailsOperations.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserAccountRecoveryDetails/OrganizationUsersAccountRecoveryDetailsOperations.cs new file mode 100644 index 0000000000..0744577ba4 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserAccountRecoveryDetails/OrganizationUsersAccountRecoveryDetailsOperations.cs @@ -0,0 +1,11 @@ +using Microsoft.AspNetCore.Authorization.Infrastructure; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization.OrganizationUserAccountRecoveryDetails; + +public class OrganizationUsersAccountRecoveryDetailsOperationRequirement : OperationAuthorizationRequirement; + +public static class OrganizationUsersAccountRecoveryDetailsOperations +{ + public static readonly OrganizationUsersAccountRecoveryDetailsOperationRequirement Read = new() { Name = nameof(Read) }; + public static readonly OrganizationUsersAccountRecoveryDetailsOperationRequirement ReadAll = new() { Name = nameof(ReadAll) }; +} diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index e1449b1a96..b2cf5d1850 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -12,9 +12,9 @@ using Bit.Core.AdminConsole.OrganizationFeatures.Organizations; using Bit.Core.AdminConsole.OrganizationFeatures.Organizations.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization.OrganizationUserAccountRecoveryDetails; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization.OrganizationUserDetails; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization.OrganizationUserGroups; -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization.OrganizationUsersResetPasswordDetails; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Models.Business.Tokenables; using Bit.Core.OrganizationFeatures.OrganizationCollections; @@ -174,7 +174,7 @@ public static class OrganizationServiceCollectionExtensions services.AddScoped(); services.AddScoped(); services.AddScoped(); - services.AddScoped(); + services.AddScoped(); services.AddScoped(); } diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs index b8498da91e..7d6df6ee21 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs @@ -6,6 +6,7 @@ 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.OrganizationUsers.Authorization.OrganizationUserAccountRecoveryDetails; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization.OrganizationUserDetails; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.Shared.Authorization; @@ -294,7 +295,13 @@ public class OrganizationUsersControllerTests ICollection resetPasswordDetails, SutProvider sutProvider) { - sutProvider.GetDependency().ManageResetPassword(organizationId).Returns(true); + sutProvider.GetDependency() + .AuthorizeAsync( + user: Arg.Any(), + resource: Arg.Is(x => x == organizationId), + requirements: Arg.Is>(x => + x.Any(y => y == OrganizationUsersAccountRecoveryDetailsOperations.ReadAll))) + .Returns(AuthorizationResult.Success()); sutProvider.GetDependency() .GetManyAccountRecoveryDetailsByOrganizationUserAsync(organizationId, bulkRequestModel.Ids) .Returns(resetPasswordDetails); @@ -320,7 +327,13 @@ public class OrganizationUsersControllerTests OrganizationUserBulkRequestModel bulkRequestModel, SutProvider sutProvider) { - sutProvider.GetDependency().ManageResetPassword(organizationId).Returns(false); + sutProvider.GetDependency() + .AuthorizeAsync( + user: Arg.Any(), + resource: Arg.Is(x => x == organizationId), + requirements: Arg.Is>(x => + x.Any(y => y == OrganizationUsersAccountRecoveryDetailsOperations.ReadAll))) + .Returns(AuthorizationResult.Failed()); await Assert.ThrowsAsync(async () => await sutProvider.Sut.GetAccountRecoveryDetails(organizationId, bulkRequestModel)); } diff --git a/test/Core.Test/AdminConsole/Authorization/OrganizationUserAccountRecoveryDetailsAuthorizationHandlerTests.cs b/test/Core.Test/AdminConsole/Authorization/OrganizationUserAccountRecoveryDetailsAuthorizationHandlerTests.cs new file mode 100644 index 0000000000..b2ec575034 --- /dev/null +++ b/test/Core.Test/AdminConsole/Authorization/OrganizationUserAccountRecoveryDetailsAuthorizationHandlerTests.cs @@ -0,0 +1,87 @@ +using System.Security.Claims; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization.OrganizationUserAccountRecoveryDetails; +using Bit.Core.AdminConsole.OrganizationFeatures.Shared.Authorization; +using Bit.Core.Context; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Authorization; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.Authorization; + +[SutProviderCustomize] +public class OrganizationUserAccountRecoveryDetailsAuthorizationHandlerTests +{ + [Theory] + [BitAutoData] + public async Task ReadAll_UserCanManageResetPassword_ShouldReturnSuccess( + CurrentContextOrganization contextOrganization, + SutProvider sutProvider) + { + sutProvider.GetDependency().ManageResetPassword(contextOrganization.Id).Returns(true); + + var context = new AuthorizationHandlerContext( + [OrganizationUsersAccountRecoveryDetailsOperations.ReadAll], + new ClaimsPrincipal(), + new OrganizationScope(contextOrganization.Id)); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task ReadAll_UserCannotManageResetPassword_ShouldReturnFailure( + CurrentContextOrganization contextOrganization, + SutProvider sutProvider) + { + sutProvider.GetDependency().ManageResetPassword(contextOrganization.Id).Returns(false); + + var context = new AuthorizationHandlerContext( + [OrganizationUsersAccountRecoveryDetailsOperations.ReadAll], + new ClaimsPrincipal(), + new OrganizationScope(contextOrganization.Id)); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasFailed); + } + + [Theory] + [BitAutoData] + public async Task Read_UserCanManageResetPassword_ShouldReturnSuccess( + CurrentContextOrganization contextOrganization, + SutProvider sutProvider) + { + sutProvider.GetDependency().ManageResetPassword(contextOrganization.Id).Returns(true); + + var context = new AuthorizationHandlerContext( + [OrganizationUsersAccountRecoveryDetailsOperations.Read], + new ClaimsPrincipal(), + new OrganizationScope(contextOrganization.Id)); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task Read_UserCannotManageResetPassword_ShouldReturnFailure( + CurrentContextOrganization contextOrganization, + SutProvider sutProvider) + { + sutProvider.GetDependency().ManageResetPassword(contextOrganization.Id).Returns(false); + + var context = new AuthorizationHandlerContext( + [OrganizationUsersAccountRecoveryDetailsOperations.Read], + new ClaimsPrincipal(), + new OrganizationScope(contextOrganization.Id)); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasFailed); + } +} diff --git a/test/Core.Test/AdminConsole/Authorization/OrganizationUserResetPasswordDetailsAuthorizationHandlerTests.cs b/test/Core.Test/AdminConsole/Authorization/OrganizationUserResetPasswordDetailsAuthorizationHandlerTests.cs deleted file mode 100644 index 03c60dfea6..0000000000 --- a/test/Core.Test/AdminConsole/Authorization/OrganizationUserResetPasswordDetailsAuthorizationHandlerTests.cs +++ /dev/null @@ -1,51 +0,0 @@ -using System.Security.Claims; -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization.OrganizationUsersResetPasswordDetails; -using Bit.Core.AdminConsole.OrganizationFeatures.Shared.Authorization; -using Bit.Core.Context; -using Bit.Test.Common.AutoFixture; -using Bit.Test.Common.AutoFixture.Attributes; -using Microsoft.AspNetCore.Authorization; -using NSubstitute; -using Xunit; - -namespace Bit.Core.Test.AdminConsole.Authorization; - -[SutProviderCustomize] -public class OrganizationUserResetPasswordDetailsAuthorizationHandlerTests -{ - [Theory] - [BitAutoData] - public async Task Read_UserCanManageResetPassword_ShouldReturnSuccess( - CurrentContextOrganization contextOrganization, - SutProvider sutProvider) - { - sutProvider.GetDependency().ManageResetPassword(contextOrganization.Id).Returns(true); - - var context = new AuthorizationHandlerContext( - [OrganizationUsersResetPasswordDetailsOperations.Read], - new ClaimsPrincipal(), - new OrganizationScope(contextOrganization.Id)); - - await sutProvider.Sut.HandleAsync(context); - - Assert.True(context.HasSucceeded); - } - - [Theory] - [BitAutoData] - public async Task Read_UserCannotManageResetPassword_ShouldReturnFailure( - CurrentContextOrganization contextOrganization, - SutProvider sutProvider) - { - sutProvider.GetDependency().ManageResetPassword(contextOrganization.Id).Returns(false); - - var context = new AuthorizationHandlerContext( - [OrganizationUsersResetPasswordDetailsOperations.Read], - new ClaimsPrincipal(), - new OrganizationScope(contextOrganization.Id)); - - await sutProvider.Sut.HandleAsync(context); - - Assert.True(context.HasFailed); - } -}