[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

@@ -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);
 

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;
+        }
     }
 }

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));

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)
+        { }
+    }
 }

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()));
+        }
+    }
+}