[PM-40300]: Restrict Provider Users' Account Recovery

src/Api/AdminConsole/Authorization/Organizations/Handlers/RecoverAccountAuthorizationHandler.cs

@@ -1,4 +1,6 @@
 using System.Security.Claims;
+using Bit.Core.AdminConsole.Entities.Provider;
+using Bit.Core.AdminConsole.Enums.Provider;
 using Bit.Core.AdminConsole.Repositories;
 using Bit.Core.Context;
 using Bit.Core.Entities;
@@ -21,7 +23,7 @@ public class RecoverAccountAuthorizationRequirement : IAuthorizationRequirement;
 /// </summary>
 /// <remarks>
 /// This prevents privilege escalation by ensuring that a user cannot recover the account of
-/// another user with a higher role or with provider membership.
+/// another user with a higher role, in either the organization or a provider.
 /// </remarks>
 public class RecoverAccountAuthorizationHandler(
     IOrganizationContext organizationContext,
@@ -30,7 +32,7 @@ public class RecoverAccountAuthorizationHandler(
     : AuthorizationHandler<RecoverAccountAuthorizationRequirement, OrganizationUser>
 {
     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.";
+    public const string ProviderFailureReason = "You are not permitted to recover this Provider member's account.";
 
     protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context,
         RecoverAccountAuthorizationRequirement requirement,
@@ -49,7 +51,8 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext
         }
 
         // 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.
+        // This prevents an organization admin performing privilege escalation into an unrelated provider,
+        // and prevents a provider member escalating into a higher role within their own provider.
         var canRecoverProviderMember = await CanRecoverProviderAsync(targetOrganizationUser);
         if (!canRecoverProviderMember)
         {
@@ -99,11 +102,24 @@ private async Task<bool> CanRecoverProviderAsync(OrganizationUser targetOrganiza
         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.
+        // The current user must be able to recover the target user in every provider the target belongs to.
         // 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));
+        var authorized = targetUserProviderUsers.All(AuthorizeProviderMember);
+        return authorized;
+    }
+
+    private bool AuthorizeProviderMember(ProviderUser targetProviderUser)
+    {
+        // Current user must be a member of the provider with equal or greater permissions than the user
+        // account being recovered. Membership alone is not enough: a Service User recovering a Provider admin
+        // would escalate into the higher role.
+        var authorized = targetProviderUser.Type switch
+        {
+            ProviderUserType.ProviderAdmin => currentContext.ProviderProviderAdmin(targetProviderUser.ProviderId),
+            _ => currentContext.ProviderUser(targetProviderUser.ProviderId)
+        };
+
         return authorized;
     }
 }

src/Api/AdminConsole/Controllers/OrganizationUsersController.cs

@@ -261,6 +261,11 @@ public async Task<OrganizationUserResetPasswordDetailsResponseModel> GetResetPas
             throw new NotFoundException();
         }
 
+        if (!await CanRecoverAccountAsync(organizationUser))
+        {
+            throw new NotFoundException();
+        }
+
         // Retrieve data necessary for response (KDF, KDF Iterations, ResetPasswordKey)
         // TODO Reset Password - Revisit this and create SPROC to reduce DB calls
         var user = await _userService.GetUserByIdAsync(organizationUser.UserId.Value);
@@ -276,10 +281,29 @@ public async Task<OrganizationUserResetPasswordDetailsResponseModel> GetResetPas
     [Authorize<ManageAccountRecoveryRequirement>]
     public async Task<ListResponseModel<OrganizationUserResetPasswordDetailsResponseModel>> GetAccountRecoveryDetails(Guid orgId, [FromBody] OrganizationUserBulkRequestModel model)
     {
-        var responses = await _organizationUserRepository.GetManyAccountRecoveryDetailsByOrganizationUserAsync(orgId, model.Ids);
+        var organizationUsers = await _organizationUserRepository.GetManyAsync(model.Ids);
+        var authorizedIds = new List<Guid>();
+        foreach (var organizationUser in organizationUsers.Where(organizationUser => organizationUser.OrganizationId == orgId))
+        {
+            if (await CanRecoverAccountAsync(organizationUser))
+            {
+                authorizedIds.Add(organizationUser.Id);
+            }
+        }
+
+        if (authorizedIds.Count == 0)
+        {
+            return new ListResponseModel<OrganizationUserResetPasswordDetailsResponseModel>([]);
+        }
+
+        var responses = await _organizationUserRepository.GetManyAccountRecoveryDetailsByOrganizationUserAsync(orgId, authorizedIds);
         return new ListResponseModel<OrganizationUserResetPasswordDetailsResponseModel>(responses.Select(r => new OrganizationUserResetPasswordDetailsResponseModel(r)));
     }
 
+    private async Task<bool> CanRecoverAccountAsync(OrganizationUser organizationUser) =>
+        (await _authorizationService.AuthorizeAsync(
+            User, organizationUser, new RecoverAccountAuthorizationRequirement())).Succeeded;
+
     [HttpPost("invite")]
     [Authorize<ManageUsersRequirement>]
     public async Task Invite(Guid orgId, [FromBody] OrganizationUserInviteRequestModel model)

test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationUsersControllerRecoverAccountTests.cs

@@ -1,6 +1,7 @@
 using System.Net;
 using Bit.Api.AdminConsole.Authorization;
 using Bit.Api.AdminConsole.Models.Request.Organizations;
+using Bit.Api.AdminConsole.Models.Response.Organizations;
 using Bit.Api.IntegrationTest.Factories;
 using Bit.Api.IntegrationTest.Helpers;
 using Bit.Core.AdminConsole.Entities;
@@ -70,6 +71,22 @@ public Task DisposeAsync()
         return Task.CompletedTask;
     }
 
+    /// <summary>
+    /// Minimal shape for reading the account recovery details response.
+    /// <see cref="OrganizationUserResetPasswordDetailsResponseModel"/> is response-only and has no
+    /// constructor that System.Text.Json can bind to.
+    /// </summary>
+    private sealed class AccountRecoveryDetails
+    {
+        public Guid OrganizationUserId { get; set; }
+        public string? ResetPasswordKey { get; set; }
+    }
+
+    private sealed class AccountRecoveryDetailsList
+    {
+        public List<AccountRecoveryDetails> Data { get; set; } = [];
+    }
+
     /// <summary>
     /// Helper method to set the ResetPasswordKey on an organization user, which is required for account recovery
     /// </summary>
@@ -268,4 +285,190 @@ await providerUserRepository.CreateAsync(new ProviderUser
         var model = await response.Content.ReadFromJsonAsync<ErrorResponseModel>();
         Assert.Equal(RecoverAccountAuthorizationHandler.ProviderFailureReason, model.Message);
     }
+
+    [Fact]
+    public async Task RecoverAccount_AsFellowProviderMember_CannotRecoverProviderAdmin()
+    {
+        // Arrange - the caller and the target belong to the same provider. Shared membership is not enough
+        // on its own: the caller must also hold an equal or greater role, otherwise a Service User could
+        // take over a Provider admin.
+        var provider = await ProviderTestHelpers.CreateProviderAndLinkToOrganizationAsync(
+            _factory, _organization.Id, ProviderType.Msp);
+
+        var (callerEmail, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory,
+            _organization.Id, OrganizationUserType.Owner);
+        await ProviderTestHelpers.CreateProviderUserAsync(_factory, provider.Id, callerEmail,
+            ProviderUserType.ServiceUser);
+
+        var (targetEmail, targetOrgUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(
+            _factory, _organization.Id, OrganizationUserType.User);
+        await ProviderTestHelpers.CreateProviderUserAsync(_factory, provider.Id, targetEmail,
+            ProviderUserType.ProviderAdmin);
+        await SetResetPasswordKeyAsync(targetOrgUser);
+
+        // Log in only once the provider membership exists, so that it is present in the caller's claims
+        await _loginHelper.LoginAsync(callerEmail);
+
+        var resetPasswordRequest = new OrganizationUserResetPasswordRequestModel
+        {
+            ResetMasterPassword = true,
+            NewMasterPasswordHash = "new-master-password-hash",
+            Key = "encrypted-recovery-key"
+        };
+
+        // Act
+        var response = await _client.PutAsJsonAsync(
+            $"organizations/{_organization.Id}/users/{targetOrgUser.Id}/recover-account",
+            resetPasswordRequest);
+
+        // Assert
+        Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
+        var model = await response.Content.ReadFromJsonAsync<ErrorResponseModel>();
+        Assert.Equal(RecoverAccountAuthorizationHandler.ProviderFailureReason, model.Message);
+    }
+
+    [Fact]
+    public async Task GetResetPasswordDetails_AsLowerRole_DoesNotDiscloseHigherRoleKeyMaterial()
+    {
+        // 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);
+
+        // Act
+        var response = await _client.GetAsync(
+            $"organizations/{_organization.Id}/users/{targetOwnerOrgUser.Id}/reset-password-details");
+
+        // Assert
+        Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
+    }
+
+    [Fact]
+    public async Task GetResetPasswordDetails_ForProviderMemberOutsideCallersProviders_DoesNotDiscloseKeyMaterial()
+    {
+        // Arrange - the caller is an organization Owner with no membership of the target's provider
+        var (ownerEmail, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory,
+            _organization.Id, OrganizationUserType.Owner);
+        await _loginHelper.LoginAsync(ownerEmail);
+
+        var (targetEmail, targetOrgUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(
+            _factory, _organization.Id, OrganizationUserType.User);
+        await SetResetPasswordKeyAsync(targetOrgUser);
+
+        var provider = await ProviderTestHelpers.CreateProviderAndLinkToOrganizationAsync(
+            _factory, _organization.Id, ProviderType.Msp);
+        await ProviderTestHelpers.CreateProviderUserAsync(_factory, provider.Id, targetEmail,
+            ProviderUserType.ProviderAdmin);
+
+        // Act
+        var response = await _client.GetAsync(
+            $"organizations/{_organization.Id}/users/{targetOrgUser.Id}/reset-password-details");
+
+        // Assert
+        Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
+    }
+
+    [Fact]
+    public async Task GetResetPasswordDetails_AsFellowProviderMember_DoesNotDiscloseProviderAdminKeyMaterial()
+    {
+        // Arrange - a Service User must not be able to read the key material of a Provider admin in their
+        // own provider. Reading it is equivalent to the recovery itself, so it is denied on the same terms.
+        var provider = await ProviderTestHelpers.CreateProviderAndLinkToOrganizationAsync(
+            _factory, _organization.Id, ProviderType.Msp);
+
+        var (callerEmail, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory,
+            _organization.Id, OrganizationUserType.Owner);
+        await ProviderTestHelpers.CreateProviderUserAsync(_factory, provider.Id, callerEmail,
+            ProviderUserType.ServiceUser);
+
+        var (targetEmail, targetOrgUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(
+            _factory, _organization.Id, OrganizationUserType.User);
+        await ProviderTestHelpers.CreateProviderUserAsync(_factory, provider.Id, targetEmail,
+            ProviderUserType.ProviderAdmin);
+        await SetResetPasswordKeyAsync(targetOrgUser);
+
+        await _loginHelper.LoginAsync(callerEmail);
+
+        // Act
+        var response = await _client.GetAsync(
+            $"organizations/{_organization.Id}/users/{targetOrgUser.Id}/reset-password-details");
+
+        // Assert
+        Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
+    }
+
+    [Fact]
+    public async Task GetAccountRecoveryDetails_AsFellowProviderMember_OmitsProviderAdmin()
+    {
+        // Arrange
+        var provider = await ProviderTestHelpers.CreateProviderAndLinkToOrganizationAsync(
+            _factory, _organization.Id, ProviderType.Msp);
+
+        var (callerEmail, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory,
+            _organization.Id, OrganizationUserType.Owner);
+        await ProviderTestHelpers.CreateProviderUserAsync(_factory, provider.Id, callerEmail,
+            ProviderUserType.ServiceUser);
+
+        var (providerAdminEmail, providerAdminOrgUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(
+            _factory, _organization.Id, OrganizationUserType.User);
+        await ProviderTestHelpers.CreateProviderUserAsync(_factory, provider.Id, providerAdminEmail,
+            ProviderUserType.ProviderAdmin);
+        await SetResetPasswordKeyAsync(providerAdminOrgUser);
+
+        var (_, memberOrgUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(
+            _factory, _organization.Id, OrganizationUserType.User);
+        await SetResetPasswordKeyAsync(memberOrgUser);
+
+        await _loginHelper.LoginAsync(callerEmail);
+
+        var request = new OrganizationUserBulkRequestModel { Ids = [providerAdminOrgUser.Id, memberOrgUser.Id] };
+
+        // Act
+        var response = await _client.PostAsJsonAsync(
+            $"organizations/{_organization.Id}/users/account-recovery-details", request);
+
+        // Assert - the Provider admin is dropped, the ordinary member is still returned
+        Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+        var result = await response.Content
+            .ReadFromJsonAsync<AccountRecoveryDetailsList>();
+        var details = Assert.Single(result.Data);
+        Assert.Equal(memberOrgUser.Id, details.OrganizationUserId);
+        // The filtering is what withholds the key material, so confirm it is really being returned
+        // for the user who is allowed through
+        Assert.Equal("encrypted-reset-password-key", details.ResetPasswordKey);
+    }
+
+    [Fact]
+    public async Task GetAccountRecoveryDetails_OmitsUsersTheCallerCannotRecover()
+    {
+        // Arrange
+        var (adminEmail, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory,
+            _organization.Id, OrganizationUserType.Admin);
+        await _loginHelper.LoginAsync(adminEmail);
+
+        var (_, ownerOrgUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(
+            _factory, _organization.Id, OrganizationUserType.Owner);
+        await SetResetPasswordKeyAsync(ownerOrgUser);
+
+        var (_, memberOrgUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(
+            _factory, _organization.Id, OrganizationUserType.User);
+        await SetResetPasswordKeyAsync(memberOrgUser);
+
+        var request = new OrganizationUserBulkRequestModel { Ids = [ownerOrgUser.Id, memberOrgUser.Id] };
+
+        // Act
+        var response = await _client.PostAsJsonAsync(
+            $"organizations/{_organization.Id}/users/account-recovery-details", request);
+
+        // Assert - an Admin may recover the member but not the Owner
+        Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+        var result = await response.Content
+            .ReadFromJsonAsync<AccountRecoveryDetailsList>();
+        var details = Assert.Single(result.Data);
+        Assert.Equal(memberOrgUser.Id, details.OrganizationUserId);
+    }
 }