[SM-568] Delete service accounts (#2748)

bitwarden_license/src/Commercial.Core/SecretsManager/Commands/ServiceAccounts/DeleteServiceAccountsCommand.cs

@@ -0,0 +1,83 @@
+using Bit.Core.Context;
+using Bit.Core.Enums;
+using Bit.Core.Exceptions;
+using Bit.Core.SecretsManager.Commands.ServiceAccounts.Interfaces;
+using Bit.Core.SecretsManager.Entities;
+using Bit.Core.SecretsManager.Repositories;
+
+namespace Bit.Commercial.Core.SecretsManager.Commands.ServiceAccounts;
+
+public class DeleteServiceAccountsCommand : IDeleteServiceAccountsCommand
+{
+    private readonly IServiceAccountRepository _serviceAccountRepository;
+    private readonly ICurrentContext _currentContext;
+
+    public DeleteServiceAccountsCommand(
+        IServiceAccountRepository serviceAccountRepository,
+        ICurrentContext currentContext)
+    {
+        _serviceAccountRepository = serviceAccountRepository;
+        _currentContext = currentContext;
+    }
+
+    public async Task<List<Tuple<ServiceAccount, string>>> DeleteServiceAccounts(List<Guid> ids, Guid userId)
+    {
+        if (ids.Any() != true || userId == new Guid())
+        {
+            throw new ArgumentNullException();
+        }
+
+        var serviceAccounts = (await _serviceAccountRepository.GetManyByIds(ids))?.ToList();
+
+        if (serviceAccounts?.Any() != true || serviceAccounts.Count != ids.Count)
+        {
+            throw new NotFoundException();
+        }
+
+        // Ensure all service accounts belongs to the same organization
+        var organizationId = serviceAccounts.First().OrganizationId;
+        if (serviceAccounts.Any(p => p.OrganizationId != organizationId))
+        {
+            throw new BadRequestException();
+        }
+
+        if (!_currentContext.AccessSecretsManager(organizationId))
+        {
+            throw new NotFoundException();
+        }
+
+        var orgAdmin = await _currentContext.OrganizationAdmin(organizationId);
+        var accessClient = AccessClientHelper.ToAccessClient(_currentContext.ClientType, orgAdmin);
+
+        var results = new List<Tuple<ServiceAccount, String>>(serviceAccounts.Count);
+        var deleteIds = new List<Guid>();
+
+        foreach (var sa in serviceAccounts)
+        {
+            var hasAccess = accessClient switch
+            {
+                AccessClientType.NoAccessCheck => true,
+                AccessClientType.User => await _serviceAccountRepository.UserHasWriteAccessToServiceAccount(sa.Id, userId),
+                _ => false,
+            };
+
+            if (!hasAccess)
+            {
+                results.Add(new Tuple<ServiceAccount, string>(sa, "access denied"));
+            }
+            else
+            {
+                results.Add(new Tuple<ServiceAccount, string>(sa, ""));
+                deleteIds.Add(sa.Id);
+            }
+        }
+
+        if (deleteIds.Count > 0)
+        {
+            await _serviceAccountRepository.DeleteManyByIdAsync(deleteIds);
+        }
+
+        return results;
+    }
+}
+

bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs

@@ -28,6 +28,7 @@ public static class SecretsManagerCollectionExtensions
         services.AddScoped<IDeleteProjectCommand, DeleteProjectCommand>();
         services.AddScoped<ICreateServiceAccountCommand, CreateServiceAccountCommand>();
         services.AddScoped<IUpdateServiceAccountCommand, UpdateServiceAccountCommand>();
+        services.AddScoped<IDeleteServiceAccountsCommand, DeleteServiceAccountsCommand>();
         services.AddScoped<IRevokeAccessTokensCommand, RevokeAccessTokensCommand>();
         services.AddScoped<ICreateAccessTokenCommand, CreateAccessTokenCommand>();
         services.AddScoped<ICreateAccessPoliciesCommand, CreateAccessPoliciesCommand>();

bitwarden_license/src/Commercial.Infrastructure.EntityFramework/SecretsManager/Repositories/ServiceAccountRepository.cs

@@ -32,6 +32,16 @@ public class ServiceAccountRepository : Repository<Core.SecretsManager.Entities.
         return Mapper.Map<List<Core.SecretsManager.Entities.ServiceAccount>>(serviceAccounts);
     }
 
+    public async Task<IEnumerable<Core.SecretsManager.Entities.ServiceAccount>> GetManyByIds(IEnumerable<Guid> ids)
+    {
+        using var scope = ServiceScopeFactory.CreateScope();
+        var dbContext = GetDatabaseContext(scope);
+        var serviceAccounts = await dbContext.ServiceAccount
+            .Where(c => ids.Contains(c.Id))
+            .ToListAsync();
+        return Mapper.Map<List<Core.SecretsManager.Entities.ServiceAccount>>(serviceAccounts);
+    }
+
     public async Task<bool> UserHasReadAccessToServiceAccount(Guid id, Guid userId)
     {
         using var scope = ServiceScopeFactory.CreateScope();
@@ -71,6 +81,26 @@ public class ServiceAccountRepository : Repository<Core.SecretsManager.Entities.
         return Mapper.Map<List<Core.SecretsManager.Entities.ServiceAccount>>(serviceAccounts);
     }
 
+    public async Task DeleteManyByIdAsync(IEnumerable<Guid> ids)
+    {
+        using var scope = ServiceScopeFactory.CreateScope();
+        var dbContext = GetDatabaseContext(scope);
+
+        // Policies can't have a cascade delete, so we need to delete them manually.
+        var policies = dbContext.AccessPolicies.Where(ap =>
+            ((ServiceAccountProjectAccessPolicy)ap).ServiceAccountId.HasValue && ids.Contains(((ServiceAccountProjectAccessPolicy)ap).ServiceAccountId!.Value) ||
+            ((GroupServiceAccountAccessPolicy)ap).GrantedServiceAccountId.HasValue && ids.Contains(((GroupServiceAccountAccessPolicy)ap).GrantedServiceAccountId!.Value) ||
+            ((UserServiceAccountAccessPolicy)ap).GrantedServiceAccountId.HasValue && ids.Contains(((UserServiceAccountAccessPolicy)ap).GrantedServiceAccountId!.Value));
+        dbContext.RemoveRange(policies);
+
+        var apiKeys = dbContext.ApiKeys.Where(a => a.ServiceAccountId.HasValue && ids.Contains(a.ServiceAccountId!.Value));
+        dbContext.RemoveRange(apiKeys);
+
+        var serviceAccounts = dbContext.ServiceAccount.Where(c => ids.Contains(c.Id));
+        dbContext.RemoveRange(serviceAccounts);
+        await dbContext.SaveChangesAsync();
+    }
+
     private static Expression<Func<ServiceAccount, bool>> UserHasReadAccessToServiceAccount(Guid userId) => sa =>
         sa.UserAccessPolicies.Any(ap => ap.OrganizationUser.User.Id == userId && ap.Read) ||
         sa.GroupAccessPolicies.Any(ap => ap.Group.GroupUsers.Any(gu => gu.OrganizationUser.User.Id == userId && ap.Read));

src/Api/SecretsManager/Controllers/ServiceAccountsController.cs

@@ -25,6 +25,7 @@ public class ServiceAccountsController : Controller
     private readonly ICreateAccessTokenCommand _createAccessTokenCommand;
     private readonly ICreateServiceAccountCommand _createServiceAccountCommand;
     private readonly IUpdateServiceAccountCommand _updateServiceAccountCommand;
+    private readonly IDeleteServiceAccountsCommand _deleteServiceAccountsCommand;
     private readonly IRevokeAccessTokensCommand _revokeAccessTokensCommand;
 
     public ServiceAccountsController(
@@ -35,6 +36,7 @@ public class ServiceAccountsController : Controller
         ICreateAccessTokenCommand createAccessTokenCommand,
         ICreateServiceAccountCommand createServiceAccountCommand,
         IUpdateServiceAccountCommand updateServiceAccountCommand,
+        IDeleteServiceAccountsCommand deleteServiceAccountsCommand,
         IRevokeAccessTokensCommand revokeAccessTokensCommand)
     {
         _currentContext = currentContext;
@@ -43,6 +45,7 @@ public class ServiceAccountsController : Controller
         _apiKeyRepository = apiKeyRepository;
         _createServiceAccountCommand = createServiceAccountCommand;
         _updateServiceAccountCommand = updateServiceAccountCommand;
+        _deleteServiceAccountsCommand = deleteServiceAccountsCommand;
         _revokeAccessTokensCommand = revokeAccessTokensCommand;
         _createAccessTokenCommand = createAccessTokenCommand;
     }
@@ -90,6 +93,16 @@ public class ServiceAccountsController : Controller
         return new ServiceAccountResponseModel(result);
     }
 
+    [HttpPost("delete")]
+    public async Task<ListResponseModel<BulkDeleteResponseModel>> BulkDeleteAsync([FromBody] List<Guid> ids)
+    {
+        var userId = _userService.GetProperUserId(User).Value;
+
+        var results = await _deleteServiceAccountsCommand.DeleteServiceAccounts(ids, userId);
+        var responses = results.Select(r => new BulkDeleteResponseModel(r.Item1.Id, r.Item2));
+        return new ListResponseModel<BulkDeleteResponseModel>(responses);
+    }
+
     [HttpGet("{id}/access-tokens")]
     public async Task<ListResponseModel<AccessTokenResponseModel>> GetAccessTokens([FromRoute] Guid id)
     {

src/Core/SecretsManager/Commands/ServiceAccounts/Interfaces/IDeleteServiceAccountsCommand.cs

@@ -0,0 +1,9 @@
+using Bit.Core.SecretsManager.Entities;
+
+namespace Bit.Core.SecretsManager.Commands.ServiceAccounts.Interfaces;
+
+public interface IDeleteServiceAccountsCommand
+{
+    Task<List<Tuple<ServiceAccount, string>>> DeleteServiceAccounts(List<Guid> ids, Guid userId);
+}
+

src/Core/SecretsManager/Repositories/IServiceAccountRepository.cs

@@ -7,8 +7,10 @@ public interface IServiceAccountRepository
 {
     Task<IEnumerable<ServiceAccount>> GetManyByOrganizationIdAsync(Guid organizationId, Guid userId, AccessClientType accessType);
     Task<ServiceAccount> GetByIdAsync(Guid id);
+    Task<IEnumerable<ServiceAccount>> GetManyByIds(IEnumerable<Guid> ids);
     Task<ServiceAccount> CreateAsync(ServiceAccount serviceAccount);
     Task ReplaceAsync(ServiceAccount serviceAccount);
+    Task DeleteManyByIdAsync(IEnumerable<Guid> ids);
     Task<bool> UserHasReadAccessToServiceAccount(Guid id, Guid userId);
     Task<bool> UserHasWriteAccessToServiceAccount(Guid id, Guid userId);
     Task<IEnumerable<ServiceAccount>> GetManyByOrganizationIdWriteAccessAsync(Guid organizationId, Guid userId, AccessClientType accessType);

test/Api.IntegrationTest/SecretsManager/Controllers/ServiceAccountsControllerTests.cs

@@ -275,6 +275,92 @@ public class ServiceAccountsControllerTest : IClassFixture<ApiApplicationFactory
         Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
     }
 
+    [Theory]
+    [InlineData(false, false)]
+    [InlineData(true, false)]
+    [InlineData(false, true)]
+    public async Task Delete_SmNotEnabled_NotFound(bool useSecrets, bool accessSecrets)
+    {
+        var (org, _) = await _organizationHelper.Initialize(useSecrets, accessSecrets);
+        await LoginAsync(_email);
+
+        var initialServiceAccount = await _serviceAccountRepository.CreateAsync(new ServiceAccount
+        {
+            OrganizationId = org.Id,
+            Name = _mockEncryptedString,
+        });
+
+        var request = new List<Guid> { initialServiceAccount.Id };
+
+        var response = await _client.PutAsJsonAsync("/service-accounts/delete", request);
+        Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
+    }
+
+    [Fact]
+    public async Task Delete_MissingAccessPolicy_AccessDenied()
+    {
+        var (org, _) = await _organizationHelper.Initialize(true, true);
+        var (email, _) = await _organizationHelper.CreateNewUser(OrganizationUserType.User, true);
+        await LoginAsync(email);
+
+        var serviceAccount = await _serviceAccountRepository.CreateAsync(new ServiceAccount
+        {
+            OrganizationId = org.Id,
+            Name = _mockEncryptedString,
+        });
+
+        var ids = new List<Guid> { serviceAccount.Id };
+
+        var response = await _client.PostAsJsonAsync("/service-accounts/delete", ids);
+
+        var results = await response.Content.ReadFromJsonAsync<ListResponseModel<BulkDeleteResponseModel>>();
+        Assert.NotNull(results);
+    }
+
+    [Theory]
+    [InlineData(PermissionType.RunAsAdmin)]
+    [InlineData(PermissionType.RunAsUserWithPermission)]
+    public async Task Delete_Success(PermissionType permissionType)
+    {
+        var (org, _) = await _organizationHelper.Initialize(true, true);
+
+        var serviceAccount = await _serviceAccountRepository.CreateAsync(new ServiceAccount
+        {
+            OrganizationId = org.Id,
+            Name = _mockEncryptedString,
+        });
+
+        await _apiKeyRepository.CreateAsync(new ApiKey { ServiceAccountId = serviceAccount.Id });
+
+        if (permissionType == PermissionType.RunAsAdmin)
+        {
+            await LoginAsync(_email);
+        }
+        else
+        {
+            var (email, orgUser) = await _organizationHelper.CreateNewUser(OrganizationUserType.User, true);
+            await LoginAsync(email);
+
+            await _accessPolicyRepository.CreateManyAsync(new List<BaseAccessPolicy> {
+                new UserServiceAccountAccessPolicy
+                {
+                    GrantedServiceAccountId = serviceAccount.Id,
+                    OrganizationUserId = orgUser.Id,
+                    Write = true,
+                    Read = true,
+                },
+            });
+        }
+
+        var ids = new List<Guid> { serviceAccount.Id };
+
+        var response = await _client.PostAsJsonAsync("/service-accounts/delete", ids);
+        response.EnsureSuccessStatusCode();
+
+        var sa = await _serviceAccountRepository.GetManyByIds(ids);
+        Assert.Empty(sa);
+    }
+
     [Theory]
     [InlineData(false, false)]
     [InlineData(true, false)]