From 8fefae98e4cf117b19de10d3423363fef6f3e003 Mon Sep 17 00:00:00 2001 From: Jared McCannon Date: Wed, 18 Feb 2026 08:09:45 -0600 Subject: [PATCH] [PM-18715] - SCIM Revoke User v2 (#7024) * Migrated SCIM revoke user call to the v2 implementation. * Correcting feature string --- .../Scim/Controllers/v2/UsersController.cs | 43 +++++++++++++++++-- .../Controllers/v2/UsersControllerTests.cs | 17 ++++++-- .../v2/RevokeOrganizationUsersValidator.cs | 3 +- src/Core/Constants.cs | 1 + .../RevokeOrganizationUsersValidatorTests.cs | 29 +++++++++++++ 5 files changed, 85 insertions(+), 8 deletions(-) diff --git a/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs b/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs index 91d79542b5..acb9f225e6 100644 --- a/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs +++ b/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs @@ -1,17 +1,22 @@ // FIXME: Update this file to be null safe and then delete the line below #nullable disable +using Bit.Core; +using Bit.Core.AdminConsole.Models.Data; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1; -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RevokeUser.v1; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RevokeUser.v2; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; +using Bit.Core.Services; using Bit.Scim.Models; using Bit.Scim.Users.Interfaces; using Bit.Scim.Utilities; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; +using IRevokeOrganizationUserCommand = Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RevokeUser.v1.IRevokeOrganizationUserCommand; +using IRevokeOrganizationUserCommandV2 = Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RevokeUser.v2.IRevokeOrganizationUserCommand; namespace Bit.Scim.Controllers.v2; @@ -28,6 +33,8 @@ public class UsersController : Controller private readonly IPostUserCommand _postUserCommand; private readonly IRestoreOrganizationUserCommand _restoreOrganizationUserCommand; private readonly IRevokeOrganizationUserCommand _revokeOrganizationUserCommand; + private readonly IFeatureService _featureService; + private readonly IRevokeOrganizationUserCommandV2 _revokeOrganizationUserCommandV2; public UsersController(IOrganizationUserRepository organizationUserRepository, IGetUsersListQuery getUsersListQuery, @@ -35,7 +42,9 @@ public class UsersController : Controller IPatchUserCommand patchUserCommand, IPostUserCommand postUserCommand, IRestoreOrganizationUserCommand restoreOrganizationUserCommand, - IRevokeOrganizationUserCommand revokeOrganizationUserCommand) + IRevokeOrganizationUserCommand revokeOrganizationUserCommand, + IFeatureService featureService, + IRevokeOrganizationUserCommandV2 revokeOrganizationUserCommandV2) { _organizationUserRepository = organizationUserRepository; _getUsersListQuery = getUsersListQuery; @@ -44,6 +53,8 @@ public class UsersController : Controller _postUserCommand = postUserCommand; _restoreOrganizationUserCommand = restoreOrganizationUserCommand; _revokeOrganizationUserCommand = revokeOrganizationUserCommand; + _featureService = featureService; + _revokeOrganizationUserCommandV2 = revokeOrganizationUserCommandV2; } [HttpGet("{id}")] @@ -100,7 +111,33 @@ public class UsersController : Controller } else if (!model.Active && orgUser.Status != OrganizationUserStatusType.Revoked) { - await _revokeOrganizationUserCommand.RevokeUserAsync(orgUser, EventSystemUser.SCIM); + if (_featureService.IsEnabled(FeatureFlagKeys.ScimRevokeV2)) + { + var results = await _revokeOrganizationUserCommandV2.RevokeUsersAsync( + new RevokeOrganizationUsersRequest( + organizationId, + [id], + new SystemUser(EventSystemUser.SCIM))); + + var errors = results.Select(x => x.Result.Match( + y => $"{y.Message} for user {x.Id}", + _ => null)) + .Where(x => !string.IsNullOrWhiteSpace(x)) + .ToList(); + + if (errors.Count != 0) + { + return new BadRequestObjectResult(new ScimErrorResponseModel + { + Status = 400, + Detail = string.Join(", ", errors) + }); + } + } + else + { + await _revokeOrganizationUserCommand.RevokeUserAsync(orgUser, EventSystemUser.SCIM); + } } // Have to get full details object for response model diff --git a/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/UsersControllerTests.cs b/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/UsersControllerTests.cs index 1f86d99b63..b8dc1b1123 100644 --- a/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/UsersControllerTests.cs +++ b/bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/UsersControllerTests.cs @@ -394,9 +394,18 @@ public class UsersControllerTests : IClassFixture, IAsyn Assert.Equal(_initialUserCount, databaseContext.OrganizationUsers.Count()); } - [Fact] - public async Task Put_RevokeUser_Success() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task Put_RevokeUser_Success(bool scimRevokeV2Enabled) { + var localFactory = new ScimApplicationFactory(); + localFactory.SubstituteService((IFeatureService featureService) + => featureService.IsEnabled(FeatureFlagKeys.ScimRevokeV2) + .Returns(scimRevokeV2Enabled)); + + localFactory.ReinitializeDbForTests(localFactory.GetDatabaseContext()); + var organizationUserId = ScimApplicationFactory.TestOrganizationUserId2; var inputModel = new ScimUserRequestModel { @@ -418,13 +427,13 @@ public class UsersControllerTests : IClassFixture, IAsyn Schemas = new List { ScimConstants.Scim2SchemaUser } }; - var context = await _factory.UsersPutAsync(ScimApplicationFactory.TestOrganizationId1, organizationUserId, inputModel); + var context = await localFactory.UsersPutAsync(ScimApplicationFactory.TestOrganizationId1, organizationUserId, inputModel); var responseModel = JsonSerializer.Deserialize(context.Response.Body, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }); Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode); AssertHelper.AssertPropertyEqual(expectedResponse, responseModel); - var databaseContext = _factory.GetDatabaseContext(); + var databaseContext = localFactory.GetDatabaseContext(); var revokedUser = databaseContext.OrganizationUsers.FirstOrDefault(g => g.Id == organizationUserId); Assert.Equal(OrganizationUserStatusType.Revoked, revokedUser.Status); } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/RevokeOrganizationUsersValidator.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/RevokeOrganizationUsersValidator.cs index d2f47ed713..00e8f5e6b4 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/RevokeOrganizationUsersValidator.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/RevokeOrganizationUsersValidator.cs @@ -29,7 +29,8 @@ public class RevokeOrganizationUsersValidator(IHasConfirmedOwnersExceptQuery has Invalid(x, new UserAlreadyRevoked()), { Type: OrganizationUserType.Owner } when !hasRemainingOwner => Invalid(x, new MustHaveConfirmedOwner()), - { Type: OrganizationUserType.Owner } when !request.PerformedBy.IsOrganizationOwnerOrProvider => + { Type: OrganizationUserType.Owner } when request.PerformedBy is not SystemUser + && !request.PerformedBy.IsOrganizationOwnerOrProvider => Invalid(x, new OnlyOwnersCanRevokeOwners()), _ => Valid(x) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 9a18cd32d9..25d5f8d4b9 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -139,6 +139,7 @@ public static class FeatureFlagKeys public const string ScimInviteUserOptimization = "pm-16811-optimize-invite-user-flow-to-fail-fast"; public const string CreateDefaultLocation = "pm-19467-create-default-location"; public const string AutomaticConfirmUsers = "pm-19934-auto-confirm-organization-users"; + public const string ScimRevokeV2 = "pm-32394-scim-revoke-put-v2"; public const string PM23845_VNextApplicationCache = "pm-24957-refactor-memory-application-cache"; public const string DefaultUserCollectionRestore = "pm-30883-my-items-restored-users"; public const string PremiumAccessQuery = "pm-29495-refactor-premium-interface"; diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/RevokeOrganizationUsersValidatorTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/RevokeOrganizationUsersValidatorTests.cs index fe5802b00b..cfeffb6e1e 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/RevokeOrganizationUsersValidatorTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/RevokeOrganizationUsersValidatorTests.cs @@ -236,6 +236,35 @@ public class RevokeOrganizationUsersValidatorTests Assert.True(results.First().IsValid); } + [Theory] + [BitAutoData] + public async Task ValidateAsync_WithSystemUser_RevokingOwner_ReturnsSuccess( + SutProvider sutProvider, + Guid organizationId, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser ownerUser) + { + // Arrange + ownerUser.OrganizationId = organizationId; + ownerUser.UserId = Guid.NewGuid(); + + var actingUser = CreateActingUser(null, false, EventSystemUser.SCIM); + var request = CreateValidationRequest( + organizationId, + [ownerUser], + actingUser); + + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(organizationId, Arg.Any>()) + .Returns(true); + + // Act + var results = (await sutProvider.Sut.ValidateAsync(request)).ToList(); + + // Assert + Assert.Single(results); + Assert.True(results.First().IsValid); + } + [Theory] [BitAutoData] public async Task ValidateAsync_WhenRevokingLastOwner_ReturnsErrorForThatUser(