Commit 63ec505eda
Verified · cmc
Layout: unified · split
internal/httpd/accounts.go +7 −2
| @@ -18,6 +18,11 @@ import ( | |||
| 18 | 18 | ||
| 19 | const sessionCookie = "gitbay_session" | 19 | const sessionCookie = "gitbay_session" |
| 20 | 20 | ||
| 21 | // sessionSameSite is Lax so a login link followed from a mail client keeps | ||
| 22 | // its session through the redirect. Cross-site POSTs are refused by | ||
| 23 | // checkOrigin and carry no Lax cookie anyway. | ||
| 24 | const sessionSameSite = http.SameSiteLaxMode | ||
| 25 | |||
| 21 | // viewer returns the logged-in user, or a zero User for anonymous visitors. | 26 | // viewer returns the logged-in user, or a zero User for anonymous visitors. |
| 22 | // Only meaningful in accounts mode; in view_only no session route exists so | 27 | // Only meaningful in accounts mode; in view_only no session route exists so |
| 23 | // every request is anonymous. | 28 | // every request is anonymous. |
| @@ -92,7 +97,7 @@ func (s *Server) login(w http.ResponseWriter, r *http.Request) { | |||
| 92 | } | 97 | } |
| 93 | http.SetCookie(w, &http.Cookie{ | 98 | http.SetCookie(w, &http.Cookie{ |
| 94 | Name: sessionCookie, Value: sessTok, Path: "/", | 99 | Name: sessionCookie, Value: sessTok, Path: "/", |
| 95 | HttpOnly: true, SameSite: http.SameSiteStrictMode, | 100 | HttpOnly: true, SameSite: sessionSameSite, |
| 96 | Secure: s.cfg.HTTP.TLS != "off", | 101 | Secure: s.cfg.HTTP.TLS != "off", |
| 97 | MaxAge: 7 * 24 * 3600, | 102 | MaxAge: 7 * 24 * 3600, |
| 98 | }) | 103 | }) |
| @@ -103,7 +108,7 @@ func (s *Server) logout(w http.ResponseWriter, r *http.Request) { | |||
| 103 | if ck, err := r.Cookie(sessionCookie); err == nil { | 108 | if ck, err := r.Cookie(sessionCookie); err == nil { |
| 104 | s.st.DeleteWebSession(store.HashToken(ck.Value)) | 109 | s.st.DeleteWebSession(store.HashToken(ck.Value)) |
| 105 | } | 110 | } |
| 106 | http.SetCookie(w, s.clearCookie(sessionCookie, http.SameSiteStrictMode)) | 111 | http.SetCookie(w, s.clearCookie(sessionCookie, sessionSameSite)) |
| 107 | http.Redirect(w, r, "/", http.StatusSeeOther) | 112 | http.Redirect(w, r, "/", http.StatusSeeOther) |
| 108 | } | 113 | } |
| 109 | 114 | ||
internal/httpd/logincookie_test.go added +24
| @@ -0,0 +1,24 @@ | |||
| 1 | package httpd | ||
| 2 | |||
| 3 | import ( | ||
| 4 | "net/http" | ||
| 5 | "testing" | ||
| 6 | |||
| 7 | "gitbay.org/gitbay/internal/config" | ||
| 8 | ) | ||
| 9 | |||
| 10 | // The session cookie must be Lax, not Strict. A login link clicked in a mail | ||
| 11 | // client is a cross-site top-level navigation, and Strict can withhold the | ||
| 12 | // cookie through the redirect that follows, so the visitor lands logged out | ||
| 13 | // (#155). Cross-site POSTs stay protected: Lax withholds the cookie from them, | ||
| 14 | // and checkOrigin refuses them besides. | ||
| 15 | func TestSessionCookieIsLax(t *testing.T) { | ||
| 16 | s := &Server{cfg: config.Config{}} | ||
| 17 | s.cfg.HTTP.TLS = "acme" | ||
| 18 | if got := s.clearCookie(sessionCookie, sessionSameSite); got.SameSite != http.SameSiteLaxMode { | ||
| 19 | t.Errorf("clearing cookie SameSite = %v, want Lax", got.SameSite) | ||
| 20 | } | ||
| 21 | if sessionSameSite != http.SameSiteLaxMode { | ||
| 22 | t.Errorf("sessionSameSite = %v, want Lax", sessionSameSite) | ||
| 23 | } | ||
| 24 | } | ||