authlib
authlib copied to clipboard
Session cookie grows indefinitely, results in CSRF Warning.
Describe the bug
When session cookie exceeds 4096 bytes, user is getting CSRF Warning. set_state_data assumes cookie can grow indefinitely
Error
CSRF Warning! State not equal in request and response.'
To Reproduce
Use SessionMiddleware from starlette, ie. fastapi demo. Go to login page several times but never login. Eventually set-cookie will try to write more than 4096 bytes, browsers won't save the cookie, resulting in CSRF error because latest state is not in cookie.
Expected behavior
A cookie should never exceed 4096 bytes, so users can login without CSRF errors.
Additional context
This issue is potentially caused by commit Refactor whole client integrations. · lepture/authlib@179dc8a which were aiming to solve issue Allowing multiple authentication flows in parallel (e.g., in different tabs) · Issue #285 · lepture/authlib
Also see: Browser Cookie Limits: If you want to support most browsers, then don't exceed 50 cookies per domain, and don't exceed 4093** bytes per domain (i.e. total size of all cookies <= 4093 bytes).
Also reporting the same issue. What I did to solve this in our integration (see https://github.com/gradio-app/gradio/pull/8000) is to catch the MismatchingStateError when it happens and delete all legacy states from the session cookie.
try:
oauth_info = await oauth.huggingface.authorize_access_token(request)
except MismatchingStateError:
# Delete all keys that are related to the OAuth state
for key in list(request.session.keys()):
if key.startswith("_state_huggingface"):
request.session.pop(key)
# Redirect back to login
return RedirectResponse(login_uri)
Thanks @frozturk when founding out the root cause and hope this snippet will help future users until this bug is fixed.
(Regarding how to fix things in authlib itself, I don't know what should be the best approach. A naive solution is to clear previous states when adding a new one but it might cause (minor) discrepancies if a user is working in several tabs. @lepture Happy to open a PR though if this solution looks fine for you.)
@Wauplin A PR is welcome. Thank you.
@lepture I just opened https://github.com/lepture/authlib/pull/644. Please let me know what you think.