From 5fb27d4039911c0c69506edc584c7c11ddae971a Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Sat, 17 Oct 2026 17:03:08 +0100 Subject: [PATCH] Keep instance administration away from org-bound API tokens The organization-owner fix judged browser sessions only. API tokens on instance routes were still checked for scope alone, and the tenant middleware resolves an org-bound token to its own organization without the caller naming one. A token bound to a tenant organization and holding settings:write therefore reached system settings, SSO providers, token management, relay, log level, updates and the other instance surfaces. ensureAdminSession, behind RequireAdmin, and ensureSettingsScope now refuse a token unless it acts for the instance: not bound to any organization other than the default one, with OrgIDs taking precedence over OrgID. The route's scope still applies. The org variants (RequireOrgAdmin, ensureOrgAdminSession, ensureOrgSettingsWriteScope) keep judging tokens by scope alone. tokenHasInstanceWideAuthority is now the same binding rule plus full-access scope. The test notification broadcasts only to the request organization, so it moves to RequireOrgAdmin. Its tenant delivery test already relied on an org-bound token sending it, and that organization's managers may now send it from a session too. No path an organization manager can reach mints a settings scope: relay mobile, install, deploy and action-runner tokens carry relay, agent and report scopes only, and token management is instance administration. The API reference and the api-contracts, security-privacy, agent-lifecycle and storage-recovery contracts record the token rule. --- docs/API.md | 10 + .../v6/internal/subsystems/agent-lifecycle.md | 3 + .../v6/internal/subsystems/api-contracts.md | 20 ++ .../internal/subsystems/security-privacy.md | 11 +- .../internal/subsystems/storage-recovery.md | 4 +- frontend-modern/public/docs/API.md | 10 + .../src/utils/__tests__/docsLinks.test.ts | 13 ++ internal/api/auth.go | 6 +- internal/api/contract_test.go | 38 ++++ .../api/org_bound_token_authority_test.go | 207 ++++++++++++++++++ internal/api/route_inventory_test.go | 56 +++++ internal/api/router_routes_registration.go | 5 +- internal/api/security_setup_fix.go | 75 +++++-- internal/api/security_setup_fix_test.go | 41 ++++ 14 files changed, 472 insertions(+), 27 deletions(-) create mode 100644 internal/api/org_bound_token_authority_test.go diff --git a/docs/API.md b/docs/API.md index 01ae152d68..c2d6272c94 100644 --- a/docs/API.md +++ b/docs/API.md @@ -187,6 +187,14 @@ Some endpoints require admin privileges and/or scopes. Common scopes include: Endpoints that require admin access are noted below. +Admin endpoints are instance administration unless they act only on the +request organization. On instance administration an API token must not be +bound to any organization other than the default one: a token bound to a +tenant organization gets `403` whatever its scopes, as that organization's +owner does. Endpoints that serve only the request organization, such as nodes +and platform connections, agents, install commands, configuration transfer and +reports, accept tokens bound to that organization with the listed scope. + --- ## 📡 Core Endpoints @@ -1082,6 +1090,8 @@ Returns authentication status, proxy auth state, and security posture flags. ``` In Docker installs, the response includes a restart notice. +Token management is instance administration, so a token bound to a tenant +organization gets `403` on these routes. Among the tokens that remain, API-token callers manage only the tokens in their own organizations: every organization a target token is bound to must be one the caller is bound to. Tokens from other organizations are left out of the list and answer `404` on diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index e9d8a2a4ac..bc6a9ec665 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -4439,6 +4439,9 @@ agent link, unlink, uninstall, configuration, Docker and Kubernetes management, agent profiles and fleet diagnostics) declare that authority explicitly with `RequireOrgAdmin`. `RequireAdmin` now admits instance administrators only, so an agent route that does not opt in is closed to organization owners. +Tokens follow the same split: a token bound to the organization keeps these +routes with the listed scope, while `RequireAdmin` and the instance settings +checks refuse it whatever its scopes. The same owner can also build reports on the hosts and agents that organization enrolled: the `/api/admin/reports/*` routes use `ensureOrgAdminSession`, and a diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index c5dc3b795f..2b22f84d12 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -5081,6 +5081,26 @@ auto-register mutation boundary. `pkg/reporting/engine_org_scope_test.go` prove one organization cannot read another's metrics through any report path. Every other capability, `detailLevel` and `authUsername` stay instance administration. + API tokens follow the same split. On instance routes (`RequireAdmin`, + `ensureAdminSession` and the settings scope helpers) a token must act for + the instance: `tokenActsForInstance` admits only a token that is not bound + to any organization other than the default one, with `OrgIDs` taking + precedence over `OrgID`, and the route's own scope still applies. A token + bound to a tenant organization is refused there whatever its scopes, `*` + included, as that organization's owner is. The org variants keep judging + tokens by scope alone, so an org-bound `settings:write` token still reaches + the org-bound routes above, and `/api/test-notification`, which broadcasts + only to the request organization, is one of them. + `tokenHasInstanceWideAuthority` is the same binding rule plus full-access + scope, for quick setup and token regeneration. No path an organization + manager can reach mints a `settings:*` scope: the relay mobile token + carries only `relay:mobile:access`, install, deploy and action-runner + tokens carry agent and report scopes, and token management is instance + administration. `internal/api/org_bound_token_authority_test.go` holds the + token matrix, with + `TestSettingsWriteScopeSplitsInstanceAndOrgBoundToken`, + `TestContract_OrgBoundTokenCannotReachInstanceAdministration` and + `TestRegisteredTestNotificationAdmitsRequestOrganizationManagers`. `internal/api/org_admin_session_authority_test.go` holds the session matrix. Before this, a provider opening a client workspace from the Pulse Account portal arrived as the org owner with every capability `false`, so Settings hid the Infrastructure and Reporting pages diff --git a/docs/release-control/v6/internal/subsystems/security-privacy.md b/docs/release-control/v6/internal/subsystems/security-privacy.md index c73d7f1f5f..1fe82ffd28 100644 --- a/docs/release-control/v6/internal/subsystems/security-privacy.md +++ b/docs/release-control/v6/internal/subsystems/security-privacy.md @@ -483,7 +483,7 @@ administrator or settings-scope checks, expose queue contents to new callers, or change tenant selection. Missing owners fail with 503 rather than dereferencing nil. -The authenticated security-status currentUsername is the validated caller principal, not an administrator configuration disclosure. Public responses omit it. An organisation-scoped local session may receive its own identity while authUsername and instance-settings capabilities remain withheld; clients must not interpret identity availability as privilege. A manager of that organisation additionally receives the org-bound infrastructureRead, availabilityRead and reportingRead capabilities, because their routes admit org managers through RequireOrgAdmin / ensureOrgAdminSession and read only that organisation's state (reports read only its inventory and metrics store, and reportingRead still requires read:nodes); this never extends to authentication, SSO, API tokens, users, roles, audit, relay, system, updates, logs, diagnostics or billing surfaces, whose routes use ensureAdminSession and admit instance administrators only. +The authenticated security-status currentUsername is the validated caller principal, not an administrator configuration disclosure. Public responses omit it. An organisation-scoped local session may receive its own identity while authUsername and instance-settings capabilities remain withheld; clients must not interpret identity availability as privilege. A manager of that organisation additionally receives the org-bound infrastructureRead, availabilityRead and reportingRead capabilities, because their routes admit org managers through RequireOrgAdmin / ensureOrgAdminSession and read only that organisation's state (reports read only its inventory and metrics store, and reportingRead still requires read:nodes); this never extends to authentication, SSO, API tokens, users, roles, audit, relay, system, updates, logs, diagnostics or billing surfaces, whose routes use ensureAdminSession and admit instance administrators only, and refuse an API token bound to a tenant organization whatever its scopes. API token scope copy must match runtime authority. `ai:chat` covers Assistant conversation, model selection, sessions, and knowledge reads only. Knowledge @@ -3385,7 +3385,14 @@ instance RBAC admin grant, whichever organization the request names. It used to admit any manager of the request's organization, which on a multi-tenant install let an organization owner rotate the instance administrator's API token and receive its secret, widen token scopes, change SSO providers and -system settings, apply updates and restart the service. Routes that act only on +system settings, apply updates and restart the service. API tokens follow the +same boundary: `ensureAdminSession` and `ensureSettingsScope` refuse a token +bound to any organization other than the default one (`tokenActsForInstance`) +whatever its scopes. Before that, an org-bound `settings:write` token resolved +to its own organization without naming it, passed the tenant middleware and +reached system settings, SSO providers, token management, relay, log level, +updates and diagnostics. `internal/api/org_bound_token_authority_test.go` holds +the token matrix. Routes that act only on the request organization opt in through `RequireOrgAdmin`, `ensureOrgAdminSession` or `ensureOrgSettingsWriteScope`; every other route fails closed. `internal/api/org_admin_session_authority_test.go` holds the session diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index c21439baa7..d3b4bfb6eb 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -6985,7 +6985,9 @@ scope auto-registration and unregistration events and their cached discovery refreshes to the authorized request organization, including its setup token binding. The registered test-notification route sends its synthetic `node_auto_registered` event to that same request organization. An empty -organization ID maps to the default tenant. +organization ID maps to the default tenant. Because the event stays inside the +request organization, the route uses `RequireOrgAdmin`: that organization's +managers, and tokens bound to it with `settings:write`, may send it. Storage and recovery consumers receive these inventory refresh signals within their own tenant; discovery results and successful node mutations do not establish backup coverage, protection freshness, restore readiness or recovery diff --git a/frontend-modern/public/docs/API.md b/frontend-modern/public/docs/API.md index 01ae152d68..c2d6272c94 100644 --- a/frontend-modern/public/docs/API.md +++ b/frontend-modern/public/docs/API.md @@ -187,6 +187,14 @@ Some endpoints require admin privileges and/or scopes. Common scopes include: Endpoints that require admin access are noted below. +Admin endpoints are instance administration unless they act only on the +request organization. On instance administration an API token must not be +bound to any organization other than the default one: a token bound to a +tenant organization gets `403` whatever its scopes, as that organization's +owner does. Endpoints that serve only the request organization, such as nodes +and platform connections, agents, install commands, configuration transfer and +reports, accept tokens bound to that organization with the listed scope. + --- ## 📡 Core Endpoints @@ -1082,6 +1090,8 @@ Returns authentication status, proxy auth state, and security posture flags. ``` In Docker installs, the response includes a restart notice. +Token management is instance administration, so a token bound to a tenant +organization gets `403` on these routes. Among the tokens that remain, API-token callers manage only the tokens in their own organizations: every organization a target token is bound to must be one the caller is bound to. Tokens from other organizations are left out of the list and answer `404` on diff --git a/frontend-modern/src/utils/__tests__/docsLinks.test.ts b/frontend-modern/src/utils/__tests__/docsLinks.test.ts index 347b24fb20..811b8a6fa0 100644 --- a/frontend-modern/src/utils/__tests__/docsLinks.test.ts +++ b/frontend-modern/src/utils/__tests__/docsLinks.test.ts @@ -247,6 +247,19 @@ describe('docsLinks', () => { ); }); + it('documents instance administration refusing organization-bound tokens', () => { + const api = readFileSync(path.join(repoRoot, 'docs', 'API.md'), 'utf8'); + expect(api).toContain('tenant organization gets `403` whatever its scopes'); + expect(api).toContain('Token management is instance administration'); + const guard = readFileSync( + path.join(repoRoot, 'internal', 'api', 'security_setup_fix.go'), + 'utf8', + ); + expect(guard).toContain( + 'ensureTokenSettingsScope(w, req, record, scope) && refuseOrgBoundToken(w, req, record)', + ); + }); + it('documents network discovery without the removed AI Discovery routes', () => { const api = readFileSync(path.join(repoRoot, 'docs', 'API.md'), 'utf8'); expect(api).toContain('`POST /api/discover` (admin, `settings:write`)'); diff --git a/internal/api/auth.go b/internal/api/auth.go index 1522b3db97..114d4980d8 100644 --- a/internal/api/auth.go +++ b/internal/api/auth.go @@ -1048,14 +1048,16 @@ func RequireAuth(cfg *config.Config, handler http.HandlerFunc) http.HandlerFunc // RequireAdmin middleware checks for authentication and instance admin // privileges. Proxy-auth users must have the configured admin role. Session and // OIDC users must be instance administrators (ensureAdminSession); an -// organization's owner or admin is refused. +// organization's owner or admin is refused, and so is an API token bound to a +// tenant organization, whatever scopes it holds. func RequireAdmin(cfg *config.Config, handler http.HandlerFunc) http.HandlerFunc { return requireAdminSession(cfg, ensureAdminSession, handler) } // RequireOrgAdmin is RequireAdmin for routes whose handlers act only on the // request organization's own config and monitor state. Session users who can -// manage that organization are admitted as well (ensureOrgAdminSession). Use it +// manage that organization are admitted as well (ensureOrgAdminSession), and +// tokens bound to it are judged by the route's scope alone. Use it // only after checking that the handler cannot reach instance-wide state or // another organization's data. func RequireOrgAdmin(cfg *config.Config, handler http.HandlerFunc) http.HandlerFunc { diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 87da635d4e..001ff0ed25 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -23812,6 +23812,44 @@ func TestContract_OrgBoundTokenIsScopedAwayFromDefaultOrg(t *testing.T) { } } +// TestContract_OrgBoundTokenCannotReachInstanceAdministration pins the token +// side of the instance/org split: a token bound to a tenant organization +// resolves to that organization and keeps the org routes its scope covers, +// but instance routes refuse it whatever its scope. +func TestContract_OrgBoundTokenCannotReachInstanceAdministration(t *testing.T) { + defer SetMultiTenantEnabled(false) + SetMultiTenantEnabled(true) + t.Setenv("PULSE_DEV", "true") + + rawToken := "org-bound-instance-admin-token.12345678" + record := newTokenRecord(t, rawToken, []string{config.ScopeWildcard}, nil) + record.OrgID = "org-a" + cfg := newTestConfigWithTokens(t, record) + if err := config.NewMultiTenantPersistence(cfg.DataPath).SaveOrganization(&models.Organization{ID: "org-a", DisplayName: "Org A"}); err != nil { + t.Fatalf("save organization: %v", err) + } + router := newMultiTenantRouter(t, cfg) + + serve := func(method, path, body string) *httptest.ResponseRecorder { + req := httptest.NewRequest(method, path, strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-API-Token", rawToken) + rec := httptest.NewRecorder() + router.Handler().ServeHTTP(rec, req) + return rec + } + + if rec := serve(http.MethodPost, "/api/system/settings/update", `{"discoveryEnabled":true}`); rec.Code != http.StatusForbidden { + t.Fatalf("system settings update status = %d, want 403: %s", rec.Code, rec.Body.String()) + } + if rec := serve(http.MethodGet, "/api/security/tokens", ""); rec.Code != http.StatusForbidden { + t.Fatalf("token list status = %d, want 403: %s", rec.Code, rec.Body.String()) + } + if rec := serve(http.MethodGet, "/api/security/tokens/relay-mobile", ""); rec.Code != http.StatusMethodNotAllowed { + t.Fatalf("org-bound relay mobile route status = %d, want 405 from its handler: %s", rec.Code, rec.Body.String()) + } +} + // TestContract_ProxyAuthConfiguredRoleHeaderMissingIsNonAdmin pins the // proxy-auth admin boundary: once an admin role header is configured, admin // authorization must be proven by that header rather than inherited from diff --git a/internal/api/org_bound_token_authority_test.go b/internal/api/org_bound_token_authority_test.go new file mode 100644 index 0000000000..ae51ca9641 --- /dev/null +++ b/internal/api/org_bound_token_authority_test.go @@ -0,0 +1,207 @@ +package api + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" +) + +// Instance administration is the same boundary for API tokens as for +// sessions. A token bound to a tenant organization acts for that organization, +// so on instance routes (RequireAdmin, ensureAdminSession and the settings +// scope helpers) it is refused whatever scopes it holds, the same as that +// organization's owner. Routes that serve only the request organization +// (RequireOrgAdmin, ensureOrgAdminSession, ensureOrgSettingsWriteScope) keep +// judging tokens by scope alone. Before this, an org-bound token holding +// settings:write reached system settings, SSO providers, token management, +// updates, relay, log level and the other instance surfaces. + +type orgBoundTokenBinding struct { + name string + orgID string + orgIDs []string + instance bool // holds instance-wide authority +} + +var orgBoundTokenBindings = []orgBoundTokenBinding{ + {name: "unbound legacy token", instance: true}, + {name: "bound to the default org", orgID: "default", instance: true}, + {name: "OrgIDs naming only the default org", orgIDs: []string{"default"}, instance: true}, + {name: "bound to a tenant org", orgID: "org-a", instance: false}, + {name: "OrgIDs naming a tenant org", orgIDs: []string{"org-a"}, instance: false}, + {name: "OrgIDs naming the default and a tenant org", orgIDs: []string{"default", "org-a"}, instance: false}, + {name: "OrgIDs taking precedence over a default OrgID", orgID: "default", orgIDs: []string{"org-a"}, instance: false}, +} + +func orgBoundTokenFixture(t *testing.T, binding orgBoundTokenBinding, scopes []string) (*config.Config, string) { + t.Helper() + raw := "org-bound-authority-token-" + strings.ReplaceAll(binding.name, " ", "-") + ".12345678" + record := newTokenRecord(t, raw, scopes, nil) + record.OrgID = binding.orgID + record.OrgIDs = append([]string(nil), binding.orgIDs...) + return newTestConfigWithTokens(t, record), raw +} + +func orgBoundTokenRequest(t *testing.T, cfg *config.Config, method, target, raw string) *http.Request { + t.Helper() + req := newLoopbackRequest(method, target, nil) + record, ok := cfg.ValidateAPIToken(raw) + if !ok || record == nil { + t.Fatalf("fixture token did not validate") + } + attachAPITokenRecord(req, record) + return req +} + +func TestInstanceSettingsScopeRequiresInstanceWideToken(t *testing.T) { + for _, binding := range orgBoundTokenBindings { + for _, scopes := range [][]string{ + {config.ScopeSettingsRead, config.ScopeSettingsWrite}, + {config.ScopeWildcard}, + {config.ScopeMonitoringRead}, + } { + hasSettings := scopes[0] != config.ScopeMonitoringRead + t.Run(binding.name+"/"+strings.Join(scopes, ","), func(t *testing.T) { + cfg, raw := orgBoundTokenFixture(t, binding, scopes) + checks := []struct { + name string + fn func(*config.Config, http.ResponseWriter, *http.Request) bool + want bool + }{ + {"ensureSettingsReadScope", ensureSettingsReadScope, binding.instance && hasSettings}, + {"ensureSettingsWriteScope", ensureSettingsWriteScope, binding.instance && hasSettings}, + {"ensureOrgSettingsWriteScope", ensureOrgSettingsWriteScope, hasSettings}, + {"ensureAdminSession", ensureAdminSession, binding.instance}, + {"ensureOrgAdminSession", ensureOrgAdminSession, true}, + } + for _, check := range checks { + rec := httptest.NewRecorder() + got := check.fn(cfg, rec, orgBoundTokenRequest(t, cfg, http.MethodPost, "/api/x", raw)) + if got != check.want { + t.Fatalf("%s = %v, want %v", check.name, got, check.want) + } + if !got && rec.Code != http.StatusForbidden { + t.Fatalf("%s refused with %d, want 403", check.name, rec.Code) + } + } + }) + } + } +} + +func TestRequireAdminRequiresInstanceWideTokenAndRequireOrgAdminDoesNot(t *testing.T) { + for _, binding := range orgBoundTokenBindings { + t.Run(binding.name, func(t *testing.T) { + cfg, raw := orgBoundTokenFixture(t, binding, []string{config.ScopeSettingsWrite}) + reached := func(wrap func(*config.Config, http.HandlerFunc) http.HandlerFunc) (bool, int) { + hit := false + rec := httptest.NewRecorder() + req := newLoopbackRequest(http.MethodPost, "/api/x", nil) + req.Header.Set("X-API-Token", raw) + wrap(cfg, RequireScope(config.ScopeSettingsWrite, func(http.ResponseWriter, *http.Request) { hit = true }))(rec, req) + return hit, rec.Code + } + if hit, code := reached(RequireAdmin); hit != binding.instance { + t.Fatalf("RequireAdmin reached handler = %v (status %d), want %v", hit, code, binding.instance) + } else if !hit && code != http.StatusForbidden { + t.Fatalf("RequireAdmin refused with %d, want 403", code) + } + if hit, code := reached(RequireOrgAdmin); !hit { + t.Fatalf("RequireOrgAdmin refused a settings:write token with %d", code) + } + }) + } +} + +// The full router: an org-bound token resolves to its own organization +// without naming one, passes the tenant middleware, and must still be refused +// by every instance route while reaching the org-bound routes its scope covers. +func TestOrgBoundSettingsTokenRouterMatrix(t *testing.T) { + defer SetMultiTenantEnabled(false) + SetMultiTenantEnabled(true) + t.Setenv("PULSE_DEV", "true") + + scopes := []string{config.ScopeSettingsRead, config.ScopeSettingsWrite} + instanceRaw := "instance-settings-token.12345678" + orgRaw := "org-a-settings-token.12345678" + orgReadOnlyRaw := "org-a-monitoring-token.12345678" + instance := newTokenRecord(t, instanceRaw, scopes, nil) + orgBound := newTokenRecord(t, orgRaw, scopes, nil) + orgBound.OrgID = "org-a" + orgReadOnly := newTokenRecord(t, orgReadOnlyRaw, []string{config.ScopeMonitoringRead}, nil) + orgReadOnly.OrgID = "org-a" + cfg := newTestConfigWithTokens(t, instance, orgBound, orgReadOnly) + mtp := config.NewMultiTenantPersistence(cfg.DataPath) + if err := mtp.SaveOrganization(&models.Organization{ID: "org-a", DisplayName: "Org A", OwnerUserID: "owner"}); err != nil { + t.Fatalf("save organization: %v", err) + } + mtm := monitoring.NewMultiTenantMonitor(cfg, mtp, nil) + t.Cleanup(mtm.Stop) + router := NewRouter(cfg, nil, mtm, nil, nil, "1.0.0") + cleanupTestRouter(t, router) + + serve := func(method, path, raw string) *httptest.ResponseRecorder { + body := "" + switch { + case method == http.MethodGet: + case path == "/api/security/tokens": + // Without scopes the new token would ask for full access, which a + // settings token may not grant. + body = `{"name":"matrix","scopes":["settings:read"]}` + default: + body = "{}" + } + req := httptest.NewRequest(method, path, strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-API-Token", raw) + rec := httptest.NewRecorder() + router.Handler().ServeHTTP(rec, req) + return rec + } + + instanceRoutes := []struct{ method, path string }{ + {http.MethodPost, "/api/system/settings/update"}, + {http.MethodGet, "/api/system/settings"}, + {http.MethodPost, "/api/security/sso/providers"}, + {http.MethodGet, "/api/security/sso/providers"}, + {http.MethodGet, "/api/security/tokens"}, + {http.MethodPost, "/api/security/tokens"}, + {http.MethodPost, "/api/security/reset-lockout"}, + {http.MethodGet, "/api/logs/level"}, + {http.MethodPost, "/api/logs/level"}, + {http.MethodGet, "/api/updates/status"}, + {http.MethodGet, "/api/settings/relay"}, + } + for _, route := range instanceRoutes { + t.Run("instance "+route.method+" "+route.path, func(t *testing.T) { + if rec := serve(route.method, route.path, orgRaw); rec.Code != http.StatusForbidden { + t.Fatalf("org-bound settings token answered %d, want 403: %s", rec.Code, strings.TrimSpace(rec.Body.String())) + } + if rec := serve(route.method, route.path, instanceRaw); rec.Code == http.StatusForbidden || rec.Code == http.StatusUnauthorized { + t.Fatalf("instance-wide settings token answered %d: %s", rec.Code, strings.TrimSpace(rec.Body.String())) + } + }) + } + + orgRoutes := []struct{ method, path string }{ + // The handler answers 405 to anything but POST, so reaching it proves + // ensureOrgSettingsWriteScope admitted the token without minting one. + {http.MethodGet, "/api/security/tokens/relay-mobile"}, + {http.MethodGet, "/api/agents/diagnostics"}, + } + for _, route := range orgRoutes { + t.Run("org "+route.method+" "+route.path, func(t *testing.T) { + if rec := serve(route.method, route.path, orgRaw); rec.Code == http.StatusForbidden || rec.Code == http.StatusUnauthorized { + t.Fatalf("org-bound settings token answered %d on an org route: %s", rec.Code, strings.TrimSpace(rec.Body.String())) + } + if rec := serve(route.method, route.path, orgReadOnlyRaw); rec.Code != http.StatusForbidden { + t.Fatalf("org-bound token without settings scope answered %d, want 403", rec.Code) + } + }) + } +} diff --git a/internal/api/route_inventory_test.go b/internal/api/route_inventory_test.go index 679d5ba475..394deb49b0 100644 --- a/internal/api/route_inventory_test.go +++ b/internal/api/route_inventory_test.go @@ -20,8 +20,10 @@ import ( gorillaws "github.com/gorilla/websocket" "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" "github.com/rcourtman/pulse-go-rewrite/internal/websocket" + internalauth "github.com/rcourtman/pulse-go-rewrite/pkg/auth" ) func TestReleaseNoteImageRouteAuthorization(t *testing.T) { @@ -854,3 +856,57 @@ func TestRegisteredTestNotificationStaysWithRequestOrganization(t *testing.T) { }) } } + +// The test notification reaches only the request organization's clients, so +// the route is org-bound: a manager of that organization may send it, a viewer +// may not. +func TestRegisteredTestNotificationAdmitsRequestOrganizationManagers(t *testing.T) { + SetMultiTenantEnabled(true) + t.Cleanup(func() { SetMultiTenantEnabled(false) }) + t.Setenv("PULSE_DEV", "true") + + hashed, err := internalauth.HashPassword("Password!1") + if err != nil { + t.Fatalf("hash password: %v", err) + } + dataDir := t.TempDir() + cfg := &config.Config{DataPath: dataDir, ConfigPath: dataDir, AuthUser: "platform-admin", AuthPass: hashed} + mtp := config.NewMultiTenantPersistence(dataDir) + if err := mtp.SaveOrganization(&models.Organization{ + ID: "org-a", + DisplayName: "Org A", + OwnerUserID: "org-owner", + Members: []models.OrganizationMember{ + {UserID: "org-owner", Role: models.OrgRoleOwner, AddedAt: time.Now()}, + {UserID: "org-viewer", Role: models.OrgRoleViewer, AddedAt: time.Now()}, + }, + }); err != nil { + t.Fatalf("save organization: %v", err) + } + hub := websocket.NewHub(nil) + go hub.Run() + t.Cleanup(hub.Stop) + monitors := monitoring.NewMultiTenantMonitor(cfg, mtp, nil) + t.Cleanup(monitors.Stop) + router := NewRouter(cfg, nil, monitors, hub, nil, "1.0.0") + + for _, tc := range []struct { + user string + status int + }{ + {"org-owner", http.StatusOK}, + {"org-viewer", http.StatusForbidden}, + } { + session := "test-notification-" + tc.user + "-" + strconv.FormatInt(time.Now().UnixNano(), 10) + GetSessionStore().CreateSession(session, time.Hour, "agent", "127.0.0.1", tc.user) + req := httptest.NewRequest(http.MethodPost, "/api/test-notification", nil) + req.Header.Set("X-CSRF-Token", generateCSRFToken(session)) + req.Header.Set("X-Pulse-Org-ID", "org-a") + req.AddCookie(&http.Cookie{Name: cookieNameSession, Value: session}) + rec := httptest.NewRecorder() + router.Handler().ServeHTTP(rec, req) + if rec.Code != tc.status { + t.Fatalf("%s test notification status = %d, want %d: %s", tc.user, rec.Code, tc.status, rec.Body.String()) + } + } +} diff --git a/internal/api/router_routes_registration.go b/internal/api/router_routes_registration.go index bd18ac3acd..662d78363d 100644 --- a/internal/api/router_routes_registration.go +++ b/internal/api/router_routes_registration.go @@ -476,8 +476,9 @@ func (r *Router) registerConfigSystemRoutes(updateHandlers *UpdateHandlers) { r.mux.HandleFunc("/api/auto-unregister", r.configHandlers.HandleAutoUnregister) // Discovery endpoint // Test endpoint for WebSocket notifications - // SECURITY: Require settings:write scope for test notifications to prevent unauthenticated broadcasting - r.mux.HandleFunc("/api/test-notification", RequireAdmin(r.config, RequireScope(config.ScopeSettingsWrite, func(w http.ResponseWriter, req *http.Request) { + // SECURITY: Require settings:write scope for test notifications to prevent unauthenticated broadcasting. + // The event reaches only the request organization's clients, so its managers and tokens may send it. + r.mux.HandleFunc("/api/test-notification", RequireOrgAdmin(r.config, RequireScope(config.ScopeSettingsWrite, func(w http.ResponseWriter, req *http.Request) { if req.Method != http.MethodPost { http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) return diff --git a/internal/api/security_setup_fix.go b/internal/api/security_setup_fix.go index c6665500b7..72a597a078 100644 --- a/internal/api/security_setup_fix.go +++ b/internal/api/security_setup_fix.go @@ -145,8 +145,15 @@ func isRunningAsRoot() bool { // that serves only the request's own organization opts into their authority // explicitly with ensureOrgAdminSession or RequireOrgAdmin; the default is // instance administration, so a route that forgets to choose fails closed. -// Requests without a session cookie are left to the caller's other checks. +// +// API tokens follow the same boundary. A token bound to a tenant organization +// acts for that organization, like its owner, so it is refused here whatever +// scopes it holds; the route's scope check stays with the caller. Requests +// without a token or a session cookie are left to the caller's other checks. func ensureAdminSession(cfg *config.Config, w http.ResponseWriter, req *http.Request) bool { + if record := getAPITokenRecordFromRequest(req); record != nil && !refuseOrgBoundToken(w, req, record) { + return false + } cookie, err := readSessionCookie(req) if err != nil || cookie.Value == "" || !ValidateSession(cookie.Value) { return true @@ -288,20 +295,12 @@ func sessionUserHasRBACAdminGrant(username string) bool { return false } +// ensureSettingsScope guards instance administration. A token caller needs the +// scope and instance-wide authority (refuseOrgBoundToken); a session caller +// must pass ensureAdminSession. func ensureSettingsScope(cfg *config.Config, w http.ResponseWriter, req *http.Request, scope string) bool { - record := getAPITokenRecordFromRequest(req) - if record != nil { - if record.HasScope(scope) { - return true - } - - log.Warn(). - Str("token_id", record.ID). - Str("path", req.URL.Path). - Str("required_scope", scope). - Msg("API token missing required settings scope for privileged operation") - respondMissingScope(w, scope) - return false + if record := getAPITokenRecordFromRequest(req); record != nil { + return ensureTokenSettingsScope(w, req, record, scope) && refuseOrgBoundToken(w, req, record) } if !ensureAdminSession(cfg, w, req) { @@ -312,15 +311,44 @@ func ensureSettingsScope(cfg *config.Config, w http.ResponseWriter, req *http.Re } // ensureOrgSettingsScope is ensureSettingsScope for routes that act only on -// the request organization's own state: token callers need the scope, and -// session callers pass ensureOrgAdminSession instead of ensureAdminSession. +// the request organization's own state: token callers need only the scope, +// and session callers pass ensureOrgAdminSession instead of ensureAdminSession. func ensureOrgSettingsScope(cfg *config.Config, w http.ResponseWriter, req *http.Request, scope string) bool { if record := getAPITokenRecordFromRequest(req); record != nil { - return ensureSettingsScope(cfg, w, req, scope) + return ensureTokenSettingsScope(w, req, record, scope) } return ensureOrgAdminSession(cfg, w, req) } +func ensureTokenSettingsScope(w http.ResponseWriter, req *http.Request, record *config.APITokenRecord, scope string) bool { + if record.HasScope(scope) { + return true + } + + log.Warn(). + Str("token_id", record.ID). + Str("path", req.URL.Path). + Str("required_scope", scope). + Msg("API token missing required settings scope for privileged operation") + respondMissingScope(w, scope) + return false +} + +// refuseOrgBoundToken admits a token on an instance route only when it acts +// for the instance (tokenActsForInstance). It answers 403 otherwise. +func refuseOrgBoundToken(w http.ResponseWriter, req *http.Request, record *config.APITokenRecord) bool { + if tokenActsForInstance(record) { + return true + } + + log.Warn(). + Str("token_id", record.ID). + Str("path", req.URL.Path). + Msg("Organization-bound API token refused for instance administration") + http.Error(w, "Admin privileges required", http.StatusForbidden) + return false +} + func ensureOrgSettingsWriteScope(cfg *config.Config, w http.ResponseWriter, req *http.Request) bool { return ensureOrgSettingsScope(cfg, w, req, config.ScopeSettingsWrite) } @@ -744,10 +772,17 @@ func handleQuickSecuritySetupFixed(r *Router) http.HandlerFunc { } // tokenHasInstanceWideAuthority reports whether a token holds full-access scope -// and is not bound to any organization other than the default one. OrgIDs -// takes precedence over OrgID, as it does for tenant access checks. +// and acts for the instance (tokenActsForInstance). func tokenHasInstanceWideAuthority(record *config.APITokenRecord) bool { - if record == nil || !record.HasScope(config.ScopeWildcard) { + return record != nil && record.HasScope(config.ScopeWildcard) && tokenActsForInstance(record) +} + +// tokenActsForInstance reports whether a token is not bound to any +// organization other than the default one. Only such a token may reach +// instance administration, with whatever scope the route requires. OrgIDs +// takes precedence over OrgID, as it does for tenant access checks. +func tokenActsForInstance(record *config.APITokenRecord) bool { + if record == nil { return false } isDefault := func(orgID string) bool { diff --git a/internal/api/security_setup_fix_test.go b/internal/api/security_setup_fix_test.go index 22214cb2f0..c98663149d 100644 --- a/internal/api/security_setup_fix_test.go +++ b/internal/api/security_setup_fix_test.go @@ -692,6 +692,47 @@ func TestSettingsWriteScopeSplitsInstanceAndOrgManagerSession(t *testing.T) { } } +// A token bound to a tenant organization is held to the same split as that +// organization's owner: its settings scope reaches the org variant only. +func TestSettingsWriteScopeSplitsInstanceAndOrgBoundToken(t *testing.T) { + cfg := &config.Config{AuthUser: "platform-admin"} + request := func(orgID string, orgIDs ...string) *http.Request { + record, err := config.NewAPITokenRecord("settings-split-token.12345678", "settings", []string{config.ScopeSettingsWrite}) + if err != nil { + t.Fatalf("NewAPITokenRecord: %v", err) + } + record.OrgID = orgID + record.OrgIDs = orgIDs + req := httptest.NewRequest(http.MethodPost, "/api/security/test", nil) + attachAPITokenRecord(req, record) + return req + } + + for _, tc := range []struct { + name string + req *http.Request + instance bool + }{ + {"unbound", request(""), true}, + {"default org", request("default"), true}, + {"tenant org", request("org-a"), false}, + {"default and tenant orgs", request("", "default", "org-a"), false}, + } { + t.Run(tc.name, func(t *testing.T) { + rr := httptest.NewRecorder() + if got := ensureSettingsWriteScope(cfg, rr, tc.req); got != tc.instance { + t.Fatalf("ensureSettingsWriteScope = %v, want %v (status %d)", got, tc.instance, rr.Code) + } + if !tc.instance && rr.Code != http.StatusForbidden { + t.Fatalf("expected 403 for org-bound token on instance settings, got %d", rr.Code) + } + if !ensureOrgSettingsWriteScope(cfg, httptest.NewRecorder(), tc.req) { + t.Fatal("ensureOrgSettingsWriteScope should allow a settings:write token") + } + }) + } +} + func TestValidateBcryptHash(t *testing.T) { tests := []struct { name string