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.

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

[FEATURE REQ] Support setAuthorityHost with CredentialBuilderBase #8626

Closed
jongio opened this issue Feb 29, 2020 · 2 comments · Fixed by #8765
Closed

[FEATURE REQ] Support setAuthorityHost with CredentialBuilderBase #8626

jongio opened this issue Feb 29, 2020 · 2 comments · Fixed by #8765
Assignees
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library. feature-request This issue requires a new behavior in the product in order be resolved.

Comments

@jongio
Copy link
Member

jongio commented Feb 29, 2020

Right now, it doesn't look like you can call setAuthorityHost with DefaultAzureCredentialBuilder:
https://github.com/Azure/azure-sdk-for-java/blob/master/sdk/identity/azure-identity/src/main/java/com/azure/identity/DefaultAzureCredentialBuilder.java

Which implements CredentialBuilderBase

CredentialBuilderBase doesn't expose setAuthorityHost
https://github.com/Azure/azure-sdk-for-java/blob/master/sdk/identity/azure-identity/src/main/java/com/azure/identity/CredentialBuilderBase.java

EnvironmentCredentialBuilder does expose setAuthorityHost

https://github.com/Azure/azure-sdk-for-java/blob/master/sdk/identity/azure-identity/src/main/java/com/azure/identity/EnvironmentCredentialBuilder.java

I recommend implementing this at the CredentialBuilderBase level.

Because ManagedIdentityCredential and other credential classes need this as well

https://github.com/Azure/azure-sdk-for-java/blob/master/sdk/identity/azure-identity/src/main/java/com/azure/identity/ManagedIdentityCredentialBuilder.java

! This is currently blocking use of DefaultAzureCredentialBuilder with other clouds, such as Azure Government.

@jongio
Copy link
Member Author

jongio commented Feb 29, 2020

Workaround using reflection:

IdentityClientOptions options = new IdentityClientOptions().setAuthorityHost("https://login.microsoftonline.us/");
DefaultAzureCredentialBuilder cred = new DefaultAzureCredentialBuilder();
Field identityClientOptions = cred.getClass().getSuperclass().getDeclaredField("identityClientOptions");
identityClientOptions.setAccessible(true);
identityClientOptions.set(cred, options);

KeyClient keyClient = new KeyClientBuilder().vaultUrl(dotenv.get("AZURE_KEYVAULT_URL"))
                .credential(cred.build()).buildClient();

Or use EnvironmentCredentialBuilder instead of DefaultAzureCredentialBuilder

 KeyClient keyClient = new KeyClientBuilder().vaultUrl(dotenv.get("AZURE_KEYVAULT_URL"))
  .credential(new EnvironmentCredentialBuilder().authorityHost("https://login.microsoftonline.us/").build()).buildClient();

@jongio jongio changed the title [FEATURE REQ] Support setAuthorityHost with DefaultAzureCredentialBuilder [FEATURE REQ] Support setAuthorityHost with CredentialBuilderBase Feb 29, 2020
@joshfree joshfree added Azure.Identity Client This issue points to a problem in the data-plane of the library. feature-request This issue requires a new behavior in the product in order be resolved. labels Mar 2, 2020
@triage-new-issues triage-new-issues bot removed the triage label Mar 2, 2020
@g2vinay
Copy link
Member

g2vinay commented Mar 13, 2020

This Issue was resolved in this PR: #8765
The authorityHost setter was added to DefaultAzureCredentialBuilder.
MI credential doesn't consume authHost, so that's still excluded.

@g2vinay g2vinay closed this as completed Mar 13, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library. feature-request This issue requires a new behavior in the product in order be resolved.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants