Skip to content

Remove acquire_token_silent(..., account=None) usage in a backward-compatible way #577

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

Merged
merged 1 commit into from
Jul 22, 2023

Conversation

rayluo
Copy link
Collaborator

@rayluo rayluo commented Jun 30, 2023

Problem Statement

Historically, MSAL Python's acquire_token_silent(..., account=...) has two usages:

  1. it accepts a non-empty account to find token for that user. (This can be used in both PublicClientApplication and ConfidentialClientApplication.)
  2. it also accepts account=None to find a token for the current app. (This is only used in combination with ConfidentialClientApplication.acquire_token_for_client().)

Our existing documentations and samples (for example, this one) always put #1's acquire_token_silent(...) call inside the if accounts clause, such that, when the account is None (i.e. the user signed out), the code path shall NOT accidentally attempt acquire_token_silent(..., account=None) otherwise it would potentially return a token for a different entity.

However, that relies on the app developer to NOT call acquire_token_silent(..., account=None) accidentally.

Proposal

In this PR, acquire_token_silent(..., account=None) has been changed to a NO-OP. And the usage #2 will be fulfilled by acquire_token_for_client()'s automatically looking up cache, as demonstrated in the updated samples in this PR.

The change is therefore backward compatible.

As a byproduct, this PR also modifies the telemetry so that the acquire_token_for_client(...) will include token refresh reason. For example, 4|730,2|, instead of previous 4|730,0|.

@rayluo rayluo force-pushed the silent-adjustment branch 3 times, most recently from 659eee2 to 93e5643 Compare June 30, 2023 20:20
@rayluo rayluo marked this pull request as ready for review June 30, 2023 23:56
@rayluo rayluo force-pushed the silent-adjustment branch 3 times, most recently from 05bea64 to 5423c50 Compare July 17, 2023 18:01
…mpatible way

Now acquire_token_for_client()'s cache behavior will have corresponding api id

Continue to disallow acquire_token_for_client(..., force_refresh=True)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant