[PM-10319] - Revoke Non Complaint Users for 2FA and Single Org Policy Enablement

src/Core/AdminConsole/Models/Data/IActingUser.cs

@@ -5,6 +5,6 @@ namespace Bit.Core.AdminConsole.Models.Data;
 public interface IActingUser
 {
     Guid? UserId { get; }
-    bool IsOrganizationOwner { get; }
+    bool IsOrganizationOwnerOrProvider { get; }
     EventSystemUser? SystemUserType { get; }
 }

src/Core/AdminConsole/Models/Data/StandardUser.cs

@@ -7,10 +7,10 @@
     public StandardUser(Guid userId, bool isOrganizationOwner)
     {
         UserId = userId;
-        IsOrganizationOwner = isOrganizationOwner;
+        IsOrganizationOwnerOrProvider = isOrganizationOwner;
     }
 
     public Guid? UserId { get; }
-    public bool IsOrganizationOwner { get; }
+    public bool IsOrganizationOwnerOrProvider { get; }
     public EventSystemUser? SystemUserType => throw new Exception($"{nameof(StandardUser)} does not have a {nameof(SystemUserType)}");
 }

src/Core/AdminConsole/Models/Data/SystemUser.cs

@@ -9,8 +9,8 @@
         SystemUserType = systemUser;
     }
 
     public Guid? UserId => throw new Exception($"{nameof(SystemUserType)} does not have a {nameof(UserId)}.");
 
-    public bool IsOrganizationOwner => false;
+    public bool IsOrganizationOwnerOrProvider => false;
     public EventSystemUser? SystemUserType { get; }
 }

src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeNonCompliantOrganizationUserCommand.cs

@@ -30,56 +30,57 @@
             return validationResult;
         }
 
-        await organizationUserRepository.SetOrganizationUsersStatusAsync(request.OrganizationUsers.Select(x => x.Id),
-            OrganizationUserStatusType.Revoked);
+        await organizationUserRepository.RevokeOrganizationUserAsync(request.OrganizationUsers.Select(x => x.Id));
 
-        if (request.ActionPerformedBy.GetType() == typeof(StandardUser))
-        {
-            await eventService.LogOrganizationUserEventsAsync(
-                request.OrganizationUsers.Select(x => GetRevokedUserEventTuple(x, timeProvider.GetUtcNow())));
-        }
-        else if (request.ActionPerformedBy is SystemUser { SystemUserType: not null } loggableSystem)
+        var now = timeProvider.GetUtcNow();
+
+        switch (request.ActionPerformedBy)
         {
-            await eventService.LogOrganizationUserEventsAsync(
-                request.OrganizationUsers.Select(x =>
-                    GetRevokedUserEventBySystemUserTuple(x, loggableSystem.SystemUserType.Value,
-                        timeProvider.GetUtcNow())));
+            case StandardUser:
+                await eventService.LogOrganizationUserEventsAsync(
+                    request.OrganizationUsers.Select(x => GetRevokedUserEventTuple(x, now)));
+                break;
+            case SystemUser { SystemUserType: not null } loggableSystem:
+                await eventService.LogOrganizationUserEventsAsync(
+                    request.OrganizationUsers.Select(x =>
+                        GetRevokedUserEventBySystemUserTuple(x, loggableSystem.SystemUserType.Value, now)));
+                break;
         }
 
         return validationResult;
     }
 
     private static (OrganizationUserUserDetails organizationUser, EventType eventType, DateTime? time) GetRevokedUserEventTuple(
-        OrganizationUserUserDetails organizationUser, DateTimeOffset dateTimeOffset) => new(organizationUser,
-        EventType.OrganizationUser_Revoked, dateTimeOffset.UtcDateTime);
+        OrganizationUserUserDetails organizationUser, DateTimeOffset dateTimeOffset) =>
+        new(organizationUser, EventType.OrganizationUser_Revoked, dateTimeOffset.UtcDateTime);
 
     private static (OrganizationUserUserDetails organizationUser, EventType eventType, EventSystemUser eventSystemUser, DateTime? time) GetRevokedUserEventBySystemUserTuple(
         OrganizationUserUserDetails organizationUser, EventSystemUser systemUser, DateTimeOffset dateTimeOffset) => new(organizationUser,
         EventType.OrganizationUser_Revoked, systemUser, dateTimeOffset.UtcDateTime);
 
     private async Task<CommandResult> ValidateAsync(RevokeOrganizationUsersRequest request)
     {
         if (!PerformedByIsAnExpectedType(request.ActionPerformedBy))
         {
-            return new CommandResult([ErrorRequestedByWasNotValid]);
+            return new CommandResult(ErrorRequestedByWasNotValid);
         }
 
-        if (request.ActionPerformedBy is StandardUser loggableUser
-            && request.OrganizationUsers.Any(x => x.UserId == loggableUser.UserId))
+        if (request.ActionPerformedBy is StandardUser user
+            && request.OrganizationUsers.Any(x => x.UserId == user.UserId))
         {
-            return new CommandResult([ErrorCannotRevokeSelf]);
+            return new CommandResult(ErrorCannotRevokeSelf);
         }
 
         if (request.OrganizationUsers.Any(x => x.OrganizationId != request.OrganizationId))
         {
-            return new CommandResult([ErrorInvalidUsers]);
+            return new CommandResult(ErrorInvalidUsers);
         }
 
         if (!await confirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(
                     request.OrganizationId,
                     request.OrganizationUsers.Select(x => x.Id)))
         {
-            return new CommandResult([ErrorOrgMustHaveAtLeastOneOwner]);
+            return new CommandResult(ErrorOrgMustHaveAtLeastOneOwner);
         }
 
         return request.OrganizationUsers.Aggregate(new CommandResult(), (result, userToRevoke) =>
@@ -107,9 +108,5 @@
 
     private static bool NonOwnersCannotRevokeOwners(OrganizationUserUserDetails organizationUser,
         IActingUser actingUser) =>
-        actingUser is StandardUser { IsOrganizationOwner: false } && organizationUser.Type == OrganizationUserType.Owner;
-
-    private static bool IsActingUserAllowedToRevokeOwner(OrganizationUserUserDetails organizationUser,
-        StandardUser requestingOrganizationUser) => organizationUser is { Type: OrganizationUserType.Owner }
-                                                    && requestingOrganizationUser.IsOrganizationOwner;
+        actingUser is StandardUser { IsOrganizationOwnerOrProvider: false } && organizationUser.Type == OrganizationUserType.Owner;
 }

src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidator.cs

@@ -20,6 +20,8 @@
 public class SingleOrgPolicyValidator : IPolicyValidator
 {
     public PolicyType Type => PolicyType.SingleOrg;
+    private const string OrganizationNotFoundErrorMessage = "Organization not found.";
+    private const string ClaimedDomainSingleOrganizationRequiredErrorMessage = "The Single organization policy is required for organizations that have enabled domain verification.";
 
     private readonly IOrganizationUserRepository _organizationUserRepository;
     private readonly IMailService _mailService;
@@ -59,10 +61,55 @@
     {
         if (currentPolicy is not { Enabled: true } && policyUpdate is { Enabled: true })
         {
-            await RemoveNonCompliantUsersAsync(policyUpdate.OrganizationId);
+            if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning))
+            {
+                var currentUser = _currentContext.UserId ?? Guid.Empty;
+                var isOwnerOrProvider = await _currentContext.OrganizationOwner(policyUpdate.OrganizationId);
+                await RevokeNonCompliantUsersAsync(policyUpdate.OrganizationId, policyUpdate.PerformedBy ?? new StandardUser(currentUser, isOwnerOrProvider));
+            }
+            else
+            {
+                await RemoveNonCompliantUsersAsync(policyUpdate.OrganizationId);
+            }
         }
     }
 
+    private async Task RevokeNonCompliantUsersAsync(Guid organizationId, IActingUser performedBy)
+    {
+        var organization = await _organizationRepository.GetByIdAsync(organizationId);
+
+        if (organization is null)
+        {
+            throw new NotFoundException(OrganizationNotFoundErrorMessage);
+        }
+
+        var currentActiveRevocableOrganizationUsers =
+            (await _organizationUserRepository.GetManyDetailsByOrganizationAsync(organizationId))
+            .Where(ou => ou.Status != OrganizationUserStatusType.Invited &&
+                         ou.Status != OrganizationUserStatusType.Revoked &&
+                         ou.Type != OrganizationUserType.Owner &&
+                         ou.Type != OrganizationUserType.Admin &&
+                         performedBy is StandardUser stdUser &&
+                         stdUser.UserId != ou.UserId)
+            .ToList();
+
+        if (currentActiveRevocableOrganizationUsers.Count == 0)
+        {
+            return;
+        }
+
+        var commandResult = await _revokeNonCompliantOrganizationUserCommand.RevokeNonCompliantOrganizationUsersAsync(
+            new RevokeOrganizationUsersRequest(organizationId, currentActiveRevocableOrganizationUsers, performedBy));
+
+        if (commandResult.HasErrors)
+        {
+            throw new BadRequestException(string.Join(", ", commandResult.ErrorMessages));
+        }
+
+        await Task.WhenAll(currentActiveRevocableOrganizationUsers.Select(x =>
+            _mailService.SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(organization.DisplayName(), x.Email)));
+    }
+
     private async Task RemoveNonCompliantUsersAsync(Guid organizationId)
     {
         // Remove non-compliant users
@@ -72,7 +119,7 @@
         var org = await _organizationRepository.GetByIdAsync(organizationId);
         if (org == null)
         {
-            throw new NotFoundException("Organization not found.");
+            throw new NotFoundException(OrganizationNotFoundErrorMessage);
         }
 
         var removableOrgUsers = orgUsers.Where(ou =>
@@ -86,41 +133,16 @@
         var userOrgs = await _organizationUserRepository.GetManyByManyUsersAsync(
                 removableOrgUsers.Select(ou => ou.UserId!.Value));
 
-        if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning))
+        foreach (var orgUser in removableOrgUsers)
         {
-            var isOwner = await _currentContext.OrganizationOwner(organizationId);
-
-            var revocableUsers = removableOrgUsers.Where(nonCompliantUser => orgUsers.Any(orgUser =>
-                nonCompliantUser.UserId == orgUser.UserId
-                && nonCompliantUser.OrganizationId != org.Id
-                && nonCompliantUser.Status != OrganizationUserStatusType.Invited))
-                .ToList();
-
-            var commandResult = await _revokeNonCompliantOrganizationUserCommand.RevokeNonCompliantOrganizationUsersAsync(
-                new RevokeOrganizationUsersRequest(organizationId, revocableUsers, new StandardUser(savingUserId ?? Guid.Empty, isOwner)));
-
-            if (commandResult.HasErrors)
+            if (userOrgs.Any(ou => ou.UserId == orgUser.UserId
+                                   && ou.OrganizationId != org.Id
+                                   && ou.Status != OrganizationUserStatusType.Invited))
             {
-                throw new BadRequestException(string.Join(", ", commandResult.ErrorMessages));
-            }
+                await _removeOrganizationUserCommand.RemoveUserAsync(organizationId, orgUser.Id, savingUserId);
 
-            await Task.WhenAll(revocableUsers.Select(x =>
-                _mailService.SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(org.DisplayName(), x.Email)));
-        }
-        else
-        {
-            foreach (var orgUser in removableOrgUsers)
-            {
-                if (userOrgs.Any(ou => ou.UserId == orgUser.UserId
-                                       && ou.OrganizationId != org.Id
-                                       && ou.Status != OrganizationUserStatusType.Invited))
-                {
-                    await _removeOrganizationUserCommand.RemoveUserAsync(organizationId, orgUser.Id,
-                        savingUserId);
-
-                    await _mailService.SendOrganizationUserRemovedForPolicySingleOrgEmailAsync(
-                        org.DisplayName(), orgUser.Email);
-                }
+                await _mailService.SendOrganizationUserRemovedForPolicySingleOrgEmailAsync(
+                    org.DisplayName(), orgUser.Email);
             }
         }
     }
@@ -141,7 +163,7 @@
             if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)
                 && await _organizationHasVerifiedDomainsQuery.HasVerifiedDomainsAsync(policyUpdate.OrganizationId))
             {
-                return "The Single organization policy is required for organizations that have enabled domain verification.";
+                return ClaimedDomainSingleOrganizationRequiredErrorMessage;
             }
         }
 

src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/TwoFactorAuthenticationPolicyValidator.cs

@@ -56,10 +56,58 @@
     {
         if (currentPolicy is not { Enabled: true } && policyUpdate is { Enabled: true })
         {
-            await RemoveNonCompliantUsersAsync(policyUpdate.OrganizationId);
+            if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning))
+            {
+                var currentUser = _currentContext.UserId ?? Guid.Empty;
+                var isOwnerOrProvider = await _currentContext.OrganizationOwner(policyUpdate.OrganizationId);
+                await RevokeNonCompliantUsersAsync(policyUpdate.OrganizationId, policyUpdate.PerformedBy ?? new StandardUser(currentUser, isOwnerOrProvider));
+            }
+            else
+            {
+                await RemoveNonCompliantUsersAsync(policyUpdate.OrganizationId);
+            }
         }
     }
 
+    private async Task RevokeNonCompliantUsersAsync(Guid organizationId, IActingUser performedBy)
+    {
+        var organization = await _organizationRepository.GetByIdAsync(organizationId);
+
+        var currentActiveRevocableOrganizationUsers =
+            (await _organizationUserRepository.GetManyDetailsByOrganizationAsync(organizationId))
+            .Where(ou => ou.Status != OrganizationUserStatusType.Invited &&
+                         ou.Status != OrganizationUserStatusType.Revoked &&
+                         ou.Type != OrganizationUserType.Owner &&
+                         ou.Type != OrganizationUserType.Admin &&
+                         performedBy is StandardUser stdUser &&
+                         stdUser.UserId != ou.UserId)
+            .ToList();
+
+        if (currentActiveRevocableOrganizationUsers.Count == 0)
+        {
+            return;
+        }
+
+        var organizationUsersTwoFactorEnabled =
+            await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(currentActiveRevocableOrganizationUsers);
+
+        if (DoMembersNotHaveAPasswordWithTwoFactorAuthenticationEnabled(currentActiveRevocableOrganizationUsers, organizationUsersTwoFactorEnabled))
+        {
+            throw new BadRequestException(NonCompliantMembersWillLoseAccess);
+        }
+
+        var commandResult = await _revokeNonCompliantOrganizationUserCommand.RevokeNonCompliantOrganizationUsersAsync(
+            new RevokeOrganizationUsersRequest(organizationId, currentActiveRevocableOrganizationUsers, performedBy));
+
+        if (commandResult.HasErrors)
+        {
+            throw new BadRequestException(string.Join(", ", commandResult.ErrorMessages));
+        }
+
+        await Task.WhenAll(currentActiveRevocableOrganizationUsers.Select(x =>
+            _mailService.SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(organization.DisplayName(), x.Email)));
+    }
+
     private async Task RemoveNonCompliantUsersAsync(Guid organizationId)
     {
         var org = await _organizationRepository.GetByIdAsync(organizationId);
@@ -68,54 +116,32 @@
         var orgUsers = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(organizationId);
 
         // this can get moved into the private method after AccountDeprovisiong flag check is removed.
-        var organizationUsersTwoFactorEnabled = (await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(orgUsers)).ToList();
+        var organizationUsersTwoFactorEnabled =
+            (await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(orgUsers)).ToList();
 
         var revocableOrgUsers = orgUsers.Where(ou =>
                 ou.Status != OrganizationUserStatusType.Invited && ou.Status != OrganizationUserStatusType.Revoked &&
                 ou.Type != OrganizationUserType.Owner && ou.Type != OrganizationUserType.Admin &&
                 ou.UserId != savingUserId)
             .ToList();
 
-        if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning))
+        // Reorder by HasMasterPassword to prioritize checking users without a master if they have 2FA enabled
+        foreach (var orgUser in revocableOrgUsers.OrderBy(ou => ou.HasMasterPassword))
         {
-            if (DoMembersNotHaveAPasswordWithTwoFactorAuthenticationEnabled(revocableOrgUsers, organizationUsersTwoFactorEnabled))
-            {
-                throw new BadRequestException(NonCompliantMembersWillLoseAccess);
-            }
-
-            var result = await _revokeNonCompliantOrganizationUserCommand.RevokeNonCompliantOrganizationUsersAsync(
-                new RevokeOrganizationUsersRequest(organizationId, revocableOrgUsers,
-                    new StandardUser(savingUserId ?? Guid.Empty,
-                        await _currentContext.OrganizationOwner(organizationId))));
-
-            if (result.HasErrors)
+            var userTwoFactorEnabled = organizationUsersTwoFactorEnabled
+                .FirstOrDefault(u => u.user.Id == orgUser.Id).twoFactorIsEnabled;
+            if (!userTwoFactorEnabled)
             {
-                throw new BadRequestException(string.Join(", ", result.ErrorMessages));
-            }
-
-            await Task.WhenAll(revocableOrgUsers.Select(x =>
-                _mailService.SendOrganizationUserRevokedForTwoFactoryPolicyEmailAsync(org!.DisplayName(), x.Email)));
-        }
-        else
-        {
-            // Reorder by HasMasterPassword to prioritize checking users without a master if they have 2FA enabled
-            foreach (var orgUser in revocableOrgUsers.OrderBy(ou => ou.HasMasterPassword))
-            {
-                var userTwoFactorEnabled = organizationUsersTwoFactorEnabled
-                    .FirstOrDefault(u => u.user.Id == orgUser.Id).twoFactorIsEnabled;
-                if (!userTwoFactorEnabled)
+                if (!orgUser.HasMasterPassword)
                 {
-                    if (!orgUser.HasMasterPassword)
-                    {
-                        throw new BadRequestException(NonCompliantMembersWillLoseAccess);
-                    }
+                    throw new BadRequestException(NonCompliantMembersWillLoseAccess);
+                }
 
-                    await _removeOrganizationUserCommand.RemoveUserAsync(organizationId, orgUser.Id,
-                        savingUserId);
+                await _removeOrganizationUserCommand.RemoveUserAsync(organizationId, orgUser.Id,
+                    savingUserId);
 
-                    await _mailService.SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(
-                        org!.DisplayName(), orgUser.Email);
-                }
+                await _mailService.SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(
+                    org!.DisplayName(), orgUser.Email);
             }
         }
     }

src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs

@@ -59,5 +59,5 @@ UpdateEncryptedDataForKeyRotation UpdateForKeyRotation(Guid userId,
     /// </summary>
     Task<ICollection<OrganizationUser>> GetManyByOrganizationWithClaimedDomainsAsync(Guid organizationId);
 
-    Task SetOrganizationUsersStatusAsync(IEnumerable<Guid> userIds, OrganizationUserStatusType status);
+    Task RevokeOrganizationUserAsync(IEnumerable<Guid> userIds);
 }

src/Core/Models/Commands/CommandResult.cs

@@ -2,6 +2,8 @@
 
 public class CommandResult(IEnumerable<string> errors)
 {
+    public CommandResult(string error) : this([error]) { }
+
     public bool Success => ErrorMessages.Count == 0;
     public bool HasErrors => ErrorMessages.Count > 0;
     public List<string> ErrorMessages { get; } = errors.ToList();