search

dudil / fastapi_msal

A FastAPI Plug-In to support authentication authorization using the Microsoft Authentication Library (MSAL)
MIT License
40 stars 20 forks source link

Cache doesn't survive server restart #40

Open teotwaki opened 1 month ago

teotwaki commented 1 month ago

Hi,

First of all, thanks for a great library. You've saved me many grey hairs, I'm sure.

Describe the bug

As far as I can tell, the library currently has an in-memory cache, which cannot be overridden. It appears that some work was done to prepare for using multiple types of caches, but this is not available as of yet.

I'm trying to run this using Mangum on AWS Lambda, and obviously having a single in-memory cache doesn't work. Every time the lambda gets garbage collected (roughly every 15 minutes), or anytime a user gets sent to a different lambda, the cache hit fails, and they are sent through the SSO redirects again. This is reproducible locally by restarting the server in between requests.

To Reproduce Steps to reproduce the behavior:

  1. Start a server that SSO authenticates a user
  2. Perform multiple actions, each action is instantly executed (AuthToken is found and returned). sid remains constant.
  3. Restart the server, ensure session key is maintained
  4. Perform an action
  5. The user is SSO authenticated again, new sid generated.

Expected behavior The sid should be recognised so that we don't need to re-authenticate the user.

Additional context If you can provide me with some guidance, I'm happy to implement and provide a PR.

teotwaki commented 1 month ago

Digging a bit deeper into this, I noticed something interesting. I put a breakpoint on MSALAuthCodeHandler::_save_cache, and this does get hit when Entra redirects the browser to the /token URL. I can see that the request.session dict gets updated and contains the token_cache key and what looks like a correct value.

However, inspecting the server response from FastAPI for /token shows that the cookie named session contains something that looks like a JWT, however upon decoding it, it only contains the sid value (set by SessionManager::init_session). The token_cache is not present anywhere.

I don't know if it's relevant, but I figured I would report it anyway.

dudil commented 1 month ago

Hi @teotwaki ,

Thank you for your interest and support at this work. Highly appreciated 🙏

Support for external cache was something I've initially planned to implement but I didn't.

I'll be happy to get back to it and add it, please allow me some time for investigation (or better remember what my plans were at time) and I'll share it with you to see if this would fit your needs.

teotwaki commented 1 week ago

Hi @dudil, have you had any further thoughts on this?

I really would love to get your thoughts on how to implement this, and I can either contribute code (if you give me some guidance) or happy to discuss sponsorship if that fits your agenda better.

Thanks in advance,