[SM-788] Extract authorization from secret delete command (#3003)

* Extract authorization from secret delete command
2025-06-30 15:42:48 -05:00 · 2023-06-27 13:12:34 -05:00
parent c1723d9e90
commit d020c49c0e
9 changed files with 287 additions and 196 deletions
--- a/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs
+++ b/test/Api.IntegrationTest/SecretsManager/Controllers/SecretsControllerTests.cs
@ -644,10 +644,28 @@ public class SecretsControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
        });
        var secretIds = new[] { secret.Id };

-        var response = await _client.PostAsJsonAsync($"/secrets/{org.Id}/delete", secretIds);
+        var response = await _client.PostAsJsonAsync($"/secrets/delete", secretIds);
        Assert.Equal(HttpStatusCode.NotFound, 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 (_, secretIds) = await CreateSecretsAsync(org.Id, 3);
+
+        var response = await _client.PostAsync("/secrets/delete", JsonContent.Create(secretIds));
+
+        var results = await response.Content.ReadFromJsonAsync<ListResponseModel<BulkDeleteResponseModel>>();
+        Assert.NotNull(results);
+        Assert.Equal(secretIds.OrderBy(x => x),
+            results!.Data.Select(x => x.Id).OrderBy(x => x));
+        Assert.All(results.Data, item => Assert.Equal("access denied", item.Error));
+    }
+
    [Theory]
    [InlineData(PermissionType.RunAsAdmin)]
    [InlineData(PermissionType.RunAsUserWithPermission)]
@ -656,12 +674,7 @@ public class SecretsControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
        var (org, _) = await _organizationHelper.Initialize(true, true);
        await LoginAsync(_email);

-        var project = await _projectRepository.CreateAsync(new Project()
-        {
-            Id = new Guid(),
-            OrganizationId = org.Id,
-            Name = _mockEncryptedString
-        });
+        var (project, secretIds) = await CreateSecretsAsync(org.Id, 3);

        if (permissionType == PermissionType.RunAsUserWithPermission)
        {
@ -678,21 +691,6 @@ public class SecretsControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
            await _accessPolicyRepository.CreateManyAsync(accessPolicies);
        }

-
-        var secretIds = new List<Guid>();
-        for (var i = 0; i < 3; i++)
-        {
-            var secret = await _secretRepository.CreateAsync(new Secret
-            {
-                OrganizationId = org.Id,
-                Key = _mockEncryptedString,
-                Value = _mockEncryptedString,
-                Note = _mockEncryptedString,
-                Projects = new List<Project>() { project }
-            });
-            secretIds.Add(secret.Id);
-        }
-
        var response = await _client.PostAsJsonAsync($"/secrets/delete", secretIds);
        response.EnsureSuccessStatusCode();

@ -710,4 +708,30 @@ public class SecretsControllerTests : IClassFixture<ApiApplicationFactory>, IAsy
        var secrets = await _secretRepository.GetManyByIds(secretIds);
        Assert.Empty(secrets);
    }
+
+    private async Task<(Project Project, List<Guid> secretIds)> CreateSecretsAsync(Guid orgId, int numberToCreate = 3)
+    {
+        var project = await _projectRepository.CreateAsync(new Project
+        {
+            Id = new Guid(),
+            OrganizationId = orgId,
+            Name = _mockEncryptedString
+        });
+
+        var secretIds = new List<Guid>();
+        for (var i = 0; i < numberToCreate; i++)
+        {
+            var secret = await _secretRepository.CreateAsync(new Secret
+            {
+                OrganizationId = orgId,
+                Key = _mockEncryptedString,
+                Value = _mockEncryptedString,
+                Note = _mockEncryptedString,
+                Projects = new List<Project>() { project }
+            });
+            secretIds.Add(secret.Id);
+        }
+
+        return (project, secretIds);
+    }
 }
--- a/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs
+++ b/test/Api.Test/SecretsManager/Controllers/SecretsControllerTests.cs
@ -244,45 +244,106 @@ public class SecretsControllerTests
    }

    [Theory]
-    [BitAutoData(PermissionType.RunAsAdmin)]
-    [BitAutoData(PermissionType.RunAsUserWithPermission)]
-    public async void BulkDeleteSecret_Success(PermissionType permissionType, SutProvider<SecretsController> sutProvider, List<Secret> data, Guid organizationId, Guid userId, Project mockProject)
+    [BitAutoData]
+    public async void BulkDelete_NoSecretsFound_ThrowsNotFound(SutProvider<SecretsController> sutProvider, List<Secret> data)
    {
-        sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(userId);
-
-        if (permissionType == PermissionType.RunAsAdmin)
-        {
-            sutProvider.GetDependency<ICurrentContext>().OrganizationAdmin(organizationId).Returns(true);
-        }
-        else
-        {
-            data.FirstOrDefault().Projects = new List<Project>() { mockProject };
-            sutProvider.GetDependency<ICurrentContext>().OrganizationAdmin(organizationId).Returns(false);
-            sutProvider.GetDependency<IProjectRepository>().AccessToProjectAsync(default, default, default)
-                .Returns((true, true));
-        }
-
-
-        var ids = data.Select(secret => secret.Id).ToList();
-        var mockResult = new List<Tuple<Secret, string>>();
-
-        foreach (var secret in data)
-        {
-            mockResult.Add(new Tuple<Secret, string>(secret, ""));
-        }
-        sutProvider.GetDependency<IDeleteSecretCommand>().DeleteSecrets(ids, userId).ReturnsForAnyArgs(mockResult);
-
-        var results = await sutProvider.Sut.BulkDeleteAsync(ids);
-        await sutProvider.GetDependency<IDeleteSecretCommand>().Received(1)
-                     .DeleteSecrets(Arg.Is(ids), userId);
-        Assert.Equal(data.Count, results.Data.Count());
+        var ids = data.Select(s => s.Id).ToList();
+        sutProvider.GetDependency<ISecretRepository>().GetManyByIds(Arg.Is(ids)).ReturnsForAnyArgs(new List<Secret>());
+        await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.BulkDeleteAsync(ids));
+        await sutProvider.GetDependency<IDeleteSecretCommand>().DidNotReceiveWithAnyArgs().DeleteSecrets(Arg.Any<List<Secret>>());
    }

    [Theory]
    [BitAutoData]
-    public async void BulkDeleteSecret_NoGuids_ThrowsArgumentNullException(SutProvider<SecretsController> sutProvider)
+    public async void BulkDelete_SecretsFoundMisMatch_ThrowsNotFound(SutProvider<SecretsController> sutProvider, List<Secret> data, Secret mockSecret)
    {
-        sutProvider.GetDependency<IUserService>().GetProperUserId(default).ReturnsForAnyArgs(new Guid());
-        await Assert.ThrowsAsync<ArgumentNullException>(() => sutProvider.Sut.BulkDeleteAsync(new List<Guid>()));
+        data.Add(mockSecret);
+        var ids = data.Select(s => s.Id).ToList();
+        sutProvider.GetDependency<ISecretRepository>().GetManyByIds(Arg.Is(ids)).ReturnsForAnyArgs(new List<Secret> { mockSecret });
+        await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.BulkDeleteAsync(ids));
+        await sutProvider.GetDependency<IDeleteSecretCommand>().DidNotReceiveWithAnyArgs().DeleteSecrets(Arg.Any<List<Secret>>());
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async void BulkDelete_OrganizationMistMatch_ThrowsNotFound(SutProvider<SecretsController> sutProvider, List<Secret> data)
+    {
+        var ids = data.Select(s => s.Id).ToList();
+        sutProvider.GetDependency<ISecretRepository>().GetManyByIds(Arg.Is(ids)).ReturnsForAnyArgs(data);
+        await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.BulkDeleteAsync(ids));
+        await sutProvider.GetDependency<IDeleteSecretCommand>().DidNotReceiveWithAnyArgs().DeleteSecrets(Arg.Any<List<Secret>>());
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async void BulkDelete_NoAccessToSecretsManager_ThrowsNotFound(SutProvider<SecretsController> sutProvider, List<Secret> data)
+    {
+        var ids = data.Select(s => s.Id).ToList();
+        var organizationId = data.First().OrganizationId;
+        foreach (var s in data)
+        {
+            s.OrganizationId = organizationId;
+        }
+        sutProvider.GetDependency<ICurrentContext>().AccessSecretsManager(Arg.Is(organizationId)).ReturnsForAnyArgs(false);
+        sutProvider.GetDependency<ISecretRepository>().GetManyByIds(Arg.Is(ids)).ReturnsForAnyArgs(data);
+        await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.BulkDeleteAsync(ids));
+        await sutProvider.GetDependency<IDeleteSecretCommand>().DidNotReceiveWithAnyArgs().DeleteSecrets(Arg.Any<List<Secret>>());
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async void BulkDelete_ReturnsAccessDeniedForSecretsWithoutAccess_Success(SutProvider<SecretsController> sutProvider, List<Secret> data)
+    {
+        var ids = data.Select(s => s.Id).ToList();
+        var organizationId = data.First().OrganizationId;
+        foreach (var secret in data)
+        {
+            secret.OrganizationId = organizationId;
+            sutProvider.GetDependency<IAuthorizationService>()
+                .AuthorizeAsync(Arg.Any<ClaimsPrincipal>(), secret,
+                    Arg.Any<IEnumerable<IAuthorizationRequirement>>()).ReturnsForAnyArgs(AuthorizationResult.Success());
+        }
+        sutProvider.GetDependency<IAuthorizationService>()
+            .AuthorizeAsync(Arg.Any<ClaimsPrincipal>(), data.First(),
+                Arg.Any<IEnumerable<IAuthorizationRequirement>>()).Returns(AuthorizationResult.Failed());
+        sutProvider.GetDependency<ICurrentContext>().AccessSecretsManager(Arg.Is(organizationId)).ReturnsForAnyArgs(true);
+        sutProvider.GetDependency<ISecretRepository>().GetManyByIds(Arg.Is(ids)).ReturnsForAnyArgs(data);
+
+        var results = await sutProvider.Sut.BulkDeleteAsync(ids);
+
+        Assert.Equal(data.Count, results.Data.Count());
+        Assert.Equal("access denied", results.Data.First().Error);
+
+        data.Remove(data.First());
+        await sutProvider.GetDependency<IDeleteSecretCommand>().Received(1)
+            .DeleteSecrets(Arg.Is(AssertHelper.AssertPropertyEqual(data)));
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async void BulkDelete_Success(SutProvider<SecretsController> sutProvider, List<Secret> data)
+    {
+        var ids = data.Select(sa => sa.Id).ToList();
+        var organizationId = data.First().OrganizationId;
+        foreach (var secret in data)
+        {
+            secret.OrganizationId = organizationId;
+            sutProvider.GetDependency<IAuthorizationService>()
+                .AuthorizeAsync(Arg.Any<ClaimsPrincipal>(), secret,
+                    Arg.Any<IEnumerable<IAuthorizationRequirement>>()).ReturnsForAnyArgs(AuthorizationResult.Success());
+        }
+
+        sutProvider.GetDependency<ICurrentContext>().AccessSecretsManager(Arg.Is(organizationId)).ReturnsForAnyArgs(true);
+        sutProvider.GetDependency<ISecretRepository>().GetManyByIds(Arg.Is(ids)).ReturnsForAnyArgs(data);
+
+        var results = await sutProvider.Sut.BulkDeleteAsync(ids);
+
+        await sutProvider.GetDependency<IDeleteSecretCommand>().Received(1)
+            .DeleteSecrets(Arg.Is(AssertHelper.AssertPropertyEqual(data)));
+        Assert.Equal(data.Count, results.Data.Count());
+        foreach (var result in results.Data)
+        {
+            Assert.Null(result.Error);
+        }
    }
 }