Add SameSite #8

zeripath · 2020-11-11T20:51:24Z

zeripath commented

2020-11-11 20:51:24 +00:00

Signed-off-by: Andrew Thornton art27@cantab.net

Signed-off-by: Andrew Thornton <art27@cantab.net>

zeripath added 1 commit 2020-11-11 20:51:24 +00:00

Add SameSite

Signed-off-by: Andrew Thornton <art27@cantab.net>

continuous-integration/drone/pr Build is failing

Details

95fc279655

6543 approved these changes 2020-11-11 20:53:16 +00:00

Dismissed

silverwind reviewed 2020-11-11 20:56:29 +00:00

Dismissed

csrf.go Outdated

						
				@ -228,3 +233,3 @@

							x.Token = GenerateToken(x.Secret, x.ID, "POST")

							if opt.SetCookie {

								ctx.SetCookie(opt.Cookie, x.Token, 0, opt.CookiePath, opt.CookieDomain, opt.Secure, opt.CookieHttpOnly, time.Now().AddDate(0, 0, 1))

								ctx.SetCookie(opt.Cookie, x.Token, 0, opt.CookiePath, opt.CookieDomain, opt.Secure, opt.CookieHttpOnly, time.Now().AddDate(0, 0, 1), cookie.SameSite(opt.SameSite))

silverwind commented

2020-11-11 20:56:29 +00:00

First-time contributor

Unrelated, but can you maybe add an option for Expires (hardcoded to 24h here)?

Unrelated, but can you maybe add an option for `Expires` (hardcoded to 24h here)?

zeripath commented

2020-11-11 21:08:19 +00:00

I've added the option to set the cookie lifetime - we can set it to 24hours in Gitea.

silverwind commented

2020-11-11 21:12:52 +00:00

First-time contributor

Thanks. I think we should actually match it to the session cookies lifetime.

silverwind commented

2020-11-11 21:19:02 +00:00

First-time contributor

Actually I think you misunderstood. I was talking about the http.Cookie.Expires (8th argument to SetCookie). What you exposed is http.Cookie.MaxAge (don't think we'll need it).

Only one of the two should be set. If both are present, MaxAge has precedence.

https://golang.org/src/net/http/cookie.go
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie

Actually I think you misunderstood. I was talking about the `http.Cookie.Expires` (8th argument to SetCookie). What you exposed is `http.Cookie.MaxAge` (don't think we'll need it). Only one of the two should be set. If both are present, MaxAge has precedence. https://golang.org/src/net/http/cookie.go https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie

zeripath commented

2020-11-11 21:25:08 +00:00

Oh yeah I see.

Which would you prefer MaxAge or Expires

Oh yeah I see. Which would you prefer MaxAge or Expires

zeripath marked this conversation as resolved

zeripath added 1 commit 2020-11-11 21:07:31 +00:00

Add CookieLifetime

Signed-off-by: Andrew Thornton <art27@cantab.net>

continuous-integration/drone/pr Build is failing

Details

fb41625f66

6543 approved these changes 2020-11-11 21:09:25 +00:00

Dismissed

6543 left a comment

First-time contributor

👍

:+1:

zeripath added 1 commit 2020-11-11 21:34:28 +00:00

as per @silverwind

Signed-off-by: Andrew Thornton <art27@cantab.net>

continuous-integration/drone/pr Build is failing

Details

22ff3bf58d

silverwind approved these changes 2020-11-11 21:54:36 +00:00

Dismissed

6543 commented

2020-11-11 22:42:27 +00:00

First-time contributor

CI fail :/

zeripath reviewed 2020-11-11 23:46:35 +00:00

Dismissed

csrf.go Outdated

						
				@ -44,2 +45,4 @@

					// Return the flag value used for the csrf token.

					GetCookieHttpOnly() bool

					// Return the SameSite setting for the csrf token.

					GetSameSite() http.SameSite