From 7eaf7ab770a3c18553662ebd0daf0d3c0e0efd7f Mon Sep 17 00:00:00 2001
From: Matt Gibson <mgibson@bitwarden.com>
Date: Mon, 7 Dec 2020 19:35:34 -0600
Subject: [PATCH] [Bug] Fix cipher clone yielding incorrect RevisionDate
 (#1031)

* Fix cipher clone yielding incorrect RevisionDate

* PR fixes

Co-authored-by: Matt Gibson <mdgibson@Matts-MBP.lan>
---
 src/Api/Controllers/CiphersController.cs     |  4 ++--
 src/Core/Models/Table/Cipher.cs              | 12 +++++++++++-
 src/Core/Utilities/CoreHelpers.cs            |  5 +++++
 test/Core.Test/AutoFixture/CipherFixtures.cs | 15 ++++++++++++++-
 test/Core.Test/Models/CipherTests.cs         | 18 ++++++++++++++++++
 5 files changed, 50 insertions(+), 4 deletions(-)
 create mode 100644 test/Core.Test/Models/CipherTests.cs

diff --git a/src/Api/Controllers/CiphersController.cs b/src/Api/Controllers/CiphersController.cs
index fd5000d5d2..8eafd87756 100644
--- a/src/Api/Controllers/CiphersController.cs
+++ b/src/Api/Controllers/CiphersController.cs
@@ -187,7 +187,7 @@ namespace Bit.Api.Controllers
             }
 
             // object cannot be a descendant of CipherDetails, so let's clone it.
-            var cipherClone = CoreHelpers.CloneObject(model.ToCipher(cipher));
+            var cipherClone = cipher.Clone();
             await _cipherService.SaveAsync(cipherClone, userId, model.LastKnownRevisionDate, null, true, false);
 
             var response = new CipherMiniResponseModel(cipherClone, _globalSettings, cipher.OrganizationUseTotp);
@@ -276,7 +276,7 @@ namespace Bit.Api.Controllers
                 throw new NotFoundException();
             }
 
-            var original = CoreHelpers.CloneObject(cipher);
+            var original = cipher.Clone();
             await _cipherService.ShareAsync(original, model.Cipher.ToCipher(cipher), new Guid(model.Cipher.OrganizationId),
                 model.CollectionIds.Select(c => new Guid(c)), userId, model.Cipher.LastKnownRevisionDate);
 
diff --git a/src/Core/Models/Table/Cipher.cs b/src/Core/Models/Table/Cipher.cs
index af553e859a..0efa76e7f8 100644
--- a/src/Core/Models/Table/Cipher.cs
+++ b/src/Core/Models/Table/Cipher.cs
@@ -6,7 +6,7 @@ using System.Collections.Generic;
 
 namespace Bit.Core.Models.Table
 {
-    public class Cipher : ITableObject<Guid>
+    public class Cipher : ITableObject<Guid>, ICloneable
     {
         private Dictionary<string, CipherAttachment.MetaData> _attachmentData;
 
@@ -92,5 +92,15 @@ namespace Bit.Core.Models.Table
             var attachments = GetAttachments();
             return attachments?.ContainsKey(id) ?? false;
         }
+
+        object ICloneable.Clone() => Clone();
+        public Cipher Clone()
+        {
+            var clone = CoreHelpers.CloneObject(this);
+            clone.CreationDate = CreationDate;
+            clone.RevisionDate = RevisionDate;
+
+            return clone;
+        }
     }
 }
diff --git a/src/Core/Utilities/CoreHelpers.cs b/src/Core/Utilities/CoreHelpers.cs
index fd9945fa22..bf01cf225c 100644
--- a/src/Core/Utilities/CoreHelpers.cs
+++ b/src/Core/Utilities/CoreHelpers.cs
@@ -359,6 +359,11 @@ namespace Bit.Core.Utilities
             return readable.ToString("0.## ") + suffix;
         }
 
+        /// <summary>
+        /// Creates a clone of the given object through serializing to json and deserializing.
+        /// This method is subject to the limitations of Newstonsoft. For example, properties with
+        /// inaccessible setters will not be set.
+        /// </summary>
         public static T CloneObject<T>(T obj)
         {
             return JsonConvert.DeserializeObject<T>(JsonConvert.SerializeObject(obj));
diff --git a/test/Core.Test/AutoFixture/CipherFixtures.cs b/test/Core.Test/AutoFixture/CipherFixtures.cs
index 84bf37c0de..37f4c933e1 100644
--- a/test/Core.Test/AutoFixture/CipherFixtures.cs
+++ b/test/Core.Test/AutoFixture/CipherFixtures.cs
@@ -52,6 +52,19 @@ namespace Bit.Core.Test.AutoFixture.CipherFixtures
         public InlineKnownUserCipherAutoDataAttribute(string userId, params object[] values) : base(new ICustomization[]
             { new SutProviderCustomization(), new UserCipher { UserId = new Guid(userId) } }, values)
     { }
+    }
 
-}
+    internal class OrganizationCipherAutoDataAttribute : CustomAutoDataAttribute
+    {
+        public OrganizationCipherAutoDataAttribute(string organizationId = null) : base(new SutProviderCustomization(),
+            new OrganizationCipher { OrganizationId = organizationId == null ? (Guid?)null : new Guid(organizationId) })
+        { }
+    }
+
+    internal class InlineOrganizationCipherAutoDataAttribute : InlineCustomAutoDataAttribute
+    {
+        public InlineOrganizationCipherAutoDataAttribute(params object[] values) : base(new[] { typeof(SutProviderCustomization),
+            typeof(OrganizationCipher) }, values)
+        { }
+    }
 }
diff --git a/test/Core.Test/Models/CipherTests.cs b/test/Core.Test/Models/CipherTests.cs
new file mode 100644
index 0000000000..dfc9f9e996
--- /dev/null
+++ b/test/Core.Test/Models/CipherTests.cs
@@ -0,0 +1,18 @@
+using Bit.Core.Models.Table;
+using Bit.Core.Test.AutoFixture.CipherFixtures;
+using Newtonsoft.Json;
+using Xunit;
+
+namespace Core.Test.Models
+{
+    public class CipherTests
+    {
+        [Theory]
+        [InlineUserCipherAutoData]
+        [InlineOrganizationCipherAutoData]
+        public void Clone_CreatesExactCopy(Cipher cipher)
+        {
+            Assert.Equal(JsonConvert.SerializeObject(cipher), JsonConvert.SerializeObject(cipher.Clone()));
+        }
+    }
+}