fix(cabana): enforce tokens_valid_after and is_activated on admin refresh

Fixes review finding CR-01 (quick 260927-q23): POST {prefix}/api/v1/auth/refresh
minted a new token without loading the admin, so a session kept alive by the
SPA's refresh-on-401 survived admin:reset-password, deactivation and deletion.
This broke Phase 9 truth T-09-04.

- bouncer: extract the JWT guard's subject lookup into subjectPrincipal and
  issuedBeforeCutoff (same order and messages), add ErrSubjectRejected
- bouncer: add RefreshAudienceFor, which runs the guard's subject checks
  after the token-only checks and before minting; Refresh and
  RefreshAudience are unchanged (nil hook)
- cabana: share one lazyBackendUsers provider between the backend guard and
  refresh; a cookie refresh refused for its subject expires summer_admin
- test: TestAdminRefreshRevocation (Postgres, real admin:reset-password)

bouncer/jwt.go

@@ -118,19 +118,9 @@ func (g *jwtGuard) Authenticate(r *http.Request) (*Principal, error) {
 	if err != nil {
 		return nil, err
 	}
-	id, err := strconv.ParseUint(sub, 10, 64)
-	if err != nil || id == 0 {
-		return nil, errors.New(msgUserNotFound)
-	}
-	if g.users == nil {
-		return nil, errors.New(msgUserNotFound)
-	}
-	user, err := g.users.FindByID(r.Context(), uint(id))
+	user, err := subjectPrincipal(r.Context(), g.users, sub)
 	if err != nil {
-		return nil, errors.New("Authentication error")
-	}
-	if user == nil {
-		return nil, errors.New(msgUserNotFound)
+		return nil, err
 	}
 	if g.bl != nil {
 		blocked, err := g.bl.IsBlacklisted(r.Context(), jti)
@@ -141,12 +131,48 @@ func (g *jwtGuard) Authenticate(r *http.Request) (*Principal, error) {
 			return nil, errors.New(msgBadSignature)
 		}
 	}
-	if !user.TokensValidAfter.IsZero() && iat.Before(user.TokensValidAfter) {
-		return nil, errors.New(msgUserNotFound)
+	if issuedBeforeCutoff(user, iat) {
+		return nil, ErrSubjectRejected
 	}
 	return user, nil
 }
 
+// ErrSubjectRejected reports that a token's subject is not a loadable user
+// (a non-numeric or zero sub, a nil provider, or a provider that returned no
+// principal for a missing, deleted or not-activated user), or that the token
+// was issued before Principal.TokensValidAfter. Its message is "User not found".
+var ErrSubjectRejected = errors.New(msgUserNotFound)
+
+// errAuthentication reports a provider failure while loading the subject.
+var errAuthentication = errors.New("Authentication error")
+
+// subjectPrincipal loads the token subject through users, shared by the JWT
+// guard and RefreshAudienceFor. A provider error is errAuthentication; every
+// other refusal is ErrSubjectRejected.
+func subjectPrincipal(ctx context.Context, users UserProvider, sub string) (*Principal, error) {
+	id, err := strconv.ParseUint(sub, 10, 64)
+	if err != nil || id == 0 {
+		return nil, ErrSubjectRejected
+	}
+	if users == nil {
+		return nil, ErrSubjectRejected
+	}
+	user, err := users.FindByID(ctx, uint(id))
+	if err != nil {
+		return nil, errAuthentication
+	}
+	if user == nil {
+		return nil, ErrSubjectRejected
+	}
+	return user, nil
+}
+
+// issuedBeforeCutoff reports whether iat predates the user's
+// tokens_valid_after cutoff (set by a password reset). A zero cutoff never cuts.
+func issuedBeforeCutoff(user *Principal, iat time.Time) bool {
+	return !user.TokensValidAfter.IsZero() && iat.Before(user.TokensValidAfter)
+}
+
 func (g *jwtGuard) WriteUnauthorized(w http.ResponseWriter, err error) {
 	if g.writeFn != nil {
 		g.writeFn(w, err)

bouncer/refresh.go

@@ -15,7 +15,7 @@ import (
 // jwt-auth: the later of the old exp and iat+refreshTTL, plus one minute, so
 // a logged-out token cannot be refreshed again for the rest of its refresh window.
 func Refresh(secret, tokenString string, refreshTTL time.Duration, bl BlacklistStore, grace time.Duration, issuerURL string) (string, error) {
-	return refreshAudience(secret, tokenString, AudienceUser, true, refreshTTL, bl, grace, issuerURL)
+	return refreshAudience(secret, tokenString, AudienceUser, true, refreshTTL, bl, grace, issuerURL, nil)
 }
 
 // RefreshAudience reissues a token that already carries audience. Missing aud is rejected.
@@ -23,10 +23,37 @@ func RefreshAudience(secret, tokenString, audience string, refreshTTL time.Durat
 	if strings.TrimSpace(audience) == "" {
 		return "", errors.New("bouncer: jwt audience is empty")
 	}
-	return refreshAudience(secret, tokenString, audience, false, refreshTTL, bl, grace, issuerURL)
+	return refreshAudience(secret, tokenString, audience, false, refreshTTL, bl, grace, issuerURL, nil)
 }
 
-func refreshAudience(secret, tokenString, audience string, allowMissing bool, refreshTTL time.Duration, bl BlacklistStore, grace time.Duration, issuerURL string) (string, error) {
+// RefreshAudienceFor is RefreshAudience plus the JWT guard's subject checks
+// before minting: the subject is loaded through users, and a missing,
+// deleted or not-activated user, or a token issued before the user's
+// TokensValidAfter cutoff, is refused. The lookup runs only after every
+// token-only check (signature, audience, refresh window, blacklist, exp)
+// passed, and a refused subject neither mints a token nor blacklists the old
+// jti. Only subject refusals match errors.Is(err, ErrSubjectRejected); a
+// provider failure returns a different error.
+func RefreshAudienceFor(ctx context.Context, users UserProvider, secret, tokenString, audience string, refreshTTL time.Duration, bl BlacklistStore, grace time.Duration, issuerURL string) (string, error) {
+	if strings.TrimSpace(audience) == "" {
+		return "", errors.New("bouncer: jwt audience is empty")
+	}
+	check := func(sub string, iat time.Time) error {
+		user, err := subjectPrincipal(ctx, users, sub)
+		if err != nil {
+			return err
+		}
+		if issuedBeforeCutoff(user, iat) {
+			return ErrSubjectRejected
+		}
+		return nil
+	}
+	return refreshAudience(secret, tokenString, audience, false, refreshTTL, bl, grace, issuerURL, check)
+}
+
+// refreshAudience holds the shared refresh flow. check, when non-nil, runs
+// after every token-only check and immediately before minting.
+func refreshAudience(secret, tokenString, audience string, allowMissing bool, refreshTTL time.Duration, bl BlacklistStore, grace time.Duration, issuerURL string, check func(sub string, iat time.Time) error) (string, error) {
 	if strings.TrimSpace(secret) == "" {
 		return "", errors.New("bouncer: jwt secret is empty")
 	}
@@ -68,6 +95,11 @@ func refreshAudience(secret, tokenString, audience string, allowMissing bool, re
 	if !expOK || ttl <= 0 {
 		return "", errors.New(msgRequiredClaims)
 	}
+	if check != nil {
+		if err := check(sub, iat); err != nil {
+			return "", err
+		}
+	}
 	next, _, err := MintAudience(secret, sub, issuerURL, ttl, audience)
 	if err != nil {
 		return "", err

cabana/auth.go

@@ -221,8 +221,14 @@ func (s *service) refresh(w http.ResponseWriter, r *http.Request) {
 		WriteError(w, http.StatusUnauthorized, "unauthenticated", msgUnauthenticated)
 		return
 	}
-	next, err := bouncer.RefreshAudience(s.secret, raw, bouncer.AudienceBackend, s.refreshTTL, s.bl, s.grace, s.issuer)
+	next, err := bouncer.RefreshAudienceFor(r.Context(), s.users, s.secret, raw, bouncer.AudienceBackend, s.refreshTTL, s.bl, s.grace, s.issuer)
 	if err != nil {
+		// A subject the guard would refuse (deactivated, deleted, or cut off
+		// by tokens_valid_after) ends the browser session. Other failures,
+		// including a provider error, leave the cookie alone.
+		if fromCookie && errors.Is(err, bouncer.ErrSubjectRejected) {
+			s.expireSessionCookie(w)
+		}
 		s.logAuth(r, "failed", 0)
 		WriteError(w, http.StatusUnauthorized, "unauthenticated", msgUnauthenticated)
 		return

cabana/http.go

@@ -41,6 +41,7 @@ type service struct {
 	loginDecay int
 	issuer     string
 	bl         bouncer.BlacklistStore
+	users      bouncer.UserProvider // the backend guard's provider, reused by refresh
 	prefix     string
 	spa        http.Handler
 	// insecureCookie drops Secure from the admin cookie (backend.cookie_secure
@@ -109,7 +110,8 @@ func Activate(app *backpack.App, plugins []party.Plugin) (*Routes, error) {
 		}
 	}
 	bl := adminBlacklist(app)
-	guard := bouncer.NewBackendJWTGuard(secret, lazyBackendUsers{app: app, reg: reg}, bl, writeUnauthenticated, AdminCookieName)
+	users := lazyBackendUsers{app: app, reg: reg}
+	guard := bouncer.NewBackendJWTGuard(secret, users, bl, writeUnauthenticated, AdminCookieName)
 	if _, err := guards.Middleware("backend"); err != nil {
 		if err := guards.Register("summercms.cabana", "backend", guard); err != nil {
 			return nil, err
@@ -132,6 +134,7 @@ func Activate(app *backpack.App, plugins []party.Plugin) (*Routes, error) {
 		loginDecay: loginDecay,
 		issuer:     adminIssuer(app, prefix),
 		bl:         bl,
+		users:      users,
 		prefix:     prefix,
 
 		insecureCookie: !secureCookie,

cabana/phase10_coverage_test.go

@@ -2,6 +2,7 @@ package cabana
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -22,6 +23,20 @@ import (
 	"gorm.io/gorm"
 )
 
+// refreshSubjects is a map-backed bouncer.UserProvider for refresh tests
+// that run without a database; err, when set, is returned for every lookup.
+type refreshSubjects struct {
+	byID map[uint]*bouncer.Principal
+	err  error
+}
+
+func (p refreshSubjects) FindByID(_ context.Context, id uint) (*bouncer.Principal, error) {
+	if p.err != nil {
+		return nil, p.err
+	}
+	return p.byID[id], nil
+}
+
 // emptyOptionsRow is a filter source whose scope has no choices yet;
 // literalOptionsRow's choice label is a literal rather than a phrase key.
 type emptyOptionsRow struct {
@@ -281,6 +296,7 @@ func TestPhase10Coverage(t *testing.T) {
 			refreshTTL: 2 * time.Hour,
 			issuer:     "https://app.test" + DefaultAdminPrefix,
 			bl:         bouncer.NewMemoryBlacklist(),
+			users:      refreshSubjects{byID: map[uint]*bouncer.Principal{5: {ID: 5, Backend: true}}},
 		}
 		sign := func(iat, exp time.Time, jti string) string {
 			t.Helper()

cabana/refresh_revocation_test.go

@@ -0,0 +1,144 @@
+package cabana_test
+
+import (
+	"bytes"
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"git.golem15.com/golem15/summercms/bonfire"
+	"git.golem15.com/golem15/summercms/cabana"
+)
+
+// TestAdminRefreshRevocation pins CR-01: POST {prefix}/api/v1/auth/refresh
+// applies the backend guard's subject checks before minting. A token issued
+// before `summer admin:reset-password` (tokens_valid_after), or held by a
+// deactivated or soft-deleted admin, cannot be refreshed over either
+// transport, and a refused cookie refresh expires summer_admin.
+func TestAdminRefreshRevocation(t *testing.T) {
+	gdb := adminGorm(t)
+
+	cookieLogin := func(t *testing.T, h http.Handler, login string) *http.Cookie {
+		t.Helper()
+		rec := phase10Send(t, h, http.MethodPost, adminAPI("/auth/login"),
+			map[string]string{"login": login, "password": adminTestPassword}, nil, true)
+		if rec.Code != http.StatusOK {
+			t.Fatalf("cookie login status=%d body=%s", rec.Code, rec.Body.String())
+		}
+		return phase10Cookie(t, rec, cabana.DefaultAdminPrefix)
+	}
+	bearerLogin := func(t *testing.T, h http.Handler, login string) string {
+		t.Helper()
+		rec := phase10Send(t, h, http.MethodPost, adminAPI("/auth/login"),
+			map[string]string{"login": login, "password": adminTestPassword}, nil, false)
+		if rec.Code != http.StatusOK {
+			t.Fatalf("bearer login status=%d body=%s", rec.Code, rec.Body.String())
+		}
+		return accessToken(t, rec.Body.Bytes())
+	}
+	resetPassword := func(t *testing.T, login string) {
+		t.Helper()
+		reset := commandByName(t, cabana.RuntimeCommands(commandApp(t, gdb)), "admin:reset-password")
+		var buf bytes.Buffer
+		if err := reset.Run(context.Background(), flagInput{
+			args:  []string{login},
+			flags: map[string]string{"password": "rrev-replacement-password"},
+		}, bonfire.NewOutput(nil, &buf, &buf)); err != nil {
+			t.Fatalf("admin:reset-password: %v output=%s", err, buf.String())
+		}
+	}
+	assertRefusedWithExpiredCookie := func(t *testing.T, rec *httptest.ResponseRecorder) {
+		t.Helper()
+		if rec.Code != http.StatusUnauthorized || phase10ErrorCode(t, rec) != "unauthenticated" {
+			t.Fatalf("refresh status=%d body=%s, want 401 unauthenticated", rec.Code, rec.Body.String())
+		}
+		var expired *http.Cookie
+		for _, c := range rec.Result().Cookies() {
+			if c.Name == cabana.AdminCookieName {
+				expired = c
+			}
+		}
+		if expired == nil || expired.Value != "" || expired.MaxAge >= 0 || expired.Path != cabana.DefaultAdminPrefix {
+			t.Fatalf("refused refresh cookie = %+v, want an expiring %s with Path %s", expired, cabana.AdminCookieName, cabana.DefaultAdminPrefix)
+		}
+	}
+
+	t.Run("pre-reset cookie is refused and expired", func(t *testing.T) {
+		h := adminHandler(t, gdb, nil)
+		insertAdmin(t, gdb, "rrev-cookie", "rrev-cookie@example.test", adminTestPassword, true, false)
+		old := cookieLogin(t, h, "rrev-cookie")
+		resetPassword(t, "rrev-cookie")
+
+		if me := phase10Send(t, h, http.MethodGet, adminAPI("/auth/me"), nil, old, true); me.Code != http.StatusUnauthorized {
+			t.Fatalf("pre-reset cookie /auth/me status=%d body=%s", me.Code, me.Body.String())
+		}
+		rec := phase10Send(t, h, http.MethodPost, adminAPI("/auth/refresh"), nil, old, true)
+		assertRefusedWithExpiredCookie(t, rec)
+	})
+
+	t.Run("pre-reset bearer is refused without cookies", func(t *testing.T) {
+		h := adminHandler(t, gdb, nil)
+		insertAdmin(t, gdb, "rrev-bearer", "rrev-bearer@example.test", adminTestPassword, true, false)
+		token := bearerLogin(t, h, "rrev-bearer")
+		resetPassword(t, "rrev-bearer")
+
+		rec := postAuth(t, h, http.MethodPost, adminAPI("/auth/refresh"), token, nil)
+		if rec.Code != http.StatusUnauthorized || phase10ErrorCode(t, rec) != "unauthenticated" {
+			t.Fatalf("pre-reset bearer refresh status=%d body=%s, want 401 unauthenticated", rec.Code, rec.Body.String())
+		}
+		if got := rec.Header().Values("Set-Cookie"); len(got) != 0 {
+			t.Fatalf("bearer refresh set cookies: %q", got)
+		}
+	})
+
+	t.Run("deactivated admin is refused", func(t *testing.T) {
+		h := adminHandler(t, gdb, nil)
+		user := insertAdmin(t, gdb, "rrev-deactivated", "rrev-deactivated@example.test", adminTestPassword, true, false)
+		old := cookieLogin(t, h, "rrev-deactivated")
+		if err := gdb.Exec(`UPDATE backend_users SET is_activated = false WHERE id = ?`, user.ID).Error; err != nil {
+			t.Fatal(err)
+		}
+		rec := phase10Send(t, h, http.MethodPost, adminAPI("/auth/refresh"), nil, old, true)
+		assertRefusedWithExpiredCookie(t, rec)
+	})
+
+	t.Run("soft-deleted admin is refused", func(t *testing.T) {
+		h := adminHandler(t, gdb, nil)
+		user := insertAdmin(t, gdb, "rrev-deleted", "rrev-deleted@example.test", adminTestPassword, true, false)
+		old := cookieLogin(t, h, "rrev-deleted")
+		if err := gdb.Delete(&user).Error; err != nil {
+			t.Fatal(err)
+		}
+		rec := phase10Send(t, h, http.MethodPost, adminAPI("/auth/refresh"), nil, old, true)
+		assertRefusedWithExpiredCookie(t, rec)
+	})
+
+	t.Run("active admin still refreshes", func(t *testing.T) {
+		h := adminHandler(t, gdb, nil)
+		insertAdmin(t, gdb, "rrev-active", "rrev-active@example.test", adminTestPassword, true, false)
+		first := cookieLogin(t, h, "rrev-active")
+		rec := phase10Send(t, h, http.MethodPost, adminAPI("/auth/refresh"), nil, first, true)
+		if rec.Code != http.StatusOK {
+			t.Fatalf("cookie refresh status=%d body=%s", rec.Code, rec.Body.String())
+		}
+		second := phase10Cookie(t, rec, cabana.DefaultAdminPrefix)
+		if second.Value == first.Value {
+			t.Fatal("cookie refresh did not rotate the token")
+		}
+		phase10AssertCookieBody(t, rec, second.Value)
+		if me := phase10Send(t, h, http.MethodGet, adminAPI("/auth/me"), nil, second, true); me.Code != http.StatusOK {
+			t.Fatalf("rotated cookie /auth/me status=%d body=%s", me.Code, me.Body.String())
+		}
+
+		token := bearerLogin(t, h, "rrev-active")
+		bearer := postAuth(t, h, http.MethodPost, adminAPI("/auth/refresh"), token, nil)
+		if bearer.Code != http.StatusOK {
+			t.Fatalf("bearer refresh status=%d body=%s", bearer.Code, bearer.Body.String())
+		}
+		phase10AssertBearerBody(t, bearer)
+		if got := bearer.Header().Values("Set-Cookie"); len(got) != 0 {
+			t.Fatalf("bearer refresh set cookies: %q", got)
+		}
+	})
+}