From 3d90127dd06810494c95b8c56c93967109916ffb Mon Sep 17 00:00:00 2001 From: Paulo Date: Tue, 10 Nov 2026 14:08:48 +0200 Subject: [PATCH] DRU-1133 -- Let only admins change shared settings and connections --- backend/druks/accounts/dependencies.py | 3 +- backend/druks/accounts/models.py | 4 + backend/druks/browser/routes.py | 44 +- backend/druks/chat/bots/routes.py | 28 +- backend/druks/chat/channels/calls/routes.py | 15 +- .../druks/chat/channels/whatsapp/routes.py | 17 +- backend/druks/core/routes.py | 4 +- backend/druks/harnesses/routes.py | 12 +- backend/druks/mcp/routes.py | 133 ++++-- backend/druks/notifications/routes.py | 21 +- backend/druks/services/routes.py | 23 +- backend/druks/skills/routes.py | 19 +- backend/druks/user_settings/routes.py | 11 +- backend/tests/test_api_providers.py | 1 + backend/tests/test_api_settings.py | 3 + backend/tests/test_auth_boundary.py | 135 ++++-- backend/tests/test_browser_sessions.py | 19 + backend/tests/test_calls.py | 1 + backend/tests/test_identity.py | 42 ++ backend/tests/test_managed_github.py | 4 +- backend/tests/test_managed_linear.py | 4 +- backend/tests/test_managed_slack.py | 2 +- backend/tests/test_mcp_directory.py | 42 ++ backend/tests/test_mcp_oauth.py | 6 +- backend/tests/test_mcp_servers.py | 52 ++- backend/tests/test_personal_settings.py | 1 + backend/tests/test_service_disconnect.py | 3 +- backend/tests/test_whatsapp.py | 24 +- docs/concepts.md | 3 +- docs/configuration.md | 7 +- frontend/src/App.tsx | 3 +- frontend/src/api/client.ts | 20 + frontend/src/components/McpConnect.test.tsx | 60 ++- .../src/components/McpPersonalKey.test.tsx | 55 ++- frontend/src/components/McpRemoval.test.tsx | 2 +- frontend/src/components/McpScopes.test.tsx | 2 +- .../components/McpServerDirectory.test.tsx | 2 +- .../src/components/ProviderConnect.test.tsx | 12 + .../src/components/ResourcePages.test.tsx | 3 +- frontend/src/components/ServicesPane.test.tsx | 20 +- .../src/components/SettingsPages.test.tsx | 42 +- frontend/src/components/SettingsPages.tsx | 62 +-- frontend/src/components/SettingsPanes.tsx | 397 ++++++++++-------- .../components/WhatsAppNumbersPane.test.tsx | 7 +- .../src/components/WhatsAppNumbersPane.tsx | 8 +- frontend/src/pages/SchedulesPage.test.tsx | 18 +- frontend/src/pages/SchedulesPage.tsx | 71 ++-- frontend/src/styles.css | 1 + 48 files changed, 1086 insertions(+), 382 deletions(-) diff --git a/backend/druks/accounts/dependencies.py b/backend/druks/accounts/dependencies.py index f8c9d8d5f..e9656f56e 100644 --- a/backend/druks/accounts/dependencies.py +++ b/backend/druks/accounts/dependencies.py @@ -6,7 +6,6 @@ from starlette.requests import HTTPConnection from druks.accounts.context import current_account_id, current_conversation_id -from druks.accounts.enums import Role from druks.accounts.exceptions import ( AuthConfigurationError, InvalidAssertionError, @@ -172,7 +171,7 @@ async def current_session_account(request: Request, session: SessionDep) -> Asyn async def current_admin_account(account: Account = Depends(current_session_account)) -> Account: """The signed-in admin, never a bearer.""" - if account.role == Role.ADMIN: + if account.is_admin: return account raise HTTPException( status_code=403, diff --git a/backend/druks/accounts/models.py b/backend/druks/accounts/models.py index 8ebb1b916..7255aabeb 100644 --- a/backend/druks/accounts/models.py +++ b/backend/druks/accounts/models.py @@ -60,6 +60,10 @@ class Account(Model, Uuid7Pk, ordering=("created_at", "id")): is_tag_required: Mapped[bool] = mapped_column(default=False, server_default=text("false")) created_at: Mapped[datetime] = mapped_column(default=Base.utc_now) + @property + def is_admin(self) -> bool: + return self.role == Role.ADMIN + @classmethod async def get_default(cls, session: AsyncSession) -> "Account | None": """The unattended account, or None before account setup.""" diff --git a/backend/druks/browser/routes.py b/backend/druks/browser/routes.py index f31df2ce9..33c3dddee 100644 --- a/backend/druks/browser/routes.py +++ b/backend/druks/browser/routes.py @@ -5,7 +5,7 @@ from druks.accounts.dependencies import ( current_account, - current_session_account, + current_admin_account, require_operator, ) from druks.accounts.models import Account @@ -61,12 +61,11 @@ async def list_browser_sessions(session: SessionDep, account: Account = Depends( return sessions -@router.put("/{name}/state", status_code=204) +@router.put("/{name}/state", status_code=204, dependencies=[Depends(current_admin_account)]) async def upload_state( name: str, payload_format: Annotated[BrowserSessionPayloadFormat, Query(alias="payloadFormat")], request: Request, - account: Account = Depends(current_session_account), ) -> None: if declaration := browser_sessions.get(name): if declaration.anonymous: @@ -88,11 +87,8 @@ async def upload_state( raise exceptions.BrowserSessionUnknownError(name) -@router.post("/{name}/login-window", status_code=204) -async def open_login_window( - name: str, - account: Account = Depends(current_session_account), -) -> None: +@router.post("/{name}/login-window", status_code=204, dependencies=[Depends(current_admin_account)]) +async def open_login_window(name: str) -> None: if declaration := browser_sessions.get(name): if declaration.anonymous: raise exceptions.BrowserSessionAnonymousError(name) @@ -108,42 +104,38 @@ async def login_window_socket(websocket: WebSocket, name: str) -> None: return try: async with session_scope(websocket.app.state.engine) as session: - await require_operator(session, websocket) + account = await require_operator(session, websocket) window = await LoginWindow.get_for_session(name) except (HTTPException, exceptions.BrowserApiError): await websocket.close(code=1008) return + if not account.is_admin: + await websocket.close(code=1008) + return try: await window.stream(websocket) except Exception: await websocket.close(code=1011) -@router.post("/{name}/login-window/save", status_code=204) -async def save_login_window( - session: SessionDep, - name: str, - account: Account = Depends(current_session_account), -) -> None: +@router.post( + "/{name}/login-window/save", status_code=204, dependencies=[Depends(current_admin_account)] +) +async def save_login_window(session: SessionDep, name: str) -> None: window = await LoginWindow.get_for_session(name) await window.save(session) -@router.post("/{name}/login-window/cancel", status_code=204) -async def cancel_login_window( - name: str, - account: Account = Depends(current_session_account), -) -> None: +@router.post( + "/{name}/login-window/cancel", status_code=204, dependencies=[Depends(current_admin_account)] +) +async def cancel_login_window(name: str) -> None: window = await LoginWindow.get_for_session(name) await window.cancel() -@router.delete("/{name}", status_code=204) -async def delete_browser_session( - session: SessionDep, - name: str, - account: Account = Depends(current_session_account), -) -> None: +@router.delete("/{name}", status_code=204, dependencies=[Depends(current_admin_account)]) +async def delete_browser_session(session: SessionDep, name: str) -> None: if row := await StoredBrowserSession.get_for_name(session, name): await row.delete() return diff --git a/backend/druks/chat/bots/routes.py b/backend/druks/chat/bots/routes.py index 71c7fc00f..e366ab2f3 100644 --- a/backend/druks/chat/bots/routes.py +++ b/backend/druks/chat/bots/routes.py @@ -2,7 +2,11 @@ from fastapi import APIRouter, Depends, HTTPException, Path -from druks.accounts.dependencies import current_account, current_session_account +from druks.accounts.dependencies import ( + current_account, + current_admin_account, + current_session_account, +) from druks.accounts.models import Account from druks.api.dependencies import SessionDep from druks.apps.loader import get_app @@ -47,13 +51,31 @@ async def resume_conversation( response_model_by_alias=True, ) async def open_one_time_code( + session: SessionDep, + connection_id: str, + account: Account = Depends(current_admin_account), +) -> OneTimeCodeResponse: + """Open a code that makes its sender the public number's admin.""" + connection = await get_bot_connection(session, connection_id) + if connection and get_app(connection.identity["app"]).bot.access == BotAccess.PUBLIC: + code = await create_one_time_code(connection, account.id) + return OneTimeCodeResponse(code=code, expires_in=ONE_TIME_CODE_TTL_SECONDS) + raise HTTPException(404, "Connection not found.") + + +@router.post( + "/connections/{connection_id}/pairing", + response_model=OneTimeCodeResponse, + response_model_by_alias=True, +) +async def pair( session: SessionDep, connection_id: str, account: Account = Depends(current_session_account), ) -> OneTimeCodeResponse: - """A one-time code that pairs a phone, or makes its sender the number's admin.""" + """Open a code that pairs its sender with the caller's account.""" connection = await get_bot_connection(session, connection_id) - if connection: + if connection and get_app(connection.identity["app"]).bot.access == BotAccess.PRIVATE: code = await create_one_time_code(connection, account.id) return OneTimeCodeResponse(code=code, expires_in=ONE_TIME_CODE_TTL_SECONDS) raise HTTPException(404, "Connection not found.") diff --git a/backend/druks/chat/channels/calls/routes.py b/backend/druks/chat/channels/calls/routes.py index fc9188c30..0e624520b 100644 --- a/backend/druks/chat/channels/calls/routes.py +++ b/backend/druks/chat/channels/calls/routes.py @@ -3,7 +3,7 @@ from fastapi import APIRouter, Body, Depends, HTTPException from sqlalchemy import Row -from druks.accounts.dependencies import current_session_account +from druks.accounts.dependencies import current_admin_account, current_session_account from druks.accounts.enums import AccountKind from druks.accounts.models import Account from druks.api.dependencies import SessionDep @@ -36,7 +36,11 @@ async def list_unlinked_numbers(session: SessionDep) -> list[dict]: @router.post( - "/numbers", status_code=201, response_model=LinkedNumberResponse, response_model_by_alias=True + "/numbers", + status_code=201, + response_model=LinkedNumberResponse, + response_model_by_alias=True, + dependencies=[Depends(current_admin_account)], ) async def link_number( session: SessionDep, @@ -63,7 +67,9 @@ async def link_number( return await Twilio.link(session, owner, app=app, sid=sid, calls_url=calls_url) -@router.delete("/numbers/{number_id}", status_code=204) +@router.delete( + "/numbers/{number_id}", status_code=204, dependencies=[Depends(current_admin_account)] +) async def remove_number(session: SessionDep, number_id: str) -> None: if connection := await Twilio.get_connection(session, number_id): # Removing is idempotent: a second delete finds the number removed. @@ -77,8 +83,7 @@ async def remove_number(session: SessionDep, number_id: str) -> None: "/numbers/{number_id}/calls", response_model=list[CallResponse], response_model_by_alias=True ) async def list_calls(session: SessionDep, number_id: str) -> list[Row]: - """A number's calls. The number's bot account owns them, and any operator reads them, - as any operator manages an app's numbers.""" + """A number's calls. Its bot account owns them, and any operator can read them.""" if await Twilio.get_connection(session, number_id): return await CallsChannel.list_calls(session, number_id) raise HTTPException(404, "Number not found.") diff --git a/backend/druks/chat/channels/whatsapp/routes.py b/backend/druks/chat/channels/whatsapp/routes.py index d5b0a9090..2b4934b48 100644 --- a/backend/druks/chat/channels/whatsapp/routes.py +++ b/backend/druks/chat/channels/whatsapp/routes.py @@ -2,7 +2,7 @@ from fastapi import APIRouter, Body, Depends, HTTPException -from druks.accounts.dependencies import current_session_account +from druks.accounts.dependencies import current_admin_account, current_session_account from druks.accounts.enums import AccountKind from druks.accounts.models import Account from druks.api.dependencies import SessionDep @@ -34,7 +34,11 @@ async def list_sessions( @router.post( - "/sessions", status_code=201, response_model=SessionResponse, response_model_by_alias=True + "/sessions", + status_code=201, + response_model=SessionResponse, + response_model_by_alias=True, + dependencies=[Depends(current_admin_account)], ) async def link_session(session: SessionDep, app: Annotated[str, Body(embed=True)]) -> VaultSecret: """Link a number for the Bot of an app.""" @@ -63,7 +67,10 @@ async def get_qr(session: SessionDep, session_id: str) -> dict[str, str]: @router.post( - "/sessions/{session_id}/relink", response_model=SessionResponse, response_model_by_alias=True + "/sessions/{session_id}/relink", + response_model=SessionResponse, + response_model_by_alias=True, + dependencies=[Depends(current_admin_account)], ) async def relink_session(session: SessionDep, session_id: str) -> VaultSecret: """Take a new scan on a number that lost its link.""" @@ -74,7 +81,9 @@ async def relink_session(session: SessionDep, session_id: str) -> VaultSecret: raise HTTPException(404, "Session not found.") -@router.delete("/sessions/{session_id}", status_code=204) +@router.delete( + "/sessions/{session_id}", status_code=204, dependencies=[Depends(current_admin_account)] +) async def remove_session(session: SessionDep, session_id: str) -> None: connection = await Waha.get_session(session, session_id) if not connection: diff --git a/backend/druks/core/routes.py b/backend/druks/core/routes.py index b18ab94ce..39ea6ba86 100644 --- a/backend/druks/core/routes.py +++ b/backend/druks/core/routes.py @@ -5,7 +5,7 @@ from fastapi import APIRouter, Depends, HTTPException from fastapi.responses import HTMLResponse, RedirectResponse -from druks.accounts.dependencies import current_session_account +from druks.accounts.dependencies import current_admin_account from druks.apps.registry import services from druks.core.apis.github import GITHUB_API_URL from druks.core.apis.slack import SLACK_CREATE_APP_URL @@ -50,7 +50,7 @@ async def create_github_app(slug: str) -> HTMLResponse: @router.get( "/{slug}/manifest/callback", response_class=HTMLResponse, - dependencies=[Depends(current_session_account)], + dependencies=[Depends(current_admin_account)], ) async def github_manifest_callback(slug: str, code: str = "") -> HTMLResponse: service = get_manifest_service(slug) diff --git a/backend/druks/harnesses/routes.py b/backend/druks/harnesses/routes.py index bc1728647..73bdbfd8f 100644 --- a/backend/druks/harnesses/routes.py +++ b/backend/druks/harnesses/routes.py @@ -4,7 +4,11 @@ from fastapi import APIRouter, Body, Depends, HTTPException from sqlalchemy.ext.asyncio import AsyncSession -from druks.accounts.dependencies import current_session_account, current_session_or_setup +from druks.accounts.dependencies import ( + current_admin_account, + current_session_account, + current_session_or_setup, +) from druks.accounts.models import Account from druks.accounts.schemas import AccountResponse from druks.api.dependencies import SessionDep @@ -232,7 +236,7 @@ async def _store_connection( async def create_key( provider_id: str, session: SessionDep, - account: Account = Depends(current_session_account), + account: Account = Depends(current_admin_account), key: str = Body(..., embed=True), ) -> VaultSecret: """The installation's key at a provider. A key for a directory provider also adds it.""" @@ -254,9 +258,7 @@ async def create_key( return await VaultSecret.paste(session, Audience.provider(provider_id), key, pasted_by=account) -@router.delete( - "/{provider_id}/key", status_code=204, dependencies=[Depends(current_session_account)] -) +@router.delete("/{provider_id}/key", status_code=204, dependencies=[Depends(current_admin_account)]) async def remove_key(session: SessionDep, provider_id: str) -> None: """Removing a directory provider's key also removes the provider.""" if stored := await VaultSecret.lookup( diff --git a/backend/druks/mcp/routes.py b/backend/druks/mcp/routes.py index 61b8e54e8..56e87ff90 100644 --- a/backend/druks/mcp/routes.py +++ b/backend/druks/mcp/routes.py @@ -6,12 +6,14 @@ from sqlalchemy.ext.asyncio import AsyncSession from druks.accounts.context import current_account_id -from druks.accounts.dependencies import current_session_account +from druks.accounts.dependencies import current_admin_account, current_session_account +from druks.accounts.models import Account from druks.api.dependencies import SessionDep from druks.apps.registry import mcp_servers from druks.core.templates import render_page from druks.mcp import oauth from druks.mcp.constants import HEADER_NAME_PATTERN, SCOPE_PATTERN +from druks.mcp.datastructures import McpServerAccess from druks.mcp.enums import Credential, IdentityMode from druks.mcp.exceptions import ( InvalidServerNameError, @@ -67,7 +69,7 @@ async def list_mcp_server_directory() -> list[dict]: return [{"name": name, **entry} for name, entry in directory.items()] -@router.post("", response_model=McpServerResponse) +@router.post("", response_model=McpServerResponse, dependencies=[Depends(current_admin_account)]) async def add_mcp_server(session: SessionDep, body: CreateMcpServerRequest) -> McpServerResponse: if body.name in mcp_servers: raise HTTPException( @@ -98,7 +100,9 @@ async def add_mcp_server(session: SessionDep, body: CreateMcpServerRequest) -> M return await _response(session, body.name) -@router.post("/directory", response_model=McpServerResponse) +@router.post( + "/directory", response_model=McpServerResponse, dependencies=[Depends(current_admin_account)] +) async def add_directory_mcp_server( session: SessionDep, name: str = Body(embed=True) ) -> McpServerResponse: @@ -122,7 +126,9 @@ async def add_directory_mcp_server( raise HTTPException(status_code=404, detail=f"{name!r} is not in the MCP server directory.") -@router.patch("/{name}", response_model=McpServerResponse) +@router.patch( + "/{name}", response_model=McpServerResponse, dependencies=[Depends(current_admin_account)] +) async def set_mcp_server_enabled( session: SessionDep, name: str, is_enabled: bool = Body(embed=True) ) -> McpServerResponse: @@ -133,9 +139,15 @@ async def set_mcp_server_enabled( return await _response(session, name) -@router.put("/{name}/headers", response_model=McpServerResponse) +@router.put( + "/{name}/headers", + response_model=McpServerResponse, + dependencies=[Depends(current_admin_account)], +) async def set_mcp_server_secret_headers( - session: SessionDep, name: str, secret_headers: Annotated[dict[str, str], Body(embed=True)] + session: SessionDep, + name: str, + secret_headers: Annotated[dict[str, str], Body(embed=True)], ) -> McpServerResponse: # The merged view decides: only a built-in's definition knows it is OAuth. access = await McpServer.get_access(session, name, current_account_id.get()) @@ -148,7 +160,7 @@ async def set_mcp_server_secret_headers( raise HTTPException(status_code=404, detail=f"MCP server {name!r} takes no pasted key.") -@router.delete("/{name}/headers", status_code=204) +@router.delete("/{name}/headers", status_code=204, dependencies=[Depends(current_admin_account)]) async def remove_mcp_server_secret_headers(session: SessionDep, name: str) -> None: access = await McpServer.get_access(session, name, current_account_id.get()) if access and access.credential == Credential.HEADERS: @@ -158,6 +170,35 @@ async def remove_mcp_server_secret_headers(session: SessionDep, name: str) -> No raise HTTPException(status_code=404, detail=f"MCP server {name!r} takes no pasted key.") +@router.put("/{name}/headers/me", response_model=McpServerResponse) +async def set_personal_mcp_server_secret_headers( + session: SessionDep, name: str, secret_headers: Annotated[dict[str, str], Body(embed=True)] +) -> McpServerResponse: + """Set the caller's own key on a server that takes a key per person.""" + access = await McpServer.get_access(session, name, current_account_id.get()) + if access and access.credential == Credential.HEADERS: + if access.identity_mode != IdentityMode.PER_USER: + raise HTTPException(status_code=409, detail=f"MCP server {name!r} takes one key.") + server = await McpServer.get_or_create(session, name) + await server.set_secret_headers( + current_account_id.get(), _valid_secret_headers(name, secret_headers) + ) + return await _response(session, name) + raise HTTPException(status_code=404, detail=f"MCP server {name!r} takes no pasted key.") + + +@router.delete("/{name}/headers/me", status_code=204) +async def remove_personal_mcp_server_secret_headers(session: SessionDep, name: str) -> None: + access = await McpServer.get_access(session, name, current_account_id.get()) + if access and access.credential == Credential.HEADERS: + if access.identity_mode != IdentityMode.PER_USER: + raise HTTPException(status_code=409, detail=f"MCP server {name!r} takes one key.") + server = await McpServer.get_or_create(session, name) + await server.remove_secret_headers(current_account_id.get()) + return + raise HTTPException(status_code=404, detail=f"MCP server {name!r} takes no pasted key.") + + @router.get("/{name}/connections", response_model=list[McpServerConnectionResponse]) async def list_mcp_server_connections(session: SessionDep, name: str) -> list[VaultSecret]: if not await McpServer.get_for_name(session, name): @@ -165,7 +206,7 @@ async def list_mcp_server_connections(session: SessionDep, name: str) -> list[Va return await oauth.list_connections(session, name) -@router.delete("/{name}", status_code=204) +@router.delete("/{name}", status_code=204, dependencies=[Depends(current_admin_account)]) async def remove_mcp_server(session: SessionDep, name: str) -> None: if name in mcp_servers: # A built-in is druks-owned — removing it would silently drop it from @@ -194,7 +235,11 @@ async def get_mcp_server_scopes(session: SessionDep, name: str) -> McpServerScop raise HTTPException(status_code=404, detail=f"MCP server {name!r} takes no scope selection.") -@router.put("/{name}/scopes", response_model=McpServerResponse) +@router.put( + "/{name}/scopes", + response_model=McpServerResponse, + dependencies=[Depends(current_admin_account)], +) async def set_mcp_server_scopes( session: SessionDep, name: str, scopes: Annotated[list[str], Body(embed=True)] ) -> McpServerResponse: @@ -210,7 +255,7 @@ async def set_mcp_server_scopes( raise HTTPException(status_code=404, detail=f"MCP server {name!r} takes no scope selection.") -@router.delete("/{name}/scopes", status_code=204) +@router.delete("/{name}/scopes", status_code=204, dependencies=[Depends(current_admin_account)]) async def remove_mcp_server_scopes(session: SessionDep, name: str) -> None: access = await McpServer.get_access(session, name, current_account_id.get()) if access and access.supports_scope_selection: @@ -219,7 +264,11 @@ async def remove_mcp_server_scopes(session: SessionDep, name: str) -> None: raise HTTPException(status_code=404, detail=f"MCP server {name!r} takes no scope selection.") -@router.post("/{name}/connect", response_model=ConnectMcpServerResponse) +@router.post( + "/{name}/connect", + response_model=ConnectMcpServerResponse, + dependencies=[Depends(current_admin_account)], +) async def connect_mcp_server( session: SessionDep, name: str, @@ -233,6 +282,30 @@ async def connect_mcp_server( status_code=409, detail=f"MCP server {name!r} already uses {access.identity_mode!r} identity.", ) + response = await _begin_mcp_connect(name, access, identity_mode) + if access.credential == Credential.SERVICE_CONNECTION: + # The account's sign-in at the service is its credential here, and a + # sign-in belongs to one account. The service's callback knows nothing of + # this server, so Connect is where it becomes enabled. + await McpServer.set_enabled(session, name, is_enabled=True) + return response + + +@router.post("/{name}/connect/me", response_model=ConnectMcpServerResponse) +async def connect_personal_mcp_server(session: SessionDep, name: str) -> ConnectMcpServerResponse: + access = await McpServer.get_access(session, name, current_account_id.get()) + if not access or not access.is_oauth: + raise HTTPException(status_code=404, detail=f"MCP server {name!r} is not an OAuth server.") + if access.credential == Credential.SERVICE_CONNECTION or ( + access.credential == Credential.GRANT and access.identity_mode == IdentityMode.PER_USER + ): + return await _begin_mcp_connect(name, access, IdentityMode.PER_USER) + raise HTTPException(status_code=409, detail=f"MCP server {name!r} takes no personal grant.") + + +async def _begin_mcp_connect( + name: str, access: McpServerAccess, identity_mode: IdentityMode +) -> ConnectMcpServerResponse: endpoint = get_settings().urls.endpoint if not endpoint: # The authorization server redirects the operator's browser back to @@ -243,11 +316,7 @@ async def connect_mcp_server( "at, to connect OAuth MCP servers.", ) if access.credential == Credential.SERVICE_CONNECTION: - # The account's sign-in at the service is its credential here, and a - # sign-in belongs to one account. The service's callback knows nothing of - # this server, so Connect is where it becomes enabled. if identity_mode == IdentityMode.PER_USER: - await McpServer.set_enabled(session, name, is_enabled=True) return ConnectMcpServerResponse( authorization_url=f"{endpoint.rstrip('/')}/api/oauth/{access.service}" "/connect?next=/settings/mcp" @@ -270,13 +339,13 @@ async def connect_mcp_server( return ConnectMcpServerResponse(authorization_url=authorization_url) -@router.get( - "/oauth/callback", - response_class=HTMLResponse, - dependencies=[Depends(current_session_account)], -) +@router.get("/oauth/callback", response_class=HTMLResponse) async def oauth_callback( - session: SessionDep, state: str = "", code: str = "", error: str = "" + session: SessionDep, + state: str = "", + code: str = "", + error: str = "", + account: Account = Depends(current_session_account), ) -> HTMLResponse: # The operator's browser lands here from the consent screen — a human-facing # page, not a JSON API. Failures surface as loud HTTP errors (the app's @@ -291,16 +360,16 @@ async def oauth_callback( name = await oauth.complete_connect(session, state=state, code=code) except OauthConnectError as exchange_error: raise HTTPException(status_code=400, detail=str(exchange_error)) from exchange_error - # Connecting is the operator's explicit "use this server" — a - # connected-but-disabled server is a dead end nobody asks for. - await McpServer.set_enabled(session, name, is_enabled=True) + # A member's connect must leave the server as an admin set it. + if account.is_admin: + await McpServer.set_enabled(session, name, is_enabled=True) # druks opened this tab via window.open, so the page may close itself; the # broadcast tells the settings page to refetch before the tab goes. The # text stays for browsers that refuse the close. return render_page("mcp_oauth_callback.html", name=name) -@router.delete("/{name}/grant", status_code=204) +@router.delete("/{name}/grant", status_code=204, dependencies=[Depends(current_admin_account)]) async def disconnect_mcp_server(session: SessionDep, name: str) -> None: access = await McpServer.get_access(session, name, current_account_id.get()) if not access or not access.is_oauth: @@ -308,6 +377,20 @@ async def disconnect_mcp_server(session: SessionDep, name: str) -> None: if not access.identity_mode: raise HTTPException(status_code=404, detail=f"MCP server {name!r} has no grant.") account_id = get_grant_account(access.identity_mode, current_account_id.get()) + await _remove_mcp_grant(session, name, account_id) + + +@router.delete("/{name}/grant/me", status_code=204) +async def disconnect_personal_mcp_server(session: SessionDep, name: str) -> None: + access = await McpServer.get_access(session, name, current_account_id.get()) + if not access or not access.is_oauth: + raise HTTPException(status_code=404, detail=f"MCP server {name!r} is not an OAuth server.") + if access.identity_mode != IdentityMode.PER_USER: + raise HTTPException(status_code=409, detail=f"MCP server {name!r} takes no personal grant.") + await _remove_mcp_grant(session, name, current_account_id.get()) + + +async def _remove_mcp_grant(session: AsyncSession, name: str, account_id: str | None) -> None: connection = await oauth.get_connection(session, name, account_id) if not connection: raise HTTPException( diff --git a/backend/druks/notifications/routes.py b/backend/druks/notifications/routes.py index d78fcadc8..3cc2032ca 100644 --- a/backend/druks/notifications/routes.py +++ b/backend/druks/notifications/routes.py @@ -1,7 +1,8 @@ from typing import Annotated -from fastapi import APIRouter, Body, HTTPException, Query +from fastapi import APIRouter, Body, Depends, HTTPException, Query +from druks.accounts.dependencies import current_admin_account from druks.api.dependencies import SessionDep from druks.core.services import Slack from druks.notifications.delivery import deliver @@ -34,7 +35,11 @@ async def list_destinations(session: SessionDep) -> list[Destination]: return await Destination.list_all(session) -@router.post("/destinations", response_model=DestinationResponse) +@router.post( + "/destinations", + response_model=DestinationResponse, + dependencies=[Depends(current_admin_account)], +) async def create_destination(session: SessionDep, body: CreateDestinationRequest) -> Destination: if not body.name.strip(): raise HTTPException(status_code=422, detail="Destination needs a name.") @@ -55,7 +60,11 @@ async def create_destination(session: SessionDep, body: CreateDestinationRequest return destination -@router.patch("/destinations/{destination_id}", response_model=DestinationResponse) +@router.patch( + "/destinations/{destination_id}", + response_model=DestinationResponse, + dependencies=[Depends(current_admin_account)], +) async def set_destination_enabled( session: SessionDep, destination_id: str, is_enabled: Annotated[bool, Body(embed=True)] ) -> Destination: @@ -67,7 +76,11 @@ async def set_destination_enabled( return destination -@router.delete("/destinations/{destination_id}", status_code=204) +@router.delete( + "/destinations/{destination_id}", + status_code=204, + dependencies=[Depends(current_admin_account)], +) async def delete_destination(session: SessionDep, destination_id: str) -> None: destination = await session.get(Destination, destination_id) if not destination: diff --git a/backend/druks/services/routes.py b/backend/druks/services/routes.py index 6ea80b014..e425be4a0 100644 --- a/backend/druks/services/routes.py +++ b/backend/druks/services/routes.py @@ -3,7 +3,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from druks.accounts.context import current_account_id -from druks.accounts.dependencies import current_session_account +from druks.accounts.dependencies import current_admin_account, current_session_account from druks.api.dependencies import SessionDep from druks.apps.registry import services from druks.core.templates import render_page @@ -39,7 +39,7 @@ async def list_services(session: SessionDep) -> list[ServiceResponse]: ] install_url = "" if row and await service.Authentication.get_install_endpoint(): - install_url = f"/api/oauth/{service.slug}/connect?install=1" + install_url = f"/api/oauth/{service.slug}/install" entries.append( ServiceResponse.from_row( service, @@ -59,7 +59,7 @@ async def list_services(session: SessionDep) -> list[ServiceResponse]: "/{slug}", response_model=ServiceResponse, response_model_by_alias=True, - dependencies=[Depends(current_session_account)], + dependencies=[Depends(current_admin_account)], ) async def connect_service(slug: str, payload: dict[str, str]) -> ServiceResponse: service = services.get(slug) @@ -72,7 +72,7 @@ async def connect_service(slug: str, payload: dict[str, str]) -> ServiceResponse return ServiceResponse.from_row(service, row, connected=await service.is_connected()) -@router.delete("/{slug}", status_code=204, dependencies=[Depends(current_session_account)]) +@router.delete("/{slug}", status_code=204, dependencies=[Depends(current_admin_account)]) async def disconnect_service(session: SessionDep, slug: str) -> None: service = services.get(slug) if not service: @@ -82,15 +82,26 @@ async def disconnect_service(session: SessionDep, slug: str) -> None: await service.disconnect(session) +@oauth_router.get("/{slug}/install", dependencies=[Depends(current_admin_account)]) +async def install_oauth_service(session: SessionDep, slug: str, next: str = "") -> RedirectResponse: + """Start an install at the provider.""" + return await _start_oauth_service(session, slug, next=next, install=True) + + @oauth_router.get("/{slug}/connect", dependencies=[Depends(current_session_account)]) async def connect_oauth_service( session: SessionDep, slug: str, connection: str = "", next: str = "", - install: bool = False, ) -> RedirectResponse: - """Start a sign-in or an install at the provider.""" + """Start the caller's sign-in at the provider.""" + return await _start_oauth_service(session, slug, connection=connection, next=next) + + +async def _start_oauth_service( + session: AsyncSession, slug: str, *, connection: str = "", next: str = "", install: bool = False +) -> RedirectResponse: service = services.get(slug) if not service or not service.Authentication.get_client(): raise OauthPageError(f"No OAuth service {slug!r}.", status_code=404) diff --git a/backend/druks/skills/routes.py b/backend/druks/skills/routes.py index 64938197f..eeaaba4b2 100644 --- a/backend/druks/skills/routes.py +++ b/backend/druks/skills/routes.py @@ -1,6 +1,7 @@ -from fastapi import APIRouter, Body, HTTPException +from fastapi import APIRouter, Body, Depends, HTTPException from githubkit.exception import RequestFailed, RequestTimeout +from druks.accounts.dependencies import current_admin_account from druks.api.dependencies import SessionDep, SettingsDep from .install import fetch_collection, remove_files @@ -15,7 +16,7 @@ async def list_collections(session: SessionDep) -> list[SkillCollection]: return await SkillCollection.list_all(session) -@router.post("", response_model=CollectionResponse) +@router.post("", response_model=CollectionResponse, dependencies=[Depends(current_admin_account)]) async def install_collection( session: SessionDep, settings: SettingsDep, @@ -42,7 +43,11 @@ async def install_collection( ) -@router.post("/{collection_id}/sync", response_model=CollectionResponse) +@router.post( + "/{collection_id}/sync", + response_model=CollectionResponse, + dependencies=[Depends(current_admin_account)], +) async def sync_collection( session: SessionDep, collection_id: str, settings: SettingsDep ) -> SkillCollection: @@ -85,7 +90,11 @@ async def sync_collection( return collection -@router.patch("/{collection_id}/skills/{name}", response_model=SkillResponse) +@router.patch( + "/{collection_id}/skills/{name}", + response_model=SkillResponse, + dependencies=[Depends(current_admin_account)], +) async def set_skill_enabled( session: SessionDep, collection_id: str, @@ -100,7 +109,7 @@ async def set_skill_enabled( return skill -@router.delete("/{collection_id}", status_code=204) +@router.delete("/{collection_id}", status_code=204, dependencies=[Depends(current_admin_account)]) async def remove_collection(session: SessionDep, collection_id: str) -> None: collection = await session.get(SkillCollection, collection_id) if not collection: diff --git a/backend/druks/user_settings/routes.py b/backend/druks/user_settings/routes.py index d3ddba1c9..0ee3b825a 100644 --- a/backend/druks/user_settings/routes.py +++ b/backend/druks/user_settings/routes.py @@ -1,7 +1,7 @@ from fastapi import APIRouter, Depends, HTTPException from sqlalchemy.ext.asyncio import AsyncSession -from druks.accounts.dependencies import current_account, current_session_account +from druks.accounts.dependencies import current_account, current_admin_account from druks.accounts.models import Account from druks.api.dependencies import SessionDep from druks.apps.loader import get_app, iter_apps @@ -100,7 +100,12 @@ async def _settings_changes( return fields -@router.patch("", response_model=SettingsResponse, response_model_by_alias=True) +@router.patch( + "", + response_model=SettingsResponse, + response_model_by_alias=True, + dependencies=[Depends(current_admin_account)], +) async def update_settings(session: SessionDep, body: UpdateSettingsRequest) -> InstallationSettings: fields = await _settings_changes(session, body) settings = await InstallationSettings.get_or_create(session) @@ -154,7 +159,7 @@ async def get_app_setting_choices(name: str) -> dict[str, list[dict[str, str]]]: "/apps", response_model=AppsSettingsResponse, response_model_by_alias=True, - dependencies=[Depends(current_session_account)], + dependencies=[Depends(current_admin_account)], ) async def update_app_settings( session: SessionDep, body: AppsSettingsUpdate diff --git a/backend/tests/test_api_providers.py b/backend/tests/test_api_providers.py index 2f3467f03..524046f01 100644 --- a/backend/tests/test_api_providers.py +++ b/backend/tests/test_api_providers.py @@ -148,6 +148,7 @@ async def test_disconnect_removes_only_the_requesting_accounts_login(tmp_path: P async def test_removing_the_key_leaves_every_subscription(tmp_path: Path, druks_db): mine = await connect_provider(AnthropicProvider, {"claudeAiOauth": {"accessToken": "x"}}) + await Account.get_or_create(druks_db, "op@example.com") await VaultSecret.paste( druks_db, Audience.provider("anthropic"), diff --git a/backend/tests/test_api_settings.py b/backend/tests/test_api_settings.py index 2263a4aa2..e4bea6234 100644 --- a/backend/tests/test_api_settings.py +++ b/backend/tests/test_api_settings.py @@ -1,6 +1,7 @@ from pathlib import Path from conftest import settings_client +from druks.accounts.enums import Role from druks.accounts.models import Account from druks.contrib.software_factory.app import SoftwareFactory from druks.db import db_session @@ -76,6 +77,8 @@ def test_patch_settings_judges_the_default_triple_together(tmp_path: Path): async def test_accounts_report_the_default_without_a_fallback_setting(druks_db, tmp_path: Path): account = await Account.get_or_create(druks_db, "ops@example.com") + test_operator = await Account.get_or_create(druks_db, "op@example.com") + await test_operator.set_role(Role.ADMIN) with settings_client(tmp_path) as client: expected = { "id": account.id, diff --git a/backend/tests/test_auth_boundary.py b/backend/tests/test_auth_boundary.py index 5eb71915d..f929ce1df 100644 --- a/backend/tests/test_auth_boundary.py +++ b/backend/tests/test_auth_boundary.py @@ -2,11 +2,13 @@ from druks.accounts.dependencies import ( current_account, current_account_or_setup, + current_admin_account, current_session_account, current_session_or_setup, ) from druks.accounts.models import Account from druks.api.server import app +from druks.apps.loader import iter_apps from druks.harnesses.providers import get_providers from fastapi.routing import APIRoute, _IncludedRouter @@ -30,47 +32,88 @@ ("POST", "/api/chat/conversations/{conversation_id}/messages/{message_id}/retry"), ("POST", "/api/chat/conversations/{conversation_id}/cancel"), ("GET", "/api/chat/services/waha/sessions"), - ("POST", "/api/chat/services/waha/sessions"), ("GET", "/api/chat/services/waha/sessions/{session_id}/qr"), - ("POST", "/api/chat/services/waha/sessions/{session_id}/relink"), - ("DELETE", "/api/chat/services/waha/sessions/{session_id}"), ("GET", "/api/chat/services/twilio/numbers"), - ("POST", "/api/chat/services/twilio/numbers"), - ("DELETE", "/api/chat/services/twilio/numbers/{number_id}"), ("GET", "/api/chat/services/twilio/numbers/{number_id}/calls"), ("GET", "/api/chat/services/twilio/numbers/{number_id}/calls/{call_id}"), ("GET", "/api/chat/services/twilio/numbers/unlinked"), - ("POST", "/api/chat/connections/{connection_id}/one-time-code"), + ("POST", "/api/chat/connections/{connection_id}/pairing"), ("DELETE", "/api/chat/connections/{connection_id}/pairing"), ("GET", "/api/chat/services/slack/link/{token}"), - ("PUT", "/api/browser-sessions/{name}/state"), - ("POST", "/api/browser-sessions/{name}/login-window"), - ("POST", "/api/browser-sessions/{name}/login-window/save"), - ("POST", "/api/browser-sessions/{name}/login-window/cancel"), - ("DELETE", "/api/browser-sessions/{name}"), ("GET", "/api/auth/personal-tokens"), ("POST", "/api/auth/personal-tokens"), ("DELETE", "/api/auth/personal-tokens/{pat_id}"), - ("PATCH", "/api/auth/accounts/{account_id}"), ("GET", "/api/providers/catalogs"), ("GET", "/api/providers/directory"), ("GET", "/api/providers/subscriptions"), ("GET", "/api/providers/keys"), - ("POST", "/api/providers/{provider_id}/key"), - ("DELETE", "/api/providers/{provider_id}/key"), ("POST", "/api/providers/{provider_id}/sign-in/token"), ("DELETE", "/api/providers/{provider_id}/connection"), - ("PATCH", "/api/settings/apps"), - ("POST", "/api/services/{slug}"), - ("DELETE", "/api/services/{slug}"), ("GET", "/api/oauth/{slug}/connect"), ("GET", "/api/oauth/callback"), - ("GET", "/api/core/services/{slug}/manifest/callback"), ("GET", "/api/mcp-servers/oauth/callback"), ("GET", "/api/oauth/connections"), ("DELETE", "/api/oauth/connections/{connection_id}"), } +# Shared settings and connections admit an admin from the browser only. +ADMIN_API_ROUTES = { + ("PATCH", "/api/auth/accounts/{account_id}"), + ("DELETE", "/api/browser-sessions/{name}"), + ("POST", "/api/browser-sessions/{name}/login-window"), + ("POST", "/api/browser-sessions/{name}/login-window/cancel"), + ("POST", "/api/browser-sessions/{name}/login-window/save"), + ("PUT", "/api/browser-sessions/{name}/state"), + ("POST", "/api/chat/services/twilio/numbers"), + ("DELETE", "/api/chat/services/twilio/numbers/{number_id}"), + ("POST", "/api/chat/services/waha/sessions"), + ("DELETE", "/api/chat/services/waha/sessions/{session_id}"), + ("POST", "/api/chat/services/waha/sessions/{session_id}/relink"), + ("POST", "/api/chat/connections/{connection_id}/one-time-code"), + ("GET", "/api/core/services/{slug}/manifest/callback"), + ("DELETE", "/api/mcp-servers/{name}"), + ("DELETE", "/api/mcp-servers/{name}/grant"), + ("DELETE", "/api/mcp-servers/{name}/headers"), + ("DELETE", "/api/mcp-servers/{name}/scopes"), + ("POST", "/api/mcp-servers/{name}/connect"), + ("PUT", "/api/mcp-servers/{name}/headers"), + ("PUT", "/api/mcp-servers/{name}/scopes"), + ("PATCH", "/api/mcp-servers/{name}"), + ("POST", "/api/mcp-servers"), + ("POST", "/api/mcp-servers/directory"), + ("DELETE", "/api/notifications/destinations/{destination_id}"), + ("PATCH", "/api/notifications/destinations/{destination_id}"), + ("POST", "/api/notifications/destinations"), + ("DELETE", "/api/providers/{provider_id}/key"), + ("POST", "/api/providers/{provider_id}/key"), + ("DELETE", "/api/services/{slug}"), + ("POST", "/api/services/{slug}"), + ("GET", "/api/oauth/{slug}/install"), + ("DELETE", "/api/skills/{collection_id}"), + ("PATCH", "/api/skills/{collection_id}/skills/{name}"), + ("POST", "/api/skills/{collection_id}/sync"), + ("POST", "/api/skills"), + ("PATCH", "/api/settings"), + ("PATCH", "/api/settings/apps"), +} + +# These writes also admit a personal token. +TOKEN_WRITE_API_ROUTES = { + ("POST", "/api/browser/action"), + ("POST", "/api/browser/evaluate"), + ("POST", "/api/browser/navigate"), + ("POST", "/api/dashboard/schedules/{kind}/run"), + ("POST", "/api/gates/{run}/answer"), + ("DELETE", "/api/mcp-servers/{name}/grant/me"), + ("DELETE", "/api/mcp-servers/{name}/headers/me"), + ("POST", "/api/mcp-servers/{name}/connect/me"), + ("PUT", "/api/mcp-servers/{name}/headers/me"), + ("POST", "/api/runs/{run}/cancel"), + ("POST", "/api/runs/{run}/resume"), + ("POST", "/api/runs/{run}/retry"), + ("PATCH", "/api/settings/personal"), +} + # Session-gated routes that also sit behind their router's identity gate — # the route-level session dependency is the stricter of the two. DUAL_GATED_API_PATHS = { @@ -81,21 +124,14 @@ "/api/chat/conversations/{conversation_id}/cancel", "/api/chat/services/waha/sessions", "/api/chat/services/waha/sessions/{session_id}/qr", - "/api/chat/services/waha/sessions/{session_id}/relink", - "/api/chat/services/waha/sessions/{session_id}", "/api/chat/services/twilio/numbers", - "/api/chat/services/twilio/numbers/{number_id}", "/api/chat/services/twilio/numbers/{number_id}/calls", "/api/chat/services/twilio/numbers/{number_id}/calls/{call_id}", "/api/chat/services/twilio/numbers/unlinked", - "/api/chat/connections/{connection_id}/one-time-code", "/api/chat/connections/{connection_id}/pairing", "/api/chat/services/slack/link/{token}", - "/api/settings/apps", - "/api/services/{slug}", "/api/oauth/{slug}/connect", "/api/oauth/callback", - "/api/core/services/{slug}/manifest/callback", "/api/mcp-servers/oauth/callback", "/api/oauth/connections", "/api/oauth/connections/{connection_id}", @@ -141,6 +177,10 @@ def _is_session_only(route) -> bool: return any((method, route.path) in SESSION_ONLY_API_ROUTES for method in route.methods) +def _route_keys(route) -> set[tuple[str, str]]: + return {(method, route.path) for method in route.methods if method not in {"HEAD", "OPTIONS"}} + + def test_every_internal_api_route_sits_behind_the_identity_gate(api_routes): unguarded = [ route.path @@ -148,6 +188,7 @@ def test_every_internal_api_route_sits_behind_the_identity_gate(api_routes): if route.path.startswith("/api/") and route.path not in EXEMPT_API_PATHS and not _is_session_only(route) + and not any(key in ADMIN_API_ROUTES for key in _route_keys(route)) and not _gated_by(route, current_account) ] assert unguarded == [] @@ -214,5 +255,47 @@ def test_capability_management_is_session_only(api_routes): assert not _gated_by(route, current_account), route.path # And nothing else carries the session-only gate. for route in api_routes: - if not _is_session_only(route): + if not _is_session_only(route) and not any( + key in ADMIN_API_ROUTES for key in _route_keys(route) + ): assert not _gated_by(route, current_session_account), route.path + + +def test_admin_routes_use_only_the_admin_gate(api_routes): + listed = [ + route for route in api_routes if any(key in ADMIN_API_ROUTES for key in _route_keys(route)) + ] + assert {key for route in listed for key in _route_keys(route) if key in ADMIN_API_ROUTES} == ( + ADMIN_API_ROUTES + ) + for route in listed: + assert _gated_by(route, current_admin_account), route.path + assert not {"agent", "bot"} & set(route.tags), route.path + for route in api_routes: + if not any(key in ADMIN_API_ROUTES for key in _route_keys(route)): + assert not _gated_by(route, current_admin_account), route.path + + +def test_every_platform_write_has_one_access_class(api_routes): + known_groups = (ADMIN_API_ROUTES, SESSION_ONLY_API_ROUTES, TOKEN_WRITE_API_ROUTES) + assert not any( + left & right + for index, left in enumerate(known_groups) + for right in known_groups[index + 1 :] + ) + known_route_keys = {key for route in api_routes for key in _route_keys(route)} + assert set().union(*known_groups) <= known_route_keys + + app_prefixes = tuple(f"/api/{registered_app.name}/" for registered_app in iter_apps()) + for route in api_routes: + if not route.path.startswith("/api/") or route.path.startswith(app_prefixes): + continue + for key in _route_keys(route): + if key[0] == "GET": + continue + groups = [group for group in known_groups if key in group] + if route.path in SETUP_CAPABLE_API_PATHS: + groups.append(SETUP_CAPABLE_API_PATHS) + elif route.path in EXEMPT_API_PATHS: + groups.append(EXEMPT_API_PATHS) + assert len(groups) == 1, key diff --git a/backend/tests/test_browser_sessions.py b/backend/tests/test_browser_sessions.py index 882e9a26a..30a2daae2 100644 --- a/backend/tests/test_browser_sessions.py +++ b/backend/tests/test_browser_sessions.py @@ -1,6 +1,7 @@ import base64 import pytest +from conftest import IDENTITY_HEADER, header_client from druks import database from druks.accounts.dependencies import current_account, current_session_account from druks.accounts.models import Account, PersonalAccessToken @@ -10,6 +11,7 @@ from druks.browser.sessions import BrowserSession from druks.db import db_session from druks.testing import configure_app_for_test, make_settings +from fastapi import WebSocketDisconnect from fastapi.testclient import TestClient from sqlalchemy import text from sqlalchemy_encrypted_field import SecretDecryptError @@ -222,3 +224,20 @@ async def test_bearer_pat_reads_sessions_but_cannot_mutate_them(tmp_path, druks_ ) assert [response.status_code for response in mutations] == [401, 401, 401] + + +async def test_member_cannot_open_a_login_window_socket(tmp_path, druks_db): + await Account.get_or_create(druks_db, "admin@example.com") + member = await Account.get_or_create(druks_db, "member@example.com") + + with ( + header_client(tmp_path) as client, + pytest.raises(WebSocketDisconnect) as refused, + client.websocket_connect( + "/api/browser-sessions/notebook.archive/login-window/ws", + headers={IDENTITY_HEADER: member.username, "origin": "http://testserver"}, + ), + ): + pass + + assert refused.value.code == 1008 diff --git a/backend/tests/test_calls.py b/backend/tests/test_calls.py index 64df8c20b..16f316759 100644 --- a/backend/tests/test_calls.py +++ b/backend/tests/test_calls.py @@ -162,6 +162,7 @@ async def test_a_linked_number_sends_its_calls_to_druks_until_it_is_removed( async def test_disconnecting_twilio_removes_its_numbers_first(druks_db, druks_client, twilio): + await Account.get_or_create(druks_db, "op@example.com") connection = await link(druks_db) response = await druks_client.delete("/api/services/twilio") diff --git a/backend/tests/test_identity.py b/backend/tests/test_identity.py index a338979ba..fc9182113 100644 --- a/backend/tests/test_identity.py +++ b/backend/tests/test_identity.py @@ -157,6 +157,45 @@ async def test_only_an_admin_browser_identity_can_change_roles(tmp_path, druks_d assert bearer_response.status_code == 401 +async def test_shared_settings_require_an_admin_browser_identity(tmp_path, druks_db): + admin = await Account.get_or_create(druks_db, "admin@example.com") + member = await Account.get_or_create(druks_db, "member@example.com") + _, token = await PersonalAccessToken.create(druks_db, account_id=admin.id, name="admin") + + with header_client(tmp_path) as client: + member_response = client.patch( + "/api/settings", + json={"includeApps": True}, + headers={IDENTITY_HEADER: member.username}, + ) + admin_response = client.patch( + "/api/settings", + json={"includeApps": True}, + headers={IDENTITY_HEADER: admin.username}, + ) + bearer_response = client.patch( + "/api/settings", + json={"includeApps": False}, + headers={"Authorization": f"Bearer {token}"}, + ) + personal_response = client.patch( + "/api/settings/personal", + json={"timezone": "Europe/Madrid"}, + headers={IDENTITY_HEADER: member.username}, + ) + install_response = client.get( + "/api/oauth/github/install", headers={IDENTITY_HEADER: member.username} + ) + + assert member_response.status_code == 403 + assert admin_response.status_code == 200 + assert admin_response.json()["includeApps"] + assert bearer_response.status_code == 401 + assert personal_response.status_code == 200 + assert personal_response.json()["timezone"] == "Europe/Madrid" + assert install_response.status_code == 403 + + async def test_role_changes_keep_an_admin_and_reject_bot_accounts(tmp_path, druks_db): first = await Account.get_or_create(druks_db, "first@example.com") second = await Account.get_or_create(druks_db, "second@example.com") @@ -318,6 +357,9 @@ async def test_a_key_alone_finishes_onboarding(tmp_path, druks_db): async def test_a_second_pasted_key_replaces_the_first(tmp_path, druks_db): + await Account.get_or_create(druks_db, "first@example.com") + second = await Account.get_or_create(druks_db, "second@example.com") + await second.set_role(Role.ADMIN) with header_client(tmp_path) as client: for who, key in (("first@example.com", "key-one"), ("second@example.com", "key-two")): response = client.post( diff --git a/backend/tests/test_managed_github.py b/backend/tests/test_managed_github.py index 8867f6655..861d8f04b 100644 --- a/backend/tests/test_managed_github.py +++ b/backend/tests/test_managed_github.py @@ -299,7 +299,7 @@ async def test_install_binds_at_the_manager_and_links_the_installer( managed_github, githubkit, manager, github_identity ): async with asgi_client(configure_app_for_test(settings=managed_github)) as client: - response = await client.get("/api/oauth/github/connect", params={"install": "1"}) + response = await client.get("/api/oauth/github/install") assert response.status_code == 307 consent = urlparse(response.headers["location"]) query = dict(parse_qsl(consent.query)) @@ -373,7 +373,7 @@ async def test_a_refused_exchange_links_nothing(managed_github, manager): manager.answer = {"error": "access_denied"} async with asgi_client(configure_app_for_test(settings=managed_github)) as client: - response = await client.get("/api/oauth/github/connect", params={"install": "1"}) + response = await client.get("/api/oauth/github/install") state = dict(parse_qsl(urlparse(response.headers["location"]).query))["state"] page = await client.get( "/api/oauth/callback", diff --git a/backend/tests/test_managed_linear.py b/backend/tests/test_managed_linear.py index b822f1269..335d37941 100644 --- a/backend/tests/test_managed_linear.py +++ b/backend/tests/test_managed_linear.py @@ -218,7 +218,7 @@ async def test_linear_waits_for_its_install_and_keeps_its_grant_across_a_restart async with asgi_client(configure_app_for_test(settings=managed_linear)) as client: entry = await _get_entry(client) assert not entry["connected"] and entry["managed"] and entry["isOauth"] - assert entry["installUrl"] == "/api/oauth/linear/connect?install=1" + assert entry["installUrl"] == "/api/oauth/linear/install" consent = urlparse((await client.get(entry["installUrl"])).headers["location"]) query = dict(parse_qsl(consent.query)) @@ -310,7 +310,7 @@ async def test_a_revoked_grant_asks_for_the_install_again( async with asgi_client(configure_app_for_test(settings=managed_linear)) as client: entry = await _get_entry(client) assert not entry["connected"] - assert entry["installUrl"] == "/api/oauth/linear/connect?install=1" + assert entry["installUrl"] == "/api/oauth/linear/install" async def test_the_default_account_sends_the_apps_token_to_linears_mcp_server( diff --git a/backend/tests/test_managed_slack.py b/backend/tests/test_managed_slack.py index 25918b47a..99e62aa47 100644 --- a/backend/tests/test_managed_slack.py +++ b/backend/tests/test_managed_slack.py @@ -221,7 +221,7 @@ async def test_slack_waits_for_its_install_and_keeps_it_across_a_restart( async with asgi_client(configure_app_for_test(settings=managed_slack)) as client: entry = await _get_entry(client) assert not entry["connected"] and entry["managed"] - assert entry["installUrl"] == "/api/oauth/slack/connect?install=1" + assert entry["installUrl"] == "/api/oauth/slack/install" query, page = await _install(client) assert page.status_code == 200 diff --git a/backend/tests/test_mcp_directory.py b/backend/tests/test_mcp_directory.py index 26ffa5a09..8ea9cec4a 100644 --- a/backend/tests/test_mcp_directory.py +++ b/backend/tests/test_mcp_directory.py @@ -1,5 +1,7 @@ import json +from conftest import IDENTITY_HEADER +from druks.accounts.models import Account from druks.mcp.constants import NAME_PATTERN from druks.mcp.enums import IdentityMode from druks.mcp.models import McpServer @@ -51,10 +53,12 @@ async def fake_begin_connect( return "https://consent.example/authorize" monkeypatch.setattr("druks.mcp.oauth.begin_connect", fake_begin_connect) + personal = client.post("/api/mcp-servers/grafana/connect/me") connect = client.post( "/api/mcp-servers/grafana/connect", json={"identity_mode": IdentityMode.PER_USER} ) + assert personal.status_code == 409 assert connect.json()["authorizationUrl"] == "https://consent.example/authorize" assert begun == [ ("grafana", "https://mcp.grafana.com/mcp", "http://druks.test", IdentityMode.PER_USER) @@ -84,6 +88,44 @@ async def test_github_connects_through_the_service(tmp_path, druks_db): assert shared.status_code == 409 +async def test_member_connect_keeps_a_disabled_server_disabled(tmp_path, druks_db): + admin = await Account.get_or_create(druks_db, "admin@example.com") + member = await Account.get_or_create(druks_db, "member@example.com") + settings = make_settings( + tmp_path, + identity={"mode": "header", "header": IDENTITY_HEADER}, + urls={"endpoint": "http://druks.test"}, + ) + app = configure_app_for_test(settings=settings, authenticated=False) + + with TestClient(app) as client: + created = client.post( + "/api/mcp-servers/directory", + json={"name": "github"}, + headers={IDENTITY_HEADER: admin.username}, + ) + established = client.post( + "/api/mcp-servers/github/connect", + json={"identity_mode": IdentityMode.PER_USER}, + headers={IDENTITY_HEADER: admin.username}, + ) + disabled = client.patch( + "/api/mcp-servers/github", + json={"is_enabled": False}, + headers={IDENTITY_HEADER: admin.username}, + ) + connected = client.post( + "/api/mcp-servers/github/connect/me", + headers={IDENTITY_HEADER: member.username}, + ) + + assert created.status_code == 200 + assert established.status_code == 200 + assert disabled.status_code == 200 + assert connected.status_code == 200 + assert (await McpServer.get_for_name(druks_db, "github")).is_enabled is False + + def test_adding_an_unknown_or_existing_server_is_refused(tmp_path, druks_db): with _client(tmp_path) as client: unknown = client.post("/api/mcp-servers/directory", json={"name": "acme"}) diff --git a/backend/tests/test_mcp_oauth.py b/backend/tests/test_mcp_oauth.py index b0d0117be..43053bf33 100644 --- a/backend/tests/test_mcp_oauth.py +++ b/backend/tests/test_mcp_oauth.py @@ -1122,7 +1122,7 @@ async def test_per_user_disconnect_preserves_other_accounts_grant_and_token(tmp_ with TestClient(configure_app_for_test(settings=settings, authenticated=False)) as client: response = client.delete( - f"/api/mcp-servers/{_NAME}/grant", + f"/api/mcp-servers/{_NAME}/grant/me", headers={"X-Test-User": disconnected.username}, ) assert response.status_code == 204 @@ -1141,7 +1141,8 @@ async def test_per_user_last_disconnect_disables_and_reopens_the_mode(tmp_path, with TestClient(configure_app_for_test(settings=settings, authenticated=False)) as client: response = client.delete( - f"/api/mcp-servers/{_NAME}/grant", headers={"X-Test-User": account.username} + f"/api/mcp-servers/{_NAME}/grant/me", + headers={"X-Test-User": account.username}, ) assert response.status_code == 204 @@ -1151,6 +1152,7 @@ async def test_per_user_last_disconnect_disables_and_reopens_the_mode(tmp_path, async def test_removal_drops_every_grant_and_kept_token(tmp_path, druks_db): + await Account.get_or_create(druks_db, "op@example.com") first = await Account.get_or_create(druks_db, "first@example.com") second = await Account.get_or_create(druks_db, "second@example.com") await _store_grant(account_id=first.id, identity_mode=IdentityMode.PER_USER) diff --git a/backend/tests/test_mcp_servers.py b/backend/tests/test_mcp_servers.py index ad29136dd..316ab4c11 100644 --- a/backend/tests/test_mcp_servers.py +++ b/backend/tests/test_mcp_servers.py @@ -2,7 +2,7 @@ from types import SimpleNamespace import pytest -from conftest import connect_service +from conftest import IDENTITY_HEADER, connect_service from druks.accounts.models import Account, PersonalAccessToken from druks.apps.registry import mcp_servers from druks.db import db_session @@ -467,13 +467,13 @@ async def test_routes_set_and_remove_the_signed_in_account_key(tmp_path, druks_d assert (created.json()["identityMode"], created.json()["hasToken"]) == ("per_user", True) assert not await VaultSecret.list_installation_tokens(druks_db) - removed = await client.delete("/api/mcp-servers/lusha/headers") + removed = await client.delete("/api/mcp-servers/lusha/headers/me") assert removed.status_code == 204 listed = await client.get("/api/mcp-servers") assert listed.json()[0]["hasToken"] is False replaced = await client.put( - "/api/mcp-servers/lusha/headers", json={"secret_headers": _BEARER} + "/api/mcp-servers/lusha/headers/me", json={"secret_headers": _BEARER} ) assert replaced.json()["hasToken"] is True operator = await Account.get_or_create(druks_db, "op@example.com") @@ -481,6 +481,52 @@ async def test_routes_set_and_remove_the_signed_in_account_key(tmp_path, druks_d assert [row.header for row in rows] == [BEARER_HEADER] +async def test_member_sets_a_personal_key_but_not_a_shared_key(tmp_path, druks_db): + admin = await Account.get_or_create(druks_db, "admin@example.com") + member = await Account.get_or_create(druks_db, "member@example.com") + await McpServer.create( + druks_db, + name="personal", + url="https://personal.example/mcp", + secret_headers={"x-api-key": "admin-key"}, + account_id=admin.id, + ) + await McpServer.create( + druks_db, + name="shared", + url="https://shared.example/mcp", + secret_headers={"x-api-key": "shared-key"}, + ) + settings = make_settings(tmp_path, identity={"mode": "header", "header": IDENTITY_HEADER}) + + async with asgi_client( + configure_app_for_test(settings=settings, authenticated=False) + ) as client: + headers = {IDENTITY_HEADER: member.username} + personal = await client.put( + "/api/mcp-servers/personal/headers/me", + json={"secret_headers": {"x-api-key": "member-key"}}, + headers=headers, + ) + shared = await client.put( + "/api/mcp-servers/shared/headers", + json={"secret_headers": {"x-api-key": "member-key"}}, + headers=headers, + ) + personal_shared = await client.put( + "/api/mcp-servers/shared/headers/me", + json={"secret_headers": {"x-api-key": "member-key"}}, + headers=headers, + ) + + assert personal.status_code == 200 + assert personal.json()["hasToken"] is True + assert shared.status_code == 403 + assert personal_shared.status_code == 409 + [stored] = await VaultSecret.list_secret_headers(druks_db, Audience.mcp("personal"), member.id) + assert stored.secrets["value"] == "member-key" + + async def test_routes_reject_creating_an_authless_custom_server(tmp_path, druks_db): url = "https://mcp.notion.com/sse" async with asgi_client(configure_app_for_test(settings=make_settings(tmp_path))) as client: diff --git a/backend/tests/test_personal_settings.py b/backend/tests/test_personal_settings.py index 59fc4e54c..7637bdeef 100644 --- a/backend/tests/test_personal_settings.py +++ b/backend/tests/test_personal_settings.py @@ -321,6 +321,7 @@ async def test_invalid_edit_keeps_account_preferences(tmp_path, druks_db, field, async def test_shared_agent_overrides_use_installation_settings_after_a_personal_edit( tmp_path, druks_db ): + await Account.get_or_create(druks_db, "op@example.com") account = await Account.get_or_create(druks_db, "bob@example.com") await account.update_preferences(timezone="Europe/Madrid") diff --git a/backend/tests/test_service_disconnect.py b/backend/tests/test_service_disconnect.py index 063fa985b..7327ad334 100644 --- a/backend/tests/test_service_disconnect.py +++ b/backend/tests/test_service_disconnect.py @@ -15,6 +15,7 @@ async def test_disconnect_revokes_the_card_and_every_sign_in_of_that_service_onl other = await connect_service( "github_reviewer", identity={"app_id": "456"}, secrets={"private_key": "reviewer"} ) + await Account.get_or_create(druks_db, "op@example.com") account = await Account.get_or_create(druks_db, "one@example.com") connection = await VaultSecret.connect( druks_db, @@ -44,7 +45,7 @@ async def test_disconnect_revokes_the_login_its_mcp_server_derived(druks_db, dru await McpServer.create( druks_db, name="atlassian", url="https://mcp.atlassian.com/v2/mcp", is_oauth=True ) - account = await Account.get_or_create(druks_db, "default@example.com") + account = await Account.get_or_create(druks_db, "op@example.com") derived = await McpServer.store_login(druks_db, "atlassian", login, account.id) assert (await druks_client.delete("/api/services/jira")).status_code == 204 diff --git a/backend/tests/test_whatsapp.py b/backend/tests/test_whatsapp.py index c744596bd..a5069c36a 100644 --- a/backend/tests/test_whatsapp.py +++ b/backend/tests/test_whatsapp.py @@ -10,7 +10,7 @@ import httpx import pytest -from conftest import bind_ambient_session, connect_service +from conftest import IDENTITY_HEADER, bind_ambient_session, connect_service from druks.accounts.dependencies import current_account, resolve_single_operator from druks.accounts.enums import AccountKind from druks.accounts.models import Account, PersonalAccessToken @@ -861,6 +861,22 @@ async def test_the_one_time_code_makes_its_sender_the_number_admin(druks_db, hel assert (user_chat.user_id, [message.body for message in user_chat.messages]) == (BEN, [code]) +async def test_member_cannot_open_a_public_number_code(druks_db, tmp_path, helpdesk): + await Account.get_or_create(druks_db, "admin@example.com") + connection = await link(druks_db, await bot_account(druks_db)) + member = await Account.get_or_create(druks_db, "member@example.com") + settings = make_settings(tmp_path, identity={"mode": "header", "header": IDENTITY_HEADER}) + api = configure_app_for_test(settings=settings, authenticated=False) + + async with asgi_client(api) as client: + response = await client.post( + f"/api/chat/connections/{connection.id}/one-time-code", + headers={IDENTITY_HEADER: member.username}, + ) + + assert response.status_code == 403 + + async def test_bot_and_bot_admin_accounts_never_count_as_operators(druks_db): operator = await Account.get_or_create(druks_db, "op@example.com") await bot_account(druks_db) @@ -892,8 +908,7 @@ async def test_two_operators_pair_to_one_number_under_their_own_accounts( before = await client.get("/api/chat/services/waha/sessions?app=chat", headers=headers) assert not before.json()[0]["isPhonePaired"] response = await client.post( - f"/api/chat/connections/{connection.id}/one-time-code", - json={"account_id": connection.account_id}, + f"/api/chat/connections/{connection.id}/pairing", headers=headers, ) assert response.status_code == 200 @@ -1187,6 +1202,7 @@ async def test_removing_a_number_holds_its_chats_and_relinking_keeps_its_history druks_db, druks_client, helpdesk, waha, monkeypatch ): webhook_base(monkeypatch) + await Account.get_or_create(druks_db, "op@example.com") owner = await bot_account(druks_db) first = await link(druks_db, owner) await receive(first, message_event(ANA, "Hello", key="M1")) @@ -1208,6 +1224,7 @@ async def test_removing_a_number_holds_its_chats_and_relinking_keeps_its_history async def test_disconnecting_waha_removes_its_numbers_first(druks_db, druks_client, waha): + await Account.get_or_create(druks_db, "op@example.com") connection = await link(druks_db, await bot_account(druks_db)) response = await druks_client.delete("/api/services/waha") @@ -1223,6 +1240,7 @@ async def test_a_number_that_lost_its_link_takes_a_new_scan_and_keeps_its_chats( druks_db, druks_client, helpdesk, waha, monkeypatch ): webhook_base(monkeypatch) + await Account.get_or_create(druks_db, "op@example.com") connection = await link(druks_db, await bot_account(druks_db)) await receive(connection, message_event(ANA, "Hello", key="M1")) [chat] = await Conversation.list_for_connection(druks_db, connection.id) diff --git a/docs/concepts.md b/docs/concepts.md index c072e444d..ccb3fe1e4 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -309,7 +309,8 @@ mode permits account creation at first access. Each operator account has a role, `admin` or `member`. The first account is an admin. Each later account joins as a member. An admin changes a person's role -in **Settings → People**. Druks refuses to remove the last admin. +in **Settings → People**. Only admins can change settings and connections that +the installation shares. Druks refuses to remove the last admin. In `none` mode, Druks has no authentication and exactly one operator account. The first completed provider connection creates this account. Public diff --git a/docs/configuration.md b/docs/configuration.md index 2fe43d3ce..05f5ef2ff 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -216,7 +216,10 @@ Missing subscriptions fail the call. API keys belong to the installation. The first account is also an admin. Each later person joins as a member when Druks first resolves their identity. An admin changes a person's role in -**Settings → People**. +**Settings → People**. Only an admin can change shared settings, app settings, +installation connections, provider keys, browser profiles, MCP servers, and +notification destinations. A member can change personal preferences, +subscriptions, API tokens, and per-person connections and keys. The installation timezone controls schedules and operational day boundaries. Your personal timezone controls timestamp display. Gate notifications use the @@ -351,7 +354,7 @@ provider-specific behavior. A subscription is always one person's. An API key is the installation's: one per provider, owned by no account, and visible to every account in **Settings → Providers** with the name of the person who last pasted it. A -paste from any account replaces it. Disconnect clears the secret and retains +paste from an admin replaces it. Disconnect clears the secret and retains the credential identity for historical agent-call billing references. Before you enable `jwt` mode, make sure that the edge uses the configured header, diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 515f3ef6e..631df14b6 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -275,7 +275,7 @@ function AppShell({ ui?.navigation ?? rosterQuery.data?.find((entry) => entry.name === app)?.navigation const appSettingsPath = - urlApp && settingsQuery.data?.apps.some((entry) => entry.name === urlApp) + account.role === 'admin' && urlApp && settingsQuery.data?.apps.some((entry) => entry.name === urlApp) ? `/apps/${urlApp}/settings` : undefined const navigation = urlApp ? (declaredNavigation ?? []) : [] @@ -465,6 +465,7 @@ function AppShell({ rosterQuery.data?.filter((entry) => !entry.builtin).map((entry) => entry.name) ?? [] } unsavedFormRef={unsavedFormRef} + isAdmin={account.role === 'admin'} /> diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 55ff3a5f5..b1fb927ea 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -362,6 +362,11 @@ export const api = { `/api/chat/connections/${encodeURIComponent(connectionId)}/one-time-code`, {}, ), + pair: (connectionId: string) => + postJSON<{ code: string; expiresIn: number }>( + `/api/chat/connections/${encodeURIComponent(connectionId)}/pairing`, + {}, + ), unpair: (connectionId: string) => deleteRequest(`/api/chat/connections/${encodeURIComponent(connectionId)}/pairing`), linkedNumbers: (app: string) => @@ -453,16 +458,31 @@ export const api = { `/api/mcp-servers/${encodeURIComponent(name)}/connect`, { identity_mode: identityMode }, ), + connectPersonalMcpServer: (name: string) => + postJSON<{ authorizationUrl: string }>( + `/api/mcp-servers/${encodeURIComponent(name)}/connect/me`, + undefined, + ), disconnectMcpServer: (name: string) => deleteRequest(`/api/mcp-servers/${encodeURIComponent(name)}/grant`), + disconnectPersonalMcpServer: (name: string) => + deleteRequest(`/api/mcp-servers/${encodeURIComponent(name)}/grant/me`), setMcpServerHeaders: (name: string, secretHeaders: Record) => postJSON( `/api/mcp-servers/${encodeURIComponent(name)}/headers`, { secret_headers: secretHeaders }, 'PUT', ), + setPersonalMcpServerHeaders: (name: string, secretHeaders: Record) => + postJSON( + `/api/mcp-servers/${encodeURIComponent(name)}/headers/me`, + { secret_headers: secretHeaders }, + 'PUT', + ), removeMcpServerHeaders: (name: string) => deleteRequest(`/api/mcp-servers/${encodeURIComponent(name)}/headers`), + removePersonalMcpServerHeaders: (name: string) => + deleteRequest(`/api/mcp-servers/${encodeURIComponent(name)}/headers/me`), mcpServerScopes: (name: string) => getJSON(`/api/mcp-servers/${encodeURIComponent(name)}/scopes`), setMcpServerScopes: (name: string, scopes: string[]) => diff --git a/frontend/src/components/McpConnect.test.tsx b/frontend/src/components/McpConnect.test.tsx index c9b106107..acc12ed77 100644 --- a/frontend/src/components/McpConnect.test.tsx +++ b/frontend/src/components/McpConnect.test.tsx @@ -1,5 +1,5 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query' -import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' import { afterEach, expect, it, vi } from 'vitest' import { api } from '../api/client' @@ -27,7 +27,7 @@ it.each(['success', 'failure'])('shows pending sign-in and handles %s', async (o } vi.spyOn(window, 'open').mockReturnValue(popup as unknown as Window) const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) - render() + render() fireEvent.click(await screen.findByRole('button', { name: 'Connect' })) expect(screen.getByRole('status').textContent).toContain('Preparing sign-in') @@ -58,9 +58,63 @@ it('offers Connect beside the service login', async () => { credential: 'service_login' as const, service: 'jira', scopes: null, }]) const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) - render() + render() expect(await screen.findByText('jira login')).toBeTruthy() expect(screen.getByRole('button', { name: 'Connect' })).toBeTruthy() expect(screen.queryByRole('button', { name: 'Disconnect' })).toBeNull() }) + +it.each([true, false])('chooses the service sign-in route when admin is %s', async (isAdmin) => { + vi.spyOn(api, 'mcpServers').mockResolvedValue([{ + name: 'github', url: 'https://api.githubcopilot.com/mcp', isEnabled: true, + isOauth: true, identityMode: null, builtin: true, hasToken: false, + credential: 'service_connection' as const, service: 'github', scopes: null, + }]) + vi.spyOn(api, 'mcpServerDirectory').mockResolvedValue([]) + const connect = vi.spyOn(api, 'connectMcpServer').mockResolvedValue({ authorizationUrl: 'https://github.test/sign-in' }) + const connectPersonal = vi.spyOn(api, 'connectPersonalMcpServer').mockResolvedValue({ authorizationUrl: 'https://github.test/sign-in' }) + const popup = { document: document.implementation.createHTMLDocument(), location: { assign: vi.fn() } } + vi.spyOn(window, 'open').mockReturnValue(popup as unknown as Window) + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + render( + + + , + ) + + fireEvent.click(await screen.findByRole('button', { name: 'Connect' })) + + if (isAdmin) { + await waitFor(() => expect(connect).toHaveBeenCalledWith('github', 'per_user')) + expect(connectPersonal).not.toHaveBeenCalled() + } else { + await waitFor(() => expect(connectPersonal).toHaveBeenCalledWith('github')) + expect(connect).not.toHaveBeenCalled() + } +}) + +it('uses personal routes for a per-account grant', async () => { + vi.spyOn(api, 'mcpServers').mockResolvedValue([{ + name: 'jira', url: 'https://jira.test/mcp', isEnabled: true, + isOauth: true, identityMode: 'per_user', builtin: false, hasToken: true, + credential: 'grant' as const, service: null, scopes: null, + }]) + vi.spyOn(api, 'mcpServerDirectory').mockResolvedValue([]) + const connect = vi.spyOn(api, 'connectPersonalMcpServer').mockResolvedValue({ authorizationUrl: 'https://jira.test/sign-in' }) + const disconnect = vi.spyOn(api, 'disconnectPersonalMcpServer').mockResolvedValue() + vi.spyOn(api, 'connectMcpServer') + vi.spyOn(api, 'disconnectMcpServer') + vi.spyOn(window, 'confirm').mockReturnValue(true) + const popup = { document: document.implementation.createHTMLDocument(), location: { assign: vi.fn() } } + vi.spyOn(window, 'open').mockReturnValue(popup as unknown as Window) + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + render() + + fireEvent.click(await screen.findByRole('button', { name: 'Reconnect' })) + await waitFor(() => expect(connect).toHaveBeenCalledWith('jira')) + fireEvent.click(screen.getByRole('button', { name: 'Disconnect' })) + await waitFor(() => expect(disconnect).toHaveBeenCalledWith('jira')) + expect(api.connectMcpServer).not.toHaveBeenCalled() + expect(api.disconnectMcpServer).not.toHaveBeenCalled() +}) diff --git a/frontend/src/components/McpPersonalKey.test.tsx b/frontend/src/components/McpPersonalKey.test.tsx index aeeb0ac56..9f9adea5a 100644 --- a/frontend/src/components/McpPersonalKey.test.tsx +++ b/frontend/src/components/McpPersonalKey.test.tsx @@ -23,9 +23,10 @@ it.each([ const server = { ...lusha, identityMode } vi.spyOn(api, 'mcpServers').mockResolvedValue([server]) vi.spyOn(api, 'mcpServerDirectory').mockResolvedValue([]) - const set = vi.spyOn(api, 'setMcpServerHeaders').mockResolvedValue({ ...server, hasToken: true }) + const setShared = vi.spyOn(api, 'setMcpServerHeaders').mockResolvedValue({ ...server, hasToken: true }) + const setPersonal = vi.spyOn(api, 'setPersonalMcpServerHeaders').mockResolvedValue({ ...server, hasToken: true }) const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) - render() + render() expect(await screen.findByText(status)).toBeTruthy() expect(screen.queryByRole('button', { name: removeLabel })).toBeNull() @@ -36,17 +37,18 @@ it.each([ fireEvent.change(row.getByLabelText(/Header value/), { target: { value: 'k-1' } }) fireEvent.click(row.getByRole('button', { name: 'Save key' })) + const set = identityMode === 'per_user' ? setPersonal : setShared await waitFor(() => expect(set).toHaveBeenCalledWith('lusha', { 'x-api-key': 'k-1' })) }) it('keeps the typed key when the save fails', async () => { vi.spyOn(api, 'mcpServers').mockResolvedValue([lusha]) vi.spyOn(api, 'mcpServerDirectory').mockResolvedValue([]) - vi.spyOn(api, 'setMcpServerHeaders').mockRejectedValue( + vi.spyOn(api, 'setPersonalMcpServerHeaders').mockRejectedValue( new ApiError('Invalid header name', 422, 'Invalid header name'), ) const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) - render() + render() const row = within((await screen.findByText('lusha')).closest('.mcp-row') as HTMLElement) fireEvent.click(row.getByRole('button', { name: 'Set your key' })) @@ -58,3 +60,48 @@ it('keeps the typed key when the save fails', async () => { expect((await screen.findByRole('alert')).textContent).toContain('Invalid header name') expect((row.getByLabelText(/Header value/) as HTMLInputElement).value).toBe('k-1') }) + +it('keeps personal controls and hides shared server controls from a member', async () => { + const shared = { ...lusha, name: 'shared', identityMode: null } + const personalGrant = { + ...lusha, + name: 'personal-grant', + isOauth: true, + identityMode: 'per_user', + hasToken: true, + credential: 'grant' as const, + } + const serviceConnection = { + ...lusha, + name: 'service-connection', + isOauth: true, + identityMode: null, + credential: 'service_connection' as const, + service: 'jira', + } + vi.spyOn(api, 'mcpServers').mockResolvedValue([ + lusha, shared, personalGrant, serviceConnection, { ...shared, name: 'off', isEnabled: false }, + ]) + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + render( + + + , + ) + + const personalRow = within((await screen.findByText('lusha')).closest('.mcp-row') as HTMLElement) + const sharedRow = within(screen.getByText('shared').closest('.mcp-row') as HTMLElement) + const grantRow = within(screen.getByText('personal-grant').closest('.mcp-row') as HTMLElement) + const serviceRow = within( + screen.getByText('service-connection').closest('.mcp-row') as HTMLElement, + ) + expect(personalRow.getByRole('button', { name: 'Set your key' })).toBeTruthy() + expect(grantRow.getByRole('button', { name: 'Disconnect' })).toBeTruthy() + expect(grantRow.queryByRole('button', { name: 'Scopes' })).toBeNull() + expect(serviceRow.getByRole('button', { name: 'Connect' })).toBeTruthy() + expect(sharedRow.queryByRole('button', { name: 'Set key' })).toBeNull() + expect(screen.queryByRole('switch')).toBeNull() + expect(screen.queryByText('off')).toBeNull() + expect(screen.queryByRole('heading', { name: 'Add a server' })).toBeNull() + expect(screen.queryByRole('button', { name: 'Remove' })).toBeNull() +}) diff --git a/frontend/src/components/McpRemoval.test.tsx b/frontend/src/components/McpRemoval.test.tsx index a7242a110..b7a587b3a 100644 --- a/frontend/src/components/McpRemoval.test.tsx +++ b/frontend/src/components/McpRemoval.test.tsx @@ -22,7 +22,7 @@ it.each([true, false])('names affected accounts before removal, confirmed=%s', a const confirm = vi.spyOn(window, 'confirm').mockReturnValue(confirmed) const remove = vi.spyOn(api, 'removeMcpServer').mockResolvedValue() const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) - render() + render() fireEvent.click(await screen.findByRole('button', { name: 'Remove' })) await waitFor(() => expect(confirm).toHaveBeenCalledOnce()) diff --git a/frontend/src/components/McpScopes.test.tsx b/frontend/src/components/McpScopes.test.tsx index 79bdcc0d7..7ddefa67e 100644 --- a/frontend/src/components/McpScopes.test.tsx +++ b/frontend/src/components/McpScopes.test.tsx @@ -23,7 +23,7 @@ it('ticks scopes off the advertised list and pins them', async () => { }) const set = vi.spyOn(api, 'setMcpServerScopes').mockResolvedValue({ ...server, scopes: ['read:me'] }) const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) - render() + render() fireEvent.click(await screen.findByRole('button', { name: 'Scopes' })) fireEvent.click(await screen.findByLabelText('delete:jira:agent-interface')) diff --git a/frontend/src/components/McpServerDirectory.test.tsx b/frontend/src/components/McpServerDirectory.test.tsx index cc8f24b52..965130255 100644 --- a/frontend/src/components/McpServerDirectory.test.tsx +++ b/frontend/src/components/McpServerDirectory.test.tsx @@ -21,7 +21,7 @@ function renderPane(installed: string[] = []) { vi.spyOn(api, 'mcpServers').mockResolvedValue(installed.map((name) => ({ name }) as McpServer)) vi.spyOn(api, 'mcpServerDirectory').mockResolvedValue(directory) const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) - render() + render() } it('offers the directory servers not yet added and filters them as you type', async () => { diff --git a/frontend/src/components/ProviderConnect.test.tsx b/frontend/src/components/ProviderConnect.test.tsx index e4fe86f08..683390638 100644 --- a/frontend/src/components/ProviderConnect.test.tsx +++ b/frontend/src/components/ProviderConnect.test.tsx @@ -49,11 +49,13 @@ function renderCard( apiKey = null, usage = null, keySpendToday = null, + isAdmin = true, }: { subscription?: ProviderSubscription | null apiKey?: ProviderKey | null usage?: UsageProviderSummary | null keySpendToday?: number | null + isAdmin?: boolean } = {}, ) { const queryClient = new QueryClient() @@ -66,6 +68,7 @@ function renderCard( apiKey={apiKey} usage={usage} keySpendToday={keySpendToday} + isAdmin={isAdmin} /> , ) @@ -85,6 +88,15 @@ async function flush() { } describe('ProviderConnect', () => { + it('keeps personal sign-in and hides installation key controls from a member', () => { + renderCard(provider(), { apiKey: sharedKey, isAdmin: false }) + + expect(screen.getByRole('button', { name: 'Sign in with Anthropic' })).toBeTruthy() + expect(screen.getByText('…4f2a')).toBeTruthy() + expect(screen.queryByRole('button', { name: 'Replace' })).toBeNull() + expect(screen.queryByRole('button', { name: 'Remove Anthropic API key' })).toBeNull() + }) + it('keeps the API-key input collapsed until the user asks for it', () => { renderCard(provider()) diff --git a/frontend/src/components/ResourcePages.test.tsx b/frontend/src/components/ResourcePages.test.tsx index ecaa9a44a..fc06e9e8f 100644 --- a/frontend/src/components/ResourcePages.test.tsx +++ b/frontend/src/components/ResourcePages.test.tsx @@ -43,6 +43,7 @@ function renderProviders(snapshot: UsageProviderSummary = usage) { render( { ]) const view = render( - + , ) await screen.findByText('No MCP servers installed.') diff --git a/frontend/src/components/ServicesPane.test.tsx b/frontend/src/components/ServicesPane.test.tsx index e079e4bc7..28b9c77e1 100644 --- a/frontend/src/components/ServicesPane.test.tsx +++ b/frontend/src/components/ServicesPane.test.tsx @@ -111,13 +111,13 @@ function stubFetch(states: Service[][]) { return fetchMock } -function renderPane() { +function renderPane(isAdmin = true) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } }, }) render( - + , ) } @@ -138,6 +138,18 @@ async function flush() { } describe('ServicesPane', () => { + it('keeps personal sign-in and hides shared controls from a member', async () => { + stubFetch([[{ ...connected, isOauth: true, connections: [] }]]) + renderPane(false) + + fireEvent.click(await screen.findByRole('button', { name: 'View GitHub' })) + + expect(screen.getByRole('button', { name: 'Connect' })).toBeTruthy() + expect(screen.queryByRole('button', { name: 'Replace connection' })).toBeNull() + expect(screen.queryByRole('button', { name: 'Disconnect service' })).toBeNull() + expect(screen.queryByRole('button', { name: 'Create App' })).toBeNull() + }) + it('confirms service removal and refreshes its connection state', async () => { const fetchMock = stubFetch([[connected], [disconnected]]) const confirm = vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(true) @@ -359,7 +371,7 @@ describe('ServicesPane', () => { managed: true, managedBy: 'Manager', facts: { url: 'https://manager.test/slack', client_id: '1234.5678' }, - installUrl: '/api/oauth/slack/connect?install=1', + installUrl: '/api/oauth/slack/install', } const ready: Service = { ...waiting, connected: true, facts: { ...waiting.facts, team: 'Acme' } } stubFetch([[waiting], [ready]]) @@ -381,7 +393,7 @@ describe('ServicesPane', () => { }) it('offers Install on a self-hosted Slack card with a client but no grant', async () => { - const installUrl = '/api/oauth/slack/connect?install=1' + const installUrl = '/api/oauth/slack/install' stubFetch([[{ ...pasteOnly, slug: 'slack', title: 'Slack', installUrl }]]) renderPane() diff --git a/frontend/src/components/SettingsPages.test.tsx b/frontend/src/components/SettingsPages.test.tsx index ba5edd464..4311ff885 100644 --- a/frontend/src/components/SettingsPages.test.tsx +++ b/frontend/src/components/SettingsPages.test.tsx @@ -8,6 +8,7 @@ import { UserPreferencesProvider } from '../lib/preferences' import { SETTINGS_FIELDS } from './settings' import type { AgentSetting, + AccountRole, AppSettingChoices, PersonalSettings, AppsSettingsResponse, @@ -446,7 +447,7 @@ function stubFetch( ) } -function renderSettings(workPath?: string) { +function renderSettings(workPath?: string, role: AccountRole = 'admin') { window.history.replaceState(null, '', workPath ?? '/settings/personal') vi.stubGlobal('matchMedia', vi.fn(() => ({ matches: true, @@ -462,7 +463,7 @@ function renderSettings(workPath?: string) { id: 'operator', username: 'operator@example.invalid', isDefault: true, - role: 'admin', + role, }} /> @@ -481,6 +482,43 @@ afterEach(() => { }) describe('SettingsPages app fields', () => { + it('hides shared settings destinations from a member', async () => { + stubFetch() + renderSettings('/settings/connections?tab=browser', 'member') + + expect(await screen.findByRole('heading', { name: 'Services' })).toBeTruthy() + expect(screen.queryByRole('link', { name: 'Agents' })).toBeNull() + expect(screen.queryByRole('link', { name: 'Skills' })).toBeNull() + expect(screen.queryByRole('link', { name: 'Browser' })).toBeNull() + fireEvent.change(screen.getByLabelText('Search settings'), { + target: { value: 'Field Notes' }, + }) + expect(screen.getByText('No matching settings.')).toBeTruthy() + + cleanup() + renderSettings('/settings/providers', 'member') + await screen.findByRole('heading', { name: 'Providers' }) + expect(screen.queryByRole('button', { name: 'Add provider' })).toBeNull() + }) + + it('hides an app settings link from a member', async () => { + stubFetch() + renderSettings('/field_notes', 'member') + + await screen.findByRole('link', { name: 'Field Notes' }) + expect(screen.queryByRole('link', { name: 'Field Notes settings' })).toBeNull() + }) + + it('treats shared and app settings addresses as unknown for a member', async () => { + stubFetch() + renderSettings('/settings/agents', 'member') + expect(await screen.findByText('No settings page matches this address.')).toBeTruthy() + + cleanup() + renderSettings('/apps/field_notes/settings', 'member') + expect(await screen.findByText('No settings page matches this address.')).toBeTruthy() + }) + it('opens browser profiles through Connections and settings search', async () => { stubFetch() renderSettings() diff --git a/frontend/src/components/SettingsPages.tsx b/frontend/src/components/SettingsPages.tsx index 683cf13f3..86b322c8d 100644 --- a/frontend/src/components/SettingsPages.tsx +++ b/frontend/src/components/SettingsPages.tsx @@ -106,10 +106,13 @@ export function SettingsPages({ active?: boolean }) { const [location, navigate] = useLocation() + const isAdmin = account.role === 'admin' + const sections = SECTIONS.filter((entry) => isAdmin || !['agents', 'skills'].includes(entry.id)) + const connectionTabs = CONNECTION_TABS.filter((tab) => isAdmin || tab.id !== 'browser') const searchParams = new URLSearchParams(useSearch()) const fieldTarget = searchParams.get('field') const connectionsTab = - CONNECTION_TABS.find((tab) => tab.id === searchParams.get('tab'))?.id ?? 'services' + connectionTabs.find((tab) => tab.id === searchParams.get('tab'))?.id ?? 'services' const content = useRef(null) const section = appName ? `apps/${appName}` : location.slice('/settings/'.length) || 'providers' const formPath = `${import.meta.env.BASE_URL.replace(/\/$/, '')}${appName ? `/apps/${appName}/settings` : '/settings'}` @@ -121,7 +124,7 @@ export function SettingsPages({ const choicesQuery = useQuery({ queryKey: ['appSettingChoices', appName], queryFn: () => api.getAppSettingChoices(appName!), - enabled: Boolean(appName), + enabled: Boolean(appName && isAdmin), }) const harnessesQuery = useQuery({ queryKey: ['harnesses'], queryFn: api.harnesses }) const agentsQuery = useQuery({ queryKey: ['agents'], queryFn: api.agents }) @@ -218,10 +221,11 @@ export function SettingsPages({ const appTab = ['agents', 'bots', 'channels'].find((tab) => location.endsWith(`/${tab}`)) ?? 'options' const validAppPage = !appName || - location === `/apps/${appName}/settings` || - (hasAgents && location === `/apps/${appName}/settings/agents`) || - Boolean(app?.bot && location === `/apps/${appName}/settings/bots`) || - (hasChannels && location === `/apps/${appName}/settings/channels`) + (isAdmin && + (location === `/apps/${appName}/settings` || + (hasAgents && location === `/apps/${appName}/settings/agents`) || + Boolean(app?.bot && location === `/apps/${appName}/settings/bots`) || + (hasChannels && location === `/apps/${appName}/settings/channels`))) const hasOptions = Boolean( app && (app.settings.length || app.workflows.some((workflow) => workflow.fields.length)), ) @@ -230,11 +234,12 @@ export function SettingsPages({ if (hasAgents) paneSection = 'agents' else if (app?.bot) paneSection = 'bots' } - const executionPage = section === 'agents' || Boolean(appName && ['agents', 'bots'].includes(paneSection)) + const executionPage = isAdmin && (section === 'agents' || Boolean(appName && ['agents', 'bots'].includes(paneSection))) const Content = appName ? 'section' : 'main' const title = - SECTIONS.find((entry) => entry.id === section)?.label ?? (app ? appLabel(app.name) : 'Settings') - const formPage = section === 'agents' || section === 'personal' || Boolean(app && validAppPage) + sections.find((entry) => entry.id === section)?.label ?? + (app && validAppPage ? appLabel(app.name) : 'Settings') + const formPage = (isAdmin && section === 'agents') || section === 'personal' || Boolean(app && validAppPage) const executionChanged = defaults && savedDefaults && @@ -424,18 +429,20 @@ export function SettingsPages({ setAppEdits((current) => ({ ...current, [name]: change(current[name] ?? {}) })) } - const searchResults = SECTIONS.map((entry) => ({ + const searchResults = sections.map((entry) => ({ label: entry.label, owner: entry.group, kind: 'Section', terms: '', path: `/settings/${entry.id}`, - })).concat([{ + })).concat(isAdmin ? [{ label: 'Browser profiles', owner: 'Connections', kind: 'Section', terms: 'browser sessions sign-ins saved logins', path: '/settings/connections?tab=browser', - }], Object.values(SETTINGS_FIELDS).map((field) => ({ - label: field.label, owner: SECTIONS.find((entry) => entry.id === field.section)!.label, + }] : [], Object.values(SETTINGS_FIELDS).filter((field) => + sections.some((entry) => entry.id === field.section), + ).map((field) => ({ + label: field.label, owner: sections.find((entry) => entry.id === field.section)!.label, kind: 'Field', terms: field.terms, path: `/settings/${field.section}?field=${field.field}`, - })), apps.flatMap((entry) => [ + })), isAdmin ? apps.flatMap((entry) => [ { label: appLabel(entry.name), owner: 'App settings', kind: 'Section', terms: '', path: `/apps/${entry.name}/settings` }, ...(entry.bot && channels.includes('slack') ? [{ label: 'Answer only when I tag the bot', owner: appLabel(entry.name), kind: 'Field', @@ -460,7 +467,7 @@ export function SettingsPages({ terms: `agent override inheritance ${agent.description}`, path: `/apps/${entry.name}/settings/${agent.name === entry.bot ? 'bots' : 'agents'}?field=${encodeURIComponent(`agent.${agent.name}.${field.field}`)}`, }))), - ])).filter((entry) => `${entry.label} ${entry.owner} ${entry.terms}`.toLowerCase().includes(search.trim().toLowerCase())) + ]) : []).filter((entry) => `${entry.label} ${entry.owner} ${entry.terms}`.toLowerCase().includes(search.trim().toLowerCase())) useEffect(() => { if (!active || !fieldTarget) return @@ -536,7 +543,7 @@ export function SettingsPages({ {['AI execution', 'Tools & access', 'Organization', 'Personal'].map((group) => (
{group}
- {SECTIONS.filter((entry) => entry.group === group).map((entry) => ( + {sections.filter((entry) => entry.group === group).map((entry) => ( )}
- {appName && app && ( + {appName && app && validAppPage && ( <>

{app.description}

{(app.bot || (hasOptions && hasAgents)) && ( @@ -709,7 +716,7 @@ export function SettingsPages({ busy={saving} /> )} - {page === 'agents' && executionReady && effectiveDefaults && ( + {isAdmin && page === 'agents' && executionReady && effectiveDefaults && ( { for (const query of executionQueries) if (query.isError) void query.refetch() }} + isAdmin={isAdmin} /> )} {page === 'connections' && ( <>