From 513399b004d38a451b6bf4ad06285eccc7130cbe Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 22 Apr 2026 00:54:49 +0100 Subject: [PATCH] Harden hosted signup and audit webhook trust paths --- .../v6/internal/subsystems/cloud-paid.md | 11 +++- internal/cloudcp/admin/handlers.go | 4 +- internal/cloudcp/admin/status_test.go | 8 +-- internal/hosted/provisioner.go | 45 ++++++++++------ internal/hosted/provisioner_test.go | 52 +++++++++++++++++++ internal/securityutil/outbound_http.go | 10 +++- pkg/audit/webhook.go | 14 ++--- 7 files changed, 110 insertions(+), 34 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/cloud-paid.md b/docs/release-control/v6/internal/subsystems/cloud-paid.md index 65a6a0d212..db743932a6 100644 --- a/docs/release-control/v6/internal/subsystems/cloud-paid.md +++ b/docs/release-control/v6/internal/subsystems/cloud-paid.md @@ -165,6 +165,10 @@ Community limit enforcement. through `pkg/licensing/trial_activation.go` and `pkg/licensing/purchase_return.go` 12. Add or change hosted signup provisioning through `internal/hosted/provisioner.go` + That provisioning boundary owns owner-email idempotency for public hosted + signup. Repeated signup requests for the same owner email must resolve to + the existing tenant record instead of creating parallel orgs for one + account identity. 13. Add or change hosted billing-admin presentation through `frontend-modern/src/components/Settings/BillingAdminPanel.tsx`, `frontend-modern/src/components/Settings/BillingAdminOrganizationsTable.tsx`, and `frontend-modern/src/components/Settings/useBillingAdminPanelState.ts` 14. Add or change shared commercial plan/usage presentation through `frontend-modern/src/components/Settings/CommercialBillingSections.tsx` and `frontend-modern/src/utils/commercialBillingModel.ts` 15. Add or change organization billing and usage presentation through `frontend-modern/src/components/Settings/OrganizationBillingPanel.tsx`, `frontend-modern/src/components/Settings/OrganizationBillingLoadingState.tsx`, and `frontend-modern/src/components/Settings/useOrganizationBillingPanelState.ts` @@ -1448,7 +1452,12 @@ unowned hosted runtime helper. That hosted signup boundary is now also canonical in shape: the public signup handler owns request validation, trial billing initialization, and magic-link issuance, while `internal/hosted/provisioner.go` owns the shared org -bootstrap/admin-role assignment and rollback path for hosted signup failures. +bootstrap/admin-role assignment, duplicate-owner-email idempotency, and +rollback path for hosted signup failures. +That duplicate-owner-email rule is fail-closed and server-owned. Repeated +hosted signup attempts for an email that already owns a tenant must resolve to +the existing org identity instead of minting a second tenant and relying on +later auth or billing surfaces to untangle the collision. Hosted billing-state normalization now follows the same rule: a missing `plan_version` must remain missing instead of being synthesized from `subscription_state`, while explicit trial defaults remain explicit. diff --git a/internal/cloudcp/admin/handlers.go b/internal/cloudcp/admin/handlers.go index f65677e8e9..6dc7ca4f2f 100644 --- a/internal/cloudcp/admin/handlers.go +++ b/internal/cloudcp/admin/handlers.go @@ -89,9 +89,7 @@ func AdminKeyMiddleware(adminKey string, next http.Handler) http.Handler { // Provide a conservative default role so downstream account-mutation auth // checks stay fail-closed on missing role context. req := r.Clone(r.Context()) - if strings.TrimSpace(req.Header.Get("X-User-Role")) == "" { - req.Header.Set("X-User-Role", string(registry.MemberRoleOwner)) - } + req.Header.Set("X-User-Role", string(registry.MemberRoleOwner)) next.ServeHTTP(w, req) }) } diff --git a/internal/cloudcp/admin/status_test.go b/internal/cloudcp/admin/status_test.go index be5e8ea174..3d4a3abccf 100644 --- a/internal/cloudcp/admin/status_test.go +++ b/internal/cloudcp/admin/status_test.go @@ -148,7 +148,7 @@ func TestAdminKeyMiddleware(t *testing.T) { } }) - t.Run("injects default owner role when missing", func(t *testing.T) { + t.Run("injects owner role when missing", func(t *testing.T) { roleSeen := "" inspect := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { roleSeen = r.Header.Get("X-User-Role") @@ -169,7 +169,7 @@ func TestAdminKeyMiddleware(t *testing.T) { } }) - t.Run("preserves explicit caller role header", func(t *testing.T) { + t.Run("overrides explicit caller role header", func(t *testing.T) { roleSeen := "" inspect := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { roleSeen = r.Header.Get("X-User-Role") @@ -186,8 +186,8 @@ func TestAdminKeyMiddleware(t *testing.T) { if rec.Code != http.StatusOK { t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) } - if roleSeen != string(registry.MemberRoleAdmin) { - t.Fatalf("X-User-Role = %q, want %q", roleSeen, registry.MemberRoleAdmin) + if roleSeen != string(registry.MemberRoleOwner) { + t.Fatalf("X-User-Role = %q, want %q", roleSeen, registry.MemberRoleOwner) } }) diff --git a/internal/hosted/provisioner.go b/internal/hosted/provisioner.go index 41c2eaf5f0..796faa9a5b 100644 --- a/internal/hosted/provisioner.go +++ b/internal/hosted/provisioner.go @@ -131,21 +131,10 @@ func (p *Provisioner) ProvisionTenant(ctx context.Context, req ProvisionRequest) return nil, err } - orgs, err := p.persistence.ListOrganizations() - if err != nil { - return nil, &SystemError{Op: "list_organizations", Err: err} - } - for _, org := range orgs { - if org == nil { - continue - } - if strings.EqualFold(strings.TrimSpace(org.OwnerUserID), req.Email) { - return &ProvisionResult{ - OrgID: org.ID, - UserID: req.Email, - Status: ProvisionStatusExisting, - }, nil - } + if existing, err := p.findExistingOrganizationByOwnerEmail(req.Email); err != nil { + return nil, err + } else if existing != nil { + return existing, nil } orgID := p.newOrgID() @@ -165,9 +154,35 @@ func (p *Provisioner) ProvisionHostedSignup(ctx context.Context, req HostedSignu return nil, err } + if existing, err := p.findExistingOrganizationByOwnerEmail(req.Email); err != nil { + return nil, err + } else if existing != nil { + return existing, nil + } + return p.createOrganization(ctx, p.newOrgID(), req.Email, req.OrgName) } +func (p *Provisioner) findExistingOrganizationByOwnerEmail(email string) (*ProvisionResult, error) { + orgs, err := p.persistence.ListOrganizations() + if err != nil { + return nil, &SystemError{Op: "list_organizations", Err: err} + } + for _, org := range orgs { + if org == nil { + continue + } + if strings.EqualFold(strings.TrimSpace(org.OwnerUserID), email) { + return &ProvisionResult{ + OrgID: org.ID, + UserID: strings.TrimSpace(org.OwnerUserID), + Status: ProvisionStatusExisting, + }, nil + } + } + return nil, nil +} + func (p *Provisioner) RollbackProvisioning(orgID string) { if p == nil { return diff --git a/internal/hosted/provisioner_test.go b/internal/hosted/provisioner_test.go index 5a765085e0..0cb3260b95 100644 --- a/internal/hosted/provisioner_test.go +++ b/internal/hosted/provisioner_test.go @@ -405,6 +405,58 @@ func TestProvisionHostedSignupSuccess(t *testing.T) { } } +func TestProvisionHostedSignupIdempotentDuplicateEmailCaseInsensitive(t *testing.T) { + baseDir := t.TempDir() + persistence := config.NewMultiTenantPersistence(baseDir) + authManager := &mockAuthManager{} + authProvider := &mockAuthProvider{manager: authManager} + provisioner := NewProvisioner(persistence, authProvider) + + existingOrg := &models.Organization{ + ID: "existing-org", + DisplayName: "Existing Org", + CreatedAt: time.Now().UTC(), + OwnerUserID: "owner@example.com", + Members: []models.OrganizationMember{ + { + UserID: "owner@example.com", + Role: models.OrgRoleOwner, + AddedAt: time.Now().UTC(), + AddedBy: "owner@example.com", + }, + }, + } + if err := persistence.SaveOrganization(existingOrg); err != nil { + t.Fatalf("SaveOrganization returned error: %v", err) + } + + result, err := provisioner.ProvisionHostedSignup(context.Background(), HostedSignupRequest{ + Email: "Owner@Example.com", + OrgName: "Another Org", + }) + if err != nil { + t.Fatalf("ProvisionHostedSignup returned error: %v", err) + } + if result == nil { + t.Fatal("expected result, got nil") + } + if result.Status != ProvisionStatusExisting { + t.Fatalf("expected status %q, got %q", ProvisionStatusExisting, result.Status) + } + if result.OrgID != "existing-org" { + t.Fatalf("expected org ID existing-org, got %q", result.OrgID) + } + if result.UserID != "owner@example.com" { + t.Fatalf("expected normalized user ID owner@example.com, got %q", result.UserID) + } + if authProvider.calls != 0 { + t.Fatalf("expected GetManager to not be called for idempotent hosted signup, got %d", authProvider.calls) + } + if authManager.calls != 0 { + t.Fatalf("expected UpdateUserRoles to not be called for idempotent hosted signup, got %d", authManager.calls) + } +} + func TestCleanupOrgDirectorySkipsUnsafePath(t *testing.T) { baseDir := t.TempDir() p := &Provisioner{} diff --git a/internal/securityutil/outbound_http.go b/internal/securityutil/outbound_http.go index 5e02fce37f..2a3ba5b8ff 100644 --- a/internal/securityutil/outbound_http.go +++ b/internal/securityutil/outbound_http.go @@ -19,6 +19,7 @@ type RestrictedOutboundHTTPOptions struct { AllowPrivateIPs bool AllowLoopback bool TLSConfig *tls.Config + ResolveIPAddrs func(ctx context.Context, host string) ([]net.IPAddr, error) } var resolveOutboundFetchIPs = net.DefaultResolver.LookupIPAddr @@ -64,6 +65,13 @@ func validateOutboundIP(ip net.IP, opts RestrictedOutboundHTTPOptions) error { return nil } +func resolveOutboundIPAddrs(ctx context.Context, host string, opts RestrictedOutboundHTTPOptions) ([]net.IPAddr, error) { + if resolver := opts.ResolveIPAddrs; resolver != nil { + return resolver(ctx, host) + } + return resolveOutboundFetchIPs(ctx, host) +} + func resolvePermittedOutboundIP(ctx context.Context, host string, opts RestrictedOutboundHTTPOptions) (net.IP, error) { host = strings.TrimSpace(host) if host == "" { @@ -89,7 +97,7 @@ func resolvePermittedOutboundIP(ctx context.Context, host string, opts Restricte resolveCtx, cancel := context.WithTimeout(baseCtx, 5*time.Second) defer cancel() - addrs, err := resolveOutboundFetchIPs(resolveCtx, host) + addrs, err := resolveOutboundIPAddrs(resolveCtx, host, opts) if err != nil { return nil, fmt.Errorf("failed to resolve hostname %s: %w", host, err) } diff --git a/pkg/audit/webhook.go b/pkg/audit/webhook.go index 3be54bc68c..0f5181ae42 100644 --- a/pkg/audit/webhook.go +++ b/pkg/audit/webhook.go @@ -48,16 +48,10 @@ type WebhookPayload struct { func NewWebhookDelivery(urls []string) *WebhookDelivery { return &WebhookDelivery{ urls: urls, - client: &http.Client{ - Timeout: webhookTimeout, - Transport: &http.Transport{ - MaxIdleConns: 10, - IdleConnTimeout: 90 * time.Second, - DisableCompression: true, - MaxConnsPerHost: 5, - MaxIdleConnsPerHost: 2, - }, - }, + client: securityutil.NewRestrictedOutboundHTTPClient(webhookTimeout, securityutil.RestrictedOutboundHTTPOptions{ + AllowedSchemes: []string{"http", "https"}, + ResolveIPAddrs: resolveWebhookIPs, + }), queue: make(chan Event, webhookQueueSize), stopChan: make(chan struct{}), }