Skip to content

Commit d500c34

Browse files
committed
fix: harden slack management ui migration
1 parent ac47925 commit d500c34

3 files changed

Lines changed: 64 additions & 24 deletions

File tree

integrations/slack-gateway/gateway_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,28 @@ func TestOAuthCallbackRendersInstallTargetPickerWhenMultipleTargetsAvailable(t *
290290
if strings.Contains(rec.Body.String(), "xoxb-installed") {
291291
t.Fatalf("expected picker redirect to keep bot token encrypted, got %q", rec.Body.String())
292292
}
293+
294+
selectionReq := httptest.NewRequest(
295+
http.MethodGet,
296+
"/api/slack/install/selection?state="+url.QueryEscape(redirectURL.Query().Get("state")),
297+
nil,
298+
)
299+
selectionRec := httptest.NewRecorder()
300+
gateway.routes().ServeHTTP(selectionRec, selectionReq)
301+
302+
if selectionRec.Code != http.StatusOK {
303+
t.Fatalf("expected selection API response, got %d: %s", selectionRec.Code, selectionRec.Body.String())
304+
}
305+
if strings.Contains(selectionRec.Body.String(), "xoxb-installed") || strings.Contains(selectionRec.Body.String(), "botAccessToken") {
306+
t.Fatalf("selection API leaked bot token material: %s", selectionRec.Body.String())
307+
}
308+
var selectionPayload map[string]any
309+
if err := json.NewDecoder(selectionRec.Body).Decode(&selectionPayload); err != nil {
310+
t.Fatalf("decode selection API payload: %v", err)
311+
}
312+
if _, ok := selectionPayload["installation"]; ok {
313+
t.Fatalf("selection API should not expose pending installation payload: %#v", selectionPayload["installation"])
314+
}
293315
}
294316

295317
func TestWorkspaceManagementRequiresBrowserPrincipal(t *testing.T) {

integrations/slack-gateway/management_api.go

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,11 @@ type slackGatewayErrorResponse struct {
1313
}
1414

1515
type installSelectionResponse struct {
16-
Status string `json:"status"`
17-
State string `json:"state"`
18-
RequestID string `json:"requestId"`
19-
TeamID string `json:"teamId"`
20-
Installation slackInstallation `json:"installation"`
21-
Targets []backendInstallTarget `json:"targets"`
16+
Status string `json:"status"`
17+
State string `json:"state"`
18+
RequestID string `json:"requestId"`
19+
TeamID string `json:"teamId"`
20+
Targets []backendInstallTarget `json:"targets"`
2221
}
2322

2423
type installSelectionRequest struct {
@@ -102,12 +101,11 @@ func (g *slackGateway) handleInstallTargetSelectionAPIGet(w http.ResponseWriter,
102101
return
103102
}
104103
writeJSON(w, http.StatusOK, installSelectionResponse{
105-
Status: "resolved",
106-
State: state,
107-
RequestID: pendingInstall.RequestID,
108-
TeamID: pendingInstall.Installation.TeamID,
109-
Installation: pendingInstall.Installation,
110-
Targets: targets,
104+
Status: "resolved",
105+
State: state,
106+
RequestID: pendingInstall.RequestID,
107+
TeamID: pendingInstall.Installation.TeamID,
108+
Targets: targets,
111109
})
112110
}
113111

ui/src/pages/settings.tsx

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,16 @@ function teamID(installation: SlackManagedInstallation): string {
181181
return installation.route.externalTenantId;
182182
}
183183

184+
function hasAllowedAction(installation: SlackManagedInstallation, action: string): boolean {
185+
return (installation.allowedActions || []).some(
186+
(candidate) => candidate.trim().toLowerCase() === action.trim().toLowerCase(),
187+
);
188+
}
189+
190+
function installationIsDisconnected(installation: SlackManagedInstallation): boolean {
191+
return installation.state.trim().toLowerCase() === 'disconnected';
192+
}
193+
184194
function WorkspaceListPage() {
185195
const { value, error, loading, reload } = useAsyncValue(
186196
() => slackGatewayRequest<InstallationListResponse>('/api/slack/workspaces'),
@@ -218,6 +228,8 @@ function WorkspaceListPage() {
218228
<div className="overflow-hidden rounded-[var(--radius-lg)] border border-border">
219229
{installations.map((installation) => {
220230
const connection = primaryConnection(installation);
231+
const canDisconnect = hasAllowedAction(installation, 'disconnect');
232+
const canTest = !installationIsDisconnected(installation);
221233
return (
222234
<div
223235
key={installation.id || teamID(installation)}
@@ -249,17 +261,21 @@ function WorkspaceListPage() {
249261
>
250262
Target
251263
</Link>
252-
<Link
253-
to={`/settings/slack/workspaces/test?teamId=${encodeURIComponent(teamID(installation))}`}
254-
className={buttonVariants({ variant: 'outline' })}
255-
>
256-
<SendIcon aria-hidden="true" />
257-
Test
258-
</Link>
259-
<Button variant="destructive" onClick={() => disconnect(installation)}>
260-
<Trash2Icon aria-hidden="true" />
261-
Disconnect
262-
</Button>
264+
{canTest && (
265+
<Link
266+
to={`/settings/slack/workspaces/test?teamId=${encodeURIComponent(teamID(installation))}`}
267+
className={buttonVariants({ variant: 'outline' })}
268+
>
269+
<SendIcon aria-hidden="true" />
270+
Test
271+
</Link>
272+
)}
273+
{canDisconnect && (
274+
<Button variant="destructive" onClick={() => disconnect(installation)}>
275+
<Trash2Icon aria-hidden="true" />
276+
Disconnect
277+
</Button>
278+
)}
263279
</div>
264280
</div>
265281
);
@@ -368,6 +384,10 @@ function routeRequireMention(route: SlackManagedChannelRoute): boolean {
368384
return route.requireMention !== false;
369385
}
370386

387+
function routeEnabled(route: SlackManagedChannelRoute): boolean {
388+
return route.enabled !== false;
389+
}
390+
371391
function ConnectionSettingsPage() {
372392
const { installationId = '', connectionId = '' } = useParams();
373393
const loadPath = `/api/settings/channels/installations/${encodeURIComponent(
@@ -385,7 +405,7 @@ function ConnectionSettingsPage() {
385405

386406
useEffect(() => {
387407
if (value?.connection) {
388-
setRoutes([...(value.connection.routes || [])]);
408+
setRoutes([...(value.connection.routes || [])].filter(routeEnabled));
389409
}
390410
}, [value?.connection]);
391411

0 commit comments

Comments
 (0)