Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
1499299
added Org User Action Validator to handle logic for org users perform…
jrmccannon Jun 26, 2026
c65cf58
Updated to match feedback.
jrmccannon Jun 29, 2026
be9f0c7
[PM-38923] feat: Add v2 UpdateOrganizationUser command with role-esca…
jrmccannon Jul 2, 2026
ffbdd80
Fix Put collection-auth mocks to match bulk ModifyUserAccess check
jrmccannon Jul 2, 2026
b874aef
chore: Remove v1 command references from v2 UpdateOrganizationUser co…
jrmccannon Jul 2, 2026
b72ffe3
refactor: Fetch collections once and reject default collections in v2…
jrmccannon Jul 3, 2026
71e1b13
Simplified tests around user role validation in update org user valid…
jrmccannon Jul 8, 2026
ad9f9ff
refactor: Simplify v2 UpdateOrganizationUser request object
jrmccannon Jul 8, 2026
b361f7d
fix: Enforce custom-permission grant-subset check in v2 UpdateOrganiz…
jrmccannon Jul 8, 2026
6842cb9
removed unnecessary using.
jrmccannon Jul 8, 2026
76e15a4
removed unnecessary comment
jrmccannon Jul 8, 2026
0cd8623
removed unnecessary comment.
jrmccannon Jul 8, 2026
22b0edd
test: Cover escalation-error branch mapping in v2 UpdateOrganizationU…
jrmccannon Jul 9, 2026
32225c4
Merge branch 'main' into jmccannon/ac/pm-38923-update-orguser-v2
jrmccannon Jul 9, 2026
710a99b
test: Assert defaultUserCollectionName in v1 UpdateUser routing test
jrmccannon Jul 9, 2026
3ab47a6
Merge branch 'main' into jmccannon/ac/pm-38923-update-orguser-v2
jrmccannon Jul 9, 2026
03016b0
[PM-38923] Create default collection on demotion in v2 UpdateOrganiza…
jrmccannon Jul 9, 2026
0107db6
Merge branch 'main' into jmccannon/ac/pm-38923-update-orguser-v2
jrmccannon Jul 9, 2026
cea4547
Merge branch 'main' into jmccannon/ac/pm-38923-update-orguser-v2
jrmccannon Jul 13, 2026
5a0cd43
Fixed querying policy after user is demoted. Improved readability and…
jrmccannon Jul 14, 2026
6f31321
cleaned up comments
jrmccannon Jul 14, 2026
1a07035
comment clean up
jrmccannon Jul 14, 2026
dde4b4a
comment clean up
jrmccannon Jul 14, 2026
08e0866
Added private methods with state capture fields to capture if we're e…
jrmccannon Jul 17, 2026
34d3ab4
Added update user method to entity.
jrmccannon Jul 17, 2026
a90bc0a
Added method to requirement and moved common code to private method f…
jrmccannon Jul 17, 2026
7a40b38
Added xml docs
jrmccannon Jul 17, 2026
dc28076
We need org for secrets manager update so we'll just use it instead o…
jrmccannon Jul 17, 2026
c16dfe0
We're fetching organization already. No need to use the org ability.
jrmccannon Jul 17, 2026
c1a6bfa
Moved org user values into the IActingUser.
jrmccannon Jul 17, 2026
5a0fd82
cleaned up some comments
jrmccannon Jul 17, 2026
3ddea95
fixed up tests
jrmccannon Jul 17, 2026
81de970
Updated tests to not need org ability (and removed unused org ability…
jrmccannon Jul 17, 2026
788165f
Merge branch 'refs/heads/main' into jmccannon/ac/pm-38923-update-orgu…
jrmccannon Jul 17, 2026
c4f8472
Add default message to NotFoundError
jrmccannon Jul 20, 2026
0aa2c2e
Clarify custom permissions comment
jrmccannon Jul 20, 2026
a078464
Move collection access validity check onto the model
jrmccannon Jul 20, 2026
f60221e
Merge branch 'main' into jmccannon/ac/pm-38923-update-orguser-v2
jrmccannon Jul 20, 2026
d561aac
Move role and permission checks into ManageOrganizationUserValidation…
jrmccannon Jul 20, 2026
d2860ce
Move free org admin limit check into the validation service
jrmccannon Jul 20, 2026
09dea82
Remove unused role fields from IActingUser interface
jrmccannon Jul 20, 2026
cc62bef
Swapping Arg.Any to Arg.Is<T>
jrmccannon Jul 20, 2026
170b035
Rename test to match its assertions
jrmccannon Jul 20, 2026
b5ca2ab
Corrected SM related tests. added new positive case.
jrmccannon Jul 20, 2026
ad23593
Add PUT integration test suite for both feature flag states
jrmccannon Jul 21, 2026
d5e4b8c
Switched to SDK feature Service.
jrmccannon Jul 21, 2026
793af00
Merge branch 'main' into jmccannon/ac/pm-38923-update-orguser-v2
jrmccannon Jul 21, 2026
ae887c4
Checked all clients and didn't see anyone consuming. Removing obsolet…
jrmccannon Jul 21, 2026
cd2c54a
Merge branch 'main' into jmccannon/ac/pm-38923-update-orguser-v2
jrmccannon Jul 22, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 83 additions & 40 deletions src/Api/AdminConsole/Controllers/OrganizationUsersController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using Bit.Api.AdminConsole.Models.Response.Organizations;
using Bit.Api.Models.Response;
using Bit.Core;
using Bit.Core.AdminConsole.AbilitiesCache;
using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Models.Data;
using Bit.Core.AdminConsole.OrganizationFeatures.AccountRecovery;
Expand All @@ -37,6 +36,7 @@
using Bit.Core.Exceptions;
using Bit.Core.Models.Api;
using Bit.Core.Models.Business;
using Bit.Core.Models.Data;
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
using Bit.Core.OrganizationFeatures.OrganizationSubscriptions.Interface;
using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces;
Expand All @@ -49,6 +49,7 @@
using Microsoft.AspNetCore.Mvc;
using V1_RevokeOrganizationUserCommand = Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RevokeUser.v1.IRevokeOrganizationUserCommand;
using V2_RevokeOrganizationUserCommand = Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RevokeUser.v2;
using V2_UpdateUserCommand = Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.UpdateUser.v2;

namespace Bit.Api.AdminConsole.Controllers;

Expand All @@ -68,7 +69,6 @@ public class OrganizationUsersController : BaseAdminConsoleController
private readonly IUpdateOrganizationUserCommand _updateOrganizationUserCommand;
private readonly IAcceptOrgUserCommand _acceptOrgUserCommand;
private readonly IAuthorizationService _authorizationService;
private readonly IOrganizationAbilityCacheService _organizationAbilityCacheService;
private readonly ISsoConfigRepository _ssoConfigRepository;
private readonly IOrganizationUserUserDetailsQuery _organizationUserUserDetailsQuery;
private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand;
Expand All @@ -91,6 +91,8 @@ public class OrganizationUsersController : BaseAdminConsoleController
private readonly IUpdateUserResetPasswordEnrollmentCommand _updateUserResetPasswordEnrollmentCommand;
private readonly IAcceptOrganizationInviteLinkCommand _acceptOrganizationInviteLinkCommand;
private readonly IConfirmOrganizationInviteLinkCommand _confirmOrganizationInviteLinkCommand;
private readonly Bitwarden.Server.Sdk.Features.IFeatureService _featureService;
private readonly V2_UpdateUserCommand.IUpdateOrganizationUserCommand _updateOrganizationUserCommandVNext;

public OrganizationUsersController(IOrganizationRepository organizationRepository,
IOrganizationUserRepository organizationUserRepository,
Expand All @@ -104,7 +106,6 @@ public OrganizationUsersController(IOrganizationRepository organizationRepositor
IUpdateOrganizationUserCommand updateOrganizationUserCommand,
IAcceptOrgUserCommand acceptOrgUserCommand,
IAuthorizationService authorizationService,
IOrganizationAbilityCacheService organizationAbilityCacheService,
ISsoConfigRepository ssoConfigRepository,
IOrganizationUserUserDetailsQuery organizationUserUserDetailsQuery,
IRemoveOrganizationUserCommand removeOrganizationUserCommand,
Expand All @@ -126,7 +127,9 @@ public OrganizationUsersController(IOrganizationRepository organizationRepositor
IUpdateUserResetPasswordEnrollmentCommand updateUserResetPasswordEnrollmentCommand,
IGetPendingAutoConfirmUsersQuery getPendingAutoConfirmUsersQuery,
IAcceptOrganizationInviteLinkCommand acceptOrganizationInviteLinkCommand,
IConfirmOrganizationInviteLinkCommand confirmOrganizationInviteLinkCommand)
IConfirmOrganizationInviteLinkCommand confirmOrganizationInviteLinkCommand,
Bitwarden.Server.Sdk.Features.IFeatureService featureService,
V2_UpdateUserCommand.IUpdateOrganizationUserCommand updateOrganizationUserCommandVNext)
{
_organizationRepository = organizationRepository;
_organizationUserRepository = organizationUserRepository;
Expand All @@ -140,7 +143,6 @@ public OrganizationUsersController(IOrganizationRepository organizationRepositor
_updateOrganizationUserCommand = updateOrganizationUserCommand;
_acceptOrgUserCommand = acceptOrgUserCommand;
_authorizationService = authorizationService;
_organizationAbilityCacheService = organizationAbilityCacheService;
_ssoConfigRepository = ssoConfigRepository;
_organizationUserUserDetailsQuery = organizationUserUserDetailsQuery;
_removeOrganizationUserCommand = removeOrganizationUserCommand;
Expand All @@ -163,6 +165,8 @@ public OrganizationUsersController(IOrganizationRepository organizationRepositor
_updateUserResetPasswordEnrollmentCommand = updateUserResetPasswordEnrollmentCommand;
_acceptOrganizationInviteLinkCommand = acceptOrganizationInviteLinkCommand;
_confirmOrganizationInviteLinkCommand = confirmOrganizationInviteLinkCommand;
_featureService = featureService;
_updateOrganizationUserCommandVNext = updateOrganizationUserCommandVNext;
}

[HttpGet("{id}")]
Expand Down Expand Up @@ -398,10 +402,11 @@ public async Task<ListResponseModel<OrganizationUserPublicKeyResponseModel>> Use

[HttpPut("{id}")]
[Authorize<ManageUsersRequirement>]
public async Task Put(Guid orgId, Guid id, [FromBody] OrganizationUserUpdateRequestModel model)
public async Task<IResult> Put([BindOrganization] Organization organization, Guid id, [FromBody] OrganizationUserUpdateRequestModel model)
Comment thread
jrmccannon marked this conversation as resolved.
{
var (organizationUser, currentAccess) = await _organizationUserRepository.GetByIdWithCollectionsAsync(id);
if (organizationUser == null || organizationUser.OrganizationId != orgId)

if (organizationUser == null || organizationUser.OrganizationId != organization.Id)
{
throw new NotFoundException();
}
Expand All @@ -412,72 +417,110 @@ public async Task Put(Guid orgId, Guid id, [FromBody] OrganizationUserUpdateRequ
// Authorization check:
// If admins are not allowed access to all collections, you cannot add yourself to a group.
// No error is thrown for this, we just don't update groups.
var organizationAbility = await _organizationAbilityCacheService.GetOrganizationAbilityAsync(orgId);
var groupsToSave = editingSelf && !organizationAbility.AllowAdminAccessToAllCollectionItems
var groupsToSave = editingSelf && !organization.AllowAdminAccessToAllCollectionItems
? null
: model.Groups;

var currentAccessIds = currentAccess.Select(c => c.Id).ToHashSet();
var existingUserType = organizationUser.Type;

if (_featureService.IsEnabled(FeatureFlagKeys.ChangeMemberEmailNoMp))
{
var (collectionsToSave, postedCollections) = await GetAuthorizedCollectionsToSaveAsync(model, currentAccess);

var actingContext = _currentContext.GetOrganization(organization.Id);

var request = new V2_UpdateUserCommand.UpdateOrganizationUserRequest(
organizationUser,
organization,
currentAccessIds,
postedCollections,
model.Type.Value,
Comment thread
jrmccannon marked this conversation as resolved.
model.Permissions,
model.AccessSecretsManager,
collectionsToSave,
groupsToSave,
new StandardUser(
userId,
await _currentContext.OrganizationOwner(organization.Id),
actingContext?.Type,
actingContext?.Permissions),
model.DefaultUserCollectionName);

var result = await _updateOrganizationUserCommandVNext.UpdateUserAsync(request);
return Handle(result);
Comment thread
jrmccannon marked this conversation as resolved.
}

// Authorization check:
// If admins are not allowed access to all collections, you cannot add yourself to collections.
// This is not caught by the requirement below that you can ModifyUserAccess and must be checked separately
var currentAccessIds = currentAccess.Select(c => c.Id).ToHashSet();
if (editingSelf &&
!organizationAbility.AllowAdminAccessToAllCollectionItems &&
!organization.AllowAdminAccessToAllCollectionItems &&
model.Collections.Any(c => !currentAccessIds.Contains(c.Id)))
{
throw new BadRequestException("You cannot add yourself to a collection.");
}

var (collectionsToSaveV1, _) = await GetAuthorizedCollectionsToSaveAsync(model, currentAccess);

await _updateOrganizationUserCommand.UpdateUserAsync(model.ToOrganizationUser(organizationUser), existingUserType, userId,
collectionsToSaveV1, groupsToSave, model.DefaultUserCollectionName);

return TypedResults.Ok();
}

/// <summary>
/// Resolves the collection access to persist for an organization user update. Collections the saving user
/// cannot <see cref="BulkCollectionOperations.ModifyUserAccess"/> are validated and the user's current
/// access to them is preserved so it is not accidentally overwritten.
/// </summary>
private async Task<(List<CollectionAccessSelection> CollectionsToSave, ICollection<Collection> Collections)> GetAuthorizedCollectionsToSaveAsync(OrganizationUserUpdateRequestModel model, ICollection<CollectionAccessSelection> currentAccess)
{
// Authorization check:
// You must have authorization to ModifyUserAccess for all collections being saved
var postedCollections = await _collectionRepository
.GetManyByManyIdsAsync(model.Collections.Select(c => c.Id));
foreach (var collection in postedCollections)
{
if (!(await _authorizationService.AuthorizeAsync(User, collection,
BulkCollectionOperations.ModifyUserAccess))
.Succeeded)
{
throw new NotFoundException();
}
// You must have authorization to ModifyUserAccess for all collections being saved.
var postedCollections = await _collectionRepository.GetManyByManyIdsAsync(model.Collections.Select(c => c.Id));
if (postedCollections.Count != 0 &&
!(await _authorizationService.AuthorizeAsync(User, postedCollections, BulkCollectionOperations.ModifyUserAccess)).Succeeded)
{
throw new NotFoundException();
}

// The client only sends collections that the saving user has permissions to edit.
// We need to combine these with collections that the user doesn't have permissions for, so that we don't
// accidentally overwrite those
var currentCollections = await _collectionRepository
.GetManyByManyIdsAsync(currentAccess.Select(cas => cas.Id));
var currentCollections = await _collectionRepository.GetManyByManyIdsAsync(currentAccess.Select(cas => cas.Id));

var readonlyCollectionIds = new HashSet<Guid>();

foreach (var collection in currentCollections)
{
if (!(await _authorizationService.AuthorizeAsync(User, collection, BulkCollectionOperations.ModifyUserAccess))
.Succeeded)
if (!(await _authorizationService.AuthorizeAsync(User, collection, BulkCollectionOperations.ModifyUserAccess)).Succeeded)
{
readonlyCollectionIds.Add(collection.Id);
}
}

var editedCollectionAccess = model.Collections
.Select(c => c.ToSelectionReadOnly());
// Default user collections ("My Items") are never managed through this flow. Exclude any current-access
// defaults from the preserved set so they are not re-saved; posted defaults are rejected downstream.
var defaultCollectionIds = postedCollections
.Concat(currentCollections)
.Where(c => c.Type == CollectionType.DefaultUserCollection)
.Select(c => c.Id)
.ToHashSet();
Comment on lines +503 to +509

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have very low confidence in this existing code so I don't want to modify it here if we can help it. I think this will break, because an admin never has write access to a My Items collection, yet it'll be excluded from readonlyCollectionAccess which needs to be added back in to the set, which means the user will lose access to it. But regardless; if there is no documented defect here let's leave it.

Moving it into a private method makes sense (for re-use between v1 and v2 flows) but the guts of it shouldn't change.


var editedCollectionAccess = model.Collections.Select(c => c.ToSelectionReadOnly());
var readonlyCollectionAccess = currentAccess
.Where(ca => readonlyCollectionIds.Contains(ca.Id));
.Where(ca => readonlyCollectionIds.Contains(ca.Id) && !defaultCollectionIds.Contains(ca.Id));
var collectionsToSave = editedCollectionAccess
.Concat(readonlyCollectionAccess)
.ToList();

var existingUserType = organizationUser.Type;

await _updateOrganizationUserCommand.UpdateUserAsync(model.ToOrganizationUser(organizationUser), existingUserType, userId,
collectionsToSave, groupsToSave, model.DefaultUserCollectionName);
}
var collections = postedCollections
.Concat(currentCollections)
.DistinctBy(c => c.Id)
.ToList();

[HttpPost("{id}")]
[Obsolete("This endpoint is deprecated. Use PUT method instead")]
[Authorize<ManageUsersRequirement>]
public async Task PostPut(Guid orgId, Guid id, [FromBody] OrganizationUserUpdateRequestModel model)
{
await Put(orgId, id, model);
return (collectionsToSave, collections);
}

[HttpPut("{userId}/reset-password-enrollment")]
Expand Down
20 changes: 17 additions & 3 deletions src/Core/AdminConsole/Entities/OrganizationUser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,19 @@
using Bit.Core.AdminConsole.Entities;
using Bit.Core.AdminConsole.Enums;
using Bit.Core.AdminConsole.Interfaces;
using Bit.Core.AdminConsole.Models.Data;
using Bit.Core.Enums;
using Bit.Core.Models;
using Bit.Core.Models.Data;
using Bit.Core.Utilities;

#nullable enable

namespace Bit.Core.Entities;

/// <summary>
/// An association table between one <see cref="User"/> and one <see cref="Organization"/>, representing that user's
/// membership in the organization. "Member" refers to the OrganizationUser object.
/// </summary>
public class OrganizationUser : ITableObject<Guid>, IExternal, IOrganizationUser
public class OrganizationUser : ITableObject<Guid>, IExternal, IOrganizationUser, IOrganizationUserRole
{
/// <summary>
/// A unique random identifier.
Expand Down Expand Up @@ -153,4 +152,19 @@ public void SetPermissions(Permissions permissions)
{
Permissions = CoreHelpers.ClassToJsonData(permissions);
}

public OrganizationUser UpdateOrganizationUser(OrganizationUserType organizationUserType,
Permissions? permissions,
bool accessSecretsManager,
TimeProvider timeProvider)
{
if (permissions is not null)
{
SetPermissions(permissions);
}
Type = organizationUserType;
AccessSecretsManager = accessSecretsManager;
RevisionDate = timeProvider.GetUtcNow().UtcDateTime;
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,9 @@ public class CollectionAccessSelection
/// deleting it, and assigning access for other users and groups.
/// </summary>
public bool Manage { get; set; }

/// <summary>
/// Manage is mutually exclusive with ReadOnly and HidePasswords.
/// </summary>
public bool Valid() => !(Manage && (ReadOnly || HidePasswords));
}
14 changes: 14 additions & 0 deletions src/Core/AdminConsole/Models/Data/IOrganizationUserRole.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
ο»Ώusing Bit.Core.Enums;
using Bit.Core.Models.Data;

namespace Bit.Core.AdminConsole.Models.Data;

/// <summary>
/// The minimal role information needed to authorize whether one member can manage another.
/// </summary>
public interface IOrganizationUserRole
{
OrganizationUserType Type { get; }
Guid OrganizationId { get; }
Permissions? GetPermissions();
}
15 changes: 15 additions & 0 deletions src/Core/AdminConsole/Models/Data/OrganizationUserRole.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
ο»Ώusing Bit.Core.Enums;
using Bit.Core.Models.Data;

namespace Bit.Core.AdminConsole.Models.Data;

/// <summary>
/// A lightweight <see cref="IOrganizationUserRole"/> for when a full OrganizationUser isn't available.
/// </summary>
public sealed record OrganizationUserRole(
OrganizationUserType Type,
Guid OrganizationId,
Permissions? Permissions = null) : IOrganizationUserRole
{
public Permissions? GetPermissions() => Permissions;
}
17 changes: 8 additions & 9 deletions src/Core/AdminConsole/Models/Data/StandardUser.cs
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
ο»Ώusing Bit.Core.Enums;
using Bit.Core.Models.Data;

namespace Bit.Core.AdminConsole.Models.Data;

public class StandardUser : IActingUser
public class StandardUser(Guid userId, bool isOrganizationOwner, OrganizationUserType? orgUserType = null,
Permissions? permissions = null) : IActingUser
{
public StandardUser(Guid userId, bool isOrganizationOwner)
{
UserId = userId;
IsOrganizationOwnerOrProvider = isOrganizationOwner;
}

public Guid? UserId { get; }
public bool IsOrganizationOwnerOrProvider { get; }
public Guid? UserId { get; } = userId;
public bool IsOrganizationOwnerOrProvider { get; } = isOrganizationOwner;
public OrganizationUserType? OrganizationUserType { get; } = orgUserType;
public Permissions? Permissions { get; } = permissions;
public EventSystemUser? SystemUserType => throw new Exception($"{nameof(StandardUser)} does not have a {nameof(SystemUserType)}");

}
9 changes: 2 additions & 7 deletions src/Core/AdminConsole/Models/Data/SystemUser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,10 @@

namespace Bit.Core.AdminConsole.Models.Data;

public class SystemUser : IActingUser
public class SystemUser(EventSystemUser systemUser) : IActingUser
{
public SystemUser(EventSystemUser systemUser)
{
SystemUserType = systemUser;
}

public Guid? UserId => throw new Exception($"{nameof(SystemUserType)} does not have a {nameof(UserId)}.");

public bool IsOrganizationOwnerOrProvider => false;
public EventSystemUser? SystemUserType { get; }
public EventSystemUser? SystemUserType { get; } = systemUser;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.OrganizationUserAction;

/// <summary>
/// Returned when the acting user lacks the authority to act on the target member's role.
/// </summary>
public record CannotManageTargetUser()
: BadRequestError("You do not have permission to manage this user.");
public record CannotManageTargetUser() : BadRequestError("You do not have permission to manage this user.");

public record OnlyOwnersCanManageOwners() : BadRequestError("Only an Owner can manage another Owner's account.");

public record CustomUsersCannotManageAdminsOrOwners() : BadRequestError("Custom users can not manage Admins or Owners.");

public record CustomUsersCanOnlyGrantOwnPermissions() : BadRequestError("Custom users can only grant the same custom permissions that they have.");

public record CannotBeAdminOfMultipleFreeOrganizations() : BadRequestError("User can only be an admin of one free organization.");
Loading
Loading