diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/ServiceAccounts/ServiceAccountAuthorizationHandler.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/ServiceAccounts/ServiceAccountAuthorizationHandler.cs index 9a62b22c3..dac784986 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/ServiceAccounts/ServiceAccountAuthorizationHandler.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/AuthorizationHandlers/ServiceAccounts/ServiceAccountAuthorizationHandler.cs @@ -44,6 +44,15 @@ public class case not null when requirement == ServiceAccountOperations.Update: await CanUpdateServiceAccountAsync(context, requirement, resource); break; + case not null when requirement == ServiceAccountOperations.CreateAccessToken: + await CanCreateAccessTokenAsync(context, requirement, resource); + break; + case not null when requirement == ServiceAccountOperations.ReadAccessTokens: + await CanReadAccessTokensAsync(context, requirement, resource); + break; + case not null when requirement == ServiceAccountOperations.RevokeAccessTokens: + await CanRevokeAccessTokensAsync(context, requirement, resource); + break; default: throw new ArgumentException("Unsupported operation requirement type provided.", nameof(requirement)); @@ -97,4 +106,49 @@ public class context.Succeed(requirement); } } + + private async Task CanCreateAccessTokenAsync(AuthorizationHandlerContext context, + ServiceAccountOperationRequirement requirement, ServiceAccount resource) + { + var (accessClient, userId) = + await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId); + var access = + await _serviceAccountRepository.AccessToServiceAccountAsync(resource.Id, userId, + accessClient); + + if (access.Write) + { + context.Succeed(requirement); + } + } + + private async Task CanReadAccessTokensAsync(AuthorizationHandlerContext context, + ServiceAccountOperationRequirement requirement, ServiceAccount resource) + { + var (accessClient, userId) = + await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId); + var access = + await _serviceAccountRepository.AccessToServiceAccountAsync(resource.Id, userId, + accessClient); + + if (access.Read) + { + context.Succeed(requirement); + } + } + + private async Task CanRevokeAccessTokensAsync(AuthorizationHandlerContext context, + ServiceAccountOperationRequirement requirement, ServiceAccount resource) + { + var (accessClient, userId) = + await _accessClientQuery.GetAccessClientAsync(context.User, resource.OrganizationId); + var access = + await _serviceAccountRepository.AccessToServiceAccountAsync(resource.Id, userId, + accessClient); + + if (access.Write) + { + context.Succeed(requirement); + } + } } diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/AccessTokens/CreateAccessTokenCommand.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/AccessTokens/CreateAccessTokenCommand.cs index 66b525f65..f6af908fb 100644 --- a/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/AccessTokens/CreateAccessTokenCommand.cs +++ b/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/AccessTokens/CreateAccessTokenCommand.cs @@ -1,6 +1,4 @@ -using Bit.Core.Context; -using Bit.Core.Enums; -using Bit.Core.Exceptions; +using Bit.Core.Exceptions; using Bit.Core.SecretsManager.Commands.AccessTokens.Interfaces; using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Repositories; @@ -10,51 +8,21 @@ namespace Bit.Commercial.Core.SecretsManager.Commands.AccessTokens; public class CreateAccessTokenCommand : ICreateAccessTokenCommand { + private const int _clientSecretMaxLength = 30; private readonly IApiKeyRepository _apiKeyRepository; - private readonly int _clientSecretMaxLength = 30; - private readonly ICurrentContext _currentContext; - private readonly IServiceAccountRepository _serviceAccountRepository; - public CreateAccessTokenCommand( - IApiKeyRepository apiKeyRepository, - ICurrentContext currentContext, - IServiceAccountRepository serviceAccountRepository) + public CreateAccessTokenCommand(IApiKeyRepository apiKeyRepository) { _apiKeyRepository = apiKeyRepository; - _currentContext = currentContext; - _serviceAccountRepository = serviceAccountRepository; } - public async Task CreateAsync(ApiKey apiKey, Guid userId) + public async Task CreateAsync(ApiKey apiKey) { if (apiKey.ServiceAccountId == null) { throw new BadRequestException(); } - var serviceAccount = await _serviceAccountRepository.GetByIdAsync(apiKey.ServiceAccountId.Value); - - 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( - apiKey.ServiceAccountId.Value, userId), - _ => false, - }; - - if (!hasAccess) - { - throw new NotFoundException(); - } - apiKey.ClientSecret = CoreHelpers.SecureRandomString(_clientSecretMaxLength); return await _apiKeyRepository.CreateAsync(apiKey); } diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/ServiceAccounts/ServiceAccountAuthorizationHandlerTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/ServiceAccounts/ServiceAccountAuthorizationHandlerTests.cs index 5b9f139b7..2da2eafde 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/ServiceAccounts/ServiceAccountAuthorizationHandlerTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/AuthorizationHandlers/ServiceAccounts/ServiceAccountAuthorizationHandlerTests.cs @@ -178,9 +178,13 @@ public class ServiceAccountAuthorizationHandlerTests } [Theory] - [BitAutoData(PermissionType.RunAsUserWithPermission, true, false)] - [BitAutoData(PermissionType.RunAsUserWithPermission, false, false)] - public async Task CanUpdateServiceAccount_ShouldNotSucceed(PermissionType permissionType, bool read, bool write, + [BitAutoData(PermissionType.RunAsAdmin, true, true, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, false, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, true, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, false, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, true, true)] + public async Task CanUpdateServiceAccount_AccessCheck(PermissionType permissionType, bool read, bool write, + bool expected, SutProvider sutProvider, ServiceAccount serviceAccount, ClaimsPrincipal claimsPrincipal, Guid userId) @@ -195,30 +199,7 @@ public class ServiceAccountAuthorizationHandlerTests await sutProvider.Sut.HandleAsync(authzContext); - Assert.False(authzContext.HasSucceeded); - } - - [Theory] - [BitAutoData(PermissionType.RunAsAdmin, true, true)] - [BitAutoData(PermissionType.RunAsAdmin, false, true)] - [BitAutoData(PermissionType.RunAsUserWithPermission, true, true)] - [BitAutoData(PermissionType.RunAsUserWithPermission, false, true)] - public async Task CanUpdateServiceAccount_Success(PermissionType permissionType, bool read, bool write, - SutProvider sutProvider, ServiceAccount serviceAccount, - ClaimsPrincipal claimsPrincipal, - Guid userId) - { - var requirement = ServiceAccountOperations.Update; - SetupPermission(sutProvider, permissionType, serviceAccount.OrganizationId, userId); - sutProvider.GetDependency() - .AccessToServiceAccountAsync(serviceAccount.Id, userId, Arg.Any()) - .Returns((read, write)); - var authzContext = new AuthorizationHandlerContext(new List { requirement }, - claimsPrincipal, serviceAccount); - - await sutProvider.Sut.HandleAsync(authzContext); - - Assert.True(authzContext.HasSucceeded); + Assert.Equal(expected, authzContext.HasSucceeded); } @@ -257,9 +238,13 @@ public class ServiceAccountAuthorizationHandlerTests } [Theory] - [BitAutoData(PermissionType.RunAsUserWithPermission, false, false)] - [BitAutoData(PermissionType.RunAsUserWithPermission, false, true)] - public async Task CanReadServiceAccount_ShouldNotSucceed(PermissionType permissionType, bool read, bool write, + [BitAutoData(PermissionType.RunAsAdmin, true, true, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, false, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, true, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, false, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, true, true)] + public async Task CanReadServiceAccount_AccessCheck(PermissionType permissionType, bool read, bool write, + bool expected, SutProvider sutProvider, ServiceAccount serviceAccount, ClaimsPrincipal claimsPrincipal, Guid userId) @@ -274,20 +259,56 @@ public class ServiceAccountAuthorizationHandlerTests await sutProvider.Sut.HandleAsync(authzContext); + Assert.Equal(expected, authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task CanCreateAccessToken_AccessToSecretsManagerFalse_DoesNotSucceed( + SutProvider sutProvider, ServiceAccount serviceAccount, + ClaimsPrincipal claimsPrincipal) + { + var requirement = ServiceAccountOperations.CreateAccessToken; + sutProvider.GetDependency().AccessSecretsManager(serviceAccount.OrganizationId) + .Returns(false); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, serviceAccount); + + await sutProvider.Sut.HandleAsync(authzContext); + Assert.False(authzContext.HasSucceeded); } [Theory] - [BitAutoData(PermissionType.RunAsAdmin, true, true)] - [BitAutoData(PermissionType.RunAsAdmin, true, false)] - [BitAutoData(PermissionType.RunAsUserWithPermission, true, true)] - [BitAutoData(PermissionType.RunAsUserWithPermission, true, false)] - public async Task CanReadServiceAccount_Success(PermissionType permissionType, bool read, bool write, + [BitAutoData] + public async Task CanCreateAccessToken_NullResource_DoesNotSucceed( SutProvider sutProvider, ServiceAccount serviceAccount, ClaimsPrincipal claimsPrincipal, Guid userId) { - var requirement = ServiceAccountOperations.Read; + var requirement = ServiceAccountOperations.CreateAccessToken; + SetupPermission(sutProvider, PermissionType.RunAsAdmin, serviceAccount.OrganizationId, userId); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, null); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(PermissionType.RunAsAdmin, true, true, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, false, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, true, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, false, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, true, true)] + public async Task CanCreateAccessToken_AccessCheck(PermissionType permissionType, bool read, bool write, + bool expected, + SutProvider sutProvider, ServiceAccount serviceAccount, + ClaimsPrincipal claimsPrincipal, + Guid userId) + { + var requirement = ServiceAccountOperations.CreateAccessToken; SetupPermission(sutProvider, permissionType, serviceAccount.OrganizationId, userId); sutProvider.GetDependency() .AccessToServiceAccountAsync(serviceAccount.Id, userId, Arg.Any()) @@ -297,6 +318,124 @@ public class ServiceAccountAuthorizationHandlerTests await sutProvider.Sut.HandleAsync(authzContext); - Assert.True(authzContext.HasSucceeded); + Assert.Equal(expected, authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task CanReadAccessTokens_AccessToSecretsManagerFalse_DoesNotSucceed( + SutProvider sutProvider, ServiceAccount serviceAccount, + ClaimsPrincipal claimsPrincipal) + { + var requirement = ServiceAccountOperations.ReadAccessTokens; + sutProvider.GetDependency().AccessSecretsManager(serviceAccount.OrganizationId) + .Returns(false); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, serviceAccount); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task CanReadAccessTokens_NullResource_DoesNotSucceed( + SutProvider sutProvider, ServiceAccount serviceAccount, + ClaimsPrincipal claimsPrincipal, + Guid userId) + { + var requirement = ServiceAccountOperations.ReadAccessTokens; + SetupPermission(sutProvider, PermissionType.RunAsAdmin, serviceAccount.OrganizationId, userId); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, null); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(PermissionType.RunAsAdmin, true, true, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, false, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, true, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, false, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, true, true)] + public async Task CanReadAccessTokens_AccessCheck(PermissionType permissionType, bool read, bool write, + bool expected, + SutProvider sutProvider, ServiceAccount serviceAccount, + ClaimsPrincipal claimsPrincipal, + Guid userId) + { + var requirement = ServiceAccountOperations.ReadAccessTokens; + SetupPermission(sutProvider, permissionType, serviceAccount.OrganizationId, userId); + sutProvider.GetDependency() + .AccessToServiceAccountAsync(serviceAccount.Id, userId, Arg.Any()) + .Returns((read, write)); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, serviceAccount); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.Equal(expected, authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task CanRevokeAccessTokens_AccessToSecretsManagerFalse_DoesNotSucceed( + SutProvider sutProvider, ServiceAccount serviceAccount, + ClaimsPrincipal claimsPrincipal) + { + var requirement = ServiceAccountOperations.RevokeAccessTokens; + sutProvider.GetDependency().AccessSecretsManager(serviceAccount.OrganizationId) + .Returns(false); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, serviceAccount); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData] + public async Task CanRevokeAccessTokens_NullResource_DoesNotSucceed( + SutProvider sutProvider, ServiceAccount serviceAccount, + ClaimsPrincipal claimsPrincipal, + Guid userId) + { + var requirement = ServiceAccountOperations.RevokeAccessTokens; + SetupPermission(sutProvider, PermissionType.RunAsAdmin, serviceAccount.OrganizationId, userId); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, null); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.False(authzContext.HasSucceeded); + } + + [Theory] + [BitAutoData(PermissionType.RunAsAdmin, true, true, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, false, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, false, true, true)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, false, false)] + [BitAutoData(PermissionType.RunAsUserWithPermission, true, true, true)] + public async Task CanRevokeAccessTokens_AccessCheck(PermissionType permissionType, bool read, bool write, + bool expected, + SutProvider sutProvider, ServiceAccount serviceAccount, + ClaimsPrincipal claimsPrincipal, + Guid userId) + { + var requirement = ServiceAccountOperations.RevokeAccessTokens; + SetupPermission(sutProvider, permissionType, serviceAccount.OrganizationId, userId); + sutProvider.GetDependency() + .AccessToServiceAccountAsync(serviceAccount.Id, userId, Arg.Any()) + .Returns((read, write)); + var authzContext = new AuthorizationHandlerContext(new List { requirement }, + claimsPrincipal, serviceAccount); + + await sutProvider.Sut.HandleAsync(authzContext); + + Assert.Equal(expected, authzContext.HasSucceeded); } } diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/AccessTokens/CreateAccessTokenCommandTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/AccessTokens/CreateAccessTokenCommandTests.cs index e425238c9..ec83cf926 100644 --- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/AccessTokens/CreateAccessTokenCommandTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/AccessTokens/CreateAccessTokenCommandTests.cs @@ -1,5 +1,4 @@ using Bit.Commercial.Core.SecretsManager.Commands.AccessTokens; -using Bit.Core.Context; using Bit.Core.Exceptions; using Bit.Core.SecretsManager.Entities; using Bit.Core.SecretsManager.Repositories; @@ -16,59 +15,21 @@ public class CreateServiceAccountCommandTests { [Theory] [BitAutoData] - public async Task CreateAsync_NoServiceAccountId_ThrowsBadRequestException(ApiKey data, Guid userId, - SutProvider sutProvider) + public async Task CreateAsync_NoServiceAccountId_ThrowsBadRequestException( + SutProvider sutProvider, ApiKey data) { data.ServiceAccountId = null; - await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAsync(data, userId)); + await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAsync(data)); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().CreateAsync(default); } [Theory] [BitAutoData] - public async Task CreateAsync_User_NoAccess(ApiKey data, Guid userId, ServiceAccount saData, - SutProvider sutProvider) + public async Task CreateAsync_Success(SutProvider sutProvider, ApiKey data) { - data.ServiceAccountId = saData.Id; - - sutProvider.GetDependency().GetByIdAsync(saData.Id).Returns(saData); - sutProvider.GetDependency().UserHasWriteAccessToServiceAccount(saData.Id, userId).Returns(false); - - await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAsync(data, userId)); - - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().CreateAsync(default); - } - - [Theory] - [BitAutoData] - public async Task CreateAsync_User_Success(ApiKey data, Guid userId, ServiceAccount saData, - SutProvider sutProvider) - { - data.ServiceAccountId = saData.Id; - sutProvider.GetDependency().GetByIdAsync(saData.Id).Returns(saData); - sutProvider.GetDependency().UserHasWriteAccessToServiceAccount(saData.Id, userId).Returns(true); - sutProvider.GetDependency().AccessSecretsManager(saData.OrganizationId).Returns(true); - - await sutProvider.Sut.CreateAsync(data, userId); - - await sutProvider.GetDependency().Received(1) - .CreateAsync(Arg.Is(AssertHelper.AssertPropertyEqual(data))); - } - - [Theory] - [BitAutoData] - public async Task CreateAsync_Admin_Succeeds(ApiKey data, Guid userId, ServiceAccount saData, - SutProvider sutProvider) - { - data.ServiceAccountId = saData.Id; - - sutProvider.GetDependency().GetByIdAsync(saData.Id).Returns(saData); - sutProvider.GetDependency().AccessSecretsManager(saData.OrganizationId).Returns(true); - sutProvider.GetDependency().OrganizationAdmin(saData.OrganizationId).Returns(true); - - await sutProvider.Sut.CreateAsync(data, userId); + await sutProvider.Sut.CreateAsync(data); await sutProvider.GetDependency().Received(1) .CreateAsync(Arg.Is(AssertHelper.AssertPropertyEqual(data))); diff --git a/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs b/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs index 561d07813..57f0b38a7 100644 --- a/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs +++ b/src/Api/SecretsManager/Controllers/ServiceAccountsController.cs @@ -140,28 +140,12 @@ public class ServiceAccountsController : Controller [HttpGet("{id}/access-tokens")] public async Task> GetAccessTokens([FromRoute] Guid id) { - var userId = _userService.GetProperUserId(User).Value; var serviceAccount = await _serviceAccountRepository.GetByIdAsync(id); - if (serviceAccount == null) - { - throw new NotFoundException(); - } + var authorizationResult = + await _authorizationService.AuthorizeAsync(User, serviceAccount, + ServiceAccountOperations.ReadAccessTokens); - 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.UserHasReadAccessToServiceAccount(id, userId), - _ => false, - }; - if (!hasAccess) + if (!authorizationResult.Succeeded) { throw new NotFoundException(); } @@ -175,38 +159,29 @@ public class ServiceAccountsController : Controller public async Task CreateAccessTokenAsync([FromRoute] Guid id, [FromBody] AccessTokenCreateRequestModel request) { - var userId = _userService.GetProperUserId(User).Value; + var serviceAccount = await _serviceAccountRepository.GetByIdAsync(id); + var authorizationResult = + await _authorizationService.AuthorizeAsync(User, serviceAccount, + ServiceAccountOperations.CreateAccessToken); - var result = await _createAccessTokenCommand.CreateAsync(request.ToApiKey(id), userId); + if (!authorizationResult.Succeeded) + { + throw new NotFoundException(); + } + + var result = await _createAccessTokenCommand.CreateAsync(request.ToApiKey(id)); 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(); - } + var authorizationResult = + await _authorizationService.AuthorizeAsync(User, serviceAccount, + ServiceAccountOperations.RevokeAccessTokens); - 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) + if (!authorizationResult.Succeeded) { throw new NotFoundException(); } diff --git a/src/Core/SecretsManager/AuthorizationRequirements/ServiceAccountOperationRequirement.cs b/src/Core/SecretsManager/AuthorizationRequirements/ServiceAccountOperationRequirement.cs index 18d2f0701..3a079e62e 100644 --- a/src/Core/SecretsManager/AuthorizationRequirements/ServiceAccountOperationRequirement.cs +++ b/src/Core/SecretsManager/AuthorizationRequirements/ServiceAccountOperationRequirement.cs @@ -11,4 +11,7 @@ public static class ServiceAccountOperations public static readonly ServiceAccountOperationRequirement Create = new() { Name = nameof(Create) }; public static readonly ServiceAccountOperationRequirement Read = new() { Name = nameof(Read) }; public static readonly ServiceAccountOperationRequirement Update = new() { Name = nameof(Update) }; + public static readonly ServiceAccountOperationRequirement ReadAccessTokens = new() { Name = nameof(ReadAccessTokens) }; + public static readonly ServiceAccountOperationRequirement CreateAccessToken = new() { Name = nameof(CreateAccessToken) }; + public static readonly ServiceAccountOperationRequirement RevokeAccessTokens = new() { Name = nameof(RevokeAccessTokens) }; } diff --git a/src/Core/SecretsManager/Commands/AccessTokens/Interfaces/ICreateAccessTokenCommand.cs b/src/Core/SecretsManager/Commands/AccessTokens/Interfaces/ICreateAccessTokenCommand.cs index 98101d499..2d05f8ca9 100644 --- a/src/Core/SecretsManager/Commands/AccessTokens/Interfaces/ICreateAccessTokenCommand.cs +++ b/src/Core/SecretsManager/Commands/AccessTokens/Interfaces/ICreateAccessTokenCommand.cs @@ -4,5 +4,5 @@ namespace Bit.Core.SecretsManager.Commands.AccessTokens.Interfaces; public interface ICreateAccessTokenCommand { - Task CreateAsync(ApiKey apiKey, Guid userId); + Task CreateAsync(ApiKey apiKey); } diff --git a/test/Api.Test/SecretsManager/Controllers/ServiceAccountsControllerTests.cs b/test/Api.Test/SecretsManager/Controllers/ServiceAccountsControllerTests.cs index 83e013973..f267f36b4 100644 --- a/test/Api.Test/SecretsManager/Controllers/ServiceAccountsControllerTests.cs +++ b/test/Api.Test/SecretsManager/Controllers/ServiceAccountsControllerTests.cs @@ -14,7 +14,6 @@ using Bit.Test.Common.AutoFixture.Attributes; using Bit.Test.Common.Helpers; using Microsoft.AspNetCore.Authorization; using NSubstitute; -using NSubstitute.ReturnsExtensions; using Xunit; namespace Bit.Api.Test.SecretsManager.Controllers; @@ -26,37 +25,43 @@ public class ServiceAccountsControllerTests { [Theory] [BitAutoData] - public async void GetServiceAccountsByOrganization_ReturnsEmptyList(SutProvider sutProvider, Guid id) + public async void GetServiceAccountsByOrganization_ReturnsEmptyList( + SutProvider sutProvider, Guid id) { sutProvider.GetDependency().AccessSecretsManager(id).Returns(true); sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(Guid.NewGuid()); var result = await sutProvider.Sut.ListByOrganizationAsync(id); await sutProvider.GetDependency().Received(1) - .GetManyByOrganizationIdAsync(Arg.Is(AssertHelper.AssertPropertyEqual(id)), Arg.Any(), Arg.Any()); + .GetManyByOrganizationIdAsync(Arg.Is(AssertHelper.AssertPropertyEqual(id)), Arg.Any(), + Arg.Any()); Assert.Empty(result.Data); } [Theory] [BitAutoData] - public async void GetServiceAccountsByOrganization_Success(SutProvider sutProvider, ServiceAccount resultServiceAccount) + public async void GetServiceAccountsByOrganization_Success(SutProvider sutProvider, + ServiceAccount resultServiceAccount) { sutProvider.GetDependency().AccessSecretsManager(default).ReturnsForAnyArgs(true); sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(Guid.NewGuid()); - sutProvider.GetDependency().GetManyByOrganizationIdAsync(default, default, default).ReturnsForAnyArgs(new List() { resultServiceAccount }); + sutProvider.GetDependency().GetManyByOrganizationIdAsync(default, default, default) + .ReturnsForAnyArgs(new List { resultServiceAccount }); var result = await sutProvider.Sut.ListByOrganizationAsync(resultServiceAccount.OrganizationId); await sutProvider.GetDependency().Received(1) - .GetManyByOrganizationIdAsync(Arg.Is(AssertHelper.AssertPropertyEqual(resultServiceAccount.OrganizationId)), Arg.Any(), Arg.Any()); + .GetManyByOrganizationIdAsync(Arg.Is(AssertHelper.AssertPropertyEqual(resultServiceAccount.OrganizationId)), + Arg.Any(), Arg.Any()); Assert.NotEmpty(result.Data); Assert.Single(result.Data); } [Theory] [BitAutoData] - public async void GetServiceAccountsByOrganization_AccessDenied_Throws(SutProvider sutProvider, Guid orgId) + public async void GetServiceAccountsByOrganization_AccessDenied_Throws( + SutProvider sutProvider, Guid orgId) { sutProvider.GetDependency().AccessSecretsManager(default).ReturnsForAnyArgs(false); @@ -66,14 +71,16 @@ public class ServiceAccountsControllerTests [Theory] [BitAutoData] - public async void CreateServiceAccount_NoAccess_Throws(SutProvider sutProvider, ServiceAccountCreateRequestModel data, Guid organizationId) + public async void CreateServiceAccount_NoAccess_Throws(SutProvider sutProvider, + ServiceAccountCreateRequestModel data, Guid organizationId) { sutProvider.GetDependency() .AuthorizeAsync(Arg.Any(), data.ToServiceAccount(organizationId), Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Failed()); sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(Guid.NewGuid()); var resultServiceAccount = data.ToServiceAccount(organizationId); - sutProvider.GetDependency().CreateAsync(default, default).ReturnsForAnyArgs(resultServiceAccount); + sutProvider.GetDependency().CreateAsync(default, default) + .ReturnsForAnyArgs(resultServiceAccount); await Assert.ThrowsAsync(() => sutProvider.Sut.CreateAsync(organizationId, data)); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() @@ -82,30 +89,35 @@ public class ServiceAccountsControllerTests [Theory] [BitAutoData] - public async void CreateServiceAccount_Success(SutProvider sutProvider, ServiceAccountCreateRequestModel data, Guid organizationId) + public async void CreateServiceAccount_Success(SutProvider sutProvider, + ServiceAccountCreateRequestModel data, Guid organizationId) { sutProvider.GetDependency() .AuthorizeAsync(Arg.Any(), data.ToServiceAccount(organizationId), Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(Guid.NewGuid()); var resultServiceAccount = data.ToServiceAccount(organizationId); - sutProvider.GetDependency().CreateAsync(default, default).ReturnsForAnyArgs(resultServiceAccount); + sutProvider.GetDependency().CreateAsync(default, default) + .ReturnsForAnyArgs(resultServiceAccount); await sutProvider.Sut.CreateAsync(organizationId, data); await sutProvider.GetDependency().Received(1) - .CreateAsync(Arg.Any(), Arg.Any()); + .CreateAsync(Arg.Any(), Arg.Any()); } [Theory] [BitAutoData] - public async void UpdateServiceAccount_NoAccess_Throws(SutProvider sutProvider, ServiceAccountUpdateRequestModel data, ServiceAccount existingServiceAccount) + public async void UpdateServiceAccount_NoAccess_Throws(SutProvider sutProvider, + ServiceAccountUpdateRequestModel data, ServiceAccount existingServiceAccount) { sutProvider.GetDependency() .AuthorizeAsync(Arg.Any(), data.ToServiceAccount(existingServiceAccount.Id), Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Failed()); - sutProvider.GetDependency().GetByIdAsync(existingServiceAccount.Id).ReturnsForAnyArgs(existingServiceAccount); + sutProvider.GetDependency().GetByIdAsync(existingServiceAccount.Id) + .ReturnsForAnyArgs(existingServiceAccount); var resultServiceAccount = data.ToServiceAccount(existingServiceAccount.Id); - sutProvider.GetDependency().UpdateAsync(default).ReturnsForAnyArgs(resultServiceAccount); + sutProvider.GetDependency().UpdateAsync(default) + .ReturnsForAnyArgs(resultServiceAccount); await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateAsync(existingServiceAccount.Id, data)); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() @@ -114,102 +126,136 @@ public class ServiceAccountsControllerTests [Theory] [BitAutoData] - public async void UpdateServiceAccount_Success(SutProvider sutProvider, ServiceAccountUpdateRequestModel data, ServiceAccount existingServiceAccount) + public async void UpdateServiceAccount_Success(SutProvider sutProvider, + ServiceAccountUpdateRequestModel data, ServiceAccount existingServiceAccount) { sutProvider.GetDependency() .AuthorizeAsync(Arg.Any(), data.ToServiceAccount(existingServiceAccount.Id), Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); var resultServiceAccount = data.ToServiceAccount(existingServiceAccount.Id); - sutProvider.GetDependency().UpdateAsync(default).ReturnsForAnyArgs(resultServiceAccount); + sutProvider.GetDependency().UpdateAsync(default) + .ReturnsForAnyArgs(resultServiceAccount); var result = await sutProvider.Sut.UpdateAsync(existingServiceAccount.Id, data); await sutProvider.GetDependency().Received(1) - .UpdateAsync(Arg.Any()); + .UpdateAsync(Arg.Any()); } [Theory] [BitAutoData] - public async void CreateAccessToken_Success(SutProvider sutProvider, AccessTokenCreateRequestModel data, Guid serviceAccountId) + public async void CreateAccessToken_NoAccess_Throws(SutProvider sutProvider, + AccessTokenCreateRequestModel data, ServiceAccount serviceAccount) { - sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(Guid.NewGuid()); - var resultAccessToken = data.ToApiKey(serviceAccountId); + sutProvider.GetDependency().GetByIdAsync(serviceAccount.Id).Returns(serviceAccount); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), serviceAccount, + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Failed()); + var resultAccessToken = data.ToApiKey(serviceAccount.Id); - sutProvider.GetDependency().CreateAsync(default, default).ReturnsForAnyArgs(resultAccessToken); + sutProvider.GetDependency().CreateAsync(default) + .ReturnsForAnyArgs(resultAccessToken); - var result = await sutProvider.Sut.CreateAccessTokenAsync(serviceAccountId, data); + await Assert.ThrowsAsync(() => + sutProvider.Sut.CreateAccessTokenAsync(serviceAccount.Id, data)); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .CreateAsync(Arg.Any()); + } + + [Theory] + [BitAutoData] + public async void CreateAccessToken_Success(SutProvider sutProvider, + AccessTokenCreateRequestModel data, ServiceAccount serviceAccount) + { + sutProvider.GetDependency().GetByIdAsync(serviceAccount.Id).Returns(serviceAccount); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), serviceAccount, + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); + var resultAccessToken = data.ToApiKey(serviceAccount.Id); + + sutProvider.GetDependency().CreateAsync(default) + .ReturnsForAnyArgs(resultAccessToken); + + await sutProvider.Sut.CreateAccessTokenAsync(serviceAccount.Id, data); await sutProvider.GetDependency().Received(1) - .CreateAsync(Arg.Any(), Arg.Any()); + .CreateAsync(Arg.Any()); } [Theory] [BitAutoData] - public async void GetAccessTokens_NoServiceAccount_ThrowsNotFound(SutProvider sutProvider, Guid serviceAccountId) + public async void GetAccessTokens_NoAccess_Throws(SutProvider sutProvider, + ServiceAccount data, ICollection resultApiKeys) { - sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(Guid.NewGuid()); - sutProvider.GetDependency().GetByIdAsync(default).ReturnsNull(); - - await Assert.ThrowsAsync(() => sutProvider.Sut.GetAccessTokens(serviceAccountId)); - - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetManyByServiceAccountIdAsync(Arg.Any()); - } - - [Theory] - [BitAutoData] - public async void GetAccessTokens_Admin_Success(SutProvider sutProvider, ServiceAccount data, Guid userId, ICollection resultApiKeys) - { - sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); - sutProvider.GetDependency().AccessSecretsManager(data.OrganizationId).Returns(true); - sutProvider.GetDependency().OrganizationAdmin(data.OrganizationId).Returns(true); sutProvider.GetDependency().GetByIdAsync(default).ReturnsForAnyArgs(data); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), data, + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Failed()); + foreach (var apiKey in resultApiKeys) { apiKey.Scope = "[\"api.secrets\"]"; } - sutProvider.GetDependency().GetManyByServiceAccountIdAsync(default).ReturnsForAnyArgs(resultApiKeys); - var result = await sutProvider.Sut.GetAccessTokens(data.Id); - await sutProvider.GetDependency().Received(1).GetManyByServiceAccountIdAsync(Arg.Any()); - Assert.NotEmpty(result.Data); - Assert.Equal(resultApiKeys.Count, result.Data.Count()); - } - - [Theory] - [BitAutoData] - public async void GetAccessTokens_User_Success(SutProvider sutProvider, ServiceAccount data, Guid userId, ICollection resultApiKeys) - { - sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); - sutProvider.GetDependency().AccessSecretsManager(data.OrganizationId).Returns(true); - sutProvider.GetDependency().OrganizationAdmin(data.OrganizationId).Returns(false); - sutProvider.GetDependency().UserHasReadAccessToServiceAccount(default, default).ReturnsForAnyArgs(true); - sutProvider.GetDependency().GetByIdAsync(default).ReturnsForAnyArgs(data); - foreach (var apiKey in resultApiKeys) - { - apiKey.Scope = "[\"api.secrets\"]"; - } - sutProvider.GetDependency().GetManyByServiceAccountIdAsync(default).ReturnsForAnyArgs(resultApiKeys); - - var result = await sutProvider.Sut.GetAccessTokens(data.Id); - await sutProvider.GetDependency().Received(1).GetManyByServiceAccountIdAsync(Arg.Any()); - Assert.NotEmpty(result.Data); - Assert.Equal(resultApiKeys.Count, result.Data.Count()); - } - - [Theory] - [BitAutoData] - public async void GetAccessTokens_UserNoAccess_ThrowsNotFound(SutProvider sutProvider, ServiceAccount data, Guid userId, ICollection resultApiKeys) - { - sutProvider.GetDependency().GetProperUserId(default).ReturnsForAnyArgs(userId); - sutProvider.GetDependency().OrganizationAdmin(data.OrganizationId).Returns(false); - sutProvider.GetDependency().UserHasReadAccessToServiceAccount(default, default).ReturnsForAnyArgs(false); - sutProvider.GetDependency().GetByIdAsync(default).ReturnsForAnyArgs(data); - foreach (var apiKey in resultApiKeys) - { - apiKey.Scope = "[\"api.secrets\"]"; - } - sutProvider.GetDependency().GetManyByServiceAccountIdAsync(default).ReturnsForAnyArgs(resultApiKeys); + sutProvider.GetDependency().GetManyByServiceAccountIdAsync(default) + .ReturnsForAnyArgs(resultApiKeys); await Assert.ThrowsAsync(() => sutProvider.Sut.GetAccessTokens(data.Id)); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .GetManyByServiceAccountIdAsync(Arg.Any()); + } - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetManyByServiceAccountIdAsync(Arg.Any()); + [Theory] + [BitAutoData] + public async void GetAccessTokens_Success(SutProvider sutProvider, ServiceAccount data, + ICollection resultApiKeys) + { + sutProvider.GetDependency().GetByIdAsync(default).ReturnsForAnyArgs(data); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), data, + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); + + foreach (var apiKey in resultApiKeys) + { + apiKey.Scope = "[\"api.secrets\"]"; + } + + sutProvider.GetDependency().GetManyByServiceAccountIdAsync(default) + .ReturnsForAnyArgs(resultApiKeys); + + var result = await sutProvider.Sut.GetAccessTokens(data.Id); + await sutProvider.GetDependency().Received(1) + .GetManyByServiceAccountIdAsync(Arg.Any()); + Assert.NotEmpty(result.Data); + Assert.Equal(resultApiKeys.Count, result.Data.Count()); + } + + [Theory] + [BitAutoData] + public async void RevokeAccessTokens_NoAccess_Throws(SutProvider sutProvider, + RevokeAccessTokensRequest data, ServiceAccount serviceAccount) + { + sutProvider.GetDependency().GetByIdAsync(serviceAccount.Id).Returns(serviceAccount); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), serviceAccount, + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Failed()); + + await Assert.ThrowsAsync(() => + sutProvider.Sut.RevokeAccessTokensAsync(serviceAccount.Id, data)); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .RevokeAsync(Arg.Any(), Arg.Any()); + } + + [Theory] + [BitAutoData] + public async void RevokeAccessTokens_Success(SutProvider sutProvider, + RevokeAccessTokensRequest data, ServiceAccount serviceAccount) + { + sutProvider.GetDependency().GetByIdAsync(serviceAccount.Id).Returns(serviceAccount); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), serviceAccount, + Arg.Any>()).ReturnsForAnyArgs(AuthorizationResult.Success()); + + await sutProvider.Sut.RevokeAccessTokensAsync(serviceAccount.Id, data); + await sutProvider.GetDependency().Received(1) + .RevokeAsync(Arg.Any(), Arg.Any()); } }