Skip to content

Commit c28678a

Browse files
osolmazosolmaz
committed
fix: scope pending slack installs by request
1 parent 29693dd commit c28678a

8 files changed

Lines changed: 224 additions & 32 deletions

File tree

‎integrations/slack-gateway/channel_settings_handlers.go‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,25 @@ func (g *slackGateway) matchChannelSettingsConnectionPath(
8787
r *http.Request,
8888
installations []backendManagedInstallation,
8989
relativePath string,
90+
) (backendManagedInstallation, backendManagedConnection, bool) {
91+
installation, connection, ok := matchManagedChannelSettingsConnection(
92+
installations,
93+
relativePath,
94+
)
95+
if !ok {
96+
http.NotFound(w, r)
97+
return backendManagedInstallation{}, backendManagedConnection{}, false
98+
}
99+
return installation, connection, true
100+
}
101+
102+
func matchManagedChannelSettingsConnection(
103+
installations []backendManagedInstallation,
104+
relativePath string,
90105
) (backendManagedInstallation, backendManagedConnection, bool) {
91106
trimmed := strings.Trim(strings.TrimPrefix(relativePath, "/settings/channels/"), "/")
92107
parts := strings.Split(trimmed, "/")
93108
if len(parts) != 4 || parts[0] != "installations" || parts[2] != "connections" {
94-
http.NotFound(w, r)
95109
return backendManagedInstallation{}, backendManagedConnection{}, false
96110
}
97111
installationID := strings.TrimSpace(parts[1])
@@ -102,12 +116,10 @@ func (g *slackGateway) matchChannelSettingsConnectionPath(
102116
}
103117
connection, found := managedConnectionByID(installation, connectionID)
104118
if !found {
105-
http.NotFound(w, r)
106119
return backendManagedInstallation{}, backendManagedConnection{}, false
107120
}
108121
return installation, connection, true
109122
}
110-
http.NotFound(w, r)
111123
return backendManagedInstallation{}, backendManagedConnection{}, false
112124
}
113125

‎integrations/slack-gateway/gateway_test.go‎

Lines changed: 154 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -284,12 +284,16 @@ func TestOAuthCallbackRendersInstallTargetPickerWhenMultipleTargetsAvailable(t *
284284
if redirectURL.Path != "/settings/slack/install/select" {
285285
t.Fatalf("expected React picker route, got %q", redirectURL.Path)
286286
}
287-
if redirectURL.RawQuery != "" {
287+
requestID := redirectURL.Query().Get("requestId")
288+
if requestID == "" {
289+
t.Fatalf("expected picker redirect to include requestId, got %q", redirectURL.RawQuery)
290+
}
291+
if strings.Contains(redirectURL.RawQuery, "state=") {
288292
t.Fatalf("expected picker redirect without pending state query, got %q", redirectURL.RawQuery)
289293
}
290294
var pendingCookie *http.Cookie
291295
for _, cookie := range rec.Result().Cookies() {
292-
if cookie.Name == pendingInstallCookieName {
296+
if cookie.Name == pendingInstallCookieNameForRequest(requestID) {
293297
pendingCookie = cookie
294298
break
295299
}
@@ -309,7 +313,7 @@ func TestOAuthCallbackRendersInstallTargetPickerWhenMultipleTargetsAvailable(t *
309313

310314
selectionReq := httptest.NewRequest(
311315
http.MethodGet,
312-
"/api/slack/install/selection",
316+
"/api/slack/install/selection?requestId="+url.QueryEscape(requestID),
313317
nil,
314318
)
315319
selectionReq.AddCookie(pendingCookie)
@@ -331,6 +335,90 @@ func TestOAuthCallbackRendersInstallTargetPickerWhenMultipleTargetsAvailable(t *
331335
}
332336
}
333337

338+
func TestInstallTargetSelectionAPIUsesRequestScopedPendingCookie(t *testing.T) {
339+
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
340+
if r.URL.Path != "/internal/v2/spritz/channel-install-targets/list" {
341+
t.Fatalf("unexpected backend path %s", r.URL.Path)
342+
}
343+
writeJSON(w, http.StatusOK, map[string]any{
344+
"status": "resolved",
345+
"targets": []map[string]any{
346+
{
347+
"id": "ag_workspace",
348+
"profile": map[string]any{
349+
"name": "Workspace Helper",
350+
},
351+
"presetInputs": map[string]any{
352+
"agentId": "ag_workspace",
353+
},
354+
},
355+
},
356+
})
357+
}))
358+
defer backend.Close()
359+
360+
gateway := newSlackGateway(config{
361+
BackendBaseURL: backend.URL,
362+
BackendFastAPIBaseURL: backend.URL,
363+
BackendInternalToken: "backend-internal-token",
364+
OAuthStateSecret: "oauth-state-secret",
365+
PrincipalID: "shared-slack-gateway",
366+
HTTPTimeout: 5 * time.Second,
367+
}, slog.New(slog.NewTextHandler(io.Discard, nil)))
368+
369+
pendingStateA, err := gateway.state.generatePendingInstall(pendingInstallState{
370+
RequestID: "install-request-a",
371+
Installation: slackInstallation{
372+
TeamID: "T_workspace_a",
373+
InstallingUserID: "U_installer",
374+
BotAccessToken: "xoxb-installed-a",
375+
BotUserID: "U_bot",
376+
APIAppID: "A_app_1",
377+
},
378+
})
379+
if err != nil {
380+
t.Fatalf("generate first pending install state failed: %v", err)
381+
}
382+
pendingStateB, err := gateway.state.generatePendingInstall(pendingInstallState{
383+
RequestID: "install-request-b",
384+
Installation: slackInstallation{
385+
TeamID: "T_workspace_b",
386+
InstallingUserID: "U_installer",
387+
BotAccessToken: "xoxb-installed-b",
388+
BotUserID: "U_bot",
389+
APIAppID: "A_app_1",
390+
},
391+
})
392+
if err != nil {
393+
t.Fatalf("generate second pending install state failed: %v", err)
394+
}
395+
396+
req := httptest.NewRequest(http.MethodGet, "/api/slack/install/selection?requestId=install-request-a", nil)
397+
req.AddCookie(&http.Cookie{
398+
Name: pendingInstallCookieNameForRequest("install-request-a"),
399+
Value: pendingStateA,
400+
Path: "/api/slack/install/selection",
401+
})
402+
req.AddCookie(&http.Cookie{
403+
Name: pendingInstallCookieNameForRequest("install-request-b"),
404+
Value: pendingStateB,
405+
Path: "/api/slack/install/selection",
406+
})
407+
rec := httptest.NewRecorder()
408+
gateway.routes().ServeHTTP(rec, req)
409+
410+
if rec.Code != http.StatusOK {
411+
t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String())
412+
}
413+
var payload map[string]any
414+
if err := json.NewDecoder(rec.Body).Decode(&payload); err != nil {
415+
t.Fatalf("decode selection payload: %v", err)
416+
}
417+
if payload["requestId"] != "install-request-a" || payload["teamId"] != "T_workspace_a" {
418+
t.Fatalf("expected first pending install, got %#v", payload)
419+
}
420+
}
421+
334422
func TestWorkspaceManagementRequiresBrowserPrincipal(t *testing.T) {
335423
gateway := newSlackGateway(config{
336424
BackendFastAPIBaseURL: "https://backend.example.test",
@@ -779,6 +867,67 @@ func TestChannelSettingsAPIUpdatePostsRoutePolicies(t *testing.T) {
779867
}
780868
}
781869

870+
func TestChannelSettingsAPIMissingConnectionReturnsJSON(t *testing.T) {
871+
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
872+
if r.URL.Path != "/internal/v2/spritz/channel-installations/list" {
873+
t.Fatalf("unexpected backend path %s", r.URL.Path)
874+
}
875+
writeJSON(w, http.StatusOK, map[string]any{
876+
"status": "resolved",
877+
"installations": []map[string]any{
878+
{
879+
"id": "chinst_example",
880+
"route": map[string]any{
881+
"principalId": "shared-slack-gateway",
882+
"provider": "slack",
883+
"externalScopeType": "workspace",
884+
"externalTenantId": "T_workspace_1",
885+
},
886+
"state": "ready",
887+
"connections": []map[string]any{
888+
{
889+
"id": "chconn_example",
890+
"isDefault": true,
891+
"state": "ready",
892+
},
893+
},
894+
},
895+
},
896+
})
897+
}))
898+
defer backend.Close()
899+
900+
gateway := newSlackGateway(config{
901+
BackendFastAPIBaseURL: backend.URL,
902+
BackendInternalToken: "backend-internal-token",
903+
PrincipalID: "shared-slack-gateway",
904+
HTTPTimeout: 5 * time.Second,
905+
}, slog.New(slog.NewTextHandler(io.Discard, nil)))
906+
907+
req := httptest.NewRequest(
908+
http.MethodGet,
909+
"/api/settings/channels/installations/chinst_example/connections/missing",
910+
nil,
911+
)
912+
req.Header.Set("X-Spritz-User-Id", "user-1")
913+
rec := httptest.NewRecorder()
914+
gateway.routes().ServeHTTP(rec, req)
915+
916+
if rec.Code != http.StatusNotFound {
917+
t.Fatalf("expected 404, got %d: %s", rec.Code, rec.Body.String())
918+
}
919+
if contentType := rec.Header().Get("Content-Type"); !strings.Contains(contentType, "application/json") {
920+
t.Fatalf("expected JSON content type, got %q", contentType)
921+
}
922+
var payload map[string]any
923+
if err := json.NewDecoder(rec.Body).Decode(&payload); err != nil {
924+
t.Fatalf("decode error payload: %v", err)
925+
}
926+
if payload["status"] != "error" || payload["message"] != "connection not found" {
927+
t.Fatalf("expected structured missing connection error, got %#v", payload)
928+
}
929+
}
930+
782931
func TestWorkspaceManagementAcceptsConfiguredBrowserAuthHeaders(t *testing.T) {
783932
t.Setenv("SPRITZ_AUTH_HEADER_ID", "X-Forwarded-User")
784933
t.Setenv("SPRITZ_AUTH_HEADER_EMAIL", "X-Forwarded-Email")
@@ -1512,7 +1661,7 @@ func TestInstallTargetSelectionAPIPreservesClassifiedUpsertFailure(t *testing.T)
15121661
req := httptest.NewRequest(http.MethodPost, "/api/slack/install/selection", bytes.NewReader(requestBody))
15131662
req.Header.Set("Content-Type", "application/json")
15141663
req.AddCookie(&http.Cookie{
1515-
Name: pendingInstallCookieName,
1664+
Name: pendingInstallCookieNameForRequest("install-request-1"),
15161665
Value: pendingState,
15171666
Path: "/api/slack/install/selection",
15181667
})
@@ -1597,7 +1746,7 @@ func TestInstallTargetSelectionAPIRejectsStaleRequestID(t *testing.T) {
15971746
req := httptest.NewRequest(http.MethodPost, "/api/slack/install/selection", bytes.NewReader(requestBody))
15981747
req.Header.Set("Content-Type", "application/json")
15991748
req.AddCookie(&http.Cookie{
1600-
Name: pendingInstallCookieName,
1749+
Name: pendingInstallCookieNameForRequest("install-request-current"),
16011750
Value: pendingState,
16021751
Path: "/api/slack/install/selection",
16031752
})

‎integrations/slack-gateway/management_api.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,11 @@ func (g *slackGateway) handleInstallTargetSelectionAPI(w http.ResponseWriter, r
8686
}
8787

8888
func (g *slackGateway) handleInstallTargetSelectionAPIGet(w http.ResponseWriter, r *http.Request) {
89-
state := g.pendingInstallStateFromRequest(r, "")
89+
requestID := strings.TrimSpace(r.URL.Query().Get("requestId"))
90+
state := g.pendingInstallStateFromRequest(r, requestID, "")
9091
pendingInstall, err := g.state.parsePendingInstall(state)
9192
if err != nil {
92-
g.clearPendingInstallCookie(w, r)
93+
g.clearPendingInstallCookie(w, r, requestID)
9394
writeAPIError(w, http.StatusBadRequest, "install state is invalid or expired")
9495
return
9596
}
@@ -119,14 +120,14 @@ func (g *slackGateway) handleInstallTargetSelectionAPIPost(w http.ResponseWriter
119120
writeAPIError(w, http.StatusBadRequest, "invalid request body")
120121
return
121122
}
122-
state := g.pendingInstallStateFromRequest(r, body.State)
123+
bodyRequestID := strings.TrimSpace(body.RequestID)
124+
state := g.pendingInstallStateFromRequest(r, bodyRequestID, body.State)
123125
pendingInstall, err := g.state.parsePendingInstall(state)
124126
if err != nil {
125-
g.clearPendingInstallCookie(w, r)
127+
g.clearPendingInstallCookie(w, r, bodyRequestID)
126128
writeAPIError(w, http.StatusBadRequest, "install state is invalid or expired")
127129
return
128130
}
129-
bodyRequestID := strings.TrimSpace(body.RequestID)
130131
if bodyRequestID != "" && bodyRequestID != pendingInstall.RequestID {
131132
writeAPIError(w, http.StatusBadRequest, "install request is stale")
132133
return
@@ -145,7 +146,7 @@ func (g *slackGateway) handleInstallTargetSelectionAPIPost(w http.ResponseWriter
145146
"team_id", installation.TeamID,
146147
"request_id", requestID,
147148
)
148-
g.clearPendingInstallCookie(w, r)
149+
g.clearPendingInstallCookie(w, r, requestID)
149150
g.writeInstallResultAPI(w, http.StatusOK, installResult{
150151
Status: installResultStatusError,
151152
Code: classifyInstallUpsertError(err),
@@ -156,7 +157,7 @@ func (g *slackGateway) handleInstallTargetSelectionAPIPost(w http.ResponseWriter
156157
})
157158
return
158159
}
159-
g.clearPendingInstallCookie(w, r)
160+
g.clearPendingInstallCookie(w, r, requestID)
160161
writeAPIJSON(w, http.StatusOK, map[string]any{
161162
"status": "installed",
162163
"requestId": requestID,
@@ -469,13 +470,12 @@ func (g *slackGateway) handleChannelSettingsAPI(w http.ResponseWriter, r *http.R
469470
return
470471
}
471472

472-
installation, connection, found := g.matchChannelSettingsConnectionPath(
473-
w,
474-
r,
473+
installation, connection, found := matchManagedChannelSettingsConnection(
475474
installations,
476475
relativePath,
477476
)
478477
if !found {
478+
writeAPIError(w, http.StatusNotFound, "connection not found")
479479
return
480480
}
481481
switch r.Method {

‎integrations/slack-gateway/pending_install_cookie.go‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,26 @@ import (
88

99
const pendingInstallCookieName = "spritz_slack_pending_install"
1010

11+
func pendingInstallCookieNameForRequest(requestID string) string {
12+
requestID = strings.TrimSpace(requestID)
13+
if requestID == "" {
14+
return pendingInstallCookieName
15+
}
16+
var suffix strings.Builder
17+
for _, char := range requestID {
18+
if char >= 'a' && char <= 'z' ||
19+
char >= 'A' && char <= 'Z' ||
20+
char >= '0' && char <= '9' ||
21+
char == '-' || char == '_' {
22+
suffix.WriteRune(char)
23+
}
24+
}
25+
if suffix.Len() == 0 {
26+
return pendingInstallCookieName
27+
}
28+
return pendingInstallCookieName + "_" + suffix.String()
29+
}
30+
1131
func (g *slackGateway) pendingInstallCookiePath() string {
1232
return g.publicPathPrefix() + "/api/slack/install/selection"
1333
}
@@ -24,9 +44,9 @@ func (g *slackGateway) pendingInstallCookieSecure(r *http.Request) bool {
2444
return strings.HasPrefix(strings.ToLower(strings.TrimSpace(g.cfg.PublicURL)), "https://")
2545
}
2646

27-
func (g *slackGateway) setPendingInstallCookie(w http.ResponseWriter, r *http.Request, state string) {
47+
func (g *slackGateway) setPendingInstallCookie(w http.ResponseWriter, r *http.Request, requestID, state string) {
2848
http.SetCookie(w, &http.Cookie{
29-
Name: pendingInstallCookieName,
49+
Name: pendingInstallCookieNameForRequest(requestID),
3050
Value: strings.TrimSpace(state),
3151
Path: g.pendingInstallCookiePath(),
3252
Expires: time.Now().UTC().Add(g.state.ttl),
@@ -37,9 +57,9 @@ func (g *slackGateway) setPendingInstallCookie(w http.ResponseWriter, r *http.Re
3757
})
3858
}
3959

40-
func (g *slackGateway) clearPendingInstallCookie(w http.ResponseWriter, r *http.Request) {
60+
func (g *slackGateway) clearPendingInstallCookie(w http.ResponseWriter, r *http.Request, requestID string) {
4161
http.SetCookie(w, &http.Cookie{
42-
Name: pendingInstallCookieName,
62+
Name: pendingInstallCookieNameForRequest(requestID),
4363
Value: "",
4464
Path: g.pendingInstallCookiePath(),
4565
Expires: time.Unix(0, 0).UTC(),
@@ -50,14 +70,14 @@ func (g *slackGateway) clearPendingInstallCookie(w http.ResponseWriter, r *http.
5070
})
5171
}
5272

53-
func (g *slackGateway) pendingInstallStateFromRequest(r *http.Request, explicitState string) string {
73+
func (g *slackGateway) pendingInstallStateFromRequest(r *http.Request, requestID, explicitState string) string {
5474
if state := strings.TrimSpace(explicitState); state != "" {
5575
return state
5676
}
5777
if r == nil {
5878
return ""
5979
}
60-
cookie, err := r.Cookie(pendingInstallCookieName)
80+
cookie, err := r.Cookie(pendingInstallCookieNameForRequest(requestID))
6181
if err != nil {
6282
return ""
6383
}

‎integrations/slack-gateway/react_routes.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,14 @@ func reactSlackInstallResultPath() string {
1010
return "/settings/slack/install/result"
1111
}
1212

13-
func reactSlackInstallSelectPath() string {
14-
return "/settings/slack/install/select"
13+
func reactSlackInstallSelectPath(requestID string) string {
14+
target := url.URL{Path: "/settings/slack/install/select"}
15+
if requestID := strings.TrimSpace(requestID); requestID != "" {
16+
query := target.Query()
17+
query.Set("requestId", requestID)
18+
target.RawQuery = query.Encode()
19+
}
20+
return target.String()
1521
}
1622

1723
func reactSlackWorkspacesPath(query url.Values) string {

0 commit comments

Comments
 (0)