1
0
mirror of https://github.com/bitwarden/server synced 2026-01-07 11:03:37 +00:00

[PM-24192] Move account recovery logic to command (#6184)

* Move account recovery logic to command
  (temporarily duplicated behind feature flag)

* Move permission checks to authorization handler

* Prevent user from recovering provider member account
  unless they are also provider member
This commit is contained in:
Thomas Rittson
2025-11-01 07:55:25 +10:00
committed by GitHub
parent 09564947e8
commit e11458196c
16 changed files with 1261 additions and 19 deletions

View File

@@ -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<RecoverAccountAuthorizationHandler> 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<RecoverAccountAuthorizationHandler> 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<object[]> 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<RecoverAccountAuthorizationHandler> 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<object[]> 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<RecoverAccountAuthorizationHandler> 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<RecoverAccountAuthorizationHandler> 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<IProviderUserRepository>().DidNotReceiveWithAnyArgs()
.GetManyByUserAsync(Arg.Any<Guid>());
}
[Theory, BitAutoData]
public async Task HandleRequirementAsync_CurrentUserIsMemberOfAllTargetUserProviders_DoesNotBlock(
SutProvider<RecoverAccountAuthorizationHandler> sutProvider,
[OrganizationUser] OrganizationUser targetOrganizationUser,
ClaimsPrincipal claimsPrincipal,
Guid providerId1,
Guid providerId2)
{
// Arrange
var targetUserProviders = new List<ProviderUser>
{
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<IProviderUserRepository>()
.GetManyByUserAsync(targetOrganizationUser.UserId!.Value)
.Returns(targetUserProviders);
sutProvider.GetDependency<ICurrentContext>()
.ProviderUser(providerId1)
.Returns(true);
sutProvider.GetDependency<ICurrentContext>()
.ProviderUser(providerId2)
.Returns(true);
// Act
await sutProvider.Sut.HandleAsync(context);
// Assert
Assert.True(context.HasSucceeded);
}
[Theory, BitAutoData]
public async Task HandleRequirementAsync_CurrentUserMissingProviderMembership_Blocks(
SutProvider<RecoverAccountAuthorizationHandler> sutProvider,
[OrganizationUser] OrganizationUser targetOrganizationUser,
ClaimsPrincipal claimsPrincipal,
Guid providerId1,
Guid providerId2)
{
// Arrange
var targetUserProviders = new List<ProviderUser>
{
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<IProviderUserRepository>()
.GetManyByUserAsync(targetOrganizationUser.UserId!.Value)
.Returns(targetUserProviders);
sutProvider.GetDependency<ICurrentContext>()
.ProviderUser(providerId1)
.Returns(true);
// Not a member of this provider
sutProvider.GetDependency<ICurrentContext>()
.ProviderUser(providerId2)
.Returns(false);
// Act
await sutProvider.Sut.HandleAsync(context);
// Assert
AssertFailed(context, RecoverAccountAuthorizationHandler.ProviderFailureReason);
}
private static void MockOrganizationClaims(SutProvider<RecoverAccountAuthorizationHandler> sutProvider,
ClaimsPrincipal currentUser, OrganizationUser targetOrganizationUser,
CurrentContextOrganization? currentContextOrganization)
{
sutProvider.GetDependency<IOrganizationContext>()
.GetOrganizationClaims(currentUser, targetOrganizationUser.OrganizationId)
.Returns(currentContextOrganization);
}
private static void MockCurrentUserIsProvider(SutProvider<RecoverAccountAuthorizationHandler> sutProvider,
ClaimsPrincipal currentUser, OrganizationUser targetOrganizationUser)
{
sutProvider.GetDependency<IOrganizationContext>()
.IsProviderUserForOrganization(currentUser, targetOrganizationUser.OrganizationId)
.Returns(true);
}
private static void MockCurrentUserIsOwner(SutProvider<RecoverAccountAuthorizationHandler> sutProvider,
ClaimsPrincipal currentUser, OrganizationUser targetOrganizationUser)
{
var currentContextOrganization = new CurrentContextOrganization
{
Id = targetOrganizationUser.OrganizationId,
Type = OrganizationUserType.Owner
};
sutProvider.GetDependency<IOrganizationContext>()
.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);
}
}

View File

@@ -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<OrganizationUsersController> sutProvider)
{
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(false);
sutProvider.GetDependency<ICurrentContext>().OrganizationOwner(orgId).Returns(true);
sutProvider.GetDependency<IUserService>().AdminResetPasswordAsync(Arg.Any<OrganizationUserType>(), orgId, orgUserId, model.NewMasterPasswordHash, model.Key)
.Returns(Microsoft.AspNetCore.Identity.IdentityResult.Success);
var result = await sutProvider.Sut.PutResetPassword(orgId, orgUserId, model);
Assert.IsType<Ok>(result);
await sutProvider.GetDependency<IUserService>().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<OrganizationUsersController> sutProvider)
{
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(false);
sutProvider.GetDependency<ICurrentContext>().OrganizationOwner(orgId).Returns(false);
sutProvider.GetDependency<ICurrentContext>().Organizations.Returns(new List<CurrentContextOrganization>());
var result = await sutProvider.Sut.PutResetPassword(orgId, orgUserId, model);
Assert.IsType<NotFound>(result);
}
[Theory]
[BitAutoData]
public async Task PutResetPassword_WithFeatureFlagDisabled_WhenAdminResetPasswordFails_ReturnsBadRequest(
Guid orgId, Guid orgUserId, OrganizationUserResetPasswordRequestModel model,
SutProvider<OrganizationUsersController> sutProvider)
{
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(false);
sutProvider.GetDependency<ICurrentContext>().OrganizationOwner(orgId).Returns(true);
sutProvider.GetDependency<IUserService>().AdminResetPasswordAsync(Arg.Any<OrganizationUserType>(), 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<BadRequest<ModelStateDictionary>>(result);
}
[Theory]
[BitAutoData]
public async Task PutResetPassword_WithFeatureFlagEnabled_WhenOrganizationUserNotFound_ReturnsNotFound(
Guid orgId, Guid orgUserId, OrganizationUserResetPasswordRequestModel model,
SutProvider<OrganizationUsersController> sutProvider)
{
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(true);
sutProvider.GetDependency<IOrganizationUserRepository>().GetByIdAsync(orgUserId).Returns((OrganizationUser)null);
var result = await sutProvider.Sut.PutResetPassword(orgId, orgUserId, model);
Assert.IsType<NotFound>(result);
}
[Theory]
[BitAutoData]
public async Task PutResetPassword_WithFeatureFlagEnabled_WhenOrganizationIdMismatch_ReturnsNotFound(
Guid orgId, Guid orgUserId, OrganizationUserResetPasswordRequestModel model, OrganizationUser organizationUser,
SutProvider<OrganizationUsersController> sutProvider)
{
organizationUser.OrganizationId = Guid.NewGuid();
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(true);
sutProvider.GetDependency<IOrganizationUserRepository>().GetByIdAsync(orgUserId).Returns(organizationUser);
var result = await sutProvider.Sut.PutResetPassword(orgId, orgUserId, model);
Assert.IsType<NotFound>(result);
}
[Theory]
[BitAutoData]
public async Task PutResetPassword_WithFeatureFlagEnabled_WhenAuthorizationFails_ReturnsBadRequest(
Guid orgId, Guid orgUserId, OrganizationUserResetPasswordRequestModel model, OrganizationUser organizationUser,
SutProvider<OrganizationUsersController> sutProvider)
{
organizationUser.OrganizationId = orgId;
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(true);
sutProvider.GetDependency<IOrganizationUserRepository>().GetByIdAsync(orgUserId).Returns(organizationUser);
sutProvider.GetDependency<IAuthorizationService>()
.AuthorizeAsync(
Arg.Any<ClaimsPrincipal>(),
organizationUser,
Arg.Is<IEnumerable<IAuthorizationRequirement>>(x => x.SingleOrDefault() is RecoverAccountAuthorizationRequirement))
.Returns(AuthorizationResult.Failed());
var result = await sutProvider.Sut.PutResetPassword(orgId, orgUserId, model);
Assert.IsType<BadRequest<ErrorResponseModel>>(result);
}
[Theory]
[BitAutoData]
public async Task PutResetPassword_WithFeatureFlagEnabled_WhenRecoverAccountSucceeds_ReturnsOk(
Guid orgId, Guid orgUserId, OrganizationUserResetPasswordRequestModel model, OrganizationUser organizationUser,
SutProvider<OrganizationUsersController> sutProvider)
{
organizationUser.OrganizationId = orgId;
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(true);
sutProvider.GetDependency<IOrganizationUserRepository>().GetByIdAsync(orgUserId).Returns(organizationUser);
sutProvider.GetDependency<IAuthorizationService>()
.AuthorizeAsync(
Arg.Any<ClaimsPrincipal>(),
organizationUser,
Arg.Is<IEnumerable<IAuthorizationRequirement>>(x => x.SingleOrDefault() is RecoverAccountAuthorizationRequirement))
.Returns(AuthorizationResult.Success());
sutProvider.GetDependency<IAdminRecoverAccountCommand>()
.RecoverAccountAsync(orgId, organizationUser, model.NewMasterPasswordHash, model.Key)
.Returns(Microsoft.AspNetCore.Identity.IdentityResult.Success);
var result = await sutProvider.Sut.PutResetPassword(orgId, orgUserId, model);
Assert.IsType<Ok>(result);
await sutProvider.GetDependency<IAdminRecoverAccountCommand>().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<OrganizationUsersController> sutProvider)
{
organizationUser.OrganizationId = orgId;
sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.AccountRecoveryCommand).Returns(true);
sutProvider.GetDependency<IOrganizationUserRepository>().GetByIdAsync(orgUserId).Returns(organizationUser);
sutProvider.GetDependency<IAuthorizationService>()
.AuthorizeAsync(
Arg.Any<ClaimsPrincipal>(),
organizationUser,
Arg.Is<IEnumerable<IAuthorizationRequirement>>(x => x.SingleOrDefault() is RecoverAccountAuthorizationRequirement))
.Returns(AuthorizationResult.Success());
sutProvider.GetDependency<IAdminRecoverAccountCommand>()
.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<BadRequest<ModelStateDictionary>>(result);
}
}