mirror of
https://github.com/andrewyng/openworker.git
synced 2026-09-03 04:49:26 +00:00
Merge pull request #115 from andrewyng/rpSlackApprovalOwners
Harden Slack approval handling
This commit is contained in:
@@ -243,7 +243,9 @@ class SlackAdapter(BasePlatformAdapter):
|
||||
chat_id=str(channel),
|
||||
message_id=ts,
|
||||
value=str(value),
|
||||
user_id=user.get("id"),
|
||||
user_name=user.get("username") or user.get("name"),
|
||||
response_url=body.get("response_url"),
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
@@ -119,14 +119,20 @@ MessageHandler = Callable[[MessageEvent], Awaitable[None]]
|
||||
|
||||
@dataclass
|
||||
class InteractionEvent:
|
||||
"""A button click on an interactive prompt. `value` is the opaque button value (see
|
||||
`interactions.decode`); `user_name` is who clicked, for the message update."""
|
||||
"""A button click on an interactive prompt.
|
||||
|
||||
Stable actor/workspace ids are security inputs; display names are presentation only.
|
||||
`response_url` is Slack's short-lived reply capability for a private rejection notice.
|
||||
"""
|
||||
|
||||
platform: str
|
||||
chat_id: str
|
||||
message_id: Optional[str] # the clicked message's id/ts (to update it)
|
||||
value: str
|
||||
user_id: Optional[str] = None
|
||||
user_name: Optional[str] = None
|
||||
team_id: Optional[str] = None
|
||||
response_url: Optional[str] = None
|
||||
|
||||
|
||||
InteractionHandler = Callable[[InteractionEvent], Awaitable[None]]
|
||||
|
||||
@@ -9,15 +9,19 @@ super-agent runner, wired in the next increment). Outbound replies go through th
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from asyncio import to_thread
|
||||
from collections import OrderedDict
|
||||
from typing import Callable, Optional
|
||||
from urllib.parse import urlparse
|
||||
|
||||
from ..secrets import SecretStore
|
||||
from .base import (
|
||||
BasePlatformAdapter,
|
||||
InteractionEvent,
|
||||
MessageEvent,
|
||||
MessageHandler,
|
||||
SendResult,
|
||||
SessionSource,
|
||||
parse_target,
|
||||
)
|
||||
from .config import ConnectorSettings, is_authorized, load_settings
|
||||
@@ -69,10 +73,52 @@ class Gateway:
|
||||
adapter.set_interaction_handler(self._on_interaction)
|
||||
self._adapters[adapter.platform] = adapter
|
||||
|
||||
async def _on_interaction(self, event) -> None:
|
||||
async def _on_interaction(self, event: InteractionEvent) -> None:
|
||||
source = SessionSource(
|
||||
platform=event.platform,
|
||||
chat_id=event.chat_id,
|
||||
user_id=event.user_id,
|
||||
user_name=event.user_name,
|
||||
chat_type="channel",
|
||||
team_id=event.team_id,
|
||||
)
|
||||
settings = self.settings.get(event.platform)
|
||||
if settings is None or not is_authorized(settings, source):
|
||||
logger.info("rejecting unauthorized interaction from %s", source.label())
|
||||
await self.reject_interaction(event)
|
||||
return
|
||||
if self._interaction_handler is not None:
|
||||
await self._interaction_handler(event)
|
||||
|
||||
async def reject_interaction(
|
||||
self,
|
||||
event: InteractionEvent,
|
||||
text: str = "Only a designated approval owner can respond to this request.",
|
||||
) -> None:
|
||||
"""Best-effort private feedback for a rejected Slack button click."""
|
||||
response_url = str(event.response_url or "")
|
||||
parsed = urlparse(response_url)
|
||||
if (
|
||||
event.platform != "slack"
|
||||
or parsed.scheme != "https"
|
||||
or parsed.hostname not in {"hooks.slack.com", "hooks.slack-gov.com"}
|
||||
):
|
||||
return
|
||||
|
||||
def _post() -> None:
|
||||
import httpx
|
||||
|
||||
try:
|
||||
httpx.post(
|
||||
response_url,
|
||||
json={"response_type": "ephemeral", "text": text},
|
||||
timeout=10,
|
||||
)
|
||||
except Exception:
|
||||
logger.debug("Slack ephemeral interaction response failed", exc_info=True)
|
||||
|
||||
await to_thread(_post)
|
||||
|
||||
async def _on_inbound(self, event: MessageEvent) -> None:
|
||||
self._record_recent(event) # capture identity even from unauthorized senders
|
||||
settings = self.settings.get(event.source.platform)
|
||||
|
||||
@@ -358,7 +358,10 @@ class SlackRelayAdapter(BasePlatformAdapter):
|
||||
chat_id=qualify(team_id, channel),
|
||||
message_id=ts,
|
||||
value=str(value),
|
||||
user_id=user.get("id"),
|
||||
user_name=user.get("username") or user.get("name"),
|
||||
team_id=team_id,
|
||||
response_url=interaction.get("response_url"),
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
@@ -93,6 +93,9 @@ def connector_list(secrets: SecretStore) -> list[dict[str, Any]]:
|
||||
"enabled": bool(profile.get("enabled", True)) and connected,
|
||||
# The actual allow-list (the GUI manages it inline); was a bare count.
|
||||
"allowed_users": list(profile.get("allowed_users") or []),
|
||||
# Manual Socket Mode only: explicitly selected humans who may resolve
|
||||
# consequential Inbox prompts. Relay uses its OAuth installer instead.
|
||||
"approval_owner_ids": list(profile.get("approval_owner_ids") or []),
|
||||
"tools": tool_dicts(secrets, d.name),
|
||||
"experimental": d.experimental,
|
||||
"risk_notice": d.risk_notice,
|
||||
@@ -107,6 +110,10 @@ def connector_list(secrets: SecretStore) -> list[dict[str, Any]]:
|
||||
# Managed relay is multi-workspace: each `slack:team:*` profile is one
|
||||
# connected workspace with its OWN allow-list (ids are workspace-scoped).
|
||||
entry["workspaces"] = _slack_workspaces(secrets)
|
||||
if profile.get("mode") == "relay":
|
||||
# Dormant Manual-mode owners may remain beside preserved Socket
|
||||
# Mode credentials; they never authorize a bare Relay target.
|
||||
entry["approval_owner_ids"] = []
|
||||
if d.name == "gmail":
|
||||
# Multi-account: each `gmail:account:*` profile is one mailbox; the
|
||||
# :default profile is just the default pointer + privacy filters.
|
||||
@@ -185,6 +192,11 @@ def _slack_workspaces(secrets: SecretStore) -> list[dict[str, Any]]:
|
||||
"domain": profile.get("domain") or "",
|
||||
"allowed_users": list(profile.get("allowed_users") or []),
|
||||
"allow_all": bool(profile.get("allow_all")),
|
||||
# Relay approvals are installer-only. Keep the list-shaped API aligned
|
||||
# with Manual mode without creating a second editable relay role.
|
||||
"approval_owner_ids": (
|
||||
[profile["slack_user_id"]] if profile.get("slack_user_id") else []
|
||||
),
|
||||
# Who installed (authed_user) — the GUI marks their chip "you" and
|
||||
# keys the post-connect card's "your mentions get through" line.
|
||||
"installer_user_id": profile.get("slack_user_id") or "",
|
||||
@@ -353,6 +365,10 @@ def connect_connector(
|
||||
profile: dict[str, Any] = {"type": profile_type, "enabled": True, **token_creds}
|
||||
if any(f.key == "allowed_users" for f in d.fields):
|
||||
profile["allowed_users"] = allowed
|
||||
if name == "slack" and existing.get("approval_owner_ids"):
|
||||
# Re-pasting manual Socket Mode tokens must not erase the locally selected
|
||||
# approval owners.
|
||||
profile["approval_owner_ids"] = list(existing["approval_owner_ids"])
|
||||
if identity:
|
||||
profile["account"] = identity
|
||||
if d.account_field:
|
||||
|
||||
+15
-2
@@ -371,12 +371,11 @@ def create_app(manager: SessionManager) -> FastAPI:
|
||||
name = str(body.get("name", "")).strip()
|
||||
if not name:
|
||||
return {"ok": False, "error": "binding needs a `name`"}
|
||||
manager.inbox_routing.set_binding(
|
||||
return manager.set_inbox_binding(
|
||||
name,
|
||||
channel=body.get("channel") or None,
|
||||
target=str(body.get("target", "")),
|
||||
)
|
||||
return {"ok": True, "bindings": manager.inbox_routing.bindings()}
|
||||
|
||||
@app.get("/v1/sessions/{session_id}/unattended")
|
||||
def get_unattended(session_id: str) -> dict[str, Any]:
|
||||
@@ -1245,6 +1244,20 @@ def create_app(manager: SessionManager) -> FastAPI:
|
||||
name, str(body.get("user_id", "")), str(body.get("team_id", "")) or None
|
||||
)
|
||||
|
||||
@app.post("/v1/connectors/slack/approval-owners/add")
|
||||
def slack_approval_owner_add(body: dict) -> dict[str, Any]:
|
||||
return manager.set_slack_approval_owner(
|
||||
str(body.get("user_id", "")),
|
||||
add=True,
|
||||
display_name=str(body.get("name", "")),
|
||||
)
|
||||
|
||||
@app.post("/v1/connectors/slack/approval-owners/remove")
|
||||
def slack_approval_owner_remove(body: dict) -> dict[str, Any]:
|
||||
return manager.set_slack_approval_owner(
|
||||
str(body.get("user_id", "")), add=False
|
||||
)
|
||||
|
||||
# -- audit / browser observability ------------------------------------------
|
||||
@app.get("/v1/audit")
|
||||
def audit_list(
|
||||
|
||||
+191
-3
@@ -51,6 +51,7 @@ from ..connectors import (
|
||||
load_settings,
|
||||
make_adapter,
|
||||
set_experimental_enabled,
|
||||
slack_split,
|
||||
update_connector_tools,
|
||||
)
|
||||
from ..connectors.browser_automation import (
|
||||
@@ -1126,11 +1127,19 @@ class SessionManager:
|
||||
u: self._people.get(f"{c['name']}:{u}")
|
||||
for u in (c.get("allowed_users") or [])
|
||||
}
|
||||
c["approval_owner_names"] = {
|
||||
u: self._people.get(f"{c['name']}:{u}")
|
||||
for u in (c.get("approval_owner_ids") or [])
|
||||
}
|
||||
for w in c.get("workspaces") or []:
|
||||
w["allowed_user_names"] = {
|
||||
u: self._people.get(f"{c['name']}:{u}")
|
||||
for u in (w.get("allowed_users") or [])
|
||||
}
|
||||
w["approval_owner_names"] = {
|
||||
u: self._people.get(f"{c['name']}:{u}")
|
||||
for u in (w.get("approval_owner_ids") or [])
|
||||
}
|
||||
return connectors
|
||||
|
||||
def connect_connector(
|
||||
@@ -1935,8 +1944,143 @@ class SessionManager:
|
||||
def disallow_user(
|
||||
self, name: str, user_id: str, team_id: Optional[str] = None
|
||||
) -> dict[str, Any]:
|
||||
if name == "slack" and user_id in self.slack_approval_owner_ids(team_id):
|
||||
return {
|
||||
"ok": False,
|
||||
"error": "Remove this person as an approval owner first.",
|
||||
}
|
||||
return self._set_allowed(name, user_id, team_id=team_id, add=False)
|
||||
|
||||
def slack_approval_owner_ids(self, team_id: Optional[str] = None) -> set[str]:
|
||||
"""Stable Slack user ids allowed to resolve consequential Inbox prompts.
|
||||
|
||||
Managed relay installs are installer-owned. Manual Socket Mode has no
|
||||
human OAuth identity, so its owners are selected explicitly.
|
||||
"""
|
||||
key = f"slack:team:{team_id}" if team_id else "slack:default"
|
||||
profile = self.secrets.get(key) or {}
|
||||
if team_id:
|
||||
installer = str(profile.get("slack_user_id") or "").strip()
|
||||
return {installer} if installer else set()
|
||||
if profile.get("mode") == "relay":
|
||||
return set()
|
||||
return {
|
||||
str(user_id).strip()
|
||||
for user_id in (profile.get("approval_owner_ids") or [])
|
||||
if str(user_id).strip()
|
||||
}
|
||||
|
||||
def set_slack_approval_owner(
|
||||
self, user_id: str, *, add: bool, display_name: str = ""
|
||||
) -> dict[str, Any]:
|
||||
"""Edit Manual Socket Mode approval owners.
|
||||
|
||||
Owner status implies inbound permission. Relay ownership is derived from
|
||||
the OAuth installer and is intentionally not editable here.
|
||||
"""
|
||||
user_id = str(user_id).strip()
|
||||
if not user_id:
|
||||
return {"ok": False, "error": "user_id required"}
|
||||
profile = self.secrets.get("slack:default")
|
||||
if not profile:
|
||||
return {"ok": False, "error": "Slack is not connected in Manual mode."}
|
||||
if profile.get("mode") == "relay" or profile.get("managed"):
|
||||
return {
|
||||
"ok": False,
|
||||
"error": "Relay approval ownership is set by the Slack installer.",
|
||||
}
|
||||
|
||||
owners = self.slack_approval_owner_ids()
|
||||
if add:
|
||||
owners.add(user_id)
|
||||
else:
|
||||
owners.discard(user_id)
|
||||
if not owners and self._has_manual_slack_inbox_binding():
|
||||
return {
|
||||
"ok": False,
|
||||
"error": (
|
||||
"Choose another approval owner before removing the last one "
|
||||
"while Slack Inbox routing is active."
|
||||
),
|
||||
}
|
||||
profile["approval_owner_ids"] = sorted(owners)
|
||||
if add:
|
||||
allowed = set(profile.get("allowed_users") or [])
|
||||
allowed.add(user_id)
|
||||
profile["allowed_users"] = sorted(allowed)
|
||||
self.secrets.put("slack:default", profile)
|
||||
if display_name:
|
||||
self._note_person("slack", user_id, display_name)
|
||||
if self.gateway is not None and "slack" in self.gateway.settings:
|
||||
self.gateway.settings["slack"].allowed_users = set(
|
||||
profile.get("allowed_users") or []
|
||||
)
|
||||
return {
|
||||
"ok": True,
|
||||
"approval_owner_ids": sorted(owners),
|
||||
"allowed_users": list(profile.get("allowed_users") or []),
|
||||
}
|
||||
|
||||
def _has_manual_slack_inbox_binding(self) -> bool:
|
||||
for raw in self.inbox_routing.bindings():
|
||||
if raw.get("channel") != "slack":
|
||||
continue
|
||||
team_id, _ = slack_split(str(raw.get("target") or ""))
|
||||
if team_id is None:
|
||||
return True
|
||||
return False
|
||||
|
||||
def _slack_actor_owns_item(
|
||||
self,
|
||||
item,
|
||||
*,
|
||||
actor_id: str,
|
||||
chat_id: str,
|
||||
team_id: Optional[str],
|
||||
) -> bool:
|
||||
"""Authorize a Slack resolution against both its owner and delivery binding."""
|
||||
event_team, event_channel = slack_split(chat_id)
|
||||
event_team = team_id or event_team
|
||||
binding = self.inbox_routing.binding_for(item.inbox)
|
||||
owner_team = event_team
|
||||
if binding.channel == "slack":
|
||||
owner_team, bound_channel = slack_split(binding.target)
|
||||
if owner_team != event_team or bound_channel != event_channel:
|
||||
return False
|
||||
return bool(actor_id) and actor_id in self.slack_approval_owner_ids(owner_team)
|
||||
|
||||
def set_inbox_binding(
|
||||
self, name: str, *, channel: Optional[str], target: str
|
||||
) -> dict[str, Any]:
|
||||
"""Persist an Inbox transport after validating its approval identity."""
|
||||
channel = str(channel or "").strip() or None
|
||||
target = str(target or "").strip()
|
||||
if channel and not target:
|
||||
return {"ok": False, "error": "Choose a destination channel."}
|
||||
if channel == "slack":
|
||||
settings = load_settings(self.secrets).get("slack")
|
||||
if settings is None or not settings.enabled:
|
||||
return {"ok": False, "error": "Slack is not connected."}
|
||||
team_id, destination = slack_split(target)
|
||||
if not destination:
|
||||
return {"ok": False, "error": "Choose a destination channel."}
|
||||
key = f"slack:team:{team_id}" if team_id else "slack:default"
|
||||
if not self.secrets.get(key):
|
||||
return {
|
||||
"ok": False,
|
||||
"error": "That Slack workspace is not connected.",
|
||||
}
|
||||
if not self.slack_approval_owner_ids(team_id):
|
||||
return {
|
||||
"ok": False,
|
||||
"error": (
|
||||
"Choose at least one approval owner in Slack settings before "
|
||||
"routing Inbox requests there."
|
||||
),
|
||||
}
|
||||
self.inbox_routing.set_binding(name, channel=channel, target=target)
|
||||
return {"ok": True, "bindings": self.inbox_routing.bindings()}
|
||||
|
||||
def _set_allowed(
|
||||
self, name: str, user_id: str, *, team_id: Optional[str] = None, add: bool
|
||||
) -> dict[str, Any]:
|
||||
@@ -2458,6 +2602,12 @@ class SessionManager:
|
||||
binding = self.inbox_routing.binding_for(item.inbox)
|
||||
if not (binding.channel and self.gateway is not None):
|
||||
return
|
||||
if binding.channel == "slack":
|
||||
team_id, _ = slack_split(binding.target)
|
||||
# Legacy bindings may predate approval ownership. Keep the item
|
||||
# available in-app, but never mirror it to an ownerless channel.
|
||||
if not self.slack_approval_owner_ids(team_id):
|
||||
return
|
||||
target = f"{binding.channel}:{binding.target}"
|
||||
body = "\n".join(p for p in (item.title, item.body) if p).strip()
|
||||
buttons = buttons_for(item)
|
||||
@@ -2484,10 +2634,29 @@ class SessionManager:
|
||||
return
|
||||
item_id, resolution = decoded
|
||||
item = self.inbox.get(item_id)
|
||||
if item is None:
|
||||
return
|
||||
protected_kinds = {"approval", "directory", "plan"}
|
||||
if (
|
||||
getattr(event, "platform", "") == "slack"
|
||||
and item.kind in protected_kinds
|
||||
):
|
||||
actor_id = str(getattr(event, "user_id", "") or "")
|
||||
if not self._slack_actor_owns_item(
|
||||
item,
|
||||
actor_id=actor_id,
|
||||
chat_id=getattr(event, "chat_id", "") or "",
|
||||
team_id=getattr(event, "team_id", None),
|
||||
):
|
||||
if self.gateway is not None:
|
||||
await self.gateway.reject_interaction(event)
|
||||
return
|
||||
already = item is not None and item.state != "pending"
|
||||
await self.resolve_inbox(item_id, resolution)
|
||||
resolved = await self.resolve_inbox(item_id, resolution)
|
||||
if not resolved and not already:
|
||||
return
|
||||
who = getattr(event, "user_name", None) or "someone"
|
||||
title = item.title if item is not None else "Prompt"
|
||||
title = item.title
|
||||
outcome = "already resolved" if already else f"“{resolution}” — by {who}"
|
||||
if self.gateway is not None and getattr(event, "message_id", None):
|
||||
try:
|
||||
@@ -2508,7 +2677,26 @@ class SessionManager:
|
||||
from ..inbox_routing import resolve_from_reply
|
||||
|
||||
text = getattr(event, "text", "") or ""
|
||||
return resolve_from_reply(text, self.inbox.resolve) is not None
|
||||
|
||||
def _resolve(item_id: str, resolution: str) -> bool:
|
||||
item = self.inbox.get(item_id)
|
||||
if item is None:
|
||||
return False
|
||||
if (
|
||||
getattr(event.source, "platform", "") == "slack"
|
||||
and item.kind in {"approval", "directory", "plan"}
|
||||
):
|
||||
actor_id = str(getattr(event.source, "user_id", "") or "")
|
||||
if not self._slack_actor_owns_item(
|
||||
item,
|
||||
actor_id=actor_id,
|
||||
chat_id=getattr(event.source, "chat_id", "") or "",
|
||||
team_id=getattr(event.source, "team_id", None),
|
||||
):
|
||||
return False
|
||||
return self.inbox.resolve(item_id, resolution)
|
||||
|
||||
return resolve_from_reply(text, _resolve) is not None
|
||||
|
||||
# -- self-wake resumption ---------------------------------------------------
|
||||
async def resume_due_wakes(self) -> int:
|
||||
|
||||
@@ -357,11 +357,12 @@ export async function mockApi(page: import("@playwright/test").Page) {
|
||||
mode: "relay" as "" | "relay",
|
||||
account: "deeplearning.ai",
|
||||
allowed_users: [] as string[], // flat list (manual Socket Mode only)
|
||||
approval_owner_ids: [] as string[],
|
||||
workspaces: [
|
||||
// T1DL mirrors a managed install: the installer (authed_user) was pre-added
|
||||
// to the allow-list on connect (UX-027) — keys the "you" chip + setup card.
|
||||
{ team_id: "T1DL", account: "deeplearning.ai", domain: "dlaiteam", allowed_users: ["U_ME"] as string[], allow_all: false, allowed_user_names: {} as Record<string, string | null>, installer_user_id: "U_ME", installer_name: "Rohit Prasad" },
|
||||
{ team_id: "T2AC", account: "acme-partners", domain: "acmehq", allowed_users: [] as string[], allow_all: false, allowed_user_names: {} as Record<string, string | null>, installer_user_id: "", installer_name: "" },
|
||||
{ team_id: "T1DL", account: "deeplearning.ai", domain: "dlaiteam", allowed_users: ["U_ME"] as string[], allow_all: false, allowed_user_names: {} as Record<string, string | null>, approval_owner_ids: ["U_ME"] as string[], approval_owner_names: { U_ME: "Rohit Prasad" } as Record<string, string | null>, installer_user_id: "U_ME", installer_name: "Rohit Prasad" },
|
||||
{ team_id: "T2AC", account: "acme-partners", domain: "acmehq", allowed_users: [] as string[], allow_all: false, allowed_user_names: {} as Record<string, string | null>, approval_owner_ids: [] as string[], approval_owner_names: {} as Record<string, string | null>, installer_user_id: "", installer_name: "" },
|
||||
],
|
||||
};
|
||||
const slackConnector = () => ({
|
||||
@@ -369,7 +370,9 @@ export async function mockApi(page: import("@playwright/test").Page) {
|
||||
auth: "bot_token", two_way: true, channels: true, available: true, brand_color: "#611f69", logo: "slack",
|
||||
fields: [], instructions: [], connected: slackState.connected,
|
||||
account: slackState.account, enabled: slackState.connected,
|
||||
allowed_users: [...slackState.allowed_users], tools: [], managed: true,
|
||||
allowed_users: [...slackState.allowed_users],
|
||||
approval_owner_ids: [...slackState.approval_owner_ids],
|
||||
tools: [], managed: true,
|
||||
managed_profile: slackState.mode === "relay", mode: slackState.mode,
|
||||
workspaces: slackState.workspaces.map((w) => ({ ...w, allowed_users: [...w.allowed_users] })),
|
||||
unauthorized: parked.map((x) => ({ ...x })),
|
||||
@@ -911,6 +914,24 @@ export async function mockApi(page: import("@playwright/test").Page) {
|
||||
if (add && b.name && ws) ws.allowed_user_names[b.user_id] = b.name;
|
||||
return json({ ok: true, allowed_users: [...pool], team_id: b.team_id ?? null });
|
||||
}
|
||||
if (p.endsWith("/v1/connectors/slack/approval-owners/add") && m === "POST") {
|
||||
const b = req.postDataJSON();
|
||||
if (!slackState.approval_owner_ids.includes(b.user_id))
|
||||
slackState.approval_owner_ids.push(b.user_id);
|
||||
if (!slackState.allowed_users.includes(b.user_id))
|
||||
slackState.allowed_users.push(b.user_id);
|
||||
return json({
|
||||
ok: true,
|
||||
approval_owner_ids: [...slackState.approval_owner_ids],
|
||||
allowed_users: [...slackState.allowed_users],
|
||||
});
|
||||
}
|
||||
if (p.endsWith("/v1/connectors/slack/approval-owners/remove") && m === "POST") {
|
||||
const b = req.postDataJSON();
|
||||
const i = slackState.approval_owner_ids.indexOf(b.user_id);
|
||||
if (i >= 0) slackState.approval_owner_ids.splice(i, 1);
|
||||
return json({ ok: true, approval_owner_ids: [...slackState.approval_owner_ids] });
|
||||
}
|
||||
// Workspace rosters for the pickers (users.list / conversations.list, mocked).
|
||||
if (/\/v1\/connectors\/slack\/workspaces\/[^/]+\/directory$/.test(p) && m === "GET") {
|
||||
const q = (new URL(req.url()).searchParams.get("q") || "").toLowerCase();
|
||||
@@ -1137,7 +1158,7 @@ export async function mockApi(page: import("@playwright/test").Page) {
|
||||
// Slack managed install = add a workspace. The real flow completes in the system
|
||||
// browser; the mock installs instantly so the page's poll picks it up.
|
||||
if (p.includes("/connectors/slack/")) {
|
||||
slackState.workspaces.push({ team_id: "T3NEW", account: "new-workspace", allowed_users: [], allow_all: false, allowed_user_names: {} });
|
||||
slackState.workspaces.push({ team_id: "T3NEW", account: "new-workspace", domain: "new-workspace", allowed_users: ["U_ME"], allow_all: false, allowed_user_names: { U_ME: "Rohit Prasad" }, approval_owner_ids: ["U_ME"], approval_owner_names: { U_ME: "Rohit Prasad" }, installer_user_id: "U_ME", installer_name: "Rohit Prasad" });
|
||||
slackState.connected = true;
|
||||
slackState.mode = "relay";
|
||||
}
|
||||
|
||||
@@ -59,13 +59,13 @@ test("routing: Configure tab binds the mirror channel; Pending's status line fol
|
||||
await page.getByTestId("inbox-route-configure").click();
|
||||
const mirror = page.getByTestId("inbox-mirror-card");
|
||||
await expect(mirror).toContainText("in-app Inbox only");
|
||||
await mirror.getByPlaceholder("slack:C0123 or channel link").fill("C0777");
|
||||
await mirror.getByPlaceholder("slack:C0123 or channel link").fill("slack:T1DL/C0777");
|
||||
await mirror.getByRole("button", { name: "Set", exact: true }).click();
|
||||
await expect(mirror).toContainText("slack:C0777");
|
||||
await expect(mirror).toContainText("slack:T1DL/C0777");
|
||||
|
||||
// Back on Pending, the line reflects the new target immediately.
|
||||
await page.getByTestId("inbox-tab-pending").click();
|
||||
await expect(line).toContainText("slack:C0777");
|
||||
await expect(line).toContainText("slack:T1DL/C0777");
|
||||
await expect(line).toContainText("replies there resolve items here");
|
||||
|
||||
// Clearing (also on Configure) returns Pending to local-only delivery.
|
||||
|
||||
@@ -61,6 +61,7 @@ test("disconnect removes one workspace and keeps the rest relaying", async ({ pa
|
||||
test("manual Socket Mode: one card with the flat allow-list (no regression)", async ({
|
||||
page,
|
||||
}) => {
|
||||
let owners: string[] = [];
|
||||
// Override the connectors payload AFTER mockApi so this test sees a manual-mode Slack
|
||||
// (routes registered later match first).
|
||||
await page.route("**/v1/connectors", (route) =>
|
||||
@@ -74,6 +75,8 @@ test("manual Socket Mode: one card with the flat allow-list (no regression)", as
|
||||
auth: "bot_token", two_way: true, available: true, brand_color: "#611f69",
|
||||
logo: "slack", fields: [], instructions: [], connected: true, account: "acme",
|
||||
enabled: true, allowed_users: ["U0OK"], allowed_user_names: { U0OK: "Rohit" },
|
||||
approval_owner_ids: [...owners],
|
||||
approval_owner_names: Object.fromEntries(owners.map((u) => [u, u === "U9MAYA" ? "Maya Chen" : u])),
|
||||
tools: [], managed: true, managed_profile: false, mode: "", workspaces: [],
|
||||
unauthorized: [],
|
||||
},
|
||||
@@ -81,9 +84,22 @@ test("manual Socket Mode: one card with the flat allow-list (no regression)", as
|
||||
}),
|
||||
}),
|
||||
);
|
||||
await page.route("**/v1/connectors/slack/approval-owners/add", async (route) => {
|
||||
const body = route.request().postDataJSON();
|
||||
owners = [...new Set([...owners, body.user_id])];
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify({ ok: true, approval_owner_ids: owners }),
|
||||
});
|
||||
});
|
||||
await openSlackPage(page);
|
||||
await expect(page.getByTestId("slack-mode-badge")).toContainText("Socket Mode");
|
||||
const card = page.getByTestId("slack-manual-card");
|
||||
await expect(card).toContainText("acme");
|
||||
await expect(card).toContainText("Rohit"); // flat allow-list chip, named
|
||||
await expect(card).toContainText("Choose at least one owner");
|
||||
await page.getByTestId("add-approval-owner").click();
|
||||
await page.getByTestId("pick-person-U9MAYA").click();
|
||||
await expect(page.getByTestId("approval-owner-U9MAYA")).toContainText("Maya Chen");
|
||||
});
|
||||
|
||||
@@ -363,6 +363,8 @@ export interface SlackWorkspace {
|
||||
allowed_users: string[];
|
||||
allow_all: boolean;
|
||||
allowed_user_names?: Record<string, string | null>;
|
||||
approval_owner_ids?: string[];
|
||||
approval_owner_names?: Record<string, string | null>;
|
||||
// Who installed this workspace (authed_user) — pre-added to the allow-list on
|
||||
// connect (UX-027); the GUI marks their chip "you" and keys the setup card copy.
|
||||
installer_user_id?: string;
|
||||
@@ -443,6 +445,8 @@ export interface Connector {
|
||||
mcp?: boolean; // MCP-backed one-click (vendor-hosted MCP + local OAuth — no cloud sign-in)
|
||||
allowed_users: string[]; // the allow-list (managed inline in the Connectors tab)
|
||||
allowed_user_names?: Record<string, string | null>; // id → display name (people directory)
|
||||
approval_owner_ids?: string[]; // Manual Slack: humans allowed to resolve approvals
|
||||
approval_owner_names?: Record<string, string | null>;
|
||||
recent?: RecentSender[]; // recently-seen senders on a connected two-way connector
|
||||
unauthorized?: ParkedMessage[]; // parked messages from unallowed senders (§19)
|
||||
tools: ConnectorTool[];
|
||||
@@ -1585,6 +1589,32 @@ export async function disallowUser(name: string, userId: string, teamId?: string
|
||||
return res.json();
|
||||
}
|
||||
|
||||
export async function addSlackApprovalOwner(
|
||||
userId: string,
|
||||
displayName?: string,
|
||||
): Promise<{ ok: boolean; error?: string }> {
|
||||
const res = await fetch(`${httpBase()}/v1/connectors/slack/approval-owners/add`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
user_id: userId,
|
||||
...(displayName ? { name: displayName } : {}),
|
||||
}),
|
||||
});
|
||||
return res.json();
|
||||
}
|
||||
|
||||
export async function removeSlackApprovalOwner(
|
||||
userId: string,
|
||||
): Promise<{ ok: boolean; error?: string }> {
|
||||
const res = await fetch(`${httpBase()}/v1/connectors/slack/approval-owners/remove`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ user_id: userId }),
|
||||
});
|
||||
return res.json();
|
||||
}
|
||||
|
||||
/** Stop relaying one managed Slack workspace (the app stays installed in Slack). */
|
||||
export async function disconnectSlackWorkspace(teamId: string): Promise<{ ok: boolean; error?: string; remaining_workspaces?: number }> {
|
||||
const res = await fetch(
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { useEffect, useState } from "react";
|
||||
import {
|
||||
getConnectors,
|
||||
getDmRoute,
|
||||
getInboxRouting,
|
||||
getRecentChannels,
|
||||
@@ -11,6 +12,7 @@ import {
|
||||
subscribeChannel,
|
||||
unsubscribeChannel,
|
||||
type RecentChannel,
|
||||
type Connector,
|
||||
type Subscription,
|
||||
type UnroutedItem,
|
||||
} from "../api";
|
||||
@@ -53,11 +55,14 @@ export function InboxConfigure() {
|
||||
// the "default" route (sessions fall back to it); pick a channel separate from any you subscribe to.
|
||||
function InboxRoutingCard() {
|
||||
const [recent, setRecent] = useState<RecentChannel[]>([]);
|
||||
const [connectors, setConnectors] = useState<Connector[]>([]);
|
||||
const [target, setTarget] = useState(""); // current default-binding address, e.g. "slack:C0123"
|
||||
const [draft, setDraft] = useState("");
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
|
||||
const load = () => {
|
||||
getRecentChannels().then(setRecent).catch(() => setRecent([]));
|
||||
getConnectors().then(setConnectors).catch(() => setConnectors([]));
|
||||
getInboxRouting()
|
||||
.then((bs) => {
|
||||
const def = bs.find((b) => b.name === "default");
|
||||
@@ -76,15 +81,43 @@ function InboxRoutingCard() {
|
||||
if (!addr) return;
|
||||
// "slack:C0123" → channel="slack", target="C0123"; a bare id assumes slack.
|
||||
const [platform, id] = addr.includes(":") ? addr.split(":", 2) : ["slack", addr];
|
||||
await setInboxBinding("default", platform, id);
|
||||
const result = await setInboxBinding("default", platform, id);
|
||||
if (!result.ok) {
|
||||
setError(result.error || "Could not update Inbox routing.");
|
||||
return;
|
||||
}
|
||||
setError(null);
|
||||
setDraft("");
|
||||
load();
|
||||
};
|
||||
const clear = async () => {
|
||||
await setInboxBinding("default", null, "");
|
||||
const result = await setInboxBinding("default", null, "");
|
||||
if (!result.ok) {
|
||||
setError(result.error || "Could not clear Inbox routing.");
|
||||
return;
|
||||
}
|
||||
setError(null);
|
||||
load();
|
||||
};
|
||||
|
||||
const draftAddr = draft.trim();
|
||||
const [draftPlatform, draftTarget] = draftAddr.includes(":")
|
||||
? draftAddr.split(":", 2)
|
||||
: ["slack", draftAddr];
|
||||
const slack = connectors.find((c) => c.name === "slack");
|
||||
const teamId =
|
||||
draftPlatform === "slack" && draftTarget.includes("/")
|
||||
? draftTarget.split("/", 1)[0]
|
||||
: null;
|
||||
const owners =
|
||||
draftPlatform !== "slack"
|
||||
? []
|
||||
: teamId
|
||||
? slack?.workspaces?.find((w) => w.team_id === teamId)?.approval_owner_ids ?? []
|
||||
: slack?.approval_owner_ids ?? [];
|
||||
const missingSlackOwner =
|
||||
draftPlatform === "slack" && draftTarget.length > 0 && owners.length === 0;
|
||||
|
||||
// Show the channel's NAME when the recent list knows it (raw address as the fallback/tooltip).
|
||||
const known = recent.find((c) => c.channel === target)?.name;
|
||||
|
||||
@@ -103,7 +136,11 @@ function InboxRoutingCard() {
|
||||
<Icon name="plug" size={16} />
|
||||
</span>
|
||||
<ChannelPicker value={draft} onChange={setDraft} recent={recent} onSubmit={save} />
|
||||
<button className={BTN_ACCENT_SM} disabled={!draft.trim()} onClick={save}>
|
||||
<button
|
||||
className={BTN_ACCENT_SM}
|
||||
disabled={!draft.trim() || missingSlackOwner}
|
||||
onClick={save}
|
||||
>
|
||||
Set
|
||||
</button>
|
||||
{target && (
|
||||
@@ -112,6 +149,12 @@ function InboxRoutingCard() {
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
{missingSlackOwner && (
|
||||
<p className="text-[11.5px] text-warnInk mt-2">
|
||||
Choose an approval owner under Integrations → Slack before routing approvals here.
|
||||
</p>
|
||||
)}
|
||||
{error && <p className="text-[11.5px] text-warnInk mt-2">{error}</p>}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1,11 +1,13 @@
|
||||
import { useEffect, useRef, useState } from "react";
|
||||
import {
|
||||
addSlackApprovalOwner,
|
||||
allowUser,
|
||||
disallowUser,
|
||||
disconnectSlackWorkspace,
|
||||
getSlackDirectory,
|
||||
getSubscriptions,
|
||||
resolveUnauthorized,
|
||||
removeSlackApprovalOwner,
|
||||
unsubscribeChannel,
|
||||
type Connector,
|
||||
type ParkedMessage,
|
||||
@@ -130,10 +132,17 @@ export function SlackDetail({ c, cloud, slack, onChanged }: DetailProps) {
|
||||
<PeopleRow
|
||||
allowed={c.allowed_users}
|
||||
names={c.allowed_user_names}
|
||||
protectedIds={c.approval_owner_ids}
|
||||
teamId={null}
|
||||
onRemove={(u) => disallowUser("slack", u).then(changed)}
|
||||
onChanged={changed}
|
||||
/>
|
||||
<ApprovalOwnersRow
|
||||
owners={c.approval_owner_ids ?? []}
|
||||
names={c.approval_owner_names}
|
||||
editable
|
||||
onChanged={changed}
|
||||
/>
|
||||
{(c.unauthorized ?? [])
|
||||
.filter((m) => !m.team_id)
|
||||
.map((m) => (
|
||||
@@ -209,24 +218,43 @@ function WorkspaceGroup({
|
||||
</div>
|
||||
<div className={GRP}>
|
||||
{empty ? (
|
||||
<div className={ROW}>
|
||||
<span className="min-w-0 flex-1 text-[12.5px] text-muted flex items-center gap-2 flex-wrap">
|
||||
<span>No one allowed yet — mentions of the bot show up here for your OK.</span>
|
||||
<PersonPicker teamId={w.team_id} allowed={[]} onChanged={onChanged} />
|
||||
</span>
|
||||
<DisconnectBtn teamId={w.team_id} busy={busy} onClick={disconnect} />
|
||||
</div>
|
||||
<>
|
||||
<div className={ROW}>
|
||||
<span className="min-w-0 flex-1 text-[12.5px] text-muted flex items-center gap-2 flex-wrap">
|
||||
<span>No one allowed yet — mentions of the bot show up here for your OK.</span>
|
||||
<PersonPicker teamId={w.team_id} allowed={[]} onChanged={onChanged} />
|
||||
</span>
|
||||
<DisconnectBtn teamId={w.team_id} busy={busy} onClick={disconnect} />
|
||||
</div>
|
||||
<ApprovalOwnersRow
|
||||
owners={w.approval_owner_ids ?? []}
|
||||
names={w.approval_owner_names}
|
||||
installerId={w.installer_user_id}
|
||||
installerName={w.installer_name}
|
||||
editable={false}
|
||||
onChanged={onChanged}
|
||||
/>
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<PeopleRow
|
||||
allowed={w.allowed_users}
|
||||
names={w.allowed_user_names}
|
||||
protectedIds={w.approval_owner_ids}
|
||||
teamId={w.team_id}
|
||||
installerId={w.installer_user_id}
|
||||
installerName={w.installer_name}
|
||||
onRemove={(u) => disallowUser("slack", u, w.team_id).then(onChanged)}
|
||||
onChanged={onChanged}
|
||||
/>
|
||||
<ApprovalOwnersRow
|
||||
owners={w.approval_owner_ids ?? []}
|
||||
names={w.approval_owner_names}
|
||||
installerId={w.installer_user_id}
|
||||
installerName={w.installer_name}
|
||||
editable={false}
|
||||
onChanged={onChanged}
|
||||
/>
|
||||
{parked.map((m) => (
|
||||
<WaitingRow key={m.id} m={m} onChanged={onChanged} />
|
||||
))}
|
||||
@@ -259,6 +287,7 @@ function DisconnectBtn({ teamId, busy, onClick }: { teamId: string; busy: boolea
|
||||
function PeopleRow({
|
||||
allowed,
|
||||
names,
|
||||
protectedIds,
|
||||
teamId,
|
||||
installerId,
|
||||
installerName,
|
||||
@@ -267,6 +296,7 @@ function PeopleRow({
|
||||
}: {
|
||||
allowed: string[];
|
||||
names?: Record<string, string | null>;
|
||||
protectedIds?: string[];
|
||||
teamId: string | null; // null = manual flat list (directory queries as "default")
|
||||
installerId?: string; // authed_user — pre-added on managed connect (UX-027)
|
||||
installerName?: string;
|
||||
@@ -296,9 +326,18 @@ function PeopleRow({
|
||||
</span>
|
||||
{label(u)}
|
||||
{u === installerId && <span className="text-[10.5px] text-faint">· you</span>}
|
||||
<button className={XBTN} title="remove" onClick={() => onRemove(u)}>
|
||||
×
|
||||
</button>
|
||||
{protectedIds?.includes(u) ? (
|
||||
<span
|
||||
className="text-[10.5px] text-faint"
|
||||
title="Remove approval-owner access before removing this person."
|
||||
>
|
||||
· owner
|
||||
</span>
|
||||
) : (
|
||||
<button className={XBTN} title="remove" onClick={() => onRemove(u)}>
|
||||
×
|
||||
</button>
|
||||
)}
|
||||
</span>
|
||||
))}
|
||||
<PersonPicker teamId={teamId} allowed={allowed} onChanged={onChanged} />
|
||||
@@ -314,10 +353,16 @@ function PersonPicker({
|
||||
teamId,
|
||||
allowed,
|
||||
onChanged,
|
||||
onPick,
|
||||
buttonLabel = "+ Add person",
|
||||
testId,
|
||||
}: {
|
||||
teamId: string | null;
|
||||
allowed: string[];
|
||||
onChanged: () => void;
|
||||
onPick?: (member: SlackMember) => Promise<{ ok: boolean; error?: string }>;
|
||||
buttonLabel?: string;
|
||||
testId?: string;
|
||||
}) {
|
||||
const [open, setOpen] = useState(false);
|
||||
const [q, setQ] = useState("");
|
||||
@@ -360,7 +405,13 @@ function PersonPicker({
|
||||
}, [open]);
|
||||
|
||||
const pick = async (m: SlackMember) => {
|
||||
await allowUser("slack", m.id, teamId, m.name);
|
||||
const result = onPick
|
||||
? await onPick(m)
|
||||
: await allowUser("slack", m.id, teamId, m.name);
|
||||
if (result?.ok === false) {
|
||||
setErr(result.error || "could not add person");
|
||||
return;
|
||||
}
|
||||
setOpen(false);
|
||||
setQ("");
|
||||
onChanged();
|
||||
@@ -372,11 +423,11 @@ function PersonPicker({
|
||||
<button
|
||||
ref={btn}
|
||||
className="inline-flex items-center px-2 py-0.5 rounded-full border border-dashed border-line text-[12.5px] text-muted hover:text-ink hover:border-faint"
|
||||
data-testid={`add-person-${teamId || "default"}`}
|
||||
data-testid={testId || `add-person-${teamId || "default"}`}
|
||||
title="Pick from the workspace directory"
|
||||
onClick={toggle}
|
||||
>
|
||||
+ Add person
|
||||
{buttonLabel}
|
||||
</button>
|
||||
{open && (
|
||||
<div
|
||||
@@ -432,6 +483,80 @@ function PersonPicker({
|
||||
);
|
||||
}
|
||||
|
||||
function ApprovalOwnersRow({
|
||||
owners,
|
||||
names,
|
||||
installerId,
|
||||
installerName,
|
||||
editable,
|
||||
onChanged,
|
||||
}: {
|
||||
owners: string[];
|
||||
names?: Record<string, string | null>;
|
||||
installerId?: string;
|
||||
installerName?: string;
|
||||
editable: boolean;
|
||||
onChanged: () => void;
|
||||
}) {
|
||||
const [err, setErr] = useState<string | null>(null);
|
||||
const label = (u: string) =>
|
||||
names?.[u] || (u === installerId ? installerName || "You" : u);
|
||||
const remove = async (userId: string) => {
|
||||
const result = await removeSlackApprovalOwner(userId);
|
||||
if (!result.ok) {
|
||||
setErr(result.error || "could not remove approval owner");
|
||||
return;
|
||||
}
|
||||
setErr(null);
|
||||
onChanged();
|
||||
};
|
||||
return (
|
||||
<div className={ROW} data-testid="slack-approval-owners">
|
||||
<span className={LABEL}>Approvals</span>
|
||||
<span className="min-w-0 flex-1 flex flex-wrap items-center gap-1.5">
|
||||
{owners.length === 0 && (
|
||||
<span className="text-[12px] text-warnInk">
|
||||
Choose at least one owner before routing Inbox approvals to Slack.
|
||||
</span>
|
||||
)}
|
||||
{owners.map((u) => (
|
||||
<span
|
||||
key={u}
|
||||
className="inline-flex items-center gap-1.5 pl-1 pr-2 py-0.5 rounded-full bg-paper border border-line text-[12.5px]"
|
||||
title={`id ${u}`}
|
||||
data-testid={`approval-owner-${u}`}
|
||||
>
|
||||
<span className="w-5 h-5 rounded-full bg-accentSoft text-accent grid place-items-center text-[9px] font-bold">
|
||||
{initials(label(u))}
|
||||
</span>
|
||||
{label(u)}
|
||||
{u === installerId && <span className="text-[10.5px] text-faint">· installer</span>}
|
||||
{editable && (
|
||||
<button className={XBTN} title="remove approval owner" onClick={() => remove(u)}>
|
||||
×
|
||||
</button>
|
||||
)}
|
||||
</span>
|
||||
))}
|
||||
{editable && (
|
||||
<PersonPicker
|
||||
teamId={null}
|
||||
allowed={owners}
|
||||
onChanged={onChanged}
|
||||
onPick={(m) => addSlackApprovalOwner(m.id, m.name)}
|
||||
buttonLabel="+ Add owner"
|
||||
testId="add-approval-owner"
|
||||
/>
|
||||
)}
|
||||
{!editable && owners.length > 0 && (
|
||||
<span className="text-[11.5px] text-faint">Set by the workspace installer.</span>
|
||||
)}
|
||||
{err && <span className="basis-full text-[11.5px] text-warnInk">{err}</span>}
|
||||
</span>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
function WaitingRow({ m, onChanged }: { m: ParkedMessage; onChanged: () => void }) {
|
||||
const act = async (action: "dismiss" | "allow" | "allow_deliver") => {
|
||||
await resolveUnauthorized("slack", m.id, action);
|
||||
|
||||
@@ -475,6 +475,31 @@ def test_connect_missing_required_field(tmp_path):
|
||||
assert res["ok"] is False and "missing" in res["error"]
|
||||
|
||||
|
||||
def test_manual_slack_reconnect_preserves_approval_owners(tmp_path):
|
||||
from coworker.connectors import connect_connector
|
||||
|
||||
secrets = SecretStore(tmp_path / "secrets.json")
|
||||
secrets.put(
|
||||
"slack:default",
|
||||
{
|
||||
"bot_token": "xoxb-old",
|
||||
"app_token": "xapp-old",
|
||||
"allowed_users": ["U_OWNER"],
|
||||
"approval_owner_ids": ["U_OWNER"],
|
||||
},
|
||||
)
|
||||
result = connect_connector(
|
||||
secrets,
|
||||
"slack",
|
||||
{"bot_token": "xoxb-new", "app_token": "xapp-new"},
|
||||
validate=False,
|
||||
)
|
||||
assert result["ok"] is True
|
||||
profile = secrets.get("slack:default")
|
||||
assert profile["approval_owner_ids"] == ["U_OWNER"]
|
||||
assert profile["allowed_users"] == ["U_OWNER"]
|
||||
|
||||
|
||||
def test_connect_validation_runs(tmp_path):
|
||||
from coworker.connectors import connect_connector
|
||||
from coworker.connectors.descriptors import ValidationResult, get_descriptor
|
||||
|
||||
@@ -174,6 +174,7 @@ async def test_interaction_reaches_gateway_handler(fake_slack):
|
||||
ie = seen[0]
|
||||
assert ie.platform == "slack"
|
||||
assert ie.value == "item-id|allow"
|
||||
assert ie.user_id == "U1"
|
||||
assert ie.user_name == "alice"
|
||||
assert ie.chat_id == "C1"
|
||||
assert ie.message_id == "1700000001.000001"
|
||||
|
||||
@@ -60,6 +60,15 @@ def test_slack_blocks_shape():
|
||||
|
||||
def test_interaction_click_resolves_item(tmp_path):
|
||||
mgr = SessionManager(workspace=tmp_path, provider=ScriptedProvider([]))
|
||||
mgr.secrets.put(
|
||||
"slack:default",
|
||||
{
|
||||
"bot_token": "xoxb-test",
|
||||
"app_token": "xapp-test",
|
||||
"allowed_users": ["U_BOB"],
|
||||
"approval_owner_ids": ["U_BOB"],
|
||||
},
|
||||
)
|
||||
item = mgr.inbox.add_approval("sX", "Run `write_file`?")
|
||||
|
||||
resolved: list = []
|
||||
@@ -79,6 +88,7 @@ def test_interaction_click_resolves_item(tmp_path):
|
||||
chat_id="C1",
|
||||
message_id="111.2",
|
||||
value=encode(item.id, "allow"),
|
||||
user_id="U_BOB",
|
||||
user_name="bob",
|
||||
)
|
||||
)
|
||||
|
||||
@@ -0,0 +1,343 @@
|
||||
"""Slack approval ownership is distinct from ordinary inbound access."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
|
||||
import httpx
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from coworker.connectors import ConnectorSettings, Gateway, TeamAuth
|
||||
from coworker.connectors.base import InteractionEvent, MessageEvent, SessionSource
|
||||
from coworker.interactions import encode
|
||||
from coworker.providers import ModelCapabilities, ProviderClient
|
||||
from coworker.server import create_app
|
||||
from coworker.server.manager import SessionManager
|
||||
|
||||
|
||||
class NoTurnsProvider(ProviderClient):
|
||||
def complete(self, *, model, messages, tools=None, **settings):
|
||||
raise AssertionError("no model turns expected")
|
||||
|
||||
def capabilities(self, model):
|
||||
return ModelCapabilities()
|
||||
|
||||
|
||||
def _manager(tmp_path) -> SessionManager:
|
||||
return SessionManager(data_dir=tmp_path / "data", provider=NoTurnsProvider())
|
||||
|
||||
|
||||
def _manual_profile(manager: SessionManager, **extra) -> None:
|
||||
manager.secrets.put(
|
||||
"slack:default",
|
||||
{
|
||||
"bot_token": "xoxb-test",
|
||||
"app_token": "xapp-test",
|
||||
"enabled": True,
|
||||
**extra,
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
def test_manual_owner_is_required_for_binding_and_implies_allowed(tmp_path):
|
||||
manager = _manager(tmp_path)
|
||||
_manual_profile(manager)
|
||||
|
||||
denied = manager.set_inbox_binding(
|
||||
"default", channel="slack", target="C_APPROVALS"
|
||||
)
|
||||
assert denied["ok"] is False
|
||||
|
||||
added = manager.set_slack_approval_owner(
|
||||
"U_OWNER", add=True, display_name="Owner"
|
||||
)
|
||||
assert added["ok"] is True
|
||||
profile = manager.secrets.get("slack:default")
|
||||
assert profile["approval_owner_ids"] == ["U_OWNER"]
|
||||
assert profile["allowed_users"] == ["U_OWNER"]
|
||||
|
||||
bound = manager.set_inbox_binding(
|
||||
"default", channel="slack", target="C_APPROVALS"
|
||||
)
|
||||
assert bound["ok"] is True
|
||||
assert manager.disallow_user("slack", "U_OWNER")["ok"] is False
|
||||
assert manager.set_slack_approval_owner("U_OWNER", add=False)["ok"] is False
|
||||
|
||||
manager.set_slack_approval_owner("U_SECOND", add=True)
|
||||
assert manager.set_slack_approval_owner("U_OWNER", add=False)["ok"] is True
|
||||
assert manager.slack_approval_owner_ids() == {"U_SECOND"}
|
||||
|
||||
|
||||
def test_manual_owner_rest_flow_surfaces_identity_and_binding(tmp_path):
|
||||
manager = _manager(tmp_path)
|
||||
_manual_profile(manager)
|
||||
client = TestClient(create_app(manager))
|
||||
|
||||
added = client.post(
|
||||
"/v1/connectors/slack/approval-owners/add",
|
||||
json={"user_id": "U_OWNER", "name": "Ada"},
|
||||
).json()
|
||||
assert added["ok"] is True
|
||||
|
||||
slack = next(
|
||||
row
|
||||
for row in client.get("/v1/connectors").json()["connectors"]
|
||||
if row["name"] == "slack"
|
||||
)
|
||||
assert slack["approval_owner_ids"] == ["U_OWNER"]
|
||||
assert slack["approval_owner_names"] == {"U_OWNER": "Ada"}
|
||||
|
||||
bound = client.post(
|
||||
"/v1/inbox/routing/binding",
|
||||
json={"name": "default", "channel": "slack", "target": "C_APPROVALS"},
|
||||
).json()
|
||||
assert bound["ok"] is True
|
||||
|
||||
|
||||
def test_relay_binding_uses_installer_identity(tmp_path):
|
||||
manager = _manager(tmp_path)
|
||||
manager.secrets.put(
|
||||
"slack:default", {"mode": "relay", "enabled": True, "managed": True}
|
||||
)
|
||||
manager.secrets.put(
|
||||
"slack:team:T1",
|
||||
{
|
||||
"bot_token": "xoxb-team",
|
||||
"managed": True,
|
||||
"slack_user_id": "U_INSTALLER",
|
||||
"allowed_users": ["U_INSTALLER"],
|
||||
},
|
||||
)
|
||||
assert manager.set_inbox_binding(
|
||||
"default", channel="slack", target="T1/C_APPROVALS"
|
||||
)["ok"]
|
||||
|
||||
manager.secrets.put(
|
||||
"slack:team:T2",
|
||||
{"bot_token": "xoxb-team-2", "managed": True},
|
||||
)
|
||||
assert not manager.set_inbox_binding(
|
||||
"default", channel="slack", target="T2/C_APPROVALS"
|
||||
)["ok"]
|
||||
|
||||
|
||||
def test_relay_does_not_reuse_dormant_manual_owners_for_bare_target(tmp_path):
|
||||
manager = _manager(tmp_path)
|
||||
manager.secrets.put(
|
||||
"slack:default",
|
||||
{
|
||||
"mode": "relay",
|
||||
"enabled": True,
|
||||
"managed": True,
|
||||
"bot_token": "xoxb-dormant",
|
||||
"app_token": "xapp-dormant",
|
||||
"approval_owner_ids": ["U_OLD_MANUAL_OWNER"],
|
||||
},
|
||||
)
|
||||
assert manager.slack_approval_owner_ids() == set()
|
||||
assert not manager.set_inbox_binding(
|
||||
"default", channel="slack", target="C_AMBIGUOUS"
|
||||
)["ok"]
|
||||
|
||||
|
||||
def test_nonowner_cannot_resolve_approval_but_can_answer_question(tmp_path):
|
||||
manager = _manager(tmp_path)
|
||||
_manual_profile(
|
||||
manager,
|
||||
allowed_users=["U_OWNER", "U_MEMBER"],
|
||||
approval_owner_ids=["U_OWNER"],
|
||||
)
|
||||
|
||||
class GatewayStub:
|
||||
def __init__(self):
|
||||
self.rejections = []
|
||||
self.updates = []
|
||||
|
||||
async def reject_interaction(self, event, text=""):
|
||||
self.rejections.append(event.user_id)
|
||||
|
||||
async def update_message(self, *args):
|
||||
self.updates.append(args)
|
||||
|
||||
gateway = GatewayStub()
|
||||
manager.gateway = gateway
|
||||
approval = manager.inbox.add_approval("s1", "Run it?")
|
||||
question = manager.inbox.add_question("s1", "Which one?", options=["A", "B"])
|
||||
|
||||
async def scenario():
|
||||
await manager._on_interaction(
|
||||
InteractionEvent(
|
||||
platform="slack",
|
||||
chat_id="C1",
|
||||
message_id="1",
|
||||
value=encode(approval.id, "allow"),
|
||||
user_id="U_MEMBER",
|
||||
user_name="Member",
|
||||
)
|
||||
)
|
||||
await manager._on_interaction(
|
||||
InteractionEvent(
|
||||
platform="slack",
|
||||
chat_id="C1",
|
||||
message_id="2",
|
||||
value=encode(question.id, "A"),
|
||||
user_id="U_MEMBER",
|
||||
user_name="Member",
|
||||
)
|
||||
)
|
||||
|
||||
asyncio.run(scenario())
|
||||
assert manager.inbox.get(approval.id).state == "pending"
|
||||
assert manager.inbox.get(question.id).resolution == "A"
|
||||
assert gateway.rejections == ["U_MEMBER"]
|
||||
assert len(gateway.updates) == 1
|
||||
|
||||
|
||||
def test_owner_click_from_a_different_bound_channel_is_rejected(tmp_path):
|
||||
manager = _manager(tmp_path)
|
||||
_manual_profile(
|
||||
manager,
|
||||
allowed_users=["U_OWNER"],
|
||||
approval_owner_ids=["U_OWNER"],
|
||||
)
|
||||
assert manager.set_inbox_binding(
|
||||
"default", channel="slack", target="C_EXPECTED"
|
||||
)["ok"]
|
||||
item = manager.inbox.add_approval("s1", "Run it?")
|
||||
|
||||
class GatewayStub:
|
||||
def __init__(self):
|
||||
self.rejections = []
|
||||
|
||||
async def reject_interaction(self, event, text=""):
|
||||
self.rejections.append(event.chat_id)
|
||||
|
||||
gateway = GatewayStub()
|
||||
manager.gateway = gateway
|
||||
asyncio.run(
|
||||
manager._on_interaction(
|
||||
InteractionEvent(
|
||||
platform="slack",
|
||||
chat_id="C_OTHER",
|
||||
message_id="1",
|
||||
value=encode(item.id, "allow"),
|
||||
user_id="U_OWNER",
|
||||
)
|
||||
)
|
||||
)
|
||||
assert manager.inbox.get(item.id).state == "pending"
|
||||
assert gateway.rejections == ["C_OTHER"]
|
||||
|
||||
|
||||
def test_gateway_rejects_interaction_from_unallowed_actor():
|
||||
handled = []
|
||||
|
||||
async def handler(event):
|
||||
handled.append(event)
|
||||
|
||||
gateway = Gateway(
|
||||
settings={
|
||||
"slack": ConnectorSettings(
|
||||
platform="slack",
|
||||
enabled=True,
|
||||
teams={"T1": TeamAuth(allowed_users={"U_ALLOWED"})},
|
||||
)
|
||||
},
|
||||
interaction_handler=handler,
|
||||
)
|
||||
|
||||
async def scenario():
|
||||
await gateway._on_interaction(
|
||||
InteractionEvent(
|
||||
platform="slack",
|
||||
chat_id="T1/C1",
|
||||
message_id="1",
|
||||
value="ignored",
|
||||
user_id="U_OTHER",
|
||||
team_id="T1",
|
||||
)
|
||||
)
|
||||
|
||||
asyncio.run(scenario())
|
||||
assert handled == []
|
||||
|
||||
|
||||
def test_rejected_click_uses_only_slacks_ephemeral_response_url(monkeypatch):
|
||||
calls = []
|
||||
|
||||
def fake_post(url, **kwargs):
|
||||
calls.append((url, kwargs))
|
||||
|
||||
monkeypatch.setattr(httpx, "post", fake_post)
|
||||
gateway = Gateway(settings={})
|
||||
|
||||
async def scenario():
|
||||
await gateway.reject_interaction(
|
||||
InteractionEvent(
|
||||
platform="slack",
|
||||
chat_id="C1",
|
||||
message_id="1",
|
||||
value="ignored",
|
||||
response_url="https://hooks.slack.com/actions/abc",
|
||||
)
|
||||
)
|
||||
await gateway.reject_interaction(
|
||||
InteractionEvent(
|
||||
platform="slack",
|
||||
chat_id="C1",
|
||||
message_id="1",
|
||||
value="ignored",
|
||||
response_url="https://hooks.slack.com.evil.example/actions/abc",
|
||||
)
|
||||
)
|
||||
|
||||
asyncio.run(scenario())
|
||||
assert len(calls) == 1
|
||||
assert calls[0][0] == "https://hooks.slack.com/actions/abc"
|
||||
assert calls[0][1]["json"]["response_type"] == "ephemeral"
|
||||
assert "approval owner" in calls[0][1]["json"]["text"]
|
||||
|
||||
|
||||
def test_slack_reply_token_is_owner_only_for_approvals(tmp_path):
|
||||
manager = _manager(tmp_path)
|
||||
_manual_profile(
|
||||
manager,
|
||||
allowed_users=["U_OWNER", "U_MEMBER"],
|
||||
approval_owner_ids=["U_OWNER"],
|
||||
)
|
||||
item = manager.inbox.add_approval("s1", "Run it?")
|
||||
|
||||
unauthorized = MessageEvent(
|
||||
text=f"approve [ow:{item.id}]",
|
||||
source=SessionSource("slack", "C1", user_id="U_MEMBER"),
|
||||
)
|
||||
assert manager._resolve_inbox_reply(unauthorized) is True
|
||||
assert manager.inbox.get(item.id).state == "pending"
|
||||
|
||||
owner = MessageEvent(
|
||||
text=f"approve [ow:{item.id}]",
|
||||
source=SessionSource("slack", "C1", user_id="U_OWNER"),
|
||||
)
|
||||
assert manager._resolve_inbox_reply(owner) is True
|
||||
assert manager.inbox.get(item.id).resolution == "allow"
|
||||
|
||||
|
||||
def test_ownerless_legacy_binding_does_not_mirror(tmp_path):
|
||||
manager = _manager(tmp_path)
|
||||
_manual_profile(manager, allowed_users=["U_MEMBER"])
|
||||
manager.inbox_routing.set_binding(
|
||||
"default", channel="slack", target="C_APPROVALS"
|
||||
)
|
||||
item = manager.inbox.add_approval("s1", "Run it?")
|
||||
|
||||
class GatewayStub:
|
||||
def __init__(self):
|
||||
self.deliveries = []
|
||||
|
||||
async def deliver_interactive(self, *args):
|
||||
self.deliveries.append(args)
|
||||
|
||||
gateway = GatewayStub()
|
||||
manager.gateway = gateway
|
||||
asyncio.run(manager.mirror_inbox_item(item))
|
||||
assert gateway.deliveries == []
|
||||
@@ -261,9 +261,10 @@ async def test_relay_interactivity_maps_to_interaction():
|
||||
"kind": "interactivity",
|
||||
"team_id": "T2",
|
||||
"interaction": {
|
||||
"user": {"username": "bob"},
|
||||
"user": {"id": "U2", "username": "bob"},
|
||||
"channel": {"id": "C7"},
|
||||
"message": {"ts": "9.9"},
|
||||
"response_url": "https://hooks.slack.com/actions/abc",
|
||||
"actions": [{"value": "approve:42"}],
|
||||
},
|
||||
}
|
||||
@@ -282,6 +283,9 @@ async def test_relay_interactivity_maps_to_interaction():
|
||||
|
||||
assert len(seen) == 1
|
||||
assert seen[0].chat_id == "T2/C7" and seen[0].value == "approve:42"
|
||||
assert seen[0].user_id == "U2"
|
||||
assert seen[0].team_id == "T2"
|
||||
assert seen[0].response_url == "https://hooks.slack.com/actions/abc"
|
||||
|
||||
|
||||
async def test_relay_revoked_drops_team():
|
||||
|
||||
@@ -33,6 +33,7 @@ def _install_form(team_id: str) -> dict:
|
||||
"team_id": team_id,
|
||||
"access_token": f"xoxb-{team_id}",
|
||||
"bot_user_id": "B1",
|
||||
"slack_user_id": f"U_{team_id}",
|
||||
"account": f"Workspace {team_id}",
|
||||
"team_domain": f"dom-{team_id.lower()}",
|
||||
"connection_id": f"conn_{team_id}",
|
||||
@@ -71,6 +72,7 @@ def test_managed_callback_installs_and_hot_reloads(client, monkeypatch):
|
||||
if c["name"] == "slack"
|
||||
][0]
|
||||
assert [w["domain"] for w in slack["workspaces"]] == ["dom-t1"]
|
||||
assert slack["workspaces"][0]["approval_owner_ids"] == ["U_T1"]
|
||||
assert client.manager.secrets.get("slack:default")["mode"] == "relay"
|
||||
assert refreshes # new workspace's token loads without an app restart
|
||||
|
||||
|
||||
@@ -260,4 +260,5 @@ def test_workspace_listing_carries_installer_identity(tmp_path):
|
||||
)
|
||||
(w,) = _slack_workspaces(s)
|
||||
assert w["installer_user_id"] == "U_ME"
|
||||
assert w["approval_owner_ids"] == ["U_ME"]
|
||||
assert w["installer_name"] == ""
|
||||
|
||||
@@ -154,6 +154,7 @@ async def test_ui_refresh_cross_cutting_e2e(fake_slack, tmp_path, monkeypatch):
|
||||
fake_slack.add_channel(CHANNEL, CHANNEL_NAME)
|
||||
# allow-list: the inbound gate drops unknown senders unless allowed
|
||||
assert mgr.allow_user("slack", USER)["ok"] is True
|
||||
assert mgr.set_slack_approval_owner(USER, add=True)["ok"] is True
|
||||
|
||||
# an Ops "incident" session subscribed to the channel, Unattended, approvals -> the channel
|
||||
mgr.session_store.save(
|
||||
@@ -166,7 +167,9 @@ async def test_ui_refresh_cross_cutting_e2e(fake_slack, tmp_path, monkeypatch):
|
||||
)
|
||||
)
|
||||
mgr.subscriptions.subscribe(SID, f"slack:{CHANNEL}")
|
||||
mgr.inbox_routing.set_binding("ops-incidents", channel="slack", target=CHANNEL)
|
||||
assert mgr.set_inbox_binding(
|
||||
"ops-incidents", channel="slack", target=CHANNEL
|
||||
)["ok"]
|
||||
mgr.inbox_routing.set_session_override(SID, "ops-incidents")
|
||||
mgr.unattended.set(SID, True)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user