From 3596acfeb2eacbd277da8acaef79b07f2c67a7b4 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 15 Apr 2026 13:57:40 +0100 Subject: [PATCH] Trim stale SAML rebinding coverage --- .../v6/internal/subsystems/agent-lifecycle.md | 1 - .../v6/internal/subsystems/api-contracts.md | 7 -- .../subsystems/performance-and-scalability.md | 5 -- .../internal/subsystems/storage-recovery.md | 1 - internal/api/contract_test.go | 68 ------------------- 5 files changed, 82 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index b5fb9a507..441de4a7b 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -195,7 +195,6 @@ an add-only capacity posture. 4. Keep shared `internal/api/` helper edits isolated from agent lifecycle semantics: Patrol-specific status transport or alert-trigger wiring changes in shared handlers must not bleed into auto-register, installer, or fleet-control behavior unless this contract moves in the same slice. The same isolation rule applies to AI settings payload work in `internal/api/ai_handlers.go`: provider auth fields, masked-secret echoes, and provider-test model selection remain AI/runtime plus API-contract ownership and must not be reinterpreted as lifecycle setup or registration semantics just because they share backend helper layers. The same shared-helper rule now covers SSO outbound discovery and metadata fetches plus credential-file loads in `internal/api/sso_outbound.go`, `internal/api/saml_service.go`, and `internal/api/oidc_service.go`: lifecycle-adjacent setup or auth work may depend on that shared trust boundary, but it must not fork a second HTTP client, redirect policy, or file-read rule inside lifecycle-local flows. - That same shared helper boundary also owns SAML service-provider public URL rebinding: lifecycle-adjacent setup and login flows may depend on shared `internal/api/saml_handlers.go` plus `internal/api/router.go` to rebind initialized SAML providers to the canonical configured `PublicURL`, but they must not snapshot, concatenate, or locally cache alternate SP base URLs inside install or registration surfaces. 4. Keep legacy Unified Agent compatibility names explicitly secondary when touching shared `internal/api/` runtime helpers: the legacy host-route family and `host-agent:*` scope names may remain as ingress or migration aliases, but they must not retake primary ownership in router state, live runtime scope checks, handler commentary, or operator-facing guidance. 5. Add or change installer flags, persisted service arguments, or upgrade-safe re-entry behavior through `scripts/install.sh` and `scripts/install.ps1`. 6. Add or change profile management, the extracted agent profiles runtime owner, the pure unified-agent inventory/install model, the API-backed platform connections workspace shell, route model, reporting summary owner, shared install/inventory/dialog section owners, the split infrastructure install/reporting state owners, the split direct-node/discovery infrastructure settings owners plus their shared model, shared frontend install-command assembly, Proxmox setup/install API transport, TrueNAS platform-connection management, VMware platform-connection management, the shared monitored-system admission preview shell for those platform connections, setup-completion install handoff transport, deploy-fallback manual install transport, and fleet-control presentation through `frontend-modern/src/api/agentProfiles.ts`, `frontend-modern/src/api/nodes.ts`, `frontend-modern/src/components/Settings/AgentProfilesPanel.tsx`, `frontend-modern/src/components/Settings/useAgentProfilesPanelState.ts`, `frontend-modern/src/components/Settings/InfrastructureOperationsController.tsx`, `frontend-modern/src/components/Settings/infrastructureOperationsModel.tsx`, `frontend-modern/src/components/Settings/InfrastructureInstallPanel.tsx`, `frontend-modern/src/components/Settings/InfrastructureInstallerSection.tsx`, `frontend-modern/src/components/Settings/InfrastructureReportingPanel.tsx`, `frontend-modern/src/components/Settings/InfrastructureInventorySection.tsx`, `frontend-modern/src/components/Settings/InfrastructureActiveRowDetails.tsx`, `frontend-modern/src/components/Settings/InfrastructureIgnoredRowDetails.tsx`, `frontend-modern/src/components/Settings/InfrastructureStopMonitoringDialog.tsx`, `frontend-modern/src/components/Settings/InfrastructurePlatformConnectionsSummaryCard.tsx`, `frontend-modern/src/components/Settings/InfrastructureWorkspace.tsx`, `frontend-modern/src/components/Settings/infrastructureWorkspaceModel.ts`, `frontend-modern/src/components/Settings/MonitoredSystemAdmissionPreview.tsx`, `frontend-modern/src/components/Settings/PlatformConnectionsWorkspace.tsx`, `frontend-modern/src/components/Settings/platformConnectionsModel.ts`, `frontend-modern/src/components/Settings/TrueNASSettingsPanel.tsx`, `frontend-modern/src/components/Settings/useTrueNASSettingsPanelState.ts`, `frontend-modern/src/components/Settings/VMwareSettingsPanel.tsx`, `frontend-modern/src/components/Settings/useVMwareSettingsPanelState.ts`, `frontend-modern/src/components/Settings/ProxmoxSettingsPanel.tsx`, `frontend-modern/src/components/Settings/proxmoxSettingsModel.ts`, `frontend-modern/src/components/Settings/ProxmoxDirectWorkspace.tsx`, `frontend-modern/src/components/Settings/ProxmoxConfiguredNodesTable.tsx`, `frontend-modern/src/components/Settings/ProxmoxDirectConnectionsCard.tsx`, `frontend-modern/src/components/Settings/ProxmoxDiscoveryResultsCard.tsx`, `frontend-modern/src/components/Settings/ProxmoxDeleteNodeDialog.tsx`, `frontend-modern/src/components/Settings/ProxmoxNodeModalStack.tsx`, `frontend-modern/src/components/Settings/ConfiguredNodeTables.tsx`, `frontend-modern/src/components/Settings/SettingsSectionNav.tsx`, `frontend-modern/src/components/Settings/infrastructureSettingsModel.ts`, `frontend-modern/src/components/Settings/useInfrastructureConfiguredNodesState.ts`, `frontend-modern/src/components/Settings/useInfrastructureDiscoveryRuntimeState.ts`, `frontend-modern/src/components/Settings/useInfrastructureInstallState.tsx`, `frontend-modern/src/components/Settings/useInfrastructureOperationsState.tsx`, `frontend-modern/src/components/Settings/useInfrastructureReportingState.tsx`, `frontend-modern/src/components/Settings/useInfrastructureSettingsState.ts`, `frontend-modern/src/components/Settings/useProxmoxDirectWorkspaceState.ts`, `frontend-modern/src/components/Settings/NodeModal.tsx`, `frontend-modern/src/components/Settings/nodeModalModel.ts`, `frontend-modern/src/components/Settings/useNodeModalState.ts`, `frontend-modern/src/components/SetupWizard/SetupCompletionPanel.tsx`, and `frontend-modern/src/utils/agentInstallCommand.ts`. diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index ba914b3dc..ea64ca491 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -612,13 +612,6 @@ manual logout URL drift out of the request model or bypass the governed URL normalization path. That same runtime SSO contract also owns the Pulse-side public URL that feeds SAML service-provider metadata and auth requests. `internal/api/saml_handlers.go`, -`internal/api/saml_service.go`, and `internal/api/contract_test.go` must rebind -previously initialized SAML providers to the current configured `PublicURL` -before metadata or browser login flows emit SP entity, ACS, or metadata URLs, -so a stale startup-time blank/relative base URL cannot leak back into runtime -metadata or auth request generation once the canonical external URL is known. -That same runtime SSO contract also owns the Pulse-side public URL that feeds -SAML service-provider metadata and auth requests. `internal/api/saml_handlers.go`, `internal/api/saml_service.go`, and the SAML regression tests must rebind previously initialized SAML providers to the current configured `PublicURL` before metadata or browser login flows emit SP entity, ACS, or metadata URLs, diff --git a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md index 9fb8e42e0..477b698c6 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -204,11 +204,6 @@ regression protection. performance work must not add filesystem reads, GitHub release lookups, or other per-request version discovery on activation, legacy exchange, or grant-refresh traffic just to stamp authenticated install metadata. - That same shared router boundary must keep SAML public URL synchronization - change-driven rather than request-driven: `internal/api/router.go` may rebind - shared SAML service providers when the canonical `PublicURL` changes, but it - must not rebuild that auth state on every dashboard, summary, or chart read - that passes through the protected router hot path. 30. Keep dashboard summary-chart fetches scope-owned rather than page-churn-owned: `frontend-modern/src/hooks/useDashboardTrends.ts` must hydrate infrastructure and storage summaries once per org/range scope from the canonical summary caches and recompute card presentation locally as the compact dashboard overview changes, rather than refetching the infrastructure-summary transport in `frontend-modern/src/components/Infrastructure/useInfrastructureSummaryState.ts`, the dashboard storage-summary trend transport in `frontend-modern/src/utils/storageSummaryTrendCache.ts`, or the storage-page summary transport in `frontend-modern/src/utils/storageSummaryCache.ts` for every top-resource or card reshuffle on the same dashboard load. That dashboard infrastructure path must also request only the metrics it renders through the canonical infrastructure-summary route owned by `internal/api/router_routes_monitoring.go` and `internal/api/router.go`; the dashboard may not pay for disk or network summary series when it only renders CPU and memory. App-shell prewarm in `frontend-modern/src/useAppRuntimeState.ts` must not front-run that dashboard-specific route while the operator is already on the root dashboard route owned by `frontend-modern/src/App.tsx`. The same hot path must keep mock/demo chart identity on the canonical unified snapshot too: when mock mode is enabled, `internal/api/router.go` diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 5f5903b63..c4ab49a12 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -120,7 +120,6 @@ querying, and the operator-facing storage health presentation layer. 6. Preserve API-owned node identity continuity in shared `internal/api/` helpers so storage and recovery transport attachments do not fork by hostname-versus-IP drift across the same runtime. 7. Preserve fail-closed API assignment and lookup behavior in shared `internal/api/` helpers so storage and recovery surfaces do not inherit orphaned profile or resource references from unrelated transport mutations. 8. Preserve canonical configured public endpoint selection in shared `internal/api/` helpers so recovery and storage links do not inherit loopback-local scheme drift from admin-originated setup/install flows. - That same shared endpoint-selection rule also covers SAML service-provider rebinding in `internal/api/saml_handlers.go`, `internal/api/saml_service.go`, and `internal/api/router.go`: storage- and recovery-adjacent flows may depend on the canonical configured `PublicURL`, but they must not fork alternate SP metadata/login base URLs when they share the backend auth helper layer. 9. Preserve trailing-slash normalization in those shared install-command helpers so recovery-adjacent transport and link surfaces do not inherit double-slash installer paths or slash-suffixed public endpoint drift from canonical backend install payloads. 10. Preserve canonical /api/auto-register token-action truth in shared `internal/api/` helpers so adjacent setup and recovery-adjacent transport flows stay on caller-supplied credential completion instead of reviving deleted alternate completion modes. 11. Preserve the canonical setup-script `source="script"` marker through those same shared auto-register helpers, and reject non-canonical source labels there, so later canonical reruns can keep treating script-confirmed tokens differently from agent-created tokens without reviving arbitrary caller-label compatibility. diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 84af86d28..c9a2524bf 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -2025,74 +2025,6 @@ func TestContract_SSOTestRejectsCrossOriginSAMLMetadataRedirect(t *testing.T) { } } -func TestContract_SAMLRuntimePublicURLRefreshKeepsMetadataAndAuthRequestsAbsolute(t *testing.T) { - provider := config.SSOProvider{ - ID: "okta", - Name: "Okta", - Type: config.SSOProviderTypeSAML, - Enabled: true, - SAML: &config.SAMLProviderConfig{ - IDPSSOURL: "https://idp.example.com/sso", - IDPEntityID: "https://idp.example.com/metadata", - }, - } - - router := &Router{ - config: &config.Config{PublicURL: "https://pulse.example.com"}, - samlManager: NewSAMLServiceManager(""), - ssoConfig: &config.SSOConfig{ - Providers: []config.SSOProvider{provider}, - }, - } - - if err := router.samlManager.InitializeProvider(context.Background(), provider.ID, provider.SAML); err != nil { - t.Fatalf("initialize provider: %v", err) - } - service := router.samlManager.GetService(provider.ID) - if service == nil { - t.Fatal("expected initialized SAML service") - } - if got := service.GetSPEntityID(); got != "/saml/okta" { - t.Fatalf("expected relative entity ID before sync, got %q", got) - } - - metadataReq := httptest.NewRequest(http.MethodGet, "/api/saml/okta/metadata", nil) - metadataRec := httptest.NewRecorder() - router.handleSAMLMetadata(metadataRec, metadataReq) - if metadataRec.Code != http.StatusOK { - t.Fatalf("metadata status=%d want %d: %s", metadataRec.Code, http.StatusOK, metadataRec.Body.String()) - } - metadataBody := metadataRec.Body.String() - if !strings.Contains(metadataBody, `entityID="https://pulse.example.com/saml/okta"`) { - t.Fatalf("expected absolute entity ID in metadata, got %q", metadataBody) - } - if !strings.Contains(metadataBody, `Location="https://pulse.example.com/api/saml/okta/acs"`) { - t.Fatalf("expected absolute ACS URL in metadata, got %q", metadataBody) - } - - router.samlManager = NewSAMLServiceManager("") - if err := router.samlManager.InitializeProvider(context.Background(), provider.ID, provider.SAML); err != nil { - t.Fatalf("reinitialize provider: %v", err) - } - service = router.samlManager.GetService(provider.ID) - if service == nil { - t.Fatal("expected initialized SAML service after reset") - } - - loginReq := httptest.NewRequest(http.MethodGet, "/api/saml/okta/login", nil) - loginRec := httptest.NewRecorder() - router.handleSAMLLogin(loginRec, loginReq) - if loginRec.Code != http.StatusFound { - t.Fatalf("login status=%d want %d", loginRec.Code, http.StatusFound) - } - if got := service.GetSPEntityID(); got != "https://pulse.example.com/saml/okta" { - t.Fatalf("expected absolute entity ID after login sync, got %q", got) - } - if got := service.sp.AcsURL.String(); got != "https://pulse.example.com/api/saml/okta/acs" { - t.Fatalf("expected absolute ACS URL after login sync, got %q", got) - } -} - func TestContract_SSOTestRejectsCrossOriginOIDCDiscoveryRedirect(t *testing.T) { targetCalled := make(chan struct{}, 1) var targetURL string