Skip to content

Commit 8fefae9

Browse files
authored
[PM-18715] - SCIM Revoke User v2 (#7024)
* Migrated SCIM revoke user call to the v2 implementation. * Correcting feature string
1 parent 81120bd commit 8fefae9

5 files changed

Lines changed: 85 additions & 8 deletions

File tree

bitwarden_license/src/Scim/Controllers/v2/UsersController.cs

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
11
// FIXME: Update this file to be null safe and then delete the line below
22
#nullable disable
33

4+
using Bit.Core;
5+
using Bit.Core.AdminConsole.Models.Data;
46
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
57
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1;
6-
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RevokeUser.v1;
8+
using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RevokeUser.v2;
79
using Bit.Core.Enums;
810
using Bit.Core.Exceptions;
911
using Bit.Core.Repositories;
12+
using Bit.Core.Services;
1013
using Bit.Scim.Models;
1114
using Bit.Scim.Users.Interfaces;
1215
using Bit.Scim.Utilities;
1316
using Microsoft.AspNetCore.Authorization;
1417
using Microsoft.AspNetCore.Mvc;
18+
using IRevokeOrganizationUserCommand = Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RevokeUser.v1.IRevokeOrganizationUserCommand;
19+
using IRevokeOrganizationUserCommandV2 = Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RevokeUser.v2.IRevokeOrganizationUserCommand;
1520

1621
namespace Bit.Scim.Controllers.v2;
1722

@@ -28,14 +33,18 @@ public class UsersController : Controller
2833
private readonly IPostUserCommand _postUserCommand;
2934
private readonly IRestoreOrganizationUserCommand _restoreOrganizationUserCommand;
3035
private readonly IRevokeOrganizationUserCommand _revokeOrganizationUserCommand;
36+
private readonly IFeatureService _featureService;
37+
private readonly IRevokeOrganizationUserCommandV2 _revokeOrganizationUserCommandV2;
3138

3239
public UsersController(IOrganizationUserRepository organizationUserRepository,
3340
IGetUsersListQuery getUsersListQuery,
3441
IRemoveOrganizationUserCommand removeOrganizationUserCommand,
3542
IPatchUserCommand patchUserCommand,
3643
IPostUserCommand postUserCommand,
3744
IRestoreOrganizationUserCommand restoreOrganizationUserCommand,
38-
IRevokeOrganizationUserCommand revokeOrganizationUserCommand)
45+
IRevokeOrganizationUserCommand revokeOrganizationUserCommand,
46+
IFeatureService featureService,
47+
IRevokeOrganizationUserCommandV2 revokeOrganizationUserCommandV2)
3948
{
4049
_organizationUserRepository = organizationUserRepository;
4150
_getUsersListQuery = getUsersListQuery;
@@ -44,6 +53,8 @@ public UsersController(IOrganizationUserRepository organizationUserRepository,
4453
_postUserCommand = postUserCommand;
4554
_restoreOrganizationUserCommand = restoreOrganizationUserCommand;
4655
_revokeOrganizationUserCommand = revokeOrganizationUserCommand;
56+
_featureService = featureService;
57+
_revokeOrganizationUserCommandV2 = revokeOrganizationUserCommandV2;
4758
}
4859

4960
[HttpGet("{id}")]
@@ -100,7 +111,33 @@ public async Task<IActionResult> Put(Guid organizationId, Guid id, [FromBody] Sc
100111
}
101112
else if (!model.Active && orgUser.Status != OrganizationUserStatusType.Revoked)
102113
{
103-
await _revokeOrganizationUserCommand.RevokeUserAsync(orgUser, EventSystemUser.SCIM);
114+
if (_featureService.IsEnabled(FeatureFlagKeys.ScimRevokeV2))
115+
{
116+
var results = await _revokeOrganizationUserCommandV2.RevokeUsersAsync(
117+
new RevokeOrganizationUsersRequest(
118+
organizationId,
119+
[id],
120+
new SystemUser(EventSystemUser.SCIM)));
121+
122+
var errors = results.Select(x => x.Result.Match(
123+
y => $"{y.Message} for user {x.Id}",
124+
_ => null))
125+
.Where(x => !string.IsNullOrWhiteSpace(x))
126+
.ToList();
127+
128+
if (errors.Count != 0)
129+
{
130+
return new BadRequestObjectResult(new ScimErrorResponseModel
131+
{
132+
Status = 400,
133+
Detail = string.Join(", ", errors)
134+
});
135+
}
136+
}
137+
else
138+
{
139+
await _revokeOrganizationUserCommand.RevokeUserAsync(orgUser, EventSystemUser.SCIM);
140+
}
104141
}
105142

106143
// Have to get full details object for response model

bitwarden_license/test/Scim.IntegrationTest/Controllers/v2/UsersControllerTests.cs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -394,9 +394,18 @@ public async Task Post_ExistingData_Conflict(string email, string externalId)
394394
Assert.Equal(_initialUserCount, databaseContext.OrganizationUsers.Count());
395395
}
396396

397-
[Fact]
398-
public async Task Put_RevokeUser_Success()
397+
[Theory]
398+
[InlineData(true)]
399+
[InlineData(false)]
400+
public async Task Put_RevokeUser_Success(bool scimRevokeV2Enabled)
399401
{
402+
var localFactory = new ScimApplicationFactory();
403+
localFactory.SubstituteService((IFeatureService featureService)
404+
=> featureService.IsEnabled(FeatureFlagKeys.ScimRevokeV2)
405+
.Returns(scimRevokeV2Enabled));
406+
407+
localFactory.ReinitializeDbForTests(localFactory.GetDatabaseContext());
408+
400409
var organizationUserId = ScimApplicationFactory.TestOrganizationUserId2;
401410
var inputModel = new ScimUserRequestModel
402411
{
@@ -418,13 +427,13 @@ public async Task Put_RevokeUser_Success()
418427
Schemas = new List<string> { ScimConstants.Scim2SchemaUser }
419428
};
420429

421-
var context = await _factory.UsersPutAsync(ScimApplicationFactory.TestOrganizationId1, organizationUserId, inputModel);
430+
var context = await localFactory.UsersPutAsync(ScimApplicationFactory.TestOrganizationId1, organizationUserId, inputModel);
422431

423432
var responseModel = JsonSerializer.Deserialize<ScimUserResponseModel>(context.Response.Body, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase });
424433
Assert.Equal(StatusCodes.Status200OK, context.Response.StatusCode);
425434
AssertHelper.AssertPropertyEqual(expectedResponse, responseModel);
426435

427-
var databaseContext = _factory.GetDatabaseContext();
436+
var databaseContext = localFactory.GetDatabaseContext();
428437
var revokedUser = databaseContext.OrganizationUsers.FirstOrDefault(g => g.Id == organizationUserId);
429438
Assert.Equal(OrganizationUserStatusType.Revoked, revokedUser.Status);
430439
}

src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/RevokeOrganizationUsersValidator.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ _ when request.PerformedBy is not SystemUser
2929
Invalid(x, new UserAlreadyRevoked()),
3030
{ Type: OrganizationUserType.Owner } when !hasRemainingOwner =>
3131
Invalid(x, new MustHaveConfirmedOwner()),
32-
{ Type: OrganizationUserType.Owner } when !request.PerformedBy.IsOrganizationOwnerOrProvider =>
32+
{ Type: OrganizationUserType.Owner } when request.PerformedBy is not SystemUser
33+
&& !request.PerformedBy.IsOrganizationOwnerOrProvider =>
3334
Invalid(x, new OnlyOwnersCanRevokeOwners()),
3435

3536
_ => Valid(x)

src/Core/Constants.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ public static class FeatureFlagKeys
139139
public const string ScimInviteUserOptimization = "pm-16811-optimize-invite-user-flow-to-fail-fast";
140140
public const string CreateDefaultLocation = "pm-19467-create-default-location";
141141
public const string AutomaticConfirmUsers = "pm-19934-auto-confirm-organization-users";
142+
public const string ScimRevokeV2 = "pm-32394-scim-revoke-put-v2";
142143
public const string PM23845_VNextApplicationCache = "pm-24957-refactor-memory-application-cache";
143144
public const string DefaultUserCollectionRestore = "pm-30883-my-items-restored-users";
144145
public const string PremiumAccessQuery = "pm-29495-refactor-premium-interface";

test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeUser/v2/RevokeOrganizationUsersValidatorTests.cs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,35 @@ public async Task ValidateAsync_WithSystemUser_DoesNotRequireActingUserId(
236236
Assert.True(results.First().IsValid);
237237
}
238238

239+
[Theory]
240+
[BitAutoData]
241+
public async Task ValidateAsync_WithSystemUser_RevokingOwner_ReturnsSuccess(
242+
SutProvider<RevokeOrganizationUsersValidator> sutProvider,
243+
Guid organizationId,
244+
[OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser ownerUser)
245+
{
246+
// Arrange
247+
ownerUser.OrganizationId = organizationId;
248+
ownerUser.UserId = Guid.NewGuid();
249+
250+
var actingUser = CreateActingUser(null, false, EventSystemUser.SCIM);
251+
var request = CreateValidationRequest(
252+
organizationId,
253+
[ownerUser],
254+
actingUser);
255+
256+
sutProvider.GetDependency<IHasConfirmedOwnersExceptQuery>()
257+
.HasConfirmedOwnersExceptAsync(organizationId, Arg.Any<IEnumerable<Guid>>())
258+
.Returns(true);
259+
260+
// Act
261+
var results = (await sutProvider.Sut.ValidateAsync(request)).ToList();
262+
263+
// Assert
264+
Assert.Single(results);
265+
Assert.True(results.First().IsValid);
266+
}
267+
239268
[Theory]
240269
[BitAutoData]
241270
public async Task ValidateAsync_WhenRevokingLastOwner_ReturnsErrorForThatUser(

0 commit comments

Comments
 (0)