add additionally allowed tenant ids support by xiangyan99 · Pull Request #26133 · Azure/azure-sdk-for-python

xiangyan99 · 2022-09-09T20:24:09Z

Cherry picked the commits.

Removed beta changes:

tenant_id for AzureCliCredential
removed VisualStudioCodeCredential from DefaultAzureCredential token chain.
AZURE_CLIENT_CERTIFICATE_PASSWORD support for EnvironmentCredential
validate_authority support

azure-sdk · 2022-09-09T20:45:07Z

API change check

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

azure-identity

mccoyp

Did architects prefer additionally_allowed_tenant_ids over something like additionally_allowed_tenants, and/or is the naming conversation still ongoing?

…ization_code.py Co-authored-by: McCoy Patiño <39780829+mccoyp@users.noreply.github.com>

xiangyan99 · 2022-09-09T23:56:24Z

Did architects prefer additionally_allowed_tenant_ids over something like additionally_allowed_tenants, and/or is the naming conversation still ongoing?

We are working with the architects on the name. :)

…ization_code.py Co-authored-by: McCoy Patiño <39780829+mccoyp@users.noreply.github.com>

pvaneck

With Azure/azure-sdk-for-net#31037 merged after a lot of documentation review/revision, should we reflect the same documentation here for consistency? Or is that a follow-up as a part of #26159?

Co-authored-by: McCoy Patiño <39780829+mccoyp@users.noreply.github.com>

xiangyan99 · 2022-09-15T15:45:30Z

With Azure/azure-sdk-for-net#31037 merged after a lot of documentation review/revision, should we reflect the same documentation here for consistency? Or is that a follow-up as a part of #26159?

We will need more time to finalize the docs/troubleshooting guideline/etc.

My plan is to merge the code and start the API review with our architects and we will have another PR to update the docs.

mccoyp

Aside from my nitpicking, I think this looks good! I don't think I see a negative test though, where we verify that sending an invalid tenant raises the expected error; I think we should have that for at least one credential to validate the behavior of resolve_tenant. Feel free to correct me if there is such a test and I just missed it 🙂

…ization_code.py Co-authored-by: McCoy Patiño <39780829+mccoyp@users.noreply.github.com>

mccoyp

I'm still unsure about the documentation of additionally_allowed_tenants in the CLI and PowerShell credentials since it doesn't seem like the kwarg actually does anything right now, but I'm not going to block the PR on it since the feature will at least apply in the near future.

xiangyan99 · 2022-09-16T23:07:09Z

I'm still unsure about the documentation of additionally_allowed_tenants in the CLI and PowerShell credentials since it doesn't seem like the kwarg actually does anything right now, but I'm not going to block the PR on it since the feature will at least apply in the near future.

Yes. Before we add the support of user-specified tenant-id, additionally_allowed_tenants will not take effect.

* add additionally allowed tenant ids support * Update sdk/identity/azure-identity/azure/identity/_credentials/authorization_code.py Co-authored-by: McCoy Patiño <39780829+mccoyp@users.noreply.github.com> * rename additionally_allowed_tenant_ids * update * Update sdk/identity/azure-identity/azure/identity/_credentials/authorization_code.py Co-authored-by: McCoy Patiño <39780829+mccoyp@users.noreply.github.com> * updates * update * update * update * Update sdk/identity/azure-identity/CHANGELOG.md Co-authored-by: McCoy Patiño <39780829+mccoyp@users.noreply.github.com> * update * Update sdk/identity/azure-identity/azure/identity/_credentials/authorization_code.py Co-authored-by: McCoy Patiño <39780829+mccoyp@users.noreply.github.com> * update * update * update Co-authored-by: McCoy Patiño <39780829+mccoyp@users.noreply.github.com>

add additionally allowed tenant ids support

5c8646b

xiangyan99 requested review from mccoyp and schaabs as code owners September 9, 2022 20:24

ghost added the Azure.Identity label Sep 9, 2022

xiangyan99 requested a review from pvaneck September 9, 2022 23:15

mccoyp reviewed Sep 9, 2022

View reviewed changes

Comment thread sdk/identity/azure-identity/azure/identity/_credentials/authorization_code.py Outdated

Update sdk/identity/azure-identity/azure/identity/_credentials/author…

8bd6873

…ization_code.py Co-authored-by: McCoy Patiño <39780829+mccoyp@users.noreply.github.com>

xiangyan99 added 2 commits September 12, 2022 09:47

rename additionally_allowed_tenant_ids

a8aed7e

update

1d42166

xiangyan99 requested a review from mccoyp September 12, 2022 20:48

mccoyp reviewed Sep 13, 2022

View reviewed changes

Comment thread sdk/identity/azure-identity/tests/test_cli_credential.py

xiangyan99 and others added 5 commits September 13, 2022 17:40

Update sdk/identity/azure-identity/azure/identity/_credentials/author…

d26f8c7

…ization_code.py Co-authored-by: McCoy Patiño <39780829+mccoyp@users.noreply.github.com>

updates

2a5fdc3

update

1be5d85

update

f007319

update

fbcdd0a

xiangyan99 requested a review from mccoyp September 14, 2022 21:55

pvaneck reviewed Sep 15, 2022

View reviewed changes

mccoyp reviewed Sep 15, 2022

View reviewed changes

Comment thread sdk/identity/azure-identity/CHANGELOG.md Outdated

Comment thread sdk/identity/azure-identity/azure/identity/_credentials/authorization_code.py Outdated

Comment thread sdk/identity/azure-identity/azure/identity/_internal/__init__.py Outdated

Update sdk/identity/azure-identity/CHANGELOG.md

d875618

Co-authored-by: McCoy Patiño <39780829+mccoyp@users.noreply.github.com>

update

8b07c2f

xiangyan99 requested review from mccoyp and pvaneck September 15, 2022 20:05

mccoyp reviewed Sep 16, 2022

View reviewed changes

Comment thread sdk/identity/azure-identity/azure/identity/_credentials/authorization_code.py Outdated

Update sdk/identity/azure-identity/azure/identity/_credentials/author…

5e117fb

…ization_code.py Co-authored-by: McCoy Patiño <39780829+mccoyp@users.noreply.github.com>

mpodwysocki mentioned this pull request Sep 16, 2022

[identity] Add support for additional tenants Azure/azure-sdk-for-js#23174

Merged

3 tasks

update

6f25125

mccoyp reviewed Sep 16, 2022

View reviewed changes

Comment thread sdk/identity/azure-identity/tests/test_aad_client.py Outdated

Comment thread sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py

Comment thread sdk/identity/azure-identity/azure/identity/_credentials/azure_powershell.py

xiangyan99 added 2 commits September 16, 2022 11:30

update

e9d68b3

update

fb94c58

xiangyan99 requested a review from mccoyp September 16, 2022 23:00

mccoyp approved these changes Sep 16, 2022

View reviewed changes

xiangyan99 merged commit aa3d536 into main Sep 16, 2022

xiangyan99 deleted the identity_allow_tenants branch September 16, 2022 23:31

Conversation

xiangyan99 commented Sep 9, 2022

Uh oh!

azure-sdk commented Sep 9, 2022

Uh oh!

mccoyp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xiangyan99 commented Sep 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pvaneck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xiangyan99 commented Sep 15, 2022

Uh oh!

mccoyp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mccoyp left a comment

Choose a reason for hiding this comment

Uh oh!

xiangyan99 commented Sep 16, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xiangyan99 commented Sep 9, 2022 •

edited

Loading