azure-sdk-for-go

azure-sdk-for-go copied to clipboard

ARM cross-tenant auth needs support from azcore and azidentity to function end to end (see ARM docs for a rundown of how this feature works). This PR is the azcore part, with new API for specifying auxiliary tenants for ARM clients and requesting access tokens from arbitrary tenants. I'll open another PR with the azidentity part--it depends on this one, and the next MSAL release.

Usage will look like this:

// cred will request tokens from the primary tenant unless another is specified in GetToken args
cred, err := azidentity.NewClientSecretCredential("primary-tenant", "client-id", "secret", &azidentity.ClientSecretCredentialOptions{
	// cred will (attempt to) authenticate in any requested tenant; can also list specific tenants here
	AdditionallyAllowedTenants: []string{"*"},
})
client, err := armresources.NewClient("subscription", cred, &arm.ClientOptions{
	// client will add a token from each auxiliary tenant to every request's x-ms-authorization-auxiliary header
	AuxiliaryTenants: []string{"aux-tenant"},
})

In the example, is the "*" a well-known concept for MSAL, or is this something we're inventing?

It's our own invention. MSAL doesn't have a similar allow list because its multitenant API is always explicit.

Should our API be explicit too (it's what we did in track 1)? Are there any downsides/risks with using a wildcard for the additional tenants?

Should our API be explicit too (it's what we did in track 1)?

We can't (well, not without major surgery) make the tenant explicit to the developer in every token request because an application using an SDK client doesn't request tokens directly. The client does that as needed, and the developer doesn't always know which client call will trigger a token request or which tenant a request should go to.

Are there any downsides/risks with using a wildcard for the additional tenants?

Yes, there's some risk for applications that accept URIs from users (some more on this in the recent blog post). We need a wildcard escape hatch anyway because some applications can't know in advance all the tenants whose resources they will want to access.

I meant explicit in regard to only allowing a list of tenants, no wildcard (this is what we did in track 1). Regardless, it sounds like the wildcard is needed to support some scenarios.

Retargeted this to a feature branch so we can review and merge it without interfering with other features entering main.

Is there any update on this pr? Thanks in advance!

@chlowell what's the trigger for getting this into main for a beta?

We need the azidentity part of this (#19529) to complete the end-to-end feature, but that's waiting on the next version of MSAL, which has been delayed by bugs and holidays. @MartinForReal if you need a workaround in the meantime, you can try this: https://github.com/Azure/azure-sdk-for-go/issues/19726#issuecomment-1367527596

@jhendrixMSFT are you thinking of an azcore beta?

I couldn't remember if the MSAL work was complete. Clearly, we need that before we can release a beta of this feature.