Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated KeyVaultCredentialPolicy to extend BearerTokenAuthenticationPolicy in Key Vault clients. #24199

Merged
merged 20 commits into from
Sep 30, 2021
+2,373 −1,515
Merged

Updated KeyVaultCredentialPolicy to extend BearerTokenAuthenticationPolicy in Key Vault clients. #24199

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
36c18da
Replaced all uses of KeyVaultCredentialPolicy with BearerTokenAuthent…
vcolin7 Sep 18, 2021
095fc64
We now pass the appropriate scope to BearerTokenAuthenticationPolicy …
vcolin7 Sep 20, 2021
eed5e56
Added tests and recordings for KEK tests on MHSM. Fixed and cleaned u…
vcolin7 Sep 20, 2021
1d55f52
Removed unused imports.
vcolin7 Sep 20, 2021
e936df1
Renamed MHSM_SCOPE to MANAGED_HSM_SCOPE in all client builders.
vcolin7 Sep 21, 2021
911d295
Reintroduced KeyVaultCredentialPolicy and modified it to extend from …
vcolin7 Sep 24, 2021
a9a5a74
Fixed CvheckStyle errors.
vcolin7 Sep 24, 2021
d63983d
Made changes to KeyVaultCredentialPolicy so we don't set the body of …
vcolin7 Sep 24, 2021
ddc8659
Removed scope constants from Key vault client builders.
vcolin7 Sep 24, 2021
8947d6d
Attempted to fix flaky live tests.
vcolin7 Sep 24, 2021
acecf59
Removed verify test for HSM as the FromSource test already verifies t…
vcolin7 Sep 24, 2021
c331b23
Reverted KeyVaultCredentialPolicy in all libraries to set the request…
vcolin7 Sep 25, 2021
3038a19
Fixed KV Administration client live tests that failed due to the auth…
vcolin7 Sep 25, 2021
be31274
Fixed CheckStyle issues.
vcolin7 Sep 25, 2021
16de813
Fixed another CheckStyle issue.
vcolin7 Sep 25, 2021
2ba9e74
Fixed issue that caused an NPE in KeyVaultCredentialPolicy if the con…
vcolin7 Sep 26, 2021
5b58aaf
Updated KeyVaultCredentialPolicy in all other libraries.
vcolin7 Sep 26, 2021
674e4a9
Made an attempt at fixing the backup async live tests.
vcolin7 Sep 27, 2021
a7f5d98
Added sleep timer when running against service for restore operations.
vcolin7 Sep 28, 2021
938c74c
Applied PR feedback.
vcolin7 Sep 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,10 +298,6 @@ the main ServiceBusClientBuilder. -->
<suppress checks="com.azure.tools.checkstyle.checks.HttpPipelinePolicy" files="com.azure.messaging.servicebus.implementation.ServiceBusTokenCredentialHttpPolicy.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.HttpPipelinePolicy" files="com.azure.messaging.eventgrid.implementation.CloudEventTracingPipelinePolicy.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.HttpPipelinePolicy" files="com.azure.storage.common.implementation.policy.SasTokenCredentialPolicy.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.HttpPipelinePolicy" files="com.azure.security.keyvault.administration.implementation.KeyVaultCredentialPolicy.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.HttpPipelinePolicy" files="com.azure.security.keyvault.certificates.implementation.KeyVaultCredentialPolicy.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.HttpPipelinePolicy" files="com.azure.security.keyvault.keys.implementation.KeyVaultCredentialPolicy.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.HttpPipelinePolicy" files="com.azure.security.keyvault.secrets.implementation.KeyVaultCredentialPolicy.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.HttpPipelinePolicy" files="com.azure.storage.blob.implementation.util.BlobUserAgentModificationPolicy.java"/>


Expand Down
6 changes: 4 additions & 2 deletions ...n/java/com/azure/security/keyvault/administration/KeyVaultAccessControlClientBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.azure.core.http.HttpPipelineBuilder;
import com.azure.core.http.HttpPipelinePosition;
import com.azure.core.http.policy.AddHeadersPolicy;
import com.azure.core.http.policy.BearerTokenAuthenticationPolicy;
import com.azure.core.http.policy.HttpLogDetailLevel;
import com.azure.core.http.policy.HttpLogOptions;
import com.azure.core.http.policy.HttpLoggingPolicy;
Expand All @@ -23,7 +24,6 @@
import com.azure.core.util.Configuration;
import com.azure.core.util.CoreUtils;
import com.azure.core.util.logging.ClientLogger;
import com.azure.security.keyvault.administration.implementation.KeyVaultCredentialPolicy;
import com.azure.security.keyvault.administration.implementation.KeyVaultErrorCodeStrings;

import java.net.MalformedURLException;
Expand Down Expand Up @@ -52,6 +52,8 @@
*/
@ServiceClientBuilder(serviceClients = {KeyVaultAccessControlClient.class, KeyVaultAccessControlAsyncClient.class})
public final class KeyVaultAccessControlClientBuilder {
static final String MANAGED_HSM_SCOPE = "https://managedhsm.azure.net/.default";

// This is the properties file name.
private static final String AZURE_KEY_VAULT_RBAC = "azure-key-vault-administration.properties";
private static final String SDK_NAME = "name";
Expand Down Expand Up @@ -156,7 +158,7 @@ public KeyVaultAccessControlAsyncClient buildAsyncClient() {
// Add retry policy.
policies.add(retryPolicy == null ? new RetryPolicy() : retryPolicy);

policies.add(new KeyVaultCredentialPolicy(credential));
policies.add(new BearerTokenAuthenticationPolicy(credential, MANAGED_HSM_SCOPE));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to utilize the Challenge Based Auth Support in BTA policy from azure-core
we need to create a custom policy in KV package that doesn't hard code the scopes as above.
The scopes and tenant id are dynamically parsed in KV challenge that gets returned back from the service.
Here's the .NET impl for reference: https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/keyvault/Azure.Security.KeyVault.Shared/src/ChallengeBasedAuthenticationPolicy.cs#L13

You can port that over to Java and then utilize that policy here, instead of above.

cc: @schaabs @AlexGhiondea @heaths

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @g2vinay!


// Add per retry additional policies.
policies.addAll(perRetryPolicies);
Expand Down
6 changes: 4 additions & 2 deletions ...src/main/java/com/azure/security/keyvault/administration/KeyVaultBackupClientBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.azure.core.http.HttpPipelineBuilder;
import com.azure.core.http.HttpPipelinePosition;
import com.azure.core.http.policy.AddHeadersPolicy;
import com.azure.core.http.policy.BearerTokenAuthenticationPolicy;
import com.azure.core.http.policy.HttpLogDetailLevel;
import com.azure.core.http.policy.HttpLogOptions;
import com.azure.core.http.policy.HttpLoggingPolicy;
Expand All @@ -23,7 +24,6 @@
import com.azure.core.util.Configuration;
import com.azure.core.util.CoreUtils;
import com.azure.core.util.logging.ClientLogger;
import com.azure.security.keyvault.administration.implementation.KeyVaultCredentialPolicy;
import com.azure.security.keyvault.administration.implementation.KeyVaultErrorCodeStrings;

import java.net.MalformedURLException;
Expand Down Expand Up @@ -51,6 +51,8 @@
*/
@ServiceClientBuilder(serviceClients = {KeyVaultBackupClient.class, KeyVaultBackupAsyncClient.class})
public final class KeyVaultBackupClientBuilder {
static final String MANAGED_HSM_SCOPE = "https://managedhsm.azure.net/.default";

// This is the properties file name.
private static final String AZURE_KEY_VAULT_RBAC = "azure-key-vault-administration.properties";
private static final String SDK_NAME = "name";
Expand Down Expand Up @@ -155,7 +157,7 @@ public KeyVaultBackupAsyncClient buildAsyncClient() {
// Add retry policy.
policies.add(retryPolicy == null ? new RetryPolicy() : retryPolicy);

policies.add(new KeyVaultCredentialPolicy(credential));
policies.add(new BearerTokenAuthenticationPolicy(credential, MANAGED_HSM_SCOPE));

// Add per retry additional policies.
policies.addAll(perRetryPolicies);
Expand Down
125 changes: 0 additions & 125 deletions ...a/com/azure/security/keyvault/administration/implementation/KeyVaultCredentialPolicy.java
View file
Open in desktop

This file was deleted.

60 changes: 0 additions & 60 deletions .../main/java/com/azure/security/keyvault/administration/implementation/ScopeTokenCache.java
View file
Open in desktop

This file was deleted.

5 changes: 3 additions & 2 deletions ...java/com/azure/security/keyvault/administration/KeyVaultAdministrationClientTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import com.azure.core.credential.TokenCredential;
import com.azure.core.http.HttpClient;
import com.azure.core.http.policy.BearerTokenAuthenticationPolicy;
import com.azure.core.http.policy.ExponentialBackoff;
import com.azure.core.http.policy.HttpLogDetailLevel;
import com.azure.core.http.policy.HttpLogOptions;
Expand All @@ -17,7 +18,6 @@
import com.azure.core.test.TestMode;
import com.azure.core.util.Configuration;
import com.azure.identity.ClientSecretCredentialBuilder;
import com.azure.security.keyvault.administration.implementation.KeyVaultCredentialPolicy;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.params.provider.Arguments;

Expand Down Expand Up @@ -74,7 +74,8 @@ protected List<HttpPipelinePolicy> getPolicies() {
policies.add(new RetryPolicy(strategy));

if (credential != null) {
policies.add(new KeyVaultCredentialPolicy(credential));
policies.add(
new BearerTokenAuthenticationPolicy(credential, KeyVaultAccessControlClientBuilder.MANAGED_HSM_SCOPE));
}

HttpPolicyProviders.addAfterRetryPolicies(policies);
Expand Down
1 change: 0 additions & 1 deletion ...icates/src/main/java/com/azure/security/keyvault/certificates/CertificateAsyncClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ public final class CertificateAsyncClient {
static final String ACCEPT_LANGUAGE = "en-US";
static final int DEFAULT_MAX_PAGE_RESULTS = 25;
static final String CONTENT_TYPE_HEADER_VALUE = "application/json";
static final String KEY_VAULT_SCOPE = "https://vault.azure.net/.default";
// Please see <a href=https://docs.microsoft.com/en-us/azure/azure-resource-manager/management/azure-services-resource-providers>here</a>
// for more information on Azure resource provider namespaces.
private static final String KEYVAULT_TRACING_NAMESPACE_VALUE = "Microsoft.KeyVault";
Expand Down
6 changes: 4 additions & 2 deletions ...ates/src/main/java/com/azure/security/keyvault/certificates/CertificateClientBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.azure.core.http.HttpPipelineBuilder;
import com.azure.core.http.HttpPipelinePosition;
import com.azure.core.http.policy.AddHeadersPolicy;
import com.azure.core.http.policy.BearerTokenAuthenticationPolicy;
import com.azure.core.http.policy.HttpLogDetailLevel;
import com.azure.core.http.policy.HttpLogOptions;
import com.azure.core.http.policy.HttpLoggingPolicy;
Expand All @@ -23,7 +24,6 @@
import com.azure.core.util.Configuration;
import com.azure.core.util.CoreUtils;
import com.azure.core.util.logging.ClientLogger;
import com.azure.security.keyvault.certificates.implementation.KeyVaultCredentialPolicy;
import com.azure.security.keyvault.certificates.models.KeyVaultCertificateIdentifier;

import java.net.MalformedURLException;
Expand Down Expand Up @@ -68,6 +68,8 @@
*/
@ServiceClientBuilder(serviceClients = {CertificateClient.class, CertificateAsyncClient.class})
public final class CertificateClientBuilder {
static final String KEY_VAULT_SCOPE = "https://vault.azure.net/.default";

private final ClientLogger logger = new ClientLogger(CertificateClientBuilder.class);
// This is properties file's name.
private static final String AZURE_KEY_VAULT_CERTIFICATES_PROPERTIES = "azure-key-vault-certificates.properties";
Expand Down Expand Up @@ -182,7 +184,7 @@ public CertificateAsyncClient buildAsyncClient() {
// Add retry policy.
policies.add(retryPolicy == null ? new RetryPolicy() : retryPolicy);

policies.add(new KeyVaultCredentialPolicy(credential));
policies.add(new BearerTokenAuthenticationPolicy(credential, KEY_VAULT_SCOPE));

// Add per retry additional policies.
policies.addAll(perRetryPolicies);
Expand Down
Loading