Skip to content
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.

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

[SM-999] Add Bulk Move to Project Endpoint #66

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
using Bit.Core.Context;
using Bit.Core.Enums;
using Bit.Core.SecretsManager.AuthorizationRequirements;
using Bit.Core.SecretsManager.Entities;
using Bit.Core.SecretsManager.Queries.Interfaces;
using Bit.Core.SecretsManager.Repositories;
using Microsoft.AspNetCore.Authorization;

namespace Bit.Commercial.Core.SecretsManager.AuthorizationHandlers.Secrets;

public class BulkSecretAuthorizationHandler : AuthorizationHandler<BulkSecretOperationRequirement, IReadOnlyList<Secret>>
{
private readonly ICurrentContext _currentContext;
private readonly IAccessClientQuery _accessClientQuery;
private readonly ISecretRepository _secretRepository;

public BulkSecretAuthorizationHandler(
ICurrentContext currentContext,
IAccessClientQuery accessClientQuery,
ISecretRepository secretRepository)
{
_currentContext = currentContext;
_accessClientQuery = accessClientQuery;
_secretRepository = secretRepository;
}

protected override async Task HandleRequirementAsync(
AuthorizationHandlerContext context,
BulkSecretOperationRequirement requirement,
IReadOnlyList<Secret> resource)
{
var secretsByOrganizationId = resource.GroupBy(s => s.OrganizationId).ToArray();
Copy link

Choose a reason for hiding this comment

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

style: Consider using FirstOrDefault() instead of ToArray() for better performance when only checking for a single group.


// All the secrets should be part of a single organization
if (secretsByOrganizationId.Length != 1)
{
return;
}

var organizationId = secretsByOrganizationId[0].Key;

if (!_currentContext.AccessSecretsManager(organizationId))
{
return;
}

var (accessClient, userId) = await _accessClientQuery.GetAccessClientAsync(context.User, organizationId);

if (requirement == BulkSecretOperations.Update)
{
if (!await CanBulkUpdateSecretsAsync(resource, accessClient, userId))
{
return;
}

context.Succeed(requirement);
}
}

private async Task<bool> CanBulkUpdateSecretsAsync(
IReadOnlyList<Secret> secrets,
AccessClientType accessClientType,
Guid userId)
{
var secretAccesses = await _secretRepository.AccessToSecretsAsync(
secrets.Select(s => s.Id).ToArray(), userId, accessClientType);
Comment on lines +65 to +66
Copy link

Choose a reason for hiding this comment

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

style: This could potentially be a performance bottleneck for large numbers of secrets. Consider implementing a batch operation in the repository.


// If we don't have the write permission
return secretAccesses.All(a => a.Value.Write);
Copy link

Choose a reason for hiding this comment

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

logic: Ensure that secretAccesses contains an entry for every secret, otherwise this check might pass incorrectly.

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
using Bit.Core.SecretsManager.Commands.Secrets.Interfaces;
using Bit.Core.SecretsManager.Entities;
using Bit.Core.SecretsManager.Repositories;

namespace Bit.Commercial.Core.SecretsManager.Commands.Secrets;

public class MoveSecretsCommand : IMoveSecretsCommand
{
private readonly ISecretRepository _secretRepository;

public MoveSecretsCommand(ISecretRepository secretRepository)
{
_secretRepository = secretRepository;
}

public async Task MoveSecretsAsync(IEnumerable<Secret> secrets, Guid project)
{
await _secretRepository.MoveSecretsAsync(secrets, project);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public static void AddSecretsManagerServices(this IServiceCollection services)
{
services.AddScoped<IAuthorizationHandler, ProjectAuthorizationHandler>();
services.AddScoped<IAuthorizationHandler, SecretAuthorizationHandler>();
services.AddScoped<IAuthorizationHandler, BulkSecretAuthorizationHandler>();
services.AddScoped<IAuthorizationHandler, ServiceAccountAuthorizationHandler>();
services.AddScoped<IAuthorizationHandler, AccessPolicyAuthorizationHandler>();
services.AddScoped<IAuthorizationHandler, ProjectPeopleAccessPoliciesAuthorizationHandler>();
Expand All @@ -61,5 +62,6 @@ public static void AddSecretsManagerServices(this IServiceCollection services)
services.AddScoped<IImportCommand, ImportCommand>();
services.AddScoped<IEmptyTrashCommand, EmptyTrashCommand>();
services.AddScoped<IRestoreTrashCommand, RestoreTrashCommand>();
services.AddScoped<IMoveSecretsCommand, MoveSecretsCommand>();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -274,35 +274,58 @@ await secrets.ForEachAsync(secret =>
var secret = dbContext.Secret
.Where(s => s.Id == id);

var query = accessType switch
var query = BuildSecretAccessQuery(secret, userId, accessType);

var policy = await query.FirstOrDefaultAsync();

return policy == null ? (false, false) : (policy.Read, policy.Write);
}

public async Task<Dictionary<Guid, (bool Read, bool Write)>> AccessToSecretsAsync(
IEnumerable<Guid> secretIds,
Guid userId,
AccessClientType accessType)
{
await using var scope = ServiceScopeFactory.CreateAsyncScope();
Copy link

Choose a reason for hiding this comment

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

style: Use 'using' instead of 'await using' for consistency with other methods

var dbContext = GetDatabaseContext(scope);

var secrets = dbContext.Secret
.Where(s => secretIds.Contains(s.Id));

var accessQuery = BuildSecretAccessQuery(secrets, userId, accessType);

return await accessQuery.ToDictionaryAsync(sa => sa.Id, sa => (sa.Read, sa.Write));
}

private static IQueryable<SecretAccess> BuildSecretAccessQuery(IQueryable<Secret> secrets, Guid userId, AccessClientType accessType)
Copy link

Choose a reason for hiding this comment

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

style: Consider adding XML documentation for this method to explain its purpose and parameters

{
return accessType switch
{
AccessClientType.NoAccessCheck => secret.Select(_ => new { Read = true, Write = true }),
AccessClientType.User => secret.Select(s => new
{
Read = s.Projects.Any(p =>
AccessClientType.NoAccessCheck => secrets.Select(s => new SecretAccess(s.Id, true, true)),
AccessClientType.User => secrets.Select(s => new SecretAccess(
s.Id,
s.Projects.Any(p =>
p.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Read) ||
p.GroupAccessPolicies.Any(ap =>
ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Read))),
Write = s.Projects.Any(p =>
s.Projects.Any(p =>
p.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Write) ||
p.GroupAccessPolicies.Any(ap =>
ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Write))),
}),
AccessClientType.ServiceAccount => secret.Select(s => new
{
Read = s.Projects.Any(p =>
ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Write)))
)),
AccessClientType.ServiceAccount => secrets.Select(s => new SecretAccess(
s.Id,
s.Projects.Any(p =>
p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == userId && ap.Read)),
Write = s.Projects.Any(p =>
p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == userId && ap.Write)),
}),
_ => secret.Select(_ => new { Read = false, Write = false }),
s.Projects.Any(p =>
p.ServiceAccountAccessPolicies.Any(ap => ap.ServiceAccountId == userId && ap.Write))
)),
_ => secrets.Select(s => new SecretAccess(s.Id, false, false)),
};

var policy = await query.FirstOrDefaultAsync();

return policy == null ? (false, false) : (policy.Read, policy.Write);
}

private record SecretAccess(Guid Id, bool Read, bool Write);

public async Task EmptyTrash(DateTime currentDate, uint deleteAfterThisNumberOfDays)
{
using var scope = ServiceScopeFactory.CreateScope();
Expand All @@ -313,6 +336,32 @@ public async Task EmptyTrash(DateTime currentDate, uint deleteAfterThisNumberOfD
await dbContext.SaveChangesAsync();
}

public async Task MoveSecretsAsync(IEnumerable<Core.SecretsManager.Entities.Secret> secrets, Guid projectId)
{
using var scope = ServiceScopeFactory.CreateScope();
var dbContext = GetDatabaseContext(scope);

await using var transaction = await dbContext.Database.BeginTransactionAsync();

var secretIds = secrets.Select(s => s.Id).ToList();

var projectSecrets = secretIds.Select(secretId => new ProjectSecret
{
ProjectsId = projectId,
SecretsId = secretId
});

await dbContext.ProjectSecrets
.Where(ps => secretIds.Contains(ps.SecretsId))
.ExecuteDeleteAsync();
Comment on lines +354 to +356
Copy link

Choose a reason for hiding this comment

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

logic: This operation deletes all existing project-secret relationships. Ensure this is the intended behavior, as it may have unintended consequences


dbContext.ProjectSecrets.AddRange(projectSecrets);

await dbContext.SaveChangesAsync();

await transaction.CommitAsync();
}

private IQueryable<SecretPermissionDetails> SecretToPermissionDetails(IQueryable<Secret> query, Guid userId, AccessClientType accessType)
{
var secrets = accessType switch
Expand Down
Loading