From b006b4cb5cc3c77a7e24b4c0a667f2a9379a07a6 Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 2 Apr 2026 11:54:59 +0200 Subject: [PATCH 1/3] docs: define channel install result surface --- ...26-04-02-channel-install-result-surface.md | 205 ++++++++++++++++++ 1 file changed, 205 insertions(+) create mode 100644 docs/2026-04-02-channel-install-result-surface.md diff --git a/docs/2026-04-02-channel-install-result-surface.md b/docs/2026-04-02-channel-install-result-surface.md new file mode 100644 index 0000000..2e11a40 --- /dev/null +++ b/docs/2026-04-02-channel-install-result-surface.md @@ -0,0 +1,205 @@ +--- +date: 2026-04-02 +author: Onur Solmaz +title: Channel Install Result Surface +tags: [spritz, channel-gateway, slack, oauth, error-handling, architecture] +--- + +## Overview + +Shared channel installs must always end on a Spritz-controlled result surface. + +In concrete terms, the browser should never fall through from a provider +callback to an infrastructure-generated error page such as a CDN or proxy +`502`. If the install fails for an expected product reason, Spritz should show +that failure as product UI. + +This document defines the generic Spritz-side contract for: + +- install success +- install failure +- typed browser-facing install errors +- the split between Spritz-owned UX and deployment-owned install policy + +The immediate motivation is Slack workspace install UX, but the same model +should apply to any shared channel gateway. + +Related docs: + +- [Shared App Tenant Routing Architecture](2026-03-23-shared-app-tenant-routing-architecture.md) +- [Slack Channel Gateway Implementation Plan](2026-03-24-slack-channel-gateway-implementation-plan.md) +- [External Identity Resolution API Architecture](2026-03-12-external-identity-resolution-api-architecture.md) + +## Problem + +The callback flow currently has an important UX gap: + +1. the user starts a provider install flow +2. the provider redirects back to the shared channel gateway callback +3. the gateway exchanges the callback code and finalizes install state +4. a deployment-owned backend returns an expected business failure +5. the browser sees a generic proxy or upstream error page + +That is the wrong contract. + +Expected install failures such as unresolved identity, denied authorization, or +installation conflict are not infrastructure failures. They are product +outcomes and must be rendered as product UI. + +## Goals + +- always show a clear success or failure surface after install callback +- keep the install result flow provider-agnostic inside Spritz +- preserve typed error information without leaking backend internals +- keep request correlation so operators can map a visible failure back to logs + +## Non-Goals + +- changing deployment-specific owner or account-linking policy +- defining provider-specific copy inside Spritz core +- moving deployment-specific installation storage into Spritz +- replacing typed API errors with free-form HTML from external backends + +## Core Decision + +The provider callback is app-controlled. + +Here, "app-controlled" means the result is owned by the shared channel gateway +or the Spritz UI, not by a reverse proxy, ingress, CDN, or provider-generated +error page. + +The callback must always terminate in one of two Spritz-owned outcomes: + +- install success +- install failure + +Recommended high-level flow: + +1. gateway receives the provider callback +2. gateway validates callback state +3. gateway exchanges the provider code +4. gateway calls the deployment-owned install finalizer +5. gateway normalizes the outcome into a stable Spritz result +6. gateway redirects to or renders a Spritz-owned result surface + +## Result Surface + +Spritz should expose one stable install-result surface, for example: + +- a UI route such as `/install/result` +- or a minimal HTML page rendered directly by the gateway when no richer UI is + available + +The long-term preferred shape is a dedicated Spritz UI route, but either option +is acceptable as long as the surface is app-controlled. + +The normalized result payload should carry at least: + +- `status`: `success | error` +- `provider` +- `code` +- `requestId` +- `retryable` +- optional safe display metadata such as next-step hints + +Rules: + +- browser-facing copy must come from the normalized result code +- raw backend payloads must not be shown directly to the user +- expected failures must not render as raw `5xx` infrastructure pages + +## Error Taxonomy + +Spritz should define a stable set of install error codes. + +Recommended initial set: + +- `install_state_invalid` +- `install_state_expired` +- `provider_authorization_denied` +- `provider_authorization_failed` +- `external_identity_unresolved` +- `external_identity_forbidden` +- `external_identity_ambiguous` +- `installation_conflict` +- `installation_registry_unavailable` +- `runtime_binding_unavailable` +- `internal_error` + +Notes: + +- the `external_identity_*` codes should align with the typed error model from + [External Identity Resolution API Architecture](2026-03-12-external-identity-resolution-api-architecture.md) +- retryable upstream failures should map to typed availability codes, not a raw + browser-visible `502` +- unknown failures may collapse to `internal_error` + +## Scope Split + +### Spritz owns + +- callback termination behavior +- the install result route or result page +- the install error taxonomy +- safe default result-page copy +- request ID propagation into logs and user-visible result pages + +### Deployment-owned integration owns + +- install finalization policy +- identity and account-linking policy +- installation registry implementation +- branding and deployment-specific copy overrides +- any fallback policy involving an existing first-party browser session + +This keeps Spritz reusable while still letting each deployment enforce its own +ownership and identity rules. + +## Backend And Adapter Contract + +The deployment-owned install finalizer should not force the gateway to infer +product meaning from arbitrary upstream failures. + +Preferred contract: + +- success returns a normalized install success payload +- expected failures return typed machine-readable codes +- temporary failures return typed availability errors + +If an existing deployment backend does not yet expose the normalized shape, the +deployment adapter in the gateway must translate backend-specific responses into +the Spritz install-result taxonomy before the browser sees them. + +## User-Facing Behavior + +The result surface should stay simple and production-oriented: + +- always show success or failure explicitly +- always explain the next action when one exists +- always show a request ID for support and debugging +- never show stack traces or raw upstream payloads + +Examples: + +- `external_identity_unresolved`: tell the user the install could not be linked + to a product account yet +- `install_state_expired`: tell the user the install link expired and should be + started again +- `installation_registry_unavailable`: tell the user the install could not be + completed right now and can be retried + +## Validation + +Minimum validation for this flow: + +- invalid or expired state shows a controlled error surface +- unresolved external identity shows a controlled error surface +- temporary install-finalizer outage shows a controlled retryable error surface +- successful install shows a controlled confirmation surface +- every result logs the same `requestId` visible to the user + +## Follow-Ups + +- add the install-result route or gateway-rendered fallback +- wire the shared Slack gateway callback to the normalized result contract +- reuse the same result surface for future Discord and Teams shared installs From af0352eb7e3eff1fc642a9963818ed58c1bf6a7d Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 2 Apr 2026 12:06:24 +0200 Subject: [PATCH 2/3] fix(slack-gateway): render controlled install results --- integrations/slack-gateway/gateway.go | 1 + integrations/slack-gateway/gateway_test.go | 157 +++++++- integrations/slack-gateway/install_result.go | 360 +++++++++++++++++++ integrations/slack-gateway/slack_oauth.go | 99 ++++- 4 files changed, 590 insertions(+), 27 deletions(-) create mode 100644 integrations/slack-gateway/install_result.go diff --git a/integrations/slack-gateway/gateway.go b/integrations/slack-gateway/gateway.go index 4128815..7bb65aa 100644 --- a/integrations/slack-gateway/gateway.go +++ b/integrations/slack-gateway/gateway.go @@ -74,6 +74,7 @@ func (g *slackGateway) routes() http.Handler { mux := http.NewServeMux() g.registerRoute(mux, "/healthz", g.handleHealthz) g.registerRoute(mux, "/slack/install", g.handleInstallRedirect) + g.registerRoute(mux, "/slack/install/result", g.handleInstallResult) g.registerRoute(mux, "/slack/oauth/callback", g.handleOAuthCallback) g.registerRoute(mux, "/slack/events", g.handleSlackEvents) return mux diff --git a/integrations/slack-gateway/gateway_test.go b/integrations/slack-gateway/gateway_test.go index d3e6815..886ba9d 100644 --- a/integrations/slack-gateway/gateway_test.go +++ b/integrations/slack-gateway/gateway_test.go @@ -84,15 +84,35 @@ func TestOAuthCallbackStoresInstallationAndUpsertsRegistry(t *testing.T) { rec := httptest.NewRecorder() gateway.handleOAuthCallback(rec, req) - if rec.Code != http.StatusOK { - t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) + if rec.Code != http.StatusSeeOther { + t.Fatalf("expected 303, got %d: %s", rec.Code, rec.Body.String()) + } + location := rec.Header().Get("Location") + if location == "" { + t.Fatal("expected callback redirect location") + } + redirectURL, err := url.Parse(location) + if err != nil { + t.Fatalf("parse callback redirect: %v", err) + } + if redirectURL.Path != "/slack/install/result" { + t.Fatalf("expected install result path, got %s", redirectURL.Path) } - var callbackPayload map[string]any - if err := json.Unmarshal(rec.Body.Bytes(), &callbackPayload); err != nil { - t.Fatalf("decode callback body: %v", err) + if got := redirectURL.Query().Get("status"); got != "success" { + t.Fatalf("expected success status, got %q", got) } - if callbackPayload["providerInstallRef"] != "cred_slack_workspace_1" { - t.Fatalf("expected backend-assigned install ref, got %#v", callbackPayload["providerInstallRef"]) + if got := redirectURL.Query().Get("code"); got != "installed" { + t.Fatalf("expected installed code, got %q", got) + } + if got := redirectURL.Query().Get("provider"); got != "slack" { + t.Fatalf("expected slack provider, got %q", got) + } + if got := redirectURL.Query().Get("teamId"); got != "T_workspace_1" { + t.Fatalf("expected team id in result redirect, got %q", got) + } + requestID := redirectURL.Query().Get("requestId") + if strings.TrimSpace(requestID) == "" { + t.Fatal("expected request id in result redirect") } if upsertPayload["principalId"] != "shared-slack-gateway" { t.Fatalf("expected principalId to match, got %#v", upsertPayload["principalId"]) @@ -126,6 +146,27 @@ func TestOAuthCallbackStoresInstallationAndUpsertsRegistry(t *testing.T) { if upsertPayload["presetId"] != "zeno" { t.Fatalf("expected presetId zeno, got %#v", upsertPayload["presetId"]) } + if upsertPayload["requestId"] != requestID { + t.Fatalf("expected requestId to propagate to backend, got %#v", upsertPayload["requestId"]) + } + + resultReq := httptest.NewRequest(http.MethodGet, redirectURL.RequestURI(), nil) + resultRec := httptest.NewRecorder() + gateway.routes().ServeHTTP(resultRec, resultReq) + + if resultRec.Code != http.StatusOK { + t.Fatalf("expected result page 200, got %d: %s", resultRec.Code, resultRec.Body.String()) + } + if contentType := resultRec.Header().Get("Content-Type"); !strings.Contains(contentType, "text/html") { + t.Fatalf("expected html content type, got %q", contentType) + } + body := resultRec.Body.String() + if !strings.Contains(body, "Slack workspace connected") { + t.Fatalf("expected success title in result page, got %q", body) + } + if !strings.Contains(body, requestID) { + t.Fatalf("expected request id in result page, got %q", body) + } } func TestInstallRedirectUsesConfiguredSlackHost(t *testing.T) { @@ -202,7 +243,7 @@ func TestRoutesServeSlackEndpointsUnderConfiguredPublicURLPathPrefix(t *testing. } } -func TestOAuthCallbackReturnsBadGatewayWhenBackendUpsertFails(t *testing.T) { +func TestOAuthCallbackRedirectsToControlledRetryableErrorWhenBackendUpsertFails(t *testing.T) { backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.Error(w, "backend unavailable", http.StatusServiceUnavailable) })) @@ -251,8 +292,29 @@ func TestOAuthCallbackReturnsBadGatewayWhenBackendUpsertFails(t *testing.T) { rec := httptest.NewRecorder() gateway.handleOAuthCallback(rec, req) - if rec.Code != http.StatusBadGateway { - t.Fatalf("expected 502, got %d: %s", rec.Code, rec.Body.String()) + if rec.Code != http.StatusSeeOther { + t.Fatalf("expected 303, got %d: %s", rec.Code, rec.Body.String()) + } + redirectURL, err := url.Parse(rec.Header().Get("Location")) + if err != nil { + t.Fatalf("parse callback redirect: %v", err) + } + if got := redirectURL.Query().Get("status"); got != "error" { + t.Fatalf("expected error status, got %q", got) + } + if got := redirectURL.Query().Get("code"); got != "installation_registry_unavailable" { + t.Fatalf("expected installation registry unavailable code, got %q", got) + } + + resultReq := httptest.NewRequest(http.MethodGet, redirectURL.RequestURI(), nil) + resultRec := httptest.NewRecorder() + gateway.routes().ServeHTTP(resultRec, resultReq) + + if resultRec.Code != http.StatusOK { + t.Fatalf("expected result page 200, got %d: %s", resultRec.Code, resultRec.Body.String()) + } + if strings.Contains(resultRec.Body.String(), "backend unavailable") { + t.Fatalf("expected retryable result page to hide backend body, got %q", resultRec.Body.String()) } logOutput := logBuffer.String() if !strings.Contains(logOutput, "slack oauth callback installation upsert failed") { @@ -286,9 +348,11 @@ func TestExtractACPTextSupportsResourceBlocks(t *testing.T) { } } -func TestOAuthCallbackReturnsBadGatewayOnDeterministicBackendFailure(t *testing.T) { +func TestOAuthCallbackRedirectsToControlledOwnerResolutionError(t *testing.T) { backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - http.Error(w, "owner was not found", http.StatusNotFound) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"status":"unresolved","field":"ownerRef","error":"external_identity_unresolved"}`)) })) defer backend.Close() @@ -331,8 +395,73 @@ func TestOAuthCallbackReturnsBadGatewayOnDeterministicBackendFailure(t *testing. rec := httptest.NewRecorder() gateway.handleOAuthCallback(rec, req) - if rec.Code != http.StatusBadGateway { - t.Fatalf("expected 502, got %d: %s", rec.Code, rec.Body.String()) + if rec.Code != http.StatusSeeOther { + t.Fatalf("expected 303, got %d: %s", rec.Code, rec.Body.String()) + } + redirectURL, err := url.Parse(rec.Header().Get("Location")) + if err != nil { + t.Fatalf("parse callback redirect: %v", err) + } + if got := redirectURL.Query().Get("code"); got != "external_identity_unresolved" { + t.Fatalf("expected external identity unresolved code, got %q", got) + } + + resultReq := httptest.NewRequest(http.MethodGet, redirectURL.RequestURI(), nil) + resultRec := httptest.NewRecorder() + gateway.routes().ServeHTTP(resultRec, resultReq) + + if resultRec.Code != http.StatusOK { + t.Fatalf("expected result page 200, got %d: %s", resultRec.Code, resultRec.Body.String()) + } + body := resultRec.Body.String() + if !strings.Contains(body, "could not be linked") { + t.Fatalf("expected owner resolution guidance in result page, got %q", body) + } + if strings.Contains(body, "ownerRef") { + t.Fatalf("expected result page to avoid leaking backend field names, got %q", body) + } +} + +func TestOAuthCallbackRedirectsDeniedProviderAuthToControlledResult(t *testing.T) { + cfg := config{ + PublicURL: "https://gateway.example.test", + SlackClientID: "client-id", + SlackClientSecret: "client-secret", + SlackSigningSecret: "signing-secret", + OAuthStateSecret: "oauth-state-secret", + SlackAPIBaseURL: "https://slack.example.test/api", + SlackBotScopes: []string{"chat:write"}, + BackendBaseURL: "https://backend.example.test", + BackendInternalToken: "backend-internal-token", + SpritzBaseURL: "https://spritz.example.test", + SpritzServiceToken: "spritz-service-token", + PrincipalID: "shared-slack-gateway", + HTTPTimeout: 5 * time.Second, + DedupeTTL: time.Minute, + } + gateway := newSlackGateway(cfg, slog.New(slog.NewTextHandler(io.Discard, nil))) + state, err := gateway.state.generate() + if err != nil { + t.Fatalf("state generate failed: %v", err) + } + + req := httptest.NewRequest( + http.MethodGet, + "/slack/oauth/callback?error=access_denied&state="+url.QueryEscape(state), + nil, + ) + rec := httptest.NewRecorder() + gateway.handleOAuthCallback(rec, req) + + if rec.Code != http.StatusSeeOther { + t.Fatalf("expected 303, got %d: %s", rec.Code, rec.Body.String()) + } + redirectURL, err := url.Parse(rec.Header().Get("Location")) + if err != nil { + t.Fatalf("parse callback redirect: %v", err) + } + if got := redirectURL.Query().Get("code"); got != "provider_authorization_denied" { + t.Fatalf("expected provider authorization denied code, got %q", got) } } diff --git a/integrations/slack-gateway/install_result.go b/integrations/slack-gateway/install_result.go new file mode 100644 index 0000000..1f93f6b --- /dev/null +++ b/integrations/slack-gateway/install_result.go @@ -0,0 +1,360 @@ +package main + +import ( + "crypto/rand" + "encoding/hex" + "encoding/json" + "errors" + "html/template" + "net/http" + "net/url" + "strings" +) + +type installResultStatus string + +const ( + installResultStatusSuccess installResultStatus = "success" + installResultStatusError installResultStatus = "error" +) + +type installResultCode string + +const ( + installResultCodeInstalled installResultCode = "installed" + installResultCodeInstallStateInvalid installResultCode = "install_state_invalid" + installResultCodeInstallStateExpired installResultCode = "install_state_expired" + installResultCodeProviderAuthorizationDenied installResultCode = "provider_authorization_denied" + installResultCodeProviderAuthorizationFailed installResultCode = "provider_authorization_failed" + installResultCodeExternalIdentityUnresolved installResultCode = "external_identity_unresolved" + installResultCodeExternalIdentityForbidden installResultCode = "external_identity_forbidden" + installResultCodeExternalIdentityAmbiguous installResultCode = "external_identity_ambiguous" + installResultCodeInstallationConflict installResultCode = "installation_conflict" + installResultCodeInstallationRegistryUnavailable installResultCode = "installation_registry_unavailable" + installResultCodeRuntimeBindingUnavailable installResultCode = "runtime_binding_unavailable" + installResultCodeInternalError installResultCode = "internal_error" +) + +type installResult struct { + Status installResultStatus + Code installResultCode + Provider string + RequestID string + TeamID string +} + +type installResultDescriptor struct { + Title string + Message string + Retryable bool + ActionLabel string + ActionHref string +} + +type installResultPageData struct { + Title string + Message string + RequestID string + ActionLabel string + ActionHref string +} + +type backendInstallErrorPayload struct { + Status string `json:"status"` + Field string `json:"field,omitempty"` + Error string `json:"error,omitempty"` + Message string `json:"message,omitempty"` + RequestID string `json:"requestId,omitempty"` +} + +var installResultPageTemplate = template.Must(template.New("install-result").Parse(` + + + + + {{ .Title }} + + + +
+

{{ .Title }}

+

{{ .Message }}

+ {{ if .ActionHref }} + {{ .ActionLabel }} + {{ end }} + {{ if .RequestID }} +
Request ID: {{ .RequestID }}
+ {{ end }} +
+ +`)) + +func newInstallRequestID() string { + var bytes [8]byte + if _, err := rand.Read(bytes[:]); err == nil { + return hex.EncodeToString(bytes[:]) + } + return "install-request-id-unavailable" +} + +func (g *slackGateway) installResultPath() string { + return g.publicPathPrefix() + "/slack/install/result" +} + +func (g *slackGateway) installRedirectPath() string { + return g.publicPathPrefix() + "/slack/install" +} + +func (g *slackGateway) redirectToInstallResult(w http.ResponseWriter, r *http.Request, result installResult) { + target := url.URL{Path: g.installResultPath()} + query := target.Query() + query.Set("status", string(result.Status)) + query.Set("code", string(result.Code)) + query.Set("provider", firstNonEmpty(result.Provider, slackProvider)) + if requestID := strings.TrimSpace(result.RequestID); requestID != "" { + query.Set("requestId", requestID) + } + if teamID := strings.TrimSpace(result.TeamID); teamID != "" { + query.Set("teamId", teamID) + } + target.RawQuery = query.Encode() + http.Redirect(w, r, target.String(), http.StatusSeeOther) +} + +func installResultDescriptorFor(code installResultCode, installURL string) installResultDescriptor { + switch code { + case installResultCodeInstalled: + return installResultDescriptor{ + Title: "Slack workspace connected", + Message: "The shared Slack app is installed and ready. You can close this tab.", + } + case installResultCodeInstallStateExpired: + return installResultDescriptor{ + Title: "Install link expired", + Message: "This install link expired before it completed. Start the install again.", + Retryable: true, + ActionLabel: "Start install again", + ActionHref: installURL, + } + case installResultCodeInstallStateInvalid: + return installResultDescriptor{ + Title: "Install link is invalid", + Message: "This install callback could not be verified. Start the install again.", + Retryable: true, + ActionLabel: "Start install again", + ActionHref: installURL, + } + case installResultCodeProviderAuthorizationDenied: + return installResultDescriptor{ + Title: "Slack authorization was cancelled", + Message: "The Slack install did not finish because authorization was denied or cancelled.", + Retryable: true, + ActionLabel: "Start install again", + ActionHref: installURL, + } + case installResultCodeProviderAuthorizationFailed: + return installResultDescriptor{ + Title: "Slack authorization failed", + Message: "The Slack install did not complete successfully. Please try again.", + Retryable: true, + ActionLabel: "Start install again", + ActionHref: installURL, + } + case installResultCodeExternalIdentityUnresolved: + return installResultDescriptor{ + Title: "Install could not be linked", + Message: "This Slack install could not be linked to an owner account yet. Link the expected account, then start the install again.", + Retryable: true, + ActionLabel: "Start install again", + ActionHref: installURL, + } + case installResultCodeExternalIdentityForbidden: + return installResultDescriptor{ + Title: "Install is not allowed", + Message: "This Slack identity is not allowed to complete the install for this deployment.", + ActionLabel: "Start install again", + ActionHref: installURL, + } + case installResultCodeExternalIdentityAmbiguous: + return installResultDescriptor{ + Title: "Install owner is ambiguous", + Message: "This Slack install matched more than one possible owner. Resolve the account mapping, then start the install again.", + ActionLabel: "Start install again", + ActionHref: installURL, + } + case installResultCodeInstallationConflict: + return installResultDescriptor{ + Title: "Install conflicts with existing state", + Message: "This workspace already has conflicting install state. Resolve the existing binding, then start the install again.", + ActionLabel: "Start install again", + ActionHref: installURL, + } + case installResultCodeInstallationRegistryUnavailable: + return installResultDescriptor{ + Title: "Install could not be completed", + Message: "The install service is temporarily unavailable. Please try again shortly.", + Retryable: true, + ActionLabel: "Start install again", + ActionHref: installURL, + } + case installResultCodeRuntimeBindingUnavailable: + return installResultDescriptor{ + Title: "Install is still being prepared", + Message: "The workspace binding is not ready yet. Please try again shortly.", + Retryable: true, + ActionLabel: "Start install again", + ActionHref: installURL, + } + default: + return installResultDescriptor{ + Title: "Install failed", + Message: "The install did not complete successfully. Please try again.", + Retryable: true, + ActionLabel: "Start install again", + ActionHref: installURL, + } + } +} + +func normalizeInstallResultCode(raw string) installResultCode { + switch installResultCode(strings.TrimSpace(raw)) { + case installResultCodeInstalled, + installResultCodeInstallStateInvalid, + installResultCodeInstallStateExpired, + installResultCodeProviderAuthorizationDenied, + installResultCodeProviderAuthorizationFailed, + installResultCodeExternalIdentityUnresolved, + installResultCodeExternalIdentityForbidden, + installResultCodeExternalIdentityAmbiguous, + installResultCodeInstallationConflict, + installResultCodeInstallationRegistryUnavailable, + installResultCodeRuntimeBindingUnavailable, + installResultCodeInternalError: + return installResultCode(strings.TrimSpace(raw)) + default: + return installResultCodeInternalError + } +} + +func classifyInstallUpsertError(err error) installResultCode { + var statusErr *httpStatusError + if !errors.As(err, &statusErr) { + return installResultCodeInternalError + } + if statusErr.statusCode >= http.StatusInternalServerError { + return installResultCodeInstallationRegistryUnavailable + } + var payload backendInstallErrorPayload + if json.Unmarshal([]byte(strings.TrimSpace(statusErr.body)), &payload) == nil { + if code := normalizeInstallResultCode(payload.Error); code != installResultCodeInternalError { + return code + } + if statusErr.statusCode == http.StatusNotFound && payload.Status == "unresolved" && payload.Field == "ownerRef" { + return installResultCodeExternalIdentityUnresolved + } + if statusErr.statusCode == http.StatusForbidden && payload.Status == "forbidden" && payload.Field == "ownerRef" { + return installResultCodeExternalIdentityForbidden + } + if statusErr.statusCode == http.StatusConflict && payload.Status == "ambiguous" && payload.Field == "ownerRef" { + return installResultCodeExternalIdentityAmbiguous + } + if statusErr.statusCode == http.StatusConflict && payload.Status == "ambiguous" { + return installResultCodeInstallationConflict + } + if statusErr.statusCode == http.StatusServiceUnavailable && payload.Status == "unavailable" { + return installResultCodeInstallationRegistryUnavailable + } + } + switch statusErr.statusCode { + case http.StatusServiceUnavailable: + return installResultCodeInstallationRegistryUnavailable + case http.StatusConflict: + return installResultCodeInstallationConflict + default: + return installResultCodeInternalError + } +} + +func (g *slackGateway) handleInstallResult(w http.ResponseWriter, r *http.Request) { + result := installResult{ + Status: installResultStatus(firstNonEmpty(r.URL.Query().Get("status"), string(installResultStatusError))), + Code: normalizeInstallResultCode(firstNonEmpty(r.URL.Query().Get("code"), string(installResultCodeInternalError))), + Provider: firstNonEmpty(r.URL.Query().Get("provider"), slackProvider), + RequestID: strings.TrimSpace(r.URL.Query().Get("requestId")), + TeamID: strings.TrimSpace(r.URL.Query().Get("teamId")), + } + descriptor := installResultDescriptorFor(result.Code, g.installRedirectPath()) + if result.Status == installResultStatusSuccess && result.Code == installResultCodeInternalError { + descriptor = installResultDescriptorFor(installResultCodeInstalled, g.installRedirectPath()) + } + page := installResultPageData{ + Title: descriptor.Title, + Message: descriptor.Message, + RequestID: result.RequestID, + ActionLabel: descriptor.ActionLabel, + ActionHref: descriptor.ActionHref, + } + w.Header().Set("Cache-Control", "no-store") + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.WriteHeader(http.StatusOK) + _ = installResultPageTemplate.Execute(w, page) +} diff --git a/integrations/slack-gateway/slack_oauth.go b/integrations/slack-gateway/slack_oauth.go index 93e80ac..7a48f4c 100644 --- a/integrations/slack-gateway/slack_oauth.go +++ b/integrations/slack-gateway/slack_oauth.go @@ -71,26 +71,86 @@ func slackOAuthAuthorizeURL(apiBaseURL string) (*url.URL, error) { } func (g *slackGateway) handleOAuthCallback(w http.ResponseWriter, r *http.Request) { + requestID := newInstallRequestID() state := strings.TrimSpace(r.URL.Query().Get("state")) code := strings.TrimSpace(r.URL.Query().Get("code")) if err := g.state.validate(state); err != nil { - g.logger.ErrorContext(r.Context(), "slack oauth callback state validation failed", "err", err) - http.Error(w, err.Error(), http.StatusBadRequest) + g.logger.ErrorContext( + r.Context(), + "slack oauth callback state validation failed", + "err", + err, + "request_id", + requestID, + ) + resultCode := installResultCodeInstallStateInvalid + if strings.Contains(strings.ToLower(err.Error()), "expired") { + resultCode = installResultCodeInstallStateExpired + } + g.redirectToInstallResult(w, r, installResult{ + Status: installResultStatusError, + Code: resultCode, + Provider: slackProvider, + RequestID: requestID, + }) + return + } + if providerError := strings.TrimSpace(r.URL.Query().Get("error")); providerError != "" { + g.logger.InfoContext( + r.Context(), + "slack oauth callback authorization denied", + "provider_error", + providerError, + "request_id", + requestID, + ) + resultCode := installResultCodeProviderAuthorizationFailed + if providerError == "access_denied" { + resultCode = installResultCodeProviderAuthorizationDenied + } + g.redirectToInstallResult(w, r, installResult{ + Status: installResultStatusError, + Code: resultCode, + Provider: slackProvider, + RequestID: requestID, + }) return } if code == "" { - g.logger.ErrorContext(r.Context(), "slack oauth callback missing code") - http.Error(w, "code is required", http.StatusBadRequest) + g.logger.ErrorContext( + r.Context(), + "slack oauth callback missing code", + "request_id", + requestID, + ) + g.redirectToInstallResult(w, r, installResult{ + Status: installResultStatusError, + Code: installResultCodeProviderAuthorizationFailed, + Provider: slackProvider, + RequestID: requestID, + }) return } installation, err := g.exchangeSlackOAuthCode(r.Context(), code) if err != nil { - g.logger.ErrorContext(r.Context(), "slack oauth callback code exchange failed", "err", err) - http.Error(w, err.Error(), http.StatusBadGateway) + g.logger.ErrorContext( + r.Context(), + "slack oauth callback code exchange failed", + "err", + err, + "request_id", + requestID, + ) + g.redirectToInstallResult(w, r, installResult{ + Status: installResultStatusError, + Code: installResultCodeProviderAuthorizationFailed, + Provider: slackProvider, + RequestID: requestID, + }) return } - if err := g.upsertInstallation(r.Context(), &installation); err != nil { + if err := g.upsertInstallation(r.Context(), &installation, requestID); err != nil { g.logger.ErrorContext( r.Context(), "slack oauth callback installation upsert failed", @@ -100,8 +160,16 @@ func (g *slackGateway) handleOAuthCallback(w http.ResponseWriter, r *http.Reques installation.TeamID, "installing_user_id", installation.InstallingUserID, + "request_id", + requestID, ) - http.Error(w, err.Error(), http.StatusBadGateway) + g.redirectToInstallResult(w, r, installResult{ + Status: installResultStatusError, + Code: classifyInstallUpsertError(err), + Provider: slackProvider, + RequestID: requestID, + TeamID: installation.TeamID, + }) return } g.logger.InfoContext( @@ -109,11 +177,15 @@ func (g *slackGateway) handleOAuthCallback(w http.ResponseWriter, r *http.Reques "slack oauth callback installed workspace", "team_id", installation.TeamID, + "request_id", + requestID, ) - writeJSON(w, http.StatusOK, map[string]any{ - "status": "installed", - "teamId": installation.TeamID, - "providerInstallRef": installation.ProviderInstallRef, + g.redirectToInstallResult(w, r, installResult{ + Status: installResultStatusSuccess, + Code: installResultCodeInstalled, + Provider: slackProvider, + RequestID: requestID, + TeamID: installation.TeamID, }) } @@ -164,7 +236,7 @@ func (g *slackGateway) exchangeSlackOAuthCode(ctx context.Context, code string) return record, nil } -func (g *slackGateway) upsertInstallation(ctx context.Context, installation *slackInstallation) error { +func (g *slackGateway) upsertInstallation(ctx context.Context, installation *slackInstallation, requestID string) error { if installation == nil { return fmt.Errorf("installation is required") } @@ -189,6 +261,7 @@ func (g *slackGateway) upsertInstallation(ctx context.Context, installation *sla "botAccessToken": installation.BotAccessToken, "scopeSet": installation.ScopeSet, }, + "requestId": strings.TrimSpace(requestID), } var payload backendInstallationUpsertResponse if err := g.postBackendJSON(ctx, "/internal/v1/spritz/channel-installations/upsert", body, &payload); err != nil { From db02cf51040965570a2a51813a9bb6c0e50875a5 Mon Sep 17 00:00:00 2001 From: Onur Solmaz Date: Thu, 2 Apr 2026 12:14:11 +0200 Subject: [PATCH 3/3] fix(slack-gateway): preserve typed install result mapping --- integrations/slack-gateway/install_result.go | 16 +++++----- .../slack-gateway/install_result_test.go | 32 +++++++++++++++++++ 2 files changed, 40 insertions(+), 8 deletions(-) create mode 100644 integrations/slack-gateway/install_result_test.go diff --git a/integrations/slack-gateway/install_result.go b/integrations/slack-gateway/install_result.go index 1f93f6b..abcbb56 100644 --- a/integrations/slack-gateway/install_result.go +++ b/integrations/slack-gateway/install_result.go @@ -300,27 +300,24 @@ func classifyInstallUpsertError(err error) installResultCode { if !errors.As(err, &statusErr) { return installResultCodeInternalError } - if statusErr.statusCode >= http.StatusInternalServerError { - return installResultCodeInstallationRegistryUnavailable - } var payload backendInstallErrorPayload if json.Unmarshal([]byte(strings.TrimSpace(statusErr.body)), &payload) == nil { if code := normalizeInstallResultCode(payload.Error); code != installResultCodeInternalError { return code } - if statusErr.statusCode == http.StatusNotFound && payload.Status == "unresolved" && payload.Field == "ownerRef" { + if payload.Status == "unresolved" && payload.Field == "ownerRef" { return installResultCodeExternalIdentityUnresolved } - if statusErr.statusCode == http.StatusForbidden && payload.Status == "forbidden" && payload.Field == "ownerRef" { + if payload.Status == "forbidden" && payload.Field == "ownerRef" { return installResultCodeExternalIdentityForbidden } - if statusErr.statusCode == http.StatusConflict && payload.Status == "ambiguous" && payload.Field == "ownerRef" { + if payload.Status == "ambiguous" && payload.Field == "ownerRef" { return installResultCodeExternalIdentityAmbiguous } - if statusErr.statusCode == http.StatusConflict && payload.Status == "ambiguous" { + if payload.Status == "ambiguous" { return installResultCodeInstallationConflict } - if statusErr.statusCode == http.StatusServiceUnavailable && payload.Status == "unavailable" { + if payload.Status == "unavailable" { return installResultCodeInstallationRegistryUnavailable } } @@ -330,6 +327,9 @@ func classifyInstallUpsertError(err error) installResultCode { case http.StatusConflict: return installResultCodeInstallationConflict default: + if statusErr.statusCode >= http.StatusInternalServerError { + return installResultCodeInstallationRegistryUnavailable + } return installResultCodeInternalError } } diff --git a/integrations/slack-gateway/install_result_test.go b/integrations/slack-gateway/install_result_test.go new file mode 100644 index 0000000..2e593aa --- /dev/null +++ b/integrations/slack-gateway/install_result_test.go @@ -0,0 +1,32 @@ +package main + +import ( + "net/http" + "testing" +) + +func TestClassifyInstallUpsertErrorPreservesTypedBackendAvailabilityCodes(t *testing.T) { + err := &httpStatusError{ + method: http.MethodPost, + endpoint: "/internal/installations/upsert", + statusCode: http.StatusServiceUnavailable, + body: `{"error":"runtime_binding_unavailable"}`, + } + + if got := classifyInstallUpsertError(err); got != installResultCodeRuntimeBindingUnavailable { + t.Fatalf("expected runtime binding unavailable, got %q", got) + } +} + +func TestClassifyInstallUpsertErrorMapsLegacyOwnerRefUnresolvedPayloads(t *testing.T) { + err := &httpStatusError{ + method: http.MethodPost, + endpoint: "/internal/installations/upsert", + statusCode: http.StatusConflict, + body: `{"status":"unresolved","field":"ownerRef"}`, + } + + if got := classifyInstallUpsertError(err); got != installResultCodeExternalIdentityUnresolved { + t.Fatalf("expected external identity unresolved, got %q", got) + } +}