Skip to content
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

[Feature Request] msal.TokenCache is not picklable. #648

Open
xiangyan99 opened this issue Jan 13, 2024 · 6 comments
Open

[Feature Request] msal.TokenCache is not picklable. #648

xiangyan99 opened this issue Jan 13, 2024 · 6 comments

Comments

@xiangyan99
Copy link

MSAL client type

Public

Problem Statement

This is from a customer request: Azure/azure-sdk-for-python#21996

If possible, we should have everything pickable.

Proposed solution

No response

@rayluo
Copy link
Collaborator

rayluo commented Jan 13, 2024

Thanks for the suggestion. We will give it some thoughts and see what we can do.

UPDATE: More info was described in the internal work item.

@bgavrilMS
Copy link
Member

The customer ask seems to be for confidential client, not for public client?

@rayluo
Copy link
Collaborator

rayluo commented Feb 29, 2024

Pickling is a Python terminology and implementation of object serialization.

The customer ask seems to be for confidential client, not for public client?

That original customer happened to be using a confidential client, but the temptation to pickle may also present for a public client app.

[Feature Request] ... picklable

I am still self-debating whether this feature request is desirable.

  1. The security implication from identity perspective, when pickling a confidential client which contains its credential

  2. The security implication from Python perspective. Pickling mechanism itself is considered less secure in Python, as documented in the red warnings in this official doc.

  3. Conceptually, any complicated-under-the-hood object is not necessarily picklable. For example, the file handlers and socket connections in one process can't be pickled to another process. Kudos to our Azure SDK friends, they already simplified the original feature request of "pickling the entire credential class" to only "pickle the token cache class" here for MSAL.

  4. [Feature Request] msal.TokenCache is not picklable

    Yet, token cache class has its own subtlety. Per specs, each refresh token (RT) in a cache shall be considered invalid after a new RT has been issued to replace it. But, if the original RT was picked into multiple copies everywhere, there is no way to update those invalidated RTs.

    Perhaps, the right way to do this is to implement a subclass class TokenCacheInDatabase(TokenCache) and let it handle the data sharing among different processes.

@xiangyan99
Copy link
Author

Yes. There are some objects that are not picklable. It is by design. e.g. our transport.

If only partial of the object that we don't want to pickle, we can remove them when pickling and re-create them when unpickling. (This is what we have today for transport in the pipeline).

Please evaluate the object, if there is nothing other than unsafe-to-pickle information stored in the object, we are ok if is not picklable.

@rayluo
Copy link
Collaborator

rayluo commented Feb 29, 2024

Please evaluate the object, if there is nothing other than unsafe-to-pickle information stored in the object, we are ok if is not picklable.

In that case, we are not sure whether the TokenCache object shall be picklable, based on the #4 bullet point in my previous message.

Since Azure Identity already made some change on choosing what objects to be picklable, what if Azure Identity's Credentials simply choose to recreate a new empty token cache when being unpickled? @xiangyan99

@rayluo
Copy link
Collaborator

rayluo commented Jul 8, 2024

MSAL Python has long been supporting "serializing token cache to json", which is more interchangeable across different languages (thus becomes the battle-tested foundation of token cache sharing across MSALs in .Net/Java/Python); also, I believe json is more performant than pickle.

I suppose you can either:

  • Somehow have your implementation to save and load the MSAL token cache based on its existing serialization API.
  • If you prefer to have pickleable support, we accept contribution in PR. 🙂

In either case, the caveat in my earlier comment above still stands. It is OK to save a token cache and reload it later, i.e. sequentially. But it is not meant to be shared among multiple processes concurrently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants