From 3b662deec841d839a37aa9382410cdec4a9772f6 Mon Sep 17 00:00:00 2001 From: Christian Banse Date: Mon, 7 Mar 2022 14:06:22 +0100 Subject: [PATCH] Workaround for Safari cookie issue (#35) --- login/login.go | 28 ++++++++++++++++++++++------ login/login_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/login/login.go b/login/login.go index 8857f69..be2d0dc 100644 --- a/login/login.go +++ b/login/login.go @@ -17,6 +17,7 @@ import ( "net/http" "net/url" "path" + "strings" "sync" "time" @@ -52,14 +53,14 @@ func (s *session) Anonymous() bool { // Cookie returns a new http.Cookie issued for path, that can contains the session ID // and sensible cookie attributes, such as Secure and HttpOnly. -func (s *session) Cookie(path string) *http.Cookie { +func (s *session) Cookie(path string, r *http.Request) *http.Cookie { return &http.Cookie{ Name: "id", Value: s.ID, Path: path, Expires: s.ExpireAt, SameSite: http.SameSiteLaxMode, - Secure: true, + Secure: secureCookie(r), HttpOnly: true, } } @@ -153,12 +154,12 @@ func (h *handler) newSession() *session { return &session } -func (h *handler) updateSession(w http.ResponseWriter, session *session, user *User) { +func (h *handler) updateSession(w http.ResponseWriter, r *http.Request, session *session, user *User) { h.sm.Lock() session.User = user h.sm.Unlock() - http.SetCookie(w, session.Cookie(h.baseURL)) + http.SetCookie(w, session.Cookie(h.baseURL, r)) h.log.Printf("Associating session with id %s to user %s", session.ID, user.Name) } @@ -241,7 +242,7 @@ func (h *handler) doLoginPost(w http.ResponseWriter, r *http.Request) { } // Associate the user with the session and - h.updateSession(w, session, user) + h.updateSession(w, r, session, user) // Everything good, lets redirect to the return URL. http.Redirect(w, r, returnURL, http.StatusFound) @@ -331,7 +332,22 @@ func (h *handler) extractSession(w http.ResponseWriter, r *http.Request) (sessio } // Make sure, to send the cookie with the session ID back to the client - http.SetCookie(w, session.Cookie(h.baseURL)) + http.SetCookie(w, session.Cookie(h.baseURL, r)) return session } + +// secureCookie returns true or false, whether the cookie should set the secure flag. +// This is necessary because in some browser (looking at you, Safari), an HTTP connection to +// "localhost" is not regarded as "secure" anymore. So for local deployments, we need to drop +// the secure flag, otherwise our login won't work. +func secureCookie(r *http.Request) bool { + // This is a very basic heuristic to allow cookies for Safari on HTTP connections to localhost + if strings.Contains(r.UserAgent(), "Safari") && + (r.Host == "localhost" || strings.Index(r.Host, "localhost:") == 0) && + r.TLS == nil { + return false + } + + return true +} diff --git a/login/login_test.go b/login/login_test.go index 77fc35f..cb3068d 100644 --- a/login/login_test.go +++ b/login/login_test.go @@ -518,3 +518,47 @@ func Test_handler_parseReturnURL(t *testing.T) { }) } } + +func Test_secureCookie(t *testing.T) { + type args struct { + r *http.Request + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "Safari on HTTP localhost", + args: args{ + r: func() *http.Request { + r, _ := http.NewRequest("GET", "/login", nil) + r.Header.Add("User-Agent", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.3 Safari/605.1.15") + r.Host = "localhost:1234" + + return r + }(), + }, + want: false, + }, + { + name: "Some other domain", + args: args{ + r: func() *http.Request { + r, _ := http.NewRequest("GET", "/login", nil) + r.Host = "example.com" + + return r + }(), + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := secureCookie(tt.args.r); got != tt.want { + t.Errorf("secureCookie() = %v, want %v", got, tt.want) + } + }) + } +}