From 7334de636bc7e158d26e51cb61b62303a47f9e1f Mon Sep 17 00:00:00 2001
From: cd-bitwarden <106776772+cd-bitwarden@users.noreply.github.com>
Date: Tue, 7 Mar 2023 13:22:03 -0500
Subject: [PATCH] =?UTF-8?q?[SM-577]=20-=20ACCESS=20POLICY=20fixing=20issue?=
 =?UTF-8?q?=20with=20user=20being=20able=20to=20update=20a=20secret=20if?=
 =?UTF-8?q?=20they=20are=20assi=E2=80=A6=20(#2763)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fixing issue with user being able to update a secret if they are assigning it to a project that has read/write permissions. Even though the customer is only allowed to read.

* Add additional check for newly assigned project access and original project access.

* fixing Lint issue

* Fixing tests

* uneeded param removed

* Updating to extract logic into function

* renaming function

* lint fixes

* renaming function
---
 .../Commands/Secrets/UpdateSecretCommand.cs   | 33 ++++++++++---------
 .../Secrets/UpdateSecretCommandTests.cs       |  2 +-
 2 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Secrets/UpdateSecretCommand.cs b/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Secrets/UpdateSecretCommand.cs
index c7f6a9c52e31..1d55114442c0 100644
--- a/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Secrets/UpdateSecretCommand.cs
+++ b/bitwarden_license/src/Commercial.Core/SecretsManager/Commands/Secrets/UpdateSecretCommand.cs
@@ -31,21 +31,7 @@ public async Task<Secret> UpdateAsync(Secret updatedSecret, Guid userId)
         var orgAdmin = await _currentContext.OrganizationAdmin(secret.OrganizationId);
         var accessClient = AccessClientHelper.ToAccessClient(_currentContext.ClientType, orgAdmin);
 
-        var project = updatedSecret.Projects?.FirstOrDefault();
-
-        if (secret.Projects != null && secret.Projects.Any() && project == null)
-        {
-            throw new NotFoundException();
-        }
-
-        var hasAccess = accessClient switch
-        {
-            AccessClientType.NoAccessCheck => true,
-            AccessClientType.User => project != null && await _projectRepository.UserHasWriteAccessToProject(project.Id, userId),
-            _ => false,
-        };
-
-        if (!hasAccess)
+        if (!await HasAccessToOriginalAndUpdatedProject(accessClient, secret, updatedSecret, userId))
         {
             throw new NotFoundException();
         }
@@ -59,4 +45,21 @@ public async Task<Secret> UpdateAsync(Secret updatedSecret, Guid userId)
         await _secretRepository.UpdateAsync(secret);
         return secret;
     }
+
+    public async Task<bool> HasAccessToOriginalAndUpdatedProject(AccessClientType accessClient, Secret secret, Secret updatedSecret, Guid userId)
+    {
+        switch (accessClient)
+        {
+            case AccessClientType.NoAccessCheck:
+                return true;
+            case AccessClientType.User:
+                var oldProject = secret.Projects?.FirstOrDefault();
+                var newProject = updatedSecret.Projects?.FirstOrDefault();
+                var accessToOld = oldProject != null && await _projectRepository.UserHasWriteAccessToProject(oldProject.Id, userId);
+                var accessToNew = newProject != null && await _projectRepository.UserHasWriteAccessToProject(newProject.Id, userId);
+                return accessToOld && accessToNew;
+            default:
+                return false;
+        }
+    }
 }
diff --git a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Secrets/UpdateSecretCommandTests.cs b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Secrets/UpdateSecretCommandTests.cs
index faa6e7ec5eb2..5fe37c6974af 100644
--- a/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Secrets/UpdateSecretCommandTests.cs
+++ b/bitwarden_license/test/Commercial.Core.Test/SecretsManager/Secrets/UpdateSecretCommandTests.cs
@@ -34,6 +34,7 @@ public async Task UpdateAsync_SecretDoesNotExist_ThrowsNotFound(Secret data, Sut
     public async Task UpdateAsync_Success(PermissionType permissionType, Secret data, SutProvider<UpdateSecretCommand> sutProvider, Guid userId, Project mockProject)
     {
         sutProvider.GetDependency<ICurrentContext>().AccessSecretsManager(data.OrganizationId).Returns(true);
+        data.Projects = new List<Project>() { mockProject };
 
         if (permissionType == PermissionType.RunAsAdmin)
         {
@@ -41,7 +42,6 @@ public async Task UpdateAsync_Success(PermissionType permissionType, Secret data
         }
         else
         {
-            data.Projects = new List<Project>() { mockProject };
             sutProvider.GetDependency<ICurrentContext>().OrganizationAdmin(data.OrganizationId).Returns(false);
             sutProvider.GetDependency<IProjectRepository>().UserHasWriteAccessToProject((Guid)(data.Projects?.First().Id), userId).Returns(true);
         }