Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 33 additions & 0 deletions src/Api/AdminConsole/Controllers/OrganizationUsersController.cs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎨 Can we update these string too

Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ public async Task<IResult> Put([BindOrganization] Organization organization, Gui
model.Type.Value,
model.Permissions,
model.AccessSecretsManager,
model.AccessPam,
collectionAccessToSave,
groupsToSave,
model.Email,
Expand Down Expand Up @@ -799,6 +800,38 @@ public async Task BulkEnableSecretsManagerAsync(Guid orgId,
await _organizationUserRepository.ReplaceManyAsync(orgUsers);
}

/// <summary>
/// Grants PAM access to the specified members. A plain field write: PAM has no seats, so there is no
/// autoscale or billing step. Members who already have access are skipped.
/// </summary>
[HttpPut("enable-pam")]
[Authorize<ManageUsersRequirement>]
public async Task BulkEnablePamAsync(Guid orgId,
[FromBody] OrganizationUserBulkRequestModel model)
{
var orgUsers = (await _organizationUserRepository.GetManyAsync(model.Ids))
.Where(ou => ou.OrganizationId == orgId && !ou.AccessPam).ToList();
if (orgUsers.Count == 0)
{
throw new BadRequestException("Users invalid.");
}

// Granting access on an organization without PAM would be inert: claim emission ANDs AccessPam with the
// organization's UsePam.
var organization = await _organizationRepository.GetByIdAsync(orgId);
if (organization is not { UsePam: true })
{
throw new BadRequestException("To grant PAM access the organization must have PAM enabled.");
}

foreach (var orgUser in orgUsers)
{
orgUser.AccessPam = true;
}

await _organizationUserRepository.ReplaceManyAsync(orgUsers);
}

[HttpPost("{id}/auto-confirm")]
[Authorize<ManageUsersRequirement>]
public async Task<IResult> AutomaticallyConfirmOrganizationUserAsync([FromRoute] Guid orgId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ public class OrganizationUserUpdateRequestModel
[EnumDataType(typeof(OrganizationUserType))]
public OrganizationUserType? Type { get; set; }
public bool AccessSecretsManager { get; set; }
public bool AccessPam { get; set; }
public Permissions Permissions { get; set; }
public IEnumerable<SelectionReadOnlyRequestModel> Collections { get; set; }
public IEnumerable<Guid> Groups { get; set; }
Expand All @@ -118,6 +119,7 @@ public OrganizationUser ToOrganizationUser(OrganizationUser existingUser)
existingUser.Type = Type.Value;
existingUser.Permissions = CoreHelpers.ClassToJsonData(Permissions);
existingUser.AccessSecretsManager = AccessSecretsManager;
existingUser.AccessPam = AccessPam;
return existingUser;
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/Core/AdminConsole/Entities/OrganizationUser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ public void SetPermissions(Permissions permissions)
public OrganizationUser UpdateOrganizationUser(OrganizationUserType organizationUserType,
Permissions? permissions,
bool accessSecretsManager,
bool accessPam,
TimeProvider timeProvider)
{
if (permissions is not null)
Expand All @@ -168,6 +169,7 @@ public OrganizationUser UpdateOrganizationUser(OrganizationUserType organization
}
Type = organizationUserType;
AccessSecretsManager = accessSecretsManager;
AccessPam = accessPam;
RevisionDate = timeProvider.GetUtcNow().UtcDateTime;
return this;
}
Expand Down

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎨 Can we update the hardcoded error strings to follow the pattern in src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateUser/v2/Errors.cs

Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,14 @@ public async Task UpdateUserAsync(OrganizationUser organizationUser, Organizatio
}
}

// Granting PAM access to a member of an organization without PAM would be inert: claim emission ANDs
// AccessPam with the organization's UsePam. Reject so the admin gets an actionable error instead.
// Only the grant is gated β€” revoking access stays possible on an organization whose entitlement has lapsed.
if (!originalOrganizationUser.AccessPam && organizationUser.AccessPam && !organization.UsePam)
{
throw new BadRequestException("To grant PAM access the organization must have PAM enabled.");
}
Comment on lines +136 to +142

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I guess we could technically skip this check as enabling pam for a user without it being enabled in the org does nothing. I'll leave it up to AC to decide if it's worth keeping. It would eventually be replaced by the billing seat logic.


// Only autoscale (if required) after all validation has passed so that we know it's a valid request before
// updating Stripe
if (!originalOrganizationUser.AccessSecretsManager && organizationUser.AccessSecretsManager)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public record MustHaveConfirmedOwner() : BadRequestError("Organization must have
public record CustomPermissionsNotEnabled() : BadRequestError("To enable custom permissions the organization must be on an Enterprise plan.");
public record CannotAssignDefaultCollection() : BadRequestError("Default collections cannot be assigned to a member.");
public record CannotAutoscaleSecretsManagerSeatsOnSelfHost() : BadRequestError("Cannot autoscale on a self-hosted instance.");
public record PamNotEnabled() : BadRequestError("To grant PAM access the organization must have PAM enabled.");
public record CouldNotIncreaseSeatsOfSecretManager(string Message) : BadRequestError(Message);

public abstract record EmailValidationError(string Message, string Type) : BadRequestError(Message), IValidationError
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public async Task<CommandResult> UpdateUserAsync(UpdateOrganizationUserRequest r
var organizationUser = request.OrganizationUserToUpdate.UpdateOrganizationUser(request.NewType,
request.NewPermissions,
request.NewAccessSecretsManager,
request.NewAccessPam,
timeProvider);

if (request.IsEnablingSecretsManager())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public record UpdateOrganizationUserRequest(
OrganizationUserType NewType,
Permissions? NewPermissions,
bool NewAccessSecretsManager,
bool NewAccessPam,
List<CollectionAccessSelection>? CollectionsToSave,
IEnumerable<Guid>? NewGroups,
string? NewEmail,
Expand All @@ -37,6 +38,12 @@ _existingOrganizationUserType is OrganizationUserType.Admin or OrganizationUserT

public bool IsEnablingSecretsManager() => !_existingAccessSecretsManager && NewAccessSecretsManager;

/// <summary>
/// Only a false β†’ true transition is a grant. Revoking access stays possible on an organization whose PAM
/// entitlement has lapsed, so <see cref="Organization.UsePam"/> is not checked when disabling.
/// </summary>
public bool IsEnablingPam() => !_existingAccessPam && NewAccessPam;

public bool IsEmailChanged() =>
!string.IsNullOrWhiteSpace(NewEmail)
&& UserToUpdate is not null
Expand All @@ -54,4 +61,5 @@ public bool IsNameChanged() =>

private readonly OrganizationUserType _existingOrganizationUserType = OrganizationUserToUpdate.Type;
private readonly bool _existingAccessSecretsManager = OrganizationUserToUpdate.AccessSecretsManager;
private readonly bool _existingAccessPam = OrganizationUserToUpdate.AccessPam;
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@ public async Task<ValidationResult<UpdateOrganizationUserRequest>> ValidateAsync
return Invalid(request, new CustomPermissionsNotEnabled());
}

// Granting PAM access to a member of an organization without PAM would be inert: claim emission ANDs
// AccessPam with the organization's UsePam. Reject so the admin gets an actionable error instead.
if (request.IsEnablingPam() && !request.Organization.UsePam)
{
return Invalid(request, new PamNotEnabled());
}

if (request.NewType != OrganizationUserType.Owner &&
!await hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(organizationUser.OrganizationId,
[organizationUser.Id]))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,103 @@ namespace Bit.Api.Test.AdminConsole.Controllers;
[SutProviderCustomize]
public class OrganizationUsersControllerTests
{
[Theory]
[BitAutoData]
public async Task BulkEnablePam_GrantsAccessToMembersWithoutIt(Guid orgId,
OrganizationUserBulkRequestModel model, Organization organization, List<OrganizationUser> orgUsers,
SutProvider<OrganizationUsersController> sutProvider)
{
organization.UsePam = true;
foreach (var orgUser in orgUsers)
{
orgUser.OrganizationId = orgId;
orgUser.AccessPam = false;
}

sutProvider.GetDependency<IOrganizationUserRepository>().GetManyAsync(model.Ids).Returns(orgUsers);
sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(orgId).Returns(organization);

await sutProvider.Sut.BulkEnablePamAsync(orgId, model);

await sutProvider.GetDependency<IOrganizationUserRepository>()
.Received(1)
.ReplaceManyAsync(Arg.Is<IEnumerable<OrganizationUser>>(users => users.All(u => u.AccessPam)));
}

[Theory]
[BitAutoData]
public async Task BulkEnablePam_SkipsMembersOfOtherOrganizationsAndThoseWithAccess(Guid orgId,
OrganizationUserBulkRequestModel model, Organization organization, OrganizationUser targetUser,
OrganizationUser alreadyEnabledUser, OrganizationUser otherOrgUser,
SutProvider<OrganizationUsersController> sutProvider)
{
organization.UsePam = true;
targetUser.OrganizationId = alreadyEnabledUser.OrganizationId = orgId;
targetUser.AccessPam = false;
alreadyEnabledUser.AccessPam = true;
otherOrgUser.AccessPam = false;

sutProvider.GetDependency<IOrganizationUserRepository>().GetManyAsync(model.Ids)
.Returns([targetUser, alreadyEnabledUser, otherOrgUser]);
sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(orgId).Returns(organization);

await sutProvider.Sut.BulkEnablePamAsync(orgId, model);

Assert.False(otherOrgUser.AccessPam);
await sutProvider.GetDependency<IOrganizationUserRepository>()
.Received(1)
.ReplaceManyAsync(Arg.Is<IEnumerable<OrganizationUser>>(users =>
users.Count() == 1 && users.Single().Id == targetUser.Id && users.Single().AccessPam));
}

[Theory]
[BitAutoData]
public async Task BulkEnablePam_WhenOrganizationDoesNotUsePam_Throws(Guid orgId,
OrganizationUserBulkRequestModel model, Organization organization, List<OrganizationUser> orgUsers,
SutProvider<OrganizationUsersController> sutProvider)
{
organization.UsePam = false;
foreach (var orgUser in orgUsers)
{
orgUser.OrganizationId = orgId;
orgUser.AccessPam = false;
}

sutProvider.GetDependency<IOrganizationUserRepository>().GetManyAsync(model.Ids).Returns(orgUsers);
sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(orgId).Returns(organization);

var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.BulkEnablePamAsync(orgId, model));

Assert.Contains("must have PAM enabled", exception.Message);
await sutProvider.GetDependency<IOrganizationUserRepository>()
.DidNotReceiveWithAnyArgs()
.ReplaceManyAsync(default);
}

[Theory]
[BitAutoData]
public async Task BulkEnablePam_WhenNoMembersNeedAccess_Throws(Guid orgId,
OrganizationUserBulkRequestModel model, List<OrganizationUser> orgUsers,
SutProvider<OrganizationUsersController> sutProvider)
{
foreach (var orgUser in orgUsers)
{
orgUser.OrganizationId = orgId;
orgUser.AccessPam = true;
}

sutProvider.GetDependency<IOrganizationUserRepository>().GetManyAsync(model.Ids).Returns(orgUsers);

var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.BulkEnablePamAsync(orgId, model));

Assert.Equal("Users invalid.", exception.Message);
await sutProvider.GetDependency<IOrganizationUserRepository>()
.DidNotReceiveWithAnyArgs()
.ReplaceManyAsync(default);
}

[Theory]
[BitAutoData]
public async Task PutResetPasswordEnrollment_InvitedUser_AcceptsInvite(Guid orgId, Guid userId, OrganizationUserResetPasswordEnrollmentRequestModel model,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,77 @@ await sutProvider.GetDependency<IHasConfirmedOwnersExceptQuery>().Received(1).Ha
Arg.Is<IEnumerable<Guid>>(i => i.Contains(newUserData.Id)));
}

[Theory, BitAutoData]
public async Task UpdateUserAsync_WhenGrantingPam_AndOrganizationDoesNotUsePam_Throws(
Organization organization,
OrganizationUser oldUserData,
OrganizationUser newUserData,
[OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser savingUser,
SutProvider<UpdateOrganizationUserCommand> sutProvider)
{
Setup(sutProvider, organization, newUserData, oldUserData);
organization.UsePam = false;
newUserData.Permissions = null;
oldUserData.AccessPam = false;
newUserData.AccessPam = true;
newUserData.Type = OrganizationUserType.User;

var exception = await Assert.ThrowsAsync<BadRequestException>(() =>
sutProvider.Sut.UpdateUserAsync(newUserData, OrganizationUserType.User, savingUser.UserId, null, null));

Assert.Contains("must have PAM enabled", exception.Message);
await sutProvider.GetDependency<IOrganizationUserRepository>()
.DidNotReceiveWithAnyArgs()
.ReplaceAsync(default, default(IEnumerable<CollectionAccessSelection>));
}

[Theory, BitAutoData]
public async Task UpdateUserAsync_WhenGrantingPam_AndOrganizationUsesPam_Persists(
Organization organization,
OrganizationUser oldUserData,
OrganizationUser newUserData,
[OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser savingUser,
SutProvider<UpdateOrganizationUserCommand> sutProvider)
{
Setup(sutProvider, organization, newUserData, oldUserData);
organization.UsePam = true;
newUserData.Permissions = null;
oldUserData.AccessPam = false;
newUserData.AccessPam = true;
newUserData.Type = OrganizationUserType.User;

await sutProvider.Sut.UpdateUserAsync(newUserData, OrganizationUserType.User, savingUser.UserId, null, null);

await sutProvider.GetDependency<IOrganizationUserRepository>()
.Received(1)
.ReplaceAsync(Arg.Is<OrganizationUser>(ou => ou.AccessPam),
Arg.Any<IEnumerable<CollectionAccessSelection>>());
}

[Theory, BitAutoData]
public async Task UpdateUserAsync_WhenRevokingPam_AndOrganizationDoesNotUsePam_Persists(
Organization organization,
OrganizationUser oldUserData,
OrganizationUser newUserData,
[OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser savingUser,
SutProvider<UpdateOrganizationUserCommand> sutProvider)
{
// Revoking access must stay possible on an organization whose PAM entitlement has lapsed.
Setup(sutProvider, organization, newUserData, oldUserData);
organization.UsePam = false;
newUserData.Permissions = null;
oldUserData.AccessPam = true;
newUserData.AccessPam = false;
newUserData.Type = OrganizationUserType.User;

await sutProvider.Sut.UpdateUserAsync(newUserData, OrganizationUserType.User, savingUser.UserId, null, null);

await sutProvider.GetDependency<IOrganizationUserRepository>()
.Received(1)
.ReplaceAsync(Arg.Is<OrganizationUser>(ou => !ou.AccessPam),
Arg.Any<IEnumerable<CollectionAccessSelection>>());
}

[Theory]
[BitAutoData(OrganizationUserType.Admin)]
[BitAutoData(OrganizationUserType.Owner)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,44 @@ await sutProvider.GetDependency<IEventService>()
.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Updated);
}

[Theory]
[BitAutoData]
public async Task UpdateUserAsync_WhenGrantingPam_PersistsAccessPam(
SutProvider<UpdateOrganizationUserCommand> sutProvider,
Organization organization,
[OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.User)] OrganizationUser organizationUser)
{
organizationUser.AccessPam = false;
var request = Setup(sutProvider, organization, organizationUser, targetAccessPam: true);

var result = await sutProvider.Sut.UpdateUserAsync(request);

Assert.True(result.IsSuccess);
await sutProvider.GetDependency<IOrganizationUserRepository>()
.Received(1)
.ReplaceAsync(Arg.Is<OrganizationUser>(ou => ou.AccessPam),
Arg.Any<IEnumerable<CollectionAccessSelection>>());
}

[Theory]
[BitAutoData]
public async Task UpdateUserAsync_WhenRevokingPam_PersistsAccessPamAsFalse(
SutProvider<UpdateOrganizationUserCommand> sutProvider,
Organization organization,
[OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.User)] OrganizationUser organizationUser)
{
organizationUser.AccessPam = true;
var request = Setup(sutProvider, organization, organizationUser, targetAccessPam: false);

var result = await sutProvider.Sut.UpdateUserAsync(request);

Assert.True(result.IsSuccess);
await sutProvider.GetDependency<IOrganizationUserRepository>()
.Received(1)
.ReplaceAsync(Arg.Is<OrganizationUser>(ou => !ou.AccessPam),
Arg.Any<IEnumerable<CollectionAccessSelection>>());
}

[Theory]
[BitAutoData]
public async Task UpdateUserAsync_WhenEmailChanged_NotifiesMemberAtPreviousEmail(
Expand Down Expand Up @@ -429,7 +467,8 @@ private static UpdateOrganizationUserRequest Setup(
bool targetAccessSecretsManager = false,
string defaultUserCollectionName = null,
string newEmail = null,
string newName = null)
string newName = null,
bool targetAccessPam = false)
{
organization.PlanType = PlanType.EnterpriseAnnually;
organizationUser.OrganizationId = organization.Id;
Expand All @@ -446,6 +485,7 @@ private static UpdateOrganizationUserRequest Setup(
type,
null,
targetAccessSecretsManager,
targetAccessPam,
collections,
groups,
newEmail,
Expand Down
Loading
Loading