Add ChallengeAuthenticationPolicy #17489

chlowell · 2021-03-22T20:26:51Z

Clients expecting authentication challenges should subclass ChallengeAuthenticationPolicy and override on_challenge to handle those challenges.

xiangyan99 · 2021-04-20T15:46:55Z

Are you ready to GA? If not, maybe we want to keep it in feature branch.

chlowell · 2021-04-20T16:04:40Z

Yes, we want to GA this next month.

sdk/core/azure-core/azure/core/_version.py

sdk/core/azure-core/setup.py

xiangyan99 · 2021-04-20T16:33:38Z

sdk/core/azure-core/azure/core/pipeline/policies/_authentication.py

+        # type: () -> bool
+        return not self._token or self._token.expires_on - time.time() < 300
+
+    def authorize_request(self, request, *scopes, **kwargs):


What is the typical way for users to use the policy? Not in the pipeline?

Why in addition to send, we also need to expose authorize_request, on_request & on_challenge?

In a pipeline. Client library developers should subclass this to accommodate their particular services.

authorize_request is a helper method for updating the policy's token and authorizing a request

on_request allows clients to override the policy's default behavior of authorizing requests. For example, Key Vault clients want to send an unauthorized request to elicit an authentication challenge (which would look like this)

on_challenge is the method client libraries override to implement challenge handling for their particular service (ARM, for example)

To help me understand, do you mean users are not supposed to use it directly. Instead, they should always inherit the class and use the customized one?

If they want to handle authentication challenges. There's no particular reason to use this policy otherwise, although doing so wouldn't break anything. The policy by itself authorizes requests with bearer tokens just fine.

It looks to me we set auth header in both authorize_request & on_request. My understanding is the one in on_request will override authorize_request?

Why user needs to set it twice?

on_request is called first when a request arrives. authorize_request is a helper for subclasses to use, so they don't have to handle the credential or cached token (for example). Assuming neither method is overridden, authorize_request would overwrite a token applied by on_request. Which is fine, the policy is designed for a scenario like this:

on_request authorizes the request

policy sends the request to the RP

RP responds with a challenge

on_challenge authorizes the request with a new token (perhaps by calling authorize_request)

policy sends the request to the RP again

xiangyan99 · 2021-04-27T16:19:46Z

sdk/core/azure-core/azure/core/pipeline/policies/_authentication.py

-            raise ServiceRequestError(
-                "Bearer token authentication is not permitted for non-TLS protected (non-https) URLs."
-            )
+        return _enforce_https(request)


Are you sure you want to use the exactly same method name? Looks confusing. :)

Hmmm I hear you but the good name is taken 🤣

🤷 _enforce_https_impl?

xiangyan99 · 2021-04-27T16:20:27Z

sdk/core/azure-core/azure/core/pipeline/policies/_authentication.py

+
+    def _need_new_token(self):
+        # type: () -> bool
+        return not self._token or self._token.expires_on - time.time() < 300


Do we want to make '300' configurable?

Not initially. We could add configuration later.

glaubitz · 2021-05-10T10:07:03Z

It seems this feature has already been added to the azure-mgmt-core package and these patches are the missing part for azure-core if I understand correctly?

glaubitz@suse-laptop:~> az login
The default web browser has been opened at https://login.microsoftonline.com/common/oauth2/authorize. Please continue the login in the web browser. If no web browser is available or if the web browser fails to open, use device code flow with `az login --use-device-code`.
You have logged in. Now let us find all the subscriptions to which you have access...
The command failed with an unexpected error. Here is the traceback:
cannot import name 'ChallengeAuthenticationPolicy' from 'azure.core.pipeline.policies' (/usr/lib/python3.8/site-packages/azure/core/pipeline/policies/__init__.py)
Traceback (most recent call last):
  File "/usr/lib/python3.8/site-packages/knack/cli.py", line 231, in invoke
    cmd_result = self.invocation.execute(args)
  File "/usr/lib/python3.8/site-packages/azure/cli/core/commands/__init__.py", line 657, in execute
    raise ex
  File "/usr/lib/python3.8/site-packages/azure/cli/core/commands/__init__.py", line 720, in _run_jobs_serially
    results.append(self._run_job(expanded_arg, cmd_copy))
  File "/usr/lib/python3.8/site-packages/azure/cli/core/commands/__init__.py", line 691, in _run_job
    result = cmd_copy(params)
  File "/usr/lib/python3.8/site-packages/azure/cli/core/commands/__init__.py", line 328, in __call__
    return self.handler(*args, **kwargs)
  File "/usr/lib/python3.8/site-packages/azure/cli/core/commands/command_operation.py", line 121, in handler
    return op(**command_args)
  File "/usr/lib/python3.8/site-packages/azure/cli/command_modules/profile/custom.py", line 152, in login
    subscriptions = profile.find_subscriptions_on_login(
  File "/usr/lib/python3.8/site-packages/azure/cli/core/_profile.py", line 201, in find_subscriptions_on_login
    subscriptions = subscription_finder.find_through_authorization_code_flow(
  File "/usr/lib/python3.8/site-packages/azure/cli/core/_profile.py", line 917, in find_through_authorization_code_flow
    result = self._find_using_common_tenant(token_entry[_ACCESS_TOKEN], resource)
  File "/usr/lib/python3.8/site-packages/azure/cli/core/_profile.py", line 961, in _find_using_common_tenant
    client = self._arm_client_factory(token_credential)
  File "/usr/lib/python3.8/site-packages/azure/cli/core/_profile.py", line 871, in create_arm_client_factory
    client_type = self._get_subscription_client_class()
  File "/usr/lib/python3.8/site-packages/azure/cli/core/_profile.py", line 1050, in _get_subscription_client_class
    from azure.cli.core.vendored_sdks.subscriptions import SubscriptionClient
  File "/usr/lib/python3.8/site-packages/azure/cli/core/vendored_sdks/subscriptions/__init__.py", line 9, in <module>
    from ._subscription_client import SubscriptionClient
  File "/usr/lib/python3.8/site-packages/azure/cli/core/vendored_sdks/subscriptions/_subscription_client.py", line 12, in <module>
    from azure.mgmt.core import ARMPipelineClient
  File "/usr/lib/python3.8/site-packages/azure/mgmt/core/__init__.py", line 29, in <module>
    from ._pipeline_client import ARMPipelineClient
  File "/usr/lib/python3.8/site-packages/azure/mgmt/core/_pipeline_client.py", line 32, in <module>
    from .policies import ARMAutoResourceProviderRegistrationPolicy, ARMHttpLoggingPolicy
  File "/usr/lib/python3.8/site-packages/azure/mgmt/core/policies/__init__.py", line 28, in <module>
    from ._authentication import ARMChallengeAuthenticationPolicy
  File "/usr/lib/python3.8/site-packages/azure/mgmt/core/policies/_authentication.py", line 32, in <module>
    from azure.core.pipeline.policies import ChallengeAuthenticationPolicy
ImportError: cannot import name 'ChallengeAuthenticationPolicy' from 'azure.core.pipeline.policies' (/usr/lib/python3.8/site-packages/azure/core/pipeline/policies/__init__.py)
To open an issue, please run: 'az feedback'
glaubitz@suse-laptop:~>

chlowell · 2021-05-10T15:08:05Z

It's missing from our master branch, but this code is in the latest azure-core beta, 1.14.0b1. Only the latest azure-mgmt-core beta (currently 1.3.0b1) requires this code. So, the solution to that ImportError is to install azure-core 1.14.0b1 or azure-mgmt-core 1.2. Which version of the CLI are you using?

FYI @jiasli

glaubitz · 2021-05-10T15:23:29Z

@chlowell Thanks for the explanation. I actually packaged azure-mgmt-core 1.3.0b1:

https://build.opensuse.org/package/show/devel:languages:python:azure/python-azure-mgmt-core

From a packager's view, the problem with the Azure SDK and CLI is that some of the individual packages require beta versions of other packages as well, even when these packages have a stable version number themselves. In particular, the Azure CLI packages depend on beta versions.

I will update the azure-core package and see if that helps.

chlowell · 2021-05-13T16:45:31Z

Next month's azure-core release will end the requirement to install matching azure-core and azure-mgmt-core betas. I hope that will make managing the dependencies easier but I don't know when the CLI will update its requirements. It seems odd to me that a stable version of the Azure CLI would require beta SDK packages. Have you opened an issue about these packaging difficulties in Azure/azure-cli?

As for this PR, I'm closing it in favor of #18437.

chlowell added the Azure.Core label Mar 22, 2021

chlowell mentioned this pull request Mar 22, 2021

Authentication policy performance test #17490

Merged

chlowell force-pushed the feature/challenge-auth-policy branch from 2db24be to d1870ec Compare April 2, 2021 22:33

chlowell marked this pull request as ready for review April 19, 2021 23:39

chlowell requested review from lmazuel and xiangyan99 as code owners April 19, 2021 23:39

chlowell requested review from christothes, g2vinay, mccoyp, sadasant and schaabs April 19, 2021 23:39

xiangyan99 reviewed Apr 20, 2021

View reviewed changes

sdk/core/azure-core/azure/core/_version.py Outdated Show resolved Hide resolved

xiangyan99 reviewed Apr 20, 2021

View reviewed changes

sdk/core/azure-core/setup.py Outdated Show resolved Hide resolved

xiangyan99 reviewed Apr 20, 2021

View reviewed changes

chlowell added 5 commits April 26, 2021 11:51

Async/ChallengeAuthenticationPolicy (azure-core)

75406a0

tests (azure-core)

420909f

package metadata (azure-core)

2f52480

remove 'challenge' parameter from on_challenge (core)

8938731

update tests (core)

15e2521

chlowell force-pushed the feature/challenge-auth-policy branch from 0125a62 to 15e2521 Compare April 26, 2021 19:01

chlowell requested a review from xiangyan99 April 26, 2021 20:21

xiangyan99 reviewed Apr 27, 2021

View reviewed changes

chlowell mentioned this pull request Apr 29, 2021

Extend BearerTokenCredentialPolicy to handle auth challenges #18437

Merged

chlowell closed this May 13, 2021

Add ChallengeAuthenticationPolicy #17489

Add ChallengeAuthenticationPolicy #17489

Uh oh!

Conversation

chlowell commented Mar 22, 2021

Uh oh!

xiangyan99 commented Apr 20, 2021

Uh oh!

chlowell commented Apr 20, 2021

Uh oh!

Uh oh!

Uh oh!

xiangyan99 Apr 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glaubitz commented May 10, 2021

Uh oh!

chlowell commented May 10, 2021

Uh oh!

glaubitz commented May 10, 2021

Uh oh!

chlowell commented May 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xiangyan99 Apr 20, 2021 •

edited

Loading