From 1d09b88ade1a101ce06582919c1668225728925c Mon Sep 17 00:00:00 2001
From: Justin Baur <19896123+justindbaur@users.noreply.github.com>
Date: Wed, 3 Jul 2024 15:17:10 -0400
Subject: [PATCH] [PM-2944] Enable Nullable For Secrets Manager (#4389)

* Enable `nullable` for `ApiKey`

* Switch to Using `required`

* Make Scope Be Valid JSON

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

Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com>

* Move Nullable Directive

---------

Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com>
Co-authored-by: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com>
---
 .../RevokeAccessTokenCommandTests.cs          | 12 +++-
 src/Core/SecretsManager/Entities/ApiKey.cs    | 14 ++--
 .../Models/Data/ApiKeyDetails.cs              |  4 +-
 .../ServiceAccountsControllerTests.cs         | 64 ++++++++++---------
 4 files changed, 54 insertions(+), 40 deletions(-)

diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/ServiceAccounts/RevokeAccessTokenCommandTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/ServiceAccounts/RevokeAccessTokenCommandTests.cs
index 2bf006165e..203bdd322a 100644
--- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/ServiceAccounts/RevokeAccessTokenCommandTests.cs
+++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Commands/ServiceAccounts/RevokeAccessTokenCommandTests.cs
@@ -18,13 +18,21 @@ public class RevokeAccessTokenCommandTests
         var apiKey1 = new ApiKey
         {
             Id = Guid.NewGuid(),
-            ServiceAccountId = serviceAccount.Id
+            ServiceAccountId = serviceAccount.Id,
+            Name = "Test Name",
+            Scope = "Test Scope",
+            EncryptedPayload = "Test EncryptedPayload",
+            Key = "Test Key",
         };
 
         var apiKey2 = new ApiKey
         {
             Id = Guid.NewGuid(),
-            ServiceAccountId = serviceAccount.Id
+            ServiceAccountId = serviceAccount.Id,
+            Name = "Test Name",
+            Scope = "Test Scope",
+            EncryptedPayload = "Test EncryptedPayload",
+            Key = "Test Key",
         };
 
         sutProvider.GetDependency<IApiKeyRepository>()
diff --git a/src/Core/SecretsManager/Entities/ApiKey.cs b/src/Core/SecretsManager/Entities/ApiKey.cs
index 2f9c5b48c2..e45cf8e735 100644
--- a/src/Core/SecretsManager/Entities/ApiKey.cs
+++ b/src/Core/SecretsManager/Entities/ApiKey.cs
@@ -1,4 +1,6 @@
-using System.ComponentModel.DataAnnotations;
+#nullable enable
+
+using System.ComponentModel.DataAnnotations;
 using Bit.Core.Entities;
 using Bit.Core.Utilities;
 
@@ -9,15 +11,15 @@ public class ApiKey : ITableObject<Guid>
     public Guid Id { get; set; }
     public Guid? ServiceAccountId { get; set; }
     [MaxLength(200)]
-    public string Name { get; set; }
+    public required string Name { get; set; }
     [MaxLength(128)]
-    public string ClientSecretHash { get; set; }
+    public string? ClientSecretHash { get; set; }
     [MaxLength(4000)]
-    public string Scope { get; set; }
+    public required string Scope { get; set; }
     [MaxLength(4000)]
-    public string EncryptedPayload { get; set; }
+    public required string EncryptedPayload { get; set; }
     // Key for decrypting `EncryptedPayload`. Encrypted using the organization key.
-    public string Key { get; set; }
+    public required string Key { get; set; }
     public DateTime? ExpireAt { get; set; }
     public DateTime CreationDate { get; internal set; } = DateTime.UtcNow;
     public DateTime RevisionDate { get; internal set; } = DateTime.UtcNow;
diff --git a/src/Core/SecretsManager/Models/Data/ApiKeyDetails.cs b/src/Core/SecretsManager/Models/Data/ApiKeyDetails.cs
index f8945e9610..47fea5a52e 100644
--- a/src/Core/SecretsManager/Models/Data/ApiKeyDetails.cs
+++ b/src/Core/SecretsManager/Models/Data/ApiKeyDetails.cs
@@ -1,4 +1,5 @@
-using Bit.Core.SecretsManager.Entities;
+using System.Diagnostics.CodeAnalysis;
+using Bit.Core.SecretsManager.Entities;
 
 namespace Bit.Core.SecretsManager.Models.Data;
 
@@ -28,6 +29,7 @@ public class ServiceAccountApiKeyDetails : ApiKeyDetails
 
     }
 
+    [SetsRequiredMembers]
     public ServiceAccountApiKeyDetails(ApiKey apiKey, Guid organizationId) : base(apiKey)
     {
         ServiceAccountOrganizationId = organizationId;
diff --git a/test/Api.IntegrationTest/SecretsManager/Controllers/ServiceAccountsControllerTests.cs b/test/Api.IntegrationTest/SecretsManager/Controllers/ServiceAccountsControllerTests.cs
index ae7b522750..6884de5b26 100644
--- a/test/Api.IntegrationTest/SecretsManager/Controllers/ServiceAccountsControllerTests.cs
+++ b/test/Api.IntegrationTest/SecretsManager/Controllers/ServiceAccountsControllerTests.cs
@@ -1,4 +1,5 @@
 using System.Net;
+using System.Text.Json.Nodes;
 using Bit.Api.IntegrationTest.Factories;
 using Bit.Api.IntegrationTest.SecretsManager.Enums;
 using Bit.Api.IntegrationTest.SecretsManager.Helpers;
@@ -440,7 +441,7 @@ public class ServiceAccountsControllerTests : IClassFixture<ApiApplicationFactor
             Name = _mockEncryptedString,
         });
 
-        await _apiKeyRepository.CreateAsync(new ApiKey { ServiceAccountId = serviceAccount.Id });
+        await _apiKeyRepository.CreateAsync(CreateTestApiKey(serviceAccount.Id));
 
         if (permissionType == PermissionType.RunAsAdmin)
         {
@@ -507,12 +508,9 @@ public class ServiceAccountsControllerTests : IClassFixture<ApiApplicationFactor
             Name = _mockEncryptedString,
         });
 
-        await _apiKeyRepository.CreateAsync(new ApiKey
-        {
-            ServiceAccountId = serviceAccount.Id,
-            Name = _mockEncryptedString,
-            ExpireAt = DateTime.UtcNow.AddDays(30),
-        });
+        await _apiKeyRepository.CreateAsync(
+            CreateTestApiKey(serviceAccount.Id, _mockEncryptedString, DateTime.UtcNow.AddDays(30))
+        );
 
         var response = await _client.GetAsync($"/service-accounts/{serviceAccount.Id}/access-tokens");
         Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
@@ -548,12 +546,9 @@ public class ServiceAccountsControllerTests : IClassFixture<ApiApplicationFactor
             });
         }
 
-        var accessToken = await _apiKeyRepository.CreateAsync(new ApiKey
-        {
-            ServiceAccountId = serviceAccount.Id,
-            Name = _mockEncryptedString,
-            ExpireAt = DateTime.UtcNow.AddDays(30),
-        });
+        var accessToken = await _apiKeyRepository.CreateAsync(
+            CreateTestApiKey(serviceAccount.Id, _mockEncryptedString, DateTime.UtcNow.AddDays(30))
+        );
 
 
         var response = await _client.GetAsync($"/service-accounts/{serviceAccount.Id}/access-tokens");
@@ -746,12 +741,9 @@ public class ServiceAccountsControllerTests : IClassFixture<ApiApplicationFactor
             Name = _mockEncryptedString
         });
 
-        var accessToken = await _apiKeyRepository.CreateAsync(new ApiKey
-        {
-            ServiceAccountId = serviceAccount.Id,
-            Name = _mockEncryptedString,
-            ExpireAt = DateTime.UtcNow.AddDays(30)
-        });
+        var accessToken = await _apiKeyRepository.CreateAsync(
+            CreateTestApiKey(serviceAccount.Id, _mockEncryptedString, DateTime.UtcNow.AddDays(30))
+        );
 
         var request = new RevokeAccessTokensRequest
         {
@@ -790,12 +782,9 @@ public class ServiceAccountsControllerTests : IClassFixture<ApiApplicationFactor
             });
         }
 
-        var accessToken = await _apiKeyRepository.CreateAsync(new ApiKey
-        {
-            ServiceAccountId = serviceAccount.Id,
-            Name = _mockEncryptedString,
-            ExpireAt = DateTime.UtcNow.AddDays(30)
-        });
+        var accessToken = await _apiKeyRepository.CreateAsync(
+            CreateTestApiKey(serviceAccount.Id, _mockEncryptedString, DateTime.UtcNow.AddDays(30))
+        );
 
         var request = new RevokeAccessTokensRequest
         {
@@ -839,12 +828,9 @@ public class ServiceAccountsControllerTests : IClassFixture<ApiApplicationFactor
             });
         }
 
-        var accessToken = await _apiKeyRepository.CreateAsync(new ApiKey
-        {
-            ServiceAccountId = serviceAccount.Id,
-            Name = _mockEncryptedString,
-            ExpireAt = DateTime.UtcNow.AddDays(30),
-        });
+        var accessToken = await _apiKeyRepository.CreateAsync(
+            CreateTestApiKey(serviceAccount.Id, _mockEncryptedString, DateTime.UtcNow.AddDays(30))
+        );
 
         var request = new RevokeAccessTokensRequest
         {
@@ -943,6 +929,22 @@ public class ServiceAccountsControllerTests : IClassFixture<ApiApplicationFactor
         return (org.Id, serviceAccountIds);
     }
 
+    private static ApiKey CreateTestApiKey(Guid serviceAccountId, string name = _mockEncryptedString, DateTime? expiresAt = null)
+    {
+        return new ApiKey
+        {
+            ServiceAccountId = serviceAccountId,
+            Name = name,
+            ExpireAt = expiresAt,
+            Scope = new JsonArray
+            {
+                "api.secrets",
+            }.ToJsonString(),
+            EncryptedPayload = _mockEncryptedString,
+            Key = _mockEncryptedString,
+        };
+    }
+
     private async Task<Dictionary<Guid, int>> SetupServiceAccountSecretAccessAsync(List<Guid> serviceAccountIds,
         Guid organizationId)
     {