Fix OAuth credentials to support web apps #7246
Conversation
jianghaolu
changed the title
jianghaolu
changed the title
jianghaolu
changed the title
| * @param clientSecret the secret value of the AAD application. | ||
| * @return the AuthorizationCodeCredentialBuilder itself | ||
| */ | ||
| public AuthorizationCodeCredentialBuilder clientSecret(String clientSecret) { |
There was a problem hiding this comment.
Do we need to fix this in other languages too ?
Feels like a missed feature, that should exist in parity across languages.
@schaabs
There was a problem hiding this comment.
It's funny since in the beginning .NET and Python supporting providing a client secret and Java didn't. Now Java supports providing a client secret or a certificate :)
| * @param certificatePath the PEM file containing the certificate | ||
| * @return the InteractiveBrowserCredentialBuilder itself | ||
| */ | ||
| public InteractiveBrowserCredentialBuilder pemCertificate(String certificatePath) { |
There was a problem hiding this comment.
need to add this in other languages too ?
There was a problem hiding this comment.
Currently, .NET only supports client secret when using the AzuthorizationCodeCredential not certificate. Adding support for this makes sense but we'd need to discuss how to best do it. I see there's been an ask to allow certificates to be loaded from an InputStream for ClientCertificateCredential, in addition to file path. Would we need to have all these overloads on both?
...tity/azure-identity/src/main/java/com/azure/identity/AuthorizationCodeCredentialBuilder.java
Show resolved
Hide resolved
joshfree
added
Azure.Identity
Client
|
Closing old PR from mid-January with no activity since then |
jianghaolu
added
Do Not Merge
blocked
|
Waiting for other languages to prioritize this fix - do not merge now for consistency with other languages. |
|
@jianghaolu Can we get a status update on this PR? It's one of our oldest PRs in the repo, so finding a conclusion for this PR soon would be great! Thanks. |
|
Actions needed for this PR:
|
Tried to push for this a few times but at the time I couldn't get other languages to understand the issue, thus not prioritized. No customer complained about this either so I'm not sure if a fix is still worth it. |
|
Closing this until it becomes a feature request. |
This PR brings an overhaul of the IdentityClient, allowing different
authenticateWithXxx()methods to share the same instance ofConfidentialClientApplication.The reason behind this change is the need of using a
ConfidentialClientApplicationinstead of aPublicClientApplicationfor OAuth2 auth code flow. However,ConfidentialClientApplicationis created in the method usually, whose token cache and refresh token will be lost after authentication. This PR allows using aConfidentialClientApplication's token cache and refresh token for acquiring a new access token.In the case of an invalid certificate for a client application, this PR also fails early at
ClientCertificateCredentialBuilder.build()instead of later atgetToken().Additive changes to user facing APIs
Methods to specify a client secret, or a client certificate, are now added to
AuthorizationCodeCredentialBuilder, throughAuthorizationCodeCredentialBuilder.clientSecret(String),AuthorizationCodeCredentialBuilder.pemCertificate(String),AuthorizationCodeCredentialBuilder.pfxCertificate(String, String).