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)
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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()
|
||||
|
||||
144
cabana/refresh_revocation_test.go
Normal file
144
cabana/refresh_revocation_test.go
Normal file
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
Reference in New Issue
Block a user