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.

Sign up for GitHub

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

Jump to bottom

Enforce SameSite=Strict #1713

Merged
merged 1 commit into from Jun 30, 2023
Merged

Enforce SameSite=Strict #1713

merged 1 commit into from Jun 30, 2023

Conversation

diamondburned
Copy link
Contributor

This adds additional guard into the JWT cookie so that it does not leak on cross-origin requests.

Related issue: LemmyNet/lemmy#3301

@diamondburned
Enforce SameSite=Strict
a5e00a6 
This adds additional guard into the JWT cookie so that it does not leak
on cross-origin requests.
@diamondburned
Copy link
Contributor Author

It's worth mentioning that this PR completely rips out isomorphic-cookie and replaces it with code that directly uses document.cookie. This keeps the same exact behavior as before, since the library never used the cookie in the backend anyway. All cookie-setting was done in the frontend!

SleeplessOne1917
SleeplessOne1917 approved these changes Jun 30, 2023
View reviewed changes
Copy link
Member

@SleeplessOne1917 SleeplessOne1917 left a comment

Choose a reason for hiding this comment

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

@alectrocute @dessalines @jsit I wonder if this will also solve the user cache busting bug?

@alectrocute
Copy link
Contributor

@SleeplessOne1917 Good point.

Can we use this to fix the inoperable user.auth() conditional on middleware.ts as well?

@SleeplessOne1917
Copy link
Member

SleeplessOne1917 commented Jun 30, 2023 •
edited

Can we use this to fix the inoperable user.auth() conditional on middleware.ts as well?

I think so. I'll do it in a separate PR when I get the chance and this PR is merged.

@alectrocute alectrocute mentioned this pull request Jun 30, 2023
Merged
alectrocute
alectrocute approved these changes Jun 30, 2023
View reviewed changes
Copy link
Contributor

@alectrocute alectrocute left a comment

Choose a reason for hiding this comment

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

Nice.

@alectrocute alectrocute merged commit a7592d7 into LemmyNet:main Jun 30, 2023
1 check passed
@alectrocute alectrocute mentioned this pull request Jun 30, 2023
Merged
@auouymous auouymous mentioned this pull request Jul 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SleeplessOne1917 SleeplessOne1917 SleeplessOne1917 approved these changes

@alectrocute alectrocute alectrocute approved these changes

@dessalines dessalines Awaiting requested review from dessalines dessalines is a code owner

@jsit jsit Awaiting requested review from jsit

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@diamondburned @alectrocute @SleeplessOne1917