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

Adding SameSite to Antikythera.Http.SetCookie #186

Open
sylph01 opened this issue Dec 7, 2021 · 2 comments
Open

Adding SameSite to Antikythera.Http.SetCookie #186

sylph01 opened this issue Dec 7, 2021 · 2 comments

Comments

@sylph01
Copy link
Contributor

sylph01 commented Dec 7, 2021

Rationale

Antikythera.Http.SetCookie lacks the option to set the SameSite attribute of Set-Cookie header, and now it is forced into using SameSite=Lax.
As my team and I came across a need to set the SameSite directive to SameSite=none (especially in combination with Antikythera.Session), I am raising this issue and proposing the addition of this functionality. This would also help gears that want to enforce SameSite=strict.

Proposed changes

  • Add same_site field to Antikythera.Http.SetCookie
    • This will be an enum that takes either :lax, :strict, or :none
    • Adding a field under the SetCookie module's field list and adding a type would do this
    • I am ready to write up a patch for this change
  • Add an interface to Antikythera.Plug.Session.load/2
    • When explicitly adding a Cookie entry, passing an optional argument to Antikythera.Conn.put_resp_cookie/4 can achieve this
    • But when used in combination with Antikythera.Plug.Session.load/2 it is not trivial, so I would like advice on how to change this
      • As of right now, I am thinking of passing options under :set_cookie key, then passing this option to make_before_send/2 (this would add an argument and thus change the signature to make_before_send/3 ) so that it can be passed onto Antikythera.Conn.put_resp_cookie/4 (now called with only 3 arguments).

Relevant references

@aYukiSekiguchi
Copy link
Contributor

Thank you for filing a bug!

Since cowlib which Antikythera uses, has :same_site, Antikythera.Http.SetCookie should have it as well. However, cowlib 2.9 or earlier only supports :lax and :strict, so we have to update cowlib to 2.10. It means we have to update cowboy to 2.9.

Could you wait for the cowboy update?

@sylph01
Copy link
Contributor Author

sylph01 commented Dec 8, 2021

I have checked with our team that we can figure out a workaround to our project's specific problem, so we can wait for the cowboy update.

Meanwhile, we found out that we need to specify the session's expiration explicitly, so I sent a patch that does this and also addresses the second part of this issue (Add an interface to Antikythera.Plug.Session.load/2). The first part will be addressed after the cowboy update, because it is dependent on cowboy supporting the :none value for same_site key.

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

No branches or pull requests

2 participants