Skip to content

Commit bd4952e

Browse files
vcolin7vcolin7
authored
[Key Vault Certificates] Added missing parameters to createIssuer() and updateIssuer() requests. (Azure#29260)
* Updated the CertificateIssuer create and update methods in `CertificateAsyncClient` to ensure they send the `organizationId` and `enabled` parameters to the service. * Updated tests. * Updated test recordings. * Updated CHANGELOG.
1 parent 700e865 commit bd4952e

File tree

9 files changed

+376
-248
lines changed

9 files changed

+376
-248
lines changed

‎sdk/keyvault/azure-security-keyvault-certificates/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
### Breaking Changes
88

99
### Bugs Fixed
10+
- Fixed an issue that caused the `organizationId` and `enabled` parameters not be sent to the Key Vault service when creating or updating a `CertificateIssuer`.
1011

1112
### Other Changes
1213

‎sdk/keyvault/azure-security-keyvault-certificates/src/main/java/com/azure/security/keyvault/certificates/CertificateAsyncClient.java‎

Lines changed: 64 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.azure.security.keyvault.certificates.implementation.CertificateService;
3535
import com.azure.security.keyvault.certificates.implementation.CertificateUpdateParameters;
3636
import com.azure.security.keyvault.certificates.implementation.Contacts;
37+
import com.azure.security.keyvault.certificates.implementation.IssuerAttributes;
3738
import com.azure.security.keyvault.certificates.implementation.IssuerCredentials;
3839
import com.azure.security.keyvault.certificates.implementation.OrganizationDetails;
3940
import com.azure.security.keyvault.certificates.models.CertificateContact;
@@ -1617,14 +1618,23 @@ Mono<Response<CertificateIssuer>> createIssuerWithResponse(CertificateIssuer iss
16171618
context = context == null ? Context.NONE : context;
16181619
CertificateIssuerSetParameters parameters = new CertificateIssuerSetParameters()
16191620
.provider(issuer.getProvider())
1620-
.credentials(new IssuerCredentials().accountId(issuer.getAccountId()).password(issuer.getPassword()))
1621-
.organizationDetails(new OrganizationDetails().adminDetails(issuer.getAdministratorContacts()))
1622-
.credentials(new IssuerCredentials().password(issuer.getPassword()).accountId(issuer.getAccountId()));
1623-
return service.setCertificateIssuer(vaultUrl, apiVersion, ACCEPT_LANGUAGE, issuer.getName(), parameters, CONTENT_TYPE_HEADER_VALUE,
1624-
context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
1625-
.doOnRequest(ignored -> logger.verbose("Creating certificate issuer - {}", issuer.getName()))
1626-
.doOnSuccess(response -> logger.verbose("Created the certificate issuer - {}", response.getValue().getName()))
1627-
.doOnError(error -> logger.warning("Failed to create the certificate issuer - {}", issuer.getName(), error));
1621+
.organizationDetails(new OrganizationDetails()
1622+
.id(issuer.getOrganizationId())
1623+
.adminDetails(issuer.getAdministratorContacts()))
1624+
.credentials(new IssuerCredentials()
1625+
.password(issuer.getPassword())
1626+
.accountId(issuer.getAccountId()))
1627+
.attributes(new IssuerAttributes()
1628+
.enabled(issuer.isEnabled()));
1629+
1630+
return service.setCertificateIssuer(vaultUrl, apiVersion, ACCEPT_LANGUAGE, issuer.getName(), parameters,
1631+
CONTENT_TYPE_HEADER_VALUE, context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
1632+
.doOnRequest(ignored ->
1633+
logger.verbose("Creating certificate issuer - {}", issuer.getName()))
1634+
.doOnSuccess(response ->
1635+
logger.verbose("Created the certificate issuer - {}", response.getValue().getName()))
1636+
.doOnError(error ->
1637+
logger.warning("Failed to create the certificate issuer - {}", issuer.getName(), error));
16281638
}
16291639

16301640
/**
@@ -1694,11 +1704,14 @@ public Mono<CertificateIssuer> getIssuer(String issuerName) {
16941704

16951705
Mono<Response<CertificateIssuer>> getIssuerWithResponse(String issuerName, Context context) {
16961706
context = context == null ? Context.NONE : context;
1697-
return service.getCertificateIssuer(vaultUrl, apiVersion, ACCEPT_LANGUAGE, issuerName, CONTENT_TYPE_HEADER_VALUE,
1698-
context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
1699-
.doOnRequest(ignored -> logger.verbose("Retrieving certificate issuer - {}", issuerName))
1700-
.doOnSuccess(response -> logger.verbose("Retrieved the certificate issuer - {}", response.getValue().getName()))
1701-
.doOnError(error -> logger.warning("Failed to retreive the certificate issuer - {}", issuerName, error));
1707+
return service.getCertificateIssuer(vaultUrl, apiVersion, ACCEPT_LANGUAGE, issuerName,
1708+
CONTENT_TYPE_HEADER_VALUE, context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
1709+
.doOnRequest(ignored ->
1710+
logger.verbose("Retrieving certificate issuer - {}", issuerName))
1711+
.doOnSuccess(response ->
1712+
logger.verbose("Retrieved the certificate issuer - {}", response.getValue().getName()))
1713+
.doOnError(error ->
1714+
logger.warning("Failed to retreive the certificate issuer - {}", issuerName, error));
17021715
}
17031716

17041717
/**
@@ -1766,11 +1779,15 @@ public Mono<CertificateIssuer> deleteIssuer(String issuerName) {
17661779

17671780
Mono<Response<CertificateIssuer>> deleteIssuerWithResponse(String issuerName, Context context) {
17681781
context = context == null ? Context.NONE : context;
1769-
return service.deleteCertificateIssuer(vaultUrl, issuerName, apiVersion, ACCEPT_LANGUAGE, CONTENT_TYPE_HEADER_VALUE,
1770-
context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
1771-
.doOnRequest(ignored -> logger.verbose("Deleting certificate issuer - {}", issuerName))
1772-
.doOnSuccess(response -> logger.verbose("Deleted the certificate issuer - {}", response.getValue().getName()))
1773-
.doOnError(error -> logger.warning("Failed to delete the certificate issuer - {}", issuerName, error));
1782+
1783+
return service.deleteCertificateIssuer(vaultUrl, issuerName, apiVersion, ACCEPT_LANGUAGE,
1784+
CONTENT_TYPE_HEADER_VALUE, context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
1785+
.doOnRequest(ignored ->
1786+
logger.verbose("Deleting certificate issuer - {}", issuerName))
1787+
.doOnSuccess(response ->
1788+
logger.verbose("Deleted the certificate issuer - {}", response.getValue().getName()))
1789+
.doOnError(error ->
1790+
logger.warning("Failed to delete the certificate issuer - {}", issuerName, error));
17741791
}
17751792

17761793

@@ -1814,8 +1831,8 @@ PagedFlux<IssuerProperties> listPropertiesOfIssuers(Context context) {
18141831

18151832
private Mono<PagedResponse<IssuerProperties>> listPropertiesOfIssuersFirstPage(Context context) {
18161833
try {
1817-
return service.getCertificateIssuers(vaultUrl, DEFAULT_MAX_PAGE_RESULTS, apiVersion, ACCEPT_LANGUAGE, CONTENT_TYPE_HEADER_VALUE,
1818-
context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
1834+
return service.getCertificateIssuers(vaultUrl, DEFAULT_MAX_PAGE_RESULTS, apiVersion, ACCEPT_LANGUAGE,
1835+
CONTENT_TYPE_HEADER_VALUE, context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
18191836
.doOnRequest(ignored -> logger.verbose("Listing certificate issuers - {}"))
18201837
.doOnSuccess(response -> logger.verbose("Listed certificate issuers - {}"))
18211838
.doOnError(error -> logger.warning("Failed to list certificate issuers - {}", error));
@@ -1833,11 +1850,15 @@ private Mono<PagedResponse<IssuerProperties>> listPropertiesOfIssuersFirstPage(C
18331850
*/
18341851
private Mono<PagedResponse<IssuerProperties>> listPropertiesOfIssuersNextPage(String continuationToken, Context context) {
18351852
try {
1836-
return service.getCertificateIssuers(vaultUrl, continuationToken, ACCEPT_LANGUAGE, CONTENT_TYPE_HEADER_VALUE,
1837-
context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
1838-
.doOnRequest(ignored -> logger.verbose("Listing next certificate issuers page - Page {} ", continuationToken))
1839-
.doOnSuccess(response -> logger.verbose("Listed next certificate issuers page - Page {} ", continuationToken))
1840-
.doOnError(error -> logger.warning("Failed to list next certificate issuers page - Page {} ", continuationToken, error));
1853+
return service.getCertificateIssuers(vaultUrl, continuationToken, ACCEPT_LANGUAGE,
1854+
CONTENT_TYPE_HEADER_VALUE, context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
1855+
.doOnRequest(ignored ->
1856+
logger.verbose("Listing next certificate issuers page - Page {} ", continuationToken))
1857+
.doOnSuccess(response ->
1858+
logger.verbose("Listed next certificate issuers page - Page {} ", continuationToken))
1859+
.doOnError(error ->
1860+
logger.warning("Failed to list next certificate issuers page - Page {} ", continuationToken,
1861+
error));
18411862
} catch (RuntimeException ex) {
18421863
return monoError(logger, ex);
18431864
}
@@ -1927,13 +1948,24 @@ Mono<Response<CertificateIssuer>> updateIssuerWithResponse(CertificateIssuer iss
19271948
context = context == null ? Context.NONE : context;
19281949
CertificateIssuerUpdateParameters updateParameters = new CertificateIssuerUpdateParameters()
19291950
.provider(issuer.getProvider())
1930-
.organizationDetails(new OrganizationDetails().adminDetails(issuer.getAdministratorContacts()))
1931-
.credentials(new IssuerCredentials().password(issuer.getPassword()).accountId(issuer.getAccountId()));
1932-
return service.updateCertificateIssuer(vaultUrl, issuer.getName(), apiVersion, ACCEPT_LANGUAGE, updateParameters, CONTENT_TYPE_HEADER_VALUE,
1933-
context.addData(AZ_TRACING_NAMESPACE_KEY, KEYVAULT_TRACING_NAMESPACE_VALUE))
1934-
.doOnRequest(ignored -> logger.verbose("Updating certificate issuer - {}", issuer.getName()))
1935-
.doOnSuccess(response -> logger.verbose("Updated up the certificate issuer - {}", response.getValue().getName()))
1936-
.doOnError(error -> logger.warning("Failed to updated the certificate issuer - {}", issuer.getName(), error));
1951+
.organizationDetails(new OrganizationDetails()
1952+
.id(issuer.getOrganizationId())
1953+
.adminDetails(issuer.getAdministratorContacts()))
1954+
.credentials(new IssuerCredentials()
1955+
.password(issuer.getPassword())
1956+
.accountId(issuer.getAccountId()))
1957+
.attributes(new IssuerAttributes()
1958+
.enabled(issuer.isEnabled()));
1959+
1960+
return service.updateCertificateIssuer(vaultUrl, issuer.getName(), apiVersion, ACCEPT_LANGUAGE,
1961+
updateParameters, CONTENT_TYPE_HEADER_VALUE, context.addData(AZ_TRACING_NAMESPACE_KEY,
1962+
KEYVAULT_TRACING_NAMESPACE_VALUE))
1963+
.doOnRequest(ignored ->
1964+
logger.verbose("Updating certificate issuer - {}", issuer.getName()))
1965+
.doOnSuccess(response ->
1966+
logger.verbose("Updated up the certificate issuer - {}", response.getValue().getName()))
1967+
.doOnError(error ->
1968+
logger.warning("Failed to updated the certificate issuer - {}", issuer.getName(), error));
19371969
}
19381970

19391971
/**

‎sdk/keyvault/azure-security-keyvault-certificates/src/test/java/com/azure/security/keyvault/certificates/CertificateClientTest.java‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -608,7 +608,7 @@ public void createIssuer(HttpClient httpClient, CertificateServiceVersion servic
608608
createIssuerRunner((issuer) -> {
609609
CertificateIssuer createdIssuer = certificateClient.createIssuer(issuer);
610610

611-
validateIssuer(issuer, createdIssuer);
611+
assertTrue(issuerCreatedCorrectly(issuer, createdIssuer));
612612
});
613613
}
614614

@@ -647,7 +647,7 @@ public void getCertificateIssuer(HttpClient httpClient, CertificateServiceVersio
647647
CertificateIssuer createdIssuer = certificateClient.createIssuer(issuer);
648648
CertificateIssuer retrievedIssuer = certificateClient.getIssuer(issuer.getName());
649649

650-
validateIssuer(issuer, retrievedIssuer);
650+
assertTrue(issuerCreatedCorrectly(issuer, retrievedIssuer));
651651
});
652652
}
653653

@@ -669,7 +669,7 @@ public void deleteCertificateIssuer(HttpClient httpClient, CertificateServiceVer
669669
CertificateIssuer createdIssuer = certificateClient.createIssuer(issuer);
670670
CertificateIssuer deletedIssuer = certificateClient.deleteIssuer(issuer.getName());
671671

672-
validateIssuer(issuer, deletedIssuer);
672+
assertTrue(issuerCreatedCorrectly(issuer, deletedIssuer));
673673
});
674674
}
675675

@@ -693,7 +693,7 @@ public void listCertificateIssuers(HttpClient httpClient, CertificateServiceVers
693693
for (CertificateIssuer issuer : certificateIssuersToList.values()) {
694694
CertificateIssuer certificateIssuer = certificateClient.createIssuer(issuer);
695695

696-
validateIssuer(issuer, certificateIssuer);
696+
assertTrue(issuerCreatedCorrectly(issuer, certificateIssuer));
697697
}
698698

699699
for (IssuerProperties issuerProperties : certificateClient.listPropertiesOfIssuers()) {
@@ -708,6 +708,19 @@ public void listCertificateIssuers(HttpClient httpClient, CertificateServiceVers
708708
});
709709
}
710710

711+
@ParameterizedTest(name = DISPLAY_NAME_WITH_ARGUMENTS)
712+
@MethodSource("getTestParameters")
713+
public void updateIssuer(HttpClient httpClient, CertificateServiceVersion serviceVersion) {
714+
createCertificateClient(httpClient, serviceVersion);
715+
716+
updateIssuerRunner((issuerToCreate, issuerToUpdate) -> {
717+
CertificateIssuer createdIssuer = certificateClient.createIssuer(issuerToCreate);
718+
CertificateIssuer updatedIssuer = certificateClient.updateIssuer(issuerToUpdate);
719+
720+
assertTrue(issuerUpdatedCorrectly(issuerToCreate, updatedIssuer));
721+
});
722+
}
723+
711724
@ParameterizedTest(name = DISPLAY_NAME_WITH_ARGUMENTS)
712725
@MethodSource("getTestParameters")
713726
@SuppressWarnings("ArraysAsListWithZeroOrOneArgument")

‎sdk/keyvault/azure-security-keyvault-certificates/src/test/java/com/azure/security/keyvault/certificates/CertificateClientTestBase.java‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,23 @@ void listCertificateIssuersRunner(Consumer<HashMap<String, CertificateIssuer>> t
375375
testRunner.accept(certificateIssuers);
376376
}
377377

378+
void updateIssuerRunner(BiConsumer<CertificateIssuer, CertificateIssuer> testRunner) {
379+
String issuerName = testResourceNamer.randomName("testIssuer", 25);
380+
final CertificateIssuer certificateIssuer = setupIssuer(issuerName);
381+
final CertificateIssuer issuerForUpdate = new CertificateIssuer(issuerName, "Test")
382+
.setAdministratorContacts(Arrays.asList(new AdministratorContact()
383+
.setFirstName("otherFirst")
384+
.setLastName("otherLast")
385+
.setEmail("otherFirst.otherLast@hotmail.com")
386+
.setPhone("67890")))
387+
.setAccountId("otherIssuerAccountId")
388+
.setEnabled(false)
389+
.setOrganizationId("otherOrgId")
390+
.setPassword("test456");
391+
392+
testRunner.accept(certificateIssuer, issuerForUpdate);
393+
}
394+
378395
@Test
379396
public abstract void setContacts(HttpClient httpClient, CertificateServiceVersion serviceVersion);
380397

@@ -540,7 +557,7 @@ X509Certificate loadCerToX509Certificate(KeyVaultCertificateWithPolicy certifica
540557
return x509Certificate;
541558
}
542559

543-
Boolean validateIssuer(CertificateIssuer expected, CertificateIssuer actual) {
560+
boolean issuerCreatedCorrectly(CertificateIssuer expected, CertificateIssuer actual) {
544561
return expected.getAccountId().equals(actual.getAccountId())
545562
&& expected.isEnabled().equals(actual.isEnabled())
546563
&& (actual.getCreatedOn() != null)
@@ -552,6 +569,18 @@ Boolean validateIssuer(CertificateIssuer expected, CertificateIssuer actual) {
552569
&& expected.getAdministratorContacts().size() == actual.getAdministratorContacts().size();
553570
}
554571

572+
boolean issuerUpdatedCorrectly(CertificateIssuer expected, CertificateIssuer actual) {
573+
return !expected.getAccountId().equals(actual.getAccountId())
574+
&& !expected.isEnabled().equals(actual.isEnabled())
575+
&& (actual.getCreatedOn() != null)
576+
&& (actual.getUpdatedOn() != null)
577+
&& (actual.getId() != null)
578+
&& (actual.getId().length() > 0)
579+
&& expected.getName().equals(actual.getName())
580+
&& !expected.getOrganizationId().equals(actual.getOrganizationId())
581+
&& expected.getAdministratorContacts().size() == actual.getAdministratorContacts().size();
582+
}
583+
555584
CertificatePolicy setupPolicy() {
556585
return new CertificatePolicy(WellKnownIssuerNames.SELF, "CN=default")
557586
.setKeyUsage(CertificateKeyUsage.KEY_CERT_SIGN, CertificateKeyUsage.KEY_AGREEMENT)

‎sdk/keyvault/azure-security-keyvault-certificates/src/test/resources/session-records/CertificateClientTest.createIssuer[1].json‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,29 @@
11
{
22
"networkCallRecords" : [ {
33
"Method" : "PUT",
4-
"Uri" : "https://REDACTED.vault.azure.net/certificates/issuers/testissuer95254923fe?api-version=7.3",
4+
"Uri" : "https://REDACTED.vault.azure.net/certificates/issuers/testissuer201354f3c0?api-version=7.3",
55
"Headers" : {
66
"User-Agent" : "azsdk-java-client_name/client_version (11.0.6; Windows 10; 10.0)",
77
"Content-Type" : "application/json"
88
},
99
"Response" : {
10-
"content-length" : "360",
10+
"content-length" : "373",
1111
"X-Content-Type-Options" : "nosniff",
1212
"Pragma" : "no-cache",
1313
"retry-after" : "0",
1414
"StatusCode" : "200",
15-
"Date" : "Thu, 31 Mar 2022 00:15:16 GMT",
15+
"Date" : "Tue, 07 Jun 2022 09:07:52 GMT",
1616
"Strict-Transport-Security" : "max-age=31536000;includeSubDomains",
1717
"Cache-Control" : "no-cache",
1818
"x-ms-keyvault-region" : "westus",
1919
"x-ms-keyvault-network-info" : "conn_type=Ipv4;addr=172.92.148.195;act_addr_fam=InterNetwork;",
2020
"Expires" : "-1",
21-
"x-ms-request-id" : "d34b188f-f36b-4294-8aea-2e09af43e184",
22-
"x-ms-keyvault-service-version" : "1.9.331.5",
23-
"Body" : "{\"id\":\"https://azsdktests.vault.azure.net/certificates/issuers/testissuer95254923fe\",\"provider\":\"Test\",\"credentials\":{\"account_id\":\"issuerAccountId\"},\"org_details\":{\"zip\":0,\"admin_details\":[{\"first_name\":\"first\",\"last_name\":\"last\",\"email\":\"first.last@hotmail.com\",\"phone\":\"12345\"}]},\"attributes\":{\"enabled\":true,\"created\":1648685716,\"updated\":1648685716}}",
24-
"Content-Type" : "application/json; charset=utf-8",
25-
"X-Powered-By" : "ASP.NET"
21+
"x-ms-request-id" : "69a3e9f4-0e74-4b0e-9f90-0ab5fee0bd2c",
22+
"x-ms-keyvault-service-version" : "1.9.422.1",
23+
"Body" : "{\"id\":\"https://azure-kv-tests2.vault.azure.net/certificates/issuers/testissuer201354f3c0\",\"provider\":\"Test\",\"credentials\":{\"account_id\":\"issuerAccountId\"},\"org_details\":{\"id\":\"orgId\",\"zip\":0,\"admin_details\":[{\"first_name\":\"first\",\"last_name\":\"last\",\"email\":\"first.last@hotmail.com\",\"phone\":\"12345\"}]},\"attributes\":{\"enabled\":true,\"created\":1654592872,\"updated\":1654592872}}",
24+
"Content-Type" : "application/json; charset=utf-8"
2625
},
2726
"Exception" : null
2827
} ],
29-
"variables" : [ "testissuer95254923fe" ]
28+
"variables" : [ "testissuer201354f3c0" ]
3029
}

0 commit comments

Comments
 (0)