[Key Vault] Add support for multi-tenant authentication #21290
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mccoyp
added
KeyVault
Client
mccoyp
changed the title
mccoyp
force-pushed
the
kv-multitenant
branch
from
882eae0
to
e50dfb1
Compare
xiangyan99
reviewed
Nov 5, 2021
|
|
||
|
|
||
| class AsyncChallengeAuthPolicy(ChallengeAuthPolicyBase, AsyncHTTPPolicy): | ||
| class AsyncChallengeAuthPolicy(AsyncBearerTokenCredentialPolicy): |
There was a problem hiding this comment.
Could you give more context about this change?
There was a problem hiding this comment.
Just updated the PR description with more context!
chlowell
reviewed
Nov 5, 2021
...yvault-administration/azure/keyvault/administration/_internal/async_challenge_auth_policy.py
Outdated
Show resolved
Hide resolved
...ault/azure-keyvault-administration/azure/keyvault/administration/_internal/http_challenge.py
Outdated
Show resolved
Hide resolved
chlowell
reviewed
Nov 8, 2021
sdk/keyvault/azure-keyvault-keys/azure/keyvault/keys/_shared/async_challenge_auth_policy.py
Outdated
Show resolved
Hide resolved
chlowell
reviewed
Nov 9, 2021
...yvault-administration/azure/keyvault/administration/_internal/async_challenge_auth_policy.py
Outdated
Show resolved
Hide resolved
...yvault-administration/azure/keyvault/administration/_internal/async_challenge_auth_policy.py
Outdated
Show resolved
Hide resolved
chlowell
reviewed
Nov 9, 2021
...ure-keyvault-certificates/azure/keyvault/certificates/_shared/async_challenge_auth_policy.py
Outdated
Show resolved
Hide resolved
...yvault-administration/azure/keyvault/administration/_internal/async_challenge_auth_policy.py
Outdated
Show resolved
Hide resolved
mccoyp
commented
Nov 9, 2021
| """policy for handling HTTP authentication challenges""" | ||
|
|
||
| def __init__(self, credential: "AsyncTokenCredential", **kwargs: "Any") -> None: | ||
| def __init__(self, credential: "AsyncTokenCredential", *scopes: str, **kwargs: "Any") -> None: |
There was a problem hiding this comment.
I added an __init__ back in because _need_new_token made me realize that we shouldn't add any dependencies on private parent class fields. I had been referencing self._credential and self._token from the parent class before
|
|
||
| @property | ||
| def _need_new_token(self) -> bool: | ||
| # pylint:disable=invalid-overridden-method |
There was a problem hiding this comment.
I debated internally a bit over adding this override as a property. I suppressed this warning because the situation in azure-core is likely unintentional, where _need_new_token is a property for BearerTokenCredentialPolicy but a method for AsyncBearerTokenCredentialPolicy. Assuming that will be changed in the future, we can remove this suppression, but I figured that makes more sense than re-implementing the property/method pattern here for the sake of consistency
chlowell
approved these changes
Nov 10, 2021
...ure-keyvault-administration/azure/keyvault/administration/_internal/challenge_auth_policy.py
Outdated
Show resolved
Hide resolved
iscai-msft
added a commit
to iscai-msft/azure-sdk-for-python
that referenced
this pull request
Nov 10, 2021
…into add_webpubsub_tests * 'main' of https://github.com/Azure/azure-sdk-for-python: [Key Vault] Add support for multi-tenant authentication (Azure#21290) [webpubsub] regen with hub as a client parameter (Azure#21688) update automatic close mechanism (Azure#21580) [Test Proxy] Add fixture to automatically start/stop Docker container (Azure#21538) Update Monitor Query API ref link (Azure#21683) Migration Guide from Azure-loganalytics (Azure#21674) Update docs for Web PubSub GA (Azure#21659) Update CHANGELOG.md (Azure#21681) Increment version for formrecognizer releases (Azure#21678) Increment version for videoanalyzer releases (Azure#21455) Increment version for cognitivelanguage releases (Azure#21566) Increment version for storage releases (Azure#21652) Increment version for communication releases (Azure#21667) raise decode error instead of ContentDecodingError (Azure#19433) Update CHANGELOG.md (Azure#21679) resolve mac agent failure (Azure#21677) Re-add get-codeowners.ps1 (Azure#21676) [SchemaRegistry] remove schema prefix in params (Azure#21675) Validate python docs packages using docker (Azure#21657) update git helper (Azure#21670)
This was referenced Feb 10, 2022
azure-sdk
pushed a commit
to azure-sdk/azure-sdk-for-python
that referenced
this pull request
Nov 29, 2022
{AzureRecoveryServices} fixes Azure/azure-rest-api-specs#20953 (Azure#21290)
* {AzureRecoveryServices} fixes Azure/azure-rest-api-specs#20953
fixes Azure/azure-rest-api-specs#20953
swagger link: https://github.com/Azure/azure-rest-api-specs/blob/main/specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2021-12-01/bms.json#L4362
example link: https://github.com/Azure/azure-rest-api-specs/blob/main/specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2021-12-01/examples/ResourceGuardProxyCRUD/PutResourceGuardProxy.json
In the swagger file, there is no parameter named resourceGuardResourceId in ResourceGuardProxy_Put while in the example there is. the parameter is needed to create.
* Update bms.json
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #20698.
Context: now that
azure-identitysupports providing a tenant ID to token requests, we can allow Key Vault clients to make use of tenant discovery. This updates the challenge authentication policy to parse out the tenant ID provided in a challenge and authenticate requests with that tenant. Based off of work Charles did in his fork of the repo, this also updates the challenge auth policy to inherit fromazure-core's BearerTokenCredentialPolicy instead of re-implementing the wheel. As a result, doing so also requiresazure-core>= 1.15.0 (see changelog).