Skip to content

Ability to specify tenant_id#25207

Merged
xiangyan99 merged 11 commits intoAzure:mainfrom
tikicoder:patch-1
Jul 22, 2022
Merged

Ability to specify tenant_id#25207
xiangyan99 merged 11 commits intoAzure:mainfrom
tikicoder:patch-1

Conversation

@tikicoder
Copy link
Copy Markdown
Contributor

Updates the Azure CLI SDK To allow to create credential objects that are connected to tenants. This is EXTREMELY useful when you work with multiple tenants.

Description

This allows developers to specify what tenant the credentials should use. This way if you have access to multiple you do not need to worry about switching other ways.

Testing Guidelines

I created the following wrapper class locally and it has fixed my issue. No reason I should have to be that hacky.

from azure.identity import AzureCliCredential as main_AzureCliCredential

class AzureCliCredential(main_AzureCliCredential):
    def __init__(self, tenant_id = None):
        main_AzureCliCredential.__init__(self)

        self.tenant_id = tenant_id

    def get_token(self, *scopes: str, **kwargs):
        if self.tenant_id is not None:
            if kwargs is None:
                kwargs = {}
            kwargs["tenant_id"] = self.tenant_id
            return main_AzureCliCredential.get_token(self, *scopes, **kwargs)
        
        return main_AzureCliCredential.get_token(self, *scopes, **kwargs)

Updates the Azure CLI SDK To allow to create credential objects that are connected to tenants. This is EXTREMELY useful when you work with multiple tenants.
@ghost ghost added Azure.Identity customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Jul 13, 2022
@ghost
Copy link
Copy Markdown

ghost commented Jul 13, 2022

Thank you for your contribution tikicoder! We will review the pull request and get back to you soon.

@mccoyp
Copy link
Copy Markdown
Member

mccoyp commented Jul 14, 2022

Hi @tikicoder, thank you for the PR and feedback! Some folks are out of office today, but we'll discuss your proposal and get back to you as soon as possible 🙂

@tikicoder
Copy link
Copy Markdown
Contributor Author

Hi @tikicoder, thank you for the PR and feedback! Some folks are out of office today, but we'll discuss your proposal and get back to you as soon as possible 🙂

Thanks

@xiangyan99
Copy link
Copy Markdown
Member

Thank you for the contribution.

Could you add the sync version too and also add some tests?

Thank you.

@xiangyan99 xiangyan99 added the needs-author-feedback Workflow: More information is needed from author to address the issue. label Jul 18, 2022
@tikicoder
Copy link
Copy Markdown
Contributor Author

Thank you for the contribution.

Could you add the sync version too and also add some tests?

Thank you.

Ok that should be ready.

Comment thread sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py Outdated
Copy link
Copy Markdown
Member

@xiangyan99 xiangyan99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test fails with error:

TypeError: resolve_tenant() got multiple values for keyword argument 'tenant_id'

Seems like we need:

if 'tenant_id' not in kwargs:
tenant = resolve_tenant(default_tenant= "", tenant_id= self.tenant_id, **kwargs)
else:
tenant = resolve_tenant(default_tenant= "", **kwargs)

@tikicoder
Copy link
Copy Markdown
Contributor Author

Test fails with error:

TypeError: resolve_tenant() got multiple values for keyword argument 'tenant_id'

Seems like we need:

if 'tenant_id' not in kwargs: tenant = resolve_tenant(default_tenant= "", tenant_id= self.tenant_id, **kwargs) else: tenant = resolve_tenant(default_tenant= "", **kwargs)

Ok I updated it so instead of passing in default_tenant of empty it now passes something in. In theory, I would hope the tenant_id that is being passed in is None. I could update to include it in the logging but that seems overkill. Either this will work or it won't. It would make sense also that if you pass in a tenant_id on the class level it will become the default.

Copy link
Copy Markdown
Member

@xiangyan99 xiangyan99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pylint error:

************* Module azure.identity.aio._credentials.azure_cli
azure/identity/aio/_credentials/azure_cli.py:63: [C0303(trailing-whitespace), ] Trailing whitespace
************* Module azure.identity._credentials.azure_cli
azure/identity/_credentials/azure_cli.py:75: [C0303(trailing-whitespace), ] Trailing whitespace

:)

@tikicoder
Copy link
Copy Markdown
Contributor Author

That should be resolved now. I edited it in the github editor so looks like it added a tab that wasn't needed.

@xiangyan99 xiangyan99 merged commit addc839 into Azure:main Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure.Identity customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-author-feedback Workflow: More information is needed from author to address the issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants