All checks were successful
check / check (push) Successful in 5s
closes #47 Fixes the two remaining `gosec` findings on `main`, both `G124` (http.Cookie missing or has insecure `Secure`, `HttpOnly`, or `SameSite` attribute): - `internal/session/session.go:84` (`CreateSession`, the login set-cookie path) - `internal/session/session.go:128` (`ClearSession`, the logout delete-cookie path) ## What changed - Both cookie-writing paths now unconditionally set `Secure: true`, `HttpOnly: true`, and `SameSite: http.SameSiteStrictMode`. - The `secure` field (previously wired to `!config.Debug`) and the `sameSite` field are removed from `session.Manager`, and the dead secure-toggle parameter is removed from `session.NewManager`, which now takes only the signing key (reviewer-directed; the mechanical call-shape updates in `session_test.go` leave every assertion untouched). - TDD per repo rules: the first commit adds `TestSessionCookieAttributesAlwaysSecure` (failing), asserting that every cookie emitted by the session manager carries `HttpOnly`, `Secure`, and `SameSite` of Lax or stricter, for both write paths. The second commit makes it pass. - `TODO.md` updated per its Workflow section (Next Step completed, next Future Step promoted, stale "10 open findings" Status text corrected). ## Attribute choices and reasoning - `Secure: true` always: the `G124` analyzer only accepts a constant `true` store, and there is no legitimate configuration in which the authentication cookie should be sent over plaintext HTTP. The old behavior disabled `Secure` whenever `debug` was on. Local development over `http://localhost` keeps working: browsers treat `localhost` as a trustworthy origin and accept `Secure` cookies there. Any plain-HTTP flow on a non-localhost host will no longer keep a session, which is the point of the fix. - `SameSite: Strict` (unchanged from current production behavior, and stricter than the Lax minimum): the login form is a same-origin POST to `/` followed by a same-site redirect, so `Strict` breaks nothing. - `HttpOnly: true` (unchanged). ## Verification `make check` (tests, golangci-lint, fmt-check) is fully green on the branch head `cb9e14e`: all tests pass and the linter reports 0 issues, independently confirmed by the reviewer in a fresh worktree. Commit history: `ca15f52` (failing test) → `02ca16a` (fix + TODO.md, closes #47) → `cb9e14e` (drop the dead `NewManager` parameter). Co-authored-by: sneak <sneak@sneak.berlin> Reviewed-on: #48 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>
146 lines
3.5 KiB
Go
146 lines
3.5 KiB
Go
// Package session provides encrypted session cookie management.
|
|
package session
|
|
|
|
import (
|
|
"errors"
|
|
"net/http"
|
|
"time"
|
|
|
|
"github.com/gorilla/securecookie"
|
|
|
|
"sneak.berlin/go/pixa/internal/seal"
|
|
)
|
|
|
|
// Session configuration constants.
|
|
const (
|
|
CookieName = "pixa_session"
|
|
SessionTTL = 30 * 24 * time.Hour // 30 days
|
|
|
|
// HKDF salts for key derivation
|
|
hashKeySalt = "pixa-session-hash-v1"
|
|
blockKeySalt = "pixa-session-block-v1"
|
|
)
|
|
|
|
// Errors returned by session operations.
|
|
var (
|
|
ErrInvalidSession = errors.New("invalid or expired session")
|
|
ErrNoSession = errors.New("no session cookie present")
|
|
)
|
|
|
|
// Data contains the session payload stored in the encrypted cookie.
|
|
type Data struct {
|
|
Authenticated bool `json:"auth"`
|
|
CreatedAt time.Time `json:"created"`
|
|
ExpiresAt time.Time `json:"expires"`
|
|
}
|
|
|
|
// Manager handles session creation and validation using encrypted cookies.
|
|
type Manager struct {
|
|
sc *securecookie.SecureCookie
|
|
}
|
|
|
|
// NewManager creates a session manager with keys derived from the signing key.
|
|
//
|
|
// Session cookies always carry the Secure, HttpOnly, and SameSite=Strict
|
|
// attributes; this cannot be configured. Browsers treat http://localhost as a
|
|
// trustworthy origin and accept Secure cookies there, so local development
|
|
// keeps working.
|
|
func NewManager(signingKey string) (*Manager, error) {
|
|
masterKey := []byte(signingKey)
|
|
|
|
// Derive separate keys for HMAC (hash) and encryption (block)
|
|
hashKey, err := seal.DeriveKey(masterKey, hashKeySalt)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
blockKey, err := seal.DeriveKey(masterKey, blockKeySalt)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
sc := securecookie.New(hashKey[:], blockKey[:])
|
|
sc.MaxAge(int(SessionTTL.Seconds()))
|
|
|
|
return &Manager{
|
|
sc: sc,
|
|
}, nil
|
|
}
|
|
|
|
// CreateSession creates a new authenticated session and sets the cookie.
|
|
func (m *Manager) CreateSession(w http.ResponseWriter) error {
|
|
now := time.Now()
|
|
data := &Data{
|
|
Authenticated: true,
|
|
CreatedAt: now,
|
|
ExpiresAt: now.Add(SessionTTL),
|
|
}
|
|
|
|
encoded, err := m.sc.Encode(CookieName, data)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
|
|
http.SetCookie(w, &http.Cookie{
|
|
Name: CookieName,
|
|
Value: encoded,
|
|
Path: "/",
|
|
MaxAge: int(SessionTTL.Seconds()),
|
|
HttpOnly: true,
|
|
Secure: true,
|
|
SameSite: http.SameSiteStrictMode,
|
|
})
|
|
|
|
return nil
|
|
}
|
|
|
|
// ValidateSession checks if the request has a valid session cookie.
|
|
// Returns the session data if valid, or an error if invalid/missing.
|
|
func (m *Manager) ValidateSession(r *http.Request) (*Data, error) {
|
|
cookie, err := r.Cookie(CookieName)
|
|
if err != nil {
|
|
if errors.Is(err, http.ErrNoCookie) {
|
|
return nil, ErrNoSession
|
|
}
|
|
|
|
return nil, err
|
|
}
|
|
|
|
var data Data
|
|
if err := m.sc.Decode(CookieName, cookie.Value, &data); err != nil {
|
|
return nil, ErrInvalidSession
|
|
}
|
|
|
|
// Check if session has expired (defense in depth - cookie MaxAge should handle this)
|
|
if time.Now().After(data.ExpiresAt) {
|
|
return nil, ErrInvalidSession
|
|
}
|
|
|
|
if !data.Authenticated {
|
|
return nil, ErrInvalidSession
|
|
}
|
|
|
|
return &data, nil
|
|
}
|
|
|
|
// ClearSession removes the session cookie.
|
|
func (m *Manager) ClearSession(w http.ResponseWriter) {
|
|
http.SetCookie(w, &http.Cookie{
|
|
Name: CookieName,
|
|
Value: "",
|
|
Path: "/",
|
|
MaxAge: -1, // Delete immediately
|
|
HttpOnly: true,
|
|
Secure: true,
|
|
SameSite: http.SameSiteStrictMode,
|
|
})
|
|
}
|
|
|
|
// IsAuthenticated is a convenience method that returns true if the request
|
|
// has a valid authenticated session.
|
|
func (m *Manager) IsAuthenticated(r *http.Request) bool {
|
|
data, err := m.ValidateSession(r)
|
|
|
|
return err == nil && data != nil && data.Authenticated
|
|
}
|