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

Fix: Ensure clear_auth_cookie uses the correct secure and samesite settings #1959

Merged
merged 3 commits into from
Mar 7, 2025

Conversation

ilkertuna
Copy link
Contributor

This pull request fixes an issue where clear_auth_cookie did not explicitly specify the secure and samesite attributes when deleting authentication cookies.

Issue:

  • The set_auth_cookie function correctly sets cookies using the _cookie_secure and _cookie_samesite values.
  • These values are derived from the environment variable CHAINLIT_COOKIE_SAMESITE.
    • If CHAINLIT_COOKIE_SAMESITE is "none", then _cookie_secure is set to True, ensuring the cookie is only sent over HTTPS.
    • Otherwise, _cookie_secure is False.
  • However, clear_auth_cookie did not apply these settings when deleting cookies.
  • If a user had customized CHAINLIT_COOKIE_SAMESITE to a value other than the default ("lax"), the logout process could fail because the browser might not clear the cookie correctly.

Fix:

  • clear_auth_cookie now ensures that cookies are deleted using the same secure and samesite settings as set_auth_cookie.
  • This ensures that logout behaves correctly regardless of custom cookie settings.

This change improves consistency and ensures proper authentication cleanup across different deployment environments.

…amesite settings

Ensure clear_auth_cookie uses correct secure and samesite settings

clear_auth_cookie was not explicitly specifying the secure and samesite 
attributes when deleting cookies. This could cause logout issues if 
CHAINLIT_COOKIE_SAMESITE was set to a custom value. 

Now, clear_auth_cookie applies the same secure and samesite settings 
as set_auth_cookie to ensure consistent behavior.
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. auth Pertaining to authentication. security labels Mar 5, 2025
@ilkertuna ilkertuna changed the title Ensure clear_auth_cookie uses the correct secure and samesite settings Fix: Ensure clear_auth_cookie uses the correct secure and samesite settings Mar 5, 2025
Copy link
Collaborator

@willydouhard willydouhard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@willydouhard
Copy link
Collaborator

Please run ruff check --fix and ruff format in the backend dir. CI is red.

fix format
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Mar 7, 2025
@willydouhard willydouhard added this pull request to the merge queue Mar 7, 2025
Merged via the queue into Chainlit:main with commit 033517e Mar 7, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth Pertaining to authentication. security size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants