Skip to content

Commit

Permalink
Workaround for Safari cookie issue (#35)
Browse files Browse the repository at this point in the history
  • Loading branch information
oxisto authored Mar 7, 2022
1 parent 8e0ac3f commit 3b662de
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 6 deletions.
28 changes: 22 additions & 6 deletions login/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"net/http"
"net/url"
"path"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
44 changes: 44 additions & 0 deletions login/login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}

0 comments on commit 3b662de

Please sign in to comment.