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: set max-age default cookie option to 400 days #54

Merged
merged 1 commit into from
Aug 28, 2024
Merged

Conversation

hf
Copy link
Collaborator

@hf hf commented Aug 28, 2024

Some browsers didn't like the large number used by the Max-Age default cookie option, causing weird behavior. It's now set to 400 days.

@j4w8n
Copy link
Contributor

j4w8n commented Aug 28, 2024

Thanks for this, but if you look at the details of #37, this involves more than browser implementations - namely Hono throwing an error when max age is over 400 days.

I don't think it's necessarily Supabase's responsibility to work around other libraries' code, and possibly poor choices, but adhering to the draft rfc mentioned on the other pr - to set this at 400 days - seems reasonable.

Can someone explain the rationale so that if this stays at 5 years, we at least know why the decision was made to not follow the rf?

Truly appreciate all you do 🙏; just trying to understand.

@idan
Copy link

idan commented Aug 28, 2024

The upcoming RFC for the HTTP cookie spec (6265) explicitly defines a lifetime maximum of 400 days for cookies. I'd set it to the max value as defined by the spec, 34560000 seconds.

But either way, thank you!

Copy link

@idan idan left a comment

Choose a reason for hiding this comment

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

Made a suggested change here to track the new cookie lifetime limits in the HTTP spec

src/utils/constants.ts Outdated Show resolved Hide resolved
src/utils/constants.ts Outdated Show resolved Hide resolved
@idan
Copy link

idan commented Aug 28, 2024

Also linking to supabase/auth-helpers#441 for posterity

@hf hf changed the title fix: set max-age default cookie option to a sensible value fix: set max-age default cookie option to 400 days Aug 28, 2024
@hf hf merged commit f4ed2e0 into main Aug 28, 2024
3 checks passed
@hf hf deleted the hf/fix-max-age branch August 28, 2024 16:58
hf pushed a commit that referenced this pull request Aug 28, 2024
🤖 I have created a release *beep* *boop*
---


## [0.5.1](v0.5.0...v0.5.1)
(2024-08-28)


### Bug Fixes

* remove optional dependencies
([#41](#41))
([a48fe6f](a48fe6f))
* set `max-age` default cookie option to 400 days
([#54](#54))
([f4ed2e0](f4ed2e0))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants