diff --git a/docs/OIDC.md b/docs/OIDC.md index 75d89c286..acd9b6342 100644 --- a/docs/OIDC.md +++ b/docs/OIDC.md @@ -34,6 +34,14 @@ Restrict access to specific users or groups: - **Allowed Domains**: Restrict to specific email domains (e.g., `example.com`). - **Allowed Emails**: Allow specific email addresses. +> **Administrator requirement**: SSO authentication does not grant instance +> administrator privileges by itself. Before removing the configured local +> administrator or relying on SSO-only access, map a trusted IdP group to the +> built-in `admin` role. Keep that administrator group in **Allowed Groups** so +> the login and authorization boundaries describe the same trusted population. +> An empty **Allowed Groups** list allows every IdP user to sign in, but does not +> make those users administrators. + ### Group-to-Role Mapping Automatically assign Pulse roles based on OIDC group membership. When a user logs in, Pulse checks their groups claim and assigns the corresponding roles. Mapping groups to the built-in `admin`, `operator`, and `viewer` roles is included with Community SSO. Creating custom roles and manually managing user assignments remain Pro RBAC features. @@ -62,6 +70,9 @@ See [CONFIGURATION.md](CONFIGURATION.md). - Multiple groups can map to multiple roles (user gets all matching roles). - Role assignments are updated on every login to reflect current group membership. - Role changes are logged to the audit log for compliance tracking. +- Instance-administration routes require the built-in `admin` role or another + role with an explicit `admin` grant on all resources. The `operator` and + `viewer` mappings never inherit administrator access on SSO-only instances. **Example:** If a user has groups `["oidc-admins", "developers"]` and you have mappings: diff --git a/docs/RBAC.md b/docs/RBAC.md index 57f680dec..3162eaa43 100644 --- a/docs/RBAC.md +++ b/docs/RBAC.md @@ -27,6 +27,12 @@ Users can hold multiple roles. Their effective permissions are combined across a When using OIDC/SSO, built-in roles can be automatically assigned based on group membership on every plan. See [OIDC Group-to-Role Mapping](OIDC.md#group-to-role-mapping) for configuration. Creating custom roles and manually managing user assignments require Pro RBAC. +SSO authentication is not an administrator grant. An SSO-only deployment must +map at least one trusted IdP group to the built-in `admin` role (or retain a +configured local administrator) before operators can use instance-administration +routes. Mapping a user to `operator`, `viewer`, or no role never elevates that +user merely because no local administrator is configured. + --- ## Quick Start diff --git a/docs/UPGRADE_v6.md b/docs/UPGRADE_v6.md index 9d9390a7a..c60a2acab 100644 --- a/docs/UPGRADE_v6.md +++ b/docs/UPGRADE_v6.md @@ -127,6 +127,7 @@ stable. Keep v5.1.35 as the explicit rollback target for the v6.0.0 cutover. - Create an encrypted config backup: **Settings → System → Recovery → Create Backup** (older versions labeled this **Backups**) - Open **Settings → System → Updates** and review the upgrade checks on the update plan. Pulse checks the server update path, current agent continuity, and agent reporting token scope before you install. These checks describe the currently reported fleet; they do not prove every installed agent is online or already updated. - Confirm you can access the host/container console (for rollback and bootstrap token retrieval) +- If the instance uses SSO without a configured local administrator, map a trusted IdP group to the built-in `admin` role before upgrading. SSO authentication alone does not grant instance-administrator access; an unmapped, `operator`, or `viewer` SSO session cannot manage settings, transfer configuration, or control infrastructure actions. - If you have any external integrations or scripts: review the **API Changes** section below ## Upgrade Paths diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 991935724..886da60e0 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -2655,6 +2655,15 @@ state, including notification-delivery reconciliation, grant no agent enrollment, report, command, or lifecycle authority. They must remain separate from the fail-closed host continuity hydration that agent lifecycle owns. +Shared browser-session authorization for the action plan, decision, and execute +lifecycle must require a configured local administrator or an effective RBAC +`admin` grant on `*`. SSO authentication and the `sso:` session-user prefix are +identity evidence only; agent lifecycle must never reinterpret them as command +authority when no local administrator is configured. Keep +`TestActionRoutesRequireExplicitSSOAdminOnSSOOnlyInstance` proving that an +unassigned SSO session is refused across all three action stages while an +explicitly mapped administrator reaches the lifecycle handlers. + 1. Update this contract when agent lifecycle ownership changes. Routes added under the shared `internal/api/` extension point that are clearly outside lifecycle ownership (for example `POST /api/ai/patrol/preflight`, the `patrol_preflight` snapshot field added to `/api/settings/ai`, the auto-trigger preflight dispatch on settings save, the startup-seed dispatch in `NewAISettingsHandler`, and the cached-preflight integration into the Patrol `tools` readiness check — all owned by ai-runtime) do not extend this subsystem's contract; they live in their owning subsystem. Canonical scoped Patrol resolution on `POST /api/ai/patrol/run` and structured `patrol_assess_finding` lifecycle outcomes are likewise adjacent AI/API contracts: they may consume agent-reported identities and evidence, but they do not change agent registration, install, token, profile, command transport, update, or fleet-lifecycle authority. 2. Keep shared API proof routing aligned whenever install, register, or profile payloads change. 3. Update runtime and settings tests in the same slice when lifecycle behavior changes. Shell installer lifecycle changes must keep `scripts/installtests/install_sh_test.go` covering explicit flags, persisted connection state, legacy running-process/service recovery, legacy single-dash v5 agent flag recovery, repeated disk-exclusion recovery, and secure token-file service argument rendering for update re-entry. Host metrics tests must prove excluded FreeBSD `fdescfs` mounts are filtered before filesystem usage is attempted. diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index d21a4ada8..1b96d49be 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -4308,19 +4308,22 @@ auto-register mutation boundary. RBAC licence allows everything, while `ensureSettingsScope` refused the request (`28fd2d1c1`). `canCapturePublicURL`, `discovery.isAdminRequest` and the config export and import guards compared the session user against - `cfg.AuthUser` directly, which is empty on an instance whose only - administrators are SSO principals, so they refused callers the settings - routes admitted (`7d066f1d5`). `RequirePlatformAdmin` did the same against - the `billingAdmin` capability (`563a3aa06`). + `cfg.AuthUser` directly, which cannot recognize an SSO principal carrying + an explicit RBAC administrator grant (`7d066f1d5`). + `RequirePlatformAdmin` did the same against the `billingAdmin` capability + (`563a3aa06`). Therefore: a session admin decision is `sessionUserCarriesAdminPrivileges`, reached through `ensureAdminSession` or `snapshot.sessionIsAdmin` rather - than re-derived. A caller bound to a tenant organization is never an - instance administrator regardless of username, tested with - `sessionIsOrgScoped`, because that helper's SSO fallback is correct for a - single-tenant instance and would hand every tenant platform admin on a - control plane configuring no local admin. Authorizer output alone cannot - establish a capability while `DefaultAuthorizer` allows every action. + than re-derived. That decision accepts only the configured local + administrator identity or an effective RBAC allow for `admin` on `*`. + SSO authentication, an `sso:` principal prefix, and the absence of a local + administrator are never authorization. SSO-only deployments retain their + administration path by mapping a trusted IdP group to an administrator + role. A caller bound to a tenant organization is never an instance + administrator regardless of username, tested with `sessionIsOrgScoped`. + Authorizer output alone cannot establish a capability while + `DefaultAuthorizer` allows every action. New capability fields must state which route they describe and be exercised against it in both directions, since a source read cannot establish what a @@ -4328,12 +4331,23 @@ auto-register mutation boundary. handler. Regression coverage: `TestSettingsCapabilitiesMatchRouteEnforcementWithoutRBAC` in `internal/api/security_status_capability_enforcement_test.go`, - `TestOIDCOnlyAdminReachesEveryAdminGuard` and + `TestSettingsCapabilitiesRequireExplicitSSOAdminWhenNoLocalAdminConfigured` + in `internal/api/security_status_capability_enforcement_test.go`, + `TestSSOOnlyExplicitAdminReachesEveryAdminGuard` and `TestUnrelatedSSOUserStillRefusedWhenLocalAdminConfigured` in `internal/api/oidc_only_admin_parity_test.go`, and `TestPlatformAdminRouteAgreesWithBillingAdminCapability` and `TestPlatformAdminRouteRefusesOrgScopedTenantSession` in - `internal/api/platform_admin_session_parity_test.go`. + `internal/api/platform_admin_session_parity_test.go`. Configuration + transfer and infrastructure-action routes must prove the same boundary with + `TestConfigTransferSSOViewerDeniedBeforeBodyRead` and + `TestActionRoutesRequireExplicitSSOAdminOnSSOOnlyInstance`. + +36. Configured SSO admission restrictions fail closed on missing identity + claims. OIDC and SAML providers with `allowedEmails` or `allowedDomains` + must reject an assertion that omits the email claim; absence is not a way + around an allowlist. Group restrictions remain independently mandatory + when configured. ## Current State diff --git a/docs/release-control/v6/internal/subsystems/organization-settings.md b/docs/release-control/v6/internal/subsystems/organization-settings.md index 9937c15b1..16ae4e726 100644 --- a/docs/release-control/v6/internal/subsystems/organization-settings.md +++ b/docs/release-control/v6/internal/subsystems/organization-settings.md @@ -144,7 +144,16 @@ create, review, and approve cross-organization shares. rows, disable mutations, retain a visible retry notice, and do not render the normal empty state until a healthy response has been received. -11. A rejected legacy RBAC import must not destroy the store. The import is +11. Keep the canonical and shipped RBAC guides explicit that SSO + authentication is identity, not instance-administrator authority. An + SSO-only deployment must map a trusted IdP group to the built-in `admin` + role (or retain a configured local administrator); `operator`, `viewer`, + and unassigned sessions remain non-administrative even when no local + administrator exists. Preserve this boundary in + `frontend-modern/src/utils/__tests__/docsLinks.test.ts` together with exact + canonical/public guide parity. + +12. A rejected legacy RBAC import must not destroy the store. The import is transactional and leaves the legacy files in place, so a failure leaves the database un-migrated rather than half-migrated, which denies access rather than granting it. Failing manager construction instead takes the whole org's diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 5e59a5afa..83cd197e7 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -2081,6 +2081,14 @@ corrupt or stale inputs must keep their source files, and non-default organization stores must never consume the default organization's legacy files. +Configuration export and import are recovery-adjacent privileged operations. +On an SSO-enabled instance, browser sessions must carry an effective RBAC +`admin` grant on `*`; SSO authentication alone is never recovery authority when +no local administrator is configured. Keep +`TestConfigTransferSSOViewerDeniedBeforeBodyRead` proving that an unassigned or +viewer SSO session is refused before an import body is read, and keep the +canonical pre-handler authorization proof covering both transfer routes. + 1. Update this contract when canonical storage or recovery entry points move. Routes added under the shared `internal/api/` extension point that are clearly outside storage/recovery ownership (for example `POST /api/ai/patrol/preflight`, the `patrol_preflight` snapshot field added to `/api/settings/ai`, the auto-trigger preflight dispatch on settings save, the startup-seed dispatch in `NewAISettingsHandler`, and the cached-preflight integration into the Patrol `tools` readiness check — all owned by ai-runtime) do not extend this subsystem's contract; they live in their owning subsystem. Exact scoped-run identity resolution and structured `present`, `resolved`, or `uncertain` Patrol finding assessments on that shared boundary are also adjacent AI/API state. Storage and recovery may supply evidence or observe the resulting finding, but an assessment is not backup success, restore verification, recovery freshness, storage health, or permission to mutate a storage system. Content-free Pulse Intelligence telemetry rollups under shared `internal/api/` are also adjacent-only. Storage and recovery may consume diff --git a/frontend-modern/public/docs/OIDC.md b/frontend-modern/public/docs/OIDC.md index 75d89c286..acd9b6342 100644 --- a/frontend-modern/public/docs/OIDC.md +++ b/frontend-modern/public/docs/OIDC.md @@ -34,6 +34,14 @@ Restrict access to specific users or groups: - **Allowed Domains**: Restrict to specific email domains (e.g., `example.com`). - **Allowed Emails**: Allow specific email addresses. +> **Administrator requirement**: SSO authentication does not grant instance +> administrator privileges by itself. Before removing the configured local +> administrator or relying on SSO-only access, map a trusted IdP group to the +> built-in `admin` role. Keep that administrator group in **Allowed Groups** so +> the login and authorization boundaries describe the same trusted population. +> An empty **Allowed Groups** list allows every IdP user to sign in, but does not +> make those users administrators. + ### Group-to-Role Mapping Automatically assign Pulse roles based on OIDC group membership. When a user logs in, Pulse checks their groups claim and assigns the corresponding roles. Mapping groups to the built-in `admin`, `operator`, and `viewer` roles is included with Community SSO. Creating custom roles and manually managing user assignments remain Pro RBAC features. @@ -62,6 +70,9 @@ See [CONFIGURATION.md](CONFIGURATION.md). - Multiple groups can map to multiple roles (user gets all matching roles). - Role assignments are updated on every login to reflect current group membership. - Role changes are logged to the audit log for compliance tracking. +- Instance-administration routes require the built-in `admin` role or another + role with an explicit `admin` grant on all resources. The `operator` and + `viewer` mappings never inherit administrator access on SSO-only instances. **Example:** If a user has groups `["oidc-admins", "developers"]` and you have mappings: diff --git a/frontend-modern/public/docs/RBAC.md b/frontend-modern/public/docs/RBAC.md index 57f680dec..3162eaa43 100644 --- a/frontend-modern/public/docs/RBAC.md +++ b/frontend-modern/public/docs/RBAC.md @@ -27,6 +27,12 @@ Users can hold multiple roles. Their effective permissions are combined across a When using OIDC/SSO, built-in roles can be automatically assigned based on group membership on every plan. See [OIDC Group-to-Role Mapping](OIDC.md#group-to-role-mapping) for configuration. Creating custom roles and manually managing user assignments require Pro RBAC. +SSO authentication is not an administrator grant. An SSO-only deployment must +map at least one trusted IdP group to the built-in `admin` role (or retain a +configured local administrator) before operators can use instance-administration +routes. Mapping a user to `operator`, `viewer`, or no role never elevates that +user merely because no local administrator is configured. + --- ## Quick Start diff --git a/frontend-modern/public/docs/UPGRADE_v6.md b/frontend-modern/public/docs/UPGRADE_v6.md index 9d9390a7a..c60a2acab 100644 --- a/frontend-modern/public/docs/UPGRADE_v6.md +++ b/frontend-modern/public/docs/UPGRADE_v6.md @@ -127,6 +127,7 @@ stable. Keep v5.1.35 as the explicit rollback target for the v6.0.0 cutover. - Create an encrypted config backup: **Settings → System → Recovery → Create Backup** (older versions labeled this **Backups**) - Open **Settings → System → Updates** and review the upgrade checks on the update plan. Pulse checks the server update path, current agent continuity, and agent reporting token scope before you install. These checks describe the currently reported fleet; they do not prove every installed agent is online or already updated. - Confirm you can access the host/container console (for rollback and bootstrap token retrieval) +- If the instance uses SSO without a configured local administrator, map a trusted IdP group to the built-in `admin` role before upgrading. SSO authentication alone does not grant instance-administrator access; an unmapped, `operator`, or `viewer` SSO session cannot manage settings, transfer configuration, or control infrastructure actions. - If you have any external integrations or scripts: review the **API Changes** section below ## Upgrade Paths diff --git a/frontend-modern/src/utils/__tests__/docsLinks.test.ts b/frontend-modern/src/utils/__tests__/docsLinks.test.ts index 76dbe0b82..86fcebc19 100644 --- a/frontend-modern/src/utils/__tests__/docsLinks.test.ts +++ b/frontend-modern/src/utils/__tests__/docsLinks.test.ts @@ -172,6 +172,13 @@ describe('docsLinks', () => { expect(compactGuide).toContain( 'Creating custom roles and manually managing user assignments require Pro RBAC', ); + expect(compactGuide).toContain('SSO authentication is not an administrator grant'); + expect(compactGuide).toContain( + 'map at least one trusted IdP group to the built-in `admin` role', + ); + expect(compactGuide).toContain( + 'never elevates that user merely because no local administrator is configured', + ); }); it('ships the configuration transfer authorization contract', () => { diff --git a/internal/api/action_authority_test.go b/internal/api/action_authority_test.go index c9be47592..2ab572c33 100644 --- a/internal/api/action_authority_test.go +++ b/internal/api/action_authority_test.go @@ -5,6 +5,7 @@ import ( "errors" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -112,6 +113,60 @@ func TestExplicitActionAuthorizerCanGrantNonAdminBrowserSession(t *testing.T) { } } +func TestActionRoutesRequireExplicitSSOAdminOnSSOOnlyInstance(t *testing.T) { + previousAuthorizer := auth.GetAuthorizer() + auth.SetAuthorizer(&auth.DefaultAuthorizer{}) + t.Cleanup(func() { auth.SetAuthorizer(previousAuthorizer) }) + + router := newConfigTransferTestRouter(t, false, enabledConfigTransferSSO(config.SSOProviderTypeOIDC)) + manager := installTestRBACManager(t) + const ssoAdmin = "sso:action-admin@example.invalid" + if err := manager.UpdateUserRoles(ssoAdmin, []string{auth.RoleAdmin}); err != nil { + t.Fatalf("assign SSO administrator role: %v", err) + } + + tests := []struct { + name string + path string + body string + }{ + {name: "plan", path: "/api/actions/plan", body: `{}`}, + {name: "decision", path: "/api/actions/missing/decision", body: `{"outcome":"approved"}`}, + {name: "execute", path: "/api/actions/missing/execute", body: `{}`}, + } + + for _, user := range []struct { + name string + principal string + wantDenied bool + }{ + {name: "unassigned viewer", principal: "sso:action-viewer@example.invalid", wantDenied: true}, + {name: "explicit administrator", principal: ssoAdmin, wantDenied: false}, + } { + t.Run(user.name, func(t *testing.T) { + sessionToken := "action-route-" + strings.ReplaceAll(user.name, " ", "-") + GetSessionStore().CreateSession(sessionToken, time.Hour, "browser", "127.0.0.1", user.principal) + csrf := generateCSRFToken(sessionToken) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, tc.path, strings.NewReader(tc.body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-CSRF-Token", csrf) + req.AddCookie(&http.Cookie{Name: cookieNameSession, Value: sessionToken}) + rec := httptest.NewRecorder() + router.Handler().ServeHTTP(rec, req) + if user.wantDenied && rec.Code != http.StatusForbidden { + t.Fatalf("status=%d, want 403: %s", rec.Code, rec.Body.String()) + } + if !user.wantDenied && rec.Code == http.StatusForbidden { + t.Fatalf("explicit SSO administrator was denied: %s", rec.Body.String()) + } + }) + } + }) + } +} + func TestActionAuthorityAllowsOwnerBoundTokenWithCanonicalApproveAndExecuteScopes(t *testing.T) { authority := testActionAuthority() record := ownerBoundActionToken("owned", "alice", config.ScopeActionsApprove, config.ScopeActionsExecute) diff --git a/internal/api/cloud_org_admin_auth.go b/internal/api/cloud_org_admin_auth.go index 3fd9a8c41..11fa64d51 100644 --- a/internal/api/cloud_org_admin_auth.go +++ b/internal/api/cloud_org_admin_auth.go @@ -16,13 +16,11 @@ import ( // - Dev bypass // // Session/OIDC are allowed only for an instance administrator, which is the -// configured platform admin user, a holder of an RBAC admin grant, or an SSO -// principal on an instance that configures no local admin at all. That is the -// same rule canAccessPlatformAdminSurface uses to publish the billingAdmin -// capability, so the surface the UI offers and the routes behind it agree. -// An org-scoped tenant session is never a platform admin whatever its username, -// which is what keeps a tenant on a control plane with no local admin from -// inheriting the OIDC-only fallback. +// configured platform admin user or a holder of an explicit RBAC admin grant. +// That is the same rule canAccessPlatformAdminSurface uses to publish the +// billingAdmin capability, so the surface the UI offers and the routes behind +// it agree. An org-scoped tenant session is never a platform admin whatever its +// username. // API tokens are denied to prevent tenant users from invoking hosted // control-plane operations with bearer credentials. func RequirePlatformAdmin(cfg *config.Config, handler http.HandlerFunc) http.HandlerFunc { diff --git a/internal/api/config_transfer_authorization_test.go b/internal/api/config_transfer_authorization_test.go index 66f34f5be..d81dba15b 100644 --- a/internal/api/config_transfer_authorization_test.go +++ b/internal/api/config_transfer_authorization_test.go @@ -235,8 +235,13 @@ func TestConfigTransferAuthorizedInstanceModesReachHandler(t *testing.T) { for _, providerType := range []config.SSOProviderType{config.SSOProviderTypeOIDC, config.SSOProviderTypeSAML} { t.Run("SSO session "+string(providerType), func(t *testing.T) { router := newConfigTransferTestRouter(t, false, enabledConfigTransferSSO(providerType)) + manager := installTestRBACManager(t) + const ssoAdmin = "sso:owner@example.invalid" + if err := manager.UpdateUserRoles(ssoAdmin, []string{internalauth.RoleAdmin}); err != nil { + t.Fatalf("assign SSO administrator role: %v", err) + } sessionToken := "synthetic-sso-session-" + string(providerType) - GetSessionStore().CreateSession(sessionToken, time.Hour, "browser", "127.0.0.1", "sso:owner@example.invalid") + GetSessionStore().CreateSession(sessionToken, time.Hour, "browser", "127.0.0.1", ssoAdmin) csrf := generateCSRFToken(sessionToken) for _, path := range []string{"/api/config/export", "/api/config/import"} { body := &countingConfigTransferBody{reader: strings.NewReader(`{not-json`)} @@ -337,6 +342,33 @@ func TestConfigTransferTokenScopesAndOrganizationBinding(t *testing.T) { } } +func TestConfigTransferSSOViewerDeniedBeforeBodyRead(t *testing.T) { + for _, providerType := range []config.SSOProviderType{config.SSOProviderTypeOIDC, config.SSOProviderTypeSAML} { + t.Run(string(providerType), func(t *testing.T) { + router := newConfigTransferTestRouter(t, false, enabledConfigTransferSSO(providerType)) + installTestRBACManager(t) + sessionToken := "synthetic-sso-viewer-session-" + string(providerType) + GetSessionStore().CreateSession(sessionToken, time.Hour, "browser", "127.0.0.1", "sso:viewer@example.invalid") + csrf := generateCSRFToken(sessionToken) + + for _, path := range []string{"/api/config/export", "/api/config/import"} { + body := &countingConfigTransferBody{reader: strings.NewReader(`{not-json`)} + req := httptest.NewRequest(http.MethodPost, path, body) + req.AddCookie(&http.Cookie{Name: cookieNameSession, Value: sessionToken}) + req.Header.Set("X-CSRF-Token", csrf) + rec := httptest.NewRecorder() + router.Handler().ServeHTTP(rec, req) + if rec.Code != http.StatusForbidden { + t.Errorf("%s SSO viewer = status %d, want 403: %s", path, rec.Code, rec.Body.String()) + } + if body.reads != 0 { + t.Errorf("%s SSO viewer denial read body %d times", path, body.reads) + } + } + }) + } +} + func TestConfigTransferTenantSessionsRequireManagement(t *testing.T) { defer SetMultiTenantEnabled(false) SetMultiTenantEnabled(true) @@ -445,8 +477,13 @@ func TestSecurityStatusMatchesConfigTransferPolicy(t *testing.T) { t.Run("export override is ineffective with SSO", func(t *testing.T) { t.Setenv("ALLOW_UNPROTECTED_EXPORT", "true") router := newConfigTransferTestRouter(t, false, enabledConfigTransferSSO(config.SSOProviderTypeOIDC)) + manager := installTestRBACManager(t) + const ssoAdmin = "sso:owner@example.invalid" + if err := manager.UpdateUserRoles(ssoAdmin, []string{internalauth.RoleAdmin}); err != nil { + t.Fatalf("assign SSO administrator role: %v", err) + } sessionToken := "synthetic-security-status-session" - GetSessionStore().CreateSession(sessionToken, time.Hour, "browser", "127.0.0.1", "sso:owner@example.invalid") + GetSessionStore().CreateSession(sessionToken, time.Hour, "browser", "127.0.0.1", ssoAdmin) req := httptest.NewRequest(http.MethodGet, "/api/security/status", nil) req.AddCookie(&http.Cookie{Name: cookieNameSession, Value: sessionToken}) rec := httptest.NewRecorder() diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 8a2f9e574..14cafe24c 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -229,6 +229,47 @@ func TestContract_ConfigTransferRoutesUseCanonicalPreHandlerAuthorization(t *tes } } +func TestContract_SSOAuthenticationNeverImpliesInstanceAdmin(t *testing.T) { + adminSource, err := os.ReadFile(filepath.Clean("security_setup_fix.go")) + if err != nil { + t.Fatalf("read canonical session administrator authority: %v", err) + } + adminAuthority := string(adminSource) + for _, invariant := range []string{ + "if sessionUserHasRBACAdminGrant(sessionUser)", + "return false", + } { + if !strings.Contains(adminAuthority, invariant) { + t.Fatalf("canonical session administrator authority missing %q", invariant) + } + } + if strings.Contains(adminAuthority, `strings.HasPrefix(sessionUser, "sso:")`) { + t.Fatal("SSO identity prefix must not imply instance administrator authority") + } + + samlSource, err := os.ReadFile(filepath.Clean("saml_handlers.go")) + if err != nil { + t.Fatalf("read SAML authentication authority: %v", err) + } + samlAuthority := string(samlSource) + for _, invariant := range []string{ + "if len(provider.AllowedDomains) > 0 {", + "if len(provider.AllowedEmails) > 0 {", + } { + if !strings.Contains(samlAuthority, invariant) { + t.Fatalf("SAML authentication authority missing fail-closed allowlist check %q", invariant) + } + } + for _, forbidden := range []string{ + `len(provider.AllowedDomains) > 0 && result.Email != ""`, + `len(provider.AllowedEmails) > 0 && result.Email != ""`, + } { + if strings.Contains(samlAuthority, forbidden) { + t.Fatalf("SAML allowlist must not be bypassed when the email claim is absent: found %q", forbidden) + } + } +} + type basicActionContractAuthorizer struct { wantUser string } diff --git a/internal/api/discovery_handlers.go b/internal/api/discovery_handlers.go index 06da80430..54dff9e5c 100644 --- a/internal/api/discovery_handlers.go +++ b/internal/api/discovery_handlers.go @@ -204,9 +204,9 @@ func (h *DiscoveryHandlers) isAdminRequest(r *http.Request) bool { // 3. Check for an admin session (OIDC/SAML/local session). The admin test is // sessionUserCarriesAdminPrivileges, the same one the settings routes apply, - // so an RBAC admin grant and the SSO-principal-with-no-local-admin case both - // count. Comparing against h.config.AuthUser alone cannot match on an - // instance whose only administrators are SSO principals. + // so both the configured local administrator and an explicit RBAC admin + // grant count. Comparing against h.config.AuthUser alone cannot recognize an + // explicitly authorized SSO administrator. if cookie, err := readSessionCookie(r); err == nil && cookie.Value != "" { if ValidateSession(cookie.Value) { sessionUser := strings.TrimSpace(GetSessionUsername(cookie.Value)) diff --git a/internal/api/ensure_admin_session_sso_test.go b/internal/api/ensure_admin_session_sso_test.go index e551c3f62..d75795906 100644 --- a/internal/api/ensure_admin_session_sso_test.go +++ b/internal/api/ensure_admin_session_sso_test.go @@ -10,6 +10,18 @@ import ( internalauth "github.com/rcourtman/pulse-go-rewrite/pkg/auth" ) +func installTestRBACManager(t *testing.T) internalauth.Manager { + t.Helper() + manager, err := internalauth.NewFileManager(t.TempDir()) + if err != nil { + t.Fatalf("new file manager: %v", err) + } + original := internalauth.GetManager() + internalauth.SetManager(manager) + t.Cleanup(func() { internalauth.SetManager(original) }) + return manager +} + // The privileged-session gate only recognised cfg.AuthUser, so SSO users — // keyed by their provider-scoped principal — could never reach settings-scoped // routes, even with an admin group role mapping (#1533, #1535). @@ -17,13 +29,7 @@ func TestEnsureAdminSessionSSOUsers(t *testing.T) { dir := t.TempDir() InitSessionStore(dir) - manager, err := internalauth.NewFileManager(t.TempDir()) - if err != nil { - t.Fatalf("new file manager: %v", err) - } - origManager := internalauth.GetManager() - internalauth.SetManager(manager) - t.Cleanup(func() { internalauth.SetManager(origManager) }) + manager := installTestRBACManager(t) newSessionRequest := func(t *testing.T, username string) *http.Request { t.Helper() @@ -57,7 +63,8 @@ func TestEnsureAdminSessionSSOUsers(t *testing.T) { {name: "sso user with admin role passes", cfg: cfgWithAdmin, user: ssoAdmin, want: true}, {name: "sso user without roles is rejected when local admin exists", cfg: cfgWithAdmin, user: ssoPlain, want: false}, {name: "local non-admin user is rejected", cfg: cfgWithAdmin, user: "mallory", want: false}, - {name: "sso user passes on an instance with no local admin", cfg: cfgNoAdmin, user: ssoPlain, want: true}, + {name: "sso admin role passes on an instance with no local admin", cfg: cfgNoAdmin, user: ssoAdmin, want: true}, + {name: "sso user without roles is rejected when no local admin exists", cfg: cfgNoAdmin, user: ssoPlain, want: false}, {name: "non-sso session is rejected when no local admin configured", cfg: cfgNoAdmin, user: "mallory", want: false}, } @@ -77,13 +84,7 @@ func TestEnsureAdminSessionSSOUsers(t *testing.T) { } func TestSessionUserHasRBACAdminGrant(t *testing.T) { - manager, err := internalauth.NewFileManager(t.TempDir()) - if err != nil { - t.Fatalf("new file manager: %v", err) - } - origManager := internalauth.GetManager() - internalauth.SetManager(manager) - t.Cleanup(func() { internalauth.SetManager(origManager) }) + manager := installTestRBACManager(t) if err := manager.UpdateUserRoles("adm", []string{internalauth.RoleAdmin}); err != nil { t.Fatalf("assign admin: %v", err) diff --git a/internal/api/oidc_only_admin_parity_test.go b/internal/api/oidc_only_admin_parity_test.go index efc6a9feb..2e5425fe9 100644 --- a/internal/api/oidc_only_admin_parity_test.go +++ b/internal/api/oidc_only_admin_parity_test.go @@ -54,19 +54,21 @@ func exportStatusFor(t *testing.T, router *Router, user string) int { return rec.Code } -// On the OIDC-only pattern there is no local admin, so SSO principals are the -// instance's administrators and ensureAdminSession admits them. Several guards -// compared the session user against cfg.AuthUser instead, which is empty here, -// so they could admit nobody at all and locked the operator out of discovery, -// public URL capture and their own config export. -func TestOIDCOnlyAdminReachesEveryAdminGuard(t *testing.T) { +// SSO-only deployments retain a usable administration path through an +// explicit RBAC administrator grant. Authentication without that grant is not +// instance administration. +func TestSSOOnlyExplicitAdminReachesEveryAdminGuard(t *testing.T) { prev := auth.GetAuthorizer() auth.SetAuthorizer(&auth.DefaultAuthorizer{}) defer auth.SetAuthorizer(prev) cfg := adminParityConfig(t, "") router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") + manager := installTestRBACManager(t) ssoAdmin := "sso:owner@example.com" + if err := manager.UpdateUserRoles(ssoAdmin, []string{auth.RoleAdmin}); err != nil { + t.Fatalf("assign SSO administrator role: %v", err) + } if !sessionUserCarriesAdminPrivileges(cfg, ssoAdmin) { t.Fatal("precondition: the canonical helper must treat this principal as an admin") @@ -75,18 +77,36 @@ func TestOIDCOnlyAdminReachesEveryAdminGuard(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/api/health", nil) req.AddCookie(adminParitySession(t, ssoAdmin)) if !canCapturePublicURL(cfg, req) { - t.Error("canCapturePublicURL refused the OIDC-only administrator") + t.Error("canCapturePublicURL refused the explicit SSO administrator") } h := &DiscoveryHandlers{config: cfg} req2 := httptest.NewRequest(http.MethodGet, "/api/discovery/status", nil) req2.AddCookie(adminParitySession(t, ssoAdmin)) if !h.isAdminRequest(req2) { - t.Error("discovery isAdminRequest refused the OIDC-only administrator") + t.Error("discovery isAdminRequest refused the explicit SSO administrator") } if code := exportStatusFor(t, router, ssoAdmin); code == http.StatusForbidden { - t.Error("config export refused the OIDC-only administrator") + t.Error("config export refused the explicitly authorized SSO administrator") + } + + ssoViewer := "sso:viewer@example.com" + if sessionUserCarriesAdminPrivileges(cfg, ssoViewer) { + t.Fatal("unassigned SSO user inherited instance administration") + } + req3 := httptest.NewRequest(http.MethodGet, "/api/health", nil) + req3.AddCookie(adminParitySession(t, ssoViewer)) + if canCapturePublicURL(cfg, req3) { + t.Error("public URL capture admitted an unassigned SSO user") + } + req4 := httptest.NewRequest(http.MethodGet, "/api/discovery/status", nil) + req4.AddCookie(adminParitySession(t, ssoViewer)) + if h.isAdminRequest(req4) { + t.Error("discovery admitted an unassigned SSO user") + } + if code := exportStatusFor(t, router, ssoViewer); code != http.StatusForbidden { + t.Errorf("config export for an unassigned SSO user = %d, want 403", code) } } diff --git a/internal/api/platform_admin_session_parity_test.go b/internal/api/platform_admin_session_parity_test.go index 679139ea5..a0535bb44 100644 --- a/internal/api/platform_admin_session_parity_test.go +++ b/internal/api/platform_admin_session_parity_test.go @@ -54,11 +54,8 @@ func billingAdminCapability(t *testing.T, router *Router, user string) bool { return caps["billingAdmin"] == true } -// The billingAdmin capability is published from canAccessPlatformAdminSurface, -// which treats any instance administrator as a platform admin. The route behind -// it compared the session user against cfg.AuthUser alone, so on an instance -// whose only administrators are SSO principals the UI offered the surface and -// the route refused it. +// The billingAdmin capability and its route share the explicit RBAC +// administrator boundary on an SSO-only instance. func TestPlatformAdminRouteAgreesWithBillingAdminCapability(t *testing.T) { prev := auth.GetAuthorizer() auth.SetAuthorizer(&auth.DefaultAuthorizer{}) @@ -66,10 +63,14 @@ func TestPlatformAdminRouteAgreesWithBillingAdminCapability(t *testing.T) { cfg := platformAdminConfig(t, "") router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") + manager := installTestRBACManager(t) ssoOwner := "sso:owner@example.com" + if err := manager.UpdateUserRoles(ssoOwner, []string{auth.RoleAdmin}); err != nil { + t.Fatalf("assign SSO administrator role: %v", err) + } if !billingAdminCapability(t, router, ssoOwner) { - t.Fatal("precondition: billingAdmin must be advertised to the OIDC-only administrator") + t.Fatal("precondition: billingAdmin must be advertised to the explicit SSO administrator") } req := httptest.NewRequest(http.MethodGet, "/api/hosted/organizations", nil) @@ -81,10 +82,8 @@ func TestPlatformAdminRouteAgreesWithBillingAdminCapability(t *testing.T) { } } -// An org-scoped tenant session is not an instance administrator, whatever its -// username. Without this the SSO fallback in sessionUserCarriesAdminPrivileges -// would make every tenant on a control plane with no local admin a platform -// admin, which is a far worse failure than the one being fixed. +// An org-scoped tenant session is not an instance administrator, even when the +// same principal has a global RBAC administrator grant. func TestPlatformAdminRouteRefusesOrgScopedTenantSession(t *testing.T) { prev := auth.GetAuthorizer() auth.SetAuthorizer(&auth.DefaultAuthorizer{}) @@ -92,6 +91,10 @@ func TestPlatformAdminRouteRefusesOrgScopedTenantSession(t *testing.T) { cfg := platformAdminConfig(t, "") tenant := "sso:tenant@example.com" + manager := installTestRBACManager(t) + if err := manager.UpdateUserRoles(tenant, []string{auth.RoleAdmin}); err != nil { + t.Fatalf("assign SSO administrator role: %v", err) + } handlerReached := false guarded := RequirePlatformAdmin(cfg, func(w http.ResponseWriter, r *http.Request) { @@ -112,7 +115,8 @@ func TestPlatformAdminRouteRefusesOrgScopedTenantSession(t *testing.T) { t.Fatalf("org-scoped tenant session reached the platform admin surface (code %d)", rec.Code) } - // The same principal without an org binding is the instance administrator. + // The same explicitly authorized principal without an org binding is the + // instance administrator. req2 := httptest.NewRequest(http.MethodGet, "/api/hosted/organizations", nil) req2.AddCookie(platformAdminSession(t, tenant)) rec2 := httptest.NewRecorder() diff --git a/internal/api/saml_handlers.go b/internal/api/saml_handlers.go index 56d1ab359..f4706ec4a 100644 --- a/internal/api/saml_handlers.go +++ b/internal/api/saml_handlers.go @@ -231,7 +231,7 @@ func (r *Router) handleSAMLACS(w http.ResponseWriter, req *http.Request) { } // Check domain restrictions - if len(provider.AllowedDomains) > 0 && result.Email != "" { + if len(provider.AllowedDomains) > 0 { if !matchesDomain(result.Email, provider.AllowedDomains) { log.Debug(). Str("email", result.Email). @@ -244,7 +244,7 @@ func (r *Router) handleSAMLACS(w http.ResponseWriter, req *http.Request) { } // Check email restrictions - if len(provider.AllowedEmails) > 0 && result.Email != "" { + if len(provider.AllowedEmails) > 0 { if !matchesValue(result.Email, provider.AllowedEmails) { log.Debug(). Str("email", result.Email). diff --git a/internal/api/security_setup_fix.go b/internal/api/security_setup_fix.go index f17c4226d..065e1c300 100644 --- a/internal/api/security_setup_fix.go +++ b/internal/api/security_setup_fix.go @@ -179,9 +179,7 @@ func ensureAdminSession(cfg *config.Config, w http.ResponseWriter, req *http.Req // sessionIsOrgScoped reports whether the request is bound to a tenant // organization rather than the instance itself. Instance-admin rules must not // be applied to an org-scoped session: those callers are governed by their -// organization's own management rules, and on a control plane configuring no -// local admin they would otherwise inherit the SSO fallback in -// sessionUserCarriesAdminPrivileges and each become an instance administrator. +// organization's own management rules rather than global RBAC assignments. func sessionIsOrgScoped(req *http.Request) bool { if req == nil { return false @@ -196,10 +194,10 @@ func sessionIsOrgScoped(req *http.Request) bool { // sessionUserCarriesAdminPrivileges reports whether a non-org-scoped session // username carries instance admin privileges: the configured local admin -// identity, an RBAC assignment granting the admin action (how SSO group role -// mappings make an SSO user an admin, #1533/#1535), or any SSO principal when -// the instance has no local admin configured at all — the v5 OIDC-only -// pattern, where SSO sessions are the only administrators the instance has. +// identity or an RBAC assignment granting the admin action on all resources. +// SSO authentication alone never grants instance administration; SSO-only +// deployments must map a trusted identity-provider group to an administrator +// role or retain a configured local administrator. func sessionUserCarriesAdminPrivileges(cfg *config.Config, sessionUser string) bool { sessionUser = strings.TrimSpace(sessionUser) if sessionUser == "" { @@ -215,7 +213,7 @@ func sessionUserCarriesAdminPrivileges(cfg *config.Config, sessionUser string) b if sessionUserHasRBACAdminGrant(sessionUser) { return true } - return configuredAdmin == "" && strings.HasPrefix(sessionUser, "sso:") + return false } // sessionUserHasRBACAdminGrant reports whether the user's effective RBAC diff --git a/internal/api/security_status_capabilities.go b/internal/api/security_status_capabilities.go index c633ef6d1..04e207292 100644 --- a/internal/api/security_status_capabilities.go +++ b/internal/api/security_status_capabilities.go @@ -186,9 +186,8 @@ func (r *Router) buildSecurityStatusAuthSnapshot(req *http.Request) securityStat username := strings.TrimSpace(GetSessionUsername(cookie.Value)) snapshotReq := attachUserContext(req, username) // Same privilege rule as ensureAdminSession: the configured admin - // identity, an RBAC admin grant (SSO group role mappings), or an SSO - // session on an instance with no local admin. Org-scoped sessions keep - // their own management rules. + // identity or an explicit RBAC admin grant (including SSO group role + // mappings). Org-scoped sessions keep their own management rules. sessionIsAdmin := false if !sessionIsOrgScoped(req) { sessionIsAdmin = sessionUserCarriesAdminPrivileges(r.config, username) diff --git a/internal/api/security_status_capability_enforcement_test.go b/internal/api/security_status_capability_enforcement_test.go index ebe10904f..3bb9871d3 100644 --- a/internal/api/security_status_capability_enforcement_test.go +++ b/internal/api/security_status_capability_enforcement_test.go @@ -166,26 +166,44 @@ func TestSettingsCapabilitiesGrantConfiguredAdminWithoutRBAC(t *testing.T) { } } -// On the OIDC-only pattern there is no local admin, so SSO principals are the -// instance's administrators. They must keep the tabs, and the routes agree. -func TestSettingsCapabilitiesGrantSSOAdminWhenNoLocalAdminConfigured(t *testing.T) { +// An SSO-only deployment advertises admin settings only to a principal with +// an explicit RBAC administrator grant. +func TestSettingsCapabilitiesRequireExplicitSSOAdminWhenNoLocalAdminConfigured(t *testing.T) { prevAuthorizer := auth.GetAuthorizer() auth.SetAuthorizer(&auth.DefaultAuthorizer{}) defer auth.SetAuthorizer(prevAuthorizer) cfg := newCapabilityConfig(t, "") router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") + manager := installTestRBACManager(t) + const ssoAdmin = "sso:owner@example.com" + if err := manager.UpdateUserRoles(ssoAdmin, []string{auth.RoleAdmin}); err != nil { + t.Fatalf("assign SSO administrator role: %v", err) + } - caps := fetchSettingsCapabilities(t, router, capabilitySessionCookie(t, "sso:owner@example.com")) + caps := fetchSettingsCapabilities(t, router, capabilitySessionCookie(t, ssoAdmin)) if caps["apiAccessRead"] != true { - t.Fatalf("apiAccessRead = %v on an OIDC-only instance, want true", caps["apiAccessRead"]) + t.Fatalf("apiAccessRead = %v for explicit SSO administrator, want true", caps["apiAccessRead"]) } req := httptest.NewRequest(http.MethodGet, "/api/security/tokens", nil) - req.AddCookie(capabilitySessionCookie(t, "sso:owner@example.com")) + req.AddCookie(capabilitySessionCookie(t, ssoAdmin)) rec := httptest.NewRecorder() router.Handler().ServeHTTP(rec, req) if rec.Code == http.StatusForbidden { - t.Fatal("OIDC-only instance must not refuse its own SSO administrator") + t.Fatal("SSO-only instance refused its explicit SSO administrator") + } + + const ssoViewer = "sso:viewer@example.com" + viewerCaps := fetchSettingsCapabilities(t, router, capabilitySessionCookie(t, ssoViewer)) + if viewerCaps["apiAccessRead"] == true { + t.Fatal("unassigned SSO user was advertised API access administration") + } + viewerReq := httptest.NewRequest(http.MethodGet, "/api/security/tokens", nil) + viewerReq.AddCookie(capabilitySessionCookie(t, ssoViewer)) + viewerRec := httptest.NewRecorder() + router.Handler().ServeHTTP(viewerRec, viewerReq) + if viewerRec.Code != http.StatusForbidden { + t.Fatalf("unassigned SSO user status = %d, want 403", viewerRec.Code) } }