Add Token Caching Support For Managed Identity #30282

g2vinay · 2022-08-04T20:54:51Z

Integrates the Managed Identity Auth flow to use In Memory Token Caching

sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java

...ty/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClientOptions.java

...zure-identity/src/main/java/com/azure/identity/implementation/ManagedIdentityParameters.java

...tity/azure-identity/src/main/java/com/azure/identity/implementation/ManagedIdentityType.java

azure-sdk · 2022-08-04T21:03:33Z

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-identity

billwert

Can we add better tests for the new codepath? I think the modification to the existing tests is just to ensure we hit the getTokenFromTargetManagedIdentity change.
In the places where a public credential type has been changed to call authenticateWithManagedIdentityConfidentialClient should we private the correponding IdentityClient method? (For example authenticateToArcManagedIdentityEndpoint

billwert · 2022-08-05T00:16:26Z

sdk/identity/azure-identity/src/main/java/com/azure/identity/ManagedIdentityCredential.java

+                return clientOptions.setManagedIdentityType(ManagedIdentityType.VM);
+            case AKS:
+                return clientOptions.setManagedIdentityType(ManagedIdentityType.AKS);
+            default:


is NONE really a valid value? should this be an error?

Removed NONE on second design iteration.

Hmm. I'm not sure that's entirely right either - now won't the default value be VM? We probably need a NONE or DEFAULT to indicate the unchosen value?

yeah in our code flow we default to VM, that's our default based on the logic we follow.
None or Unchosen are invalid states to have and are equivalent to the value being null.

sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java

...zure-identity/src/main/java/com/azure/identity/implementation/ManagedIdentityParameters.java

...tity/azure-identity/src/main/java/com/azure/identity/implementation/ManagedIdentityType.java

sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java

g2vinay · 2022-08-11T16:24:31Z

Can we add better tests for the new codepath? I think the modification to the existing tests is just to ensure we hit the getTokenFromTargetManagedIdentity change.

In the places where a public credential type has been changed to call authenticateWithManagedIdentityConfidentialClient should we private the correponding IdentityClient method? (For example authenticateToArcManagedIdentityEndpoint

For 1. We can add some Identity Client level helper method tests, which I will push in next commit, Msal callback flow is tested on msal end

For 2. It is an impl class, nothing that impacts users from API point of view, we can make it private, but won't create any user impact.

billwert · 2022-08-11T16:38:19Z

For 2. It is an impl class, nothing that impacts users from API point of view, we can make it private, but won't create any user impact.

Yeah, I was thinking to be clear about our intent that these methods are not to be used from other creds in the future.

sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentityClient.java

billwert

Vinay Gera added 2 commits August 1, 2022 12:11

update

5ff9852

update

a921874

ghost added the Azure.Identity label Aug 4, 2022