-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Extend BearerTokenCredentialPolicy to handle auth challenges #18437
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
Changes from all commits
806c06e
4a61fed
8c3365a
18dea5c
e8f2efe
2613ef4
ba60e85
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ | |
| import time | ||
| import six | ||
|
|
||
| from . import SansIOHTTPPolicy | ||
| from . import HTTPPolicy, SansIOHTTPPolicy | ||
| from ...exceptions import ServiceRequestError | ||
|
|
||
| try: | ||
|
|
@@ -18,7 +18,7 @@ | |
| # pylint:disable=unused-import | ||
| from typing import Any, Dict, Optional | ||
| from azure.core.credentials import AccessToken, TokenCredential, AzureKeyCredential, AzureSasCredential | ||
| from azure.core.pipeline import PipelineRequest | ||
| from azure.core.pipeline import PipelineRequest, PipelineResponse | ||
|
|
||
|
|
||
| # pylint:disable=too-few-public-methods | ||
|
|
@@ -71,7 +71,7 @@ def _need_new_token(self): | |
| return not self._token or self._token.expires_on - time.time() < 300 | ||
|
|
||
|
|
||
| class BearerTokenCredentialPolicy(_BearerTokenCredentialPolicyBase, SansIOHTTPPolicy): | ||
| class BearerTokenCredentialPolicy(_BearerTokenCredentialPolicyBase, HTTPPolicy): | ||
| """Adds a bearer token Authorization header to requests. | ||
|
|
||
| :param credential: The credential. | ||
|
|
@@ -82,17 +82,94 @@ class BearerTokenCredentialPolicy(_BearerTokenCredentialPolicyBase, SansIOHTTPPo | |
|
|
||
| def on_request(self, request): | ||
| # type: (PipelineRequest) -> None | ||
| """Adds a bearer token Authorization header to request and sends request to next policy. | ||
| """Called before the policy sends a request. | ||
|
|
||
| :param request: The pipeline request object | ||
| :type request: ~azure.core.pipeline.PipelineRequest | ||
| The base implementation authorizes the request with a bearer token. | ||
|
|
||
| :param ~azure.core.pipeline.PipelineRequest request: the request | ||
| """ | ||
| self._enforce_https(request) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do you think about moving _enforce_https into shared utils?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense the next time another policy needs it. Do we want the key and SAS policies to enforce HTTPS as well?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I asked this because I saw ACR also used it. :) |
||
|
|
||
| if self._token is None or self._need_new_token: | ||
| self._token = self._credential.get_token(*self._scopes) | ||
| self._update_headers(request.http_request.headers, self._token.token) | ||
|
|
||
| def authorize_request(self, request, *scopes, **kwargs): | ||
| # type: (PipelineRequest, *str, **Any) -> None | ||
| """Acquire a token from the credential and authorize the request with it. | ||
|
|
||
| Keyword arguments are passed to the credential's get_token method. The token will be cached and used to | ||
| authorize future requests. | ||
|
|
||
| :param ~azure.core.pipeline.PipelineRequest request: the request | ||
| :param str scopes: required scopes of authentication | ||
| """ | ||
| self._token = self._credential.get_token(*scopes, **kwargs) | ||
| self._update_headers(request.http_request.headers, self._token.token) | ||
|
|
||
| def send(self, request): | ||
| # type: (PipelineRequest) -> PipelineResponse | ||
| """Authorize request with a bearer token and send it to the next policy | ||
|
|
||
| :param request: The pipeline request object | ||
| :type request: ~azure.core.pipeline.PipelineRequest | ||
| """ | ||
| self.on_request(request) | ||
| try: | ||
| response = self.next.send(request) | ||
| self.on_response(request, response) | ||
| except Exception: # pylint:disable=broad-except | ||
| handled = self.on_exception(request) | ||
| if not handled: | ||
| raise | ||
| else: | ||
| if response.http_response.status_code == 401: | ||
| self._token = None # any cached token is invalid | ||
| if "WWW-Authenticate" in response.http_response.headers: | ||
| request_authorized = self.on_challenge(request, response) | ||
| if request_authorized: | ||
| response = self.next.send(request) | ||
| self.on_response(request, response) | ||
|
|
||
| return response | ||
|
|
||
| def on_challenge(self, request, response): | ||
| # type: (PipelineRequest, PipelineResponse) -> bool | ||
| """Authorize request according to an authentication challenge | ||
|
|
||
| This method is called when the resource provider responds 401 with a WWW-Authenticate header. | ||
|
|
||
| :param ~azure.core.pipeline.PipelineRequest request: the request which elicited an authentication challenge | ||
| :param ~azure.core.pipeline.PipelineResponse response: the resource provider's response | ||
| :returns: a bool indicating whether the policy should send the request | ||
| """ | ||
| # pylint:disable=unused-argument,no-self-use | ||
| return False | ||
|
|
||
| def on_response(self, request, response): | ||
| # type: (PipelineRequest, PipelineResponse) -> None | ||
| """Executed after the request comes back from the next policy. | ||
|
|
||
| :param request: Request to be modified after returning from the policy. | ||
| :type request: ~azure.core.pipeline.PipelineRequest | ||
| :param response: Pipeline response object | ||
| :type response: ~azure.core.pipeline.PipelineResponse | ||
| """ | ||
|
|
||
| def on_exception(self, request): | ||
| # type: (PipelineRequest) -> bool | ||
| """Executed when an exception is raised while executing the next policy. | ||
|
|
||
| This method is executed inside the exception handler. | ||
|
|
||
| :param request: The Pipeline request object | ||
| :type request: ~azure.core.pipeline.PipelineRequest | ||
| :return: False by default, override with True to stop the exception. | ||
| :rtype: bool | ||
| """ | ||
| # pylint: disable=no-self-use,unused-argument | ||
| return False | ||
|
|
||
|
|
||
| class AzureKeyCredentialPolicy(SansIOHTTPPolicy): | ||
| """Adds a key header for the provided credential. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember we planned to modify
to check hasattibute('on_request'), will that change conflict with this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's #16726. No conflict with this change, and this change doesn't require that PR.