From 4bf44fc6eaa3a1ca53bc6861716aecd8ed93b940 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 22 Oct 2026 22:29:59 +0200 Subject: [PATCH] fix: logout revokes the session under serve: the jwt blacklist and GORM hooks are wired when the database is published Boot runs before summer serve publishes the database, so the plugin's GORM hooks, the Postgres jwt_blacklist store and its sweep now go through lagoon.OnDatabase, and the jwt guard resolves the published store on every request. A logged-out token is refused on jwt.auth, /fetch and /refresh. The jwt guard reads the cookie names from classes.SessionCookieNames(). --- README.md | 4 +- admin_harness_test.go | 55 ++++++++++++++++++++++++--- blacklist.go | 61 ++++++++++++++++++++++++++++++ blacklist_test.go | 86 +++++++++++++++++++++++++++++++++++++++++++ plugin.go | 29 +++++++-------- serve_order_test.go | 81 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 294 insertions(+), 22 deletions(-) create mode 100644 blacklist.go create mode 100644 blacklist_test.go create mode 100644 serve_order_test.go diff --git a/README.md b/README.md index b057e79..46e29fa 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ It owns the `users` table extensions, `user_throttle`, `jwt_blacklist`, `user_ap ## Features - **Registration and activation.** `POST /register`, `POST /activate`, `POST /activate-by-code`. Registration can be switched off and throttled; `activate_mode` is `auto`, `user` (a mailed code) or `admin`, with codes from `IssueActivationCode` and `VerifyActivationCode`. A `/register` body sets only the `User.RegisterFillable` columns: `name`, `surname`, `email`, `password` (stored as its hash), `preferred_locale` and `marketing_consent`. It cannot set `organisation_id` or `organisation_role` (membership is the administrator's or the application's), `is_onboarded`, `must_change_password`, or the activation, reset, address proof, handle and permission columns; other keys are ignored (they still reach `RegisterEvent.Payload`). `created_ip_address` and `last_ip_address` are server-set from the client address only. With `require_activation` on (the default), `POST /login` refuses an account that is not activated with the same 401 invalid-login answer as a wrong password and counts the attempt for the login throttle. Such an account is activated by the mailed code (`POST /activate-by-code` answers with a token), by the invitation link or by an administrator. `POST /activate` serves the sessions of not-activated accounts, which exist when `require_activation` is off. `Login` checks in this order: a ban or an active suspension (`RejectIfThrottled`), the password, the activation, then the deactivation restore. -- **JWT sessions.** `POST /login`, `GET /fetch`, `POST /refresh`, `POST /logout`. Boot registers the `jwt` guard with the framework's `bouncer.Registry` and exposes it as the `jwt.auth` middleware for other plugins. `POST /logout` ends the session however the token travels: the bearer header, or only the `token` or `auth_token` cookie the guard also reads. Every session token the request carries goes to the Postgres `jwt_blacklist` until its refresh window closes (the later of `exp` and `iat` plus `refresh_ttl`, plus a minute), so the guard refuses it on the next request and `POST /refresh` cannot revive it after the access token expired; a token whose access lifetime passed is revoked too while it could still be refreshed. The answer, a 401 included, expires the `auth.cookie.*` token cookie (the one `golem15.sociallogin`'s `token_cookie` sets), and any other session cookie the request carries, with that cookie's path, domain, SameSite and Secure. A background sweep prunes the blacklist. `POST /refresh` loads the token's user and refuses (401, `Could not refresh token`, `User not found`) a deactivated or banned user and a token issued before the user's `tokens_valid_after`; a suspension alone does not stop a refresh. `GET /fetch`, `POST /refresh`, `POST /update` and `POST /change-password`, like the plugin's other signed-in routes, read the session token as the `jwt` guard does: the bearer header first, then the `token` and `auth_token` cookies, so a cookie-only session works on them and a bearer header wins when both are sent. A cookie-carried `POST /refresh` answers the rotated token in the body and writes no cookie; the old token is blacklisted after `golem15.user.jwt.blacklist_grace`, so the caller that wrote the cookie stores the new token in it. A cookie-carried session relies on the cookie's SameSite attribute against cross-site requests (the `auth.cookie` default is `lax`), as on the `jwt.auth` routes. +- **JWT sessions.** `POST /login`, `GET /fetch`, `POST /refresh`, `POST /logout`. Boot registers the `jwt` guard with the framework's `bouncer.Registry` and exposes it as the `jwt.auth` middleware for other plugins. The guard reads the session cookies `classes.SessionCookieNames()` lists and resolves the published `bouncer.BlacklistStore` on every request. Boot installs the plugin's GORM hooks (`classes.RegisterHooks`), publishes the Postgres `jwt_blacklist` store (`bouncer.NewPostgresBlacklist`) and starts its background sweep, which prunes the blacklist, through `lagoon.OnDatabase`. Under `summer serve`, which publishes the database after Boot, they take effect when `lagoon.Publish` runs, before the router is built; when the database is already published they take effect at once. While no store is published, no token counts as blacklisted. `POST /logout` ends the session however the token travels: the bearer header, or only the `token` or `auth_token` cookie the guard also reads. Every session token the request carries goes to the Postgres `jwt_blacklist` until its refresh window closes (the later of `exp` and `iat` plus `refresh_ttl`, plus a minute), so the guard refuses it on the next request and `POST /refresh` cannot revive it after the access token expired; a token whose access lifetime passed is revoked too while it could still be refreshed. The answer, a 401 included, expires the `auth.cookie.*` token cookie (the one `golem15.sociallogin`'s `token_cookie` sets), and any other session cookie the request carries, with that cookie's path, domain, SameSite and Secure. `POST /refresh` loads the token's user and refuses (401, `Could not refresh token`, `User not found`) a deactivated or banned user and a token issued before the user's `tokens_valid_after`; a suspension alone does not stop a refresh. `GET /fetch`, `POST /refresh`, `POST /update` and `POST /change-password`, like the plugin's other signed-in routes, read the session token as the `jwt` guard does: the bearer header first, then the `token` and `auth_token` cookies, so a cookie-only session works on them and a bearer header wins when both are sent. A cookie-carried `POST /refresh` answers the rotated token in the body and writes no cookie; the old token is blacklisted after `golem15.user.jwt.blacklist_grace`, so the caller that wrote the cookie stores the new token in it. A cookie-carried session relies on the cookie's SameSite attribute against cross-site requests (the `auth.cookie` default is `lax`), as on the `jwt.auth` routes. - **Password reset and change.** `POST /forgot-password` answers the same body for known and unknown addresses; `POST /reset-password` consumes a code from `IssueResetCode`. `POST /change-password` requires the current password. - **Must-change-password lock.** The `user:require-password-change` console command flags an account. The flag rides on the authenticated principal (`bouncer.Principal.MustChangePassword`) for guards and other plugins to act on, and a successful `POST /change-password` clears it. - **Login throttling.** Failed logins are counted per user and IP in `user_throttle` (`CheckAndRecordLogin`, `RejectIfThrottled`) and suspend the account for a configured time. @@ -261,6 +261,8 @@ go test ./... # full run, starts Postgres with testcontainers (needs D `TestLogoutEndsTheSession` signs in through `POST /login` and logs out with the bearer header, with only the `token` or `auth_token` cookie, and with both a bearer and a different cookie token, then checks that the token is refused on a `jwt.auth` route, at `/fetch` and at `/refresh`, each through every carrier (bearer header, `token` cookie, `auth_token` cookie), that the forget cookies match the token cookie's attributes (also with a configured domain and SameSite), that the blacklist row lasts for the refresh window so `/refresh` stays refused after the sweep that follows the access token's expiry, that an expired but refreshable token is revoked, and which tokens a logout refuses with 401. +`TestBootBeforeDatabase` boots in the serve order (`party.Activate` with no database, then `lagoon.Publish`, through `newServeOrderEnv` in `admin_harness_test.go`). It checks that the blacklist store and the GORM hooks are installed on Publish and not before, and that a token ended by `POST /logout` is refused on a `jwt.auth` route through every carrier, at `/fetch` and at `/refresh`, with its `jwt_blacklist` row. `TestAppBlacklist` (in `-short`) covers the guard's per-request store lookup: no store, delegation and a failing store. + `TestSessionRoutesAcceptCookies` checks that a session `jwt.auth` accepts through only the `token` or `auth_token` cookie is served by `GET /fetch`, `POST /refresh` (the rotated token in the body, no cookie written, the old token refused afterwards), `POST /update`, `POST /change-password` (the calling token stays valid) and `POST /marketing-consent`; that a request without credentials still gets the old 401 bodies; that the bearer header wins over a cookie exactly as on `jwt.auth`, an invalid bearer included; and that a cut-off token and a banned user are refused through the cookie. `TestLoginRequiresActivation` pins the activation gate of `POST /login`: under the default and an explicit `require_activation: true` a not-activated account's correct password gets the byte-identical 401 (status, body, `Content-Type` and `Cache-Control`) of a wrong password, stamps no `last_seen`, adds one throttle attempt and suspends the account after `attempt_limit` refusals; `require_activation: false` and activated accounts sign in; a deactivated activated account is restored with the reactivate mail, and a deactivated not-activated account is refused and stays deactivated unless the key is off. diff --git a/admin_harness_test.go b/admin_harness_test.go index 82159cb..e7c7b35 100644 --- a/admin_harness_test.go +++ b/admin_harness_test.go @@ -126,11 +126,10 @@ func adminHarnessDB(t *testing.T) (*gorm.DB, *sql.DB) { return adminHarnessGDB, adminHarnessSQL } -// newAdminEnv boots the plugin alone with the admin API mounted. configure -// functions adjust the config before the plugin is activated. -func newAdminEnv(t *testing.T, configure ...func(*compass.Config)) *adminEnv { +// adminEnvConfig is the config every harness boot starts from: the plugin's +// JWT secret, the admin secret, body limits and the log mail driver. +func adminEnvConfig(t *testing.T) *compass.Config { t.Helper() - gdb, sqlDB := adminHarnessDB(t) dir := t.TempDir() if err := os.WriteFile(filepath.Join(dir, "app.yaml"), []byte("name: user-admin\nlocale: en\nfallback_locale: en\nurl: https://app.example.test\n"), 0o644); err != nil { t.Fatal(err) @@ -158,7 +157,15 @@ func newAdminEnv(t *testing.T, configure ...func(*compass.Config)) *adminEnv { t.Fatal(err) } } - app := backpack.New(cfg) + return cfg +} + +// newAdminEnv boots the plugin alone with the admin API mounted. configure +// functions adjust the config before the plugin is activated. +func newAdminEnv(t *testing.T, configure ...func(*compass.Config)) *adminEnv { + t.Helper() + gdb, sqlDB := adminHarnessDB(t) + app := backpack.New(adminEnvConfig(t)) if err := lagoon.Publish(app, sqlDB, gdb); err != nil { t.Fatal(err) } @@ -191,6 +198,44 @@ func newAdminEnv(t *testing.T, configure ...func(*compass.Config)) *adminEnv { return &adminEnv{h: h, app: app, gdb: gdb, bucket: bucket, mail: mail} } +// newServeOrderEnv boots the plugin the way surf's serve command does after +// the generated main's party.Activate: every Boot runs with no database +// published, then lagoon.Publish, the uploads bucket and the router. +// beforePublish, when set, runs between Activate and Publish. Migrate is +// there only because the shared harness database may not be migrated yet. +func newServeOrderEnv(t *testing.T, beforePublish func(*backpack.App)) *adminEnv { + t.Helper() + gdb, sqlDB := adminHarnessDB(t) + app := backpack.New(adminEnvConfig(t)) + mail := &mailCapture{next: slog.Default().Handler()} + if err := app.Publish(slog.New(mail)); err != nil { + t.Fatal(err) + } + plugins, err := party.Activate(app, []string{"golem15.user"}) + if err != nil { + t.Fatal(err) + } + if beforePublish != nil { + beforePublish(app) + } + if err := lagoon.Publish(app, sqlDB, gdb); err != nil { + t.Fatal(err) + } + bucket := memblob.OpenBucket(nil) + t.Cleanup(func() { _ = bucket.Close() }) + if err := attach.Publish(app, bucket); err != nil { + t.Fatal(err) + } + if err := lagoon.Migrate(gdb, plugins); err != nil { + t.Fatal(err) + } + h, err := surf.Assemble(app, plugins) + if err != nil { + t.Fatal(err) + } + return &adminEnv{h: h, app: app, gdb: gdb, bucket: bucket, mail: mail} +} + // admin inserts a backend role holding exactly permissions and a backend // user in it, logs the user in and returns the bearer token. No permission // at all gives an administrator who may sign in and open nothing. diff --git a/blacklist.go b/blacklist.go new file mode 100644 index 0000000..24aeb3b --- /dev/null +++ b/blacklist.go @@ -0,0 +1,61 @@ +package user + +import ( + "context" + "time" + + "git.golem15.com/golem15/summercms/modules/backpack" + "git.golem15.com/golem15/summercms/modules/bouncer" +) + +var _ bouncer.BlacklistStore = (*appBlacklist)(nil) + +// appBlacklist is the blacklist store the jwt guard is built with. The guard +// is built in Boot, which under summer serve runs before the database is +// published, so the store the plugin publishes on lagoon.Publish is resolved +// on every call. While none is published, no token counts as blacklisted and +// Add and Sweep do nothing. An appBlacklist must never be published as the +// app's bouncer.BlacklistStore: the lookup would then find itself. +type appBlacklist struct { + app *backpack.App +} + +// store returns the published blacklist store, or nil when there is none +// (or the published value is an appBlacklist). +func (b *appBlacklist) store() bouncer.BlacklistStore { + if b == nil || b.app == nil { + return nil + } + s, ok := b.app.Lookup[bouncer.BlacklistStore]() + if !ok || s == nil { + return nil + } + if _, self := s.(*appBlacklist); self { + return nil + } + return s +} + +func (b *appBlacklist) Add(ctx context.Context, jti string, expiresAt, validUntil time.Time) error { + s := b.store() + if s == nil { + return nil + } + return s.Add(ctx, jti, expiresAt, validUntil) +} + +func (b *appBlacklist) IsBlacklisted(ctx context.Context, jti string) (bool, error) { + s := b.store() + if s == nil { + return false, nil + } + return s.IsBlacklisted(ctx, jti) +} + +func (b *appBlacklist) Sweep(ctx context.Context, now time.Time) error { + s := b.store() + if s == nil { + return nil + } + return s.Sweep(ctx, now) +} diff --git a/blacklist_test.go b/blacklist_test.go new file mode 100644 index 0000000..c09405b --- /dev/null +++ b/blacklist_test.go @@ -0,0 +1,86 @@ +package user + +import ( + "errors" + "testing" + "time" + + "git.golem15.com/golem15/summercms/modules/bouncer" +) + +// TestAppBlacklist: the jwt guard's store resolves the app's published +// bouncer.BlacklistStore on every call. With none published (or only an +// appBlacklist, which would recurse) no token is blacklisted and Add and +// Sweep do nothing; otherwise calls are delegated and a store error comes +// back unchanged, so the guard still fails closed. +func TestAppBlacklist(t *testing.T) { + ctx := t.Context() + now := time.Now() + newApp := func() *appBlacklist { + return &appBlacklist{app: sessionUserApp(t, "SUMMER_GOLEM15__USER__JWT__SECRET="+sessionSecret)} + } + assertNoStore := func(t *testing.T, bl *appBlacklist) { + t.Helper() + if got, err := bl.IsBlacklisted(ctx, "jti-none"); got || err != nil { + t.Fatalf("IsBlacklisted with no store = %v, %v", got, err) + } + if err := bl.Add(ctx, "jti-none", now.Add(time.Hour), now); err != nil { + t.Fatalf("Add with no store = %v", err) + } + if err := bl.Sweep(ctx, now); err != nil { + t.Fatalf("Sweep with no store = %v", err) + } + } + + t.Run("no store", func(t *testing.T) { + assertNoStore(t, newApp()) + }) + + t.Run("wrapper published as the store", func(t *testing.T) { + bl := newApp() + if err := bl.app.Publish[bouncer.BlacklistStore](&appBlacklist{app: bl.app}); err != nil { + t.Fatal(err) + } + assertNoStore(t, bl) + }) + + t.Run("delegates", func(t *testing.T) { + bl := newApp() + mem := bouncer.NewMemoryBlacklist() + if err := bl.app.Publish[bouncer.BlacklistStore](mem); err != nil { + t.Fatal(err) + } + // Blacklisted from now on, kept for an hour. + if err := bl.Add(ctx, "jti-ended", now.Add(time.Hour), now); err != nil { + t.Fatal(err) + } + if got, err := mem.IsBlacklisted(ctx, "jti-ended"); !got || err != nil { + t.Fatalf("memory store after Add through the wrapper = %v, %v", got, err) + } + if got, err := bl.IsBlacklisted(ctx, "jti-ended"); !got || err != nil { + t.Fatalf("IsBlacklisted(jti-ended) = %v, %v", got, err) + } + if got, err := bl.IsBlacklisted(ctx, "jti-other"); got || err != nil { + t.Fatalf("IsBlacklisted(jti-other) = %v, %v", got, err) + } + if err := bl.Sweep(ctx, now); err != nil { + t.Fatal(err) + } + }) + + t.Run("store error passes through", func(t *testing.T) { + bl := newApp() + if err := bl.app.Publish[bouncer.BlacklistStore](failingBlacklist{}); err != nil { + t.Fatal(err) + } + if _, err := bl.IsBlacklisted(ctx, "jti"); !errors.Is(err, errBlacklistDown) { + t.Fatalf("IsBlacklisted error = %v", err) + } + if err := bl.Add(ctx, "jti", now.Add(time.Hour), now); !errors.Is(err, errBlacklistDown) { + t.Fatalf("Add error = %v", err) + } + if err := bl.Sweep(ctx, now); !errors.Is(err, errBlacklistDown) { + t.Fatalf("Sweep error = %v", err) + } + }) +} diff --git a/plugin.go b/plugin.go index ec20953..cbf9cc8 100644 --- a/plugin.go +++ b/plugin.go @@ -70,10 +70,21 @@ func (p *Plugin) Boot(app *backpack.App) error { if err != nil { return err } - if gdb, ok := app.Lookup[*gorm.DB](); ok { + // Serve publishes the database after Boot, so the GORM hooks, the + // blacklist store and its sweep are installed on lagoon.Publish, or at + // once when the database is already published. + if err := lagoon.OnDatabase(app, func(sqlDB *sql.DB, gdb *gorm.DB) error { if err := classes.RegisterHooks(gdb); err != nil { return err } + store := bouncer.NewPostgresBlacklist(sqlDB, "jwt_blacklist") + if err := app.Publish[bouncer.BlacklistStore](store); err != nil { + return err + } + p.startBlacklistSweep(store) + return nil + }); err != nil { + return err } reg, ok := app.Lookup[*bouncer.Registry]() if !ok || reg == nil { @@ -82,21 +93,7 @@ func (p *Plugin) Boot(app *backpack.App) error { return err } } - var bl bouncer.BlacklistStore - if sqlDB, ok := app.Lookup[*sql.DB](); ok && sqlDB != nil { - bl = bouncer.NewPostgresBlacklist(sqlDB, "jwt_blacklist") - } else if gdb, ok := app.Lookup[*gorm.DB](); ok && gdb != nil { - if sqlDB, err := gdb.DB(); err == nil && sqlDB != nil { - bl = bouncer.NewPostgresBlacklist(sqlDB, "jwt_blacklist") - } - } - if bl != nil { - if err := app.Publish[bouncer.BlacklistStore](bl); err != nil { - return err - } - p.startBlacklistSweep(bl) - } - if err := reg.Register(p.ID(), "jwt", bouncer.NewJWTGuard(secret, classes.GormUsers{App: app}, bl, "token", "auth_token")); err != nil { + if err := reg.Register(p.ID(), "jwt", bouncer.NewJWTGuard(secret, classes.GormUsers{App: app}, &appBlacklist{app: app}, classes.SessionCookieNames()...)); err != nil { return err } prefix, err := classes.APITokenPrefix(app) diff --git a/serve_order_test.go b/serve_order_test.go new file mode 100644 index 0000000..1cea099 --- /dev/null +++ b/serve_order_test.go @@ -0,0 +1,81 @@ +package user + +import ( + "net/http" + "sync/atomic" + "testing" + + "git.golem15.com/golem15/sm-user-plugin/classes" + "git.golem15.com/golem15/summercms/modules/backpack" + "git.golem15.com/golem15/summercms/modules/bouncer" + "gorm.io/gorm" +) + +// TestBootBeforeDatabase pins logout revocation when Boot runs before the +// database is published, as under summer serve: nothing database-backed is +// installed before lagoon.Publish, the Postgres blacklist store and the GORM +// hooks are installed when it runs, and a token ended by POST /logout is +// refused on jwt.auth through every carrier, at /fetch and at /refresh, with +// its jwt_blacklist row. +func TestBootBeforeDatabase(t *testing.T) { + // The classes hook registrants are process-global and cannot be removed, + // so the test hook is gated and goes inert once this test ends. + var hooked atomic.Pointer[gorm.DB] + var active atomic.Bool + active.Store(true) + t.Cleanup(func() { active.Store(false) }) + classes.RegisterHook(func(gdb *gorm.DB) error { + if active.Load() { + hooked.Store(gdb) + } + return nil + }) + + e := newServeOrderEnv(t, func(app *backpack.App) { + if _, ok := app.Lookup[*gorm.DB](); ok { + t.Fatal("a *gorm.DB is published before lagoon.Publish") + } + if _, ok := app.Lookup[bouncer.BlacklistStore](); ok { + t.Fatal("a bouncer.BlacklistStore is published before lagoon.Publish") + } + if hooked.Load() != nil { + t.Fatal("the GORM hooks ran before lagoon.Publish") + } + }) + + store, ok := e.app.Lookup[bouncer.BlacklistStore]() + if !ok { + t.Fatal("no bouncer.BlacklistStore published after lagoon.Publish") + } + if _, ok := store.(*bouncer.PostgresBlacklist); !ok { + t.Fatalf("published blacklist store is %T, want *bouncer.PostgresBlacklist", store) + } + if got := hooked.Load(); got != e.gdb { + t.Fatalf("the GORM hooks ran with %p, want the published handle %p", got, e.gdb) + } + + token := e.sessionLogin(t, "serve-order") + if code := e.guarded(t, token, ""); code != http.StatusOK { + t.Fatalf("jwt.auth before logout (bearer) = %d", code) + } + for _, name := range classes.SessionCookieNames() { + if code := e.guarded(t, token, name); code != http.StatusOK { + t.Fatalf("jwt.auth before logout (cookie %q) = %d", name, code) + } + } + + assertLoggedOut(t, e.call(t, http.MethodPost, userLogoutPath, token, nil)) + e.assertSessionEnded(t, token) + + jti, _ := parseToken(t, token)["jti"].(string) + if jti == "" { + t.Fatal("session token has no jti") + } + var rows int64 + if err := e.gdb.Raw(`SELECT COUNT(*) FROM jwt_blacklist WHERE jti = ?`, jti).Scan(&rows).Error; err != nil { + t.Fatal(err) + } + if rows != 1 { + t.Fatalf("jwt_blacklist rows for %s = %d, want 1", jti, rows) + } +}