From 3f476164f97f63f3928b7403e03d39eb967d7f75 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 1 Oct 2026 21:15:05 +0200 Subject: [PATCH] fix(09): WR-10 fail boot when another plugin already owns the backend guard --- docs/backend/users-and-permissions.md | 2 +- modules/cabana/README.md | 2 +- .../cabana/backend_guard_collision_test.go | 42 +++++++++++++++++++ modules/cabana/http.go | 10 +++-- 4 files changed, 50 insertions(+), 6 deletions(-) create mode 100644 modules/cabana/backend_guard_collision_test.go diff --git a/docs/backend/users-and-permissions.md b/docs/backend/users-and-permissions.md index 8f88ca8..e653a7f 100644 --- a/docs/backend/users-and-permissions.md +++ b/docs/backend/users-and-permissions.md @@ -17,7 +17,7 @@ The admin API accepts the token two ways: - API clients send `Authorization: Bearer `. - The admin SPA sends `X-Requested-With: XMLHttpRequest` and receives the token in an HttpOnly, SameSite=Strict cookie. A cookie-authenticated request that changes state must carry that header, which blocks cross-site request forgery. -The guard is registered in [bouncer](../../modules/bouncer/README.md) under the name `backend` and is the middleware of every admin route except login, refresh and the language bundle. See [Authentication](../services/authentication.md) for tokens and guards in general. +The guard is registered in [bouncer](../../modules/bouncer/README.md) under the name `backend` and is the middleware of every admin route except login, refresh and the language bundle. `cabana.Activate` always registers its own guard under that name, so a plugin that registers another guard as `backend` makes the start-up fail with an error naming the plugin instead of replacing admin authentication. See [Authentication](../services/authentication.md) for tokens and guards in general. The admin keys go in `config/admin.yaml`, with the secret in the environment (`SUMMER_ADMIN__JWT__SECRET`): diff --git a/modules/cabana/README.md b/modules/cabana/README.md index 4bf0589..be473f3 100644 --- a/modules/cabana/README.md +++ b/modules/cabana/README.md @@ -20,7 +20,7 @@ Schema-driven admin backend that compiles WinterCMS-style YAML list, form, filte - Server-rendered partials: `headerPartial: ` in `config_list.yaml` (a strip above the list) and `type: partial` with `path: ` in `fields.yaml` render the template `{ConfigDir}/_.htm` with `html/template` against a view model from the controller's `pact.AdminPartialData`. The result reaches the SPA as an allowlisted node tree, never as an HTML string. A missing or unparsable template, a free-form path or a controller without `pact.AdminPartialData` fails boot. - Singleton settings screens declared with `pact.HasSettings`, read and saved by `cabana.SettingsService`. - Backend navigation (`pact.HasNavigation`) and permissions (`pact.HasPermissions`), filtered per user by `cabana.Registry.Metadata`. `cabana.Allows` implements the permission check with Winter's `hasAnyAccess` semantics: superusers pass, a principal needs any one of the listed codes, and wildcards match on both sides (a grant ending in `.*` covers every code with that prefix, and a required code ending in `.*` is met by any grant under it). -- Admin authentication against WinterCMS's `backend_users` and `backend_user_roles` tables (`cabana.BackendUser`, `cabana.BackendUserRole`, `cabana.BackendUsers`): a JWT guard registered in [bouncer](../bouncer/README.md) as `backend`, login throttling, token refresh and revocation, and two transports. API clients use a Bearer token; the SPA sends `X-Requested-With: XMLHttpRequest` and receives the token in the HttpOnly, SameSite=Strict cookie named by `cabana.AdminCookieName`. Cookie-authenticated requests that change state must carry that header, which blocks cross-site request forgery. +- Admin authentication against WinterCMS's `backend_users` and `backend_user_roles` tables (`cabana.BackendUser`, `cabana.BackendUserRole`, `cabana.BackendUsers`): a JWT guard registered in [bouncer](../bouncer/README.md) as `backend` (a guard another plugin already registered under that name fails `cabana.Activate`), login throttling, token refresh and revocation, and two transports. API clients use a Bearer token; the SPA sends `X-Requested-With: XMLHttpRequest` and receives the token in the HttpOnly, SameSite=Strict cookie named by `cabana.AdminCookieName`. Cookie-authenticated requests that change state must carry that header, which blocks cross-site request forgery. - A consistent JSON envelope for every response: `cabana.WriteData`, `cabana.WriteError` and `cabana.WriteErrorDetails`, typed for documentation as `cabana.Envelope`, `cabana.ListEnvelope`, `cabana.RecordEnvelope` and `cabana.ErrorEnvelope`. A body that cannot be encoded is logged and answered with the generic 500 envelope, never a success status with a truncated body. - OpenAPI documentation: `cabana.AdminList`, `cabana.AdminCreate` and the other `Admin*` functions have empty bodies and exist only to carry the swag annotations of each admin route. - Operator commands for creating administrators and resetting their passwords (see CLI commands). diff --git a/modules/cabana/backend_guard_collision_test.go b/modules/cabana/backend_guard_collision_test.go new file mode 100644 index 0000000..bf15cbe --- /dev/null +++ b/modules/cabana/backend_guard_collision_test.go @@ -0,0 +1,42 @@ +package cabana_test + +import ( + "net/http" + "strings" + "testing" + + "git.golem15.com/golem15/summercms/modules/bouncer" + "git.golem15.com/golem15/summercms/modules/cabana" + "git.golem15.com/golem15/summercms/modules/party" +) + +// openGuard authenticates every request: the guard a plugin would have to +// register under "backend" to take over admin authentication. +type openGuard struct{} + +func (openGuard) Authenticate(*http.Request) (*bouncer.Principal, error) { + return &bouncer.Principal{ID: 1, Backend: true, IsSuperuser: true}, nil +} + +// TestActivateRefusesAForeignBackendGuard pins WR-10: a guard another plugin +// registered under "backend" fails boot; it is never silently reused for the +// admin API. +func TestActivateRefusesAForeignBackendGuard(t *testing.T) { + app := phase10App(t, "development", nil) + guards := bouncer.NewRegistry() + if err := guards.Register("evil.plugin", "backend", openGuard{}); err != nil { + t.Fatal(err) + } + if err := app.Publish(guards); err != nil { + t.Fatal(err) + } + _, err := cabana.Activate(app, []party.Plugin{demoPlugin{fsys: demoFS()}}) + if err == nil || !strings.Contains(err.Error(), "backend") || !strings.Contains(err.Error(), "evil.plugin") { + t.Fatalf("Activate with a foreign backend guard: err=%v, want a boot error naming the guard and its owner", err) + } + + clean := phase10App(t, "development", nil) + if _, err := cabana.Activate(clean, []party.Plugin{demoPlugin{fsys: demoFS()}}); err != nil { + t.Fatalf("Activate without a conflicting guard: %v", err) + } +} diff --git a/modules/cabana/http.go b/modules/cabana/http.go index ef36361..158a6c7 100644 --- a/modules/cabana/http.go +++ b/modules/cabana/http.go @@ -113,10 +113,12 @@ func Activate(app *backpack.App, plugins []party.Plugin) (*Routes, error) { bl := adminBlacklist(app) 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 - } + // The admin API is only as strong as this guard (audience, secret, user + // provider), so cabana always registers its own and never mounts the API + // behind a guard another plugin already put under the name "backend": a + // taken name fails boot instead of silently replacing admin authentication. + if err := guards.Register("summercms.cabana", "backend", guard); err != nil { + return nil, fmt.Errorf("cabana: backend guard: %w", err) } mw, err := guards.Middleware("backend") if err != nil {