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

[Identity] Support cross-tenant authentication #8313

Closed
joshfree opened this issue Oct 31, 2019 · 6 comments
Closed

[Identity] Support cross-tenant authentication #8313

joshfree opened this issue Oct 31, 2019 · 6 comments
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.
Milestone

Comments

@joshfree
Copy link
Member

Support the scenario described in https://docs.microsoft.com/bs-latn-ba/azure/azure-resource-manager/authenticate-multi-tenant.

To test it, the shared image gallery scenario takes advantage of this type of authentication header: https://docs.microsoft.com/en-us/azure/virtual-machines/linux/share-images-across-tenants

A previous issue which users were able to workaround: Azure/azure-sdk-for-java#3819. We need to provide a more elegant way.

Related Azure/azure-sdk-for-java#6040

@joshfree joshfree added feature-request This issue requires a new behavior in the product in order be resolved. Client This issue points to a problem in the data-plane of the library. Azure.Identity labels Oct 31, 2019
@joshfree joshfree added this to the [2019] December milestone Oct 31, 2019
@chlowell chlowell removed this from the [2020] January milestone Mar 24, 2020
@chlowell
Copy link
Member

This feature will be implemented by management clients, not within azure-identity.

@chlowell
Copy link
Member

chlowell commented Dec 8, 2020

Reopened for tracking.

@chlowell chlowell reopened this Dec 8, 2020
@chlowell chlowell added this to the Backlog milestone Dec 8, 2020
jiasli referenced this issue in Azure/azure-cli Feb 1, 2021
* [Network] Migrate network to track2 SDK

* Add comments to regular expression change

* Remove useless argument

* Uncomment test
@jiasli
Copy link
Member

jiasli commented Feb 1, 2021

Impact

This limitation makes cross-tenant operations impossible and has so far affected network, vm (future resource) command modules.

Root cause

The old "ADAL/msrest"-based Azure CLI adds headers

  • Authorization
  • x-ms-authorization-auxiliary

in AdalAuthentication.signed_session:

    def signed_session(self, session=None):  # pylint: disable=arguments-differ
        ...
        session.headers['Authorization'] = header
        if external_tenant_tokens:
            aux_tokens = ';'.join(['{} {}'.format(scheme2, tokens2) for scheme2, tokens2, _ in external_tenant_tokens])
            session.headers['x-ms-authorization-auxiliary'] = aux_tokens
        ...

The new "MSAL/Azure Core"-based Azure CLI follows TokenCredential.get_token pattern

    class TokenCredential(Protocol):
            
        def get_token(self, *scopes, **kwargs):
            # type: (*str, **Any) -> AccessToken
            pass

and is not responsible for adding Authorization and x-ms-authorization-auxiliary headers anymore.

Instead, the logic for adding Authorization header is now moved to Azure Core BearerTokenCredentialPolicy:

class BearerTokenCredentialPolicy(_BearerTokenCredentialPolicyBase, SansIOHTTPPolicy):

    def on_request(self, request):
        ...
            self._token = self._credential.get_token(*self._scopes)
        self._update_headers(request.http_request.headers, self._token.token)

Due to the lack of the logic to add x-ms-authorization-auxiliary, cross-tenant operation can't be accomplished.

Workaround

The current workaround Azure CLI adopted is to manually add x-ms-authorization-auxiliary header, as shown in Azure/azure-cli#15750 (comment):

    return client.gallery_image_versions.begin_create_or_update(
        ...
        headers={'x-ms-authorization-auxiliary': external_bearer_token}

Problem with this workaround:

  • It's against the TokenCredential.get_token pattern.
  • It can't refresh the access token which means long-running operation will fail when the token provided to x-ms-authorization-auxiliary expires.

Possible solution

SDK clients' __init__ method can take an optional external_credentials, like

class NetworkManagementClient(NetworkManagementClientOperationsMixin, MultiApiClientMixin, _SDKClient):

    def __init__(
        self,
        credential,  # type: "TokenCredential"
        external_credentials,
        ...        

When external_credentials is provided, for each request, Azure Core or Azure Management Core calls get_token on each item of external_credentials and the access tokens are added to the request in x-ms-authorization-auxiliary header, joined by ;:

x-ms-authorization-auxiliary: Bearer <token1>;Bearer <token2>;...

References

@jiasli
Copy link
Member

jiasli commented Jan 27, 2022

Azure CLI now uses a workaround to implement cross-tenant authentication by adding x-ms-authorization-auxiliary header via headers kwarg which is used to initialize HeadersPolicy:

https://github.com/Azure/azure-cli/blob/110f7b402020f3d3ebd2bfb923ac5a01d026cdd1/src/azure-cli-core/azure/cli/core/commands/client_factory.py#L180-L189

    # Track 2 currently lacks the ability to take external credentials.
    #   https://github.com/Azure/azure-sdk-for-python/issues/8313
    # As a temporary workaround, manually add external tokens to 'x-ms-authorization-auxiliary' header.
    #   https://docs.microsoft.com/en-us/azure/azure-resource-manager/management/authenticate-multi-tenant
    if hasattr(cred, "get_auxiliary_tokens"):
        aux_tokens = cred.get_auxiliary_tokens(*scopes)
        if aux_tokens:
            # Hard-code scheme to 'Bearer' as _BearerTokenCredentialPolicyBase._update_headers does.
            client_kwargs['headers']['x-ms-authorization-auxiliary'] = \
                ', '.join("Bearer {}".format(token.token) for token in aux_tokens)

So implementing this feature request is no longer mandatory from Azure CLI perspective.

@jiasli
Copy link
Member

jiasli commented Jan 27, 2022

According to Azure/azure-sdk-for-java#3819 (comment), to support cross-tenant authentication natively, Java SDK adopted similar approach as my proposal (#8313 (comment)) by accepting multiple credentials:

            AzureResourceManager azureResourceManager = AzureResourceManager
                .configure()
                .withAuxiliaryCredential(anotherTokenCredential)
                .authenticate(credential, profile)
                .withDefaultSubscription();

Copy link

Hi @joshfree, we deeply appreciate your input into this project. Regrettably, this issue has remained inactive for over 2 years, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 18, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 18, 2024
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

No branches or pull requests

4 participants