diff --git a/README.md b/README.md index c764516..d374368 100644 --- a/README.md +++ b/README.md @@ -30,14 +30,32 @@ export MEALQUEST_PUBLIC_URL=http://localhost:5173 export MEALQUEST_DATA_DIR="$HOME/.mealquest-dev" export MEALQUEST_SHUTDOWN_TIMEOUT=10s export MEALQUEST_SECRET_KEY_FILE="$HOME/.mealquest-dev/secret_key" +# Authentik OAuth2/OpenID provider for MealQuest (redirect URI: +# $MEALQUEST_PUBLIC_URL/auth/callback). Save its client secret to the file. +export MEALQUEST_OIDC_ISSUER=https://auth.example.com/application/o/mealquest/ +export MEALQUEST_OIDC_CLIENT_ID=mealquest +export MEALQUEST_OIDC_CLIENT_SECRET_FILE="$HOME/.mealquest-dev/oidc_client_secret" +export MEALQUEST_OIDC_ALLOWED_GROUPS=mealquest-admin +# Optional: sessions end after MEALQUEST_SESSION_IDLE unused (default 720h) +# and always MEALQUEST_SESSION_MAX after sign-in (default 4320h). +# export MEALQUEST_SESSION_IDLE=720h +# export MEALQUEST_SESSION_MAX=4320h +# Optional: comma-separated IPs/CIDRs of reverse proxies whose +# X-Forwarded-For/-Proto headers are honoured (production: Traefik). Unset +# trusts no peer, which is right for local development. +# export MEALQUEST_TRUSTED_PROXY=10.60.1.33 make dev ``` `make dev` runs the Go server on `:8080` and the Vite dev server on `:5173` (which proxies `/api`, `/auth` and `/media` to `:8080`). Open http://localhost:5173. Configuration is environment-only and fails fast: -every variable above is required, and secrets are read only from files named -by `*_FILE` variables. +every variable above is required except the commented-out session +lifetimes (when set they must be positive, with MAX at least IDLE) and +trusted proxy list (every entry must be an IP or CIDR), and +secrets are read only from files named by `*_FILE` variables. `MEALQUEST_PUBLIC_URL`, `MEALQUEST_OIDC_ISSUER` and the +endpoints the issuer advertises must be `https://`; plain `http://` is +accepted only for `localhost`, `127.0.0.1` and `[::1]`. ## Common commands diff --git a/cmd/server/main.go b/cmd/server/main.go index f98335e..c937169 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -13,7 +13,9 @@ import ( "syscall" "time" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/config" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/frontend" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/server" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" @@ -30,7 +32,7 @@ func main() { os.Exit(1) } - logger := slog.New(slog.NewJSONHandler(os.Stdout, nil)) + logger := slog.New(forwarded.LogHandler(slog.NewJSONHandler(os.Stdout, nil))) if err := run(cfg, logger); err != nil { logger.Error("server stopped", "error", err) os.Exit(1) @@ -51,7 +53,23 @@ func run(cfg config.Config, logger *slog.Logger) error { } }() - router, err := server.NewRouter(logger, version, frontend.Assets()) + sessions, err := auth.NewSessionManager(db, cfg.SessionIdle, cfg.SessionMax) + if err != nil { + return err + } + authFlow, err := auth.New(ctx, auth.Config{ + Issuer: cfg.OIDCIssuer.String(), + ClientID: cfg.OIDCClientID, + ClientSecret: cfg.OIDCClientSecret.Reveal(), + PublicURL: cfg.PublicURL, + SecretKey: []byte(cfg.SecretKey.Reveal()), + AllowedGroups: cfg.OIDCAllowedGroups, + }, sessions, logger) + if err != nil { + return err + } + + router, err := server.NewRouter(logger, version, cfg.TrustedProxy, authFlow, sessions, frontend.Assets()) if err != nil { return err } @@ -109,6 +127,7 @@ func logListening(logger *slog.Logger, cfg config.Config) { logger.Info("listening", "addr", cfg.HTTPAddr, "public_origin", cfg.PublicOrigin(), + "trusted_proxy", cfg.TrustedProxy, "data_dir", cfg.DataDir, "version", version, ) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 4beae8c..6f9eaab 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -29,6 +29,15 @@ docker compose project "mealquest" — 10.60.1.14 One Go service serves the API, the auth flow, media and the PWA. Its SQLite database is replicated by Litestream to the NAS. +Traefik terminates TLS. The service honours `X-Forwarded-For` and +`X-Forwarded-Proto` only from peers listed in `MEALQUEST_TRUSTED_PROXY` +(comma-separated IPs/CIDRs; production lists `10.60.1.33`); from any other +peer they are ignored. The client is the rightmost `X-Forwarded-For` address +that is not a trusted proxy. The resolved client IP is added as `client_ip` +to every log line written while handling a request. The OIDC redirect URI +and the Secure cookies are fixed by `MEALQUEST_PUBLIC_URL`, never by request +headers, so sign-in works whichever peer the request arrives from. + ## Repository layout | Path | Purpose | diff --git a/go.mod b/go.mod index a517797..39a6ce9 100644 --- a/go.mod +++ b/go.mod @@ -3,13 +3,16 @@ module code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest go 1.25.0 require ( + github.com/coreos/go-oidc/v3 v3.21.0 github.com/go-chi/chi/v5 v5.3.1 github.com/oklog/ulid/v2 v2.1.2 + golang.org/x/oauth2 v0.36.0 modernc.org/sqlite v1.59.0 ) require ( github.com/dustin/go-humanize v1.0.1 // indirect + github.com/go-jose/go-jose/v4 v4.1.4 // indirect github.com/google/uuid v1.6.0 // indirect github.com/mattn/go-isatty v0.0.24 // indirect github.com/ncruces/go-strftime v1.0.0 // indirect diff --git a/go.sum b/go.sum index 3a07785..ba3f090 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,11 @@ +github.com/coreos/go-oidc/v3 v3.21.0 h1:wZo4Q9Pum8dYEj0eMUPrqR+kvuGkeUplbLpNCkBqoWM= +github.com/coreos/go-oidc/v3 v3.21.0/go.mod h1:DYCf24+ncYi+XkIH97GY1+dqoRlbaSI26KVTCI9SrY4= github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= github.com/go-chi/chi/v5 v5.3.1 h1:3j4HZLGZQ3JpMCrPJF/Jl3mYJfWLKBfNJ6quurUGCf8= github.com/go-chi/chi/v5 v5.3.1/go.mod h1:R+tYY2hNuVUUjxoPtqUdgBqevM9s9njzkTLutVsOCto= +github.com/go-jose/go-jose/v4 v4.1.4 h1:moDMcTHmvE6Groj34emNPLs/qtYXRVcd6S7NHbHz3kA= +github.com/go-jose/go-jose/v4 v4.1.4/go.mod h1:x4oUasVrzR7071A4TnHLGSPpNOm2a21K9Kf04k1rs08= github.com/google/pprof v0.0.0-20260802141513-ef3492d7dac3 h1:LMLX+LgTNWpfvCBdFebv6EsYotImrt/Ppc5cXIriCSo= github.com/google/pprof v0.0.0-20260802141513-ef3492d7dac3/go.mod h1:jl5iWTm0/hd5PjEYEOuwAJ57L/CibdZfrqZ5XA5GrCk= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= @@ -19,6 +23,8 @@ github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec h1:W09IVJc94 github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo= golang.org/x/mod v0.38.0 h1:MECBjubtXD7yj4HrhIUcywNaGeNVUdfVnxmPajOk4yk= golang.org/x/mod v0.38.0/go.mod h1:V6Xz0pq8TQ3dGqVQ1FVHuelZpAL0uNhSkk9ogYP3c40= +golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= +golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= diff --git a/internal/auth/actor.go b/internal/auth/actor.go new file mode 100644 index 0000000..c781091 --- /dev/null +++ b/internal/auth/actor.go @@ -0,0 +1,45 @@ +package auth + +import ( + "context" + "slices" +) + +// Actor is the signed-in household member a request acts for. Handlers +// record Actor.Subject as updated_by on edits. +type Actor struct { + Subject string + Email string + Name string + Groups []string +} + +// Actor returns the household member s belongs to. +func (s Session) Actor() Actor { + return Actor{Subject: s.Subject, Email: s.Email, Name: s.Name, Groups: slices.Clone(s.Groups)} +} + +type sessionKey struct{} + +// WithSession returns a copy of ctx carrying s. The session middleware calls +// it for every authenticated request. +func WithSession(ctx context.Context, s Session) context.Context { + return context.WithValue(ctx, sessionKey{}, s) +} + +// SessionFrom returns the session WithSession stored in ctx. ok is false when +// ctx has none, which behind the session middleware means a wiring bug. +func SessionFrom(ctx context.Context) (s Session, ok bool) { + s, ok = ctx.Value(sessionKey{}).(Session) + return s, ok +} + +// ActorFrom returns the actor of the session WithSession stored in ctx. ok is +// false when ctx has none. +func ActorFrom(ctx context.Context) (actor Actor, ok bool) { + s, ok := SessionFrom(ctx) + if !ok { + return Actor{}, false + } + return s.Actor(), true +} diff --git a/internal/auth/actor_test.go b/internal/auth/actor_test.go new file mode 100644 index 0000000..fff57cb --- /dev/null +++ b/internal/auth/actor_test.go @@ -0,0 +1,40 @@ +package auth + +import ( + "context" + "slices" + "testing" +) + +func TestActorContext(t *testing.T) { + t.Parallel() + if _, ok := ActorFrom(context.Background()); ok { + t.Error("ActorFrom found an actor in an empty context") + } + if _, ok := SessionFrom(context.Background()); ok { + t.Error("SessionFrom found a session in an empty context") + } + s := Session{Subject: "user-123", Email: "cook@example.com", Name: "Home Cook", Groups: []string{"mealquest-admin"}, CSRFToken: "t"} + ctx := WithSession(context.Background(), s) + gotSession, ok := SessionFrom(ctx) + if !ok || gotSession.Subject != s.Subject || gotSession.CSRFToken != s.CSRFToken { + t.Errorf("SessionFrom = %+v (ok=%v), want %+v", gotSession, ok, s) + } + got, ok := ActorFrom(ctx) + if !ok || got.Subject != s.Subject || got.Email != s.Email || got.Name != s.Name || !slices.Equal(got.Groups, s.Groups) { + t.Errorf("ActorFrom = %+v (ok=%v), want the session's actor", got, ok) + } +} + +func TestSessionActor(t *testing.T) { + t.Parallel() + s := Session{Subject: "user-123", Email: "cook@example.com", Name: "Home Cook", Groups: []string{"mealquest-admin"}, CSRFToken: "t"} + a := s.Actor() + if a.Subject != s.Subject || a.Email != s.Email || a.Name != s.Name || !slices.Equal(a.Groups, s.Groups) { + t.Errorf("Actor = %+v, want the session's identity", a) + } + a.Groups[0] = "changed" + if s.Groups[0] != "mealquest-admin" { + t.Error("Actor shares the session's groups slice") + } +} diff --git a/internal/auth/auth.go b/internal/auth/auth.go new file mode 100644 index 0000000..d82d33d --- /dev/null +++ b/internal/auth/auth.go @@ -0,0 +1,439 @@ +// Package auth signs the household in with Authentik over OpenID Connect. +// The service is a confidential client: the authorization code, tokens and +// client secret stay on the server and never reach JavaScript. +package auth + +import ( + "context" + "crypto/rand" + "crypto/subtle" + "errors" + "fmt" + "log/slog" + "net/http" + "net/url" + "slices" + "sync/atomic" + "time" + + "github.com/coreos/go-oidc/v3/oidc" + "github.com/go-chi/chi/v5" + "golang.org/x/oauth2" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secureurl" +) + +// Routes owned by this package. +const ( + LoginPath = "/auth/login" + CallbackPath = "/auth/callback" + LogoutPath = "/auth/logout" +) + +// providerTimeout bounds every request to the identity provider. +const providerTimeout = 10 * time.Second + +// scopes requested at login. +var scopes = []string{oidc.ScopeOpenID, "profile", "email", "groups", oidc.ScopeOfflineAccess} + +// Claims are the verified ID token claims handed to the session layer. +type Claims struct { + Subject string `json:"sub"` + Email string `json:"email"` + EmailVerified bool `json:"email_verified"` + Name string `json:"name"` + PreferredUsername string `json:"preferred_username"` + Groups []string `json:"groups"` +} + +// Sessions is the session layer. StartSession is called once per successful +// login with claims from a verified ID token; it must set the session cookie +// on w. EndSession is called on logout; it must end the session r carries, +// if any, and clear the cookie on w. A returned error fails the request with +// 500. +type Sessions interface { + StartSession(w http.ResponseWriter, r *http.Request, claims Claims) error + EndSession(w http.ResponseWriter, r *http.Request) error +} + +// Config is what New needs to reach the identity provider. +type Config struct { + // Issuer is the exact issuer URL; discovery runs against it. + Issuer string + ClientID string + ClientSecret string + // PublicURL is the service's public URL; its origin prefixes CallbackPath + // to form the redirect URI. + PublicURL *url.URL + // SecretKey is the server key the login cookie's signing key derives from. + SecretKey []byte + // AllowedGroups are the household's groups; a sign-in is refused unless + // the account is in at least one of them. + AllowedGroups []string +} + +// Auth serves the login and callback routes. +type Auth struct { + oauth oauth2.Config + verifier *oidc.IDTokenVerifier + provider *oidc.Provider + client *http.Client + cookie loginCookie + sessions Sessions + allowed []string + // endSessionURL is the provider's end_session_endpoint; logout ends + // there so the provider's own session ends too. + endSessionURL string + logger *slog.Logger +} + +// New discovers the provider at cfg.Issuer and returns the login flow. It +// fails if discovery fails or the issuer the provider reports differs. +func New(ctx context.Context, cfg Config, sessions Sessions, logger *slog.Logger) (*Auth, error) { + switch { + case cfg.Issuer == "", cfg.ClientID == "", cfg.ClientSecret == "": + return nil, errors.New("auth: issuer, client ID and client secret are required") + case cfg.PublicURL == nil: + return nil, errors.New("auth: public URL is required") + case len(cfg.SecretKey) == 0: + return nil, errors.New("auth: secret key is required") + case sessions == nil: + return nil, errors.New("auth: session layer is required") + case len(cfg.AllowedGroups) == 0: + return nil, errors.New("auth: at least one allowed group is required") + } + + if err := checkEndpoint("issuer", cfg.Issuer); err != nil { + return nil, err + } + + client := &http.Client{Timeout: providerTimeout, CheckRedirect: checkRedirect} + discovery := *client + recorder := &statusRecorder{next: http.DefaultTransport} + discovery.Transport = recorder + provider, err := oidc.NewProvider(oidc.ClientContext(ctx, &discovery), cfg.Issuer) + if err != nil { + return nil, discoveryError(err, int(recorder.status.Load())) + } + endpoint := provider.Endpoint() + var discovered struct { + JWKSURI string `json:"jwks_uri"` + EndSessionURI string `json:"end_session_endpoint"` + } + if err := provider.Claims(&discovered); err != nil { + return nil, fmt.Errorf("auth: OIDC discovery: %w", err) + } + // The token endpoint receives the client secret and the key set decides + // which tokens verify, so every endpoint used must pass the same rule. + for _, e := range []struct{ name, raw string }{ + {"authorization endpoint", endpoint.AuthURL}, + {"token endpoint", endpoint.TokenURL}, + {"JWKS endpoint", discovered.JWKSURI}, + {"end session endpoint", discovered.EndSessionURI}, + // Groups come from userinfo when the ID token carries none, and the + // access token is sent there. + {"userinfo endpoint", provider.UserInfoEndpoint()}, + } { + if err := checkEndpoint(e.name, e.raw); err != nil { + return nil, err + } + } + // client_secret_basic, the OIDC default; no auto-detection retries. + endpoint.AuthStyle = oauth2.AuthStyleInHeader + + return &Auth{ + oauth: oauth2.Config{ + ClientID: cfg.ClientID, + ClientSecret: cfg.ClientSecret, + Endpoint: endpoint, + RedirectURL: cfg.PublicURL.Scheme + "://" + cfg.PublicURL.Host + CallbackPath, + Scopes: scopes, + }, + verifier: provider.Verifier(&oidc.Config{ + ClientID: cfg.ClientID, + SupportedSigningAlgs: []string{oidc.RS256}, + }), + provider: provider, + client: client, + allowed: slices.Clone(cfg.AllowedGroups), + cookie: newLoginCookie(cfg.SecretKey), + sessions: sessions, + endSessionURL: discovered.EndSessionURI, + logger: logger, + }, nil +} + +// statusRecorder records the status of the last response it carries. +type statusRecorder struct { + next http.RoundTripper + status atomic.Int32 +} + +func (s *statusRecorder) RoundTrip(req *http.Request) (*http.Response, error) { + resp, err := s.next.RoundTrip(req) + if err == nil { + s.status.Store(int32(resp.StatusCode)) + } + return resp, err +} + +// discoveryError reports a failed discovery. go-oidc puts the provider's +// response body in its error for a non-200 discovery response, so that +// error is replaced by the status alone. Transport errors carry no response +// content and keep their cause. +func discoveryError(err error, status int) error { + var urlErr *url.Error + if !errors.As(err, &urlErr) && status != 0 && status != http.StatusOK { + return fmt.Errorf("auth: OIDC discovery: provider answered HTTP %d", status) + } + return fmt.Errorf("auth: OIDC discovery: %w", err) +} + +// checkEndpoint applies secureurl.Check to the provider URL raw, naming the +// endpoint in the error. +func checkEndpoint(name, raw string) error { + u, err := url.Parse(raw) + if err != nil { + return fmt.Errorf("auth: OIDC %s %q is not a valid URL", name, raw) + } + if err := secureurl.Check(u); err != nil { + return fmt.Errorf("auth: OIDC %s %q: %w", name, raw, err) + } + return nil +} + +// maxRedirects matches net/http's default redirect limit. +const maxRedirects = 10 + +// checkRedirect is the provider client's redirect policy. On a 307 or 308 to +// the same host Go resends the body and Authorization header, which carry the +// code and client secret, so every target must pass secureurl.Check and an +// https:// request never follows a redirect to plain http://, even loopback. +func checkRedirect(req *http.Request, via []*http.Request) error { + if len(via) >= maxRedirects { + return fmt.Errorf("stopped after %d redirects", maxRedirects) + } + if err := secureurl.Check(req.URL); err != nil { + return fmt.Errorf("redirect refused: %w", err) + } + if via[len(via)-1].URL.Scheme == "https" && req.URL.Scheme != "https" { + return errors.New("redirect refused: https:// to plain http:// downgrade") + } + return nil +} + +// Mount registers GET LoginPath, GET CallbackPath and POST LogoutPath on r. +func (a *Auth) Mount(r chi.Router) { + r.Get(LoginPath, a.login) + r.Get(CallbackPath, a.callback) + r.Post(LogoutPath, a.logout) +} + +// EndSessionURL is the provider's end_session_endpoint. +func (a *Auth) EndSessionURL() string { return a.endSessionURL } + +// login starts the authorization code flow with PKCE (S256). The optional +// "return" query parameter must be a same-origin path; anything else fails +// the login with 400. +func (a *Auth) login(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Cache-Control", "no-store") + + returnTo, err := returnPath(r.URL.Query().Get("return")) + if err != nil { + http.Error(w, "invalid return path", http.StatusBadRequest) + return + } + state := loginState{ + State: rand.Text(), + Nonce: rand.Text(), + Verifier: oauth2.GenerateVerifier(), + Return: returnTo, + } + if err := a.cookie.set(w, state, time.Now()); err != nil { + a.logger.ErrorContext(r.Context(), "set login cookie", "error", err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + http.Redirect(w, r, a.oauth.AuthCodeURL(state.State, + oidc.Nonce(state.Nonce), + oauth2.S256ChallengeOption(state.Verifier), + ), http.StatusFound) +} + +// callback completes the flow: it checks state against the login cookie, +// exchanges the code, verifies the ID token and nonce, then starts a session +// if the account is in an allowed group. Membership is checked on every +// login, so an account removed from the group is refused at its next one. +func (a *Auth) callback(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Cache-Control", "no-store") + ctx := r.Context() + // The login cookie is single-use. + a.cookie.clear(w) + + state, err := a.cookie.read(r, time.Now()) + if err != nil { + a.logger.WarnContext(ctx, "login callback without a valid login cookie", "error", err) + http.Error(w, "sign-in expired; start again", http.StatusBadRequest) + return + } + q := r.URL.Query() + if subtle.ConstantTimeCompare([]byte(q.Get("state")), []byte(state.State)) != 1 { + a.logger.WarnContext(ctx, "login callback state mismatch") + http.Error(w, "sign-in state mismatch; start again", http.StatusBadRequest) + return + } + if q.Get("error") != "" { + // error and error_description are provider-supplied text; only the + // fixed category is logged. + a.logger.WarnContext(ctx, "identity provider returned an authorization error") + http.Error(w, "sign-in was refused", http.StatusUnauthorized) + return + } + code := q.Get("code") + if code == "" { + http.Error(w, "missing authorization code", http.StatusBadRequest) + return + } + + claims, err := a.verify(ctx, code, state) + if err != nil { + a.logger.WarnContext(ctx, "sign-in failed", "error", err) + http.Error(w, "sign-in failed", http.StatusUnauthorized) + return + } + if !a.member(claims.Groups) { + if claims.Groups == nil { + a.logger.WarnContext(ctx, "sign-in refused: no groups claim in the ID token or userinfo", "subject", claims.Subject) + } else { + a.logger.WarnContext(ctx, "sign-in refused: not in an allowed group", "subject", claims.Subject) + } + // A session this browser already holds ends too, so a refused + // sign-in never leaves it signed in. + if err := a.sessions.EndSession(w, r); err != nil { + a.logger.ErrorContext(ctx, "end session", "error", err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + a.refuse(w, r) + return + } + if err := a.sessions.StartSession(w, r, claims); err != nil { + a.logger.ErrorContext(ctx, "start session", "error", err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + http.Redirect(w, r, state.Return, http.StatusSeeOther) +} + +// logout ends the session, clears the cookie and sends the browser to the +// provider's end_session_endpoint to end the provider session as well. +func (a *Auth) logout(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Cache-Control", "no-store") + if err := a.sessions.EndSession(w, r); err != nil { + a.logger.ErrorContext(r.Context(), "end session", "error", err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + http.Redirect(w, r, a.endSessionURL, http.StatusSeeOther) +} + +// verify exchanges code for tokens and returns the verified ID token claims. +// The verifier enforces RS256, issuer, audience and expiry; nonce is checked +// here. When the ID token has no groups claim, Groups comes from userinfo; +// it stays nil when neither has one. +func (a *Auth) verify(ctx context.Context, code string, state loginState) (Claims, error) { + token, err := a.oauth.Exchange(context.WithValue(ctx, oauth2.HTTPClient, a.client), code, + oauth2.VerifierOption(state.Verifier)) + if err != nil { + return Claims{}, exchangeError(err) + } + raw, ok := token.Extra("id_token").(string) + if !ok || raw == "" { + return Claims{}, errors.New("token response has no id_token") + } + idToken, err := a.verifier.Verify(ctx, raw) + if err != nil { + return Claims{}, verifyError(err) + } + if subtle.ConstantTimeCompare([]byte(idToken.Nonce), []byte(state.Nonce)) != 1 { + return Claims{}, errors.New("ID token nonce mismatch") + } + var claims Claims + if err := idToken.Claims(&claims); err != nil { + return Claims{}, fmt.Errorf("decode ID token claims: %w", err) + } + if claims.Subject == "" { + return Claims{}, errors.New("ID token has no subject") + } + if claims.Groups == nil { + groups, err := a.userinfoGroups(ctx, token, claims.Subject) + if err != nil { + return Claims{}, err + } + claims.Groups = groups + } + return claims, nil +} + +// userinfoGroups returns the groups claim from the userinfo endpoint, nil if +// it has none. The userinfo subject must match the ID token's subject. +func (a *Auth) userinfoGroups(ctx context.Context, token *oauth2.Token, subject string) ([]string, error) { + info, err := a.provider.UserInfo(oidc.ClientContext(ctx, a.client), oauth2.StaticTokenSource(token)) + if err != nil { + return nil, userinfoError(err) + } + if info.Subject != subject { + return nil, errors.New("userinfo subject does not match the ID token") + } + var claims struct { + Groups []string `json:"groups"` + } + if err := info.Claims(&claims); err != nil { + return nil, fmt.Errorf("decode userinfo claims: %w", err) + } + return claims.Groups, nil +} + +// userinfoError reduces a failed userinfo request to a fixed category, like +// exchangeError: go-oidc's error text can carry the response body. +func userinfoError(err error) error { + var transport *url.Error + if errors.As(err, &transport) { + return fmt.Errorf("userinfo: request failed: %s: %w", transport.Op, transport.Err) + } + return errors.New("userinfo: endpoint returned an unusable response") +} + +// verifyError reduces a failed ID token verification to a fixed category. +// go-oidc's error text can carry the JWKS endpoint's response body or +// token-supplied values, so only the expiry of an expired token is kept. +func verifyError(err error) error { + var expired *oidc.TokenExpiredError + if errors.As(err, &expired) { + return fmt.Errorf("verify ID token: token expired at %s", expired.Expiry.UTC().Format(time.RFC3339)) + } + return errors.New("verify ID token: token or provider key set rejected") +} + +// member reports whether groups contains an allowed group. +func (a *Auth) member(groups []string) bool { + return slices.ContainsFunc(groups, func(g string) bool { return slices.Contains(a.allowed, g) }) +} + +// exchangeError reduces a failed code exchange to a fixed category and safe +// metadata. The oauth2 error text can carry the token endpoint's response +// body or error_description, which may echo credentials, so it is dropped. +func exchangeError(err error) error { + var retrieve *oauth2.RetrieveError + var transport *url.Error + switch { + case errors.As(err, &retrieve): + return fmt.Errorf("exchange code: token endpoint rejected the request (HTTP %d)", retrieve.Response.StatusCode) + case errors.As(err, &transport): + // Request failures never carry the response. The URL is dropped: after + // a refused redirect it is the provider-supplied Location. + return fmt.Errorf("exchange code: token request failed: %s: %w", transport.Op, transport.Err) + default: + return errors.New("exchange code: token endpoint returned an unusable response") + } +} diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go new file mode 100644 index 0000000..407bbb8 --- /dev/null +++ b/internal/auth/auth_test.go @@ -0,0 +1,1075 @@ +package auth + +import ( + "bytes" + "context" + "crypto" + "crypto/hmac" + "crypto/rand" + "crypto/rsa" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "io" + "log/slog" + "maps" + "math/big" + "net/http" + "net/http/httptest" + "net/url" + "slices" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/go-chi/chi/v5" +) + +const ( + testClientID = "mealquest" + testClientSecret = "client-s3cret" + testKID = "test-key" +) + +var testPublicURL = &url.URL{Scheme: "https", Host: "meals.example.com"} + +// testAllowedGroups are the household's groups in tests. +var testAllowedGroups = []string{"mealquest-admin", "mealquest-family"} + +// fakeProvider is an OIDC provider serving discovery, JWKS and a token +// endpoint. Each issued code maps to the ID token the token endpoint returns. +type fakeProvider struct { + t *testing.T + srv *httptest.Server + key *rsa.PrivateKey + + mu sync.Mutex + codes map[string]issuedCode + // discoveryOverrides replace fields of the discovery document. + discoveryOverrides map[string]any + // tokenReply, when set, is returned by the token endpoint instead of a + // token. + tokenReply *tokenReply + // userinfo is the userinfo endpoint's response; userinfoHits counts + // requests to it. + userinfo map[string]any + userinfoHits int + // userinfoReply, when set, is returned by the userinfo endpoint instead. + userinfoReply *tokenReply + // jwksReply, when set, is returned by the JWKS endpoint instead. + jwksReply *tokenReply +} + +type tokenReply struct { + status int + body string +} + +type issuedCode struct { + challenge string + idToken string +} + +func newFakeProvider(t *testing.T) *fakeProvider { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + p := &fakeProvider{t: t, key: key, codes: map[string]issuedCode{}} + mux := http.NewServeMux() + mux.HandleFunc("GET /.well-known/openid-configuration", p.discovery) + mux.HandleFunc("GET /jwks", p.jwks) + mux.HandleFunc("POST /token", p.token) + mux.HandleFunc("GET /userinfo", p.userinfoEndpoint) + p.srv = httptest.NewServer(mux) + t.Cleanup(p.srv.Close) + return p +} + +func (p *fakeProvider) issuer() string { return p.srv.URL } + +func (p *fakeProvider) discovery(w http.ResponseWriter, _ *http.Request) { + doc := map[string]any{ + "issuer": p.issuer(), + "authorization_endpoint": p.issuer() + "/authorize", + "token_endpoint": p.issuer() + "/token", + "jwks_uri": p.issuer() + "/jwks", + "userinfo_endpoint": p.issuer() + "/userinfo", + "end_session_endpoint": p.issuer() + "/end-session/", + "response_types_supported": []string{"code"}, + "subject_types_supported": []string{"public"}, + "id_token_signing_alg_values_supported": []string{"RS256", "HS256"}, + } + p.mu.Lock() + maps.Copy(doc, p.discoveryOverrides) + p.mu.Unlock() + writeJSON(w, doc) +} + +func (p *fakeProvider) jwks(w http.ResponseWriter, _ *http.Request) { + p.mu.Lock() + reply := p.jwksReply + p.mu.Unlock() + if reply != nil { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(reply.status) + io.WriteString(w, reply.body) + return + } + writeJSON(w, map[string]any{"keys": []map[string]string{{ + "kty": "RSA", + "kid": testKID, + "use": "sig", + "alg": "RS256", + "n": b64(p.key.N.Bytes()), + "e": b64(big.NewInt(int64(p.key.E)).Bytes()), + }}}) +} + +// token checks client authentication, the code and the PKCE verifier before +// returning the code's ID token. +func (p *fakeProvider) token(w http.ResponseWriter, r *http.Request) { + p.mu.Lock() + reply := p.tokenReply + p.mu.Unlock() + if reply != nil { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(reply.status) + io.WriteString(w, reply.body) + return + } + id, secret, ok := r.BasicAuth() + if !ok || id != testClientID || secret != testClientSecret { + http.Error(w, `{"error":"invalid_client"}`, http.StatusUnauthorized) + return + } + if err := r.ParseForm(); err != nil { + http.Error(w, `{"error":"invalid_request"}`, http.StatusBadRequest) + return + } + p.mu.Lock() + issued, ok := p.codes[r.PostForm.Get("code")] + delete(p.codes, r.PostForm.Get("code")) + p.mu.Unlock() + if r.PostForm.Get("grant_type") != "authorization_code" || !ok || + challengeS256(r.PostForm.Get("code_verifier")) != issued.challenge { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + io.WriteString(w, `{"error":"invalid_grant"}`) + return + } + writeJSON(w, map[string]any{ + "access_token": "access-token", + "token_type": "Bearer", + "expires_in": 3600, + "id_token": issued.idToken, + }) +} + +// userinfoEndpoint checks the access token and returns p.userinfo, which +// defaults to the test user without a groups claim. +func (p *fakeProvider) userinfoEndpoint(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer access-token" { + http.Error(w, `{"error":"invalid_token"}`, http.StatusUnauthorized) + return + } + p.mu.Lock() + p.userinfoHits++ + info, reply := p.userinfo, p.userinfoReply + p.mu.Unlock() + if reply != nil { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(reply.status) + io.WriteString(w, reply.body) + return + } + if info == nil { + info = map[string]any{"sub": "user-123", "email": "cook@example.com"} + } + writeJSON(w, info) +} + +func (p *fakeProvider) setUserinfo(info map[string]any) { + p.mu.Lock() + defer p.mu.Unlock() + p.userinfo = info +} + +func (p *fakeProvider) userinfoRequests() int { + p.mu.Lock() + defer p.mu.Unlock() + return p.userinfoHits +} + +// authorize stands in for the user signing in at the provider: it records a +// code bound to the PKCE challenge and returns it. +func (p *fakeProvider) authorize(challenge, idToken string) string { + code := rand.Text() + p.mu.Lock() + p.codes[code] = issuedCode{challenge: challenge, idToken: idToken} + p.mu.Unlock() + return code +} + +// claims returns valid ID token claims for nonce. +func (p *fakeProvider) claims(nonce string) map[string]any { + now := time.Now() + return map[string]any{ + "iss": p.issuer(), + "sub": "user-123", + "aud": testClientID, + "iat": now.Unix(), + "exp": now.Add(5 * time.Minute).Unix(), + "nonce": nonce, + "email": "cook@example.com", + "email_verified": true, + "name": "Home Cook", + "preferred_username": "cook", + "groups": []string{"mealquest-admin"}, + } +} + +func (p *fakeProvider) signRS256(claims map[string]any) string { + return signRS256(p.t, p.key, claims) +} + +func signRS256(t *testing.T, key *rsa.PrivateKey, claims map[string]any) string { + t.Helper() + signing := jwtSigningInput(t, "RS256", claims) + digest := sha256.Sum256([]byte(signing)) + sig, err := rsa.SignPKCS1v15(rand.Reader, key, crypto.SHA256, digest[:]) + if err != nil { + t.Fatal(err) + } + return signing + "." + b64(sig) +} + +// signHS256 signs with the client secret, as in an algorithm-confusion attack. +func (p *fakeProvider) signHS256(claims map[string]any) string { + signing := jwtSigningInput(p.t, "HS256", claims) + mac := hmac.New(sha256.New, []byte(testClientSecret)) + mac.Write([]byte(signing)) + return signing + "." + b64(mac.Sum(nil)) +} + +func jwtSigningInput(t *testing.T, alg string, claims map[string]any) string { + t.Helper() + header, err := json.Marshal(map[string]string{"alg": alg, "typ": "JWT", "kid": testKID}) + if err != nil { + t.Fatal(err) + } + body, err := json.Marshal(claims) + if err != nil { + t.Fatal(err) + } + return b64(header) + "." + b64(body) +} + +func challengeS256(verifier string) string { + sum := sha256.Sum256([]byte(verifier)) + return b64(sum[:]) +} + +func b64(b []byte) string { return base64.RawURLEncoding.EncodeToString(b) } + +func writeJSON(w http.ResponseWriter, v any) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(v) +} + +// memorySessions is an in-memory Sessions that records the claims it was +// handed and how many sessions were ended. +type memorySessions struct { + mu sync.Mutex + claims []Claims + ended int + err error +} + +func (m *memorySessions) StartSession(w http.ResponseWriter, _ *http.Request, claims Claims) error { + m.mu.Lock() + defer m.mu.Unlock() + if m.err != nil { + return m.err + } + m.claims = append(m.claims, claims) + http.SetCookie(w, &http.Cookie{Name: "test_session", Value: claims.Subject}) + return nil +} + +func (m *memorySessions) EndSession(http.ResponseWriter, *http.Request) error { + m.mu.Lock() + defer m.mu.Unlock() + if m.err != nil { + return m.err + } + m.ended++ + return nil +} + +func (m *memorySessions) started() []Claims { + m.mu.Lock() + defer m.mu.Unlock() + return slices.Clone(m.claims) +} + +type harness struct { + auth *Auth + provider *fakeProvider + sessions *memorySessions + router http.Handler + logs *syncBuffer +} + +// syncBuffer is a bytes.Buffer safe for concurrent log writes. +type syncBuffer struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (b *syncBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *syncBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} + +func newHarness(t *testing.T) *harness { + t.Helper() + p := newFakeProvider(t) + sessions := &memorySessions{} + logs := &syncBuffer{} + a, err := New(context.Background(), Config{ + Issuer: p.issuer(), + ClientID: testClientID, + ClientSecret: testClientSecret, + PublicURL: testPublicURL, + SecretKey: []byte("server-secret-key"), + AllowedGroups: testAllowedGroups, + }, sessions, slog.New(slog.NewTextHandler(logs, nil))) + if err != nil { + t.Fatal(err) + } + r := chi.NewRouter() + a.Mount(r) + return &harness{auth: a, provider: p, sessions: sessions, router: r, logs: logs} +} + +func (h *harness) get(target string, cookies ...*http.Cookie) *http.Response { + req := httptest.NewRequest(http.MethodGet, target, nil) + for _, c := range cookies { + req.AddCookie(c) + } + rec := httptest.NewRecorder() + h.router.ServeHTTP(rec, req) + return rec.Result() +} + +// login is a started login: the authorize URL's query and the login cookie. +type login struct { + query url.Values + cookie *http.Cookie +} + +func (h *harness) login(t *testing.T, target string) login { + t.Helper() + resp := h.get(target) + if resp.StatusCode != http.StatusFound { + t.Fatalf("GET %s status = %d, want 302", target, resp.StatusCode) + } + loc, err := url.Parse(resp.Header.Get("Location")) + if err != nil { + t.Fatal(err) + } + if got, want := loc.Scheme+"://"+loc.Host+loc.Path, h.provider.issuer()+"/authorize"; got != want { + t.Fatalf("redirect to %s, want %s", got, want) + } + var cookie *http.Cookie + for _, c := range resp.Cookies() { + if c.Name == loginCookieName { + cookie = c + } + } + if cookie == nil { + t.Fatalf("login set no %s cookie", loginCookieName) + } + return login{query: loc.Query(), cookie: cookie} +} + +// callback completes l at the provider with idToken and calls the callback. +func (h *harness) callback(l login, state, idToken string) *http.Response { + code := h.provider.authorize(l.query.Get("code_challenge"), idToken) + q := url.Values{"code": {code}, "state": {state}} + return h.get(CallbackPath+"?"+q.Encode(), l.cookie) +} + +func TestLoginRedirect(t *testing.T) { + t.Parallel() + h := newHarness(t) + l := h.login(t, LoginPath+"?return=/plan") + + want := map[string]string{ + "client_id": testClientID, + "response_type": "code", + "scope": "openid profile email groups offline_access", + "redirect_uri": "https://meals.example.com/auth/callback", + "code_challenge_method": "S256", + } + for k, v := range want { + if got := l.query.Get(k); got != v { + t.Errorf("authorize %s = %q, want %q", k, got, v) + } + } + for _, k := range []string{"state", "nonce", "code_challenge"} { + if l.query.Get(k) == "" { + t.Errorf("authorize URL has no %s", k) + } + } + + c := l.cookie + if !c.HttpOnly || !c.Secure || c.SameSite != http.SameSiteLaxMode || c.Path != "/" { + t.Errorf("login cookie HttpOnly=%v Secure=%v SameSite=%v Path=%q, want HttpOnly, Secure, Lax, /", + c.HttpOnly, c.Secure, c.SameSite, c.Path) + } + if c.MaxAge <= 0 || c.MaxAge > 600 { + t.Errorf("login cookie MaxAge = %d, want 1-600 seconds", c.MaxAge) + } +} + +func TestLoginStateIsFreshEachTime(t *testing.T) { + t.Parallel() + h := newHarness(t) + a, b := h.login(t, LoginPath), h.login(t, LoginPath) + for _, k := range []string{"state", "nonce", "code_challenge"} { + if a.query.Get(k) == b.query.Get(k) { + t.Errorf("%s repeated across logins", k) + } + } +} + +func TestCallbackSuccess(t *testing.T) { + t.Parallel() + h := newHarness(t) + l := h.login(t, LoginPath+"?return="+url.QueryEscape("/plan?week=2026-W40")) + + resp := h.callback(l, l.query.Get("state"), h.provider.signRS256(h.provider.claims(l.query.Get("nonce")))) + + if resp.StatusCode != http.StatusSeeOther { + t.Fatalf("callback status = %d, want 303", resp.StatusCode) + } + if got := resp.Header.Get("Location"); got != "/plan?week=2026-W40" { + t.Errorf("callback redirect = %q, want the return path", got) + } + started := h.sessions.started() + want := Claims{ + Subject: "user-123", + Email: "cook@example.com", + EmailVerified: true, + Name: "Home Cook", + PreferredUsername: "cook", + Groups: []string{"mealquest-admin"}, + } + if len(started) != 1 || started[0].Subject != want.Subject || started[0].Email != want.Email || + started[0].Name != want.Name || started[0].PreferredUsername != want.PreferredUsername || + !started[0].EmailVerified || !slices.Equal(started[0].Groups, want.Groups) { + t.Errorf("session started with %+v, want [%+v]", started, want) + } + assertLoginCookieCleared(t, resp) +} + +func TestCallbackDefaultReturnPath(t *testing.T) { + t.Parallel() + h := newHarness(t) + l := h.login(t, LoginPath) + resp := h.callback(l, l.query.Get("state"), h.provider.signRS256(h.provider.claims(l.query.Get("nonce")))) + if resp.StatusCode != http.StatusSeeOther || resp.Header.Get("Location") != "/" { + t.Errorf("callback = %d to %q, want 303 to /", resp.StatusCode, resp.Header.Get("Location")) + } +} + +func TestCallbackRejected(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + // state returns the state sent to the callback. + state func(l login) string + // token returns the ID token the provider issues. + token func(p *fakeProvider, l login) string + wantStatus int + }{ + { + name: "mismatched state", + state: func(login) string { return "attacker-state" }, + wantStatus: http.StatusBadRequest, + }, + { + name: "wrong nonce", + token: func(p *fakeProvider, _ login) string { + return p.signRS256(p.claims("other-nonce")) + }, + wantStatus: http.StatusUnauthorized, + }, + { + name: "missing nonce", + token: func(p *fakeProvider, _ login) string { + c := p.claims("") + delete(c, "nonce") + return p.signRS256(c) + }, + wantStatus: http.StatusUnauthorized, + }, + { + name: "HS256-signed token", + token: func(p *fakeProvider, l login) string { + return p.signHS256(p.claims(l.query.Get("nonce"))) + }, + wantStatus: http.StatusUnauthorized, + }, + { + name: "expired token", + token: func(p *fakeProvider, l login) string { + c := p.claims(l.query.Get("nonce")) + c["iat"] = time.Now().Add(-2 * time.Hour).Unix() + c["exp"] = time.Now().Add(-time.Hour).Unix() + return p.signRS256(c) + }, + wantStatus: http.StatusUnauthorized, + }, + { + name: "wrong issuer", + token: func(p *fakeProvider, l login) string { + c := p.claims(l.query.Get("nonce")) + c["iss"] = "https://evil.example.com" + return p.signRS256(c) + }, + wantStatus: http.StatusUnauthorized, + }, + { + name: "wrong audience", + token: func(p *fakeProvider, l login) string { + c := p.claims(l.query.Get("nonce")) + c["aud"] = "another-client" + return p.signRS256(c) + }, + wantStatus: http.StatusUnauthorized, + }, + { + name: "signed by an unknown key", + token: func(p *fakeProvider, l login) string { + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + p.t.Fatal(err) + } + return signRS256(p.t, key, p.claims(l.query.Get("nonce"))) + }, + wantStatus: http.StatusUnauthorized, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + h := newHarness(t) + l := h.login(t, LoginPath) + state := l.query.Get("state") + if tc.state != nil { + state = tc.state(l) + } + token := h.provider.signRS256(h.provider.claims(l.query.Get("nonce"))) + if tc.token != nil { + token = tc.token(h.provider, l) + } + + resp := h.callback(l, state, token) + + if resp.StatusCode != tc.wantStatus { + t.Errorf("callback status = %d, want %d", resp.StatusCode, tc.wantStatus) + } + if n := len(h.sessions.started()); n != 0 { + t.Errorf("%d sessions started, want none", n) + } + assertLoginCookieCleared(t, resp) + }) + } +} + +func TestCallbackWithoutLoginCookie(t *testing.T) { + t.Parallel() + h := newHarness(t) + l := h.login(t, LoginPath) + l.cookie = &http.Cookie{Name: "unrelated", Value: "x"} + resp := h.callback(l, l.query.Get("state"), h.provider.signRS256(h.provider.claims(l.query.Get("nonce")))) + if resp.StatusCode != http.StatusBadRequest { + t.Errorf("callback status = %d, want 400", resp.StatusCode) + } + if n := len(h.sessions.started()); n != 0 { + t.Errorf("%d sessions started, want none", n) + } +} + +func TestCallbackTamperedLoginCookie(t *testing.T) { + t.Parallel() + h := newHarness(t) + l := h.login(t, LoginPath) + payload, sig, _ := strings.Cut(l.cookie.Value, ".") + data, err := base64.RawURLEncoding.DecodeString(payload) + if err != nil { + t.Fatal(err) + } + var s loginState + if err := json.Unmarshal(data, &s); err != nil { + t.Fatal(err) + } + s.Return = "/somewhere-else" + data, err = json.Marshal(s) + if err != nil { + t.Fatal(err) + } + l.cookie = &http.Cookie{Name: loginCookieName, Value: b64(data) + "." + sig} + + resp := h.callback(l, l.query.Get("state"), h.provider.signRS256(h.provider.claims(l.query.Get("nonce")))) + if resp.StatusCode != http.StatusBadRequest { + t.Errorf("callback status = %d, want 400", resp.StatusCode) + } +} + +// The provider's error and error_description are untrusted text and never +// reach the logs; only the fixed category does. +func TestCallbackProviderError(t *testing.T) { + t.Parallel() + const ( + errorMarker = "provider-error-marker" + descriptionMarker = "description-marker-" + testClientSecret + ) + h := newHarness(t) + l := h.login(t, LoginPath) + q := url.Values{ + "error": {errorMarker}, + "error_description": {descriptionMarker}, + "state": {l.query.Get("state")}, + } + resp := h.get(CallbackPath+"?"+q.Encode(), l.cookie) + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("callback status = %d, want 401", resp.StatusCode) + } + assertLoginCookieCleared(t, resp) + if n := len(h.sessions.started()); n != 0 { + t.Errorf("started %d sessions, want 0", n) + } + logs := h.logs.String() + for _, marker := range []string{errorMarker, descriptionMarker} { + if strings.Contains(logs, marker) { + t.Errorf("logs contain provider text %q: %s", marker, logs) + } + } + if !strings.Contains(logs, "identity provider returned an authorization error") { + t.Errorf("logs = %q, want the authorization error category", logs) + } +} + +func TestCallbackSessionError(t *testing.T) { + t.Parallel() + h := newHarness(t) + h.sessions.err = errors.New("database down") + l := h.login(t, LoginPath) + resp := h.callback(l, l.query.Get("state"), h.provider.signRS256(h.provider.claims(l.query.Get("nonce")))) + if resp.StatusCode != http.StatusInternalServerError { + t.Errorf("callback status = %d, want 500", resp.StatusCode) + } +} + +// A failed code exchange is logged by category and status only: the token +// endpoint's body and error_description may echo credentials. +func TestCallbackExchangeFailureDoesNotLogProviderResponse(t *testing.T) { + t.Parallel() + const leaked = "leaked-" + testClientSecret + tests := []struct { + name string + reply tokenReply + wantLogs string + }{ + { + name: "error response", + reply: tokenReply{http.StatusBadRequest, `{"error":"invalid_grant","error_description":"` + leaked + `"}`}, + wantLogs: "HTTP 400", + }, + { + name: "unparseable error response", + reply: tokenReply{http.StatusInternalServerError, leaked}, + wantLogs: "HTTP 500", + }, + { + name: "success without access token", + reply: tokenReply{http.StatusOK, `{"id_token":"` + leaked + `"}`}, + wantLogs: "unusable response", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + h := newHarness(t) + h.provider.mu.Lock() + h.provider.tokenReply = &tc.reply + h.provider.mu.Unlock() + l := h.login(t, LoginPath) + + resp := h.callback(l, l.query.Get("state"), "unused") + + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("callback status = %d, want 401", resp.StatusCode) + } + logs := h.logs.String() + if strings.Contains(logs, leaked) { + t.Errorf("logs contain the provider response: %s", logs) + } + if !strings.Contains(logs, tc.wantLogs) { + t.Errorf("logs = %q, want %q", logs, tc.wantLogs) + } + }) + } +} + +func TestCallbackKeySetFailureDoesNotLogProviderResponse(t *testing.T) { + t.Parallel() + const sentinel = "sentinel-jwks-body-5c1e" + tests := []struct { + name string + reply tokenReply + }{ + {"error response", tokenReply{http.StatusInternalServerError, sentinel}}, + {"malformed success", tokenReply{http.StatusOK, `{"keys":"` + sentinel + `"`}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + h := newHarness(t) + h.provider.mu.Lock() + h.provider.jwksReply = &tc.reply + h.provider.mu.Unlock() + + // The returned error, checked through verify directly. + l := h.login(t, LoginPath) + req := httptest.NewRequest(http.MethodGet, CallbackPath, nil) + req.AddCookie(l.cookie) + state, err := h.auth.cookie.read(req, time.Now()) + if err != nil { + t.Fatal(err) + } + code := h.provider.authorize(l.query.Get("code_challenge"), h.provider.signRS256(h.provider.claims(l.query.Get("nonce")))) + _, err = h.auth.verify(context.Background(), code, state) + if err == nil { + t.Fatal("verify succeeded with an unusable key set") + } + if strings.Contains(err.Error(), sentinel) { + t.Errorf("verify error contains the JWKS response: %v", err) + } + + // The logs and response, through the callback. + l = h.login(t, LoginPath) + resp := h.callback(l, l.query.Get("state"), h.provider.signRS256(h.provider.claims(l.query.Get("nonce")))) + + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("callback status = %d, want 401", resp.StatusCode) + } + body, _ := io.ReadAll(resp.Body) + if strings.Contains(string(body), sentinel) { + t.Errorf("response contains the JWKS response: %s", body) + } + logs := h.logs.String() + if strings.Contains(logs, sentinel) { + t.Errorf("logs contain the JWKS response: %s", logs) + } + if want := "token or provider key set rejected"; !strings.Contains(logs, want) { + t.Errorf("logs = %q, want %q", logs, want) + } + if len(h.sessions.started()) != 0 { + t.Error("a session started") + } + }) + } +} + +func TestLoginCookieExpires(t *testing.T) { + t.Parallel() + c := newLoginCookie([]byte("server-secret-key")) + now := time.Now() + rec := httptest.NewRecorder() + if err := c.set(rec, loginState{State: "s", Nonce: "n", Verifier: "v", Return: "/"}, now); err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodGet, CallbackPath, nil) + req.AddCookie(rec.Result().Cookies()[0]) + + if _, err := c.read(req, now.Add(loginCookieTTL-time.Second)); err != nil { + t.Errorf("read before expiry: %v", err) + } + if _, err := c.read(req, now.Add(loginCookieTTL)); err == nil { + t.Error("read at expiry succeeded") + } +} + +func TestLoginRejectsOffOriginReturn(t *testing.T) { + t.Parallel() + h := newHarness(t) + for _, ret := range []string{ + "https://evil.example.com/", + "http://meals.example.com/plan", + "//evil.example.com/", + "/\\evil.example.com", + "\\\\evil.example.com", + "javascript:alert(1)", + "plan", + "/plan\r\nSet-Cookie: x=y", + } { + resp := h.get(LoginPath + "?return=" + url.QueryEscape(ret)) + if resp.StatusCode != http.StatusBadRequest { + t.Errorf("return=%q: status = %d, want 400", ret, resp.StatusCode) + } + if loc := resp.Header.Get("Location"); loc != "" { + t.Errorf("return=%q: redirected to %q", ret, loc) + } + if len(resp.Cookies()) != 0 { + t.Errorf("return=%q: set cookies %v", ret, resp.Cookies()) + } + } +} + +func TestNewFailsWhenDiscoveryFails(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.NotFoundHandler()) + t.Cleanup(srv.Close) + _, err := New(context.Background(), Config{ + Issuer: srv.URL, + ClientID: testClientID, + ClientSecret: testClientSecret, + PublicURL: testPublicURL, + SecretKey: []byte("k"), + AllowedGroups: testAllowedGroups, + }, &memorySessions{}, slog.New(slog.DiscardHandler)) + if err == nil { + t.Fatal("New succeeded without a discovery document") + } +} + +func TestNewDiscoveryErrorOmitsProviderBody(t *testing.T) { + t.Parallel() + const sentinel = "sentinel-discovery-body-7f3a" + for _, status := range []int{http.StatusNotFound, http.StatusInternalServerError} { + t.Run(http.StatusText(status), func(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(status) + io.WriteString(w, sentinel) + })) + t.Cleanup(srv.Close) + logs := &syncBuffer{} + _, err := New(context.Background(), Config{ + Issuer: srv.URL, + ClientID: testClientID, + ClientSecret: testClientSecret, + PublicURL: testPublicURL, + SecretKey: []byte("k"), + AllowedGroups: testAllowedGroups, + }, &memorySessions{}, slog.New(slog.NewTextHandler(logs, nil))) + if err == nil { + t.Fatal("New succeeded without a discovery document") + } + // cmd/server logs the returned error at startup. + slog.New(slog.NewTextHandler(logs, nil)).Error("startup failed", "err", err) + if strings.Contains(logs.String(), sentinel) { + t.Errorf("logs contain the provider response body: %s", logs) + } + if want := fmt.Sprintf("auth: OIDC discovery: provider answered HTTP %d", status); err.Error() != want { + t.Errorf("error = %q, want %q", err, want) + } + }) + } +} + +func TestNewFailsOnIssuerMismatch(t *testing.T) { + t.Parallel() + p := newFakeProvider(t) + // Discovery trims the trailing slash, so this fetches the provider's + // document, which reports the issuer without it. + _, err := New(context.Background(), Config{ + Issuer: p.issuer() + "/", + ClientID: testClientID, + ClientSecret: testClientSecret, + PublicURL: testPublicURL, + SecretKey: []byte("k"), + AllowedGroups: testAllowedGroups, + }, &memorySessions{}, slog.New(slog.DiscardHandler)) + if err == nil { + t.Fatal("New accepted a provider reporting a different issuer") + } +} + +func assertLoginCookieCleared(t *testing.T, resp *http.Response) { + t.Helper() + for _, c := range resp.Cookies() { + if c.Name == loginCookieName && c.MaxAge < 0 { + return + } + } + t.Errorf("callback did not clear the %s cookie", loginCookieName) +} + +func TestNewRejectsRemoteHTTPIssuer(t *testing.T) { + t.Parallel() + for _, issuer := range []string{"http://auth.example.com/application/o/mealquest/", "http://192.0.2.10"} { + _, err := New(context.Background(), Config{ + Issuer: issuer, + ClientID: testClientID, + ClientSecret: testClientSecret, + PublicURL: testPublicURL, + SecretKey: []byte("k"), + AllowedGroups: testAllowedGroups, + }, &memorySessions{}, slog.New(slog.DiscardHandler)) + if err == nil || !strings.Contains(err.Error(), "issuer") { + t.Errorf("New(issuer %q) error = %v, want issuer rejected", issuer, err) + } + } +} + +func TestNewRejectsRemoteHTTPEndpoint(t *testing.T) { + t.Parallel() + tests := []struct { + field, name string + }{ + {"authorization_endpoint", "authorization endpoint"}, + {"token_endpoint", "token endpoint"}, + {"jwks_uri", "JWKS endpoint"}, + {"end_session_endpoint", "end session endpoint"}, + {"userinfo_endpoint", "userinfo endpoint"}, + } + for _, tc := range tests { + t.Run(tc.field, func(t *testing.T) { + t.Parallel() + p := newFakeProvider(t) + p.discoveryOverrides = map[string]any{tc.field: "http://auth.example.com/" + tc.field} + _, err := New(context.Background(), Config{ + Issuer: p.issuer(), + ClientID: testClientID, + ClientSecret: testClientSecret, + PublicURL: testPublicURL, + SecretKey: []byte("k"), + AllowedGroups: testAllowedGroups, + }, &memorySessions{}, slog.New(slog.DiscardHandler)) + if err == nil || !strings.Contains(err.Error(), tc.name) { + t.Errorf("New error = %v, want %s rejected", err, tc.name) + } + }) + } +} + +// A token endpoint on https:// that redirects the code exchange to plain +// http:// must not receive a follow-up request: Go would resend the client +// secret in the Authorization header. The plaintext target is loopback, so +// only the downgrade rule stops it. +func TestCallbackRefusesTokenRedirectToHTTP(t *testing.T) { + t.Parallel() + for _, status := range []int{http.StatusTemporaryRedirect, http.StatusPermanentRedirect} { + t.Run(http.StatusText(status), func(t *testing.T) { + t.Parallel() + var plaintextHits atomic.Int32 + plaintext := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + plaintextHits.Add(1) + http.Error(w, "unreachable", http.StatusTeapot) + })) + t.Cleanup(plaintext.Close) + var tlsHits atomic.Int32 + tlsToken := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + tlsHits.Add(1) + http.Redirect(w, r, plaintext.URL+"/token", status) + })) + t.Cleanup(tlsToken.Close) + + p := newFakeProvider(t) + p.discoveryOverrides = map[string]any{"token_endpoint": tlsToken.URL + "/token"} + sessions := &memorySessions{} + logs := &syncBuffer{} + a, err := New(context.Background(), Config{ + Issuer: p.issuer(), + ClientID: testClientID, + ClientSecret: testClientSecret, + PublicURL: testPublicURL, + SecretKey: []byte("server-secret-key"), + AllowedGroups: testAllowedGroups, + }, sessions, slog.New(slog.NewTextHandler(logs, nil))) + if err != nil { + t.Fatal(err) + } + // Trust the test server's certificate; the redirect policy stays. + a.client.Transport = tlsToken.Client().Transport + r := chi.NewRouter() + a.Mount(r) + h := &harness{provider: p, sessions: sessions, router: r, logs: logs} + l := h.login(t, LoginPath) + + resp := h.callback(l, l.query.Get("state"), "unused") + + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("callback status = %d, want 401", resp.StatusCode) + } + if n := tlsHits.Load(); n != 1 { + t.Errorf("https token endpoint hit %d times, want 1", n) + } + if n := plaintextHits.Load(); n != 0 { + t.Errorf("plaintext endpoint hit %d times, want 0", n) + } + if n := len(sessions.started()); n != 0 { + t.Errorf("started %d sessions, want 0", n) + } + if got := logs.String(); !strings.Contains(got, "downgrade") { + t.Errorf("logs = %q, want the downgrade refusal", got) + } + }) + } +} + +func TestCheckRedirect(t *testing.T) { + t.Parallel() + request := func(raw string) *http.Request { + req, err := http.NewRequest(http.MethodPost, raw, nil) + if err != nil { + t.Fatal(err) + } + return req + } + tests := []struct { + from, to string + ok bool + }{ + {"https://auth.example.com/token", "https://auth2.example.com/token", true}, + {"http://127.0.0.1:9000/token", "http://localhost:9001/token", true}, + {"http://127.0.0.1:9000/token", "https://auth.example.com/token", true}, + {"https://auth.example.com/token", "http://auth.example.com/token", false}, + {"https://127.0.0.1:9000/token", "http://127.0.0.1:9001/token", false}, + {"http://127.0.0.1:9000/token", "http://auth.example.com/token", false}, + {"https://auth.example.com/token", "ftp://auth.example.com/token", false}, + } + for _, tc := range tests { + err := checkRedirect(request(tc.to), []*http.Request{request(tc.from)}) + if (err == nil) != tc.ok { + t.Errorf("redirect %s -> %s: error = %v, want ok=%v", tc.from, tc.to, err, tc.ok) + } + } + via := make([]*http.Request, maxRedirects) + for i := range via { + via[i] = request("https://auth.example.com/token") + } + if err := checkRedirect(request("https://auth.example.com/token"), via); err == nil { + t.Errorf("redirect after %d hops: error = nil, want limit", maxRedirects) + } +} diff --git a/internal/auth/cookie.go b/internal/auth/cookie.go new file mode 100644 index 0000000..4be2d7d --- /dev/null +++ b/internal/auth/cookie.go @@ -0,0 +1,124 @@ +package auth + +import ( + "crypto/hmac" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/url" + "strings" + "time" +) + +const ( + loginCookieName = "__Host-mealquest_login" + loginCookieTTL = 10 * time.Minute + // loginCookieKeyLabel separates the login cookie's signing key from other + // keys derived from the same server secret. + loginCookieKeyLabel = "mealquest auth login cookie v1" +) + +// loginState travels from login to callback in the signed login cookie. +type loginState struct { + State string `json:"s"` + Nonce string `json:"n"` + Verifier string `json:"v"` + Return string `json:"r"` + Expires int64 `json:"e"` // Unix seconds +} + +// loginCookie signs and verifies the login cookie with HMAC-SHA256. +type loginCookie struct { + key []byte +} + +func newLoginCookie(secretKey []byte) loginCookie { + mac := hmac.New(sha256.New, secretKey) + mac.Write([]byte(loginCookieKeyLabel)) + return loginCookie{key: mac.Sum(nil)} +} + +func (c loginCookie) sign(payload string) string { + mac := hmac.New(sha256.New, c.key) + mac.Write([]byte(payload)) + return base64.RawURLEncoding.EncodeToString(mac.Sum(nil)) +} + +func (c loginCookie) set(w http.ResponseWriter, s loginState, now time.Time) error { + s.Expires = now.Add(loginCookieTTL).Unix() + data, err := json.Marshal(s) + if err != nil { + return err + } + payload := base64.RawURLEncoding.EncodeToString(data) + http.SetCookie(w, &http.Cookie{ + Name: loginCookieName, + Value: payload + "." + c.sign(payload), + Path: "/", + MaxAge: int(loginCookieTTL / time.Second), + Secure: true, + HttpOnly: true, + // Lax so the cookie rides the provider's top-level redirect back. + SameSite: http.SameSiteLaxMode, + }) + return nil +} + +func (c loginCookie) clear(w http.ResponseWriter) { + http.SetCookie(w, &http.Cookie{ + Name: loginCookieName, + Path: "/", + MaxAge: -1, + Secure: true, + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + }) +} + +func (c loginCookie) read(r *http.Request, now time.Time) (loginState, error) { + cookie, err := r.Cookie(loginCookieName) + if err != nil { + return loginState{}, errors.New("login cookie missing") + } + payload, sig, ok := strings.Cut(cookie.Value, ".") + if !ok || !hmac.Equal([]byte(sig), []byte(c.sign(payload))) { + return loginState{}, errors.New("login cookie signature invalid") + } + data, err := base64.RawURLEncoding.DecodeString(payload) + if err != nil { + return loginState{}, fmt.Errorf("login cookie encoding: %w", err) + } + var s loginState + if err := json.Unmarshal(data, &s); err != nil { + return loginState{}, fmt.Errorf("login cookie payload: %w", err) + } + if now.Unix() >= s.Expires { + return loginState{}, errors.New("login cookie expired") + } + return s, nil +} + +// returnPath validates the optional post-login return path. Empty means "/". +// Only same-origin absolute paths are allowed: no scheme, host, protocol- +// relative "//" prefix, backslashes or control characters. +func returnPath(raw string) (string, error) { + if raw == "" { + return "/", nil + } + if !strings.HasPrefix(raw, "/") || strings.HasPrefix(raw, "//") || strings.ContainsRune(raw, '\\') { + return "", errors.New("return path must be a same-origin path") + } + for _, c := range raw { + if c < 0x20 || c == 0x7f { + return "", errors.New("return path contains a control character") + } + } + u, err := url.Parse(raw) + if err != nil || u.Scheme != "" || u.Host != "" || u.User != nil { + return "", errors.New("return path must be a same-origin path") + } + return raw, nil +} diff --git a/internal/auth/gate_test.go b/internal/auth/gate_test.go new file mode 100644 index 0000000..0f18362 --- /dev/null +++ b/internal/auth/gate_test.go @@ -0,0 +1,264 @@ +package auth + +import ( + "context" + "errors" + "io" + "log/slog" + "net/http" + "net/url" + "slices" + "strings" + "testing" +) + +// signInWith completes a login whose ID token has claims edited by edit. +func (h *harness) signInWith(t *testing.T, edit func(claims map[string]any)) *http.Response { + t.Helper() + l := h.login(t, LoginPath) + c := h.provider.claims(l.query.Get("nonce")) + edit(c) + return h.callback(l, l.query.Get("state"), h.provider.signRS256(c)) +} + +// assertRefused checks resp is the refused-sign-in page and starts no +// session. +func assertRefused(t *testing.T, resp *http.Response) { + t.Helper() + if resp.StatusCode != http.StatusForbidden { + t.Fatalf("callback status = %d, want 403", resp.StatusCode) + } + if ct := resp.Header.Get("Content-Type"); ct != "text/html; charset=utf-8" { + t.Errorf("Content-Type = %q, want text/html; charset=utf-8", ct) + } + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{ + "This kitchen is for the household only", + `
`, + "Sign out", + } { + if !strings.Contains(string(body), want) { + t.Errorf("refused page lacks %q:\n%s", want, body) + } + } + for _, c := range resp.Cookies() { + // Clearing a session cookie (negative Max-Age) is expected. + if (c.Name == SessionCookieName || c.Name == "test_session") && c.MaxAge >= 0 { + t.Errorf("refused sign-in set session cookie %s", c.Name) + } + } + assertLoginCookieCleared(t, resp) +} + +func TestGroupGateMember(t *testing.T) { + t.Parallel() + // Any one allowed group is enough, alongside groups that are not allowed. + h := newHarness(t) + resp := h.signInWith(t, func(c map[string]any) { c["groups"] = []string{"guests", "mealquest-family"} }) + if resp.StatusCode != http.StatusSeeOther { + t.Fatalf("callback status = %d, want 303", resp.StatusCode) + } + if started := h.sessions.started(); len(started) != 1 || !slices.Equal(started[0].Groups, []string{"guests", "mealquest-family"}) { + t.Errorf("sessions started = %+v, want one with the ID token groups", started) + } + if n := h.provider.userinfoRequests(); n != 0 { + t.Errorf("userinfo requested %d times, want 0 when the ID token has groups", n) + } +} + +func TestGroupGateRefused(t *testing.T) { + t.Parallel() + tests := []struct { + name string + groups any // ID token groups claim; nil removes it + userinfo map[string]any + }{ + {name: "non-member", groups: []string{"guests", "MEALQUEST-ADMIN"}}, + {name: "empty groups claim", groups: []string{}}, + {name: "groups claim missing from ID token and userinfo", groups: nil}, + { + name: "non-member in userinfo", + groups: nil, + userinfo: map[string]any{"sub": "user-123", "groups": []string{"guests"}}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + h := newHarness(t) + if tc.userinfo != nil { + h.provider.setUserinfo(tc.userinfo) + } + resp := h.signInWith(t, func(c map[string]any) { + if tc.groups == nil { + delete(c, "groups") + } else { + c["groups"] = tc.groups + } + }) + assertRefused(t, resp) + if started := h.sessions.started(); len(started) != 0 { + t.Errorf("sessions started = %+v, want none", started) + } + if !strings.Contains(h.logs.String(), "sign-in refused") { + t.Errorf("refusal not logged:\n%s", h.logs.String()) + } + }) + } +} + +func TestGroupGateUsesUserinfoWhenIDTokenHasNoGroups(t *testing.T) { + t.Parallel() + h := newHarness(t) + h.provider.setUserinfo(map[string]any{"sub": "user-123", "groups": []string{"mealquest-admin"}}) + resp := h.signInWith(t, func(c map[string]any) { delete(c, "groups") }) + if resp.StatusCode != http.StatusSeeOther { + t.Fatalf("callback status = %d, want 303", resp.StatusCode) + } + if started := h.sessions.started(); len(started) != 1 || !slices.Equal(started[0].Groups, []string{"mealquest-admin"}) { + t.Errorf("sessions started = %+v, want one with the userinfo groups", started) + } + if n := h.provider.userinfoRequests(); n != 1 { + t.Errorf("userinfo requested %d times, want 1", n) + } +} + +func TestGroupGateUserinfoFailures(t *testing.T) { + t.Parallel() + tests := []struct { + name string + userinfo map[string]any + }{ + {"subject mismatch", map[string]any{"sub": "someone-else", "groups": []string{"mealquest-admin"}}}, + {"malformed groups", map[string]any{"sub": "user-123", "groups": "mealquest-admin"}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + h := newHarness(t) + h.provider.setUserinfo(tc.userinfo) + resp := h.signInWith(t, func(c map[string]any) { delete(c, "groups") }) + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("callback status = %d, want 401", resp.StatusCode) + } + if started := h.sessions.started(); len(started) != 0 { + t.Errorf("sessions started = %+v, want none", started) + } + }) + } +} + +// A failed userinfo request fails the sign-in without logging the +// endpoint's response, which may echo credentials. +func TestGroupGateUserinfoErrorNotLogged(t *testing.T) { + t.Parallel() + h := newHarness(t) + h.provider.mu.Lock() + h.provider.userinfoReply = &tokenReply{status: http.StatusInternalServerError, body: `{"error":"leaked-s3cret"}`} + h.provider.mu.Unlock() + resp := h.signInWith(t, func(c map[string]any) { delete(c, "groups") }) + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("callback status = %d, want 401", resp.StatusCode) + } + if started := h.sessions.started(); len(started) != 0 { + t.Errorf("sessions started = %+v, want none", started) + } + logs := h.logs.String() + if !strings.Contains(logs, "userinfo") || strings.Contains(logs, "leaked-s3cret") { + t.Errorf("logs = %s, want the userinfo failure without the response body", logs) + } +} + +// Membership is checked at every login: after removal from the group, the +// next login is refused and starts no session. +func TestGroupGateRemovedMemberRefusedAtNextLogin(t *testing.T) { + t.Parallel() + h := newSessionHarness(t) + h.signIn(t) + if n := len(h.rows(t)); n != 1 { + t.Fatalf("sessions after first login = %d, want 1", n) + } + + resp := h.signInWith(t, func(c map[string]any) { c["groups"] = []string{"former-household"} }) + assertRefused(t, resp) + if n := len(h.rows(t)); n != 1 { + t.Errorf("sessions after refused login = %d, want still 1", n) + } +} + +// A refused sign-in from a browser that still holds a session ends that +// session and clears its cookie. +func TestGroupGateRefusalEndsExistingSession(t *testing.T) { + t.Parallel() + h := newSessionHarness(t) + _, cookie := h.signIn(t) + + l := h.login(t, LoginPath) + c := h.provider.claims(l.query.Get("nonce")) + c["groups"] = []string{"former-household"} + code := h.provider.authorize(l.query.Get("code_challenge"), h.provider.signRS256(c)) + q := url.Values{"code": {code}, "state": {l.query.Get("state")}} + resp := h.get(CallbackPath+"?"+q.Encode(), l.cookie, cookie) + + if resp.StatusCode != http.StatusForbidden { + t.Fatalf("callback status = %d, want 403", resp.StatusCode) + } + if n := len(h.rows(t)); n != 0 { + t.Errorf("sessions after refused login = %d, want 0", n) + } + if _, err := h.use(cookie); !errors.Is(err, ErrNoSession) { + t.Errorf("old session lookup error = %v, want ErrNoSession", err) + } + cleared := false + for _, c := range resp.Cookies() { + cleared = cleared || (c.Name == SessionCookieName && c.MaxAge < 0) + } + if !cleared { + t.Error("refused sign-in did not clear the session cookie") + } +} + +func TestGroupGateRefusalEndSessionError(t *testing.T) { + t.Parallel() + h := newHarness(t) + h.sessions.err = errors.New("database down") + resp := h.signInWith(t, func(c map[string]any) { c["groups"] = []string{"guests"} }) + if resp.StatusCode != http.StatusInternalServerError { + t.Errorf("callback status = %d, want 500", resp.StatusCode) + } +} + +func TestNewRequiresAllowedGroups(t *testing.T) { + t.Parallel() + p := newFakeProvider(t) + _, err := New(context.Background(), Config{ + Issuer: p.issuer(), + ClientID: testClientID, + ClientSecret: testClientSecret, + PublicURL: testPublicURL, + SecretKey: []byte("k"), + }, &memorySessions{}, slog.New(slog.DiscardHandler)) + if err == nil || !strings.Contains(err.Error(), "allowed group") { + t.Errorf("New error = %v, want missing allowed groups named", err) + } +} + +func TestNewRequiresUserinfoEndpoint(t *testing.T) { + t.Parallel() + p := newFakeProvider(t) + p.discoveryOverrides = map[string]any{"userinfo_endpoint": nil} + _, err := New(context.Background(), Config{ + Issuer: p.issuer(), + ClientID: testClientID, + ClientSecret: testClientSecret, + PublicURL: testPublicURL, + SecretKey: []byte("k"), + AllowedGroups: testAllowedGroups, + }, &memorySessions{}, slog.New(slog.DiscardHandler)) + if err == nil || !strings.Contains(err.Error(), "userinfo endpoint") { + t.Errorf("New error = %v, want the missing userinfo endpoint named", err) + } +} diff --git a/internal/auth/refused.go b/internal/auth/refused.go new file mode 100644 index 0000000..399e705 --- /dev/null +++ b/internal/auth/refused.go @@ -0,0 +1,37 @@ +package auth + +import ( + "io" + "net/http" +) + +// refusedPage is shown when a sign-in succeeds at the provider but the +// account is not in the household's group. Signing out ends the provider +// session too, so another account can sign in. +const refusedPage = ` + + + + +MealQuest + + +
+

This kitchen is for the household only

+

The account you signed in with is not part of our kitchen.

+ + + +
+ + +` + +// refuse writes the refused-sign-in page with 403. +func (a *Auth) refuse(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.WriteHeader(http.StatusForbidden) + if _, err := io.WriteString(w, refusedPage); err != nil { + a.logger.WarnContext(r.Context(), "write refused sign-in page", "error", err) + } +} diff --git a/internal/auth/session.go b/internal/auth/session.go new file mode 100644 index 0000000..89d061e --- /dev/null +++ b/internal/auth/session.go @@ -0,0 +1,238 @@ +package auth + +import ( + "crypto/rand" + "crypto/sha256" + "database/sql" + "encoding/base64" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "net/http" + "time" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +// SessionCookieName is the session cookie. The __Host- prefix makes the +// browser insist on Secure, Path=/ and no Domain. +const SessionCookieName = "__Host-mealquest_session" + +// sessionIDBytes is the size of the random session ID: 256 bits. +const sessionIDBytes = 32 + +// ErrNoSession means the request carries no live session: the cookie is +// missing or malformed, or its session is unknown, ended or expired. +var ErrNoSession = errors.New("auth: no valid session") + +// Session is a signed-in household member's server-side session. +type Session struct { + Subject string + Email string + Name string + Groups []string + CSRFToken string + CreatedAt time.Time + // LastSeenAt is the last time the session was used. + LastSeenAt time.Time + // ExpiresAt is when the session ends unless used again first. + ExpiresAt time.Time +} + +// SessionManager keeps sessions in the sessions table. The cookie holds only +// the random session ID; the table holds only its SHA-256 hash. A session +// ends after idle without use and always max after it was created. +type SessionManager struct { + db *store.Store + idle time.Duration + max time.Duration + // now is the clock; tests replace it. + now func() time.Time +} + +// NewSessionManager returns a SessionManager over db. idle and max must be +// positive with max >= idle. +func NewSessionManager(db *store.Store, idle, max time.Duration) (*SessionManager, error) { + switch { + case db == nil: + return nil, errors.New("auth: session store is required") + case idle <= 0 || max <= 0: + return nil, errors.New("auth: session idle and max lifetimes must be positive") + case max < idle: + return nil, errors.New("auth: session max lifetime must be at least the idle lifetime") + } + return &SessionManager{db: db, idle: idle, max: max, now: time.Now}, nil +} + +// StartSession creates a session for claims and sets the session cookie. +func (m *SessionManager) StartSession(w http.ResponseWriter, r *http.Request, claims Claims) error { + if claims.Subject == "" { + return errors.New("auth: session needs a subject") + } + id, csrf := randomToken(), randomToken() + groups := claims.Groups + if groups == nil { + groups = []string{} + } + groupsJSON, err := json.Marshal(groups) + if err != nil { + return fmt.Errorf("auth: encode session groups: %w", err) + } + + now := m.timestamp() + if _, err := m.db.Writer.ExecContext(r.Context(), ` + INSERT INTO sessions (id_hash, subject, email, name, groups, csrf_token, created_at, last_seen_at, expires_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, + hashSessionID(id), claims.Subject, claims.Email, claims.Name, string(groupsJSON), csrf, + now, now, m.expiry(now.Time, now.Time), + ); err != nil { + return fmt.Errorf("auth: create session: %w", err) + } + + // The cookie outlives idle gaps so a returning device presents its ID; + // the server decides whether the session is still live. Max-Age is whole + // seconds, rounded up so the cookie never ends before the absolute limit. + http.SetCookie(w, sessionCookie(id, cookieMaxAge(m.max))) + return nil +} + +// cookieMaxAge is lifetime in whole seconds, rounded up. It rounds the +// quotient rather than adding to lifetime, which would overflow near the +// largest time.Duration. +func cookieMaxAge(lifetime time.Duration) int { + seconds := lifetime / time.Second + if lifetime%time.Second != 0 { + seconds++ + } + return int(seconds) +} + +// SessionFor returns the live session r carries and slides its last use to +// now. It returns ErrNoSession when there is none; an expired session is +// deleted. +func (m *SessionManager) SessionFor(r *http.Request) (Session, error) { + idHash, err := sessionIDHash(r) + if err != nil { + return Session{}, err + } + now := m.timestamp() + + var s Session + live := false + err = m.db.WithTx(r.Context(), func(tx *sql.Tx) error { + var groupsJSON string + var created, lastSeen, expires store.Timestamp + err := tx.QueryRowContext(r.Context(), ` + SELECT subject, email, name, groups, csrf_token, created_at, last_seen_at, expires_at + FROM sessions WHERE id_hash = ?`, idHash, + ).Scan(&s.Subject, &s.Email, &s.Name, &groupsJSON, &s.CSRFToken, &created, &lastSeen, &expires) + if errors.Is(err, sql.ErrNoRows) { + return nil + } + if err != nil { + return fmt.Errorf("auth: read session: %w", err) + } + + if !now.Before(expires.Time) || now.Sub(lastSeen.Time) >= m.idle || now.Sub(created.Time) >= m.max { + if _, err := tx.ExecContext(r.Context(), `DELETE FROM sessions WHERE id_hash = ?`, idHash); err != nil { + return fmt.Errorf("auth: delete expired session: %w", err) + } + return nil + } + + if err := json.Unmarshal([]byte(groupsJSON), &s.Groups); err != nil { + return fmt.Errorf("auth: decode session groups: %w", err) + } + expiresAt := m.expiry(created.Time, now.Time) + if _, err := tx.ExecContext(r.Context(), ` + UPDATE sessions SET last_seen_at = ?, expires_at = ? WHERE id_hash = ?`, + now, expiresAt, idHash, + ); err != nil { + return fmt.Errorf("auth: slide session: %w", err) + } + s.CreatedAt, s.LastSeenAt, s.ExpiresAt = created.Time, now.Time, expiresAt.Time + live = true + return nil + }) + if err != nil { + return Session{}, err + } + if !live { + return Session{}, ErrNoSession + } + return s, nil +} + +// EndSession deletes the session r carries, if any, and clears the cookie. +func (m *SessionManager) EndSession(w http.ResponseWriter, r *http.Request) error { + http.SetCookie(w, sessionCookie("", -1)) + idHash, err := sessionIDHash(r) + if errors.Is(err, ErrNoSession) { + return nil + } + if err != nil { + return err + } + if _, err := m.db.Writer.ExecContext(r.Context(), `DELETE FROM sessions WHERE id_hash = ?`, idHash); err != nil { + return fmt.Errorf("auth: delete session: %w", err) + } + return nil +} + +// timestamp is the clock's current instant at stored precision. +func (m *SessionManager) timestamp() store.Timestamp { + return store.Timestamp{Time: m.now().UTC().Truncate(time.Millisecond)} +} + +// expiry is when a session created at created and last used at lastSeen +// ends: idle after lastSeen, but never later than max after created. +func (m *SessionManager) expiry(created, lastSeen time.Time) store.Timestamp { + idle, absolute := lastSeen.Add(m.idle), created.Add(m.max) + if absolute.Before(idle) { + return store.Timestamp{Time: absolute} + } + return store.Timestamp{Time: idle} +} + +// sessionCookie is the session cookie carrying value, with maxAge seconds; +// a negative maxAge clears it. +func sessionCookie(value string, maxAge int) *http.Cookie { + return &http.Cookie{ + Name: SessionCookieName, + Value: value, + Path: "/", + MaxAge: maxAge, + Secure: true, + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + } +} + +// sessionIDHash returns the stored hash of r's session ID, or ErrNoSession +// when the cookie is missing or is not a well-formed ID. +func sessionIDHash(r *http.Request) (string, error) { + c, err := r.Cookie(SessionCookieName) + if err != nil { + return "", ErrNoSession + } + raw, err := base64.RawURLEncoding.DecodeString(c.Value) + if err != nil || len(raw) != sessionIDBytes { + return "", ErrNoSession + } + return hashSessionID(c.Value), nil +} + +// hashSessionID is the SHA-256 hex digest of the cookie value. +func hashSessionID(id string) string { + sum := sha256.Sum256([]byte(id)) + return hex.EncodeToString(sum[:]) +} + +// randomToken returns 256 random bits, base64url-encoded without padding. +// crypto/rand.Read never returns an error; it crashes the program instead. +func randomToken() string { + b := make([]byte, sessionIDBytes) + rand.Read(b) + return base64.RawURLEncoding.EncodeToString(b) +} diff --git a/internal/auth/session_test.go b/internal/auth/session_test.go new file mode 100644 index 0000000..3f86f02 --- /dev/null +++ b/internal/auth/session_test.go @@ -0,0 +1,523 @@ +package auth + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "log/slog" + "net/http" + "net/http/httptest" + "slices" + "strings" + "testing" + "time" + + "github.com/go-chi/chi/v5" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" +) + +const ( + testIdle = 30 * 24 * time.Hour + testMax = 180 * 24 * time.Hour +) + +// testEpoch is the fake clock's starting instant. +var testEpoch = time.Date(2026, 10, 3, 18, 30, 0, 0, time.UTC) + +// sessionHarness runs the real login flow against a fake provider with a +// SessionManager over a migrated database and a fake clock. +type sessionHarness struct { + *harness + db *store.Store + sessions *SessionManager + auth *Auth + clock time.Time +} + +func newSessionHarness(t *testing.T) *sessionHarness { + t.Helper() + ctx := context.Background() + db, err := store.Open(ctx, t.TempDir()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := db.Close(); err != nil { + t.Error(err) + } + }) + if err := store.Migrate(ctx, db.Writer, migrations.FS); err != nil { + t.Fatal(err) + } + sessions, err := NewSessionManager(db, testIdle, testMax) + if err != nil { + t.Fatal(err) + } + h := &sessionHarness{db: db, sessions: sessions, clock: testEpoch} + sessions.now = func() time.Time { return h.clock } + + p := newFakeProvider(t) + logs := &syncBuffer{} + a, err := New(ctx, Config{ + Issuer: p.issuer(), + ClientID: testClientID, + ClientSecret: testClientSecret, + PublicURL: testPublicURL, + SecretKey: []byte("server-secret-key"), + AllowedGroups: testAllowedGroups, + }, sessions, slog.New(slog.NewTextHandler(logs, nil))) + if err != nil { + t.Fatal(err) + } + r := chi.NewRouter() + a.Mount(r) + h.harness = &harness{provider: p, router: r, logs: logs} + h.auth = a + return h +} + +func (h *sessionHarness) advance(d time.Duration) { h.clock = h.clock.Add(d) } + +// signIn completes a login and returns the callback response and the +// session cookie it set. +func (h *sessionHarness) signIn(t *testing.T) (*http.Response, *http.Cookie) { + t.Helper() + l := h.login(t, LoginPath) + resp := h.callback(l, l.query.Get("state"), h.provider.signRS256(h.provider.claims(l.query.Get("nonce")))) + if resp.StatusCode != http.StatusSeeOther { + t.Fatalf("callback status = %d, want 303", resp.StatusCode) + } + for _, c := range resp.Cookies() { + if c.Name == SessionCookieName { + return resp, c + } + } + t.Fatalf("callback set no %s cookie", SessionCookieName) + return nil, nil +} + +// use looks up the session cookie c names. +func (h *sessionHarness) use(c *http.Cookie) (Session, error) { + req := httptest.NewRequest(http.MethodGet, "/api/anything", nil) + if c != nil { + req.AddCookie(c) + } + return h.sessions.SessionFor(req) +} + +type sessionRow struct { + idHash, subject, email, name, groups, csrf string + created, lastSeen, expires store.Timestamp +} + +func (h *sessionHarness) rows(t *testing.T) []sessionRow { + t.Helper() + rs, err := h.db.Reader.Query(`SELECT id_hash, subject, email, name, groups, csrf_token, + created_at, last_seen_at, expires_at FROM sessions`) + if err != nil { + t.Fatal(err) + } + defer rs.Close() + var out []sessionRow + for rs.Next() { + var r sessionRow + if err := rs.Scan(&r.idHash, &r.subject, &r.email, &r.name, &r.groups, &r.csrf, + &r.created, &r.lastSeen, &r.expires); err != nil { + t.Fatal(err) + } + out = append(out, r) + } + if err := rs.Err(); err != nil { + t.Fatal(err) + } + return out +} + +// sessionSetCookie returns the raw Set-Cookie header for the session cookie. +func sessionSetCookie(t *testing.T, resp *http.Response) string { + t.Helper() + for _, v := range resp.Header.Values("Set-Cookie") { + if strings.HasPrefix(v, SessionCookieName+"=") { + return v + } + } + t.Fatalf("no Set-Cookie for %s in %q", SessionCookieName, resp.Header.Values("Set-Cookie")) + return "" +} + +// assertSessionCookieAttributes checks the attributes the __Host- prefix +// and D-004 require, on the raw header so nothing is lost in parsing. +func assertSessionCookieAttributes(t *testing.T, raw string) { + t.Helper() + attrs := map[string]bool{} + for _, a := range strings.Split(raw, ";")[1:] { + name, _, _ := strings.Cut(strings.TrimSpace(a), "=") + attrs[strings.ToLower(name)] = true + } + for _, want := range []string{"Path=/", "HttpOnly", "Secure", "SameSite=Lax"} { + if !slices.Contains(strings.Split(raw, "; "), want) { + t.Errorf("session Set-Cookie %q lacks %s", raw, want) + } + } + if attrs["domain"] { + t.Errorf("session Set-Cookie %q has a Domain", raw) + } +} + +func sha256Hex(s string) string { + sum := sha256.Sum256([]byte(s)) + return hex.EncodeToString(sum[:]) +} + +func TestCallbackStartsSession(t *testing.T) { + t.Parallel() + h := newSessionHarness(t) + resp, cookie := h.signIn(t) + + assertSessionCookieAttributes(t, sessionSetCookie(t, resp)) + if cookie.MaxAge != int(testMax/time.Second) { + t.Errorf("session cookie MaxAge = %d, want %d", cookie.MaxAge, int(testMax/time.Second)) + } + + rows := h.rows(t) + if len(rows) != 1 { + t.Fatalf("sessions has %d rows, want 1", len(rows)) + } + r := rows[0] + if len(cookie.Value) != 43 { + t.Errorf("session ID %q is %d chars, want 43 (256 bits, base64url)", cookie.Value, len(cookie.Value)) + } + if r.idHash != sha256Hex(cookie.Value) { + t.Errorf("id_hash = %q, want the SHA-256 of the cookie value", r.idHash) + } + if r.subject != "user-123" || r.email != "cook@example.com" || r.name != "Home Cook" || + r.groups != `["mealquest-admin"]` { + t.Errorf("session row = %+v, want the ID token claims", r) + } + if len(r.csrf) != 43 || r.csrf == cookie.Value { + t.Errorf("csrf_token = %q, want a separate 256-bit token", r.csrf) + } + if !r.created.Equal(testEpoch) || !r.lastSeen.Equal(testEpoch) || !r.expires.Equal(testEpoch.Add(testIdle)) { + t.Errorf("created, last_seen, expires = %v, %v, %v, want %v, %v, %v", + r.created, r.lastSeen, r.expires, testEpoch, testEpoch, testEpoch.Add(testIdle)) + } + + s, err := h.use(cookie) + if err != nil { + t.Fatalf("SessionFor = %v, want the new session", err) + } + if s.Subject != "user-123" || s.CSRFToken != r.csrf || !slices.Equal(s.Groups, []string{"mealquest-admin"}) { + t.Errorf("SessionFor = %+v", s) + } +} + +func TestStartSessionStoresOnlyTheHash(t *testing.T) { + t.Parallel() + h := newSessionHarness(t) + _, cookie := h.signIn(t) + var n int + err := h.db.Reader.QueryRow(`SELECT count(*) FROM sessions WHERE + id_hash = ?1 OR subject = ?1 OR email = ?1 OR name = ?1 OR groups = ?1 OR csrf_token = ?1`, + cookie.Value).Scan(&n) + if err != nil { + t.Fatal(err) + } + if n != 0 { + t.Error("the raw session ID is stored in the sessions table") + } +} + +func TestSessionLastSeenSlides(t *testing.T) { + t.Parallel() + h := newSessionHarness(t) + _, cookie := h.signIn(t) + + // Used every 20 days, the session outlives several idle periods. + for range 4 { + h.advance(20 * 24 * time.Hour) + s, err := h.use(cookie) + if err != nil { + t.Fatalf("at %v SessionFor = %v, want the session", h.clock, err) + } + if !s.LastSeenAt.Equal(h.clock) || !s.ExpiresAt.Equal(h.clock.Add(testIdle)) { + t.Errorf("at %v session last seen %v, expires %v, want %v, %v", + h.clock, s.LastSeenAt, s.ExpiresAt, h.clock, h.clock.Add(testIdle)) + } + r := h.rows(t)[0] + if !r.lastSeen.Equal(h.clock) || !r.expires.Equal(h.clock.Add(testIdle)) || !r.created.Equal(testEpoch) { + t.Errorf("at %v row created %v, last seen %v, expires %v", h.clock, r.created, r.lastSeen, r.expires) + } + } +} + +func TestSessionIdleExpiry(t *testing.T) { + t.Parallel() + h := newSessionHarness(t) + _, cookie := h.signIn(t) + + h.advance(testIdle - time.Millisecond) + if _, err := h.use(cookie); err != nil { + t.Fatalf("just inside the idle limit SessionFor = %v, want the session", err) + } + h.advance(testIdle) + if _, err := h.use(cookie); !errors.Is(err, ErrNoSession) { + t.Fatalf("after idle SessionFor = %v, want ErrNoSession", err) + } + if n := len(h.rows(t)); n != 0 { + t.Errorf("expired session left %d rows, want 0", n) + } + // Turning the clock back cannot revive it. + h.clock = testEpoch + if _, err := h.use(cookie); !errors.Is(err, ErrNoSession) { + t.Errorf("after deletion SessionFor = %v, want ErrNoSession", err) + } +} + +func TestSessionAbsoluteExpiry(t *testing.T) { + t.Parallel() + h := newSessionHarness(t) + _, cookie := h.signIn(t) + + for h.clock.Before(testEpoch.Add(testMax - 20*24*time.Hour)) { + h.advance(20 * 24 * time.Hour) + if _, err := h.use(cookie); err != nil { + t.Fatalf("at %v SessionFor = %v, want the session", h.clock, err) + } + } + // Last used 20 days before the absolute limit: the idle window would run + // past it, so expiry is capped there. + if r := h.rows(t)[0]; !r.expires.Equal(testEpoch.Add(testMax)) { + t.Errorf("expires_at = %v, want the absolute limit %v", r.expires, testEpoch.Add(testMax)) + } + + h.clock = testEpoch.Add(testMax - time.Millisecond) + if _, err := h.use(cookie); err != nil { + t.Fatalf("just inside the absolute limit SessionFor = %v, want the session", err) + } + h.clock = testEpoch.Add(testMax) + if _, err := h.use(cookie); !errors.Is(err, ErrNoSession) { + t.Fatalf("at the absolute limit, used 1ms ago, SessionFor = %v, want ErrNoSession", err) + } + if n := len(h.rows(t)); n != 0 { + t.Errorf("expired session left %d rows, want 0", n) + } +} + +// A max with fractional seconds rounds the cookie Max-Age up, so the browser +// keeps the cookie until the server-side absolute limit. +func TestSessionCookieOutlivesFractionalMax(t *testing.T) { + t.Parallel() + h := newSessionHarness(t) + h.sessions.idle, h.sessions.max = 1500*time.Millisecond, 2500*time.Millisecond + _, cookie := h.signIn(t) + if cookie.MaxAge != 3 { + t.Errorf("session cookie MaxAge = %d, want 3", cookie.MaxAge) + } + if r := h.rows(t)[0]; !r.expires.Equal(testEpoch.Add(1500 * time.Millisecond)) { + t.Errorf("expires_at = %v, want %v", r.expires, testEpoch.Add(1500*time.Millisecond)) + } + + h.advance(time.Second) + if _, err := h.use(cookie); err != nil { + t.Fatalf("at 1s SessionFor = %v, want the session", err) + } + h.clock = testEpoch.Add(2500*time.Millisecond - time.Millisecond) + if _, err := h.use(cookie); err != nil { + t.Fatalf("just inside the absolute limit SessionFor = %v, want the session", err) + } + h.clock = testEpoch.Add(2500 * time.Millisecond) + if _, err := h.use(cookie); !errors.Is(err, ErrNoSession) { + t.Errorf("at the absolute limit SessionFor = %v, want ErrNoSession", err) + } +} + +// A lifetime near the largest accepted duration still yields a positive, +// rounded-up cookie Max-Age rather than overflowing. +func TestSessionCookieMaxAgeNearLargestMax(t *testing.T) { + t.Parallel() + largest, err := time.ParseDuration("2562047h47m16.854s") + if err != nil { + t.Fatal(err) + } + h := newSessionHarness(t) + h.sessions.max = largest + _, cookie := h.signIn(t) + if want := 9223372037; cookie.MaxAge != want { + t.Errorf("session cookie MaxAge = %d, want %d", cookie.MaxAge, want) + } +} + +// A lifetime shortened by configuration applies to existing sessions even +// though their stored expires_at is later. +func TestSessionLifetimeChecksUseCurrentConfig(t *testing.T) { + t.Parallel() + h := newSessionHarness(t) + _, cookie := h.signIn(t) + h.sessions.idle = 24 * time.Hour + h.advance(24 * time.Hour) + if _, err := h.use(cookie); !errors.Is(err, ErrNoSession) { + t.Errorf("past the shortened idle SessionFor = %v, want ErrNoSession", err) + } + + _, cookie = h.signIn(t) + h.sessions.idle, h.sessions.max = 24*time.Hour, 48*time.Hour + for range 3 { + h.advance(12 * time.Hour) + if _, err := h.use(cookie); err != nil { + t.Fatalf("SessionFor = %v, want the session", err) + } + } + h.advance(12 * time.Hour) + if _, err := h.use(cookie); !errors.Is(err, ErrNoSession) { + t.Errorf("past the shortened max SessionFor = %v, want ErrNoSession", err) + } + + // Used recently and with expires_at still ahead, but older than a max + // shortened after that use: only the absolute check ends it. + h.sessions.idle, h.sessions.max = testIdle, testMax + _, cookie = h.signIn(t) + h.advance(10 * 24 * time.Hour) + if _, err := h.use(cookie); err != nil { + t.Fatalf("SessionFor = %v, want the session", err) + } + h.sessions.max = 10 * 24 * time.Hour + h.advance(time.Hour) + if _, err := h.use(cookie); !errors.Is(err, ErrNoSession) { + t.Errorf("past a max shortened after the last use SessionFor = %v, want ErrNoSession", err) + } +} + +func TestSessionForRejectsBadCookies(t *testing.T) { + t.Parallel() + h := newSessionHarness(t) + h.signIn(t) + for name, c := range map[string]*http.Cookie{ + "missing": nil, + "empty": {Name: SessionCookieName, Value: ""}, + "not base64": {Name: SessionCookieName, Value: strings.Repeat("!", 43)}, + "short": {Name: SessionCookieName, Value: "AAAA"}, + "unknown ID": {Name: SessionCookieName, Value: randomToken()}, + "other cookie": {Name: "mealquest_session", Value: randomToken()}, + } { + if _, err := h.use(c); !errors.Is(err, ErrNoSession) { + t.Errorf("%s: SessionFor = %v, want ErrNoSession", name, err) + } + } + if n := len(h.rows(t)); n != 1 { + t.Errorf("sessions has %d rows, want the signed-in session only", n) + } +} + +func (h *sessionHarness) logout(cookies ...*http.Cookie) *http.Response { + req := httptest.NewRequest(http.MethodPost, LogoutPath, nil) + for _, c := range cookies { + req.AddCookie(c) + } + rec := httptest.NewRecorder() + h.router.ServeHTTP(rec, req) + return rec.Result() +} + +func TestLogout(t *testing.T) { + t.Parallel() + h := newSessionHarness(t) + _, cookie := h.signIn(t) + _, other := h.signIn(t) + + resp := h.logout(cookie) + if resp.StatusCode != http.StatusSeeOther { + t.Fatalf("logout status = %d, want 303", resp.StatusCode) + } + want := h.provider.issuer() + "/end-session/" + if got := resp.Header.Get("Location"); got != want || h.auth.EndSessionURL() != want { + t.Errorf("logout redirect = %q, EndSessionURL = %q, want %q", got, h.auth.EndSessionURL(), want) + } + raw := sessionSetCookie(t, resp) + assertSessionCookieAttributes(t, raw) + if !strings.HasPrefix(raw, SessionCookieName+"=;") || !strings.Contains(raw, "Max-Age=0") { + t.Errorf("logout Set-Cookie = %q, want the cookie cleared", raw) + } + + rows := h.rows(t) + if len(rows) != 1 || rows[0].idHash != sha256Hex(other.Value) { + t.Errorf("after logout sessions = %+v, want only the other session", rows) + } + if _, err := h.use(cookie); !errors.Is(err, ErrNoSession) { + t.Errorf("logged-out cookie: SessionFor = %v, want ErrNoSession", err) + } + if _, err := h.use(other); err != nil { + t.Errorf("other session: SessionFor = %v, want it untouched", err) + } +} + +func TestLogoutWithoutSession(t *testing.T) { + t.Parallel() + h := newSessionHarness(t) + resp := h.logout() + if resp.StatusCode != http.StatusSeeOther || resp.Header.Get("Location") != h.provider.issuer()+"/end-session/" { + t.Errorf("logout = %d to %q, want 303 to the end session endpoint", resp.StatusCode, resp.Header.Get("Location")) + } + assertSessionCookieAttributes(t, sessionSetCookie(t, resp)) +} + +func TestLogoutIsPostOnly(t *testing.T) { + t.Parallel() + h := newSessionHarness(t) + _, cookie := h.signIn(t) + if resp := h.get(LogoutPath, cookie); resp.StatusCode != http.StatusMethodNotAllowed { + t.Errorf("GET %s status = %d, want 405", LogoutPath, resp.StatusCode) + } + if _, err := h.use(cookie); err != nil { + t.Errorf("after GET %s SessionFor = %v, want the session", LogoutPath, err) + } +} + +func TestLogoutSessionError(t *testing.T) { + t.Parallel() + h := newHarness(t) + h.sessions.err = errors.New("database down") + rec := httptest.NewRecorder() + h.router.ServeHTTP(rec, httptest.NewRequest(http.MethodPost, LogoutPath, nil)) + if rec.Code != http.StatusInternalServerError { + t.Errorf("logout status = %d, want 500", rec.Code) + } +} + +func TestNewRequiresEndSessionEndpoint(t *testing.T) { + t.Parallel() + p := newFakeProvider(t) + p.discoveryOverrides = map[string]any{"end_session_endpoint": nil} + _, err := New(context.Background(), Config{ + Issuer: p.issuer(), + ClientID: testClientID, + ClientSecret: testClientSecret, + PublicURL: testPublicURL, + SecretKey: []byte("k"), + AllowedGroups: testAllowedGroups, + }, &memorySessions{}, slog.New(slog.DiscardHandler)) + if err == nil || !strings.Contains(err.Error(), "end session endpoint") { + t.Errorf("New error = %v, want the missing end session endpoint named", err) + } +} + +func TestNewSessionManagerValidates(t *testing.T) { + t.Parallel() + db := &store.Store{} + for _, tc := range []struct { + name string + db *store.Store + idle, max time.Duration + }{ + {"nil store", nil, time.Hour, time.Hour}, + {"zero idle", db, 0, time.Hour}, + {"negative max", db, time.Hour, -time.Hour}, + {"max below idle", db, 2 * time.Hour, time.Hour}, + } { + if _, err := NewSessionManager(tc.db, tc.idle, tc.max); err == nil { + t.Errorf("%s: NewSessionManager succeeded", tc.name) + } + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 5673595..d3ab5e2 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -11,6 +11,9 @@ import ( "strconv" "strings" "time" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secureurl" ) // Environment variable names. @@ -20,6 +23,23 @@ const ( EnvDataDir = "MEALQUEST_DATA_DIR" EnvShutdownTimeout = "MEALQUEST_SHUTDOWN_TIMEOUT" EnvSecretKeyFile = "MEALQUEST_SECRET_KEY_FILE" + + EnvOIDCIssuer = "MEALQUEST_OIDC_ISSUER" + EnvOIDCClientID = "MEALQUEST_OIDC_CLIENT_ID" + EnvOIDCClientSecretFile = "MEALQUEST_OIDC_CLIENT_SECRET_FILE" + EnvOIDCAllowedGroups = "MEALQUEST_OIDC_ALLOWED_GROUPS" + + EnvSessionIdle = "MEALQUEST_SESSION_IDLE" + EnvSessionMax = "MEALQUEST_SESSION_MAX" + + EnvTrustedProxy = "MEALQUEST_TRUSTED_PROXY" +) + +// Session lifetime defaults (D-004): long enough that the kitchen tablet +// stays signed in. +const ( + DefaultSessionIdle = 720 * time.Hour // 30 days + DefaultSessionMax = 4320 * time.Hour // 180 days ) // Config is the validated service configuration. @@ -29,6 +49,22 @@ type Config struct { DataDir string ShutdownTimeout time.Duration SecretKey Secret + + // OIDCIssuer is the Authentik issuer URL used for discovery. + OIDCIssuer *url.URL + OIDCClientID string + OIDCClientSecret Secret + // OIDCAllowedGroups lists the groups whose members may sign in. + OIDCAllowedGroups []string + + // SessionIdle ends a session left unused this long. + SessionIdle time.Duration + // SessionMax ends every session this long after sign-in, used or not. + SessionMax time.Duration + + // TrustedProxy lists the reverse proxies whose X-Forwarded-For and + // X-Forwarded-Proto headers are honoured. Empty trusts no peer. + TrustedProxy forwarded.Trusted } // PublicOrigin returns the scheme and host of PublicURL, the only parts of it @@ -47,10 +83,23 @@ func load(lookup func(string) (string, bool)) (Config, error) { r := reader{lookup: lookup} cfg := Config{ HTTPAddr: r.listenAddr(EnvHTTPAddr), - PublicURL: r.httpURL(EnvPublicURL), + PublicURL: r.secureURL(EnvPublicURL), DataDir: r.dir(EnvDataDir), ShutdownTimeout: r.duration(EnvShutdownTimeout), SecretKey: r.secretFile(EnvSecretKeyFile), + + OIDCIssuer: r.secureURL(EnvOIDCIssuer), + OIDCClientID: r.str(EnvOIDCClientID), + OIDCClientSecret: r.secretFile(EnvOIDCClientSecretFile), + OIDCAllowedGroups: r.list(EnvOIDCAllowedGroups), + + SessionIdle: r.sessionDuration(EnvSessionIdle, DefaultSessionIdle), + SessionMax: r.sessionDuration(EnvSessionMax, DefaultSessionMax), + + TrustedProxy: r.trustedProxy(EnvTrustedProxy), + } + if cfg.SessionIdle > 0 && cfg.SessionMax > 0 && cfg.SessionMax < cfg.SessionIdle { + r.fail(EnvSessionMax, "must be at least %s (%s)", EnvSessionIdle, cfg.SessionIdle) } if len(r.problems) > 0 { return Config{}, &Error{Problems: r.problems} @@ -114,6 +163,41 @@ func (r *reader) required(name string) (string, bool) { return v, true } +func (r *reader) str(name string) string { + v, _ := r.required(name) + return v +} + +// list parses a comma-separated list. Every entry must be non-empty after +// trimming spaces. +func (r *reader) list(name string) []string { + v, ok := r.required(name) + if !ok { + return nil + } + items := strings.Split(v, ",") + for i, item := range items { + items[i] = strings.TrimSpace(item) + if items[i] == "" { + r.fail(name, "empty entry in comma-separated list %q", v) + return nil + } + } + return items +} + +// trustedProxy parses an optional comma-separated list of IP addresses and +// CIDR prefixes with forwarded.ParseTrusted. Unset or blank trusts no peer. +func (r *reader) trustedProxy(name string) forwarded.Trusted { + v, _ := r.lookup(name) + t, err := forwarded.ParseTrusted(v) + if err != nil { + r.fail(name, "%v", err) + return nil + } + return t +} + func (r *reader) listenAddr(name string) string { v, ok := r.required(name) if !ok { @@ -155,15 +239,21 @@ func validHostname(h string) bool { return true } -func (r *reader) httpURL(name string) *url.URL { +// secureURL reads an absolute URL that must pass secureurl.Check: https://, +// or http:// to a loopback host only. +func (r *reader) secureURL(name string) *url.URL { v, ok := r.required(name) if !ok { return nil } // The value is never echoed: it may carry credentials. u, err := url.Parse(v) - if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Hostname() == "" { - r.fail(name, "invalid URL (want absolute http:// or https:// URL)") + if err != nil { + r.fail(name, "invalid URL (want absolute https:// URL)") + return nil + } + if err := secureurl.Check(u); err != nil { + r.fail(name, "invalid URL: %v", err) return nil } if u.User != nil { @@ -195,6 +285,32 @@ func (r *reader) duration(name string) time.Duration { if !ok { return 0 } + return r.parseDuration(name, v) +} + +// optionalDuration returns def when name is unset. A set value, even an empty +// one, must be a valid positive duration. +func (r *reader) optionalDuration(name string, def time.Duration) time.Duration { + v, ok := r.lookup(name) + if !ok { + return def + } + return r.parseDuration(name, v) +} + +// sessionDuration is optionalDuration restricted to whole milliseconds: +// session expiry is stored as a store.Timestamp, which keeps millisecond +// precision, so a finer duration could not be honoured. +func (r *reader) sessionDuration(name string, def time.Duration) time.Duration { + d := r.optionalDuration(name, def) + if d%time.Millisecond != 0 { + r.fail(name, "duration must be a whole number of milliseconds, got %s", d) + return 0 + } + return d +} + +func (r *reader) parseDuration(name, v string) time.Duration { d, err := time.ParseDuration(v) if err != nil { r.fail(name, "invalid duration %q (want e.g. 10s, 1m)", v) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 01a184f..8816bf7 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -3,13 +3,17 @@ package config import ( "errors" "fmt" + "net/netip" "net/url" "os" "path/filepath" + "reflect" "slices" "strings" "testing" "time" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded" ) func TestLoad(t *testing.T) { @@ -21,6 +25,10 @@ func TestLoad(t *testing.T) { t.Fatal(err) } unreadablePath := filepath.Join(dir, "missing_secret") + clientSecretPath := filepath.Join(dir, "oidc_client_secret") + if err := os.WriteFile(clientSecretPath, []byte("client-s3cret\n"), 0o600); err != nil { + t.Fatal(err) + } emptyPath := filepath.Join(dir, "empty_secret") if err := os.WriteFile(emptyPath, []byte("\n"), 0o600); err != nil { t.Fatal(err) @@ -33,6 +41,11 @@ func TestLoad(t *testing.T) { EnvDataDir: dir, EnvShutdownTimeout: "15s", EnvSecretKeyFile: secretPath, + + EnvOIDCIssuer: "https://auth.example.com/application/o/mealquest/", + EnvOIDCClientID: "mealquest", + EnvOIDCClientSecretFile: clientSecretPath, + EnvOIDCAllowedGroups: "mealquest-admin", } } with := func(changes map[string]string, unset ...string) map[string]string { @@ -70,6 +83,7 @@ func TestLoad(t *testing.T) { env: map[string]string{}, badVars: []string{ EnvHTTPAddr, EnvPublicURL, EnvDataDir, EnvShutdownTimeout, EnvSecretKeyFile, + EnvOIDCIssuer, EnvOIDCClientID, EnvOIDCClientSecretFile, EnvOIDCAllowedGroups, }, }, { @@ -139,6 +153,24 @@ func TestLoad(t *testing.T) { env: with(map[string]string{EnvPublicURL: "http://:8080"}), badVars: []string{EnvPublicURL}, }, + { + name: "plain HTTP public URL on a remote host", + env: with(map[string]string{EnvPublicURL: "http://meals.example.com"}), + badVars: []string{EnvPublicURL}, + }, + { + name: "plain HTTP public URL on a non-loopback IP", + env: with(map[string]string{EnvPublicURL: "http://192.168.1.20:8080"}), + badVars: []string{EnvPublicURL}, + }, + { + name: "plain HTTP public URL on 127.0.0.1", + env: with(map[string]string{EnvPublicURL: "http://127.0.0.1:8080"}), + }, + { + name: "plain HTTP public URL on [::1]", + env: with(map[string]string{EnvPublicURL: "http://[::1]:8080"}), + }, { name: "relative public URL", env: with(map[string]string{EnvPublicURL: "meals.example.com"}), @@ -169,6 +201,143 @@ func TestLoad(t *testing.T) { env: with(map[string]string{"MEALQUEST_SECRET_KEY": "inline"}), badVars: []string{"MEALQUEST_SECRET_KEY"}, }, + { + name: "inline OIDC client secret rejected", + env: with(map[string]string{"MEALQUEST_OIDC_CLIENT_SECRET": "inline"}), + badVars: []string{"MEALQUEST_OIDC_CLIENT_SECRET"}, + }, + { + name: "unreadable OIDC client secret file", + env: with(map[string]string{EnvOIDCClientSecretFile: unreadablePath}), + badVars: []string{EnvOIDCClientSecretFile}, + }, + { + name: "relative OIDC issuer", + env: with(map[string]string{EnvOIDCIssuer: "auth.example.com"}), + badVars: []string{EnvOIDCIssuer}, + }, + { + name: "non-http OIDC issuer", + env: with(map[string]string{EnvOIDCIssuer: "ftp://auth.example.com"}), + badVars: []string{EnvOIDCIssuer}, + }, + { + name: "plain HTTP OIDC issuer on a remote host", + env: with(map[string]string{EnvOIDCIssuer: "http://auth.example.com/application/o/mealquest/"}), + badVars: []string{EnvOIDCIssuer}, + }, + { + name: "plain HTTP OIDC issuer on localhost", + env: with(map[string]string{EnvOIDCIssuer: "http://localhost:9000/application/o/mealquest/"}), + }, + { + name: "several allowed groups", + env: with(map[string]string{EnvOIDCAllowedGroups: "mealquest-admin, family"}), + }, + { + name: "empty allowed group entry", + env: with(map[string]string{EnvOIDCAllowedGroups: "mealquest-admin,,family"}), + badVars: []string{EnvOIDCAllowedGroups}, + }, + { + name: "trailing comma in allowed groups", + env: with(map[string]string{EnvOIDCAllowedGroups: "mealquest-admin,"}), + badVars: []string{EnvOIDCAllowedGroups}, + }, + { + name: "blank OIDC client ID", + env: with(map[string]string{EnvOIDCClientID: " "}), + badVars: []string{EnvOIDCClientID}, + }, + { + name: "session lifetimes set", + env: with(map[string]string{EnvSessionIdle: "24h", EnvSessionMax: "48h"}), + }, + { + name: "session max equal to idle", + env: with(map[string]string{EnvSessionIdle: "24h", EnvSessionMax: "24h"}), + }, + { + name: "invalid session idle", + env: with(map[string]string{EnvSessionIdle: "a month"}), + badVars: []string{EnvSessionIdle}, + }, + { + name: "empty session idle", + env: with(map[string]string{EnvSessionIdle: ""}), + badVars: []string{EnvSessionIdle}, + }, + { + name: "non-positive session max", + env: with(map[string]string{EnvSessionMax: "-1h"}), + badVars: []string{EnvSessionMax}, + }, + { + name: "whole-millisecond session lifetimes", + env: with(map[string]string{EnvSessionIdle: "1ms", EnvSessionMax: "2ms"}), + }, + { + name: "sub-millisecond session idle", + env: with(map[string]string{EnvSessionIdle: "1500us", EnvSessionMax: "2ms"}), + badVars: []string{EnvSessionIdle}, + }, + { + name: "sub-millisecond session max", + env: with(map[string]string{EnvSessionIdle: "1ms", EnvSessionMax: "1ms500us"}), + badVars: []string{EnvSessionMax}, + }, + { + name: "session max below idle", + env: with(map[string]string{EnvSessionIdle: "48h", EnvSessionMax: "24h"}), + badVars: []string{EnvSessionMax}, + }, + { + name: "session max below default idle", + env: with(map[string]string{EnvSessionMax: "719h"}), + badVars: []string{EnvSessionMax}, + }, + { + name: "trusted proxy IP", + env: with(map[string]string{EnvTrustedProxy: "10.60.1.33"}), + }, + { + name: "trusted proxy IPs and CIDRs", + env: with(map[string]string{EnvTrustedProxy: "10.60.1.33, 172.16.0.0/12,2001:db8::/32,::1"}), + }, + { + name: "blank trusted proxy trusts no peer", + env: with(map[string]string{EnvTrustedProxy: " "}), + }, + { + name: "trusted proxy not an IP", + env: with(map[string]string{EnvTrustedProxy: "traefik"}), + badVars: []string{EnvTrustedProxy}, + }, + { + name: "trusted proxy invalid CIDR", + env: with(map[string]string{EnvTrustedProxy: "10.60.1.0/33"}), + badVars: []string{EnvTrustedProxy}, + }, + { + name: "trusted proxy CIDR with host bits", + env: with(map[string]string{EnvTrustedProxy: "10.60.1.33/24"}), + badVars: []string{EnvTrustedProxy}, + }, + { + name: "trusted proxy empty entry", + env: with(map[string]string{EnvTrustedProxy: "10.60.1.33,,10.60.1.34"}), + badVars: []string{EnvTrustedProxy}, + }, + { + name: "trusted proxy trusting everyone", + env: with(map[string]string{EnvTrustedProxy: "0.0.0.0/0"}), + badVars: []string{EnvTrustedProxy}, + }, + { + name: "trusted proxy with port", + env: with(map[string]string{EnvTrustedProxy: "10.60.1.33:443"}), + badVars: []string{EnvTrustedProxy}, + }, { name: "several invalid values are all named", env: with(map[string]string{ @@ -211,7 +380,7 @@ func TestLoad(t *testing.T) { t.Errorf("error message %q does not name %s", err.Error(), v) } } - if cfg != (Config{}) { + if !reflect.ValueOf(cfg).IsZero() { t.Errorf("load() returned non-zero Config alongside error: %+v", cfg) } }) @@ -226,12 +395,21 @@ func TestLoadValues(t *testing.T) { if err := os.WriteFile(secretPath, []byte("s3cret-value\r\n"), 0o600); err != nil { t.Fatal(err) } + clientSecretPath := filepath.Join(dir, "oidc_client_secret") + if err := os.WriteFile(clientSecretPath, []byte("client-s3cret\n"), 0o600); err != nil { + t.Fatal(err) + } env := map[string]string{ EnvHTTPAddr: "127.0.0.1:9000", EnvPublicURL: "http://localhost:9000", EnvDataDir: dir, EnvShutdownTimeout: "1m30s", EnvSecretKeyFile: secretPath, + + EnvOIDCIssuer: "https://auth.example.com/application/o/mealquest/", + EnvOIDCClientID: "mealquest", + EnvOIDCClientSecretFile: clientSecretPath, + EnvOIDCAllowedGroups: " mealquest-admin ,family", } cfg, err := load(func(k string) (string, bool) { v, ok := env[k]; return v, ok }) if err != nil { @@ -253,6 +431,40 @@ func TestLoadValues(t *testing.T) { if cfg.SecretKey.Reveal() != "s3cret-value" { t.Errorf("SecretKey.Reveal() = %q, want trailing newline trimmed", cfg.SecretKey.Reveal()) } + if got, want := cfg.OIDCIssuer.String(), "https://auth.example.com/application/o/mealquest/"; got != want { + t.Errorf("OIDCIssuer = %q, want %q", got, want) + } + if cfg.OIDCClientID != "mealquest" { + t.Errorf("OIDCClientID = %q", cfg.OIDCClientID) + } + if cfg.OIDCClientSecret.Reveal() != "client-s3cret" { + t.Errorf("OIDCClientSecret.Reveal() = %q", cfg.OIDCClientSecret.Reveal()) + } + if want := []string{"mealquest-admin", "family"}; !slices.Equal(cfg.OIDCAllowedGroups, want) { + t.Errorf("OIDCAllowedGroups = %q, want %q", cfg.OIDCAllowedGroups, want) + } + if cfg.SessionIdle != 720*time.Hour || cfg.SessionMax != 4320*time.Hour { + t.Errorf("SessionIdle, SessionMax = %v, %v, want defaults 720h, 4320h", cfg.SessionIdle, cfg.SessionMax) + } + + if cfg.TrustedProxy != nil { + t.Errorf("TrustedProxy = %v, want none when unset", cfg.TrustedProxy) + } + + env[EnvSessionIdle] = "12h" + env[EnvSessionMax] = "36h" + env[EnvTrustedProxy] = "10.60.1.33, 172.16.0.0/12" + cfg, err = load(func(k string) (string, bool) { v, ok := env[k]; return v, ok }) + if err != nil { + t.Fatalf("load() error = %v", err) + } + if cfg.SessionIdle != 12*time.Hour || cfg.SessionMax != 36*time.Hour { + t.Errorf("SessionIdle, SessionMax = %v, %v, want 12h, 36h", cfg.SessionIdle, cfg.SessionMax) + } + want := forwarded.Trusted{netip.MustParsePrefix("10.60.1.33/32"), netip.MustParsePrefix("172.16.0.0/12")} + if !slices.Equal(cfg.TrustedProxy, want) { + t.Errorf("TrustedProxy = %v, want %v", cfg.TrustedProxy, want) + } } func TestSecretRedacted(t *testing.T) { @@ -260,7 +472,7 @@ func TestSecretRedacted(t *testing.T) { s := Secret{value: "s3cret-value"} for _, format := range []string{"%v", "%s", "%+v", "%#v"} { - cfg := Config{SecretKey: s} + cfg := Config{SecretKey: s, OIDCClientSecret: s} for _, out := range []string{fmt.Sprintf(format, s), fmt.Sprintf(format, cfg)} { if strings.Contains(out, "s3cret-value") { t.Errorf("format %s leaked secret: %s", format, out) diff --git a/internal/forwarded/forwarded.go b/internal/forwarded/forwarded.go new file mode 100644 index 0000000..8ac429e --- /dev/null +++ b/internal/forwarded/forwarded.go @@ -0,0 +1,221 @@ +// Package forwarded resolves where a request really came from. The service +// runs behind a reverse proxy (Traefik), so the TCP peer is usually the proxy; +// the X-Forwarded-For and X-Forwarded-Proto headers it adds are believed only +// when the peer is a trusted proxy. From anyone else they are ignored, so a +// client cannot spoof its address or scheme. +package forwarded + +import ( + "context" + "errors" + "fmt" + "log/slog" + "net/http" + "net/netip" + "strings" +) + +// Header names honoured from trusted proxies. +const ( + HeaderFor = "X-Forwarded-For" + HeaderProto = "X-Forwarded-Proto" +) + +// ErrInvalidHeader marks a malformed forwarded header from a trusted peer. +var ErrInvalidHeader = errors.New("invalid forwarded header") + +// Trusted is the set of proxy addresses whose forwarded headers are honoured. +// The zero value trusts no peer. +type Trusted []netip.Prefix + +// ParseTrusted parses a comma-separated list of IP addresses and CIDR +// prefixes. A blank list trusts no peer. Each entry must be a plain address +// or a prefix with no host bits set; zones, IPv4-mapped IPv6 forms and +// zero-length prefixes, which would trust every peer, are rejected. +func ParseTrusted(s string) (Trusted, error) { + if strings.TrimSpace(s) == "" { + return nil, nil + } + var t Trusted + for _, entry := range strings.Split(s, ",") { + entry = strings.TrimSpace(entry) + if entry == "" { + return nil, fmt.Errorf("empty entry in comma-separated list %q", s) + } + p, err := parseEntry(entry) + if err != nil { + return nil, err + } + t = append(t, p) + } + return t, nil +} + +func parseEntry(entry string) (netip.Prefix, error) { + var p netip.Prefix + if strings.Contains(entry, "/") { + var err error + p, err = netip.ParsePrefix(entry) + if err != nil { + return netip.Prefix{}, fmt.Errorf("invalid CIDR %q", entry) + } + if p != p.Masked() { + return netip.Prefix{}, fmt.Errorf("CIDR %q has host bits set (did you mean %s?)", entry, p.Masked()) + } + } else { + a, err := netip.ParseAddr(entry) + if err != nil { + return netip.Prefix{}, fmt.Errorf("invalid IP address %q", entry) + } + if a.Zone() != "" { + return netip.Prefix{}, fmt.Errorf("IP address %q must not have a zone", entry) + } + p = netip.PrefixFrom(a, a.BitLen()) + } + switch { + case p.Addr().Is4In6(): + return netip.Prefix{}, fmt.Errorf("%q is IPv4-mapped IPv6; write the IPv4 form", entry) + case p.Bits() == 0: + return netip.Prefix{}, fmt.Errorf("%q would trust every peer", entry) + } + return p, nil +} + +// Contains reports whether a is a trusted proxy address. IPv4-mapped IPv6 +// addresses, as a dual-stack listener reports IPv4 peers, match their IPv4 +// form. +func (t Trusted) Contains(a netip.Addr) bool { + a = normalize(a) + for _, p := range t { + if p.Contains(a) { + return true + } + } + return false +} + +func normalize(a netip.Addr) netip.Addr { return a.Unmap().WithZone("") } + +// Origin is where a request came from. +type Origin struct { + // ClientIP is the client's address. + ClientIP netip.Addr + // Scheme is "https" or "http": the scheme the client used. + Scheme string +} + +// Resolve works out r's Origin. When the TCP peer (r.RemoteAddr) is not in +// trusted, both forwarded headers are ignored: the client is the peer and the +// scheme is that of the connection. +// +// When the peer is trusted: +// - X-Forwarded-For entries (across all its header lines, in order) are +// walked right to left, skipping trusted proxy addresses; the first +// untrusted address is the client. Entries left of it are client-supplied +// and not examined. If every entry is trusted the leftmost is the client; +// with no X-Forwarded-For the peer is the client. +// - X-Forwarded-Proto, if present, must be a single "http" or "https" and +// is the scheme; without it the connection's scheme is used. +// +// A malformed header from a trusted peer is an error wrapping +// ErrInvalidHeader; any other error means RemoteAddr is not an IP and port. +func Resolve(r *http.Request, trusted Trusted) (Origin, error) { + peer, err := netip.ParseAddrPort(r.RemoteAddr) + if err != nil { + return Origin{}, fmt.Errorf("peer address %q is not an IP and port", r.RemoteAddr) + } + o := Origin{ClientIP: normalize(peer.Addr()), Scheme: "http"} + if r.TLS != nil { + o.Scheme = "https" + } + if !trusted.Contains(o.ClientIP) { + return o, nil + } + if values := r.Header.Values(HeaderFor); len(values) > 0 { + o.ClientIP, err = clientFromChain(strings.Split(strings.Join(values, ","), ","), trusted) + if err != nil { + return Origin{}, err + } + } + switch values := r.Header.Values(HeaderProto); len(values) { + case 0: + case 1: + switch scheme := strings.ToLower(strings.TrimSpace(values[0])); scheme { + case "http", "https": + o.Scheme = scheme + default: + return Origin{}, fmt.Errorf("%w: %s %q is not http or https", ErrInvalidHeader, HeaderProto, values[0]) + } + default: + return Origin{}, fmt.Errorf("%w: %d %s headers, want one", ErrInvalidHeader, len(values), HeaderProto) + } + return o, nil +} + +// clientFromChain returns the rightmost untrusted address in chain, or the +// leftmost address when all are trusted. +func clientFromChain(chain []string, trusted Trusted) (netip.Addr, error) { + var client netip.Addr + for i := len(chain) - 1; i >= 0; i-- { + a, err := netip.ParseAddr(strings.TrimSpace(chain[i])) + if err != nil { + return netip.Addr{}, fmt.Errorf("%w: %s entry %q is not an IP address", ErrInvalidHeader, HeaderFor, strings.TrimSpace(chain[i])) + } + client = normalize(a) + if !trusted.Contains(client) { + break + } + } + return client, nil +} + +type contextKey struct{} + +// NewContext returns ctx carrying o. +func NewContext(ctx context.Context, o Origin) context.Context { + return context.WithValue(ctx, contextKey{}, o) +} + +// FromContext returns the Origin ctx carries; ok is false when the request +// was not resolved. +func FromContext(ctx context.Context) (o Origin, ok bool) { + o, ok = ctx.Value(contextKey{}).(Origin) + return o, ok +} + +// ClientIP returns the resolved client address ctx carries. +func ClientIP(ctx context.Context) (netip.Addr, bool) { + o, ok := FromContext(ctx) + return o.ClientIP, ok +} + +// Scheme returns the resolved request scheme ctx carries. +func Scheme(ctx context.Context) (string, bool) { + o, ok := FromContext(ctx) + return o.Scheme, ok +} + +// LogAttr is the log attribute key for the resolved client address. +const LogAttr = "client_ip" + +// LogHandler wraps h so every record logged with a resolved request's +// context carries the client address as LogAttr. +func LogHandler(h slog.Handler) slog.Handler { return logHandler{h} } + +type logHandler struct{ slog.Handler } + +func (h logHandler) Handle(ctx context.Context, rec slog.Record) error { + if ip, ok := ClientIP(ctx); ok { + rec = rec.Clone() + rec.AddAttrs(slog.String(LogAttr, ip.String())) + } + return h.Handler.Handle(ctx, rec) +} + +func (h logHandler) WithAttrs(attrs []slog.Attr) slog.Handler { + return logHandler{h.Handler.WithAttrs(attrs)} +} + +func (h logHandler) WithGroup(name string) slog.Handler { + return logHandler{h.Handler.WithGroup(name)} +} diff --git a/internal/forwarded/forwarded_test.go b/internal/forwarded/forwarded_test.go new file mode 100644 index 0000000..298e9ab --- /dev/null +++ b/internal/forwarded/forwarded_test.go @@ -0,0 +1,260 @@ +package forwarded + +import ( + "bytes" + "context" + "crypto/tls" + "errors" + "log/slog" + "net/http" + "net/http/httptest" + "net/netip" + "slices" + "strings" + "testing" +) + +func TestParseTrusted(t *testing.T) { + t.Parallel() + tests := []struct { + in string + want []string + wantErr string + }{ + {in: "", want: nil}, + {in: " ", want: nil}, + {in: "10.60.1.33", want: []string{"10.60.1.33/32"}}, + {in: "10.60.1.33, 172.16.0.0/12", want: []string{"10.60.1.33/32", "172.16.0.0/12"}}, + {in: "2001:db8::1,2001:db8::/32", want: []string{"2001:db8::1/128", "2001:db8::/32"}}, + {in: "traefik", wantErr: "invalid IP address"}, + {in: "10.60.1.33:443", wantErr: "invalid IP address"}, + {in: "10.60.1.0/33", wantErr: "invalid CIDR"}, + {in: "10.60.1.33/24", wantErr: "did you mean 10.60.1.0/24"}, + {in: "10.60.1.33,,10.60.1.34", wantErr: "empty entry"}, + {in: "10.60.1.33,", wantErr: "empty entry"}, + {in: "fe80::1%eth0", wantErr: "zone"}, + {in: "::ffff:10.60.1.33", wantErr: "IPv4-mapped"}, + {in: "0.0.0.0/0", wantErr: "every peer"}, + {in: "::/0", wantErr: "every peer"}, + } + for _, tt := range tests { + got, err := ParseTrusted(tt.in) + if tt.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("ParseTrusted(%q) error = %v, want one containing %q", tt.in, err, tt.wantErr) + } + continue + } + if err != nil { + t.Errorf("ParseTrusted(%q) error = %v", tt.in, err) + continue + } + var gotStr []string + for _, p := range got { + gotStr = append(gotStr, p.String()) + } + if !slices.Equal(gotStr, tt.want) { + t.Errorf("ParseTrusted(%q) = %v, want %v", tt.in, gotStr, tt.want) + } + } +} + +func TestTrustedContainsMappedIPv4(t *testing.T) { + t.Parallel() + trusted := mustTrusted(t, "10.60.1.33") + if !trusted.Contains(netip.MustParseAddr("::ffff:10.60.1.33")) { + t.Error("IPv4-mapped form of a trusted address is not trusted") + } + if Trusted(nil).Contains(netip.MustParseAddr("10.60.1.33")) { + t.Error("an empty set trusts a peer") + } +} + +func mustTrusted(t *testing.T, s string) Trusted { + t.Helper() + tr, err := ParseTrusted(s) + if err != nil { + t.Fatal(err) + } + return tr +} + +func TestResolve(t *testing.T) { + t.Parallel() + // 10.60.1.33 is Traefik; 10.60.2.0/24 is an inner proxy tier. + trusted := mustTrusted(t, "10.60.1.33,10.60.2.0/24") + tests := []struct { + name string + peer string + tls bool + xff []string + xfp []string + wantIP string + wantScheme string + }{ + { + name: "untrusted peer, no headers", + peer: "198.51.100.9:5000", wantIP: "198.51.100.9", wantScheme: "http", + }, + { + name: "untrusted peer over TLS", + peer: "198.51.100.9:5000", tls: true, wantIP: "198.51.100.9", wantScheme: "https", + }, + { + name: "untrusted peer spoofing both headers is ignored", + peer: "198.51.100.9:5000", xff: []string{"203.0.113.7"}, xfp: []string{"https"}, + wantIP: "198.51.100.9", wantScheme: "http", + }, + { + name: "untrusted peer's malformed headers are ignored, not rejected", + peer: "198.51.100.9:5000", xff: []string{"not-an-ip"}, xfp: []string{"gopher"}, + wantIP: "198.51.100.9", wantScheme: "http", + }, + { + name: "trusted peer: client from X-Forwarded-For, scheme from X-Forwarded-Proto", + peer: "10.60.1.33:41000", xff: []string{"203.0.113.7"}, xfp: []string{"https"}, + wantIP: "203.0.113.7", wantScheme: "https", + }, + { + name: "trusted peer seen as IPv4-mapped IPv6", + peer: "[::ffff:10.60.1.33]:41000", xff: []string{"203.0.113.7"}, xfp: []string{"https"}, + wantIP: "203.0.113.7", wantScheme: "https", + }, + { + name: "trusted peer without headers is itself the client", + peer: "10.60.1.33:41000", wantIP: "10.60.1.33", wantScheme: "http", + }, + { + name: "rightmost untrusted address wins over a client-supplied prefix", + peer: "10.60.1.33:41000", xff: []string{"1.2.3.4, 203.0.113.7"}, xfp: []string{"https"}, + wantIP: "203.0.113.7", wantScheme: "https", + }, + { + name: "trusted proxies in the chain are skipped", + peer: "10.60.1.33:41000", xff: []string{"1.2.3.4, 203.0.113.7, 10.60.2.5"}, xfp: []string{"https"}, + wantIP: "203.0.113.7", wantScheme: "https", + }, + { + name: "several header lines form one chain", + peer: "10.60.1.33:41000", xff: []string{"1.2.3.4", "203.0.113.7, 10.60.2.5"}, + wantIP: "203.0.113.7", wantScheme: "http", + }, + { + name: "garbage left of the client is not examined", + peer: "10.60.1.33:41000", xff: []string{"spoofed, 203.0.113.7"}, + wantIP: "203.0.113.7", wantScheme: "http", + }, + { + name: "all trusted: leftmost is the client", + peer: "10.60.1.33:41000", xff: []string{"10.60.2.5, 10.60.2.6"}, + wantIP: "10.60.2.5", wantScheme: "http", + }, + { + name: "IPv6 client", + peer: "10.60.1.33:41000", xff: []string{"2001:db8::7"}, xfp: []string{"HTTPS"}, + wantIP: "2001:db8::7", wantScheme: "https", + }, + { + name: "trusted peer may downgrade to http", + peer: "10.60.1.33:41000", tls: true, xfp: []string{"http"}, + wantIP: "10.60.1.33", wantScheme: "http", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + r := request(tt.peer, tt.xff, tt.xfp) + if tt.tls { + r.TLS = &tls.ConnectionState{} + } + o, err := Resolve(r, trusted) + if err != nil { + t.Fatalf("Resolve error = %v", err) + } + if o.ClientIP.String() != tt.wantIP || o.Scheme != tt.wantScheme { + t.Errorf("Resolve = %s %s, want %s %s", o.ClientIP, o.Scheme, tt.wantIP, tt.wantScheme) + } + }) + } +} + +func TestResolveRejectsMalformedHeadersFromTrustedPeer(t *testing.T) { + t.Parallel() + trusted := mustTrusted(t, "10.60.1.33") + for _, tt := range []struct { + name string + xff, xfp []string + }{ + {"X-Forwarded-For not an IP", []string{"not-an-ip"}, nil}, + {"X-Forwarded-For with a port", []string{"203.0.113.7:5000"}, nil}, + {"X-Forwarded-For empty entry", []string{"203.0.113.7, "}, nil}, + {"X-Forwarded-For empty", []string{""}, nil}, + {"X-Forwarded-Proto unknown scheme", nil, []string{"ftp"}}, + {"X-Forwarded-Proto list", nil, []string{"https, http"}}, + {"X-Forwarded-Proto repeated", nil, []string{"https", "https"}}, + } { + _, err := Resolve(request("10.60.1.33:41000", tt.xff, tt.xfp), trusted) + if !errors.Is(err, ErrInvalidHeader) { + t.Errorf("%s: Resolve error = %v, want ErrInvalidHeader", tt.name, err) + } + } +} + +func TestResolveBadPeerAddress(t *testing.T) { + t.Parallel() + r := request("@", nil, nil) + _, err := Resolve(r, nil) + if err == nil || errors.Is(err, ErrInvalidHeader) { + t.Errorf("Resolve error = %v, want a peer address error", err) + } +} + +func request(peer string, xff, xfp []string) *http.Request { + r := httptest.NewRequest(http.MethodGet, "/", nil) + r.RemoteAddr = peer + for _, v := range xff { + r.Header.Add(HeaderFor, v) + } + for _, v := range xfp { + r.Header.Add(HeaderProto, v) + } + return r +} + +func TestContextHelpers(t *testing.T) { + t.Parallel() + if _, ok := ClientIP(context.Background()); ok { + t.Error("ClientIP ok on an unresolved context") + } + if _, ok := Scheme(context.Background()); ok { + t.Error("Scheme ok on an unresolved context") + } + ctx := NewContext(context.Background(), Origin{ClientIP: netip.MustParseAddr("203.0.113.7"), Scheme: "https"}) + if ip, ok := ClientIP(ctx); !ok || ip.String() != "203.0.113.7" { + t.Errorf("ClientIP = %s %v, want 203.0.113.7 true", ip, ok) + } + if s, ok := Scheme(ctx); !ok || s != "https" { + t.Errorf("Scheme = %q %v, want https true", s, ok) + } +} + +func TestLogHandler(t *testing.T) { + t.Parallel() + var buf bytes.Buffer + logger := slog.New(LogHandler(slog.NewTextHandler(&buf, nil))).With("component", "test") + ctx := NewContext(context.Background(), Origin{ClientIP: netip.MustParseAddr("203.0.113.7"), Scheme: "https"}) + + logger.InfoContext(ctx, "resolved") + logger.InfoContext(context.Background(), "unresolved") + + lines := strings.Split(strings.TrimSpace(buf.String()), "\n") + if len(lines) != 2 { + t.Fatalf("logged %d lines, want 2: %q", len(lines), buf.String()) + } + if !strings.Contains(lines[0], "client_ip=203.0.113.7") || !strings.Contains(lines[0], "component=test") { + t.Errorf("resolved line = %q, want client_ip and the With attribute", lines[0]) + } + if strings.Contains(lines[1], "client_ip") { + t.Errorf("unresolved line = %q, want no client_ip", lines[1]) + } +} diff --git a/internal/secureurl/secureurl.go b/internal/secureurl/secureurl.go new file mode 100644 index 0000000..09f3129 --- /dev/null +++ b/internal/secureurl/secureurl.go @@ -0,0 +1,32 @@ +// Package secureurl holds the one transport rule for URLs that carry +// credentials or session cookies: HTTPS, or plain HTTP to a loopback host for +// local development and tests. +package secureurl + +import ( + "errors" + "fmt" + "net/url" + "strings" +) + +// loopbackHosts are the only hosts plain HTTP is allowed for, compared +// case-insensitively. url.URL's Hostname strips the brackets from "[::1]". +var loopbackHosts = map[string]bool{"localhost": true, "127.0.0.1": true, "::1": true} + +// Check reports an error unless u is an absolute https:// URL, or an http:// +// URL whose host is localhost, 127.0.0.1 or ::1. +func Check(u *url.URL) error { + switch { + case u.Hostname() == "": + return errors.New("URL has no host") + case u.Scheme == "https": + return nil + case u.Scheme == "http" && loopbackHosts[strings.ToLower(u.Hostname())]: + return nil + case u.Scheme == "http": + return errors.New("plain http:// is allowed only for localhost, 127.0.0.1 or [::1]; use https://") + default: + return fmt.Errorf("unsupported scheme %q (want https://)", u.Scheme) + } +} diff --git a/internal/secureurl/secureurl_test.go b/internal/secureurl/secureurl_test.go new file mode 100644 index 0000000..bebc707 --- /dev/null +++ b/internal/secureurl/secureurl_test.go @@ -0,0 +1,40 @@ +package secureurl + +import ( + "net/url" + "testing" +) + +func TestCheck(t *testing.T) { + t.Parallel() + tests := []struct { + raw string + ok bool + }{ + {"https://meals.example.com", true}, + {"https://auth.example.com/application/o/mealquest/", true}, + {"https://192.0.2.10:8443", true}, + {"http://localhost:8080", true}, + {"http://LOCALHOST:8080", true}, + {"http://127.0.0.1:8080/path", true}, + {"http://[::1]:8080", true}, + {"http://[::1]", true}, + {"http://meals.example.com", false}, + {"http://192.0.2.10", false}, + {"http://127.0.0.2", false}, + {"http://localhost.example.com", false}, + {"http://0.0.0.0:8080", false}, + {"ftp://localhost", false}, + {"https://", false}, + {"meals.example.com", false}, + } + for _, tc := range tests { + u, err := url.Parse(tc.raw) + if err != nil { + t.Fatal(err) + } + if err := Check(u); (err == nil) != tc.ok { + t.Errorf("Check(%q) error = %v, want ok=%v", tc.raw, err, tc.ok) + } + } +} diff --git a/internal/server/csrf.go b/internal/server/csrf.go new file mode 100644 index 0000000..65c21a3 --- /dev/null +++ b/internal/server/csrf.go @@ -0,0 +1,42 @@ +package server + +import ( + "crypto/subtle" + "log/slog" + "net/http" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth" +) + +// CSRFHeader carries the session's CSRF token, which the client reads from +// GET /api/v1/me. +const CSRFHeader = "X-MealQuest-CSRF" + +// requireCSRF passes GET requests to next; every other +// request must carry CSRFHeader equal to its session's CSRF token or get 403 +// csrf. It runs behind requireSession, which puts the session in the context. +func requireCSRF(logger *slog.Logger) func(http.Handler) http.Handler { + forbidden := apiErrorHandler(logger, http.StatusForbidden, "csrf", "missing or invalid CSRF token") + internal := apiErrorHandler(logger, http.StatusInternalServerError, "internal", "internal error") + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodGet: + next.ServeHTTP(w, r) + return + } + s, ok := auth.SessionFrom(r.Context()) + if !ok || s.CSRFToken == "" { + // An empty token would match a missing header. + logger.ErrorContext(r.Context(), "csrf check without a session token", "session", ok) + internal(w, r) + return + } + if subtle.ConstantTimeCompare([]byte(r.Header.Get(CSRFHeader)), []byte(s.CSRFToken)) != 1 { + forbidden(w, r) + return + } + next.ServeHTTP(w, r) + }) + } +} diff --git a/internal/server/csrf_test.go b/internal/server/csrf_test.go new file mode 100644 index 0000000..7e1dd54 --- /dev/null +++ b/internal/server/csrf_test.go @@ -0,0 +1,166 @@ +package server + +import ( + "context" + "encoding/json" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth" +) + +const ( + csrfJSON = `{"error":{"code":"csrf","message":"missing or invalid CSRF token","retryable":false}}` + methodNotAllowedJSON = `{"error":{"code":"method_not_allowed","message":"method not allowed","retryable":false}}` +) + +func TestCSRFThroughRouter(t *testing.T) { + t.Parallel() + + router := newTestRouter(t, "dev") + for _, tc := range []struct { + name, method, path string + header []string // nil sends no CSRF header + wantStatus int + wantBody string + }{ + {"POST without header", http.MethodPost, "/api/v1/me", nil, http.StatusForbidden, csrfJSON}, + {"POST with wrong header", http.MethodPost, "/api/v1/me", []string{"wrong-token"}, http.StatusForbidden, csrfJSON}, + {"POST with empty header", http.MethodPost, "/api/v1/me", []string{""}, http.StatusForbidden, csrfJSON}, + {"POST with token prefix", http.MethodPost, "/api/v1/me", []string{"csrf-token"}, http.StatusForbidden, csrfJSON}, + {"PATCH without header", http.MethodPatch, "/api/v1/settings/household", nil, http.StatusForbidden, csrfJSON}, + {"DELETE with wrong header", http.MethodDelete, "/api/v1/me", []string{"wrong-token"}, http.StatusForbidden, csrfJSON}, + // The right token passes the check and reaches the router, which has + // no POST /api/v1/me. + {"POST with right header", http.MethodPost, "/api/v1/me", []string{testSession.CSRFToken}, http.StatusMethodNotAllowed, methodNotAllowedJSON}, + {"PATCH with right header", http.MethodPatch, "/api/v1/settings/household", []string{testSession.CSRFToken}, http.StatusNotFound, notFoundJSON}, + {"GET without header", http.MethodGet, "/api/v1/me", nil, http.StatusOK, ""}, + // Only GET is exempt; HEAD and OPTIONS need the header too. + {"HEAD without header", http.MethodHead, "/api/v1/me", nil, http.StatusForbidden, csrfJSON}, + {"OPTIONS without header", http.MethodOptions, "/api/v1/me", nil, http.StatusForbidden, csrfJSON}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + req := httptest.NewRequest(tc.method, tc.path, nil) + req.AddCookie(validSessionCookie) + if tc.header != nil { + req.Header.Set(CSRFHeader, tc.header[0]) + } + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != tc.wantStatus { + t.Errorf("status = %d, want %d (body %s)", rec.Code, tc.wantStatus, rec.Body.String()) + } + if tc.wantBody != "" && rec.Body.String() != tc.wantBody { + t.Errorf("body = %s, want %s", rec.Body.String(), tc.wantBody) + } + }) + } +} + +func TestCSRFUnauthenticatedGets401(t *testing.T) { + t.Parallel() + + req := httptest.NewRequest(http.MethodPost, "/api/v1/me", nil) + rec := httptest.NewRecorder() + newTestRouter(t, "dev").ServeHTTP(rec, req) + + if rec.Code != http.StatusUnauthorized || rec.Body.String() != unauthenticatedJSON { + t.Errorf("response = %d %s, want 401 %s", rec.Code, rec.Body.String(), unauthenticatedJSON) + } +} + +func TestRequireCSRF(t *testing.T) { + t.Parallel() + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + for _, tc := range []struct { + name string + method string + session *auth.Session + header string + wantStatus int + }{ + {"right header reaches handler", http.MethodPost, &testSession, testSession.CSRFToken, http.StatusNoContent}, + {"wrong header", http.MethodPut, &testSession, "nope", http.StatusForbidden}, + {"GET needs no header", http.MethodGet, &testSession, "", http.StatusNoContent}, + {"HEAD needs the header", http.MethodHead, &testSession, "", http.StatusForbidden}, + {"OPTIONS needs the header", http.MethodOptions, &testSession, "", http.StatusForbidden}, + {"no session in context", http.MethodPost, nil, "", http.StatusInternalServerError}, + {"session without token", http.MethodPost, &auth.Session{Subject: "user-123"}, "", http.StatusInternalServerError}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + called := false + h := requireCSRF(logger)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + called = true + w.WriteHeader(http.StatusNoContent) + })) + ctx := context.Background() + if tc.session != nil { + ctx = auth.WithSession(ctx, *tc.session) + } + req := httptest.NewRequestWithContext(ctx, tc.method, "/api/v1/anything", nil) + if tc.header != "" { + req.Header.Set(CSRFHeader, tc.header) + } + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + if rec.Code != tc.wantStatus { + t.Errorf("status = %d, want %d", rec.Code, tc.wantStatus) + } + if called != (tc.wantStatus == http.StatusNoContent) { + t.Errorf("handler called = %v with status %d", called, rec.Code) + } + }) + } +} + +func TestMe(t *testing.T) { + t.Parallel() + + req := httptest.NewRequest(http.MethodGet, "/api/v1/me", nil) + req.AddCookie(validSessionCookie) + rec := httptest.NewRecorder() + newTestRouter(t, "dev").ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body %s)", rec.Code, rec.Body.String()) + } + if ct := rec.Header().Get("Content-Type"); ct != "application/json" { + t.Errorf("Content-Type = %q, want application/json", ct) + } + if cc := rec.Header().Get("Cache-Control"); cc != "no-store" { + t.Errorf("Cache-Control = %q, want no-store", cc) + } + var got map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("decode %s: %v", rec.Body.String(), err) + } + want := map[string]any{ + "name": "Home Cook", + "email": "cook@example.com", + "household": map[string]any{"label": "Our kitchen"}, + "csrfToken": "csrf-token-123", + } + gotJSON, _ := json.Marshal(got) + wantJSON, _ := json.Marshal(want) + if string(gotJSON) != string(wantJSON) { + t.Errorf("body = %s, want %s", gotJSON, wantJSON) + } +} + +func TestMeNeedsSession(t *testing.T) { + t.Parallel() + + rec := httptest.NewRecorder() + newTestRouter(t, "dev").ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/api/v1/me", nil)) + if rec.Code != http.StatusUnauthorized || rec.Body.String() != unauthenticatedJSON { + t.Errorf("response = %d %s, want 401 %s", rec.Code, rec.Body.String(), unauthenticatedJSON) + } +} diff --git a/internal/server/login_test.go b/internal/server/login_test.go new file mode 100644 index 0000000..d928de7 --- /dev/null +++ b/internal/server/login_test.go @@ -0,0 +1,140 @@ +package server + +import ( + "context" + "encoding/json" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "testing" + "testing/fstest" + "time" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" +) + +// newDiscoveryProvider serves the OIDC discovery document auth.New needs; +// login only redirects to its authorization endpoint. +func newDiscoveryProvider(t *testing.T) *httptest.Server { + t.Helper() + var srv *httptest.Server + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/.well-known/openid-configuration" { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]any{ + "issuer": srv.URL, + "authorization_endpoint": srv.URL + "/authorize", + "token_endpoint": srv.URL + "/token", + "jwks_uri": srv.URL + "/jwks", + "userinfo_endpoint": srv.URL + "/userinfo", + "end_session_endpoint": srv.URL + "/end-session/", + "response_types_supported": []string{"code"}, + "subject_types_supported": []string{"public"}, + "id_token_signing_alg_values_supported": []string{"RS256"}, + }) + })) + t.Cleanup(srv.Close) + return srv +} + +// newLoginRouter wires the production router as cmd/server does: the real +// auth flow and session manager, trusting Traefik only. +func newLoginRouter(t *testing.T, issuer string) http.Handler { + t.Helper() + ctx := context.Background() + db, err := store.Open(ctx, t.TempDir()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := db.Close(); err != nil { + t.Error(err) + } + }) + if err := store.Migrate(ctx, db.Writer, migrations.FS); err != nil { + t.Fatal(err) + } + sessions, err := auth.NewSessionManager(db, 720*time.Hour, 4320*time.Hour) + if err != nil { + t.Fatal(err) + } + logger := slog.New(forwarded.LogHandler(slog.NewTextHandler(io.Discard, nil))) + flow, err := auth.New(ctx, auth.Config{ + Issuer: issuer, + ClientID: "mealquest", + ClientSecret: "client-s3cret", + PublicURL: &url.URL{Scheme: "https", Host: "meals.example.com"}, + SecretKey: []byte("server-secret-key"), + AllowedGroups: []string{"mealquest-admin"}, + }, sessions, logger) + if err != nil { + t.Fatal(err) + } + trusted, err := forwarded.ParseTrusted(traefik) + if err != nil { + t.Fatal(err) + } + r, err := NewRouter(logger, "dev", trusted, flow, sessions, + fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + if err != nil { + t.Fatal(err) + } + return r +} + +// TestLoginThroughRouter starts a login through the production router. The +// redirect URI and the Secure login cookie come from MEALQUEST_PUBLIC_URL +// whatever the direct peer: forwarded headers from an untrusted peer are +// ignored but never fail sign-in. +func TestLoginThroughRouter(t *testing.T) { + t.Parallel() + provider := newDiscoveryProvider(t) + router := newLoginRouter(t, provider.URL) + // Traefik reaches the service over plain http on its internal address; + // the host headers must not leak into the redirect URI. + headers := map[string]string{ + forwarded.HeaderFor: "203.0.113.7", + forwarded.HeaderProto: "https", + "X-Forwarded-Host": "evil.example.com", + } + tests := []struct { + name string + peer string + }{ + {"trusted proxy", traefik + ":41000"}, + {"untrusted peer", "172.17.0.1:5000"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + rec := httptest.NewRecorder() + router.ServeHTTP(rec, originRequest("http://mealquest:8080"+auth.LoginPath, tt.peer, headers)) + resp := rec.Result() + if resp.StatusCode != http.StatusFound { + t.Fatalf("login status = %d, want 302", resp.StatusCode) + } + loc, err := url.Parse(resp.Header.Get("Location")) + if err != nil { + t.Fatal(err) + } + if got, want := loc.Scheme+"://"+loc.Host+loc.Path, provider.URL+"/authorize"; got != want { + t.Errorf("redirect to %s, want %s", got, want) + } + if got, want := loc.Query().Get("redirect_uri"), "https://meals.example.com/auth/callback"; got != want { + t.Errorf("redirect_uri = %q, want %q from the public URL", got, want) + } + cookies := resp.Cookies() + if len(cookies) != 1 || cookies[0].Name != "__Host-mealquest_login" || !cookies[0].Secure || !cookies[0].HttpOnly { + t.Errorf("login cookies = %v, want one Secure, HttpOnly __Host-mealquest_login", cookies) + } + }) + } +} diff --git a/internal/server/me.go b/internal/server/me.go new file mode 100644 index 0000000..e9a8a26 --- /dev/null +++ b/internal/server/me.go @@ -0,0 +1,52 @@ +package server + +import ( + "encoding/json" + "log/slog" + "net/http" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth" +) + +// householdLabel is the single shared household identity. +const householdLabel = "Our kitchen" + +type meResponse struct { + Name string `json:"name"` + Email string `json:"email"` + Household householdInfo `json:"household"` + CSRFToken string `json:"csrfToken"` +} + +type householdInfo struct { + Label string `json:"label"` +} + +// me answers GET /api/v1/me with the signed-in member, the household and the +// session's CSRF token for CSRFHeader. +func me(logger *slog.Logger) http.HandlerFunc { + internal := apiErrorHandler(logger, http.StatusInternalServerError, "internal", "internal error") + return func(w http.ResponseWriter, r *http.Request) { + s, ok := auth.SessionFrom(r.Context()) + if !ok { + logger.ErrorContext(r.Context(), "GET /api/v1/me without a session") + internal(w, r) + return + } + body, err := json.Marshal(meResponse{ + Name: s.Name, + Email: s.Email, + Household: householdInfo{Label: householdLabel}, + CSRFToken: s.CSRFToken, + }) + if err != nil { + panic(err) // string fields cannot fail to marshal + } + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Cache-Control", "no-store") + w.WriteHeader(http.StatusOK) + if _, err := w.Write(body); err != nil { + logger.WarnContext(r.Context(), "write me response", "error", err) + } + } +} diff --git a/internal/server/origin.go b/internal/server/origin.go new file mode 100644 index 0000000..9f711ad --- /dev/null +++ b/internal/server/origin.go @@ -0,0 +1,34 @@ +package server + +import ( + "errors" + "log/slog" + "net/http" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded" +) + +// resolveOrigin resolves every request's client address and scheme with +// forwarded.Resolve and puts them in the context (read them with +// forwarded.ClientIP and forwarded.Scheme). Forwarded headers are honoured +// only from a peer in trusted. A malformed forwarded header from a trusted +// peer gets 400 bad_request. +func resolveOrigin(logger *slog.Logger, trusted forwarded.Trusted) func(http.Handler) http.Handler { + badRequest := apiErrorHandler(logger, http.StatusBadRequest, "bad_request", "invalid forwarded headers") + internal := apiErrorHandler(logger, http.StatusInternalServerError, "internal", "internal error") + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + o, err := forwarded.Resolve(r, trusted) + switch { + case errors.Is(err, forwarded.ErrInvalidHeader): + logger.WarnContext(r.Context(), "rejected forwarded headers", "peer", r.RemoteAddr, "error", err) + badRequest(w, r) + case err != nil: + logger.ErrorContext(r.Context(), "resolve request origin", "error", err) + internal(w, r) + default: + next.ServeHTTP(w, r.WithContext(forwarded.NewContext(r.Context(), o))) + } + }) + } +} diff --git a/internal/server/origin_test.go b/internal/server/origin_test.go new file mode 100644 index 0000000..1df8212 --- /dev/null +++ b/internal/server/origin_test.go @@ -0,0 +1,145 @@ +package server + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + "testing/fstest" + + "github.com/go-chi/chi/v5" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded" +) + +// traefik is the production proxy's address. +const traefik = "10.60.1.33" + +// probeAuth mounts GET /auth/probe, which reports the request's resolved +// origin, in place of the real auth flow. +type probeAuth struct{} + +func (probeAuth) Mount(r chi.Router) { + r.Get("/auth/probe", func(w http.ResponseWriter, r *http.Request) { + ip, ipOK := forwarded.ClientIP(r.Context()) + scheme, schemeOK := forwarded.Scheme(r.Context()) + if !ipOK || !schemeOK { + http.Error(w, "origin not resolved", http.StatusInternalServerError) + return + } + fmt.Fprintf(w, "%s %s", ip, scheme) + }) +} + +// newOriginRouter builds the production router trusting Traefik, logging +// through forwarded.LogHandler as cmd/server does. +func newOriginRouter(t *testing.T, logs io.Writer) http.Handler { + t.Helper() + trusted, err := forwarded.ParseTrusted(traefik) + if err != nil { + t.Fatal(err) + } + logger := slog.New(forwarded.LogHandler(slog.NewJSONHandler(logs, nil))) + r, err := NewRouter(logger, "dev", trusted, probeAuth{}, stubSessions{}, + fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + if err != nil { + t.Fatal(err) + } + return r +} + +func originRequest(target, peer string, headers map[string]string) *http.Request { + req := httptest.NewRequest(http.MethodGet, target, nil) + req.RemoteAddr = peer + for k, v := range headers { + req.Header.Set(k, v) + } + return req +} + +func TestRouterResolvesOrigin(t *testing.T) { + t.Parallel() + spoofed := map[string]string{forwarded.HeaderFor: "203.0.113.7", forwarded.HeaderProto: "https"} + tests := []struct { + name string + peer string + want string + }{ + {"trusted proxy's forwarded headers are honoured", traefik + ":41000", "203.0.113.7 https"}, + {"untrusted peer spoofing them is ignored", "198.51.100.9:5000", "198.51.100.9 http"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + rec := httptest.NewRecorder() + newOriginRouter(t, io.Discard).ServeHTTP(rec, originRequest("/auth/probe", tt.peer, spoofed)) + if rec.Code != http.StatusOK || rec.Body.String() != tt.want { + t.Errorf("probe = %d %q, want 200 %q", rec.Code, rec.Body.String(), tt.want) + } + }) + } +} + +func TestRouterLogsResolvedClientIP(t *testing.T) { + t.Parallel() + spoofed := map[string]string{forwarded.HeaderFor: "203.0.113.7", forwarded.HeaderProto: "https"} + tests := []struct { + name string + peer string + want string + }{ + {"trusted proxy", traefik + ":41000", "203.0.113.7"}, + {"untrusted peer spoofing", "198.51.100.9:5000", "198.51.100.9"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + var logs bytes.Buffer + // A failed session lookup is logged with the request's context. + req := originRequest("/api/v1/me", tt.peer, spoofed) + req.AddCookie(brokenSessionCookie) + rec := httptest.NewRecorder() + newOriginRouter(t, &logs).ServeHTTP(rec, req) + if rec.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want 500", rec.Code) + } + var line map[string]any + if err := json.Unmarshal(logs.Bytes(), &line); err != nil { + t.Fatalf("log %q: %v", logs.String(), err) + } + if line["msg"] != "look up session" || line[forwarded.LogAttr] != tt.want { + t.Errorf("log = %v, want the session error with %s=%s", line, forwarded.LogAttr, tt.want) + } + }) + } +} + +func TestRouterRejectsMalformedForwardedHeaders(t *testing.T) { + t.Parallel() + const badRequestJSON = `{"error":{"code":"bad_request","message":"invalid forwarded headers","retryable":false}}` + for _, headers := range []map[string]string{ + {forwarded.HeaderFor: "not-an-ip"}, + {forwarded.HeaderProto: "ftp"}, + } { + var logs bytes.Buffer + rec := httptest.NewRecorder() + newOriginRouter(t, &logs).ServeHTTP(rec, originRequest("/health", traefik+":41000", headers)) + if rec.Code != http.StatusBadRequest || rec.Body.String() != badRequestJSON { + t.Errorf("%v: response = %d %s, want 400 %s", headers, rec.Code, rec.Body.String(), badRequestJSON) + } + if !bytes.Contains(logs.Bytes(), []byte("rejected forwarded headers")) { + t.Errorf("%v: logs = %q, want the rejection logged", headers, logs.String()) + } + + // The same headers from an untrusted peer are not examined at all. + rec = httptest.NewRecorder() + newOriginRouter(t, io.Discard).ServeHTTP(rec, originRequest("/health", "198.51.100.9:5000", headers)) + if rec.Code != http.StatusOK { + t.Errorf("%v from an untrusted peer: status = %d, want 200", headers, rec.Code) + } + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 929c846..46520c2 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -9,6 +9,7 @@ import ( "github.com/go-chi/chi/v5" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/frontend" ) @@ -16,9 +17,18 @@ import ( // SPA's index.html. var reservedPrefixes = []string{"/api", "/auth", "/media", "/health"} -// NewRouter returns the HTTP handler for the service. version is reported by -// GET /health; assets is the web build served for every other GET. -func NewRouter(logger *slog.Logger, version string, assets fs.FS) (http.Handler, error) { +// RouteMounter registers its own routes on the router. +type RouteMounter interface { + Mount(r chi.Router) +} + +// NewRouter returns the HTTP handler for the service. Every request first +// has its client address and scheme resolved, honouring forwarded headers +// only from a peer in trusted (see resolveOrigin). version is reported by +// GET /health; auth mounts the /auth routes; sessions guards /api/v1 and +// /media; assets is the web build served for every other GET. /health, /auth +// and the web build are public. Requests to /api/v1 other than GET also need the session's CSRF token in CSRFHeader. +func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, auth RouteMounter, sessions SessionLookup, assets fs.FS) (http.Handler, error) { notFound := apiErrorHandler(logger, http.StatusNotFound, "not_found", "not found") methodNotAllowed := apiErrorHandler(logger, http.StatusMethodNotAllowed, "method_not_allowed", "method not allowed") @@ -28,16 +38,31 @@ func NewRouter(logger *slog.Logger, version string, assets fs.FS) (http.Handler, } r := chi.NewRouter() + r.Use(resolveOrigin(logger, trusted)) r.MethodNotAllowed(methodNotAllowed) r.NotFound(notFound) r.Get("/health", health(logger, version)) + auth.Mount(r) + // The session middleware wraps the whole /api/v1 and /media subrouters + // rather than being added with Use: chi skips a subrouter's middleware + // when it has no routes, which would leave it unguarded. + signedIn := requireSession(logger, sessions) + csrf := requireCSRF(logger) + apiV1, media := chi.NewRouter(), chi.NewRouter() + for _, sub := range []*chi.Mux{apiV1, media} { + sub.MethodNotAllowed(methodNotAllowed) + sub.NotFound(notFound) + } + apiV1.Get("/me", me(logger)) r.Route("/api", func(r chi.Router) { r.MethodNotAllowed(methodNotAllowed) r.NotFound(notFound) + r.Mount("/v1", signedIn(csrf(apiV1))) }) - // Catch-all for the PWA. /auth and /media have no routes yet; the frontend - // handler passes them to notFound rather than the SPA fallback. + r.Mount("/media", signedIn(media)) + // Catch-all for the PWA. Unknown /auth paths reach the frontend handler, + // which passes them to notFound rather than the SPA fallback. r.Method(http.MethodGet, "/*", web) r.Method(http.MethodHead, "/*", web) return r, nil diff --git a/internal/server/server_test.go b/internal/server/server_test.go index f1adc7e..7419658 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -10,6 +10,8 @@ import ( "strings" "testing" "testing/fstest" + + "github.com/go-chi/chi/v5" ) const ( @@ -25,13 +27,39 @@ func newTestRouter(t *testing.T, version string) http.Handler { "index.html": {Data: []byte(fixtureIndex)}, "assets/index-AbC123.js": {Data: []byte(fixtureAsset)}, } - r, err := NewRouter(logger, version, assets) + r, err := NewRouter(logger, version, nil, stubAuth{}, stubSessions{}, assets) if err != nil { t.Fatal(err) } return r } +// stubAuth mounts a fixed GET /auth/login in place of the real auth flow. +type stubAuth struct{} + +func (stubAuth) Mount(r chi.Router) { + r.Get("/auth/login", func(w http.ResponseWriter, _ *http.Request) { + io.WriteString(w, "stub login") + }) +} + +func TestAuthRoutesMounted(t *testing.T) { + t.Parallel() + + router := newTestRouter(t, "dev") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/auth/login", nil)) + if rec.Code != http.StatusOK || rec.Body.String() != "stub login" { + t.Errorf("GET /auth/login = %d %q, want the mounted handler", rec.Code, rec.Body.String()) + } + + rec = httptest.NewRecorder() + router.ServeHTTP(rec, httptest.NewRequest(http.MethodPost, "/auth/login", nil)) + if rec.Code != http.StatusMethodNotAllowed { + t.Errorf("POST /auth/login status = %d, want 405", rec.Code) + } +} + func TestHealth(t *testing.T) { t.Parallel() @@ -98,8 +126,10 @@ func TestReservedPathsNotFound(t *testing.T) { }{ {http.MethodGet, "/api"}, {http.MethodGet, "/api/unknown"}, + {http.MethodGet, "/api/v1/unknown"}, + {http.MethodPost, "/api/v1/unknown"}, {http.MethodPost, "/api/unknown"}, - {http.MethodGet, "/auth/callback"}, + {http.MethodGet, "/auth/unknown"}, {http.MethodGet, "/media/photo.jpg"}, {http.MethodGet, "/health/deep"}, {http.MethodGet, "/auth/../quests/42"}, @@ -109,8 +139,13 @@ func TestReservedPathsNotFound(t *testing.T) { t.Run(tt.method+" "+tt.path, func(t *testing.T) { t.Parallel() + // Signed in with the CSRF token, so /api/v1 and /media reach + // their routers. + req := httptest.NewRequest(tt.method, tt.path, nil) + req.AddCookie(validSessionCookie) + req.Header.Set(CSRFHeader, testSession.CSRFToken) rec := httptest.NewRecorder() - router.ServeHTTP(rec, httptest.NewRequest(tt.method, tt.path, nil)) + router.ServeHTTP(rec, req) if rec.Code != http.StatusNotFound { t.Errorf("status = %d, want 404", rec.Code) diff --git a/internal/server/session.go b/internal/server/session.go new file mode 100644 index 0000000..e718389 --- /dev/null +++ b/internal/server/session.go @@ -0,0 +1,37 @@ +package server + +import ( + "errors" + "log/slog" + "net/http" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth" +) + +// SessionLookup finds the live session a request carries; it returns +// auth.ErrNoSession when there is none. *auth.SessionManager implements it. +type SessionLookup interface { + SessionFor(r *http.Request) (auth.Session, error) +} + +// requireSession is the RequireSession middleware. It passes requests with a +// live session to next with the session in the context (read it with +// auth.SessionFrom or auth.ActorFrom); others get 401 unauthenticated. +func requireSession(logger *slog.Logger, sessions SessionLookup) func(http.Handler) http.Handler { + unauthenticated := apiErrorHandler(logger, http.StatusUnauthorized, "unauthenticated", "sign in required") + internal := apiErrorHandler(logger, http.StatusInternalServerError, "internal", "internal error") + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + s, err := sessions.SessionFor(r) + switch { + case errors.Is(err, auth.ErrNoSession): + unauthenticated(w, r) + case err != nil: + logger.ErrorContext(r.Context(), "look up session", "error", err) + internal(w, r) + default: + next.ServeHTTP(w, r.WithContext(auth.WithSession(r.Context(), s))) + } + }) + } +} diff --git a/internal/server/session_test.go b/internal/server/session_test.go new file mode 100644 index 0000000..d221b4c --- /dev/null +++ b/internal/server/session_test.go @@ -0,0 +1,152 @@ +package server + +import ( + "errors" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "slices" + "testing" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth" +) + +const ( + unauthenticatedJSON = `{"error":{"code":"unauthenticated","message":"sign in required","retryable":false}}` + internalJSON = `{"error":{"code":"internal","message":"internal error","retryable":false}}` +) + +var ( + validSessionCookie = &http.Cookie{Name: auth.SessionCookieName, Value: "valid"} + brokenSessionCookie = &http.Cookie{Name: auth.SessionCookieName, Value: "broken"} + testSession = auth.Session{ + Subject: "user-123", + Email: "cook@example.com", + Name: "Home Cook", + Groups: []string{"mealquest-admin"}, + CSRFToken: "csrf-token-123", + } +) + +// stubSessions knows one valid session cookie; the broken cookie fails the +// lookup as a database error would. +type stubSessions struct{} + +func (stubSessions) SessionFor(r *http.Request) (auth.Session, error) { + c, err := r.Cookie(auth.SessionCookieName) + switch { + case err != nil: + return auth.Session{}, auth.ErrNoSession + case c.Value == validSessionCookie.Value: + return testSession, nil + case c.Value == brokenSessionCookie.Value: + return auth.Session{}, errors.New("database down") + default: + return auth.Session{}, auth.ErrNoSession + } +} + +func TestProtectedPathsRequireSession(t *testing.T) { + t.Parallel() + + router := newTestRouter(t, "dev") + for _, tc := range []struct { + method, path string + cookie *http.Cookie + }{ + {http.MethodGet, "/api/v1/settings/household", nil}, + {http.MethodPatch, "/api/v1/settings/household", nil}, + {http.MethodGet, "/api/v1", nil}, + {http.MethodGet, "/media/photo.jpg", nil}, + {http.MethodGet, "/api/v1/settings/household", &http.Cookie{Name: auth.SessionCookieName, Value: "unknown"}}, + } { + t.Run(tc.method+" "+tc.path, func(t *testing.T) { + t.Parallel() + req := httptest.NewRequest(tc.method, tc.path, nil) + if tc.cookie != nil { + req.AddCookie(tc.cookie) + } + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusUnauthorized { + t.Errorf("status = %d, want 401", rec.Code) + } + if ct := rec.Header().Get("Content-Type"); ct != "application/json" { + t.Errorf("Content-Type = %q, want application/json", ct) + } + if got := rec.Body.String(); got != unauthenticatedJSON { + t.Errorf("body = %s, want %s", got, unauthenticatedJSON) + } + }) + } +} + +func TestPublicPathsNeedNoSession(t *testing.T) { + t.Parallel() + + router := newTestRouter(t, "dev") + for _, path := range []string{"/health", "/auth/login", "/", "/assets/index-AbC123.js", "/quests/42"} { + t.Run(path, func(t *testing.T) { + t.Parallel() + rec := httptest.NewRecorder() + router.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil)) + if rec.Code != http.StatusOK { + t.Errorf("GET %s without a cookie = %d, want 200", path, rec.Code) + } + }) + } +} + +func TestSessionLookupFailure(t *testing.T) { + t.Parallel() + + req := httptest.NewRequest(http.MethodGet, "/api/v1/settings/household", nil) + req.AddCookie(brokenSessionCookie) + rec := httptest.NewRecorder() + newTestRouter(t, "dev").ServeHTTP(rec, req) + + if rec.Code != http.StatusInternalServerError || rec.Body.String() != internalJSON { + t.Errorf("response = %d %s, want 500 %s", rec.Code, rec.Body.String(), internalJSON) + } +} + +func TestRequireSessionAttachesActor(t *testing.T) { + t.Parallel() + + var got auth.Actor + var ok bool + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got, ok = auth.ActorFrom(r.Context()) + w.WriteHeader(http.StatusNoContent) + }) + h := requireSession(slog.New(slog.NewTextHandler(io.Discard, nil)), stubSessions{})(next) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/settings/household", nil) + req.AddCookie(validSessionCookie) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + if rec.Code != http.StatusNoContent { + t.Fatalf("status = %d, want the handler's 204", rec.Code) + } + if !ok || got.Subject != "user-123" || got.Email != "cook@example.com" || got.Name != "Home Cook" || + !slices.Equal(got.Groups, []string{"mealquest-admin"}) { + t.Errorf("actor = %+v (ok=%v), want the session's household member", got, ok) + } +} + +func TestRequireSessionStopsUnauthenticated(t *testing.T) { + t.Parallel() + + called := false + next := http.HandlerFunc(func(http.ResponseWriter, *http.Request) { called = true }) + h := requireSession(slog.New(slog.NewTextHandler(io.Discard, nil)), stubSessions{})(next) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/api/v1/settings/household", nil)) + + if called || rec.Code != http.StatusUnauthorized { + t.Errorf("handler called = %v, status = %d; want not called and 401", called, rec.Code) + } +}