diff --git a/src/Core/AdminConsole/Enums/OrganizationUserActionType.cs b/src/Core/AdminConsole/Enums/OrganizationUserActionType.cs new file mode 100644 index 000000000000..796016e90994 --- /dev/null +++ b/src/Core/AdminConsole/Enums/OrganizationUserActionType.cs @@ -0,0 +1,8 @@ +namespace Bit.Core.AdminConsole.Enums; + +public enum OrganizationUserActionType +{ + Remove, + Revoke, + Restore, +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs index 936e328e74c1..aaca7829ec10 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RestoreUser/v1/RestoreOrganizationUserCommand.cs @@ -2,6 +2,8 @@ #nullable disable using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Validators; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Enforcement.AutoConfirm; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; @@ -31,7 +33,8 @@ public class RestoreOrganizationUserCommand( IPolicyRequirementQuery policyRequirementQuery, ICollectionRepository collectionRepository, IAutomaticUserConfirmationPolicyEnforcementValidator automaticUserConfirmationPolicyEnforcementValidator, - IDeleteEmergencyAccessCommand deleteEmergencyAccessCommand) : IRestoreOrganizationUserCommand + IDeleteEmergencyAccessCommand deleteEmergencyAccessCommand, + ICustomUserActingOnAdminValidator customUserActingOnAdminValidator) : IRestoreOrganizationUserCommand { public async Task RestoreUserAsync(OrganizationUser organizationUser, Guid? restoringUserId, string defaultCollectionName) { @@ -46,6 +49,8 @@ public async Task RestoreUserAsync(OrganizationUser organizationUser, Guid? rest throw new BadRequestException("Only owners can restore other owners."); } + await customUserActingOnAdminValidator.EnforceAsync(organizationUser, OrganizationUserActionType.Restore); + await RepositoryRestoreUserAsync(organizationUser, defaultCollectionName); await eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); @@ -218,6 +223,8 @@ public async Task>> RestoreUsersAsync(Guid throw new BadRequestException("Only owners can restore other owners."); } + await customUserActingOnAdminValidator.EnforceAsync(organizationUser, OrganizationUserActionType.Restore); + var twoFactorIsEnabled = organizationUser.UserId.HasValue && organizationUsersTwoFactorEnabled .FirstOrDefault(ou => ou.userId == organizationUser.UserId.Value) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v1/RevokeOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v1/RevokeOrganizationUserCommand.cs index f8a7e3cff9db..a6812a9d2de3 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v1/RevokeOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v1/RevokeOrganizationUserCommand.cs @@ -1,4 +1,6 @@ -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Validators; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; @@ -14,7 +16,8 @@ public class RevokeOrganizationUserCommand( IPushNotificationService pushNotificationService, IOrganizationUserRepository organizationUserRepository, ICurrentContext currentContext, - IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery) + IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery, + ICustomUserActingOnAdminValidator customUserActingOnAdminValidator) : IRevokeOrganizationUserCommand { public async Task RevokeUserAsync(OrganizationUser organizationUser, Guid? revokingUserId, RevocationReason reason) @@ -30,6 +33,8 @@ public async Task RevokeUserAsync(OrganizationUser organizationUser, Guid? revok throw new BadRequestException("Only owners can revoke other owners."); } + await customUserActingOnAdminValidator.EnforceAsync(organizationUser, OrganizationUserActionType.Revoke); + await RepositoryRevokeUserAsync(organizationUser, reason); await eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Revoked); diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/Errors.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/Errors.cs index a30894c7d540..b6c091d3f9a2 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/Errors.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/Errors.cs @@ -6,3 +6,4 @@ public record UserAlreadyRevoked() : BadRequestError("Already revoked."); public record CannotRevokeYourself() : BadRequestError("You cannot revoke yourself."); public record OnlyOwnersCanRevokeOwners() : BadRequestError("Only owners can revoke other owners."); public record MustHaveConfirmedOwner() : BadRequestError("Organization must have at least one confirmed owner."); +public record CustomUserCannotRevokeAdmin() : BadRequestError("Custom users can not revoke admins."); diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/RevokeOrganizationUsersValidator.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/RevokeOrganizationUsersValidator.cs index 00e8f5e6b48c..ab92393b32f7 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/RevokeOrganizationUsersValidator.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/RevokeOrganizationUsersValidator.cs @@ -1,5 +1,6 @@ using Bit.Core.AdminConsole.Models.Data; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Validators; using Bit.Core.AdminConsole.Utilities.v2.Validation; using Bit.Core.Entities; using Bit.Core.Enums; @@ -7,7 +8,9 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RevokeUser.v2; -public class RevokeOrganizationUsersValidator(IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery) +public class RevokeOrganizationUsersValidator( + IHasConfirmedOwnersExceptQuery hasConfirmedOwnersExceptQuery, + ICustomUserActingOnAdminValidator customUserActingOnAdminValidator) : IRevokeOrganizationUserValidator { public async Task>> ValidateAsync( @@ -17,6 +20,8 @@ public async Task>> ValidateAsync request.OrganizationUsersToRevoke.Select(x => x.Id) // users excluded because they are going to be revoked ); + var customUserCannotRevokeAdmin = await CustomUserCannotRevokeAdminAsync(request.OrganizationUsersToRevoke); + return request.OrganizationUsersToRevoke.Select(x => { return x switch @@ -32,9 +37,20 @@ _ when request.PerformedBy is not SystemUser { Type: OrganizationUserType.Owner } when request.PerformedBy is not SystemUser && !request.PerformedBy.IsOrganizationOwnerOrProvider => Invalid(x, new OnlyOwnersCanRevokeOwners()), + { Type: OrganizationUserType.Admin } when customUserCannotRevokeAdmin => + Invalid(x, new CustomUserCannotRevokeAdmin()), _ => Valid(x) }; }).ToList(); } + + // Probes with any admin in the batch. The rule's answer is uniform across every admin + // in the same organization, so one cached lookup covers the whole batch. + private async Task CustomUserCannotRevokeAdminAsync(IEnumerable users) + { + var anyAdmin = users.FirstOrDefault(x => x.Type == OrganizationUserType.Admin); + return anyAdmin is not null + && await customUserActingOnAdminValidator.IsBlockedAsync(anyAdmin); + } } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Validators/CustomUserActingOnAdminValidator.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Validators/CustomUserActingOnAdminValidator.cs new file mode 100644 index 000000000000..6a9b704edec8 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Validators/CustomUserActingOnAdminValidator.cs @@ -0,0 +1,54 @@ +using Bit.Core.AdminConsole.Enums; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Validators; + +public class CustomUserActingOnAdminValidator(ICurrentContext currentContext) : ICustomUserActingOnAdminValidator +{ + // Memoize OrganizationCustom answers for the lifetime of this request-scoped instance. + private readonly Dictionary _organizationCustomLookup = new(); + + public async Task EnforceAsync(OrganizationUser targetUser, OrganizationUserActionType actionType) + { + if (await IsBlockedAsync(targetUser)) + { + throw new BadRequestException(actionType.ToCustomUserCannotModifyAdminMessage()); + } + } + + public async Task IsBlockedAsync(OrganizationUser targetUser) + { + if (targetUser.Type != OrganizationUserType.Admin) + { + return false; + } + + return await IsActingUserCustomAsync(targetUser.OrganizationId); + } + + private async Task IsActingUserCustomAsync(Guid organizationId) + { + if (_organizationCustomLookup.TryGetValue(organizationId, out var cached)) + { + return cached; + } + + var isCustom = await currentContext.OrganizationCustom(organizationId); + _organizationCustomLookup[organizationId] = isCustom; + return isCustom; + } +} + +internal static class OrganizationUserActionExtensions +{ + public static string ToCustomUserCannotModifyAdminMessage(this OrganizationUserActionType actionType) => actionType switch + { + OrganizationUserActionType.Remove => "Custom users can not remove admins.", + OrganizationUserActionType.Revoke => "Custom users can not revoke admins.", + OrganizationUserActionType.Restore => "Custom users can not restore admins.", + _ => throw new ArgumentOutOfRangeException(nameof(actionType), actionType, null), + }; +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Validators/ICustomUserActingOnAdminValidator.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Validators/ICustomUserActingOnAdminValidator.cs new file mode 100644 index 000000000000..43e38a5510ad --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Validators/ICustomUserActingOnAdminValidator.cs @@ -0,0 +1,25 @@ +using Bit.Core.AdminConsole.Enums; +using Bit.Core.Entities; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Validators; + +public interface ICustomUserActingOnAdminValidator +{ + /// + /// Throws when the current acting user + /// has the Custom role in the target user's organization and the target user is an Admin. + /// Custom users (even those granted the ManageUsers permission) are not permitted to modify + /// Admins under the role hierarchy. The exception message is scoped to the supplied actionType. + /// + /// The organization user that the acting user is attempting to modify. + /// The actionType being attempted; selects the error message. + Task EnforceAsync(OrganizationUser targetUser, OrganizationUserActionType actionType); + + /// + /// Returns true when the current acting user has the Custom role in the target user's + /// organization and the target user is an Admin. Custom users (even those granted the + /// ManageUsers permission) are not permitted to modify Admins under the role hierarchy. + /// + /// The organization user that the acting user is attempting to modify. + Task IsBlockedAsync(OrganizationUser targetUser); +} diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index 2942e67e8c88..ec5653b1583e 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -30,6 +30,7 @@ using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.OrganizationConfirmation; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.SelfRevokeUser; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Validators; using Bit.Core.Models.Business.Tokenables; using Bit.Core.OrganizationFeatures.OrganizationSponsorships.FamiliesForEnterprise; using Bit.Core.OrganizationFeatures.OrganizationSponsorships.FamiliesForEnterprise.Cloud; @@ -227,6 +228,7 @@ private static void AddOrganizationUserCommandsQueries(this IServiceCollection s services.AddScoped(); services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped();