-
-
Notifications
You must be signed in to change notification settings - Fork 617
Invalidate split_cookies client-side #930
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
Comments
BacLuc
added a commit
to BacLuc/ecamp3
that referenced
this issue
Oct 2, 2021
SplitCookieExtractor does not check, if both JWT_HP and JWT_S cookies are present. The following logic fails if not. lexik/LexikJWTAuthenticationBundle#930 Issue: ecamp#1999
BacLuc
added a commit
to BacLuc/ecamp3
that referenced
this issue
Oct 2, 2021
SplitCookieExtractor does not check, if both JWT_HP and JWT_S cookies are present. The following logic fails if not. lexik/LexikJWTAuthenticationBundle#930 Issue: ecamp#1999
BacLuc
added a commit
to BacLuc/ecamp3
that referenced
this issue
Oct 3, 2021
The provided SplitCookieExtractor does not check, if all needed cookies are set. Thus we replace it. lexik/LexikJWTAuthenticationBundle#930 Issue: ecamp#1999
carlobeltrame
pushed a commit
to BacLuc/ecamp3
that referenced
this issue
Oct 4, 2021
The provided SplitCookieExtractor does not check, if all needed cookies are set. Thus we replace it. lexik/LexikJWTAuthenticationBundle#930 Issue: ecamp#1999
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Like in #809, we use the split_cookie extractor to store the JWT token in two separate cookies, one of which is available in the JS frontend and one of which is not.
Also I read in #847 (comment) that on logout, JWT tokens should just be deleted client-side.
Now our frontend JS code can only delete the
jwt_hp
cookie, but not thejwt_s
cookie, because that one is markedhttponly
. I see a few options how we could solve this:jwt_hp=eyABCD.ey12341234
and a missingjwt_s
cookie will result in the tokeneyABCD.ey12341234.
, which can never be a valid JWT token because of the period in the end. For this reason, this wouldn't be a breaking change either, because no developers could have successfully used partial split cookies so far.I'd be happy to prepare a pull request to fix this if you're open to this change.
I have found no easy way to extend / override the existing SplitCookieExtractor, otherwise that would be an option as well.
The text was updated successfully, but these errors were encountered: