Skip to content

Sm prototype for manage permissions #7238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
maxkpower wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Sm prototype for manage permissions #7238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1f6705a
prototype for sm manage permissions
maxkpower Mar 17, 2026
955d290
prototype for sm manage permissions
maxkpower Mar 17, 2026
0f32728
tighten SM manage permission checks and fix edge cases
maxkpower Mar 17, 2026
7c0120a
address manage permission issues
maxkpower Mar 18, 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
60 changes: 43 additions & 17 deletions ...r/AuthorizationHandlers/AccessPolicies/ProjectPeopleAccessPoliciesAuthorizationHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Bit.Core.Context;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.SecretsManager.AuthorizationRequirements;
using Bit.Core.SecretsManager.Models.Data;
using Bit.Core.SecretsManager.Queries.AccessPolicies.Interfaces;
Expand All @@ -15,19 +16,22 @@ public class
ProjectPeopleAccessPolicies>
{
private readonly IAccessClientQuery _accessClientQuery;
private readonly IAccessPolicyRepository _accessPolicyRepository;
private readonly ICurrentContext _currentContext;
private readonly IProjectRepository _projectRepository;
private readonly ISameOrganizationQuery _sameOrganizationQuery;

public ProjectPeopleAccessPoliciesAuthorizationHandler(ICurrentContext currentContext,
IAccessClientQuery accessClientQuery,
ISameOrganizationQuery sameOrganizationQuery,
IProjectRepository projectRepository)
IProjectRepository projectRepository,
IAccessPolicyRepository accessPolicyRepository)
{
_currentContext = currentContext;
_accessClientQuery = accessClientQuery;
_sameOrganizationQuery = sameOrganizationQuery;
_projectRepository = projectRepository;
_accessPolicyRepository = accessPolicyRepository;
}

protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context,
Expand All @@ -39,13 +43,8 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext
return;
}

// Only users and admins should be able to manipulate access policies
var (accessClient, userId) =
await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId);
if (accessClient != AccessClientType.User && accessClient != AccessClientType.NoAccessCheck)
{
return;
}

switch (requirement)
{
Expand All @@ -63,27 +62,54 @@ private async Task CanReplaceProjectPeopleAsync(AuthorizationHandlerContext cont
AccessClientType accessClient, Guid userId)
{
var access = await _projectRepository.AccessToProjectAsync(resource.Id, userId, accessClient);
if (access.Write)
if (!access.Manage)
{
if (resource.UserAccessPolicies != null && resource.UserAccessPolicies.Any())
return;
}

var newUserManageCount = resource.UserAccessPolicies?.Count(ap => ap.Manage) ?? 0;
var newGroupManageCount = resource.GroupAccessPolicies?.Count(ap => ap.Manage) ?? 0;
if (newUserManageCount + newGroupManageCount == 0)
{
var currentPolicies = await _accessPolicyRepository.GetPeoplePoliciesByGrantedProjectIdAsync(resource.Id, userId);
if (currentPolicies.Any(ap => ap.Manage))
{
var orgUserIds = resource.UserAccessPolicies.Select(ap => ap.OrganizationUserId!.Value).ToList();
if (!await _sameOrganizationQuery.OrgUsersInTheSameOrgAsync(orgUserIds, resource.OrganizationId))
{
return;
}
throw new BadRequestException("At least one user or group must retain Manage permission on this project.");
}
}

if (resource.GroupAccessPolicies != null && resource.GroupAccessPolicies.Any())
if (accessClient == AccessClientType.ServiceAccount)
{
var hasManageGrant = (resource.UserAccessPolicies?.Any(ap => ap.Manage) ?? false) ||
(resource.GroupAccessPolicies?.Any(ap => ap.Manage) ?? false);
if (hasManageGrant)
{
var groupIds = resource.GroupAccessPolicies.Select(ap => ap.GroupId!.Value).ToList();
if (!await _sameOrganizationQuery.GroupsInTheSameOrgAsync(groupIds, resource.OrganizationId))
var creatorId = await _projectRepository.GetProjectCreatorServiceAccountIdAsync(resource.Id);
if (creatorId != userId)
{
return;
}
}
}

context.Succeed(requirement);
if (resource.UserAccessPolicies != null && resource.UserAccessPolicies.Any())
{
var orgUserIds = resource.UserAccessPolicies.Select(ap => ap.OrganizationUserId!.Value).ToList();
if (!await _sameOrganizationQuery.OrgUsersInTheSameOrgAsync(orgUserIds, resource.OrganizationId))
{
return;
}
}

if (resource.GroupAccessPolicies != null && resource.GroupAccessPolicies.Any())
{
var groupIds = resource.GroupAccessPolicies.Select(ap => ap.GroupId!.Value).ToList();
if (!await _sameOrganizationQuery.GroupsInTheSameOrgAsync(groupIds, resource.OrganizationId))
{
return;
}
}

context.Succeed(requirement);
}
}
34 changes: 26 additions & 8 deletions ...zationHandlers/AccessPolicies/ProjectServiceAccountsAccessPoliciesAuthorizationHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,8 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext
return;
}

// Only users and admins should be able to manipulate access policies
var (accessClient, userId) =
await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId);
if (accessClient != AccessClientType.User && accessClient != AccessClientType.NoAccessCheck)
{
return;
}

switch (requirement)
{
Expand All @@ -67,11 +62,34 @@ private async Task CanUpdateAsync(AuthorizationHandlerContext context,
var access =
await _projectRepository.AccessToProjectAsync(resource.ProjectId, userId,
accessClient);
if (!access.Write)
if (!access.Manage)
{
return;
}

if (accessClient == AccessClientType.ServiceAccount)
{
var hasManageGrant = resource.ServiceAccountAccessPolicyUpdates
.Any(u => (u.Operation == AccessPolicyOperation.Create || u.Operation == AccessPolicyOperation.Update)
&& u.AccessPolicy.Manage);
if (hasManageGrant)
{
var creatorId = await _projectRepository.GetProjectCreatorServiceAccountIdAsync(resource.ProjectId);
if (creatorId != userId)
{
return;
}

if (resource.ServiceAccountAccessPolicyUpdates.Any(u =>
(u.Operation == AccessPolicyOperation.Create || u.Operation == AccessPolicyOperation.Update) &&
u.AccessPolicy.Manage &&
u.AccessPolicy.ServiceAccountId != userId))
{
return;
}
}
}

var serviceAccountIds = resource.ServiceAccountAccessPolicyUpdates.Select(update =>
update.AccessPolicy.ServiceAccountId!.Value).ToList();

Expand All @@ -84,7 +102,7 @@ await _serviceAccountRepository.ServiceAccountsAreInOrganizationAsync(serviceAcc
}

// Users can only create access policies for service accounts they have access to.
// User can delete and update any service account access policy if they have write access to the project.
// Users can delete and update any service account access policy if they have Manage access to the project.
var serviceAccountIdsToCheck = resource.ServiceAccountAccessPolicyUpdates
.Where(update => update.Operation == AccessPolicyOperation.Create).Select(update =>
update.AccessPolicy.ServiceAccountId!.Value).ToList();
Expand All @@ -99,7 +117,7 @@ await _serviceAccountRepository.ServiceAccountsAreInOrganizationAsync(serviceAcc
await _serviceAccountRepository.AccessToServiceAccountsAsync(serviceAccountIdsToCheck, userId,
accessClient);
if (serviceAccountsAccess.Count == serviceAccountIdsToCheck.Count &&
serviceAccountsAccess.All(a => a.Value.Write))
serviceAccountsAccess.All(a => a.Value.Manage))
{
context.Succeed(requirement);
}
Expand Down
55 changes: 46 additions & 9 deletions ...r/AuthorizationHandlers/AccessPolicies/SecretAccessPoliciesUpdatesAuthorizationHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public class SecretAccessPoliciesUpdatesAuthorizationHandler : AuthorizationHand
{
private readonly IAccessClientQuery _accessClientQuery;
private readonly ICurrentContext _currentContext;
private readonly IProjectRepository _projectRepository;
private readonly ISameOrganizationQuery _sameOrganizationQuery;
private readonly ISecretRepository _secretRepository;
private readonly IServiceAccountRepository _serviceAccountRepository;
Expand All @@ -25,13 +26,15 @@ public SecretAccessPoliciesUpdatesAuthorizationHandler(ICurrentContext currentCo
IAccessClientQuery accessClientQuery,
ISecretRepository secretRepository,
ISameOrganizationQuery sameOrganizationQuery,
IServiceAccountRepository serviceAccountRepository)
IServiceAccountRepository serviceAccountRepository,
IProjectRepository projectRepository)
{
_currentContext = currentContext;
_accessClientQuery = accessClientQuery;
_sameOrganizationQuery = sameOrganizationQuery;
_serviceAccountRepository = serviceAccountRepository;
_secretRepository = secretRepository;
_projectRepository = projectRepository;
}

protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context,
Expand All @@ -43,13 +46,8 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext
return;
}

// Only users and admins should be able to manipulate access policies
var (accessClient, userId) =
await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId);
if (accessClient != AccessClientType.User && accessClient != AccessClientType.NoAccessCheck)
{
return;
}

switch (requirement)
{
Expand All @@ -74,18 +72,36 @@ private async Task CanUpdateAsync(AuthorizationHandlerContext context,
{
var access = await _secretRepository
.AccessToSecretAsync(resource.SecretId, userId, accessClient);
if (!access.Write)
if (!access.Manage)
{
return;
}

if (accessClient == AccessClientType.ServiceAccount)
{
var hasManageGrant =
resource.UserAccessPolicyUpdates.Any(u =>
(u.Operation == AccessPolicyOperation.Create || u.Operation == AccessPolicyOperation.Update) && u.AccessPolicy.Manage) ||
resource.GroupAccessPolicyUpdates.Any(u =>
(u.Operation == AccessPolicyOperation.Create || u.Operation == AccessPolicyOperation.Update) && u.AccessPolicy.Manage) ||
resource.ServiceAccountAccessPolicyUpdates.Any(u =>
(u.Operation == AccessPolicyOperation.Create || u.Operation == AccessPolicyOperation.Update) && u.AccessPolicy.Manage);
if (hasManageGrant)
{
if (!await _projectRepository.IsServiceAccountCreatorOfAnyProjectForSecretAsync(resource.SecretId, userId))
{
return;
}
}
}

if (!await GranteesInTheSameOrganizationAsync(resource))
{
return;
}

// Users can only create access policies for service accounts they have access to.
// User can delete and update any service account access policy if they have write access to the secret.
// User can delete and update any service account access policy if they have Manage access to the secret.
if (await HasAccessToTargetServiceAccountsAsync(resource, accessClient, userId))
{
context.Succeed(requirement);
Expand All @@ -97,13 +113,34 @@ private async Task CanCreateAsync(AuthorizationHandlerContext context,
SecretAccessPoliciesUpdates resource,
AccessClientType accessClient, Guid userId)
{
var access = await _secretRepository.AccessToSecretAsync(resource.SecretId, userId, accessClient);
if (!access.Manage)
{
return;
}

if (resource.UserAccessPolicyUpdates.Any(x => x.Operation != AccessPolicyOperation.Create) ||
resource.GroupAccessPolicyUpdates.Any(x => x.Operation != AccessPolicyOperation.Create) ||
resource.ServiceAccountAccessPolicyUpdates.Any(x => x.Operation != AccessPolicyOperation.Create))
{
return;
}

if (accessClient == AccessClientType.ServiceAccount)
{
var hasManageGrant =
resource.UserAccessPolicyUpdates.Any(u => u.AccessPolicy.Manage) ||
resource.GroupAccessPolicyUpdates.Any(u => u.AccessPolicy.Manage) ||
resource.ServiceAccountAccessPolicyUpdates.Any(u => u.AccessPolicy.Manage);
if (hasManageGrant)
{
if (!await _projectRepository.IsServiceAccountCreatorOfAnyProjectForSecretAsync(resource.SecretId, userId))
{
return;
}
}
}

if (!await GranteesInTheSameOrganizationAsync(resource))
{
return;
Expand Down Expand Up @@ -157,6 +194,6 @@ await _serviceAccountRepository.AccessToServiceAccountsAsync(serviceAccountIdsTo
accessClient);

return serviceAccountsAccess.Count == serviceAccountIdsToCheck.Count &&
serviceAccountsAccess.All(a => a.Value.Write);
serviceAccountsAccess.All(a => a.Value.Manage);
}
}
9 changes: 2 additions & 7 deletions ...AuthorizationHandlers/AccessPolicies/ServiceAccountGrantedPoliciesAuthorizationHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,8 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext
return;
}

// Only users and admins should be able to manipulate access policies
var (accessClient, userId) =
await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId);
if (accessClient != AccessClientType.User && accessClient != AccessClientType.NoAccessCheck)
{
return;
}

switch (requirement)
{
Expand All @@ -65,7 +60,7 @@ private async Task CanUpdateAsync(AuthorizationHandlerContext context,
var access =
await _serviceAccountRepository.AccessToServiceAccountAsync(resource.ServiceAccountId, userId,
accessClient);
if (access.Write)
if (access.Manage)
{
var projectIdsToCheck = resource.ProjectGrantedPolicyUpdates.Select(update =>
update.AccessPolicy.GrantedProjectId!.Value).ToList();
Expand All @@ -79,7 +74,7 @@ await _serviceAccountRepository.AccessToServiceAccountAsync(resource.ServiceAcco

var projectsAccess =
await _projectRepository.AccessToProjectsAsync(projectIdsToCheck, userId, accessClient);
if (projectsAccess.Count == projectIdsToCheck.Count && projectsAccess.All(a => a.Value.Write))
if (projectsAccess.Count == projectIdsToCheck.Count && projectsAccess.All(a => a.Value.Manage))
{
context.Succeed(requirement);
}
Expand Down
11 changes: 9 additions & 2 deletions ...rizationHandlers/AccessPolicies/ServiceAccountPeopleAccessPoliciesAuthorizationHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,16 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext
return;
}

// Only users and admins should be able to manipulate access policies
var (accessClient, userId) =
await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId);

// Policy decision: service accounts cannot manage human grantees on service accounts.
// This is intentional and must not be removed without security review.
if (accessClient == AccessClientType.ServiceAccount)
{
return;
}

if (accessClient != AccessClientType.User && accessClient != AccessClientType.NoAccessCheck)
{
return;
Expand All @@ -63,7 +70,7 @@ private async Task CanReplaceServiceAccountPeopleAsync(AuthorizationHandlerConte
AccessClientType accessClient, Guid userId)
{
var access = await _serviceAccountRepository.AccessToServiceAccountAsync(resource.Id, userId, accessClient);
if (access.Write)
if (access.Manage)
{
if (resource.UserAccessPolicies != null && resource.UserAccessPolicies.Any())
{
Expand Down
8 changes: 1 addition & 7 deletions ...ommercial.Core/SecretsManager/AuthorizationHandlers/Secrets/SecretAuthorizationHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,9 @@ private async Task CanReadAccessPoliciesAsync(AuthorizationHandlerContext contex
{
var (accessClient, userId) = await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId);

// Only users and admins can read access policies
if (accessClient != AccessClientType.User && accessClient != AccessClientType.NoAccessCheck)
{
return;
}

var access = await _secretRepository.AccessToSecretAsync(resource.Id, userId, accessClient);

if (access.Write)
if (access.Manage)
{
context.Succeed(requirement);
}
Expand Down
Loading
Loading