make cookie options more configurable and more secure by default

auth/auth.go

@@ -36,6 +36,7 @@ var auth = &Auth{}
 type Auth struct {
 	sessions *SessionManager
 	oidc     *OIDCBackend
+	config   *Config
 }
 
 func Init(c *Config) (err error) {
@@ -43,8 +44,8 @@ func Init(c *Config) (err error) {
 		// authentication is disabled
 		return
 	}
-
-	if auth.sessions, err = NewSessionManager(c.Sessions); err != nil {
+	auth.config = c
+	if auth.sessions, err = NewSessionManager(); err != nil {
 		return
 	}
 

auth/config.go

@@ -25,12 +25,60 @@
 package auth
 
 import (
+	"errors"
+	"net/http"
 	"os"
+	"strings"
 	"time"
 )
 
+type SameSite http.SameSite
+
+func (s SameSite) String() string {
+	switch http.SameSite(s) {
+	case http.SameSiteLaxMode:
+		return "lax"
+	case http.SameSiteStrictMode:
+		return "strict"
+	case http.SameSiteDefaultMode:
+		return "default"
+	}
+	return "unset"
+}
+
+func (s *SameSite) fromString(str string) error {
+	switch strings.ToLower(os.ExpandEnv(str)) {
+	case "lax":
+		*s = SameSite(http.SameSiteLaxMode)
+	case "strict":
+		*s = SameSite(http.SameSiteStrictMode)
+	case "default":
+		*s = SameSite(http.SameSiteDefaultMode)
+	default:
+		return errors.New("invalid same site policy: '" + str + "'")
+	}
+	return nil
+}
+
+func (s SameSite) MarshalText() (data []byte, err error) {
+	data = []byte(s.String())
+	return
+}
+
+func (s *SameSite) UnmarshalText(data []byte) (err error) {
+	return s.fromString(string(data))
+}
+
+type SessionCookiesConfig struct {
+	Secure   bool     `json:"secure" yaml:"secure" toml:"secure"`
+	Path     string   `json:"path" yaml:"path" toml:"path"`
+	Domain   string   `json:"domain" yaml:"domain" toml:"domain"`
+	SameSite SameSite `json:"same-site" yaml:"same-site" toml:"same-site"`
+}
+
 type SessionsConfig struct {
-	MaxAge time.Duration `json:"max-age" yaml:"max-age" toml:"max-age"`
+	MaxAge  time.Duration        `json:"max-age" yaml:"max-age" toml:"max-age"`
+	Cookies SessionCookiesConfig `json:"cookies" yaml:"cookies" toml:"cookies"`
 }
 
 type OIDCConfig struct {
@@ -46,6 +94,8 @@ type Config struct {
 }
 
 func (c *Config) ExpandEnv() {
+	c.Sessions.Cookies.Path = os.ExpandEnv(c.Sessions.Cookies.Path)
+	c.Sessions.Cookies.Domain = os.ExpandEnv(c.Sessions.Cookies.Domain)
 	if c.OIDC != nil {
 		c.OIDC.IssuerURL = os.ExpandEnv(c.OIDC.IssuerURL)
 		c.OIDC.ClientID = os.ExpandEnv(c.OIDC.ClientID)

auth/oidc.go

@@ -28,6 +28,7 @@ import (
 	"context"
 	"encoding/json"
 	"net/http"
+	"time"
 
 	oidc "github.com/coreos/go-oidc"
 	"golang.org/x/oauth2"
@@ -109,13 +110,12 @@ func (b *OIDCBackend) HandleLogin(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "Failed to generate new OIDC session: "+err.Error(), http.StatusInternalServerError)
 		return
 	}
-	// TODO: this session should expire in ~a minute
-	s = &Session{oidc: os}
+	s = &Session{oidc: os, Expires: time.Now().Add(time.Minute)}
 	if sid, err = auth.sessions.insert(s); err != nil {
 		http.Error(w, "Failed to generate new session: "+err.Error(), http.StatusInternalServerError)
 		return
 	}
-	setSessionCookie(w, sid, s.Expires)
+	setSessionCookie(w, sid)
 	http.Redirect(w, r, b.oauth2Config.AuthCodeURL(s.oidc.State, oidc.Nonce(s.oidc.Nonce)), http.StatusFound)
 }
 
@@ -187,8 +187,11 @@ func (h *oidcCallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
-	// TODO: extend this to MaxAge to make short lived initial sessions possible
-	newS.Expires = s.Expires
+	maxAge := defaultAge
+	if auth.config.Sessions.MaxAge > 0 {
+		maxAge = auth.config.Sessions.MaxAge
+	}
+	newS.Expires = time.Now().Add(maxAge * time.Second)
 	newS.oidc = &OIDCSession{State: s.oidc.State, Nonce: s.oidc.Nonce}
 	newS.oidc.token = oauth2Token
 	if err = auth.sessions.update(sid, newS); err != nil {
@@ -197,6 +200,7 @@ func (h *oidcCallbackHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		http.Error(w, "Updating session failed: "+err.Error(), http.StatusInternalServerError)
 		return
 	}
+	setSessionCookie(w, sid) // reset the cookie to update max-age
 
 	data, _ := json.MarshalIndent(newS, "", "    ")
 	w.Write(data)

auth/sessions.go

@@ -27,6 +27,7 @@ package auth
 import (
 	"context"
 	"errors"
+	"math"
 	"net/http"
 	"sync"
 	"time"
@@ -73,19 +74,32 @@ func getSessionFromCookie(r *http.Request) (string, *Session) {
 	return sid, s
 }
 
-func setSessionCookie(w http.ResponseWriter, sid string, expires time.Time) {
-	// TODO: make cookie settings more secure!
+func newSessionCookie(sid string, maxAge int) *http.Cookie {
 	sc := &http.Cookie{Name: SessionCookieName}
 	sc.Value = sid
-	sc.Expires = expires
-	sc.Path = "/"
-	http.SetCookie(w, sc)
+	sc.HttpOnly = true
+	sc.MaxAge = maxAge
+
+	sc.Secure = auth.config.Sessions.Cookies.Secure
+	sc.Path = auth.config.Sessions.Cookies.Path
+	if sc.Path == "" {
+		sc.Path = "/"
+	}
+	sc.Domain = auth.config.Sessions.Cookies.Domain
+	sc.SameSite = http.SameSite(auth.config.Sessions.Cookies.SameSite)
+	return sc
+}
+
+func setSessionCookie(w http.ResponseWriter, sid string) {
+	maxAge := int(math.Ceil(defaultAge.Seconds()))
+	if auth.config.Sessions.MaxAge > 0 {
+		maxAge = int(math.Ceil(auth.config.Sessions.MaxAge.Seconds()))
+	}
+	http.SetCookie(w, newSessionCookie(sid, maxAge))
 }
 
 func invalidateSessionCookie(w http.ResponseWriter) {
-	// TODO: this needs to improve
-	sc := &http.Cookie{Name: SessionCookieName, Value: "invalid", MaxAge: -1}
-	http.SetCookie(w, sc)
+	http.SetCookie(w, newSessionCookie("invalid", -1))
 }
 
 func attachSessionToRequest(r *http.Request, s *Session) *http.Request {
@@ -100,15 +114,11 @@ func SessionFromRequest(r *http.Request) (*Session, bool) {
 
 type SessionManager struct {
 	mutex    sync.RWMutex
-	maxAge   time.Duration
 	sessions map[string]*Session
 }
 
-func NewSessionManager(cfg SessionsConfig) (sm *SessionManager, err error) {
-	sm = &SessionManager{maxAge: defaultAge}
-	if cfg.MaxAge > 0 {
-		sm.maxAge = cfg.MaxAge
-	}
+func NewSessionManager() (sm *SessionManager, err error) {
+	sm = &SessionManager{}
 	sm.sessions = make(map[string]*Session)
 	go sm.runMaintenance()
 	return
@@ -126,7 +136,6 @@ func (sm *SessionManager) insert(s *Session) (id string, err error) {
 	if id, err = generateRandomString(32); err != nil {
 		return
 	}
-	s.Expires = time.Now().Add(sm.maxAge)
 
 	sm.mutex.Lock()
 	defer sm.mutex.Unlock()

auth/sessions_test.go

@@ -38,21 +38,18 @@ var (
 //
 
 func TestCreateSessionManager(t *testing.T) {
-	cfg := SessionsConfig{}
-	if _, err := NewSessionManager(cfg); err != nil {
+	if _, err := NewSessionManager(); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 }
 
 func TestSessionExpiry(t *testing.T) {
-	cfg := SessionsConfig{}
-	cfg.MaxAge = time.Second
-	sm, err := NewSessionManager(cfg)
+	sm, err := NewSessionManager()
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 
-	s1 := &Session{Username: "test"}
+	s1 := &Session{Username: "test", Expires: time.Now().Add(time.Second)}
 	id, err := sm.insert(s1)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)

contrib/sample-cfg.yaml

@@ -33,6 +33,12 @@ importer:
 #   sessions:
 #     ## defaults to 24h
 #     max-age: 12h
+#     # cookies:
+#     #   secure: true
+#     #   path: "/tank/"
+#     #   domain: "aura.example.com"
+#     #   ## allowed values: strict, lax or default
+#     #   same-site: strict
 #   oidc:
 #     issuer-url: http://localhost:8000/openid
 #     client-id: ${OIDC_CLIENT_ID}