mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-11 22:12:23 +00:00
Trim stale SAML rebinding coverage
This commit is contained in:
@@ -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`.
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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`
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user