Answer 503 while the session store is down, and stop only on a refused login
Fourth section of #141. At start the console probes the session store once. A store that refuses the console's credentials (WRONGPASS, NOAUTH, NOPERM, or an ERR reply to AUTH other than "max number of clients reached") stops the start, as before; anything else, a refused or silent address or a store that answers LOADING, BUSY or MASTERDOWN, is logged as "session store not ready at startup; requests that need a session answer 503" and the console serves. The probe takes its connection with pool.GetContext, so the dial's error is classified apart from PING's. valkey-addr is read once. Every store error a request meets now answers a styled 503 "Sign-in unavailable" with Retry-After: 5 and a "Try again" action to the request's own path (a GET or HEAD that validReturnTo accepts) or to /, in place of scs's bare 500; for /callback the action is /login, because the authorization code it carries is spent once. The store is wrapped so that its errors are tagged; scs's ErrorFunc answers a tagged error with the 503 and logs "session store request failed", and any other error, a session that cannot be decoded or encoded, with the 500 and "session could not be decoded or encoded". The handlers' own store calls (RenewToken in /login and /register, Destroy in POST /logout) answer a store error through the same path. (*Config).LoadAndSave wraps the writer in a latch: once the ErrorFunc answers, nothing the handler set before reaches the client, since every HX- header, Location, Set-Cookie, Content-Type and Content-Length is deleted and the latched writer drops later writes and headers. A request without a session cookie makes no store call, and the routes that need no session (the static files, the favicon, and the exempt webhook and ask paths) skip the session layer. start now exits 1 when serving fails, after "server failed to start", so an orchestrator that restarts only on failure restarts it. Full suite on reset databases: 4456 pass, 15 skip, none failed. The console booted on the test stack with the session store connected. go test -race -p 1 over internal/auth and internal/server after reset-test-db.sh: every package passes, no data race. make lint passes. Breaks, each caught: - probe fatal for every error, as today - probe fatal for every error reply, LOADING included - probe never fatal - max clients fatal (the exception removed) - ErrorFunc not set (scs default) - store not wrapped (errors returned untagged) - every error classified as a store error - latch not marked - latch not marked (HX-Redirect row) - latched writer still passes a body - ErrorFunc deletes a fixed list of headers without HX-Refresh - 503 without Retry-After - action always / - action built from RequestURI without validReturnTo - skipSession without /static/ - skipSession without /favicon.ico - skipSession without the exempt list - the ErrorFunc keeps Location - the ErrorFunc keeps every HX- header - the probe takes its connection without GetContext - an ERR reply to AUTH not fatal - the PING path classified as the dial - a header set after the 503 follows it under TimeoutHandler - tagStoreError without its nil check - copy 17 at Warn - copy 17 without addr - Retry-After set for both branches - login keeps its 500 on a store error - register keeps its 500 on a store error - logout keeps its 500 on a store error - logout before discovery keeps its 500 on a store error - callback keeps its 500 on a store error - the 503 for a callback repeats the spent callback
This commit is contained in:
@@ -532,6 +532,10 @@ var startCmd = &cobra.Command{
|
||||
// Start the server
|
||||
if err := server.Start(ctx, serverConfig); err != nil {
|
||||
logger.Error("server failed to start", slog.Any("error", err))
|
||||
// A process that stopped serving exits non-zero, so an
|
||||
// orchestrator that restarts only on failure restarts it
|
||||
// (design D18).
|
||||
os.Exit(1)
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
+46
-12
@@ -45,6 +45,10 @@ import (
|
||||
// cannot import the server to reach the renderer directly (design.md, D1).
|
||||
type FailurePage interface {
|
||||
RenderAuthFailure(w http.ResponseWriter, r *http.Request, status int, heading, actionLabel, actionHref string)
|
||||
|
||||
// RenderErrorPage renders the ordinary error page, for a failure that is
|
||||
// not a refused flow: a session the codec cannot decode.
|
||||
RenderErrorPage(w http.ResponseWriter, r *http.Request, status int, msg string)
|
||||
}
|
||||
|
||||
// authFailure is what a person is told when a flow refuses. Every refusal in
|
||||
@@ -179,17 +183,30 @@ func (c *Config) showFailure(w http.ResponseWriter, r *http.Request, status int,
|
||||
// Setup builds the auth configuration over the session store. It probes the
|
||||
// store once and fetches nothing from the identity provider: the Config it
|
||||
// returns has empty discovery holders, and Discover fills them (design D3).
|
||||
//
|
||||
// The probe fails Setup only when the store refuses the console's credentials,
|
||||
// which a restart does not change. A store that does not answer, or answers
|
||||
// with an error such as LOADING while it reads its dataset, is logged and
|
||||
// Setup continues: the pool dials again on each request, and the routes that
|
||||
// need no session serve meanwhile (design D14).
|
||||
func Setup(database *sql.DB, identityQ identity.Querier, orgQ organization.Querier) (*Config, error) {
|
||||
ctx := context.Background()
|
||||
return setup(context.Background(), database, identityQ, orgQ)
|
||||
}
|
||||
|
||||
// setup is Setup under a context that carries the logger, which a test
|
||||
// replaces with one it reads.
|
||||
func setup(ctx context.Context, database *sql.DB, identityQ identity.Querier, orgQ organization.Querier) (*Config, error) {
|
||||
logger := logging.FromContext(ctx)
|
||||
|
||||
// Create Redis pool for Valkey
|
||||
// The pool's dial takes the address once, here, so no later dial reads
|
||||
// viper (design D13).
|
||||
addr := viper.GetString("valkey-addr")
|
||||
dialOptions := valkeyDialOptions(ctx)
|
||||
pool := &redis.Pool{
|
||||
MaxIdle: 10,
|
||||
IdleTimeout: 240 * time.Second,
|
||||
Dial: func() (redis.Conn, error) {
|
||||
return redis.Dial("tcp", viper.GetString("valkey-addr"), dialOptions...)
|
||||
return redis.Dial("tcp", addr, dialOptions...)
|
||||
},
|
||||
TestOnBorrow: func(c redis.Conn, t time.Time) error {
|
||||
if time.Since(t) < time.Minute {
|
||||
@@ -200,14 +217,16 @@ func Setup(database *sql.DB, identityQ identity.Querier, orgQ organization.Queri
|
||||
},
|
||||
}
|
||||
|
||||
// Test connection
|
||||
conn := pool.Get()
|
||||
defer func() { _ = conn.Close() }() // returns the probe connection to the pool; a failed return discards it
|
||||
if _, err := conn.Do("PING"); err != nil {
|
||||
return nil, fmt.Errorf("failed to connect to Valkey: %w", err)
|
||||
switch probe := probeStore(ctx, pool); {
|
||||
case probe.refused:
|
||||
return nil, fmt.Errorf("failed to connect to Valkey: %w", probe.err)
|
||||
case probe.err != nil:
|
||||
logger.Error("session store not ready at startup; requests that need a session answer 503",
|
||||
slog.String("addr", addr),
|
||||
slog.Any("error", probe.err))
|
||||
default:
|
||||
logger.Info("session store connected", slog.String("addr", addr))
|
||||
}
|
||||
logger.Info("session store connected",
|
||||
slog.String("addr", viper.GetString("valkey-addr")))
|
||||
|
||||
// Nothing is fetched from the identity provider here: Discover does that
|
||||
// in the background, so the console serves the routes that need only the
|
||||
@@ -259,8 +278,8 @@ func (c *Config) RegisterHandlers(mux *http.ServeMux) {
|
||||
// webhooks checking an HMAC signature) — they must bypass session auth or
|
||||
// the provider's delivery bounces off the /login redirect. The server
|
||||
// passes the paths collected from RouteMount.CSRFExemptPaths: a path that
|
||||
// authenticates by signature is exempt from both protections for the same
|
||||
// reason.
|
||||
// authenticates by signature is exempt from all three for the same reason: cross-origin protection,
|
||||
// session auth and the session layer.
|
||||
func (c *Config) Middleware(selfAuthenticatedPaths ...string) func(http.Handler) http.Handler {
|
||||
publicPaths := map[string]bool{
|
||||
"/login": true,
|
||||
@@ -445,6 +464,9 @@ func (c *Config) LoginHandler(w http.ResponseWriter, r *http.Request) {
|
||||
// to the IdP, guaranteeing the cookie that returns to /callback maps
|
||||
// to the session we just wrote state into.
|
||||
if err := c.SessionManager.RenewToken(r.Context()); err != nil {
|
||||
if c.answerStoreError(w, r, err) {
|
||||
return
|
||||
}
|
||||
logging.FromContext(r.Context()).Error("sign-in could not renew the session token",
|
||||
slog.Any("error", err))
|
||||
c.showFailure(w, r, http.StatusInternalServerError, signInFailed)
|
||||
@@ -652,6 +674,9 @@ func (c *Config) CallbackHandler(w http.ResponseWriter, r *http.Request) {
|
||||
// else is lost) and issues a fresh token under which the identity below
|
||||
// is the first thing written.
|
||||
if err := c.SessionManager.RenewToken(ctx); err != nil {
|
||||
if c.answerStoreError(w, r, err) {
|
||||
return
|
||||
}
|
||||
logger.Error("callback could not renew the session token",
|
||||
slog.Any("error", err))
|
||||
c.showFailure(w, r, http.StatusInternalServerError, signInFailed)
|
||||
@@ -795,6 +820,9 @@ func (c *Config) LogoutHandler(w http.ResponseWriter, r *http.Request) {
|
||||
endSessionURL.RawQuery = q.Encode()
|
||||
|
||||
if err := c.SessionManager.Destroy(ctx); err != nil {
|
||||
if c.answerStoreError(w, r, err) {
|
||||
return
|
||||
}
|
||||
logger.Error("could not end the session at sign-out",
|
||||
slog.Any("error", err))
|
||||
c.showFailure(w, r, http.StatusInternalServerError, signOutFailed)
|
||||
@@ -839,6 +867,9 @@ func (c *Config) logoutBeforeSignIn(w http.ResponseWriter, r *http.Request, v *d
|
||||
|
||||
storedRefreshToken := c.SessionManager.GetString(ctx, sessionKeyRefreshToken)
|
||||
if err := c.SessionManager.Destroy(ctx); err != nil {
|
||||
if c.answerStoreError(w, r, err) {
|
||||
return
|
||||
}
|
||||
logger.Error("could not end the session at sign-out",
|
||||
slog.Any("error", err))
|
||||
c.showFailure(w, r, http.StatusInternalServerError, signOutFailed)
|
||||
@@ -1062,6 +1093,9 @@ func (c *Config) RegistrationHandler(w http.ResponseWriter, r *http.Request) {
|
||||
// cookie returning to /callback maps to the session the state was written
|
||||
// into.
|
||||
if err := c.SessionManager.RenewToken(ctx); err != nil {
|
||||
if c.answerStoreError(w, r, err) {
|
||||
return
|
||||
}
|
||||
logger.Error("registration could not renew the session token",
|
||||
slog.Any("error", err))
|
||||
c.showFailure(w, r, http.StatusInternalServerError, registrationFailed)
|
||||
|
||||
@@ -19,6 +19,11 @@ type fakeFailurePage struct {
|
||||
heading string
|
||||
actionLabel string
|
||||
actionHref string
|
||||
|
||||
// errorStatus and errorMessage record RenderErrorPage, which the session layer uses for a
|
||||
// session the codec cannot decode.
|
||||
errorStatus int
|
||||
errorMessage string
|
||||
}
|
||||
|
||||
func (f *fakeFailurePage) RenderAuthFailure(w http.ResponseWriter, r *http.Request, status int, heading, actionLabel, actionHref string) {
|
||||
@@ -31,6 +36,13 @@ func (f *fakeFailurePage) RenderAuthFailure(w http.ResponseWriter, r *http.Reque
|
||||
_, _ = fmt.Fprintf(w, `<h1>%s</h1><a href="%s">%s</a>`, heading, actionHref, actionLabel) // the test reads this body
|
||||
}
|
||||
|
||||
func (f *fakeFailurePage) RenderErrorPage(w http.ResponseWriter, r *http.Request, status int, msg string) {
|
||||
f.errorStatus = status
|
||||
f.errorMessage = msg
|
||||
w.WriteHeader(status)
|
||||
_, _ = fmt.Fprintf(w, `<h1>%s</h1>`, msg) // the test reads this body
|
||||
}
|
||||
|
||||
// callbackWithStaleState drives /callback with a session holding savedState
|
||||
// and a query carrying queryState, which is the shape a sign-in tab left open
|
||||
// too long comes back in.
|
||||
|
||||
@@ -59,13 +59,24 @@ const discoveryEscalation = 90 * time.Second
|
||||
|
||||
// NewAwaitingDiscovery returns a Config over sessionManager whose discovery
|
||||
// holders are empty. Discover fills them; until it has, the sign-in routes
|
||||
// answer 503 (design D3). Setup builds its Config with it.
|
||||
// answer 503 (design D3). Setup builds its Config with it, and so does the
|
||||
// server's test of the session-free routes.
|
||||
//
|
||||
// It also gives the session manager what a running console needs of its
|
||||
// store (design D14): the store is wrapped so that every error it returns is
|
||||
// tagged, and the manager's ErrorFunc answers 503 for a tagged error. Both
|
||||
// are set here so that Setup and the tests get them from one place.
|
||||
func NewAwaitingDiscovery(sessionManager *scs.SessionManager) *Config {
|
||||
return &Config{
|
||||
if _, tagged := sessionManager.Store.(taggedStore); !tagged {
|
||||
sessionManager.Store = taggedStore{sessionManager.Store}
|
||||
}
|
||||
c := &Config{
|
||||
SessionManager: sessionManager,
|
||||
awaiting: true,
|
||||
wake: make(chan struct{}, 1),
|
||||
}
|
||||
sessionManager.ErrorFunc = c.sessionErrorFunc
|
||||
return c
|
||||
}
|
||||
|
||||
// verifier returns the verifier value, which is nil for a Config built by
|
||||
|
||||
@@ -0,0 +1,245 @@
|
||||
// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial
|
||||
// SPDX-FileCopyrightText: 2025-2026 Christian Galo
|
||||
|
||||
package auth
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
|
||||
"github.com/alexedwards/scs/v2"
|
||||
|
||||
"git.coopcloud.tech/wiki-cafe/member-console/internal/logging"
|
||||
)
|
||||
|
||||
// storeError marks an error that came from the session store, as opposed to
|
||||
// one that came from decoding what the store returned. The session manager's
|
||||
// ErrorFunc answers 503 for the first and the styled 500 for the second
|
||||
// (design D14).
|
||||
type storeError struct{ err error }
|
||||
|
||||
func (e *storeError) Error() string { return e.err.Error() }
|
||||
func (e *storeError) Unwrap() error { return e.err }
|
||||
|
||||
// tagStoreError wraps err in a storeError, and leaves nil alone.
|
||||
func tagStoreError(err error) error {
|
||||
if err == nil {
|
||||
return nil
|
||||
}
|
||||
return &storeError{err: err}
|
||||
}
|
||||
|
||||
// taggedStore wraps the session manager's store so that every error the store
|
||||
// returns is a storeError. The classification is by origin and not by error
|
||||
// type, because no list of types can tell the store from the codec: the
|
||||
// codec's decode returns io.EOF and io.ErrUnexpectedEOF for an empty or
|
||||
// truncated value, which a list that included them would answer 503 for the
|
||||
// cookie's week, and redigo returns a failed TLS handshake and a protocol
|
||||
// error as types that no list names.
|
||||
//
|
||||
// It implements the Ctx variants scs prefers; the store underneath gets its
|
||||
// own Ctx variant when it has one. Neither redisstore nor memstore has them
|
||||
// today; the branches keep a future CtxStore's context intact.
|
||||
type taggedStore struct{ scs.Store }
|
||||
|
||||
func (s taggedStore) Find(token string) ([]byte, bool, error) {
|
||||
b, found, err := s.Store.Find(token)
|
||||
return b, found, tagStoreError(err)
|
||||
}
|
||||
|
||||
func (s taggedStore) Commit(token string, b []byte, expiry time.Time) error {
|
||||
return tagStoreError(s.Store.Commit(token, b, expiry))
|
||||
}
|
||||
|
||||
func (s taggedStore) Delete(token string) error {
|
||||
return tagStoreError(s.Store.Delete(token))
|
||||
}
|
||||
|
||||
func (s taggedStore) FindCtx(ctx context.Context, token string) ([]byte, bool, error) {
|
||||
cs, ok := s.Store.(scs.CtxStore)
|
||||
if !ok {
|
||||
return s.Find(token)
|
||||
}
|
||||
b, found, err := cs.FindCtx(ctx, token)
|
||||
return b, found, tagStoreError(err)
|
||||
}
|
||||
|
||||
func (s taggedStore) CommitCtx(ctx context.Context, token string, b []byte, expiry time.Time) error {
|
||||
cs, ok := s.Store.(scs.CtxStore)
|
||||
if !ok {
|
||||
return s.Commit(token, b, expiry)
|
||||
}
|
||||
return tagStoreError(cs.CommitCtx(ctx, token, b, expiry))
|
||||
}
|
||||
|
||||
func (s taggedStore) DeleteCtx(ctx context.Context, token string) error {
|
||||
cs, ok := s.Store.(scs.CtxStore)
|
||||
if !ok {
|
||||
return s.Delete(token)
|
||||
}
|
||||
return tagStoreError(cs.DeleteCtx(ctx, token))
|
||||
}
|
||||
|
||||
// latchWriter is the response writer the session layer hands to the session
|
||||
// manager. scs commits the session inside the handler's first WriteHeader or
|
||||
// Write and then passes that write on to the response anyway, so a handler
|
||||
// that set headers before its first write (LoginHandler's Location,
|
||||
// bounceToLogin's HX-Redirect) would have them copied into the 503 the
|
||||
// ErrorFunc wrote, and its body would follow the page. Once the ErrorFunc has
|
||||
// answered it latches the writer: every later WriteHeader and Write is
|
||||
// dropped, and the header map a handler gets afterwards is a throwaway one,
|
||||
// because http.TimeoutHandler, which sits outside, copies the header map it
|
||||
// handed out when the handler returns.
|
||||
type latchWriter struct {
|
||||
http.ResponseWriter
|
||||
latched atomic.Bool
|
||||
}
|
||||
|
||||
func (l *latchWriter) latch() { l.latched.Store(true) }
|
||||
|
||||
func (l *latchWriter) Header() http.Header {
|
||||
if l.latched.Load() {
|
||||
return http.Header{}
|
||||
}
|
||||
return l.ResponseWriter.Header()
|
||||
}
|
||||
|
||||
func (l *latchWriter) WriteHeader(status int) {
|
||||
if l.latched.Load() {
|
||||
return
|
||||
}
|
||||
l.ResponseWriter.WriteHeader(status)
|
||||
}
|
||||
|
||||
func (l *latchWriter) Write(b []byte) (int, error) {
|
||||
if l.latched.Load() {
|
||||
return len(b), nil
|
||||
}
|
||||
return l.ResponseWriter.Write(b)
|
||||
}
|
||||
|
||||
func (l *latchWriter) Unwrap() http.ResponseWriter { return l.ResponseWriter }
|
||||
|
||||
// LoadAndSave is the session layer: the session manager's LoadAndSave over a
|
||||
// latch writer, which is what lets the ErrorFunc's answer be the whole
|
||||
// answer. The latch lives here because the ErrorFunc, which marks it, is in
|
||||
// this package and cannot import the server.
|
||||
func (c *Config) LoadAndSave(next http.Handler) http.Handler {
|
||||
inner := c.SessionManager.LoadAndSave(next)
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
inner.ServeHTTP(&latchWriter{ResponseWriter: w}, r)
|
||||
})
|
||||
}
|
||||
|
||||
// sessionUnavailable is copy row 3. Its action repeats the request: the
|
||||
// request's own path for a GET or HEAD when validReturnTo accepts it, and the
|
||||
// console's root otherwise, because a request line such as //host/x reaches
|
||||
// the session layer before the mux and would make a protocol-relative link.
|
||||
// A callback is the exception: its authorization code is spent once, so a
|
||||
// repeated callback fails, and the action starts sign-in again at /login.
|
||||
func sessionUnavailable(r *http.Request) authFailure {
|
||||
failure := authFailure{"Sign-in unavailable", "Try again", "/"}
|
||||
if r.URL.Path == "/callback" {
|
||||
failure.actionHref = "/login"
|
||||
return failure
|
||||
}
|
||||
if r.Method == http.MethodGet || r.Method == http.MethodHead {
|
||||
if dest, ok := validReturnTo(r.URL.RequestURI()); ok {
|
||||
failure.actionHref = dest
|
||||
}
|
||||
}
|
||||
return failure
|
||||
}
|
||||
|
||||
// unexpectedErrorMessage is the recovery page's message, reused for a session
|
||||
// the codec cannot decode.
|
||||
// The recovery handler in internal/server repeats the literal.
|
||||
const unexpectedErrorMessage = "An unexpected error occurred. Try again."
|
||||
|
||||
// sessionErrorFunc is the session manager's ErrorFunc (design D14). It answers
|
||||
// 503 with Retry-After for an error the tagged store returned, and the styled
|
||||
// 500 page for any other error, and writes one structured line for either in
|
||||
// place of scs's unstructured one.
|
||||
//
|
||||
// It first deletes what a handler may have set before the commit failed:
|
||||
// every HX- header (HX-Redirect, HX-Refresh and HX-Trigger among them, because
|
||||
// a handler such as the workspace partial puts a value in the session and sets
|
||||
// HX-Refresh without writing, and htmx reloads the page on it), Location,
|
||||
// Set-Cookie and Content-Type, and Content-Length, which would not describe
|
||||
// the page. Either answer latches the writer when it is the session layer's.
|
||||
func (c *Config) sessionErrorFunc(w http.ResponseWriter, r *http.Request, err error) {
|
||||
dropHandlerHeaders(w.Header())
|
||||
logger := logging.FromContext(r.Context())
|
||||
attrs := []any{
|
||||
slog.String("method", r.Method),
|
||||
slog.String("path", r.URL.Path),
|
||||
slog.Any("error", err),
|
||||
}
|
||||
|
||||
var tagged *storeError
|
||||
if errors.As(err, &tagged) {
|
||||
logger.Error("session store request failed", attrs...)
|
||||
w.Header().Set("Retry-After", "5")
|
||||
c.showFailure(w, r, http.StatusServiceUnavailable, sessionUnavailable(r))
|
||||
} else {
|
||||
logger.Error("session could not be decoded or encoded", attrs...)
|
||||
c.showErrorPage(w, r, http.StatusInternalServerError, unexpectedErrorMessage)
|
||||
}
|
||||
|
||||
if lw, ok := w.(*latchWriter); ok {
|
||||
lw.latch()
|
||||
}
|
||||
}
|
||||
|
||||
// answerStoreError is a handler's way into the ErrorFunc's 503: when err is a
|
||||
// store error it answers through sessionErrorFunc, on the latch writer under
|
||||
// the session manager's own, so the answer does not pass through a second
|
||||
// commit, and reports true. For any other error it does nothing and reports
|
||||
// false, and the handler keeps its own answer (design D14).
|
||||
func (c *Config) answerStoreError(w http.ResponseWriter, r *http.Request, err error) bool {
|
||||
var tagged *storeError
|
||||
if !errors.As(err, &tagged) {
|
||||
return false
|
||||
}
|
||||
for cur := w; ; {
|
||||
if lw, ok := cur.(*latchWriter); ok {
|
||||
w = lw
|
||||
break
|
||||
}
|
||||
u, ok := cur.(interface{ Unwrap() http.ResponseWriter })
|
||||
if !ok {
|
||||
break
|
||||
}
|
||||
cur = u.Unwrap()
|
||||
}
|
||||
c.sessionErrorFunc(w, r, err)
|
||||
return true
|
||||
}
|
||||
|
||||
// dropHandlerHeaders removes the headers a handler may have set before its
|
||||
// answer was replaced.
|
||||
func dropHandlerHeaders(h http.Header) {
|
||||
for name := range h {
|
||||
if strings.HasPrefix(strings.ToLower(name), "hx-") {
|
||||
delete(h, name)
|
||||
}
|
||||
}
|
||||
for _, name := range []string{"Location", "Set-Cookie", "Content-Type", "Content-Length"} {
|
||||
h.Del(name)
|
||||
}
|
||||
}
|
||||
|
||||
// showErrorPage answers with the styled error page, or with msg as plain text
|
||||
// when no renderer is wired.
|
||||
func (c *Config) showErrorPage(w http.ResponseWriter, r *http.Request, status int, msg string) {
|
||||
if c.Failures != nil {
|
||||
c.Failures.RenderErrorPage(w, r, status, msg)
|
||||
return
|
||||
}
|
||||
http.Error(w, msg, status)
|
||||
}
|
||||
@@ -0,0 +1,666 @@
|
||||
// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial
|
||||
// SPDX-FileCopyrightText: 2025-2026 Christian Galo
|
||||
|
||||
package auth
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"context"
|
||||
"crypto/tls"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"log/slog"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/alexedwards/scs/v2"
|
||||
"github.com/alexedwards/scs/v2/memstore"
|
||||
"golang.org/x/oauth2"
|
||||
|
||||
"git.coopcloud.tech/wiki-cafe/member-console/internal/logging"
|
||||
"git.coopcloud.tech/wiki-cafe/member-console/internal/testkit"
|
||||
)
|
||||
|
||||
// These tests cover the session store's two failure classes (design D14): a
|
||||
// store that refuses the console's credentials stops the start, and a store
|
||||
// that does not answer leaves the console serving and answers 503 for the
|
||||
// requests that need a session.
|
||||
|
||||
const (
|
||||
copy17 = "session store not ready at startup; requests that need a session answer 503"
|
||||
copy19 = "session store request failed"
|
||||
// copy19Decode is copy row 19's second message.
|
||||
copy19Decode = "session could not be decoded or encoded"
|
||||
|
||||
// copy3Heading and copy3Action are copy row 3.
|
||||
copy3Heading = "Sign-in unavailable"
|
||||
copy3Action = "Try again"
|
||||
|
||||
// recoveryMessage is the recovery page's message, reused for a session the
|
||||
// codec cannot decode.
|
||||
recoveryMessage = "An unexpected error occurred. Try again."
|
||||
)
|
||||
|
||||
// storeServer listens on 127.0.0.1:0 and hands every connection to serve. It
|
||||
// stops with the test.
|
||||
func storeServer(t *testing.T, serve func(net.Conn)) string {
|
||||
t.Helper()
|
||||
lis, err := net.Listen("tcp", "127.0.0.1:0")
|
||||
if err != nil {
|
||||
t.Fatalf("listen: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { testkit.Close(t, lis) })
|
||||
go func() {
|
||||
for {
|
||||
conn, err := lis.Accept()
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
go func() {
|
||||
// The client may have closed first, which is not a failure of the fake.
|
||||
defer func() { _ = conn.Close() }()
|
||||
serve(conn)
|
||||
}()
|
||||
}
|
||||
}()
|
||||
return lis.Addr().String()
|
||||
}
|
||||
|
||||
// readCommand consumes one RESP command (an array of bulk strings), which is
|
||||
// how a client sends AUTH and PING.
|
||||
func readCommand(r *bufio.Reader) error {
|
||||
header, err := r.ReadString('\n')
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
count, err := strconv.Atoi(strings.TrimSpace(strings.TrimPrefix(header, "*")))
|
||||
if err != nil {
|
||||
return fmt.Errorf("command header %q: %w", header, err)
|
||||
}
|
||||
for range count {
|
||||
sizeLine, err := r.ReadString('\n')
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
size, err := strconv.Atoi(strings.TrimSpace(strings.TrimPrefix(sizeLine, "$")))
|
||||
if err != nil {
|
||||
return fmt.Errorf("bulk header %q: %w", sizeLine, err)
|
||||
}
|
||||
if _, err := io.CopyN(io.Discard, r, int64(size)+2); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// loadingStore answers every command with -LOADING, as a store does while it
|
||||
// reads its dataset.
|
||||
func loadingStore(t *testing.T) string {
|
||||
return storeServer(t, func(conn net.Conn) {
|
||||
r := bufio.NewReader(conn)
|
||||
// The loop ends when the client closes or sends something that is not a command.
|
||||
for readCommand(r) == nil {
|
||||
if _, err := io.WriteString(conn, "-LOADING Redis is loading the dataset in memory\r\n"); err != nil {
|
||||
return
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// busyStore writes -ERR max number of clients reached to a new connection
|
||||
// before it reads any command, as a store over maxclients does, then reads
|
||||
// the client's first command so that closing does not reset the line away.
|
||||
func busyStore(t *testing.T) string {
|
||||
return storeServer(t, func(conn net.Conn) {
|
||||
if _, err := io.WriteString(conn, "-ERR max number of clients reached\r\n"); err != nil {
|
||||
return
|
||||
}
|
||||
_ = readCommand(bufio.NewReader(conn)) // only waits for the client; the command's content is not used
|
||||
})
|
||||
}
|
||||
|
||||
// scriptedStore answers the n-th command it reads with replies[n] and closes
|
||||
// the connection after the last reply, as a store that accepts AUTH and then
|
||||
// answers PING with an error does.
|
||||
func scriptedStore(t *testing.T, replies ...string) string {
|
||||
return storeServer(t, func(conn net.Conn) {
|
||||
r := bufio.NewReader(conn)
|
||||
for _, reply := range replies {
|
||||
if readCommand(r) != nil {
|
||||
return
|
||||
}
|
||||
if _, err := io.WriteString(conn, reply); err != nil {
|
||||
return
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// refusedAddress is an address that was listened on and closed.
|
||||
func refusedAddress(t *testing.T) string {
|
||||
t.Helper()
|
||||
lis, err := net.Listen("tcp", "127.0.0.1:0")
|
||||
if err != nil {
|
||||
t.Fatalf("listen: %v", err)
|
||||
}
|
||||
addr := lis.Addr().String()
|
||||
if err := lis.Close(); err != nil {
|
||||
t.Fatalf("close: %v", err)
|
||||
}
|
||||
return addr
|
||||
}
|
||||
|
||||
func TestSetupRefusesAStoreThatRefuses(t *testing.T) {
|
||||
addr := os.Getenv("MC_VALKEY_ADDR")
|
||||
if addr == "" || os.Getenv("MC_VALKEY_PASSWORD") == "" {
|
||||
t.Skip("MC_VALKEY_ADDR or MC_VALKEY_PASSWORD is not set; the test stack's session store is not available")
|
||||
}
|
||||
setViper(t, "valkey-addr", addr)
|
||||
setViper(t, "valkey-password", os.Getenv("MC_VALKEY_PASSWORD")+"-not-the-password")
|
||||
|
||||
cfg, err := Setup(nil, nil, nil)
|
||||
if err == nil {
|
||||
t.Fatalf("Setup against a store that refuses the password returned a Config (%v), want an error", cfg)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "failed to connect to Valkey") {
|
||||
t.Errorf("Setup error = %q, want it to contain \"failed to connect to Valkey\"", err)
|
||||
}
|
||||
}
|
||||
|
||||
// An ERR reply to AUTH means the store refuses the console, and it needs no
|
||||
// stack: the fake sends it in answer to the first command.
|
||||
func TestSetupRefusesAnERRReplyToAUTH(t *testing.T) {
|
||||
setViper(t, "valkey-addr", scriptedStore(t, "-ERR invalid password\r\n"))
|
||||
setViper(t, "valkey-password", "a-password")
|
||||
|
||||
cfg, err := setup(t.Context(), nil, nil, nil)
|
||||
|
||||
if err == nil {
|
||||
t.Fatalf("setup returned a Config (%v) for an ERR reply to AUTH, want an error", cfg)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "failed to connect to Valkey") {
|
||||
t.Errorf("setup error = %q, want it to contain \"failed to connect to Valkey\"", err)
|
||||
}
|
||||
}
|
||||
|
||||
// storeRow is one session store Setup meets.
|
||||
type storeRow struct {
|
||||
name string
|
||||
addr func(t *testing.T) string
|
||||
}
|
||||
|
||||
func runSetupWithoutStore(t *testing.T, row storeRow) {
|
||||
addr := row.addr(t)
|
||||
setViper(t, "valkey-addr", addr)
|
||||
setViper(t, "valkey-password", "a-password")
|
||||
logs := &discoverytestLogs{}
|
||||
ctx := logging.WithContext(t.Context(), slog.New(logs))
|
||||
|
||||
cfg, err := setup(ctx, nil, nil, nil)
|
||||
|
||||
if err != nil {
|
||||
t.Fatalf("setup returned an error for a store that gave no refusal: %v", err)
|
||||
}
|
||||
if cfg == nil {
|
||||
t.Fatal("setup returned no Config")
|
||||
}
|
||||
lines := logs.withMessage(copy17)
|
||||
if len(lines) != 1 {
|
||||
t.Fatalf("copy 17 was logged %d times, want once", len(lines))
|
||||
}
|
||||
if lines[0].level != slog.LevelError {
|
||||
t.Errorf("copy 17 level = %v, want Error", lines[0].level)
|
||||
}
|
||||
if got := lines[0].attrs["addr"].String(); got != addr {
|
||||
t.Errorf("copy 17 addr = %q, want %q", got, addr)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetupServesWithoutAStore(t *testing.T) {
|
||||
rows := []storeRow{
|
||||
{"a port that refuses", refusedAddress},
|
||||
{"a store that answers -LOADING", loadingStore},
|
||||
{"a store over maxclients", busyStore},
|
||||
{"a store that accepts AUTH and answers PING with -LOADING", func(t *testing.T) string {
|
||||
return scriptedStore(t, "+OK\r\n", "-LOADING Redis is loading the dataset in memory\r\n")
|
||||
}},
|
||||
{"a store that accepts AUTH and answers PING with -ERR", func(t *testing.T) string {
|
||||
return scriptedStore(t, "+OK\r\n", "-ERR unknown command 'PING'\r\n")
|
||||
}},
|
||||
}
|
||||
for _, row := range rows {
|
||||
t.Run(row.name, func(t *testing.T) { runSetupWithoutStore(t, row) })
|
||||
}
|
||||
}
|
||||
|
||||
// failingStore is a session store whose Find and Commit fail with the errors
|
||||
// the test gives, or return a value the codec cannot decode.
|
||||
type failingStore struct {
|
||||
findErr error
|
||||
commitErr error
|
||||
deleteErr error
|
||||
// value is what Find returns, found, when findErr is nil and value is set.
|
||||
value []byte
|
||||
// deletes counts the sessions the store was asked to delete.
|
||||
deletes int
|
||||
}
|
||||
|
||||
func (s *failingStore) Find(string) ([]byte, bool, error) {
|
||||
if s.findErr != nil {
|
||||
return nil, false, s.findErr
|
||||
}
|
||||
return s.value, s.value != nil, nil
|
||||
}
|
||||
|
||||
func (s *failingStore) Commit(string, []byte, time.Time) error { return s.commitErr }
|
||||
|
||||
func (s *failingStore) Delete(string) error {
|
||||
s.deletes++
|
||||
return s.deleteErr
|
||||
}
|
||||
|
||||
// failurePage is the renderer the session layer is given: the styled page for
|
||||
// a browser, the heading as text for an htmx request, as the server's
|
||||
// template set does.
|
||||
type failurePage struct{ fakeFailurePage }
|
||||
|
||||
func (f *failurePage) RenderAuthFailure(w http.ResponseWriter, r *http.Request, status int, heading, actionLabel, actionHref string) {
|
||||
if r.Header.Get("HX-Request") == "true" {
|
||||
http.Error(w, heading, status)
|
||||
return
|
||||
}
|
||||
f.fakeFailurePage.RenderAuthFailure(w, r, status, heading, actionLabel, actionHref)
|
||||
}
|
||||
|
||||
// publishDiscovery stores the discovery holders directly, which a test inside
|
||||
// the package can do, so that /login gets past its gate.
|
||||
func publishDiscovery(cfg *Config) {
|
||||
v := &discovered{oauth: &oauth2.Config{
|
||||
ClientID: "console",
|
||||
RedirectURL: "https://console.test/callback",
|
||||
Endpoint: oauth2.Endpoint{AuthURL: "https://idp.test/auth", TokenURL: "https://idp.test/token"},
|
||||
}}
|
||||
cfg.verifierValue.Store(v)
|
||||
cfg.signInValue.Store(&signInState{verifier: v, endSession: "https://idp.test/logout"})
|
||||
}
|
||||
|
||||
// outage is the error a store returns when its connection fails.
|
||||
func outage() error {
|
||||
return &net.OpError{Op: "read", Net: "tcp", Err: errors.New("connection reset by peer")}
|
||||
}
|
||||
|
||||
// storeHarness is a Config built by NewAwaitingDiscovery over store, with the
|
||||
// logs its requests write.
|
||||
type storeHarness struct {
|
||||
cfg *Config
|
||||
page *failurePage
|
||||
logs *discoverytestLogs
|
||||
}
|
||||
|
||||
func newStoreHarness(store scs.Store) *storeHarness {
|
||||
sm := scs.New()
|
||||
sm.Store = store
|
||||
cfg := NewAwaitingDiscovery(sm)
|
||||
h := &storeHarness{cfg: cfg, page: &failurePage{}, logs: &discoverytestLogs{}}
|
||||
cfg.Failures = h.page
|
||||
return h
|
||||
}
|
||||
|
||||
// serve sends req through the session layer over handler.
|
||||
func (h *storeHarness) serve(handler http.Handler, req *http.Request) *httptest.ResponseRecorder {
|
||||
return h.serveThrough(h.cfg.LoadAndSave(handler), req)
|
||||
}
|
||||
|
||||
func (h *storeHarness) serveThrough(layer http.Handler, req *http.Request) *httptest.ResponseRecorder {
|
||||
req = req.WithContext(logging.WithContext(req.Context(), slog.New(h.logs)))
|
||||
rec := httptest.NewRecorder()
|
||||
layer.ServeHTTP(rec, req)
|
||||
return rec
|
||||
}
|
||||
|
||||
// pageHandler is a handler that answers a page.
|
||||
var pageHandler = http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, _ = io.WriteString(w, "the page") // the test reads this body
|
||||
})
|
||||
|
||||
// storeFailureRow is one request the session layer meets while the store
|
||||
// fails.
|
||||
type storeFailureRow struct {
|
||||
name string
|
||||
method string
|
||||
target string
|
||||
cookie bool
|
||||
htmx bool
|
||||
// timeout serves the request through http.TimeoutHandler outside the
|
||||
// session layer, as the server's stack does.
|
||||
timeout bool
|
||||
store *failingStore
|
||||
// handler is the handler under the session layer; pageHandler when nil.
|
||||
handler func(h *storeHarness) http.Handler
|
||||
// noDiscovery leaves the discovery document unpublished.
|
||||
noDiscovery bool
|
||||
// action is the href the styled page's one action leads to; empty for a
|
||||
// row that checks no page.
|
||||
action string
|
||||
// text is the plain text an htmx request gets.
|
||||
text string
|
||||
// absent are headers the answer must not carry.
|
||||
absent []string
|
||||
// pageOnly asserts that the body is the styled page and nothing after it.
|
||||
pageOnly bool
|
||||
}
|
||||
|
||||
func (row storeFailureRow) request() *http.Request {
|
||||
req := httptest.NewRequest(row.method, row.target, nil)
|
||||
if row.cookie {
|
||||
req.AddCookie(&http.Cookie{Name: "session", Value: "a-token"})
|
||||
}
|
||||
if row.htmx {
|
||||
req.Header.Set("HX-Request", "true")
|
||||
}
|
||||
return req
|
||||
}
|
||||
|
||||
// checkLoggedStoreFailure asserts copy 19's first message with its attributes.
|
||||
func checkLoggedStoreFailure(t *testing.T, h *storeHarness, row storeFailureRow) {
|
||||
t.Helper()
|
||||
lines := h.logs.withMessage(copy19)
|
||||
if len(lines) != 1 {
|
||||
t.Fatalf("copy 19 was logged %d times, want once", len(lines))
|
||||
}
|
||||
line := lines[0]
|
||||
if line.level != slog.LevelError {
|
||||
t.Errorf("copy 19 level = %v, want Error", line.level)
|
||||
}
|
||||
if got := line.attrs["method"].String(); got != row.method {
|
||||
t.Errorf("copy 19 method = %q, want %q", got, row.method)
|
||||
}
|
||||
if got, want := line.attrs["path"].String(), strings.SplitN(row.target, "?", 2)[0]; got != want {
|
||||
t.Errorf("copy 19 path = %q, want %q", got, want)
|
||||
}
|
||||
if line.attrs["error"].String() == "" {
|
||||
t.Error("copy 19 carries no error")
|
||||
}
|
||||
}
|
||||
|
||||
// checkStoreFailureAnswer asserts the 503 and what it must not carry.
|
||||
func checkStoreFailureAnswer(t *testing.T, h *storeHarness, row storeFailureRow, rec *httptest.ResponseRecorder) {
|
||||
t.Helper()
|
||||
if rec.Code != http.StatusServiceUnavailable {
|
||||
t.Fatalf("status = %d, want 503", rec.Code)
|
||||
}
|
||||
if got := rec.Header().Get("Retry-After"); got != "5" {
|
||||
t.Errorf("Retry-After = %q, want 5", got)
|
||||
}
|
||||
for _, name := range row.absent {
|
||||
if got := rec.Header().Get(name); got != "" {
|
||||
t.Errorf("the 503 carries %s = %q the handler set", name, got)
|
||||
}
|
||||
}
|
||||
if row.text != "" && strings.TrimSpace(rec.Body.String()) != row.text {
|
||||
t.Errorf("body = %q, want the plain text %q", rec.Body.String(), row.text)
|
||||
}
|
||||
if row.action != "" {
|
||||
if h.page.heading != copy3Heading || h.page.actionLabel != copy3Action || h.page.actionHref != row.action {
|
||||
t.Errorf("page = %q / %q to %q, want %q / %q to %q",
|
||||
h.page.heading, h.page.actionLabel, h.page.actionHref, copy3Heading, copy3Action, row.action)
|
||||
}
|
||||
}
|
||||
if row.pageOnly {
|
||||
want := fmt.Sprintf(`<h1>%s</h1><a href="%s">%s</a>`, copy3Heading, row.action, copy3Action)
|
||||
if rec.Body.String() != want {
|
||||
t.Errorf("body = %q, want the styled page alone, %q", rec.Body.String(), want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func runStoreFailureRow(t *testing.T, row storeFailureRow) {
|
||||
h := newStoreHarness(row.store)
|
||||
if !row.noDiscovery {
|
||||
publishDiscovery(h.cfg)
|
||||
}
|
||||
var handler http.Handler = pageHandler
|
||||
if row.handler != nil {
|
||||
handler = row.handler(h)
|
||||
}
|
||||
|
||||
var rec *httptest.ResponseRecorder
|
||||
if row.timeout {
|
||||
rec = h.serveThrough(http.TimeoutHandler(h.cfg.LoadAndSave(handler), time.Minute, ""), row.request())
|
||||
} else {
|
||||
rec = h.serve(handler, row.request())
|
||||
}
|
||||
|
||||
checkStoreFailureAnswer(t, h, row, rec)
|
||||
checkLoggedStoreFailure(t, h, row)
|
||||
}
|
||||
|
||||
// bounceShape sets HX-Redirect before it writes, as bounceToLogin does for a
|
||||
// request with no session: the value it stored makes the commit happen at the
|
||||
// first write.
|
||||
func bounceShape(h *storeHarness) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
h.cfg.SessionManager.Put(r.Context(), "return-to", "/somewhere")
|
||||
w.Header().Set("HX-Redirect", "/login")
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, _ = io.WriteString(w, "bounce body") // the test reads this body
|
||||
})
|
||||
}
|
||||
|
||||
// refreshShape puts a value in the session and sets HX-Refresh without
|
||||
// writing, as the workspace partial does (workspace_partials.go:211-214); the
|
||||
// commit happens after the handler returns.
|
||||
func refreshShape(h *storeHarness) http.Handler {
|
||||
return http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
|
||||
h.cfg.SessionManager.Put(r.Context(), "workspace", "w1")
|
||||
})
|
||||
}
|
||||
|
||||
// lateHeaderShape writes its status, which commits the session and fails, and
|
||||
// sets HX-Trigger after that, as a handler that sets headers after its first
|
||||
// write does.
|
||||
func lateHeaderShape(h *storeHarness) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
h.cfg.SessionManager.Put(r.Context(), "return-to", "/somewhere")
|
||||
w.WriteHeader(http.StatusOK)
|
||||
w.Header().Set("HX-Trigger", "x")
|
||||
_, _ = io.WriteString(w, "late body") // the test reads this body
|
||||
})
|
||||
}
|
||||
|
||||
func withRefresh(next func(h *storeHarness) http.Handler) func(h *storeHarness) http.Handler {
|
||||
return func(h *storeHarness) http.Handler {
|
||||
inner := next(h)
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("HX-Refresh", "true")
|
||||
inner.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// callbackHandler is CallbackHandler over the provider fakes of fx, published
|
||||
// as the discovery document so that the callback gets past its gate.
|
||||
func callbackHandler(fx callbackTestFixture) func(h *storeHarness) http.Handler {
|
||||
return func(h *storeHarness) http.Handler {
|
||||
v := &discovered{oauth: fx.cfg.OAuthConfig, verifier: fx.cfg.Verifier}
|
||||
h.cfg.verifierValue.Store(v)
|
||||
h.cfg.signInValue.Store(&signInState{verifier: v})
|
||||
h.cfg.IdentityQ = fx.cfg.IdentityQ
|
||||
h.cfg.OrgQ = fx.cfg.OrgQ
|
||||
return http.HandlerFunc(h.cfg.CallbackHandler)
|
||||
}
|
||||
}
|
||||
|
||||
// callbackStore is a store whose Find answers with the pending-flow session
|
||||
// /login leaves (the state runSuccessfulCallback sends, nonce and code
|
||||
// verifier) and whose Delete fails, so the failure is RenewToken's.
|
||||
func callbackStore(t *testing.T, nonce string) *failingStore {
|
||||
t.Helper()
|
||||
value, err := scs.New().Codec.Encode(time.Now().Add(time.Hour), map[string]any{
|
||||
sessionKeyState: "the-state",
|
||||
sessionKeyNonce: nonce,
|
||||
sessionKeyCodeVerifier: "verifier-1",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("encode: %v", err)
|
||||
}
|
||||
return &failingStore{value: value, deleteErr: outage()}
|
||||
}
|
||||
|
||||
func loginHandler(h *storeHarness) http.Handler { return http.HandlerFunc(h.cfg.LoginHandler) }
|
||||
|
||||
func registrationHandler(h *storeHarness) http.Handler {
|
||||
return http.HandlerFunc(h.cfg.RegistrationHandler)
|
||||
}
|
||||
|
||||
func logoutHandler(h *storeHarness) http.Handler { return http.HandlerFunc(h.cfg.LogoutHandler) }
|
||||
|
||||
// findThenDelete is a store whose Find answers with a session the codec can
|
||||
// decode and whose Delete fails, so the failure comes after the Find, from
|
||||
// the handler's own call (RenewToken or Destroy).
|
||||
func findThenDelete(t *testing.T) *failingStore {
|
||||
t.Helper()
|
||||
value, err := scs.New().Codec.Encode(time.Now().Add(time.Hour), map[string]any{"key": "a value"})
|
||||
if err != nil {
|
||||
t.Fatalf("encode: %v", err)
|
||||
}
|
||||
return &failingStore{value: value, deleteErr: outage()}
|
||||
}
|
||||
|
||||
func TestSessionStoreFailureAnswers503(t *testing.T) {
|
||||
find := func() *failingStore { return &failingStore{findErr: outage()} }
|
||||
commit := func() *failingStore { return &failingStore{commitErr: outage()} }
|
||||
fx := newCallbackTestFixture(t)
|
||||
rows := []storeFailureRow{
|
||||
{name: "a browser GET with a cookie", method: "GET", target: "/some/page?x=1", cookie: true,
|
||||
store: find(), action: "/some/page?x=1"},
|
||||
{name: "an htmx request with a cookie", method: "GET", target: "/partials/x", cookie: true, htmx: true,
|
||||
store: find(), text: copy3Heading},
|
||||
{name: "a POST with a cookie", method: "POST", target: "/some/page", cookie: true,
|
||||
store: find(), action: "/"},
|
||||
{name: "a GET to a protocol-relative target", method: "GET", target: "//evil.example/x", cookie: true,
|
||||
store: find(), action: "/"},
|
||||
{name: "a store that returns a TLS error", method: "GET", target: "/some/page", cookie: true,
|
||||
store: &failingStore{findErr: tls.RecordHeaderError{Msg: "first record does not look like a TLS handshake"}},
|
||||
action: "/some/page"},
|
||||
{name: "/login without a cookie, the commit fails", method: "GET", target: "/login",
|
||||
store: commit(), handler: loginHandler, action: "/login",
|
||||
absent: []string{"Location"}, pageOnly: true},
|
||||
{name: "a handler that sets HX-Redirect before it writes", method: "GET", target: "/bounce",
|
||||
store: commit(), handler: bounceShape, action: "/bounce",
|
||||
absent: []string{"HX-Redirect"}, pageOnly: true},
|
||||
{name: "a handler that sets HX-Refresh and writes nothing", method: "POST", target: "/workspace",
|
||||
store: commit(), handler: withRefresh(refreshShape), action: "/",
|
||||
absent: []string{"HX-Refresh"}},
|
||||
{name: "login, the store fails after the Find answered", method: "GET", target: "/login", cookie: true,
|
||||
store: findThenDelete(t), handler: loginHandler, action: "/login", pageOnly: true},
|
||||
{name: "register, the store fails after the Find answered", method: "GET", target: "/register", cookie: true,
|
||||
store: findThenDelete(t), handler: registrationHandler, action: "/register", pageOnly: true},
|
||||
{name: "callback, the store fails after the Find answered", method: "GET", target: "/callback?state=the-state&code=code-1", cookie: true,
|
||||
store: callbackStore(t, fx.nonce), handler: callbackHandler(fx), action: "/login", pageOnly: true},
|
||||
{name: "logout, the store fails after the Find answered", method: "POST", target: "/logout", cookie: true,
|
||||
store: findThenDelete(t), handler: logoutHandler, action: "/", pageOnly: true},
|
||||
{name: "logout before discovery, the store fails after the Find answered", method: "POST", target: "/logout", cookie: true,
|
||||
store: findThenDelete(t), handler: logoutHandler, noDiscovery: true, action: "/", pageOnly: true},
|
||||
{name: "a header set after the 503, under http.TimeoutHandler", method: "GET", target: "/late",
|
||||
store: commit(), handler: lateHeaderShape, timeout: true, action: "/late",
|
||||
absent: []string{"HX-Trigger"}, pageOnly: true},
|
||||
}
|
||||
for _, row := range rows {
|
||||
t.Run(row.name, func(t *testing.T) { runStoreFailureRow(t, row) })
|
||||
}
|
||||
}
|
||||
|
||||
// undecodableRow is one answer of the store that the codec cannot decode.
|
||||
type undecodableRow struct {
|
||||
name string
|
||||
value func(t *testing.T, sm *scs.SessionManager) []byte
|
||||
}
|
||||
|
||||
func garbage(*testing.T, *scs.SessionManager) []byte { return []byte("not a gob stream at all") }
|
||||
|
||||
// emptyValue is a stored value of no bytes: gob reports io.EOF.
|
||||
func emptyValue(*testing.T, *scs.SessionManager) []byte { return []byte{} }
|
||||
|
||||
// truncatedValue is a valid session cut short: gob reports io.ErrUnexpectedEOF.
|
||||
func truncatedValue(t *testing.T, sm *scs.SessionManager) []byte {
|
||||
t.Helper()
|
||||
full, err := sm.Codec.Encode(time.Now().Add(time.Hour), map[string]any{"key": "a value long enough to cut"})
|
||||
if err != nil {
|
||||
t.Fatalf("encode: %v", err)
|
||||
}
|
||||
return full[:len(full)/2]
|
||||
}
|
||||
|
||||
func runUndecodableRow(t *testing.T, row undecodableRow) {
|
||||
probe := scs.New()
|
||||
h := newStoreHarness(&failingStore{value: row.value(t, probe)})
|
||||
|
||||
req := httptest.NewRequest("GET", "/some/page", nil)
|
||||
req.AddCookie(&http.Cookie{Name: "session", Value: "a-token"})
|
||||
rec := h.serve(pageHandler, req)
|
||||
|
||||
if rec.Code != http.StatusInternalServerError {
|
||||
t.Errorf("status = %d, want 500", rec.Code)
|
||||
}
|
||||
if h.page.errorStatus != http.StatusInternalServerError || h.page.errorMessage != recoveryMessage {
|
||||
t.Errorf("error page = %d %q, want 500 %q", h.page.errorStatus, h.page.errorMessage, recoveryMessage)
|
||||
}
|
||||
if got := rec.Header().Get("Retry-After"); got != "" {
|
||||
t.Errorf("Retry-After = %q, want none", got)
|
||||
}
|
||||
if n := len(h.logs.withMessage(copy19Decode)); n != 1 {
|
||||
t.Errorf("copy 19's second message was logged %d times, want once", n)
|
||||
}
|
||||
if n := len(h.logs.withMessage(copy19)); n != 0 {
|
||||
t.Errorf("copy 19's first message was logged %d times for a decode failure, want none", n)
|
||||
}
|
||||
}
|
||||
|
||||
func TestUndecodableSessionAnswers500(t *testing.T) {
|
||||
rows := []undecodableRow{
|
||||
{"bytes the codec cannot decode", garbage},
|
||||
{"an empty value", emptyValue},
|
||||
{"a truncated value", truncatedValue},
|
||||
}
|
||||
for _, row := range rows {
|
||||
t.Run(row.name, func(t *testing.T) { runUndecodableRow(t, row) })
|
||||
}
|
||||
}
|
||||
|
||||
// The tagged store marks what the store returns and nothing else.
|
||||
func TestTaggedStoreTagsEveryStoreError(t *testing.T) {
|
||||
boom := errors.New("boom")
|
||||
ctx := context.Background()
|
||||
store := taggedStore{&failingStore{findErr: boom, commitErr: boom, deleteErr: boom}}
|
||||
calls := map[string]func() error{
|
||||
"Find": func() error { _, _, err := store.Find("t"); return err },
|
||||
"Commit": func() error { return store.Commit("t", nil, time.Time{}) },
|
||||
"Delete": func() error { return store.Delete("t") },
|
||||
"FindCtx": func() error { _, _, err := store.FindCtx(ctx, "t"); return err },
|
||||
"CommitCtx": func() error { return store.CommitCtx(ctx, "t", nil, time.Time{}) },
|
||||
"DeleteCtx": func() error { return store.DeleteCtx(ctx, "t") },
|
||||
}
|
||||
for name, call := range calls {
|
||||
var tagged *storeError
|
||||
if err := call(); !errors.As(err, &tagged) || !errors.Is(err, boom) {
|
||||
t.Errorf("%s error = %v, want a storeError wrapping the store's", name, err)
|
||||
}
|
||||
}
|
||||
|
||||
mem := taggedStore{memstore.New()}
|
||||
if _, _, err := mem.FindCtx(ctx, "t"); err != nil {
|
||||
t.Errorf("a store that answers is tagged: %T", err)
|
||||
}
|
||||
if err := mem.DeleteCtx(ctx, "t"); err != nil {
|
||||
t.Errorf("a store that answers is tagged: %T", err)
|
||||
}
|
||||
}
|
||||
@@ -6,7 +6,9 @@ package auth
|
||||
import (
|
||||
"context"
|
||||
"crypto/tls"
|
||||
"errors"
|
||||
"log/slog"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/gomodule/redigo/redis"
|
||||
@@ -73,3 +75,61 @@ func valkeyDialOptions(ctx context.Context) []redis.DialOption {
|
||||
|
||||
return options
|
||||
}
|
||||
|
||||
// storeProbe is what Setup learned from its one connection to the session
|
||||
// store (design D14).
|
||||
type storeProbe struct {
|
||||
// err is whatever the dial or the PING returned, nil when the store
|
||||
// answered.
|
||||
err error
|
||||
// refused is true only when err is the store refusing the console's
|
||||
// credentials, which a restart does not change and which therefore stops
|
||||
// the start. Every other err is a state that changes: the store is down,
|
||||
// slow, busy or still loading its dataset.
|
||||
refused bool
|
||||
}
|
||||
|
||||
// probeStore takes one connection from pool and sends it a PING.
|
||||
//
|
||||
// The connection comes from pool.GetContext, which returns the dial's error
|
||||
// apart from the PING's. pool.Get hides a failed dial inside an errorConn
|
||||
// whose Do returns the same error, so a reply to AUTH and a reply to PING
|
||||
// would come out of one call and could not be told apart; the reply to AUTH
|
||||
// is the only one whose "ERR" prefix means a refusal.
|
||||
func probeStore(ctx context.Context, pool *redis.Pool) storeProbe {
|
||||
conn, err := pool.GetContext(ctx)
|
||||
if err != nil {
|
||||
return storeProbe{err: err, refused: refusesAuthentication(err, true)}
|
||||
}
|
||||
defer func() { _ = conn.Close() }() // returns the probe connection to the pool; a failed return discards it
|
||||
if _, err := conn.Do("PING"); err != nil {
|
||||
return storeProbe{err: err, refused: refusesAuthentication(err, false)}
|
||||
}
|
||||
return storeProbe{}
|
||||
}
|
||||
|
||||
// maxClientsReply is what a Valkey over maxclients writes to a new connection
|
||||
// before it reads any command. redigo returns that line as the reply to AUTH,
|
||||
// so it looks like an ERR reply to AUTH, but it means the store is busy and
|
||||
// not that it refuses the console.
|
||||
const maxClientsReply = "ERR max number of clients reached"
|
||||
|
||||
// refusesAuthentication reports whether err is the store refusing the
|
||||
// console's credentials: an error reply beginning WRONGPASS, NOAUTH or
|
||||
// NOPERM, or, when err came from the dial, an ERR reply to AUTH that is not
|
||||
// the max-clients line. A reply such as LOADING, BUSY or MASTERDOWN says the
|
||||
// store is not ready, not that it refuses the console, and a connection that
|
||||
// fails to open is not a reply at all.
|
||||
func refusesAuthentication(err error, duringDial bool) bool {
|
||||
var reply redis.Error
|
||||
if !errors.As(err, &reply) {
|
||||
return false
|
||||
}
|
||||
text := string(reply)
|
||||
for _, prefix := range []string{"WRONGPASS", "NOAUTH", "NOPERM"} {
|
||||
if strings.HasPrefix(text, prefix) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return duringDial && strings.HasPrefix(text, "ERR") && !strings.HasPrefix(text, maxClientsReply)
|
||||
}
|
||||
|
||||
@@ -13,6 +13,8 @@ import (
|
||||
"net"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"slices"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"git.coopcloud.tech/wiki-cafe/member-console/internal/auth"
|
||||
@@ -153,7 +155,9 @@ type Deps struct {
|
||||
// constructs and registers its routes against the shared mux once Deps are
|
||||
// available, and CSRFExemptPaths lists any request paths that must bypass
|
||||
// CSRF protection (e.g. a webhook authenticated by the provider's own
|
||||
// signature instead of a CSRF token). Produced from integrations
|
||||
// signature instead of a CSRF token). A declared path also skips the session
|
||||
// layer and the auth middleware, so its handler must not read the session.
|
||||
// Produced from integrations
|
||||
// implementing RouteProvider (below); NewHandler consumes the slice without
|
||||
// naming any integration handler type, and CSRF exemptions are declared by
|
||||
// the route instead of hardcoded in NewHandler.
|
||||
@@ -202,7 +206,8 @@ type UIMount struct {
|
||||
// CSRFExemptPaths lists request paths that must bypass CSRF protection
|
||||
// (e.g. a webhook endpoint authenticated by provider signature instead) —
|
||||
// the integration declares the exemption itself rather than core
|
||||
// hardcoding the path.
|
||||
// hardcoding the path. A declared path also skips the session layer and the
|
||||
// auth middleware, so its handler must not read the session.
|
||||
type RouteProvider interface {
|
||||
RegisterRoutes(mux *http.ServeMux, deps Deps) error
|
||||
CSRFExemptPaths() []string
|
||||
@@ -441,7 +446,7 @@ func newRouter(cfg Config, authConfig *auth.Config) (*router, error) {
|
||||
// this for requests it has already confirmed match a real pattern.
|
||||
routeAware := newRouteAwareHandler(httpRequestRouter, authConfig.Middleware(csrfExemptPaths...), safeTmpl)
|
||||
|
||||
handler := preAuthStack(safeTmpl, csrfMiddleware, authConfig.SessionManager.LoadAndSave)(routeAware)
|
||||
handler := preAuthStack(safeTmpl, csrfMiddleware, skipSession(csrfExemptPaths, authConfig.LoadAndSave))(routeAware)
|
||||
|
||||
// Guard: any unmatched /partials/ path returns 404 rather than falling
|
||||
// through to the "/" catch-all (which would serve the dashboard HTML
|
||||
@@ -483,7 +488,7 @@ func registerIntegrationRoutes(httpRequestRouter *http.ServeMux, cfg Config, aut
|
||||
// domains registry, so no integration is involved in the lookup. Core
|
||||
// therefore seeds its own exemption here — the slice was previously
|
||||
// assembled only from integration mounts, and it flows to BOTH the
|
||||
// cross-origin bypass patterns and authConfig.Middleware. /domains/ask
|
||||
// cross-origin bypass patterns, to authConfig.Middleware and to skipSession. /domains/ask
|
||||
// is unauthenticated by design: the TLS-terminating proxy that calls it
|
||||
// during a handshake holds no session.
|
||||
// The handler itself is registered later (after safeTmpl exists): a
|
||||
@@ -710,6 +715,7 @@ func preAuthStack(safeTmpl *SafeTemplates, csrfMiddleware, loadAndSave middlewar
|
||||
middleware.Logging(), // Log requests with structured logging
|
||||
// Catch all panics; the styled 500 page never echoes the panic value
|
||||
middleware.Recovery(func(w http.ResponseWriter, r *http.Request) {
|
||||
// The message repeats auth.unexpectedErrorMessage.
|
||||
safeTmpl.RenderErrorPage(w, r, http.StatusInternalServerError, "An unexpected error occurred. Try again.")
|
||||
}),
|
||||
// Headers before the timeout and the body limit: both answer for the
|
||||
@@ -727,6 +733,34 @@ func preAuthStack(safeTmpl *SafeTemplates, csrfMiddleware, loadAndSave middlewar
|
||||
)
|
||||
}
|
||||
|
||||
// skipSession wraps loadAndSave so that a request that reads no session never
|
||||
// reaches the session store (design D14): a request under /static/, one for
|
||||
// /favicon.ico, and one whose path is in exempt, which is /domains/ask and the
|
||||
// paths the mounts declare self-authenticated (the webhooks). The 503 page's
|
||||
// own stylesheet request carries the session cookie, so without the skip it
|
||||
// would 503 as well and the page would render unstyled; the ask and the
|
||||
// webhooks read no session, so a caller that happens to carry a cookie must
|
||||
// not depend on the store. /favicon.ico has no route: the styled 404 answers
|
||||
// it, and needs no session either.
|
||||
func skipSession(exempt []string, loadAndSave middleware.Middleware) middleware.Middleware {
|
||||
return func(next http.Handler) http.Handler {
|
||||
withSession := loadAndSave(next)
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if readsNoSession(r.URL.Path, exempt) {
|
||||
next.ServeHTTP(w, r)
|
||||
return
|
||||
}
|
||||
withSession.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// readsNoSession reports whether the path is one skipSession serves without
|
||||
// loading the session.
|
||||
func readsNoSession(path string, exempt []string) bool {
|
||||
return strings.HasPrefix(path, "/static/") || path == "/favicon.ico" || slices.Contains(exempt, path)
|
||||
}
|
||||
|
||||
// registerMemberPages registers the member pages: the dashboard at "/", the
|
||||
// Products and Billing pages, and the redirect from the dissolved Domains
|
||||
// page. Each composes its regions in-process through include.
|
||||
|
||||
@@ -0,0 +1,79 @@
|
||||
// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial
|
||||
// SPDX-FileCopyrightText: 2025-2026 Christian Galo
|
||||
|
||||
package server
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"net/http"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/alexedwards/scs/v2"
|
||||
|
||||
"git.coopcloud.tech/wiki-cafe/member-console/internal/auth"
|
||||
)
|
||||
|
||||
// outageStore is a session store that works until the test says it does not:
|
||||
// the member's cookie is committed while it works, and every request after
|
||||
// fails the way a store that went away does.
|
||||
type outageStore struct {
|
||||
scs.Store
|
||||
down atomic.Bool
|
||||
}
|
||||
|
||||
var errStoreDown = errors.New("session store is down")
|
||||
|
||||
func (s *outageStore) Find(token string) ([]byte, bool, error) {
|
||||
if s.down.Load() {
|
||||
return nil, false, errStoreDown
|
||||
}
|
||||
return s.Store.Find(token)
|
||||
}
|
||||
|
||||
func (s *outageStore) Commit(token string, b []byte, expiry time.Time) error {
|
||||
if s.down.Load() {
|
||||
return errStoreDown
|
||||
}
|
||||
return s.Store.Commit(token, b, expiry)
|
||||
}
|
||||
|
||||
// sessionFreeRow is one request, carrying a session cookie, and the status it
|
||||
// gets while the store is down.
|
||||
type sessionFreeRow struct {
|
||||
name string
|
||||
method string
|
||||
path string
|
||||
want int
|
||||
}
|
||||
|
||||
func runSessionFreeRow(t *testing.T, env *routeEnv, row sessionFreeRow) {
|
||||
rec := env.serve(probe(row.method, row.path, func(r *http.Request) { r.AddCookie(env.member) }))
|
||||
if rec.Code != row.want {
|
||||
t.Errorf("%s %s with a session cookie = %d, want %d", row.method, row.path, rec.Code, row.want)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSessionFreeRoutesSkipTheStore: the routes that read no session answer
|
||||
// as they do without a store, and a page that does read one answers 503
|
||||
// (design D14). The route environment is built over a store that works, so
|
||||
// the member's cookie exists, and the store then goes down.
|
||||
func TestSessionFreeRoutesSkipTheStore(t *testing.T) {
|
||||
sm := scs.New()
|
||||
store := &outageStore{Store: sm.Store}
|
||||
sm.Store = store
|
||||
env := newRouteEnvOver(t, nil, auth.NewAwaitingDiscovery(sm))
|
||||
store.down.Store(true)
|
||||
|
||||
rows := []sessionFreeRow{
|
||||
{"the stylesheet", http.MethodGet, "/static/app.css", http.StatusOK},
|
||||
{"the favicon, which no route serves", http.MethodGet, "/favicon.ico", http.StatusNotFound},
|
||||
{"the ask without its domain", http.MethodGet, "/domains/ask", http.StatusBadRequest},
|
||||
{"the fake mount's declared path", http.MethodPost, "/hooks/test", http.StatusNoContent},
|
||||
{"a page that reads the session", http.MethodGet, "/", http.StatusServiceUnavailable},
|
||||
}
|
||||
for _, row := range rows {
|
||||
t.Run(row.name, func(t *testing.T) { runSessionFreeRow(t, env, row) })
|
||||
}
|
||||
}
|
||||
@@ -127,7 +127,7 @@ Reason: viper is not safe for a write concurrent with reads. Today the only writ
|
||||
|
||||
`auth.Setup` probes the store once with the existing dial timeouts (connect 5 seconds, read and write 3 seconds). The probe takes its connection with `pool.GetContext`, which returns the dial's error apart from `PING`'s (`pool.Get` hides it inside an `errorConn`, so a reply to `AUTH` and a reply to `PING` would come out of the same `Do`). An error reply that refuses authentication (a `redis.Error` beginning `WRONGPASS`, `NOAUTH` or `NOPERM`, or an `ERR` reply to `AUTH` during the dial) fails `Setup`, and `start` exits 1, with one exception: a Valkey over `maxclients` writes `-ERR max number of clients reached` to a new connection before it reads any command, redigo returns that line as the reply to `AUTH`, and it means the store is busy, not that it refuses the console, so it is logged as copy row 17 and `Setup` continues. Any other probe error is logged at Error (copy row 17) and `Setup` continues; the pool dials again on each request. That covers a refused or timed-out connection, a TLS handshake that fails, and the error replies that say the store is busy, not that it refuses the console: `LOADING` (the store answers `PING` with it while it loads its dataset, and `AUTH` is allowed then), `BUSY`, `MASTERDOWN` and the max-clients reply.
|
||||
|
||||
The classification is by origin, not by error type. The store handed to the session manager is wrapped (`taggedStore{scs.Store}`, implementing `Find`, `Commit` and `Delete` and the `Ctx` variants scs prefers) so that every error the store returns is wrapped in `storeError`. `NewAwaitingDiscovery` does the wrapping and sets the session manager's `ErrorFunc`, so `Setup` and the server's tests get both from one place. The `ErrorFunc` answers 503, `Retry-After: 5` and copy row 3 exactly when `errors.As(err, &storeError)`, and the styled 500 page, as the default does today, for any other error (a session the codec cannot decode); each writes one structured log line (copy row 19) in place of scs's unstructured one. The page's action is `validReturnTo(r.URL.RequestURI())` for a GET or HEAD and `/` when it refuses or for any other method, because a request line such as `//host/x` reaches `LoadAndSave` before the mux.
|
||||
The classification is by origin, not by error type. The store handed to the session manager is wrapped (`taggedStore{scs.Store}`, implementing `Find`, `Commit` and `Delete` and the `Ctx` variants scs prefers) so that every error the store returns is wrapped in `storeError`. `NewAwaitingDiscovery` does the wrapping and sets the session manager's `ErrorFunc`, so `Setup` and the server's tests get both from one place. The `ErrorFunc` answers 503, `Retry-After: 5` and copy row 3 exactly when `errors.As(err, &storeError)`, and the styled 500 page, as the default does today, for any other error (a session the codec cannot decode); each writes one structured log line (copy row 19) in place of scs's unstructured one. A handler's own store call answers through the same path: `LoginHandler`'s and `RegistrationHandler`'s `RenewToken` and `LogoutHandler`'s `Destroy` (both of its branches) call `(*Config).answerStoreError(w, r, err)`, which, when `errors.As(err, &storeError)`, hands the latch writer under the session manager's own to the `ErrorFunc` (the latch, the deleted headers, copy row 3, `Retry-After: 5` and copy row 19) and reports true, so the handler returns; for any other error it reports false and the handler keeps its own 500. This covers a store that answered the `Find` and fails the next call, a window one request wide. The page's action is `validReturnTo(r.URL.RequestURI())` for a GET or HEAD and `/` when it refuses or for any other method, because a request line such as `//host/x` reaches `LoadAndSave` before the mux.
|
||||
|
||||
The session layer is wrapped in a latch writer, which lives in `internal/auth` beside the `ErrorFunc` (`internal/auth` cannot import `internal/server`): `(*Config).LoadAndSave(next)` wraps the response writer and calls the session manager's `LoadAndSave`, and `preAuthStack` takes that method. scs commits inside the handler's first `WriteHeader` or `Write` and then passes that write on to the response anyway, so a handler that set headers before its first write (`LoginHandler`'s `Location`, `bounceToLogin`'s `HX-Redirect`) would have them copied into the 503 and its body appended to the page. The `ErrorFunc`, handed the latch writer, deletes what a handler may have set (every header whose name begins `HX-`, such as `HX-Redirect`, `HX-Refresh` and `HX-Trigger`, plus `Location`, `Set-Cookie` and `Content-Type`; a handler such as the workspace partial puts a value in the session and sets `HX-Refresh: true` without writing, and htmx 4 reloads the page on it), writes its 503 or 500, and marks the latch; the latch drops every later `WriteHeader` and `Write`. Requests to `/static/`, `/domains/ask`, `/favicon.ico` and the paths the integrations declare self-authenticated (the webhooks) skip the session middleware.
|
||||
|
||||
@@ -313,6 +313,7 @@ A request that read an empty holder answers for that state even if the holder is
|
||||
| F35 | A burst of sign-in requests while the identity provider refuses fast | the wake | D3: at most one attempt per second, one at a time; a wake does not shorten a wait that follows an unusable document, so such a document is fetched at the cap |
|
||||
| F46 | The 503 page's action built from `RequestURI` unchecked: a request line `//host/x` becomes a protocol-relative link | `ErrorFunc` | D14: `validReturnTo`, and `/` when it refuses |
|
||||
| F47 | Clearing a grant's end date refused though it starts no workflow | `operator_enrollment.go` | D15: only a set, changed bound is refused |
|
||||
| F48 | A store that fails mid-request: the `Find` answered, and the handler's own `RenewToken` (`/login`, `/register`) or `Destroy` (`POST /logout`) fails, so the handler answers its own 500 and not copy 3's 503 | the handlers' own store calls | D14: `answerStoreError` hands a tagged error to the `ErrorFunc`; any other error keeps the handler's 500 |
|
||||
| F36 | Two publishers of the Temporal client | `clientholder` | `Publish` panics on a second call |
|
||||
| F37 | A Temporal that goes away after publication | | Out of scope (#208); the holder is never emptied, so handlers fail on their own errors as they do today |
|
||||
| F38 | One `ErrorFunc` line per failing request during a store outage | `ErrorFunc` | Accepted: the same count as scs's line today, now structured |
|
||||
@@ -329,7 +330,7 @@ Every sentence a person reads that this change adds or changes. The UI copy rule
|
||||
|---|---|---|
|
||||
| 1 | `/login` and `/callback` before discovery: the styled page's heading and action (to `/login`); the heading alone for htmx | heading `Sign-in unavailable`, action `Try again` |
|
||||
| 2 | `/register` before discovery (action to `/register`) | heading `Registration unavailable`, action `Try again` |
|
||||
| 3 | A request the session store cannot serve (action to the request's path for a GET or HEAD when `validReturnTo` accepts it, `/` otherwise) | heading `Sign-in unavailable`, action `Try again` |
|
||||
| 3 | A request the session store cannot serve (action to the request's path for a GET or HEAD when `validReturnTo` accepts it, `/` otherwise; `/login` for `/callback`, whose authorization code is spent once) | heading `Sign-in unavailable`, action `Try again` |
|
||||
| 4 | `POST /webhooks/stripe` before Temporal connects, the 503 body Stripe shows in its delivery log | `Temporal is not connected` |
|
||||
| 5 | `core.webhook_events.error_message` of a Discourse event before Temporal connects | `start reconcile: Temporal is not connected` |
|
||||
| 6 | Issue grant and grant management, the end date's field error while Temporal is not connected | `An end date needs the expiration scheduler, which is not connected.` |
|
||||
@@ -365,7 +366,7 @@ Removed: `Grant issued, but expiration scheduling FAILED (no scheduler); it will
|
||||
| 16 | connector, the worker fails to build or start (Error from the first line), reused | `failed to create Temporal worker` and `failed to start Temporal worker`, attributes `error`, `retry_in` |
|
||||
| 17 | `auth.Setup`, the session store gives no answer, or an error reply that is not an authentication refusal such as `LOADING` (Error) | `session store not ready at startup; requests that need a session answer 503`, attributes `addr`, `error` |
|
||||
| 18 | `start`, the session store refuses authentication (Error), reused | `server failed to start` with the error `failed to connect to Valkey: <error>`, then exit 1 |
|
||||
| 19 | `ErrorFunc` (Error) | `session store request failed` for a store failure; `session could not be decoded` otherwise; attributes `method`, `path`, `error` |
|
||||
| 19 | `ErrorFunc` (Error) | `session store request failed` for a store failure; `session could not be decoded or encoded` otherwise; attributes `method`, `path`, `error` |
|
||||
| 20 | `POST /logout` while the end-session endpoint is not published (Info) | `session ended; the identity provider's session was not ended because its end-session endpoint is not known` |
|
||||
| 21 | `start`, serving stopped (Error), reused | `server failed to start`, then exit 1 |
|
||||
| 22 | Stripe webhook before Temporal connects (Warn) | `stripe webhook event recorded before Temporal connected; answering 503 so Stripe redelivers it`, attributes `event_id`, `event_type` |
|
||||
|
||||
@@ -26,7 +26,7 @@ TLS handshake, and an error reply that says the store is busy, such as
|
||||
A request whose session the store cannot load or save SHALL be answered 503 with
|
||||
a `Retry-After` header (Requirement: A request whose dependency is not ready
|
||||
answers 503) and one structured log line, whatever the type of the error the
|
||||
store returned. A session the store returned but that cannot be decoded, an empty
|
||||
store returned, in the middleware or in a handler's own session call. A session the store returned but that cannot be decoded, an empty
|
||||
or a truncated value included, SHALL be answered with the styled 500 page and one
|
||||
structured log line. Routes that read no session (`/static/`, `/favicon.ico`,
|
||||
`/domains/ask`, and the paths integrations declare as authenticated by their own
|
||||
@@ -81,7 +81,7 @@ of who is on the other end of the connection.
|
||||
#### Scenario: A session that cannot be decoded is not an outage
|
||||
|
||||
- **WHEN** the session store returns an empty or a truncated value for a request's session cookie
|
||||
- **THEN** the response SHALL be the styled 500 page and one structured log line saying the session could not be decoded
|
||||
- **THEN** the response SHALL be the styled 500 page and one structured log line saying the session could not be decoded or encoded
|
||||
|
||||
#### Scenario: Both a password value and a password file is rejected
|
||||
|
||||
|
||||
@@ -83,11 +83,11 @@ Sections 1 to 7 are one commit each, and their items are that commit's parts; se
|
||||
|
||||
## 4. The session store: a refusal stops the start, an outage answers 503
|
||||
|
||||
- [ ] 4.1 `internal/auth/auth.go` and `valkey.go` (design D14): `Setup`'s probe takes its connection with `pool.GetContext`, which returns the dial's error apart from `PING`'s (`pool.Get` hides it in an `errorConn`, so a reply to `AUTH` and a reply to `PING` would come out of the same `Do`), and classifies the two separately. It returns its error only when the store's reply refuses authentication: a `redis.Error` beginning `WRONGPASS`, `NOAUTH` or `NOPERM`, or an `ERR` reply to `AUTH` during the dial, unless that reply begins `ERR max number of clients reached`, which a Valkey over `maxclients` writes to a new connection before it reads any command; that is a busy store, and it takes the next sentence's path. Every other probe error logs copy 17 and continues, an error reply such as `LOADING` (the store answers `PING` with it while it loads its dataset, which is the state of a restart of everything together), `BUSY` or `MASTERDOWN` included. The session pool's dial takes `valkey-addr` once, when `Setup` builds the pool, in place of reading it on every dial (design D13). `NewAwaitingDiscovery` wraps the session manager's store in `taggedStore{scs.Store}` (`Setup` builds its `Config` through it, and so does the server's test of item 4.4), implementing `Find`, `Commit` and `Delete` and the `Ctx` variants scs prefers, so that every error the store returns is wrapped in `storeError`; there is no list of error types, and the doc comment says why (the codec returns `io.EOF` and `io.ErrUnexpectedEOF`, and redigo returns TLS and protocol errors of types no list names). `FailurePage` gains `RenderErrorPage(w, r, status, msg)`, which `SafeTemplates` already has. `NewAwaitingDiscovery` also sets the session manager's `ErrorFunc`, which writes through the latch writer of item 4.2: for `errors.As(err, &storeError)` it deletes the headers a handler may have set, which are every header whose name begins `HX-` (`HX-Redirect`, `HX-Refresh` and `HX-Trigger` among them), `Location`, `Set-Cookie` and `Content-Type`, answers `Retry-After: 5` and 503 through `showFailure` with copy 3 (action: `validReturnTo(r.URL.RequestURI())` for GET and HEAD, `/` when it refuses and for any other method) and logs copy 19's first message; any other error does the same with the styled 500 page, the recovery page's existing message (`An unexpected error occurred. Try again.`) and copy 19's second message. Either answer marks the latch.
|
||||
- [ ] 4.2 `internal/auth` and `internal/server/server.go` (design D14): the latch writer lives in `internal/auth`, because the `ErrorFunc` there cannot import `internal/server`, which imports `internal/auth`. `(*Config).LoadAndSave(next)` wraps `w` in the latch writer and calls the session manager's `LoadAndSave`: once the `ErrorFunc` marks the latch, the writer drops every later `WriteHeader` and `Write`, so a handler's headers and body cannot follow the 503. `preAuthStack`'s session layer (`server.go:443`) becomes `skipSession(exempt, authConfig.LoadAndSave)`, which calls the next handler without loading the session for a request under `/static/`, for `/favicon.ico`, or whose path is in `exempt` (`/domains/ask` and the paths the mounts declare, the list `registerIntegrationRoutes` returns), and calls `LoadAndSave` for every other request. Its doc comment says which routes and why. `routes.golden` does not change, because `routeAuth` builds a hand-made session manager that keeps scs's default `ErrorFunc`, which never marks the latch.
|
||||
- [ ] 4.3 `cmd/start.go` (design D18): when `server.Start` returns, `Run` logs copy 21 and calls `os.Exit(1)`.
|
||||
- [ ] 4.4 Tests. `TestSetupRefusesAStoreThatRefuses` (`internal/auth`): against the stack's session store with a password the store does not accept, `Setup` returns an error that contains `failed to connect to Valkey`. `TestSetupServesWithoutAStore`, one row per store through a named function: `valkey-addr` naming a port that refuses (listened on, then closed), and a fake store that answers every command with `-LOADING Redis is loading the dataset in memory`; a third row, a fake store that answers the connection with `-ERR max number of clients reached`, gives the same; each gives a `Config` and copy 17 at Error with `addr`. `TestSessionStoreFailureAnswers503` and `TestUndecodableSessionAnswers500` build their `Config` through `NewAwaitingDiscovery`, because they need its `ErrorFunc` and its tagged store; the `/login` row publishes the holders by storing them directly, which the test can do from inside the package. `fakeFailurePage` (`auth_failure_page_test.go:16-31`), assigned to `Failures` at `:63`, `:141`, `:164` and `:182` and at `callback_edges_test.go:307`, gains `RenderErrorPage`, recording the status and the message, so that the package's tests compile, and the tests that use it keep their results. `TestSessionStoreFailureAnswers503` (`internal/auth`, over a handler wrapped by `(*Config).LoadAndSave`, where the latch lives), with an `scs.Store` fake whose `Find` and `Commit` return a `*net.OpError`, one row per request through a named function: a browser GET with a session cookie to a page (503, `Retry-After: 5`, the styled page with copy 3's heading and `Try again` to the requested path), an htmx request with the cookie (503, the plain text `Sign-in unavailable`), a POST with the cookie (the action leads to `/`), a GET to `//evil.example/x` with the cookie (the action leads to `/`), a store whose `Find` returns a `tls.RecordHeaderError` (503, which no type list names), and `/login` without a cookie once discovery is published (503 from the commit, no `Location` header, and a body equal to the styled page alone, with no `Found` link after it); a second request without a cookie to a route whose handler sets `HX-Redirect` before its `WriteHeader` (the `bounceToLogin` shape) answers 503 with no `HX-Redirect` header; a handler that puts a value in the session and sets `HX-Refresh: true` without writing (the shape of `workspace_partials.go:211-214`) answers 503 with no `HX-Refresh` header when the commit fails; each logs copy 19's first message with `method`, `path` and `error`. `TestUndecodableSessionAnswers500`, one row per store answer: bytes the codec cannot decode, an empty value (`io.EOF`) and a truncated one (`io.ErrUnexpectedEOF`) answer the styled 500 page and log copy 19's second message, with no `Retry-After`. `TestSessionFreeRoutesSkipTheStore` (`internal/server`, the route environment over the failing store, its `Config` built through `NewAwaitingDiscovery` so that it carries the tagged store and the `ErrorFunc`): `/static/app.css` answers 200 with a session cookie, `/favicon.ico` answers 404 and not 503 with a session cookie (no route serves that path, so the styled 404 answers without loading the session), `/domains/ask` without its `domain` parameter answers 400 with a cookie, as the route table's anonymous probe does, the fake mount's declared exempt path answers as without a store, and `/` with the cookie answers 503.
|
||||
- [ ] 4.5 Breaks:
|
||||
- [x] 4.1 `internal/auth/auth.go` and `valkey.go` (design D14): `Setup`'s probe takes its connection with `pool.GetContext`, which returns the dial's error apart from `PING`'s (`pool.Get` hides it in an `errorConn`, so a reply to `AUTH` and a reply to `PING` would come out of the same `Do`), and classifies the two separately. It returns its error only when the store's reply refuses authentication: a `redis.Error` beginning `WRONGPASS`, `NOAUTH` or `NOPERM`, or an `ERR` reply to `AUTH` during the dial, unless that reply begins `ERR max number of clients reached`, which a Valkey over `maxclients` writes to a new connection before it reads any command; that is a busy store, and it takes the next sentence's path. Every other probe error logs copy 17 and continues, an error reply such as `LOADING` (the store answers `PING` with it while it loads its dataset, which is the state of a restart of everything together), `BUSY` or `MASTERDOWN` included. The session pool's dial takes `valkey-addr` once, when `Setup` builds the pool, in place of reading it on every dial (design D13). `NewAwaitingDiscovery` wraps the session manager's store in `taggedStore{scs.Store}` (`Setup` builds its `Config` through it, and so does the server's test of item 4.4), implementing `Find`, `Commit` and `Delete` and the `Ctx` variants scs prefers, so that every error the store returns is wrapped in `storeError`; there is no list of error types, and the doc comment says why (the codec returns `io.EOF` and `io.ErrUnexpectedEOF`, and redigo returns TLS and protocol errors of types no list names). `FailurePage` gains `RenderErrorPage(w, r, status, msg)`, which `SafeTemplates` already has. `NewAwaitingDiscovery` also sets the session manager's `ErrorFunc`, which writes through the latch writer of item 4.2: for `errors.As(err, &storeError)` it deletes the headers a handler may have set, which are every header whose name begins `HX-` (`HX-Redirect`, `HX-Refresh` and `HX-Trigger` among them), `Location`, `Set-Cookie` and `Content-Type`, answers `Retry-After: 5` and 503 through `showFailure` with copy 3 (action: `/login` for `/callback`, whose authorization code is spent once; otherwise `validReturnTo(r.URL.RequestURI())` for GET and HEAD, `/` when it refuses and for any other method) and logs copy 19's first message; any other error does the same with the styled 500 page, the recovery page's existing message (`An unexpected error occurred. Try again.`) and copy 19's second message. Either answer marks the latch.
|
||||
- [x] 4.2 `internal/auth` and `internal/server/server.go` (design D14): the latch writer lives in `internal/auth`, because the `ErrorFunc` there cannot import `internal/server`, which imports `internal/auth`. `(*Config).LoadAndSave(next)` wraps `w` in the latch writer and calls the session manager's `LoadAndSave`: once the `ErrorFunc` marks the latch, the writer drops every later `WriteHeader` and `Write`, so a handler's headers and body cannot follow the 503. `preAuthStack`'s session layer (`server.go:443`) becomes `skipSession(exempt, authConfig.LoadAndSave)`, which calls the next handler without loading the session for a request under `/static/`, for `/favicon.ico`, or whose path is in `exempt` (`/domains/ask` and the paths the mounts declare, the list `registerIntegrationRoutes` returns), and calls `LoadAndSave` for every other request. Its doc comment says which routes and why. `routes.golden` does not change, because `routeAuth` builds a hand-made session manager that keeps scs's default `ErrorFunc`, which never marks the latch. `(*Config).answerStoreError(w, r, err)` (`session_store.go`) sends a tagged store error to the same `ErrorFunc` (the latch, the deleted headers, copy 3, `Retry-After: 5`, the copy 19 line) and reports true; `LoginHandler`, `RegistrationHandler` and `CallbackHandler` call it after `RenewToken` fails, and `LogoutHandler` and `logoutBeforeSignIn` after `Destroy` fails, and return when it reports true; for any other error the handler keeps its own 500.
|
||||
- [x] 4.3 `cmd/start.go` (design D18): when `server.Start` returns, `Run` logs copy 21 and calls `os.Exit(1)`.
|
||||
- [x] 4.4 Tests. `TestSetupRefusesAStoreThatRefuses` (`internal/auth`): against the stack's session store with a password the store does not accept, `Setup` returns an error that contains `failed to connect to Valkey`. `TestSetupServesWithoutAStore`, one row per store through a named function: `valkey-addr` naming a port that refuses (listened on, then closed), and a fake store that answers every command with `-LOADING Redis is loading the dataset in memory`; a third row, a fake store that answers the connection with `-ERR max number of clients reached`, gives the same; each gives a `Config` and copy 17 at Error with `addr`. `TestSessionStoreFailureAnswers503` and `TestUndecodableSessionAnswers500` build their `Config` through `NewAwaitingDiscovery`, because they need its `ErrorFunc` and its tagged store; the `/login` row publishes the holders by storing them directly, which the test can do from inside the package. `fakeFailurePage` (`auth_failure_page_test.go:16-31`), assigned to `Failures` at `:63`, `:141`, `:164` and `:182` and at `callback_edges_test.go:307`, gains `RenderErrorPage`, recording the status and the message, so that the package's tests compile, and the tests that use it keep their results. `TestSessionStoreFailureAnswers503` (`internal/auth`, over a handler wrapped by `(*Config).LoadAndSave`, where the latch lives), with an `scs.Store` fake whose `Find` and `Commit` return a `*net.OpError`, one row per request through a named function: a browser GET with a session cookie to a page (503, `Retry-After: 5`, the styled page with copy 3's heading and `Try again` to the requested path), an htmx request with the cookie (503, the plain text `Sign-in unavailable`), a POST with the cookie (the action leads to `/`), a GET to `//evil.example/x` with the cookie (the action leads to `/`), a store whose `Find` returns a `tls.RecordHeaderError` (503, which no type list names), and `/login` without a cookie once discovery is published (503 from the commit, no `Location` header, and a body equal to the styled page alone, with no `Found` link after it); a second request without a cookie to a route whose handler sets `HX-Redirect` before its `WriteHeader` (the `bounceToLogin` shape) answers 503 with no `HX-Redirect` header; a handler that puts a value in the session and sets `HX-Refresh: true` without writing (the shape of `workspace_partials.go:211-214`) answers 503 with no `HX-Refresh` header when the commit fails; each logs copy 19's first message with `method`, `path` and `error`. five more rows run a store whose `Find` answers with a session the codec decodes and whose `Delete` fails with a `*net.OpError`: `/login` and `/register` with the cookie (503, `Retry-After: 5`, the styled page alone, copy 19 once), `GET /callback` with the cookie, over a session that holds the state, nonce and code verifier the callback's checks and code exchange pass (the same answer, its action leading to the requested callback URL), `POST /logout` with the cookie once discovery is published, and `POST /logout` before it is (the `Destroy` in `logoutBeforeSignIn`). `TestUndecodableSessionAnswers500`, one row per store answer: bytes the codec cannot decode, an empty value (`io.EOF`) and a truncated one (`io.ErrUnexpectedEOF`) answer the styled 500 page and log copy 19's second message, `session could not be decoded or encoded`, with no `Retry-After`. `TestSessionFreeRoutesSkipTheStore` (`internal/server`, the route environment over the failing store, its `Config` built through `NewAwaitingDiscovery` so that it carries the tagged store and the `ErrorFunc`): `/static/app.css` answers 200 with a session cookie, `/favicon.ico` answers 404 and not 503 with a session cookie (no route serves that path, so the styled 404 answers without loading the session), `/domains/ask` without its `domain` parameter answers 400 with a cookie, as the route table's anonymous probe does, the fake mount's declared exempt path answers as without a store, and `/` with the cookie answers 503.
|
||||
- [x] 4.5 Breaks:
|
||||
- the probe fatal for every error, as today: caught by `TestSetupServesWithoutAStore`;
|
||||
- the probe fatal for every error reply, `LOADING` included: caught by `TestSetupServesWithoutAStore`'s `-LOADING` row;
|
||||
- the probe never fatal: caught by `TestSetupRefusesAStoreThatRefuses`;
|
||||
@@ -95,15 +95,21 @@ Sections 1 to 7 are one commit each, and their items are that commit's parts; se
|
||||
- `ErrorFunc` not set (scs's default): caught by `TestSessionStoreFailureAnswers503` (`want 503`);
|
||||
- the store not wrapped (`taggedStore` returns the store's errors as they are): caught by the same test (the `tls.RecordHeaderError` row answers 500);
|
||||
- every error classified as a store error: caught by `TestUndecodableSessionAnswers500` (the empty-value row answers 503);
|
||||
- the latch not marked: caught by `TestSessionStoreFailureAnswers503`'s `/login` row (`Location` present) and its `HX-Redirect` row;
|
||||
- the latch not marked: caught by `TestSessionStoreFailureAnswers503`'s `/login` row (the redirect's `Found` link follows the styled page in the body) and its `HX-Redirect` row (the handler's body follows the styled page);
|
||||
- the `ErrorFunc` deletes a fixed list of headers without `HX-Refresh`: caught by `TestSessionStoreFailureAnswers503`'s `HX-Refresh` row;
|
||||
- `LoginHandler` keeps its 500 on a store error from `RenewToken`: caught by the `/login` mid-request row (`status = 500, want 503`);
|
||||
- `RegistrationHandler` keeps its 500: caught by the `/register` mid-request row;
|
||||
- `CallbackHandler` keeps its 500 on a store error from `RenewToken`: caught by the `GET /callback` mid-request row (`status = 500, want 503`);
|
||||
- the 503 for `/callback` repeats the spent callback instead of `/login`: caught by the same row;
|
||||
- `LogoutHandler` keeps its 500 on a store error from `Destroy`: caught by the `POST /logout` mid-request row;
|
||||
- `logoutBeforeSignIn` keeps its 500: caught by the `POST /logout` before-discovery row;
|
||||
- the action always `/`: caught by the GET row;
|
||||
- the action built from `RequestURI` without `validReturnTo`: caught by the `//evil.example/x` row;
|
||||
- `skipSession` without `/static/`: caught by `TestSessionFreeRoutesSkipTheStore`'s stylesheet row;
|
||||
- `skipSession` without `/favicon.ico`: caught by its favicon row;
|
||||
- `skipSession` without the exempt list: caught by its ask row;
|
||||
- the `os.Exit(1)` after `server.Start`: `Run` is not run by a test; the boot test of section 6 proves that the serving half returns the error, and the line after it is read in review.
|
||||
- [ ] 4.6 After `test/reset-test-db.sh`, `go test -race -p 1 -count=1 ./internal/auth/ ./internal/server/` passes.
|
||||
- [x] 4.6 After `test/reset-test-db.sh`, `go test -race -p 1 -count=1 ./internal/auth/ ./internal/server/` passes.
|
||||
|
||||
## 5. Temporal-backed actions read the connection per request
|
||||
|
||||
|
||||
Reference in New Issue
Block a user