Populate Issuer Name and fix Properties deserialization (#12934)

* Eliminate credentials.pwd parsing on org_details de-serialization * properly populate Name, add tests * update changelog * add recorded test coverage * fix changelog
Azure · Jun 22, 2020 · e3b81ad · e3b81ad
1 parent e952cff
commit e3b81ad
Show file tree

Hide file tree

Showing 8 changed files with 454 additions and 2 deletions.
diff --git a/sdk/keyvault/Azure.Security.KeyVault.Certificates/CHANGELOG.md b/sdk/keyvault/Azure.Security.KeyVault.Certificates/CHANGELOG.md
@@ -5,6 +5,8 @@
 ### Fixed
 
 - Fixed concurrency issue in our challenge-based authentication policy ([#9737](https://github.com/Azure/azure-sdk-for-net/issues/9737))
+- Fixed an issue where the issuer name was always null ([#10908](https://github.com/Azure/azure-sdk-for-net/issues/10908))
+- Fixed an issue where GetIssuerAsync would throw for issuers with contact information populated ([#10905](https://github.com/Azure/azure-sdk-for-net/issues/10905))
 
 ## 4.0.1 (2020-03-03)
 

diff --git a/sdk/keyvault/Azure.Security.KeyVault.Certificates/src/CertificateIssuer.cs b/sdk/keyvault/Azure.Security.KeyVault.Certificates/src/CertificateIssuer.cs
@@ -185,7 +185,6 @@ private void ReadOrgDetailsProperties(JsonElement json)
                             admin.ReadProperties(elem);
                             AdministratorContacts.Add(admin);
                         }
-                        Password = prop.Value.GetString();
                         break;
                 }
             }

diff --git a/sdk/keyvault/Azure.Security.KeyVault.Certificates/src/IssuerProperties.cs b/sdk/keyvault/Azure.Security.KeyVault.Certificates/src/IssuerProperties.cs
@@ -55,6 +55,7 @@ internal void ReadProperty(JsonProperty prop)
                 case IdPropertyName:
                     var id = prop.Value.GetString();
                     Id = new Uri(id);
+                    Name = Id.Segments[Id.Segments.Length - 1];
                     break;
 
                 case ProviderPropertyName:

diff --git a/sdk/keyvault/Azure.Security.KeyVault.Certificates/tests/CertificateClientLiveTests.cs b/sdk/keyvault/Azure.Security.KeyVault.Certificates/tests/CertificateClientLiveTests.cs
@@ -564,6 +564,40 @@ public async Task ValidateMergeCertificate()
         // GetDeletedCertificates
         // GetUpdatePolicy
         // IssuerCrud
+
+        [Test]
+        public async Task VerifyGetIssuer()
+        {
+            string issuerName = Recording.GenerateId();
+
+            string providerName = "ssladmin";
+
+            CertificateIssuer issuer = new CertificateIssuer(issuerName, providerName)
+            {
+                AdministratorContacts =
+                {
+                    new AdministratorContact
+                    {
+                        Email = "email@domain.tld",
+                        FirstName ="fName",
+                        LastName = "lName",
+                        Phone = "1234"
+                    },
+                },
+            };
+
+            RegisterForCleanupIssuer(issuerName);
+
+            await Client.CreateIssuerAsync(issuer);
+
+            CertificateIssuer getIssuer = await Client.GetIssuerAsync(issuerName);
+
+            Assert.NotNull(getIssuer);
+            Assert.NotNull(getIssuer.Id);
+            Assert.AreEqual(issuer.Provider, getIssuer.Provider);
+            Assert.AreEqual(issuer.Name, getIssuer.Name);
+        }
+
         // ContactsCrud
 
         private static CertificatePolicy DefaultPolicy => new CertificatePolicy

diff --git a/sdk/keyvault/Azure.Security.KeyVault.Certificates/tests/CertificateIssuerTests.cs b/sdk/keyvault/Azure.Security.KeyVault.Certificates/tests/CertificateIssuerTests.cs
@@ -50,6 +50,7 @@ public void UninitializedAdministrators()
         [Test]
         public void InitializedAdministrators()
         {
+            const string expectedJson = @"{""provider"":""provider"",""credentials"":{""account_id"":""accountId""},""org_details"":{""admin_details"":[{""first_name"":""fName"",""last_name"":""lName"",""email"":""email@domain.tld"",""phone"":""1234""}]}}";
             CertificateIssuer issuer = new CertificateIssuer("test", "provider")
             {
                 AccountId = "accountId",
@@ -58,10 +59,15 @@ public void InitializedAdministrators()
                     new AdministratorContact
                     {
                         Email = "email@domain.tld",
+                        FirstName ="fName",
+                        LastName = "lName",
+                        Phone = "1234"
+
                     },
                 },
             };
 
+            // Serialize the CertificateIssuer.
             using JsonStream json = new JsonStream();
             using (Utf8JsonWriter writer = json.CreateWriter())
             {
@@ -72,7 +78,48 @@ public void InitializedAdministrators()
                 writer.WriteEndObject();
             }
 
-            Assert.AreEqual(@"{""provider"":""provider"",""credentials"":{""account_id"":""accountId""},""org_details"":{""admin_details"":[{""email"":""email@domain.tld""}]}}", json.ToString());
+            // Assert that the CertificateIssuer was serialized properly.
+            Assert.That(expectedJson, Is.EqualTo(json.ToString()));
+
+            // De-Serialze the CertificateIssuer.
+            var jDoc = JsonDocument.Parse(expectedJson);
+            CertificateIssuer deserializedIssuer = new CertificateIssuer();
+            ((IJsonDeserializable)deserializedIssuer).ReadProperties(jDoc.RootElement);
+
+            // Assert that the CertificateIssuer was de-serialized properly.
+            Assert.That(deserializedIssuer.AccountId, Is.EqualTo(issuer.AccountId));
+            Assert.That(deserializedIssuer.AdministratorContacts[0].Email, Is.EqualTo(issuer.AdministratorContacts[0].Email));
+            Assert.That(deserializedIssuer.AdministratorContacts[0].FirstName, Is.EqualTo(issuer.AdministratorContacts[0].FirstName));
+            Assert.That(deserializedIssuer.AdministratorContacts[0].LastName, Is.EqualTo(issuer.AdministratorContacts[0].LastName));
+            Assert.That(deserializedIssuer.AdministratorContacts[0].Phone, Is.EqualTo(issuer.AdministratorContacts[0].Phone));
+        }
+
+        [Test]
+        public void NameDeSerialization()
+        {
+            const string name = "2100662614";
+            Uri id = new Uri($"https://some.vault.azure.net/certificates/issuers/{name}");
+            const string accountId = "accountId";
+            const string firstName = "fName";
+            const string lastName = "lName";
+            const string email = "email@domain.tld";
+            const string phone = "123-123-1234";
+
+            string expectedJson = $@"{{""id"":""{id.AbsoluteUri}"",""provider"":""ssladmin"",""credentials"":{{""account_id"":""{accountId}""}},""org_details"":{{""zip"":0,""admin_details"":[{{""first_name"":""{firstName}"",""last_name"":""{lastName}"",""email"":""{email}"",""phone"":""{phone}""}}]}},""attributes"":{{""enabled"":true,""created"":1592839861,""updated"":1592839861}}}}";
+
+            // De-Serialze the CertificateIssuer.
+            var jDoc = JsonDocument.Parse(expectedJson);
+            CertificateIssuer deserializedIssuer = new CertificateIssuer();
+            ((IJsonDeserializable)deserializedIssuer).ReadProperties(jDoc.RootElement);
+
+            // Assert that the CertificateIssuer was de-serialized properly.
+            Assert.That(deserializedIssuer.Id, Is.EqualTo(id));
+            Assert.That(deserializedIssuer.Name, Is.EqualTo(name));
+            Assert.That(deserializedIssuer.AccountId, Is.EqualTo(accountId));
+            Assert.That(deserializedIssuer.AdministratorContacts[0].Email, Is.EqualTo(email));
+            Assert.That(deserializedIssuer.AdministratorContacts[0].FirstName, Is.EqualTo(firstName));
+            Assert.That(deserializedIssuer.AdministratorContacts[0].LastName, Is.EqualTo(lastName));
+            Assert.That(deserializedIssuer.AdministratorContacts[0].Phone, Is.EqualTo(phone));
         }
     }
 }
diff --git a/sdk/keyvault/Azure.Security.KeyVault.Certificates/tests/CertificatesTestBase.cs b/sdk/keyvault/Azure.Security.KeyVault.Certificates/tests/CertificatesTestBase.cs
@@ -27,6 +27,8 @@ public class CertificatesTestBase : RecordedTestBase
         private readonly ConcurrentQueue<string> _certificatesToDelete = new ConcurrentQueue<string>();
         private readonly ConcurrentStack<string> _certificatesToPurge = new ConcurrentStack<string>();
 
+        private readonly ConcurrentQueue<string> _issuerToDelete = new ConcurrentQueue<string>();
+
         public CertificatesTestBase(bool isAsync) : base(isAsync)
         {
         }
@@ -80,6 +82,11 @@ public async Task CleanupAll()
             {
                 await PurgeCertificate(name).ConfigureAwait(false);
             }
+
+            while (_issuerToDelete.TryDequeue(out string name))
+            {
+                await DeleteIssuer(name);
+            }
         }
 
         protected async Task DeleteCertificate(string name)
@@ -101,6 +108,25 @@ protected async Task DeleteCertificate(string name)
             }
         }
 
+        protected async Task DeleteIssuer(string name)
+        {
+            if (Mode == RecordedTestMode.Playback)
+            {
+                return;
+            }
+
+            try
+            {
+                using (Recording.DisableRecording())
+                {
+                    await Client.DeleteIssuerAsync(name).ConfigureAwait(false);
+                }
+            }
+            catch (RequestFailedException ex) when (ex.Status == 404)
+            {
+            }
+        }
+
         protected async Task PurgeCertificate(string name)
         {
             try
@@ -212,6 +238,11 @@ protected void RegisterForCleanup(string certificateName)
             _certificatesToDelete.Enqueue(certificateName);
         }
 
+        protected void RegisterForCleanupIssuer(string issuerName)
+        {
+            _issuerToDelete.Enqueue(issuerName);
+        }
+
         protected IAsyncDisposable EnsureDeleted(CertificateOperation operation) => new CertificateOperationDeleter(operation);
 
         private class CertificateOperationDeleter : IAsyncDisposable

diff --git a/...eyVault.Certificates/tests/SessionRecords/CertificateClientLiveTests/VerifyGetIssuer.json b/...eyVault.Certificates/tests/SessionRecords/CertificateClientLiveTests/VerifyGetIssuer.json
-Original file line number
+Diff line change
@@ Expand Up / @@ -185,7 +185,6 @@ private void ReadOrgDetailsProperties(JsonElement json) @@
                                 admin.ReadProperties(elem);
                                 AdministratorContacts.Add(admin);
                             }
-                            Password = prop.Value.GetString();
                             break;
                     }
                 }
@@ Expand Down @@