diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/ServiceAccounts/RevokeAccessTokensCommand.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/ServiceAccounts/RevokeAccessTokensCommand.cs new file mode 100644 index 0000000000..7e3474c0c8 --- /dev/null +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/ServiceAccounts/RevokeAccessTokensCommand.cs @@ -0,0 +1,24 @@ +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 RevokeAccessTokensCommand : IRevokeAccessTokensCommand +{ + private readonly IApiKeyRepository _apiKeyRepository; + + public RevokeAccessTokensCommand(IApiKeyRepository apiKeyRepository) + { + _apiKeyRepository = apiKeyRepository; + } + + public async Task RevokeAsync(ServiceAccount serviceAccount, IEnumerable Ids) + { + var accessTokens = await _apiKeyRepository.GetManyByServiceAccountIdAsync(serviceAccount.Id); + + var tokensToDelete = accessTokens.Where(at => Ids.Contains(at.Id)); + + await _apiKeyRepository.DeleteManyAsync(tokensToDelete); + } +} diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs index 81ef53454f..429833089f 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/SecretsManagerCollectionExtensions.cs @@ -26,6 +26,7 @@ public static class SecretsManagerCollectionExtensions services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); services.AddScoped(); diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/ServiceAccounts/RevokeAccessTokenCommandTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/ServiceAccounts/RevokeAccessTokenCommandTests.cs new file mode 100644 index 0000000000..5f077c4ace --- /dev/null +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/ServiceAccounts/RevokeAccessTokenCommandTests.cs @@ -0,0 +1,39 @@ +using Bit.Commercial.Core.SecretsManager.Commands.ServiceAccounts; +using Bit.Core.SecretsManager.Entities; +using Bit.Core.SecretsManager.Repositories; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Commercial.Core.Test.SecretsManager.ServiceAccounts; + +[SutProviderCustomize] +public class RevokeAccessTokenCommandTests +{ + [Theory] + [BitAutoData] + public async Task RevokeAsyncAsync_Success(ServiceAccount serviceAccount, SutProvider sutProvider) + { + var apiKey1 = new ApiKey + { + Id = Guid.NewGuid(), + ServiceAccountId = serviceAccount.Id + }; + + var apiKey2 = new ApiKey + { + Id = Guid.NewGuid(), + ServiceAccountId = serviceAccount.Id + }; + + sutProvider.GetDependency() + .GetManyByServiceAccountIdAsync(serviceAccount.Id) + .Returns(new List { apiKey1, apiKey2 }); + + await sutProvider.Sut.RevokeAsync(serviceAccount, new List { apiKey1.Id }); + + await sutProvider.GetDependency().Received(1) + .DeleteManyAsync(Arg.Is>(arg => arg.SequenceEqual(new List { apiKey1 }))); + } +} diff --git a/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs b/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs index 175db2a1f2..ccc851c8a2 100644 --- a/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs +++ b/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs @@ -19,20 +19,23 @@ namespace Bit.Api.SecretsManager.Controllers; public class ServiceAccountsController : Controller { private readonly ICurrentContext _currentContext; + private readonly IUserService _userService; + private readonly IServiceAccountRepository _serviceAccountRepository; private readonly IApiKeyRepository _apiKeyRepository; private readonly ICreateAccessTokenCommand _createAccessTokenCommand; private readonly ICreateServiceAccountCommand _createServiceAccountCommand; - private readonly IServiceAccountRepository _serviceAccountRepository; private readonly IUpdateServiceAccountCommand _updateServiceAccountCommand; - private readonly IUserService _userService; + private readonly IRevokeAccessTokensCommand _revokeAccessTokensCommand; public ServiceAccountsController( ICurrentContext currentContext, IUserService userService, IServiceAccountRepository serviceAccountRepository, + IApiKeyRepository apiKeyRepository, ICreateAccessTokenCommand createAccessTokenCommand, - IApiKeyRepository apiKeyRepository, ICreateServiceAccountCommand createServiceAccountCommand, - IUpdateServiceAccountCommand updateServiceAccountCommand) + ICreateServiceAccountCommand createServiceAccountCommand, + IUpdateServiceAccountCommand updateServiceAccountCommand, + IRevokeAccessTokensCommand revokeAccessTokensCommand) { _currentContext = currentContext; _userService = userService; @@ -40,6 +43,7 @@ public class ServiceAccountsController : Controller _apiKeyRepository = apiKeyRepository; _createServiceAccountCommand = createServiceAccountCommand; _updateServiceAccountCommand = updateServiceAccountCommand; + _revokeAccessTokensCommand = revokeAccessTokensCommand; _createAccessTokenCommand = createAccessTokenCommand; } @@ -129,4 +133,37 @@ public class ServiceAccountsController : Controller var result = await _createAccessTokenCommand.CreateAsync(request.ToApiKey(id), userId); return new AccessTokenCreationResponseModel(result); } + + [HttpPost("{id}/access-tokens/revoke")] + public async Task RevokeAccessTokensAsync(Guid id, [FromBody] RevokeAccessTokensRequest request) + { + var userId = _userService.GetProperUserId(User).Value; + var serviceAccount = await _serviceAccountRepository.GetByIdAsync(id); + if (serviceAccount == null) + { + throw new NotFoundException(); + } + + if (!_currentContext.AccessSecretsManager(serviceAccount.OrganizationId)) + { + throw new NotFoundException(); + } + + var orgAdmin = await _currentContext.OrganizationAdmin(serviceAccount.OrganizationId); + var accessClient = AccessClientHelper.ToAccessClient(_currentContext.ClientType, orgAdmin); + + var hasAccess = accessClient switch + { + AccessClientType.NoAccessCheck => true, + AccessClientType.User => await _serviceAccountRepository.UserHasWriteAccessToServiceAccount(id, userId), + _ => false, + }; + + if (!hasAccess) + { + throw new NotFoundException(); + } + + await _revokeAccessTokensCommand.RevokeAsync(serviceAccount, request.Ids); + } } diff --git a/src/Api/SecretsManager/Models/Request/RevokeAccessTokensRequest.cs b/src/Api/SecretsManager/Models/Request/RevokeAccessTokensRequest.cs new file mode 100644 index 0000000000..ecced7a5cd --- /dev/null +++ b/src/Api/SecretsManager/Models/Request/RevokeAccessTokensRequest.cs @@ -0,0 +1,7 @@ +using System.ComponentModel.DataAnnotations; + +public class RevokeAccessTokensRequest +{ + [Required] + public Guid[] Ids { get; set; } +} diff --git a/src/Core/SecretsManager/Commands/ServiceAccounts/Interfaces/IRevokeAccessTokensCommand.cs b/src/Core/SecretsManager/Commands/ServiceAccounts/Interfaces/IRevokeAccessTokensCommand.cs new file mode 100644 index 0000000000..a6aab55d9d --- /dev/null +++ b/src/Core/SecretsManager/Commands/ServiceAccounts/Interfaces/IRevokeAccessTokensCommand.cs @@ -0,0 +1,8 @@ +using Bit.Core.SecretsManager.Entities; + +namespace Bit.Core.SecretsManager.Commands.ServiceAccounts.Interfaces; + +public interface IRevokeAccessTokensCommand +{ + Task RevokeAsync(ServiceAccount serviceAccount, IEnumerable ids); +} diff --git a/src/Core/SecretsManager/Repositories/IApiKeyRepository.cs b/src/Core/SecretsManager/Repositories/IApiKeyRepository.cs index dc48b5295d..eead6d9d75 100644 --- a/src/Core/SecretsManager/Repositories/IApiKeyRepository.cs +++ b/src/Core/SecretsManager/Repositories/IApiKeyRepository.cs @@ -8,4 +8,5 @@ public interface IApiKeyRepository : IRepository { Task GetDetailsByIdAsync(Guid id); Task> GetManyByServiceAccountIdAsync(Guid id); + Task DeleteManyAsync(IEnumerable objs); } diff --git a/src/Infrastructure.Dapper/SecretsManager/Repositories/ApiKeyRepository.cs b/src/Infrastructure.Dapper/SecretsManager/Repositories/ApiKeyRepository.cs index 8a13eb52c6..c362ae2369 100644 --- a/src/Infrastructure.Dapper/SecretsManager/Repositories/ApiKeyRepository.cs +++ b/src/Infrastructure.Dapper/SecretsManager/Repositories/ApiKeyRepository.cs @@ -42,4 +42,13 @@ public class ApiKeyRepository : Repository, IApiKeyRepository return results.ToList(); } + + public async Task DeleteManyAsync(IEnumerable objs) + { + using var connection = new SqlConnection(ConnectionString); + await connection.QueryAsync( + $"[{Schema}].[ApiKey_DeleteByIds]", + new { Ids = objs.Select(obj => obj.Id).ToGuidIdArrayTVP() }, + commandType: CommandType.StoredProcedure); + } } diff --git a/src/Infrastructure.EntityFramework/SecretsManager/Repositories/ApiKeyRepository.cs b/src/Infrastructure.EntityFramework/SecretsManager/Repositories/ApiKeyRepository.cs index 364415e066..8de2443a1f 100644 --- a/src/Infrastructure.EntityFramework/SecretsManager/Repositories/ApiKeyRepository.cs +++ b/src/Infrastructure.EntityFramework/SecretsManager/Repositories/ApiKeyRepository.cs @@ -36,4 +36,13 @@ public class ApiKeyRepository : Repository>(apiKeys); } + + public async Task DeleteManyAsync(IEnumerable objs) + { + using var scope = ServiceScopeFactory.CreateScope(); + var dbContext = GetDatabaseContext(scope); + var entities = objs.Select(obj => Mapper.Map(obj)); + dbContext.RemoveRange(entities); + await dbContext.SaveChangesAsync(); + } } diff --git a/src/Sql/SecretsManager/dbo/Stored Procedures/ApiKey/ApiKey_DeleteByIds.sql b/src/Sql/SecretsManager/dbo/Stored Procedures/ApiKey/ApiKey_DeleteByIds.sql new file mode 100644 index 0000000000..a0c1f2ace2 --- /dev/null +++ b/src/Sql/SecretsManager/dbo/Stored Procedures/ApiKey/ApiKey_DeleteByIds.sql @@ -0,0 +1,23 @@ +CREATE PROCEDURE [dbo].[ApiKey_DeleteByIds] + @Ids [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + + DECLARE @BatchSize INT = 100 + + WHILE @BatchSize > 0 + BEGIN + BEGIN TRANSACTION ApiKey_DeleteMany + + DELETE TOP(@BatchSize) AK + FROM + [dbo].[ApiKey] AK + INNER JOIN + @Ids I ON I.Id = AK.Id + + SET @BatchSize = @@ROWCOUNT + + COMMIT TRANSACTION ApiKey_DeleteMany + END +END diff --git a/src/Sql/Sql.sqlproj b/src/Sql/Sql.sqlproj index 366026063c..1bed1049a3 100644 --- a/src/Sql/Sql.sqlproj +++ b/src/Sql/Sql.sqlproj @@ -444,6 +444,7 @@ + diff --git a/test/Api.IntegrationTest/SecretsManager/Controllers/ServiceAccountsControllerTests.cs b/test/Api.IntegrationTest/SecretsManager/Controllers/ServiceAccountsControllerTests.cs index 97373422d8..32cae25037 100644 --- a/test/Api.IntegrationTest/SecretsManager/Controllers/ServiceAccountsControllerTests.cs +++ b/test/Api.IntegrationTest/SecretsManager/Controllers/ServiceAccountsControllerTests.cs @@ -1,6 +1,7 @@ using System.Net; using System.Net.Http.Headers; using Bit.Api.IntegrationTest.Factories; +using Bit.Api.IntegrationTest.SecretsManager.Enums; using Bit.Api.Models.Response; using Bit.Api.SecretsManager.Models.Request; using Bit.Api.SecretsManager.Models.Response; @@ -21,9 +22,11 @@ public class ServiceAccountsControllerTest : IClassFixture(); _accessPolicyRepository = _factory.GetService(); + _apiKeyRepository = _factory.GetService(); } public async Task InitializeAsync() @@ -426,6 +430,130 @@ public class ServiceAccountsControllerTest : IClassFixture { + new UserServiceAccountAccessPolicy + { + GrantedServiceAccountId = serviceAccount.Id, + OrganizationUserId = orgUser.Id, + Write = false, + Read = true, + }, + }); + } + + var accessToken = await _apiKeyRepository.CreateAsync(new ApiKey + { + ServiceAccountId = org.Id, + Name = _mockEncryptedString, + ExpireAt = DateTime.UtcNow.AddDays(30), + }); + + var request = new RevokeAccessTokensRequest + { + Ids = new[] { accessToken.Id }, + }; + + var response = await _client.PostAsJsonAsync($"/service-accounts/{serviceAccount.Id}/access-tokens/revoke", request); + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Theory] + [InlineData(PermissionType.RunAsAdmin)] + [InlineData(PermissionType.RunAsUserWithPermission)] + public async Task RevokeAccessToken_Success(PermissionType permissionType) + { + var (org, _) = await _organizationHelper.Initialize(true, true); + + var serviceAccount = await _serviceAccountRepository.CreateAsync(new ServiceAccount + { + OrganizationId = org.Id, + Name = _mockEncryptedString, + }); + + 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 { + new UserServiceAccountAccessPolicy + { + GrantedServiceAccountId = serviceAccount.Id, + OrganizationUserId = orgUser.Id, + Write = true, + Read = true, + }, + }); + } + + var accessToken = await _apiKeyRepository.CreateAsync(new ApiKey + { + ServiceAccountId = org.Id, + Name = _mockEncryptedString, + ExpireAt = DateTime.UtcNow.AddDays(30), + }); + + var request = new RevokeAccessTokensRequest + { + Ids = new[] { accessToken.Id }, + }; + + var response = await _client.PostAsJsonAsync($"/service-accounts/{serviceAccount.Id}/access-tokens/revoke", request); + response.EnsureSuccessStatusCode(); + } + private async Task CreateUserPolicyAsync(Guid userId, Guid serviceAccountId, bool read, bool write) { var policy = new UserServiceAccountAccessPolicy diff --git a/util/Migrator/DbScripts/2023-02-14_00_RevokeApiKeys.sql b/util/Migrator/DbScripts/2023-02-14_00_RevokeApiKeys.sql new file mode 100644 index 0000000000..044b920b53 --- /dev/null +++ b/util/Migrator/DbScripts/2023-02-14_00_RevokeApiKeys.sql @@ -0,0 +1,23 @@ +CREATE OR ALTER PROCEDURE [dbo].[ApiKey_DeleteByIds] + @Ids [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + + DECLARE @BatchSize INT = 100 + + WHILE @BatchSize > 0 + BEGIN + BEGIN TRANSACTION ApiKey_DeleteMany + + DELETE TOP(@BatchSize) AK + FROM + [dbo].[ApiKey] AK + INNER JOIN + @Ids I ON I.Id = AK.Id + + SET @BatchSize = @@ROWCOUNT + + COMMIT TRANSACTION ApiKey_DeleteMany + END +END