-
Notifications
You must be signed in to change notification settings - Fork 205
acquire_token_by_refresh_token() for RT migration #193
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
Conversation
this one gets my vote |
msal/application.py
Outdated
def acquire_token_by_refresh_token(self, refresh_token, scopes): | ||
"""Acquire token(s) based on a refresh token (RT) obtained from elsewhere. | ||
|
||
In normal MSAL scenarios, you do NOT need to use this method. |
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.
In normal MSAL scenarios, you do NOT need to use this method. [](start = 8, length = 61)
https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/1cef43d185bb0a1dcc013d2ec232ae9d4c6c053a/src/client/Microsoft.Identity.Client/IByRefreshToken.cs:
/// Acquires an access token from an existing refresh token and stores it, and the refresh token, in
/// the user token cache, where it will be available for further AcquireTokenSilent calls.
/// This method can be used in migration to MSAL from ADAL v2, and in various integration
/// scenarios where you have a RefreshToken available.
/// See https://aka.ms/msal-net-migration-adal2-msal2.
You can also get inspiration on the text from the Java API. Should be similar.
Please add aka.ms links to and create a wiki page allowing you to explain this in more details as well as updating content and linking to it easily when customers have questions.
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.
We probably do not need to create yet another wiki page. After this PR is released, we can modify the existing doc accordingly.
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.
After this PR is released, we can modify the existing doc accordingly.
UPDATE:
A new doc PR for the migration docs has been created just now.
db912cd
to
3917e41
Compare
Quoted a test feedback from @qianwens:
|
3917e41
to
fb5beea
Compare
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.
🚢
fb5beea
to
e3fa226
Compare
Remove distracting test setup from sample Demonstrate the migration-in-batch pattern
This PR implements the right-hand side of this comparison. We can go with this direction, and then this PR will close #191.
The API calling pattern is largely the same as #191, if we do not really need to use its return value:
There are still two higher level RT migration strategies:
One approach is to migrate RTs IN A BATCH within minutes, when a newer version of app (Azure CLI) is run for its first time on an end user's machine and detects a legacy RT persistence.
With this strategy, you may safely discard the return value of each
acquire_token_by_refresh_token()
call, DURING the migration.The comparison:
acquire_token_by_refresh_token()
≈ MSAL .Net'sit is not readily accessible with the IConfidentialClientApplication without first casting it to IByRefreshToken.
+acquire_token_by_refresh_token()
Another approach is to migrate RTs GRADUALLY across many days or even weeks, when the end user is using our app (such as Azure CLI) frequently. But then you would have to use
acquire_token_by_refresh_token()
in your normal code path. The following demo is copied from here.