AzureAD / microsoft-authentication-library-for-python

Microsoft Authentication Library (MSAL) for Python makes it easy to authenticate to Microsoft Entra ID. General docs are available here https://learn.microsoft.com/entra/msal/python/ Stable APIs are documented here https://msal-python.readthedocs.io. Questions can be asked on www.stackoverflow.com with tag "msal" + "python".
https://stackoverflow.com/questions/tagged/azure-ad-msal+python
Other
788 stars 194 forks source link

Support `max_age` in `initiate_auth_code_flow()` for OIDC #381

Closed kevindixon closed 2 years ago

kevindixon commented 3 years ago

Describe the bug We have a need to be able to force re-authentication for certain operations in our product.

Whilst ConfidentialClientApplication.initiate_auth_code_flow supports prompt=login this is an imperfect solution because our side there is no way to confirm that re-authentication took place.

This right way to do this is to pass max_age=0 (ref) and use the auth_time claim value to confirm re-authentication took place.

From what I can see ConfidentialClientApplication.initiate_auth_code_flow doesn't support max_age

To Reproduce N/A

Expected behavior max_age is supported, and functions as defined in the OIDC spec

What you see instead Exception thrown if max_age passed.

The MSAL Python version you are using msal==1.12.0

Additional context Add any other context about the problem here.

rayluo commented 3 years ago

Labeling this as enhancement, for now. We will need to check with our service-side team to see whether max_age is supported by our backend. max_age is not currently documented in our doc.

rayluo commented 3 years ago

@kevindixon , thanks for this suggestion! There is a PR available now. And you can test it by installing the cutting-edge MSAL by:

pip install git+https://github.com/AzureAD/microsoft-authentication-library-for-python.git@max_age

Let us know whether it works for you.

kevindixon commented 3 years ago

@rayluo thanks for the speedy response. About to go on vacation, but will endeavour to take a look on my return

kevindixon commented 3 years ago

@rayluo doesn't seem to work as I expect. When I use max_age=0, re-authentication happens as expected, but passing the code to acquire_token_by_auth_code_flow results in an exception in msal/oauth2cli/oidc.py in decode_id_token, line 76:

9. The current time MUST be before the time represented by the exp Claim. The id_token was: {
...
  "iat": 1629829463,
  "nbf": 1629829463,
  "exp": 1629829763,
  "auth_time": 1629829715,
...

By the time I've called acquire_token_by_auth_code_flow exp is after the current time. So, looks like exp has been set to an extraordinarily short time.

Or am I missing something...?

rayluo commented 3 years ago

In practice, there wouldn't make sense to use max_age=0. A rule of thumb is to consider "we support max_age down to five minutes".

By the time I've called acquire_token_by_auth_code_flow exp is after the current time.

When did you call acquire_token_by_auth_code_flow? Based on the snippet you posted above, the exp, even when issued by an edge case max_age=0 is still 5 minutes after iat, which looks good to me. You may also print the current time in your investigation and let us know what you find.

kevindixon commented 3 years ago

max_age=0 DOES make sense in practice - it is essentially an assertion to FORCE re-authentication no matter what. In some secure environments always forcing re-authentication for certain privileged actions is standard practice - for instance, systems conforming to 21 CFR part 11. See https://auth0.com/docs/login/max-age-reauthentication for some discussion on this issue.

This is exactly what I am trying to do - user is already logged in, but attempting a privileged action and I want to be able to FORCE re-authentication (and confirm authentication in the issued token).

Here is the series of operations I have re-run with timings:

  1. At 1629881476 called initiate_auth_code_flow (with no max_age) - this is initial log-in
  2. At 1629881501 called acquire_token_by_auth_code_flow and received back an ID token with an auth_time of 1629881496, exp of 1629885101 (1 hour time) and iat of 1629881201.
  3. At 1629881543 called initiate_auth_code_flow passing max_age of 0) - i.e. privileged operation, force re-auth
  4. At 1629881578 called acquire_token_by_auth_code_flow - this is where the exception is raised. The ID token has auth_time of 1629881570, exp of 1629881578 ("now"), and iat of 1629881278.

So.. the call to acquire_token_by_auth_code_flow appears to produce an ID token with an exp only 8 seconds after auth_time which seems wrong.

Note in both cases that iat appears to always be 5 minutes before the call to acquire_token_by_auth_code_flow which doesn't seem to make sense.

rayluo commented 3 years ago

Hi Kevin, thanks for the feedback. That was a good read. You are right, max_age=0 makes sense.

And I can also reproduce the issue you found. I made some adjustment to the PR, and it would work now. Besides, that PR will now automatically check the auth_time against max_age, and raise exception when the check fails, so that your app does not need to check auth_time. Please test it again.

However, my test revealed some other corner cases. I'll discuss with our service-side team before I can get back to you.

rayluo commented 3 years ago

Hi @kevindixon , did you get some time to test out the updated PR? In your test environment, you would need to do pip uninstall msal and then pip install git+https://github.com/AzureAD/microsoft-authentication-library-for-python.git@max_age. Let us know how it goes. Thanks!

ineesalmeida commented 2 years ago

Hi @rayluo, I work with @kevindixon and just tried the PR version. The max_age parameter seems to work on a first look, but it doesn't seem to be optional, which breaks existing code.

rayluo commented 2 years ago

Hi Inês, thanks for the good catch! We have updated that fix. Please re-do pip uninstall msal and then pip install git+https://github.com/AzureAD/microsoft-authentication-library-for-python.git@max_age. It should work this time. Let us know how it goes. Thanks!

ineesalmeida commented 2 years ago

Hi Ray, seems to work nicely now, thank you! What's the timeframe for this to be released?

rayluo commented 2 years ago

What's the timeframe for this to be released?

Thanks for your confirmation! You are all set. The rest of the tasks are all on our side.

  1. We will merge the fix into our code base, so that it will be automatically included into our next release.
  2. The current issue here will also be (automatically) marked as "Closed", since there will be no more actionable work for this issue.
  3. The actual release will happen at a later time. We do not have a specific date yet, but historically our release cadence is once a month.
  4. When we cut a new release, we will manually update the included issues. At that time, you as existing participants in this issue may receive a notification from github.