From 4d475043319dd04cf1461683aad9b8dfba6dbfb8 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Wed, 23 Sep 2026 22:11:03 +0100 Subject: [PATCH] Offer org owners the settings their own org's routes serve A provider opening a client workspace from the Pulse Account portal arrives in that client's organization as its owner. ensureAdminSession admits an org manager to the org-bound settings routes, but the security status snapshot reported every settings capability as false for any org-scoped session. Since the Settings navigation started hiding gated panels in August, the owner saw only General: no Infrastructure page to connect the client's Proxmox or install agents, and no Reporting page for the client reports. The portal's handoff to /settings/infrastructure fell through to General. Found on a fresh v6.4.1 provider bundle, where /api/config/nodes answered 200 for the same session that /api/security/status said could reach nothing. Route and snapshot now share one settings-admin rule, sessionUserHasSettingsAdminPrivileges. The snapshot keeps it out of sessionIsAdmin: it widens only infrastructureRead, availabilityRead and reportingRead, whose handlers read the organization's own config and monitor state. Instance administration, detailLevel, authUsername and billingAdmin stay withheld from org-scoped sessions, as the scoped local session contract requires. --- .../v6/internal/subsystems/agent-lifecycle.md | 12 ++ .../v6/internal/subsystems/api-contracts.md | 19 +++ .../internal/subsystems/security-privacy.md | 2 +- .../internal/subsystems/storage-recovery.md | 11 ++ internal/api/contract_test.go | 110 ++++++++++++++++++ internal/api/security_setup_fix.go | 73 +++++++----- internal/api/security_status_capabilities.go | 41 +++++-- 7 files changed, 227 insertions(+), 41 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 40bf3815e5..7f4167c8a8 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -3410,6 +3410,18 @@ Agent` secondary handoff against the live setup wizard instead of relying ## Current State +### Org managers reach the agent install path in their own organization + +An owner or admin of the selected organization now receives +`settingsCapabilities.infrastructureRead` from `/api/security/status`, matching +the `ensureAdminSession` rule that already let them through the Infrastructure +routes. A provider opening a client workspace from the Pulse Account portal +therefore lands on Settings → Infrastructure with the host install flow open, +instead of a navigation that hid the page and fell through to General. The +install tokens it generates stay bound to that organization; nothing about token +scope, agent registration or reporting identity changes. See the api-contracts +contract, rule 35, for the capability rule. + ### Auto-register preserves the operator's stored TLS choice Re-registering an existing Proxmox node through the canonical auto-register diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index e3f8ae9332..0e88c08791 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -4533,6 +4533,25 @@ auto-register mutation boundary. `TestConfigTransferSSOViewerDeniedBeforeBodyRead` and `TestActionRoutesRequireExplicitSSOAdminOnSSOOnlyInstance`. + Org-bound settings are the one place an org-scoped session is not + withheld. `ensureAdminSession` admits the configured administrator or a + manager (owner or admin) of the request's organization inside an + org-scoped request, and that rule is `sessionUserHasSettingsAdminPrivileges` + for both the route gate and the snapshot. The snapshot records it as + `sessionManagesOrg`, never as `sessionIsAdmin`, and it widens only + `infrastructureRead`, `availabilityRead` and `reportingRead`, whose routes + pass `ensureAdminSession` and whose handlers read that organization's own + config and monitor state; `reportingRead` still requires the authorizer's + `read` on `nodes`. Every other capability, `detailLevel` and `authUsername` + stay instance administration. 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 + that the routes would have served and the handoff to + `/settings/infrastructure` fell through to General. Regression coverage: + `TestSecurityStatusOrgBoundSettingsFollowOrgManagementRule` and + `TestSecurityStatusOrgBoundReportingRespectsAuthorizer` in + `internal/api/contract_test.go`. + 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 diff --git a/docs/release-control/v6/internal/subsystems/security-privacy.md b/docs/release-control/v6/internal/subsystems/security-privacy.md index 490ee0050a..1c7630560f 100644 --- a/docs/release-control/v6/internal/subsystems/security-privacy.md +++ b/docs/release-control/v6/internal/subsystems/security-privacy.md @@ -304,7 +304,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. +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 already admit org managers through ensureAdminSession and read only that organisation's state; this never extends to authentication, SSO, users, roles, audit, relay, system, diagnostics or billing surfaces. API token scope copy must match runtime authority. `ai:chat` covers Assistant conversation, model selection, sessions, and knowledge reads only. Knowledge diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 66774049f4..ed994ae27f 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -2618,6 +2618,17 @@ vdev layout is reported` in ## Current State +### Org managers see org-bound settings, not recovery administration + +`/api/security/status` now grants an owner or admin of the selected +organization the org-bound `infrastructureRead`, `availabilityRead` and +`reportingRead` capabilities, because those routes already admit org managers +and read only that organization's state. `systemSettingsRead`, which gates +System → Recovery with backup polling and configuration export/import, stays +withheld from every org-scoped session, so an org manager is not offered +instance recovery or configuration transfer. See the api-contracts contract, +rule 35. + ### TLS verification preference survives node re-registration A PVE or PBS node whose operator disabled certificate verification keeps that diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index d293e613c1..cf69ac9083 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -24809,6 +24809,116 @@ func TestSecurityStatusCurrentUserForScopedLocalSession(t *testing.T) { } } +func orgScopedSessionRequest(t *testing.T, user string, org *models.Organization) *http.Request { + t.Helper() + req := httptest.NewRequest(http.MethodGet, "/api/security/status", nil) + req.AddCookie(platformAdminSession(t, user)) + return req.WithContext(context.WithValue(req.Context(), OrgContextKey, org)) +} + +// A provider opening a client workspace lands in that client's organization as +// its owner. ensureAdminSession has always let an org manager through the +// org-bound settings routes, but the security status snapshot reported every +// settings capability as false for any org-scoped session, so the settings +// navigation hid Infrastructure and Reporting and the owner had no way to +// connect a single system. Those org-bound capabilities must follow the route +// rule, while instance administration stays closed to org-scoped sessions. +func TestSecurityStatusOrgBoundSettingsFollowOrgManagementRule(t *testing.T) { + prev := authpkg.GetAuthorizer() + authpkg.SetAuthorizer(&authpkg.DefaultAuthorizer{}) + defer authpkg.SetAuthorizer(prev) + + cfg := platformAdminConfig(t, "admin") + router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") + + org := &models.Organization{ + ID: "t-client01", + DisplayName: "Client Workspace", + OwnerUserID: "u_provider_owner", + Members: []models.OrganizationMember{ + {UserID: "u_provider_owner", Role: models.OrgRoleOwner}, + {UserID: "u_org_admin", Role: models.OrgRoleAdmin}, + {UserID: "u_org_viewer", Role: models.OrgRoleViewer}, + }, + } + + for _, tc := range []struct { + name string + user string + manager bool + }{ + {name: "owner", user: "u_provider_owner", manager: true}, + {name: "org admin", user: "u_org_admin", manager: true}, + {name: "viewer", user: "u_org_viewer", manager: false}, + {name: "outsider", user: "u_outsider", manager: false}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := ensureAdminSession(router.config, httptest.NewRecorder(), orgScopedSessionRequest(t, tc.user, org)); got != tc.manager { + t.Fatalf("precondition: ensureAdminSession = %v, want %v", got, tc.manager) + } + + caps := router.securityStatusSettingsCapabilitiesFromSnapshot( + router.buildSecurityStatusAuthSnapshot(orgScopedSessionRequest(t, tc.user, org))) + + orgBound := map[string]bool{ + "infrastructureRead": caps.InfrastructureRead, + "availabilityRead": caps.AvailabilityRead, + "reportingRead": caps.ReportingRead, + } + for name, got := range orgBound { + if got != tc.manager { + t.Errorf("%s = %v for an org-scoped %s; its route answers %v", name, got, tc.name, tc.manager) + } + } + + instanceWide := map[string]bool{ + "systemSettingsRead": caps.SystemSettingsRead, + "diagnosticsRead": caps.DiagnosticsRead, + "systemLogsRead": caps.SystemLogsRead, + "authenticationRead": caps.AuthenticationRead, + "authenticationWrite": caps.AuthenticationWrite, + "singleSignOnWrite": caps.SingleSignOnWrite, + "apiAccessWrite": caps.APIAccessWrite, + "users": caps.Users, + "roles": caps.Roles, + "relayWrite": caps.RelayWrite, + "billingAdmin": caps.BillingAdmin, + } + for name, got := range instanceWide { + if got { + t.Errorf("%s advertised to an org-scoped %s; instance administration stays closed to org-scoped sessions", name, tc.name) + } + } + }) + } +} + +// With a real RBAC authorizer, reporting also needs read:nodes. An org owner +// the authorizer refuses must not be offered the reporting surface, because +// RequirePermission refuses the route before ensureAdminSession runs. +func TestSecurityStatusOrgBoundReportingRespectsAuthorizer(t *testing.T) { + cfg := platformAdminConfig(t, "admin") + router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") + router.authorizer = orgBoundDenyAllAuthorizer{} + + org := &models.Organization{ID: "t-client03", DisplayName: "Client", OwnerUserID: "u_owner"} + caps := router.securityStatusSettingsCapabilitiesFromSnapshot( + router.buildSecurityStatusAuthSnapshot(orgScopedSessionRequest(t, "u_owner", org))) + + if caps.ReportingRead { + t.Fatal("reportingRead advertised although the authorizer refuses read:nodes") + } + if !caps.InfrastructureRead { + t.Fatal("infrastructureRead must still follow ensureAdminSession for the org owner") + } +} + +type orgBoundDenyAllAuthorizer struct{} + +func (orgBoundDenyAllAuthorizer) Authorize(context.Context, string, string) (bool, error) { + return false, nil +} + // Embedding the interface makes any unexpected mutation fail rather than // supplying no-op write methods to this read-only API contract fixture. type diagnosticContractManager struct{ alerting.NotificationManager } diff --git a/internal/api/security_setup_fix.go b/internal/api/security_setup_fix.go index 7f89b6d203..131b5cc4b2 100644 --- a/internal/api/security_setup_fix.go +++ b/internal/api/security_setup_fix.go @@ -138,44 +138,55 @@ func ensureAdminSession(cfg *config.Config, w http.ResponseWriter, req *http.Req // Session users must match configured admin identity for privileged operations. if cookie, err := readSessionCookie(req); err == nil && cookie.Value != "" && ValidateSession(cookie.Value) { sessionUser := strings.TrimSpace(GetSessionUsername(cookie.Value)) - configuredAdmin := "" - if cfg != nil { - configuredAdmin = strings.TrimSpace(cfg.AuthUser) - } - - if configuredAdmin != "" && strings.EqualFold(sessionUser, configuredAdmin) { + if sessionUserHasSettingsAdminPrivileges(cfg, req, sessionUser) { return true } - // Org-scoped tenant sessions preserve canonical org management - // privileges for settings-bound routes. - orgScoped := false - if org := GetOrganization(req.Context()); org != nil { - orgID := strings.TrimSpace(org.ID) - if orgID != "" && orgID != "default" { - orgScoped = true - if org.CanUserIDManage(sessionUser) { - return true - } - } - } - - if !orgScoped && sessionUserCarriesAdminPrivileges(cfg, sessionUser) { - return true - } - - if configuredAdmin == "" || !strings.EqualFold(sessionUser, configuredAdmin) { - log.Warn(). - Str("path", req.URL.Path). - Str("user", sessionUser). - Msg("Session user missing admin privileges for privileged operation") - http.Error(w, "Admin privileges required", http.StatusForbidden) - return false - } + log.Warn(). + Str("path", req.URL.Path). + Str("user", sessionUser). + Msg("Session user missing admin privileges for privileged operation") + http.Error(w, "Admin privileges required", http.StatusForbidden) + return false } return true } +// sessionUserHasSettingsAdminPrivileges is the session rule ensureAdminSession +// enforces for settings-bound routes: the configured local admin; on an +// org-scoped request, a user who can manage that organization; otherwise an +// instance administrator. The security status snapshot publishes settings +// capabilities from this same rule, so a surface the UI offers is one its +// routes accept. +// +// It is deliberately wider than the platform admin rule. An org owner +// administers their own organization's settings but is never an instance or +// platform administrator; RequirePlatformAdmin and the billingAdmin capability +// keep excluding org-scoped sessions. +func sessionUserHasSettingsAdminPrivileges(cfg *config.Config, req *http.Request, sessionUser string) bool { + sessionUser = strings.TrimSpace(sessionUser) + if sessionUser == "" { + return false + } + + configuredAdmin := "" + if cfg != nil { + configuredAdmin = strings.TrimSpace(cfg.AuthUser) + } + if configuredAdmin != "" && strings.EqualFold(sessionUser, configuredAdmin) { + return true + } + + // Org-scoped tenant sessions preserve canonical org management + // privileges for settings-bound routes. + if sessionIsOrgScoped(req) { + org := GetOrganization(req.Context()) + return org != nil && org.CanUserIDManage(sessionUser) + } + + return sessionUserCarriesAdminPrivileges(cfg, sessionUser) +} + // 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 diff --git a/internal/api/security_status_capabilities.go b/internal/api/security_status_capabilities.go index 04e207292c..5152cc71e8 100644 --- a/internal/api/security_status_capabilities.go +++ b/internal/api/security_status_capabilities.go @@ -78,7 +78,13 @@ type securityStatusAuthSnapshot struct { username string proxyIsAdmin bool sessionIsAdmin bool - tokenRecord *config.APITokenRecord + // sessionManagesOrg is set for an org-scoped session that + // ensureAdminSession admits: an owner or admin of the request's + // organization, or the configured admin. It widens only the org-bound + // settings surfaces, never sessionIsAdmin, so instance administration and + // the privileged status fields stay closed to org-scoped sessions. + sessionManagesOrg bool + tokenRecord *config.APITokenRecord } func (s securityStatusAuthSnapshot) tokenScopes() []string { @@ -189,15 +195,19 @@ func (r *Router) buildSecurityStatusAuthSnapshot(req *http.Request) securityStat // 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) { + sessionManagesOrg := false + if sessionIsOrgScoped(req) { + sessionManagesOrg = sessionUserHasSettingsAdminPrivileges(r.config, req, username) + } else { sessionIsAdmin = sessionUserCarriesAdminPrivileges(r.config, username) } return securityStatusAuthSnapshot{ - request: snapshotReq, - authenticated: true, - authMethod: "session", - username: username, - sessionIsAdmin: sessionIsAdmin, + request: snapshotReq, + authenticated: true, + authMethod: "session", + username: username, + sessionIsAdmin: sessionIsAdmin, + sessionManagesOrg: sessionManagesOrg, } } @@ -275,9 +285,22 @@ func (r *Router) securityStatusSettingsCapabilitiesFromSnapshot(snapshot securit canReadReporting := canReadSettings && r.canAccessPermissionSurface(snapshot, internalauth.ActionRead, internalauth.ResourceNodes, config.ScopeSettingsRead) + // Infrastructure, availability, and reporting are org-bound: their routes + // pass ensureAdminSession, which admits an org manager inside their own + // organization, and their handlers read that organization's config and + // monitor state. A provider opening a client workspace lands there as the + // org owner, and without these the settings navigation hides the only + // pages that connect the client's systems and build its reports. Every + // other capability stays instance administration. + canReadOrgSettings := canReadSettings || snapshot.sessionManagesOrg + if snapshot.sessionManagesOrg && !canReadReporting { + allowed, err := r.authorizer.Authorize(snapshot.request.Context(), internalauth.ActionRead, internalauth.ResourceNodes) + canReadReporting = err == nil && allowed + } + return securityStatusSettingsCapabilities{ - InfrastructureRead: canReadSettings, - AvailabilityRead: canReadSettings, + InfrastructureRead: canReadOrgSettings, + AvailabilityRead: canReadOrgSettings, PulseIntelligenceRead: canReadPulseIntelligence, DiagnosticsRead: canReadSettings, SystemLogsRead: canReadSettings,