From cb4c643e434197c62f87ee8e4a68271ca9179268 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Tue, 2 Jun 2026 15:04:20 -0700 Subject: [PATCH] feat: open Slack-triggered PRs as the triggering user (#1375) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: open Slack-triggered PRs as the triggering user Route the Slack per-user GitHub token through the dashboard OAuth store (the backend the self-service link prompt populates) and block runs that lack a valid user token, prompting the user to (re-)link. Per-user OAuth now wins over bot-token-only mode for mapped Slack/dashboard users. Flip commit/PR authorship across all sources: the triggering user is the commit author (via repo-local git identity using their resolvable GitHub noreply email) and open-swe[bot] is the Co-authored-by collaborator. * fix: address PR review — shell-escape commit identity, fix token cache impersonation - Shell-escape the triggering user's name/email with shlex.quote before embedding them in the repo-setup `git config` command, so a name like O'Connor (or a crafted one) can't break or inject into the command. - Stop consulting the shared thread-metadata token cache in _resolve_dashboard_user_token. Slack thread ids are shared across the conversation, so a cached token from a prior triggering user could be returned for the current github_login. Always resolve by login from the dashboard OAuth store instead. * feat: dashboard self-service user mapping + UI cleanup - Add session-scoped GET/PUT /dashboard/api/my-mapping so users can set their own work email / Slack member ID (keyed by their GitHub login, source=self). - Slack account-link prompt now redirects to Profile Settings after auth. - Rename "My Settings" -> "Profile Settings" and "Cloud Agents" -> "Open SWE Agent"; remove the Integrations tab/section (folded out, low value for now) and redirect /integrations to Profile Settings. - Add a "User mapping" section to Profile Settings (work email used by Slack and Linear, optional Slack member ID). - Make dashboard auth cookies scheme-aware: Secure;SameSite=None over HTTPS, non-Secure;SameSite=Lax over http://localhost so local login works. * feat: self-service Slack account linking via Sign in with Slack (OIDC) Replace the spoofable manual work-email/Slack-ID form with a verified "Sign in with Slack" flow so a logged-in GitHub user can only ever link their own Slack identity. - New agent/dashboard/slack_oauth.py: OIDC authorize URL, code exchange, userInfo identity parse, optional workspace gate, configured check. - routes.py: session-gated GET /slack/login and /slack/callback that upsert the mapping from Slack-verified user_id + email (source=slack_oauth). Remove the spoofable PUT /my-mapping; expose slack_oauth_enabled on /me. - UI: drop the editable inputs; add a Connect Slack button + status to the User mapping section. Admin-managed mappings are unaffected and still resolve at trigger time. --- agent/dashboard/oauth.py | 12 ++- agent/dashboard/routes.py | 131 ++++++++++++++++++++++- agent/dashboard/slack_oauth.py | 117 +++++++++++++++++++++ agent/dashboard/user_mappings.py | 2 +- agent/prompt.py | 27 +++-- agent/utils/auth.py | 101 +++++++++++------- agent/utils/authorship.py | 16 ++- agent/webapp.py | 68 +++++++++--- tests/test_account_link.py | 16 ++- tests/test_auth_sources.py | 124 ++++++++++++++++++++++ tests/test_authorship.py | 36 +++++++ tests/test_github_comment_prompts.py | 32 +++++- tests/test_slack_context.py | 152 +++++++++++++++++++++++---- tests/test_slack_oauth.py | 86 +++++++++++++++ ui/src/components/AppSidebar.tsx | 6 +- ui/src/lib/api.ts | 6 ++ ui/src/routes/admin.tsx | 2 +- ui/src/routes/cloud-agents.tsx | 4 +- ui/src/routes/integrations.tsx | 126 +--------------------- ui/src/routes/my-settings.tsx | 77 +++++++++++++- ui/src/routes/review.tsx | 2 +- 21 files changed, 912 insertions(+), 231 deletions(-) create mode 100644 agent/dashboard/slack_oauth.py create mode 100644 tests/test_authorship.py create mode 100644 tests/test_slack_oauth.py diff --git a/agent/dashboard/oauth.py b/agent/dashboard/oauth.py index 68c939a9..62efad89 100644 --- a/agent/dashboard/oauth.py +++ b/agent/dashboard/oauth.py @@ -170,6 +170,9 @@ def decode_state(state: str) -> dict[str, Any]: LINK_TTL_SECONDS = 7 * 24 * 60 * 60 +# Dashboard route where users manage their GitHub↔Slack link. +PROFILE_SETTINGS_PATH = "/my-settings" + def issue_account_link(*, slack_user_id: str | None, work_email: str | None) -> str: """Sign a short-lived token carrying the Slack identity to map after login. @@ -212,7 +215,14 @@ def build_account_link_url(*, slack_user_id: str | None, work_email: str | None) if not api_base: return None token = issue_account_link(slack_user_id=slack_user_id, work_email=work_email) - return f"{api_base}/dashboard/api/auth/login?link={quote(token, safe='')}" + url = f"{api_base}/dashboard/api/auth/login?link={quote(token, safe='')}" + # Land the user on the Profile Settings page so they can review/complete + # their GitHub↔Slack link after re-authenticating. + frontend_base = os.environ.get("DASHBOARD_BASE_URL", "").rstrip("/") + if frontend_base: + redirect_to = f"{frontend_base}{PROFILE_SETTINGS_PATH}" + url += f"&redirect_to={quote(redirect_to, safe='')}" + return url def require_session(request: Request) -> dict[str, Any]: diff --git a/agent/dashboard/routes.py b/agent/dashboard/routes.py index 330f3f6d..0888886c 100644 --- a/agent/dashboard/routes.py +++ b/agent/dashboard/routes.py @@ -58,6 +58,14 @@ from .review_styles import ( normalize_repo_full_name, set_custom_prompt, ) +from .slack_oauth import ( + SLACK_STATE_COOKIE_NAME, + build_authorize_url, + exchange_slack_code, + fetch_slack_identity, + slack_oauth_configured, + verify_team, +) from .team_settings import ( TeamSettingsUpdate, get_team_settings, @@ -76,6 +84,7 @@ from .thread_api import ( ) from .user_mappings import ( delete_mapping, + get_mapping, list_mappings, upsert_mapping, ) @@ -115,14 +124,29 @@ def _frontend_base_url() -> str: return v +def _cookie_security() -> tuple[bool, str]: + """Cookie ``secure``/``samesite`` flags derived from the API scheme. + + Production serves the API over HTTPS and the dashboard is a separate + (cross-site) origin, so the session cookie must be ``Secure; SameSite=None``. + Local dev runs over ``http://localhost`` where ``Secure`` cookies are + rejected and the frontend/API are same-site, so fall back to + ``SameSite=Lax`` without ``Secure``. + """ + if os.environ.get("DASHBOARD_API_BASE_URL", "").startswith("https://"): + return True, "none" + return False, "lax" + + def _set_session_cookie(response: Response, jwt_token: str) -> None: + secure, samesite = _cookie_security() response.set_cookie( key=COOKIE_NAME, value=jwt_token, max_age=SESSION_TTL_SECONDS, httponly=True, - secure=True, - samesite="none", + secure=secure, + samesite=samesite, path="/", ) @@ -131,20 +155,42 @@ def _set_state_cookie(response: Response, nonce: str) -> None: # SameSite=Lax so GitHub's top-level redirect back to /auth/callback # still presents this cookie; the cookie is single-purpose and lives # only for the duration of one OAuth round-trip. + secure, _ = _cookie_security() response.set_cookie( key=STATE_COOKIE_NAME, value=nonce, max_age=STATE_TTL_SECONDS, httponly=True, - secure=True, + secure=secure, samesite="lax", path="/dashboard/api/auth", ) def _clear_state_cookie(response: Response) -> None: + secure, _ = _cookie_security() response.delete_cookie( - STATE_COOKIE_NAME, path="/dashboard/api/auth", samesite="lax", secure=True + STATE_COOKIE_NAME, path="/dashboard/api/auth", samesite="lax", secure=secure + ) + + +def _set_slack_state_cookie(response: Response, nonce: str) -> None: + secure, _ = _cookie_security() + response.set_cookie( + key=SLACK_STATE_COOKIE_NAME, + value=nonce, + max_age=STATE_TTL_SECONDS, + httponly=True, + secure=secure, + samesite="lax", + path="/dashboard/api/slack", + ) + + +def _clear_slack_state_cookie(response: Response) -> None: + secure, _ = _cookie_security() + response.delete_cookie( + SLACK_STATE_COOKIE_NAME, path="/dashboard/api/slack", samesite="lax", secure=secure ) @@ -247,7 +293,8 @@ async def _complete_account_mapping(login: str, github_email: str | None, link_t @router.post("/auth/logout") async def auth_logout() -> Response: response = Response(status_code=204) - response.delete_cookie(COOKIE_NAME, path="/", samesite="none", secure=True) + secure, samesite = _cookie_security() + response.delete_cookie(COOKIE_NAME, path="/", samesite=samesite, secure=secure) return response @@ -258,6 +305,7 @@ async def me(session: dict[str, Any] = _SESSION_DEP) -> dict[str, Any]: "email": session.get("email"), "avatar_url": session.get("avatar_url"), "is_admin": is_admin(session.get("email")), + "slack_oauth_enabled": slack_oauth_configured(), } @@ -283,6 +331,79 @@ async def put_my_profile( return await upsert_profile(session["sub"], session.get("email") or "", update) +@router.get("/my-mapping") +async def get_my_mapping( + session: dict[str, Any] = _SESSION_DEP, +) -> dict[str, Any]: + """Return the logged-in user's own GitHub↔Slack mapping (or empty).""" + mapping = await get_mapping(session["sub"]) + return mapping or {} + + +@router.get("/slack/login") +async def slack_login( + _session: dict[str, Any] = _SESSION_DEP, +) -> RedirectResponse: + """Start the Sign in with Slack flow to link the current GitHub account.""" + if not slack_oauth_configured(): + raise HTTPException(500, "Slack OAuth is not configured") + redirect_uri = f"{_api_base_url()}/dashboard/api/slack/callback" + nonce = new_state_nonce() + state = issue_state( + redirect_to=f"{_frontend_base_url()}/my-settings", + nonce_hash=hash_state_nonce(nonce), + ) + response = RedirectResponse( + build_authorize_url(redirect_uri=redirect_uri, state=state), status_code=302 + ) + _set_slack_state_cookie(response, nonce) + return response + + +@router.get("/slack/callback") +async def slack_callback( + request: Request, + code: str, + state: str, + session: dict[str, Any] = _SESSION_DEP, +) -> RedirectResponse: + """Link the verified Slack identity to the logged-in GitHub user. + + The Slack member id and email come from Slack's verified OIDC claims, so a + user can only ever link their own Slack account — no self-asserted values. + """ + state_payload = decode_state(state) + nonce_hash = state_payload.get("nonce_hash") + cookie_nonce = request.cookies.get(SLACK_STATE_COOKIE_NAME) + if ( + not isinstance(nonce_hash, str) + or not cookie_nonce + or not hmac.compare_digest(hash_state_nonce(cookie_nonce), nonce_hash) + ): + raise HTTPException(400, "oauth state mismatch — please retry") + + redirect_to = sanitize_redirect_to(state_payload.get("redirect_to")) or _frontend_base_url() + redirect_uri = f"{_api_base_url()}/dashboard/api/slack/callback" + + access_token = await exchange_slack_code(code, redirect_uri) + identity = await fetch_slack_identity(access_token) + verify_team(identity) + if not identity.email or not identity.email_verified: + raise HTTPException(400, "your Slack account has no verified email to link") + + await upsert_mapping( + github_login=session["sub"], + work_email=identity.email, + slack_user_id=identity.user_id, + source="slack_oauth", + status="active", + ) + + response = RedirectResponse(redirect_to, status_code=302) + _clear_slack_state_cookie(response) + return response + + @router.get("/team-settings") async def api_get_team_settings( session: dict[str, Any] = _SESSION_DEP, diff --git a/agent/dashboard/slack_oauth.py b/agent/dashboard/slack_oauth.py new file mode 100644 index 00000000..59085b07 --- /dev/null +++ b/agent/dashboard/slack_oauth.py @@ -0,0 +1,117 @@ +"""Sign in with Slack (OpenID Connect) for self-service GitHub ⇄ Slack linking. + +Reuses the existing Slack app — Sign in with Slack is a capability on the same +app that owns the bot token, so it only needs the ``openid email profile`` user +scopes, a redirect URL, and the app's client id/secret. The id_token/userInfo +claims give us a Slack-*verified* member id and email, so a logged-in GitHub +user can only ever link their own Slack identity (no self-asserted spoofing). +""" + +from __future__ import annotations + +import logging +import os +from dataclasses import dataclass +from typing import Any +from urllib.parse import urlencode + +import httpx +from fastapi import HTTPException + +logger = logging.getLogger(__name__) + +SLACK_CLIENT_ID = os.environ.get("SLACK_CLIENT_ID", "") +SLACK_CLIENT_SECRET = os.environ.get("SLACK_CLIENT_SECRET", "") +# Optional: restrict linking to a single workspace (the Slack team id, T...). +SLACK_TEAM_ID = os.environ.get("SLACK_TEAM_ID", "") + +SLACK_STATE_COOKIE_NAME = "osw_slack_oauth_state" +SLACK_OIDC_SCOPES = "openid email profile" + +_AUTHORIZE_URL = "https://slack.com/openid/connect/authorize" +_TOKEN_URL = "https://slack.com/api/openid.connect.token" +_USERINFO_URL = "https://slack.com/api/openid.connect.userInfo" +_USER_ID_CLAIM = "https://slack.com/user_id" +_TEAM_ID_CLAIM = "https://slack.com/team_id" + + +def slack_oauth_configured() -> bool: + return bool(SLACK_CLIENT_ID and SLACK_CLIENT_SECRET) + + +@dataclass(frozen=True) +class SlackIdentity: + user_id: str + team_id: str + email: str | None + email_verified: bool + name: str | None + + +def build_authorize_url(*, redirect_uri: str, state: str) -> str: + params = { + "response_type": "code", + "scope": SLACK_OIDC_SCOPES, + "client_id": SLACK_CLIENT_ID, + "redirect_uri": redirect_uri, + "state": state, + } + if SLACK_TEAM_ID: + # Pre-selects the workspace so users can't accidentally sign in elsewhere. + params["team"] = SLACK_TEAM_ID + return f"{_AUTHORIZE_URL}?{urlencode(params)}" + + +def parse_slack_identity(data: dict[str, Any]) -> SlackIdentity: + """Build a SlackIdentity from an openid.connect.userInfo response.""" + if not data.get("ok", True): + raise HTTPException(400, f"slack userinfo failed: {data.get('error', 'unknown')}") + user_id = data.get(_USER_ID_CLAIM) + if not isinstance(user_id, str) or not user_id: + raise HTTPException(400, "slack userinfo missing user id") + team_id = data.get(_TEAM_ID_CLAIM) + email = data.get("email") + return SlackIdentity( + user_id=user_id, + team_id=team_id if isinstance(team_id, str) else "", + email=email if isinstance(email, str) and email else None, + email_verified=bool(data.get("email_verified")), + name=data.get("name") if isinstance(data.get("name"), str) else None, + ) + + +def verify_team(identity: SlackIdentity) -> None: + """Reject identities from a different workspace when one is configured.""" + if SLACK_TEAM_ID and identity.team_id != SLACK_TEAM_ID: + raise HTTPException(403, "Slack account is not in the authorized workspace") + + +async def exchange_slack_code(code: str, redirect_uri: str) -> str: + """Exchange an authorization code for a user access token.""" + async with httpx.AsyncClient() as client: + resp = await client.post( + _TOKEN_URL, + data={ + "client_id": SLACK_CLIENT_ID, + "client_secret": SLACK_CLIENT_SECRET, + "code": code, + "grant_type": "authorization_code", + "redirect_uri": redirect_uri, + }, + ) + resp.raise_for_status() + data = resp.json() + if not data.get("ok") or not data.get("access_token"): + raise HTTPException(400, f"slack oauth exchange failed: {data.get('error', 'unknown')}") + return data["access_token"] + + +async def fetch_slack_identity(access_token: str) -> SlackIdentity: + """Resolve the signed-in Slack user's verified identity.""" + async with httpx.AsyncClient() as client: + resp = await client.get( + _USERINFO_URL, + headers={"Authorization": f"Bearer {access_token}"}, + ) + resp.raise_for_status() + return parse_slack_identity(resp.json()) diff --git a/agent/dashboard/user_mappings.py b/agent/dashboard/user_mappings.py index 984c4bfe..f4056c47 100644 --- a/agent/dashboard/user_mappings.py +++ b/agent/dashboard/user_mappings.py @@ -35,7 +35,7 @@ logger = logging.getLogger(__name__) USER_MAPPINGS_NAMESPACE: list[str] = ["user_mappings"] -MappingSource = Literal["hardcoded", "self", "admin"] +MappingSource = Literal["hardcoded", "self", "admin", "slack_oauth"] MappingStatus = Literal["active", "pending"] diff --git a/agent/prompt.py b/agent/prompt.py index d3871b47..ea4c3e8e 100644 --- a/agent/prompt.py +++ b/agent/prompt.py @@ -1,8 +1,13 @@ import logging import os +import shlex from pathlib import Path -from .utils.authorship import CollaboratorIdentity +from .utils.authorship import ( + OPEN_SWE_BOT_EMAIL, + OPEN_SWE_BOT_NAME, + CollaboratorIdentity, +) from .utils.github_comments import UNTRUSTED_GITHUB_COMMENT_OPEN_TAG logger = logging.getLogger(__name__) @@ -90,10 +95,10 @@ Before starting any task that requires code changes, set up the repository in yo 3. **Set the commit identity** — IMMEDIATELY after cloning, `cd` into the repo and run: ```bash - git config user.name 'open-swe[bot]' && git config user.email 'open-swe@users.noreply.github.com' + git config user.name {commit_identity_name} && git config user.email {commit_identity_email} ``` - This is required: third-party CI integrations (e.g. Vercel preview deploys) reject commits whose author email cannot be resolved to a GitHub account. Do NOT set any other identity, do NOT pass `--author` to `git commit`, and do NOT export `GIT_AUTHOR_*` / `GIT_COMMITTER_*` env vars. + This sets the author of every commit you make. This is required for CI: third-party integrations (e.g. Vercel preview deploys) reject commits whose author email cannot be resolved to a GitHub account, and this email resolves. Do NOT set any other identity, do NOT pass `--author` to `git commit`, and do NOT export `GIT_AUTHOR_*` / `GIT_COMMITTER_*` env vars. 4. **Choose your branch** — Use a thread-stable branch name such as `open-swe/`. If a branch already exists for this thread/task, fetch and check it out instead of creating a new one. @@ -360,12 +365,12 @@ COLLABORATION_TEMPLATE = """--- ### Collaborative Attribution -This run was triggered by **{display_name}**. Credit them on every commit and PR you create: +This run was triggered by **{display_name}**. You author the work **as them** — their git identity is already configured in the Repository Setup step, so every commit and the PR are attributed to them. Credit open-swe as the collaborator: - **Commits**: append this trailer (verbatim, on its own line, separated from the message body by a blank line) to every commit message you author. Add it to both the first commit and any follow-up commits in this run: ``` - Co-authored-by: {commit_name} <{commit_email}> + Co-authored-by: open-swe[bot] ``` - **PR body**: append this line to the bottom of the PR description (separated from the body by a blank line) when you open or update the draft PR. Do not duplicate it if it is already present. If the PR body already contains the legacy footer `_Opened collaboratively by {display_name} and open-swe._`, replace that legacy footer with this line instead of appending a second footer: @@ -382,8 +387,6 @@ def _render_collaboration_section(identity: CollaboratorIdentity | None) -> str: return "" return COLLABORATION_TEMPLATE.format( display_name=identity.display_name, - commit_name=identity.commit_name, - commit_email=identity.commit_email, pr_attribution_name=identity.pr_attribution_name, ) @@ -425,6 +428,14 @@ def construct_system_prompt( create_prs: bool = False, ) -> str: default_prompt_section = _load_default_prompt() + # Shell-escape: display names/emails are user-controlled (e.g. O'Connor) and + # are embedded in a `git config` command the agent copies verbatim. + if triggering_user_identity is not None: + commit_identity_name = shlex.quote(triggering_user_identity.commit_name) + commit_identity_email = shlex.quote(triggering_user_identity.commit_email) + else: + commit_identity_name = shlex.quote(OPEN_SWE_BOT_NAME) + commit_identity_email = shlex.quote(OPEN_SWE_BOT_EMAIL) return SYSTEM_PROMPT_TEMPLATE.format( working_dir=working_dir, linear_project_id=linear_project_id or "", @@ -432,4 +443,6 @@ def construct_system_prompt( default_prompt_section=default_prompt_section, pr_policy_override_section=ALWAYS_CREATE_PR_SECTION if create_prs else "", collaboration_section=_render_collaboration_section(triggering_user_identity), + commit_identity_name=commit_identity_name, + commit_identity_email=commit_identity_email, ) diff --git a/agent/utils/auth.py b/agent/utils/auth.py index a42508ae..2a45d1da 100644 --- a/agent/utils/auth.py +++ b/agent/utils/auth.py @@ -23,6 +23,21 @@ logger = logging.getLogger(__name__) client = get_client() + +class GitHubUserAuthRequired(RuntimeError): + """Raised when a mapped user has no valid GitHub OAuth token. + + Signals that the run cannot proceed on the user's behalf and that the user + must (re-)authenticate. The Slack webhook blocks before creating a run, so + this is a defense-in-depth signal at execution time. + """ + + def __init__(self, source: str, github_login: str | None) -> None: + self.source = source + self.github_login = github_login + super().__init__(f"GitHub authentication required for {source} user '{github_login}'") + + LANGSMITH_API_KEY = os.environ.get("LANGSMITH_API_KEY_PROD", "") LANGSMITH_API_URL = os.environ.get("LANGSMITH_ENDPOINT", "https://api.smith.langchain.com") LANGSMITH_HOST_API_URL = os.environ.get("LANGSMITH_HOST_API_URL", "https://api.host.langchain.com") @@ -361,6 +376,36 @@ async def save_encrypted_token_from_email( return token, encrypted, expires_at +async def _resolve_dashboard_user_token( + thread_id: str, github_login: str +) -> tuple[str, str, str | None] | None: + """Resolve a per-user GitHub token from the dashboard OAuth store. + + Returns the ``(token, encrypted, expires_at)`` tuple, or ``None`` when the + user has no valid token (never linked, or expired/revoked beyond refresh). + + The thread-metadata token cache is intentionally NOT consulted here: Slack + thread ids are shared across everyone in a conversation, so a cached token + from a prior triggering user would impersonate the current ``github_login``. + We always resolve by login from the dashboard store instead. + """ + login = github_login.strip() + if not login: + raise ValueError("missing github_login") + + from ..dashboard.profiles import OAUTH_TOKENS_NAMESPACE, get_valid_access_token + from ..dashboard.profiles import _get_value as get_oauth_record + + token = await get_valid_access_token(login) + if not token: + return None + record = await get_oauth_record(OAUTH_TOKENS_NAMESPACE, login) + expires_at = record.get("token_expires_at") if isinstance(record, dict) else None + expires_at = expires_at if isinstance(expires_at, str) else None + encrypted = await persist_encrypted_github_token(thread_id, token, expires_at=expires_at) + return token, encrypted, expires_at + + async def _resolve_bot_installation_token(thread_id: str) -> tuple[str, str, str | None]: """Get a GitHub App installation token and persist it for the thread.""" bot_token, expires_at = await get_github_app_installation_token_with_expiry() @@ -396,24 +441,32 @@ async def resolve_github_token( Raises: RuntimeError: If source is missing or token resolution fails. """ - if is_bot_token_only_mode(): - return await _resolve_bot_installation_token(thread_id) - configurable = config["configurable"] source = configurable.get("source") if not source: logger.error("Missing source for thread %s; cannot route auth failure responses", thread_id) raise RuntimeError(f"GitHub auth failed for thread {thread_id}: missing source") - # Unmapped Slack/Linear users still get a run on the GitHub App installation - # token (the webhook posts a "link your account" prompt separately). This - # keeps the agent responsive while self-service onboarding completes. - if configurable.get("use_installation_token_fallback"): - cached_token, cached_encrypted, cached_expires_at = await get_github_token_from_thread( - thread_id - ) - if cached_token and cached_encrypted: - return cached_token, cached_encrypted, cached_expires_at + github_login = configurable.get("github_login") + + # Per-user OAuth from the dashboard store wins even in bot-token-only mode, + # for sources that carry a mapped GitHub login (Slack, dashboard). This is + # what lets the agent open PRs as the triggering user. + if source in ("slack", "dashboard") and isinstance(github_login, str) and github_login.strip(): + try: + user_token = await _resolve_dashboard_user_token(thread_id, github_login) + except ValueError as exc: + logger.error("GitHub auth failed for thread %s: %s", thread_id, str(exc)) + raise RuntimeError(str(exc)) from exc + if user_token is not None: + return user_token + # No valid user token. In bot-token-only mode fall back to the bot so the + # deployment stays functional; otherwise block and require auth. + if is_bot_token_only_mode(): + return await _resolve_bot_installation_token(thread_id) + raise GitHubUserAuthRequired(source, github_login) + + if is_bot_token_only_mode(): return await _resolve_bot_installation_token(thread_id) try: @@ -423,36 +476,12 @@ async def resolve_github_token( ) if cached_token and cached_encrypted: return cached_token, cached_encrypted, cached_expires_at - github_login = configurable.get("github_login") from ..dashboard.user_mappings import email_for_login email = await email_for_login(github_login) if not email: raise ValueError(f"No email mapping found for GitHub user '{github_login}'") return await save_encrypted_token_from_email(email, source) - if source == "dashboard": - cached_token, cached_encrypted, cached_expires_at = await get_github_token_from_thread( - thread_id - ) - if cached_token and cached_encrypted: - return cached_token, cached_encrypted, cached_expires_at - github_login = configurable.get("github_login") - if not isinstance(github_login, str) or not github_login.strip(): - raise ValueError("missing github_login for dashboard run") - from ..dashboard.profiles import OAUTH_TOKENS_NAMESPACE, get_valid_access_token - from ..dashboard.profiles import _get_value as get_oauth_record - - token = await get_valid_access_token(github_login.strip()) - if not token: - raise ValueError("github token unavailable, re-login required") - record = await get_oauth_record(OAUTH_TOKENS_NAMESPACE, github_login.strip()) - expires_at = record.get("token_expires_at") if isinstance(record, dict) else None - encrypted = await persist_encrypted_github_token( - thread_id, - token, - expires_at=expires_at if isinstance(expires_at, str) else None, - ) - return token, encrypted, expires_at if isinstance(expires_at, str) else None return await save_encrypted_token_from_email(configurable.get("user_email"), source) except ValueError as exc: logger.error("GitHub auth failed for thread %s: %s", thread_id, str(exc)) diff --git a/agent/utils/authorship.py b/agent/utils/authorship.py index 6cfc0f07..9a2c4d69 100644 --- a/agent/utils/authorship.py +++ b/agent/utils/authorship.py @@ -138,16 +138,14 @@ def resolve_triggering_user_identity( return _identity_from_github_token(github_token) or _identity_from_config(config) -def add_user_coauthor_trailer( - commit_message: str, - identity: CollaboratorIdentity | None, -) -> str: - """Append a Co-authored-by trailer when a user identity is available.""" - normalized_message = commit_message.rstrip() - if not identity: - return normalized_message +def add_bot_coauthor_trailer(commit_message: str) -> str: + """Append the open-swe[bot] Co-authored-by trailer. - trailer = f"Co-authored-by: {identity.commit_name} <{identity.commit_email}>" + Commits are authored by the triggering user (via the repo-local git + identity); open-swe[bot] is credited as the collaborator. + """ + normalized_message = commit_message.rstrip() + trailer = f"Co-authored-by: {OPEN_SWE_BOT_NAME} <{OPEN_SWE_BOT_EMAIL}>" if trailer in normalized_message: return normalized_message return f"{normalized_message}\n\n{trailer}" diff --git a/agent/webapp.py b/agent/webapp.py index 98371879..bf2179c6 100644 --- a/agent/webapp.py +++ b/agent/webapp.py @@ -26,7 +26,7 @@ from .dashboard.agent_overrides import ( ) from .dashboard.enabled_repos import is_review_repo_enabled from .dashboard.oauth import build_account_link_url -from .dashboard.profiles import get_profile +from .dashboard.profiles import get_profile, get_valid_access_token from .dashboard.team_settings import get_team_settings from .dashboard.user_mappings import ( email_for_login, @@ -869,18 +869,34 @@ async def process_linear_issue( # noqa: PLR0912, PLR0915 async def _post_account_link_prompt( - channel_id: str, thread_ts: str, user_id: str, user_email: str | None + channel_id: str, + thread_ts: str, + user_id: str, + user_email: str | None, + reason: str = "unlinked", ) -> None: - """Prompt an unmapped Slack user to link their GitHub account (ephemeral).""" + """Prompt a Slack user to (re-)link their GitHub account (ephemeral). + + ``reason`` is ``"unlinked"`` (no mapping yet) or ``"expired"`` (mapped but + the stored GitHub authorization is missing/expired/revoked). Open SWE opens + PRs as the triggering user, so it cannot start until the account is linked. + """ link_url = build_account_link_url(slack_user_id=user_id, work_email=user_email) if not link_url: logger.debug("Account-link URL unavailable (DASHBOARD_API_BASE_URL unset); skipping prompt") return - text = ( - "👋 I don't have your GitHub account linked yet, so I'm running with limited " - "(bot) permissions. Link your account so I can act on your behalf:\n" - f"<{link_url}|Link your GitHub account>" - ) + if reason == "expired": + text = ( + "🔐 Your GitHub authorization has expired or was revoked, so I can't act on " + "your behalf. Re-link your account to continue:\n" + f"<{link_url}|Re-link your GitHub account>" + ) + else: + text = ( + "👋 I don't have your GitHub account linked yet, so I can't open PRs on your " + "behalf. I won't start until you link your account:\n" + f"<{link_url}|Link your GitHub account>" + ) try: await post_slack_ephemeral_message(channel_id, user_id, text, thread_ts=thread_ts) except Exception: # noqa: BLE001 @@ -1010,6 +1026,37 @@ async def process_slack_mention(event_data: dict[str, Any], repo_config: dict[st mapped_login = await login_for_email(user_email) is_user_mapped = bool(mapped_login) + # Open SWE opens PRs as the triggering user, so a run only proceeds when we + # have a valid user GitHub token. Unmapped users, and mapped users whose + # token is missing/expired/revoked, are blocked and prompted to (re-)link. + # Bot-token-only deployments are exempt — they run on the installation token. + user_token: str | None = None + if mapped_login: + try: + user_token = await get_valid_access_token(mapped_login) + except Exception: # noqa: BLE001 + logger.debug( + "Failed to resolve GitHub token for %s; treating as unauthenticated", + mapped_login, + exc_info=True, + ) + user_token = None + has_valid_user_token = bool(user_token) + + if not has_valid_user_token and not is_bot_token_only_mode(): + reason = "expired" if is_user_mapped else "unlinked" + logger.info( + "Blocking Slack run for thread %s: no valid user GitHub token (%s)", + thread_id, + reason, + ) + if user_id: + await _post_account_link_prompt( + channel_id, thread_ts, user_id, user_email, reason=reason + ) + await set_slack_assistant_status(channel_id, thread_ts, status="") + return + configurable: dict[str, Any] = { "repo": repo_config, "slack_thread": { @@ -1025,9 +1072,6 @@ async def process_slack_mention(event_data: dict[str, Any], repo_config: dict[st } if mapped_login: configurable["github_login"] = mapped_login - else: - # Unmapped: run on the installation token and prompt the user to link. - configurable["use_installation_token_fallback"] = True langgraph_client = get_client(url=LANGGRAPH_URL) is_first_mention = not await _thread_exists(thread_id) @@ -1072,8 +1116,6 @@ async def process_slack_mention(event_data: dict[str, Any], repo_config: dict[st thread_id, ) run_id = run.get("run_id") - if is_first_mention and not is_user_mapped and user_id: - await _post_account_link_prompt(channel_id, thread_ts, user_id, user_email) if is_first_mention: trace_message_ts = await post_slack_trace_reply(channel_id, thread_ts, thread_id) await set_slack_assistant_status(channel_id, thread_ts) diff --git a/tests/test_account_link.py b/tests/test_account_link.py index 1f0c5060..8133854f 100644 --- a/tests/test_account_link.py +++ b/tests/test_account_link.py @@ -34,17 +34,31 @@ def test_decode_account_link_rejects_wrong_kind() -> None: def test_build_account_link_url(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("DASHBOARD_API_BASE_URL", "https://api.example.com/") + monkeypatch.delenv("DASHBOARD_BASE_URL", raising=False) url = oauth.build_account_link_url(slack_user_id="U1", work_email="d@x.com") assert url is not None assert url.startswith("https://api.example.com/dashboard/api/auth/login?link=") # The embedded token must decode back to the same identity. - token = url.split("link=", 1)[1] + token = url.split("link=", 1)[1].split("&", 1)[0] from urllib.parse import unquote payload = oauth.decode_account_link(unquote(token)) assert payload["slack_user_id"] == "U1" +def test_build_account_link_url_redirects_to_profile_settings( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("DASHBOARD_API_BASE_URL", "https://api.example.com") + monkeypatch.setenv("DASHBOARD_BASE_URL", "https://app.example.com") + url = oauth.build_account_link_url(slack_user_id="U1", work_email="d@x.com") + assert url is not None + from urllib.parse import parse_qs, urlparse + + query = parse_qs(urlparse(url).query) + assert query["redirect_to"] == ["https://app.example.com/my-settings"] + + def test_build_account_link_url_none_without_base(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.delenv("DASHBOARD_API_BASE_URL", raising=False) assert oauth.build_account_link_url(slack_user_id="U1", work_email="d@x.com") is None diff --git a/tests/test_auth_sources.py b/tests/test_auth_sources.py index 951f260e..dd42fea6 100644 --- a/tests/test_auth_sources.py +++ b/tests/test_auth_sources.py @@ -85,3 +85,127 @@ def test_leave_failure_comment_falls_back_to_slack_thread_when_ephemeral_fails( asyncio.run(auth.leave_failure_comment("slack", "auth failed")) assert thread_called == {"channel_id": "C123", "thread_ts": "1.2", "message": "auth failed"} + + +def _slack_config(github_login: str | None = "mason-gh") -> dict: + configurable: dict = { + "source": "slack", + "user_email": "mason@example.com", + "thread_id": "t1", + } + if github_login is not None: + configurable["github_login"] = github_login + return {"configurable": configurable} + + +def _stub_dashboard_store( + monkeypatch: pytest.MonkeyPatch, + *, + token: str | None, + expires_at: str | None = "2099-01-01T00:00:00Z", + cached: tuple[str | None, str | None, str | None] = (None, None, None), +) -> None: + from agent.dashboard import profiles + + async def fake_get_from_thread(thread_id: str): + return cached + + async def fake_get_valid(login: str): + return token + + async def fake_get_value(namespace, key): + return {"token_expires_at": expires_at} + + async def fake_persist(thread_id: str, tok: str, expires_at: str | None = None): + return "enc" + + monkeypatch.setattr(auth, "get_github_token_from_thread", fake_get_from_thread) + monkeypatch.setattr(auth, "persist_encrypted_github_token", fake_persist) + monkeypatch.setattr(profiles, "get_valid_access_token", fake_get_valid) + monkeypatch.setattr(profiles, "_get_value", fake_get_value) + + +def test_resolve_github_token_slack_uses_dashboard_store( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _stub_dashboard_store(monkeypatch, token="user-tok") + monkeypatch.setattr(auth, "is_bot_token_only_mode", lambda: False) + + token, encrypted, expires_at = asyncio.run(auth.resolve_github_token(_slack_config(), "t1")) + + assert token == "user-tok" + assert encrypted == "enc" + assert expires_at == "2099-01-01T00:00:00Z" + + +def test_resolve_github_token_slack_ignores_stale_thread_cache( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Slack thread ids are shared, so a prior user's cached token must NOT be + # returned. Resolution always goes by github_login via the dashboard store. + _stub_dashboard_store( + monkeypatch, + token="bob-token", + cached=("alice-token", "alice-enc", "2099-01-01T00:00:00Z"), + ) + monkeypatch.setattr(auth, "is_bot_token_only_mode", lambda: False) + + token, _, _ = asyncio.run(auth.resolve_github_token(_slack_config(), "t1")) + + assert token == "bob-token" + + +def test_resolve_github_token_slack_no_token_raises( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _stub_dashboard_store(monkeypatch, token=None) + monkeypatch.setattr(auth, "is_bot_token_only_mode", lambda: False) + + with pytest.raises(auth.GitHubUserAuthRequired): + asyncio.run(auth.resolve_github_token(_slack_config(), "t1")) + + +def test_resolve_github_token_per_user_wins_over_bot_only_mode( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _stub_dashboard_store(monkeypatch, token="user-tok") + monkeypatch.setattr(auth, "is_bot_token_only_mode", lambda: True) + + async def fail_bot(thread_id: str): + raise AssertionError("bot token must not be used when a user token exists") + + monkeypatch.setattr(auth, "_resolve_bot_installation_token", fail_bot) + + token, _, _ = asyncio.run(auth.resolve_github_token(_slack_config(), "t1")) + assert token == "user-tok" + + +def test_resolve_github_token_slack_no_token_falls_back_to_bot_in_bot_only_mode( + monkeypatch: pytest.MonkeyPatch, +) -> None: + _stub_dashboard_store(monkeypatch, token=None) + monkeypatch.setattr(auth, "is_bot_token_only_mode", lambda: True) + + async def fake_bot(thread_id: str): + return ("bot-tok", "bot-enc", None) + + monkeypatch.setattr(auth, "_resolve_bot_installation_token", fake_bot) + + token, encrypted, expires_at = asyncio.run(auth.resolve_github_token(_slack_config(), "t1")) + assert (token, encrypted, expires_at) == ("bot-tok", "bot-enc", None) + + +@pytest.mark.parametrize("source", ["github", "linear"]) +def test_resolve_github_token_bot_only_mode_non_slack_uses_bot( + monkeypatch: pytest.MonkeyPatch, source: str +) -> None: + monkeypatch.setattr(auth, "is_bot_token_only_mode", lambda: True) + + async def fake_bot(thread_id: str): + return ("bot-tok", "bot-enc", None) + + monkeypatch.setattr(auth, "_resolve_bot_installation_token", fake_bot) + + config = {"configurable": {"source": source, "github_login": "octo", "thread_id": "t1"}} + token, _, _ = asyncio.run(auth.resolve_github_token(config, "t1")) + assert token == "bot-tok" diff --git a/tests/test_authorship.py b/tests/test_authorship.py new file mode 100644 index 00000000..bb021289 --- /dev/null +++ b/tests/test_authorship.py @@ -0,0 +1,36 @@ +from __future__ import annotations + +from agent.utils.authorship import ( + OPEN_SWE_BOT_EMAIL, + OPEN_SWE_BOT_NAME, + add_bot_coauthor_trailer, + resolve_triggering_user_identity, +) + +_BOT_TRAILER = f"Co-authored-by: {OPEN_SWE_BOT_NAME} <{OPEN_SWE_BOT_EMAIL}>" + + +def test_add_bot_coauthor_trailer_appends_bot() -> None: + result = add_bot_coauthor_trailer("fix: thing") + assert result == f"fix: thing\n\n{_BOT_TRAILER}" + + +def test_add_bot_coauthor_trailer_is_idempotent() -> None: + once = add_bot_coauthor_trailer("fix: thing") + assert add_bot_coauthor_trailer(once) == once + + +def test_resolve_identity_from_config_uses_user_noreply_email() -> None: + config = { + "configurable": { + "source": "slack", + "github_login": "mason-gh", + "github_user_id": 4321, + "slack_thread": {"triggering_user_name": "Mason"}, + } + } + identity = resolve_triggering_user_identity(config) + assert identity is not None + assert identity.commit_name == "Mason" + assert identity.commit_email == "4321+mason-gh@users.noreply.github.com" + assert identity.github_login == "mason-gh" diff --git a/tests/test_github_comment_prompts.py b/tests/test_github_comment_prompts.py index 642c263a..e0626b4a 100644 --- a/tests/test_github_comment_prompts.py +++ b/tests/test_github_comment_prompts.py @@ -96,7 +96,11 @@ def test_construct_system_prompt_includes_coauthor_trailer_when_identity_present ) assert "Collaborative Attribution" in prompt - assert "Co-authored-by: octocat <1234+octocat@users.noreply.github.com>" in prompt + # The user authors the commits; open-swe[bot] is the co-author/collaborator. + # Values are shell-escaped via shlex.quote; safe tokens need no quoting. + assert "git config user.name octocat" in prompt + assert "git config user.email 1234+octocat@users.noreply.github.com" in prompt + assert "Co-authored-by: open-swe[bot] " in prompt assert "_Opened collaboratively by octocat and open-swe._" in prompt @@ -113,7 +117,10 @@ def test_construct_system_prompt_includes_github_login_in_pr_footer() -> None: triggering_user_identity=identity, ) - assert "Co-authored-by: Mona Lisa <1234+octocat@users.noreply.github.com>" in prompt + # A name with a space is shlex-quoted; the safe email is left bare. + assert "git config user.name 'Mona Lisa'" in prompt + assert "git config user.email 1234+octocat@users.noreply.github.com" in prompt + assert "Co-authored-by: open-swe[bot] " in prompt assert "_Opened collaboratively by Mona Lisa (@octocat) and open-swe._" in prompt assert ( "replace that legacy footer with this line instead of appending a second footer" in prompt @@ -121,6 +128,27 @@ def test_construct_system_prompt_includes_github_login_in_pr_footer() -> None: assert "`_Opened collaboratively by Mona Lisa and open-swe._`" in prompt +def test_construct_system_prompt_shell_escapes_user_name() -> None: + import shlex + + hostile = "O'Connor'; rm -rf / #" + identity = CollaboratorIdentity( + display_name=hostile, + commit_name=hostile, + commit_email="1234+oconnor@users.noreply.github.com", + github_login="oconnor", + ) + + prompt = construct_system_prompt( + working_dir="/workspace", + triggering_user_identity=identity, + ) + + assert f"git config user.name {shlex.quote(hostile)}" in prompt + # The raw, unescaped name must never appear as a bare shell argument. + assert f"git config user.name {hostile}" not in prompt + + def test_add_pr_collaboration_note_replaces_legacy_footer() -> None: identity = CollaboratorIdentity( display_name="Mona Lisa", diff --git a/tests/test_slack_context.py b/tests/test_slack_context.py index f008acc9..6d54c5f1 100644 --- a/tests/test_slack_context.py +++ b/tests/test_slack_context.py @@ -470,9 +470,30 @@ def _setup_slack_mention_fakes( monkeypatch.setattr( webapp, "resolve_slack_links_in_context", fake_resolve_slack_links_in_context ) + + async def fake_login_for_slack_id(slack_user_id): + return "mason-gh" + + async def fake_login_for_email(email): + return None + + async def fake_refresh_cache() -> list: + return [] + + async def fake_get_valid_access_token(login): + return "user-token" + + async def fake_post_prompt(*args, **kwargs) -> None: + captured["prompt"] = {"args": args, "kwargs": kwargs} + monkeypatch.setattr(webapp, "is_thread_active", fake_is_thread_active) monkeypatch.setattr(webapp, "post_slack_trace_reply", fake_post_slack_trace_reply) monkeypatch.setattr(webapp, "get_client", lambda url: _FakeLangGraphClientForProcess()) + monkeypatch.setattr(webapp, "login_for_slack_id", fake_login_for_slack_id) + monkeypatch.setattr(webapp, "login_for_email", fake_login_for_email) + monkeypatch.setattr(webapp, "refresh_user_mapping_cache", fake_refresh_cache) + monkeypatch.setattr(webapp, "get_valid_access_token", fake_get_valid_access_token) + monkeypatch.setattr(webapp, "_post_account_link_prompt", fake_post_prompt) def test_process_slack_mention_creates_thread_first_run_with_trace_reply( @@ -652,6 +673,23 @@ def test_process_slack_mention_queues_active_thread_message( monkeypatch.setattr(webapp, "_thread_exists", fake_thread_exists) monkeypatch.setattr(webapp, "get_client", lambda url: _FakeLangGraphClientForProcess()) + async def fake_login_for_slack_id(slack_user_id): + return "mason-gh" + + async def fake_login_for_email(email): + return None + + async def fake_refresh_cache() -> list: + return [] + + async def fake_get_valid_access_token(login): + return "user-token" + + monkeypatch.setattr(webapp, "login_for_slack_id", fake_login_for_slack_id) + monkeypatch.setattr(webapp, "login_for_email", fake_login_for_email) + monkeypatch.setattr(webapp, "refresh_user_mapping_cache", fake_refresh_cache) + monkeypatch.setattr(webapp, "get_valid_access_token", fake_get_valid_access_token) + thread_ts = "1700000000.000100" event_ts = "1700000000.000200" expected_thread_id = generate_thread_id_from_slack_thread("C123", thread_ts) @@ -677,10 +715,10 @@ def test_process_slack_mention_queues_active_thread_message( assert "## Latest Mention Request\ninclude this screenshot" in queued_payload["text"] -def test_process_slack_mention_unmapped_user_uses_fallback_and_prompts( +def test_process_slack_mention_unmapped_user_blocked_and_prompted( monkeypatch: pytest.MonkeyPatch, ) -> None: - """An unmapped Slack user runs on the installation token and is prompted to link.""" + """An unmapped Slack user is blocked (no run) and prompted to link.""" from agent.dashboard import user_mappings captured: dict[str, object] = {} @@ -690,20 +728,16 @@ def test_process_slack_mention_unmapped_user_uses_fallback_and_prompts( async def fake_thread_exists(thread_id: str) -> bool: return False - async def fake_refresh_cache() -> list: - return [] - async def fake_login_for_slack_id(slack_user_id): return None async def fake_login_for_email(email): return None - async def fake_post_prompt(channel_id, thread_ts, user_id, user_email) -> None: - captured["prompt"] = {"user_id": user_id, "user_email": user_email} + async def fake_post_prompt(channel_id, thread_ts, user_id, user_email, reason="unlinked"): + captured["prompt"] = {"user_id": user_id, "user_email": user_email, "reason": reason} monkeypatch.setattr(webapp, "_thread_exists", fake_thread_exists) - monkeypatch.setattr(webapp, "refresh_user_mapping_cache", fake_refresh_cache) monkeypatch.setattr(webapp, "login_for_slack_id", fake_login_for_slack_id) monkeypatch.setattr(webapp, "login_for_email", fake_login_for_email) monkeypatch.setattr(webapp, "_post_account_link_prompt", fake_post_prompt) @@ -722,36 +756,71 @@ def test_process_slack_mention_unmapped_user_uses_fallback_and_prompts( ) ) - run_create = captured["run_create"] - configurable = run_create["kwargs"]["config"]["configurable"] - assert configurable["use_installation_token_fallback"] is True - assert "github_login" not in configurable - assert captured["prompt"] == {"user_id": "U123", "user_email": "mason@example.com"} + assert "run_create" not in captured + assert captured["prompt"] == { + "user_id": "U123", + "user_email": "mason@example.com", + "reason": "unlinked", + } -def test_process_slack_mention_mapped_user_no_prompt( +def test_process_slack_mention_mapped_user_no_token_blocked_and_prompted( monkeypatch: pytest.MonkeyPatch, ) -> None: - """A mapped Slack user runs as themselves with no link prompt and no fallback.""" + """A mapped Slack user with no valid token is blocked and prompted to re-link.""" captured: dict[str, object] = {} _setup_slack_mention_fakes(monkeypatch, captured) async def fake_thread_exists(thread_id: str) -> bool: return False - async def fake_refresh_cache() -> list: - return [] + async def fake_login_for_slack_id(slack_user_id): + return "mason-gh" if slack_user_id == "U123" else None + + async def fake_get_valid_access_token(login): + return None + + async def fake_post_prompt(channel_id, thread_ts, user_id, user_email, reason="unlinked"): + captured["prompt"] = {"reason": reason} + + monkeypatch.setattr(webapp, "_thread_exists", fake_thread_exists) + monkeypatch.setattr(webapp, "login_for_slack_id", fake_login_for_slack_id) + monkeypatch.setattr(webapp, "get_valid_access_token", fake_get_valid_access_token) + monkeypatch.setattr(webapp, "_post_account_link_prompt", fake_post_prompt) + + asyncio.run( + webapp.process_slack_mention( + { + "channel_id": "C123", + "thread_ts": "1700000000.000100", + "event_ts": "1700000000.000200", + "user_id": "U123", + "text": "<@UBOT> do the thing", + "bot_user_id": "UBOT", + }, + {"owner": "langchain-ai", "name": "open-swe"}, + ) + ) + + assert "run_create" not in captured + assert captured["prompt"] == {"reason": "expired"} + + +def test_process_slack_mention_mapped_user_with_token_runs_as_user( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A mapped, authenticated Slack user runs as themselves with no prompt.""" + captured: dict[str, object] = {} + _setup_slack_mention_fakes(monkeypatch, captured) + + async def fake_thread_exists(thread_id: str) -> bool: + return False async def fake_login_for_slack_id(slack_user_id): return "mason-gh" if slack_user_id == "U123" else None - async def fake_post_prompt(*args, **kwargs) -> None: - captured["prompt"] = True - monkeypatch.setattr(webapp, "_thread_exists", fake_thread_exists) - monkeypatch.setattr(webapp, "refresh_user_mapping_cache", fake_refresh_cache) monkeypatch.setattr(webapp, "login_for_slack_id", fake_login_for_slack_id) - monkeypatch.setattr(webapp, "_post_account_link_prompt", fake_post_prompt) asyncio.run( webapp.process_slack_mention( @@ -772,3 +841,42 @@ def test_process_slack_mention_mapped_user_no_prompt( assert configurable["github_login"] == "mason-gh" assert "use_installation_token_fallback" not in configurable assert "prompt" not in captured + + +def test_process_slack_mention_bot_only_mode_runs_without_user_token( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """In bot-token-only mode an unmapped user still gets a run (no blocking).""" + captured: dict[str, object] = {} + _setup_slack_mention_fakes(monkeypatch, captured) + + async def fake_thread_exists(thread_id: str) -> bool: + return False + + async def fake_login_for_slack_id(slack_user_id): + return None + + async def fake_login_for_email(email): + return None + + monkeypatch.setattr(webapp, "_thread_exists", fake_thread_exists) + monkeypatch.setattr(webapp, "login_for_slack_id", fake_login_for_slack_id) + monkeypatch.setattr(webapp, "login_for_email", fake_login_for_email) + monkeypatch.setattr(webapp, "is_bot_token_only_mode", lambda: True) + + asyncio.run( + webapp.process_slack_mention( + { + "channel_id": "C123", + "thread_ts": "1700000000.000100", + "event_ts": "1700000000.000200", + "user_id": "U123", + "text": "<@UBOT> do the thing", + "bot_user_id": "UBOT", + }, + {"owner": "langchain-ai", "name": "open-swe"}, + ) + ) + + assert "run_create" in captured + assert "prompt" not in captured diff --git a/tests/test_slack_oauth.py b/tests/test_slack_oauth.py new file mode 100644 index 00000000..a0d8b463 --- /dev/null +++ b/tests/test_slack_oauth.py @@ -0,0 +1,86 @@ +from __future__ import annotations + +from urllib.parse import parse_qs, urlparse + +import pytest +from fastapi import HTTPException + +from agent.dashboard import slack_oauth + + +def test_slack_oauth_configured(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(slack_oauth, "SLACK_CLIENT_ID", "cid") + monkeypatch.setattr(slack_oauth, "SLACK_CLIENT_SECRET", "secret") + assert slack_oauth.slack_oauth_configured() is True + monkeypatch.setattr(slack_oauth, "SLACK_CLIENT_SECRET", "") + assert slack_oauth.slack_oauth_configured() is False + + +def test_build_authorize_url(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(slack_oauth, "SLACK_CLIENT_ID", "cid") + monkeypatch.setattr(slack_oauth, "SLACK_TEAM_ID", "") + url = slack_oauth.build_authorize_url( + redirect_uri="http://localhost:2024/dashboard/api/slack/callback", state="ST8" + ) + parsed = urlparse(url) + q = parse_qs(parsed.query) + assert parsed.netloc == "slack.com" + assert q["response_type"] == ["code"] + assert q["scope"] == ["openid email profile"] + assert q["client_id"] == ["cid"] + assert q["redirect_uri"] == ["http://localhost:2024/dashboard/api/slack/callback"] + assert q["state"] == ["ST8"] + assert "team" not in q + + +def test_build_authorize_url_includes_team_when_configured( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(slack_oauth, "SLACK_CLIENT_ID", "cid") + monkeypatch.setattr(slack_oauth, "SLACK_TEAM_ID", "T123") + url = slack_oauth.build_authorize_url(redirect_uri="https://x/cb", state="S") + assert parse_qs(urlparse(url).query)["team"] == ["T123"] + + +def test_parse_slack_identity_success() -> None: + identity = slack_oauth.parse_slack_identity( + { + "ok": True, + "https://slack.com/user_id": "U999", + "https://slack.com/team_id": "T123", + "email": "dev@example.com", + "email_verified": True, + "name": "Dev", + } + ) + assert identity.user_id == "U999" + assert identity.team_id == "T123" + assert identity.email == "dev@example.com" + assert identity.email_verified is True + assert identity.name == "Dev" + + +def test_parse_slack_identity_missing_user_id() -> None: + with pytest.raises(HTTPException): + slack_oauth.parse_slack_identity({"ok": True, "email": "x@y.com"}) + + +def test_parse_slack_identity_not_ok() -> None: + with pytest.raises(HTTPException): + slack_oauth.parse_slack_identity({"ok": False, "error": "bad"}) + + +def test_verify_team(monkeypatch: pytest.MonkeyPatch) -> None: + ident = slack_oauth.SlackIdentity( + user_id="U1", team_id="T1", email="a@b.com", email_verified=True, name=None + ) + # No workspace restriction configured → always allowed. + monkeypatch.setattr(slack_oauth, "SLACK_TEAM_ID", "") + slack_oauth.verify_team(ident) + # Matching workspace → allowed. + monkeypatch.setattr(slack_oauth, "SLACK_TEAM_ID", "T1") + slack_oauth.verify_team(ident) + # Different workspace → rejected. + monkeypatch.setattr(slack_oauth, "SLACK_TEAM_ID", "T2") + with pytest.raises(HTTPException): + slack_oauth.verify_team(ident) diff --git a/ui/src/components/AppSidebar.tsx b/ui/src/components/AppSidebar.tsx index 607b605d..a4752133 100644 --- a/ui/src/components/AppSidebar.tsx +++ b/ui/src/components/AppSidebar.tsx @@ -2,7 +2,6 @@ import { Link } from "@tanstack/react-router"; import { IoArrowBackOutline, IoCloudOutline, - IoExtensionPuzzleOutline, IoGitPullRequestOutline, IoOptionsOutline, IoSettingsOutline, @@ -28,10 +27,9 @@ interface NavItem { } const NAV: Array = [ - { to: "/my-settings", label: "My Settings", icon: IoOptionsOutline }, - { to: "/cloud-agents", label: "Cloud Agents", icon: IoCloudOutline }, + { to: "/my-settings", label: "Profile Settings", icon: IoOptionsOutline }, + { to: "/cloud-agents", label: "Open SWE Agent", icon: IoCloudOutline }, { to: "/review", label: "Open SWE Review", icon: IoGitPullRequestOutline }, - { to: "/integrations", label: "Integrations", icon: IoExtensionPuzzleOutline }, { to: "/admin", label: "Admin", icon: IoSettingsOutline, adminOnly: true }, ]; diff --git a/ui/src/lib/api.ts b/ui/src/lib/api.ts index 23160ab2..85a099c1 100644 --- a/ui/src/lib/api.ts +++ b/ui/src/lib/api.ts @@ -52,6 +52,7 @@ export interface SessionUser { email: string | null; avatar_url: string | null; is_admin: boolean; + slack_oauth_enabled?: boolean; } export interface ModelOption { @@ -212,6 +213,7 @@ export const api = { method: "PUT", body: JSON.stringify({ full_name, enabled }), }), + myMapping: () => request>("/my-mapping"), adminListUserMappings: (page = 1, pageSize = 20) => request( `/admin/user-mappings?page=${page}&page_size=${pageSize}`, @@ -234,3 +236,7 @@ export function loginUrl(redirectTo?: string): string { const qs = target ? `?redirect_to=${encodeURIComponent(target)}` : ""; return `${API_BASE}/dashboard/api/auth/login${qs}`; } + +export function slackConnectUrl(): string { + return `${API_BASE}/dashboard/api/slack/login`; +} diff --git a/ui/src/routes/admin.tsx b/ui/src/routes/admin.tsx index b4cc1d57..757629a8 100644 --- a/ui/src/routes/admin.tsx +++ b/ui/src/routes/admin.tsx @@ -223,7 +223,7 @@ function GlobalDefaultsSection({ models }: { models: Array }) {
diff --git a/ui/src/routes/integrations.tsx b/ui/src/routes/integrations.tsx index b8fe410a..082e9927 100644 --- a/ui/src/routes/integrations.tsx +++ b/ui/src/routes/integrations.tsx @@ -1,123 +1,7 @@ import { Navigate, createFileRoute } from "@tanstack/react-router"; -import { useQuery } from "@tanstack/react-query"; -import { GithubLogoIcon, KanbanIcon, SlackLogoIcon } from "@phosphor-icons/react"; -import type { ComponentType } from "react"; -import type { ReposPayload } from "@/lib/api"; -import { AppShell, SettingsSection } from "@/components/AppShell"; -import { Skeleton } from "@/components/ui/skeleton"; -import { ApiError, api } from "@/lib/api"; -import { useSession } from "@/lib/session"; -import { cn } from "@/lib/utils"; - -export const Route = createFileRoute("/integrations")({ component: IntegrationsPage }); - -type IconType = ComponentType<{ className?: string; weight?: "regular" | "fill" | "duotone" }>; - -interface IntegrationRowProps { - icon: IconType; - name: string; - description: string; - connected: boolean; - badge?: string; -} - -function IntegrationRow({ icon: Icon, name, description, connected, badge }: IntegrationRowProps) { - return ( -
-
-
- -
-
-
- {name} - {badge && ( - - {badge} - - )} -
-

{description}

-
-
- - {connected ? "Connected" : "Not connected"} - -
- ); -} - -function IntegrationsPage() { - const session = useSession(); - const repos = useQuery({ - queryKey: ["repos"], - queryFn: async () => { - try { - return await api.repos(); - } catch (e) { - if (e instanceof ApiError && e.status === 401) - return { installations: [], repositories: [] }; - throw e; - } - }, - enabled: !!session.data, - }); - - if (session.isLoading) { - return ( -
- -
- ); - } - if (!session.data) return ; - - const installs = repos.data?.installations ?? []; - const githubDescription = installs.length - ? `Connected to ${installs.length} ${installs.length === 1 ? "installation" : "installations"}: ${installs - .map((i) => i.account ?? "?") - .join(", ")}` - : "Install the open-swe GitHub App to enable PR and issue triggers."; - - return ( - - - 0} - /> - - - - - - - - ); -} +// Integrations were folded into Profile Settings. Keep the route as a redirect +// so existing links don't 404. +export const Route = createFileRoute("/integrations")({ + component: () => , +}); diff --git a/ui/src/routes/my-settings.tsx b/ui/src/routes/my-settings.tsx index c8b3b1ff..c40eb0f1 100644 --- a/ui/src/routes/my-settings.tsx +++ b/ui/src/routes/my-settings.tsx @@ -1,7 +1,9 @@ import { Navigate, createFileRoute, useNavigate } from "@tanstack/react-router"; import { useQuery, useQueryClient } from "@tanstack/react-query"; +import { SlackLogoIcon } from "@phosphor-icons/react"; import { useState } from "react"; +import type { SessionUser } from "@/lib/api"; import { AppShell, SettingsRow, SettingsSection } from "@/components/AppShell"; import { Button } from "@/components/ui/button"; import { @@ -12,9 +14,10 @@ import { SelectValue, } from "@/components/ui/select"; import { Skeleton } from "@/components/ui/skeleton"; -import { api } from "@/lib/api"; +import { api, slackConnectUrl } from "@/lib/api"; import { buildProfileUpdate, useOptions, useProfile, useSaveProfile } from "@/lib/profile"; import { useSession } from "@/lib/session"; +import { cn } from "@/lib/utils"; export const Route = createFileRoute("/my-settings")({ component: MySettingsPage }); @@ -32,6 +35,70 @@ function fromChoice(choice: DraftReviewChoice): boolean | null { return null; } +function UserMappingSection({ session }: { session: SessionUser }) { + const qc = useQueryClient(); + const mapping = useQuery({ queryKey: ["myMapping"], queryFn: api.myMapping }); + const [connecting, setConnecting] = useState(false); + + const slackUserId = mapping.data?.slack_user_id ?? null; + const workEmail = mapping.data?.work_email ?? null; + const connected = !!slackUserId; + + const connect = () => { + setConnecting(true); + // Refresh the cached mapping when the user returns from the OAuth redirect. + void qc.invalidateQueries({ queryKey: ["myMapping"] }); + window.location.assign(slackConnectUrl()); + }; + + return ( + +
+ {session.login}} + /> + + + {connected ? "Connected" : "Not connected"} + + {session.slack_oauth_enabled ? ( + + ) : ( + Sign in with Slack unavailable + )} +
+ } + /> +
+ + ); +} + function MySettingsPage() { const session = useSession(); const qc = useQueryClient(); @@ -84,18 +151,18 @@ function MySettingsPage() { }; return ( - + - {session.data.email ?? "—"} - + {session.data.email ?? "—"} } /> + +