force_refresh not being passed to _acquire_token_silent_from_cache ....

[arthur00]

acquire_token_silent is receiving force_refresh but it's not passing to the next function: _acquire_token_silent_from_cache_and_possibly_refresh_it. This update just adds that argument when calling the latter.

[rayluo]

Nice. Just a minor comment inside.

Out of curiosity, are you just testing, or do you really have a need in your production system that you would like to do force_refresh? That would cause your app not benefiting from MSAL's built-in cache as much. What is the reason that you would want to force_refresh?

[arthur00]

Hi @rayluo, sure, I can adjust it - should I create another branch to make it cleaner (single commit) or can I commit a new change to this one? I don't quite remember if github squashes multiple commits in a merge request branch..

To answer your question, I caught it while I was testing my application - so I wanted to make sure the refresh worked. However I do have reason to want to manage token expiry on my own code. I use access tokens to access a SQL server impersonating the logged-in user. So I store a ODBC cursor that has been authenticated using the authentication token with this technique. If I try to use the cursor without a valid access token, I will likely get a generic error from the ODBC library.

So instead, I could track the expiry time of the access token for every database query. Since there would be many calls, ideally this should be quick - like if access_token_time - time.time() > 3600: refresh. In this case, just to be sure, I could use the force_refresh since I know I want it to be refreshed. But if I didn't it, MSAL should also see the expired access token and refresh it. I'm just not sure whether calling MSAL "acquire_token_silently" on every ODBC query is efficient. If it is, then I can just do that and rely on the library's cache and auto-refresh capability.

[rayluo]

Thanks for updating this PR. We the repo maintainers would have the choice to do a merge commit or a squash commit.

Also THANKS for sharing your real world usage case! We had that case briefly documented in ADAL Python's wiki (which referring to the same source that you found). Please let us know if you would find anything special for MSAL Python.

Regarding your usage pattern:

• It is not strictly necessarily for you to manage the access token's life time. MSAL Python's acquire_token_silent() is designed to encapsulate EVERY cache lookup logic (including cache refresh timing) inside, therefore it can be called very frequently, otherwise it would defeat its purpose.
• Of course you could still choose to have a very straightforward time-based if ... else ... check in your application layer. That way you can indeed save many MSAL Python's acquire_token_silent() call, thus save some overhead. (Nothing beats an if ... else ... in terms of performance.) This can also be very easy if your usage pattern happens to have a long-lived instance with that access token. But if you ever reach to a point that you want to temporarily terminate that instance and wonder how you would retain that access token, you then realize you can simply call MSAL's acquire_token_silent() next time.
• Either way, you still do NOT have to set force_refresh=True, because MSAL would automatically triggered a refresh if the access token has less than 5 minutes life left.

I'm going to merge in your fix. Feel free to ask us more questions when needed.

[rayluo]

LGTM