Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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

@idan
Copy link

idan commented Aug 28, 2024

Also linking to https://github.com/supabase/auth-helpers/issues/441 for posterity

@hf hf force-pushed the hf/fix-max-age branch from f0c1750 to 4523bc1 Compare August 28, 2024 16:52
@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 force-pushed the hf/fix-max-age branch from 4523bc1 to 92ecd82 Compare August 28, 2024 16:53
@hf hf force-pushed the hf/fix-max-age branch from 92ecd82 to 4f8fb0b Compare August 28, 2024 16:56
@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>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants