diff --git a/docsgpt/api/user/resource_access.py b/docsgpt/api/user/resource_access.py index 9a01f017..56e1ae91 100644 --- a/docsgpt/api/user/resource_access.py +++ b/docsgpt/api/user/resource_access.py @@ -27,6 +27,7 @@ from typing import Iterable, Optional from sqlalchemy import Connection, text +from docsgpt.connectors.permissions import owner_credential_writes from docsgpt.storage.db.base_repository import canonical_uuid, looks_like_uuid from docsgpt.storage.db.repositories.agents import AgentsRepository from docsgpt.storage.db.repositories.prompts import PromptsRepository @@ -1077,28 +1078,34 @@ def _user_labels(conn: Connection, user_ids: Iterable[Optional[str]]) -> dict[st def _connection_state(conn: Connection, tool: dict, owner: Optional[str], policies_box: list) -> tuple: - """``(reason, connection)`` for a connection-backed tool the holder runs as ``owner``.""" + """``(reason, connection, mode)`` for a tool the holder runs as ``owner``. + + ``connection`` (``{id, connector_key, name}``) and ``mode`` (``owner`` or + ``member``, after any mode an admin forces) are set for a tool that has a + connection, and ``connection`` also for one that lost it. ``reason`` is + why the connection keeps the tool from running, else None. + """ from docsgpt.connectors import catalog, service - from docsgpt.connectors.resolve import connection_stop_reason, resolve_connection + from docsgpt.connectors.resolve import connection_stop_reason, effective_credential_mode, resolve_connection if not tool.get("connection_id") and not catalog.definition_for_tool(tool.get("name") or ""): - return None, None + return None, None, None if not policies_box: policies_box.append(service.load_policies(conn)) resolved = resolve_connection(tool, owner, conn=conn, policies=policies_box[0]) reason = connection_stop_reason(tool, resolved) - if reason is None: - return None, None if resolved is not None: connection = { "id": resolved.connection_id, "connector_key": resolved.connector_key, "name": resolved.connector_name, } - else: - definition = catalog.definition_for_tool(tool.get("name") or "") - connection = {"id": None, "connector_key": definition.key, "name": definition.name} - return reason, connection + mode = effective_credential_mode(tool, policies_box[0], resolved.connector_key) + return reason, connection, mode + if reason is None: + return None, None, None + definition = catalog.definition_for_tool(tool.get("name") or "") + return reason, {"id": None, "connector_key": definition.key, "name": definition.name}, None def resource_states( @@ -1137,7 +1144,14 @@ def resource_states( ``connector_disabled``. ``sponsor`` and ``contact`` are ``{user_id, label}`` or None: the recorded sponsor, and someone other than the reader who can fix it. ``connection`` (``{id, - connector_key, name}``) names the service for a connection reason. + connector_key, name}``) names the service of a tool with a + connection, and of one whose connection reason stopped it. + For a running tool, ``credential_mode`` is ``owner`` or ``member`` + when it has a connection (else None), ``account`` (``{user_id, + label}``) is whose account an ``owner``-mode connection acts as, and + ``owner_credential_writes`` names its write actions on credentials + its owner stored (what the API write allowlist covers); empty for + everything else. ``can_confirm``: the reader may take it over on their next save; ``can_reconnect``: the reader owns the connection that needs signing in again. @@ -1163,12 +1177,16 @@ def resource_states( for resource_type, rid in pairs: key = sponsor_key(resource_type, rid) info = rows.get(key) or {} - connection = None + connection = mode = account = None + writes: list[str] = [] if resource_type == "tool": tool_row, access = resolve_holder_tool(conn, holder_type, holder, rid, tools_repo=tools_repo) reason = access.reason if tool_row is not None and reason is None: - reason, connection = _connection_state(conn, tool_row, owner, policies_box) + reason, connection, mode = _connection_state(conn, tool_row, owner, policies_box) + # Whose account an owner-mode connection acts as: the tool's owner. + account = tool_row.get("user_id") if mode == "owner" else None + writes = owner_credential_writes(tool_row) else: access = ref_access(conn, holder_type, holder, resource_type, rid) reason = access.reason @@ -1204,6 +1222,9 @@ def resource_states( "sponsor": sponsor, "contact": contact, "connection": connection, + "credential_mode": mode, + "account": account, + "owner_credential_writes": writes, "can_confirm": can_confirm, "can_reconnect": bool( reason == REASON_CONNECTION_NEEDS_RECONNECT @@ -1213,9 +1234,9 @@ def resource_states( and viewer == resource_owner ), }) - labels = _user_labels(conn, [u for e in entries for u in (e["sponsor"], e["contact"])]) + labels = _user_labels(conn, [u for e in entries for u in (e["sponsor"], e["contact"], e["account"])]) for entry in entries: - for field_name in ("sponsor", "contact"): + for field_name in ("sponsor", "contact", "account"): user_id = entry[field_name] entry[field_name] = {"user_id": user_id, "label": labels.get(user_id) or user_id} if user_id else None return entries diff --git a/docsgpt/connectors/resolve.py b/docsgpt/connectors/resolve.py index 6793e2ae..21593e89 100644 --- a/docsgpt/connectors/resolve.py +++ b/docsgpt/connectors/resolve.py @@ -96,19 +96,35 @@ def resolve_connection( return _resolve(conn, resource, invoker_user_id, policies) +def effective_credential_mode(resource: dict, policies: dict, connector_key: Optional[str]) -> str: + """Whose account a connection-backed resource runs with: ``owner`` or ``member``. + + The resource's own ``credential_mode``, unless an admin forces one mode + for every share of its connector. + + Args: + resource: A ``user_tools`` or ``sources`` row. + policies: Connector policies from ``service.load_policies``. + connector_key: Catalog key of the resource's connection. + + Returns: + :data:`MODE_OWNER` or :data:`MODE_MEMBER`. + """ + policy = (policies.get(connector_key) or {}) if connector_key else {} + if policy.get("credential_mode") in (MODE_OWNER, MODE_MEMBER): + return policy["credential_mode"] + return MODE_MEMBER if resource.get("credential_mode") == MODE_MEMBER else MODE_OWNER + + def _resolve(conn, resource: dict, invoker_user_id: Optional[str], policies: Optional[dict]) -> ResolvedConnection: connection_id = resource.get("connection_id") - mode = resource.get("credential_mode") or MODE_OWNER owner = resource.get("user_id") repo = ConnectorSessionsRepository(conn) owned = repo.get(str(connection_id)) owned_key = catalog.connector_key_for_row(owned) if owned else None if policies is None: policies = service.load_policies(conn) - policy = (policies.get(owned_key) or {}) if owned_key else {} - if policy.get("credential_mode") in (MODE_OWNER, MODE_MEMBER): - # An admin forces whose account every share of this connector uses. - mode = policy["credential_mode"] + mode = effective_credential_mode(resource, policies, owned_key) if owned is not None and owner and owned.get("user_id") != owner: # A resource may only point at its own owner's connection. logger.warning( diff --git a/tests/api/user/test_resource_states.py b/tests/api/user/test_resource_states.py index c4668d0a..71b1cd4a 100644 --- a/tests/api/user/test_resource_states.py +++ b/tests/api/user/test_resource_states.py @@ -242,6 +242,92 @@ class TestReasons: assert _states(pg_conn, agent_id) == {} +class TestCredentials: + """Whose account or credentials each running tool uses, for the share dialog.""" + + _SEND = {"name": "send_message", "active": True} + _READ = {"name": "get_updates", "active": True} + + def test_owner_mode_connection_names_the_account_holder(self, pg_conn): + tool = str(UserToolsRepository(pg_conn).create( + OWNER, "telegram", connection_id=_connection(pg_conn), actions=[self._SEND, self._READ], + )["id"]) + agent_id, _ = _agent(pg_conn, tools=[tool]) + with _patch_db(pg_conn): + state = _states(pg_conn, agent_id)[f"tool:{tool}"] + assert state["state"] == "active" + assert state["credential_mode"] == "owner" + assert state["account"] == {"user_id": OWNER, "label": OWNER} + assert state["connection"]["name"] == "Telegram" + assert state["owner_credential_writes"] == ["send_message"] + + def test_member_mode_connection_has_no_account_holder(self, pg_conn): + tool = str(UserToolsRepository(pg_conn).create( + OWNER, "telegram", connection_id=_connection(pg_conn), credential_mode="member", + )["id"]) + agent_id, _ = _agent(pg_conn, tools=[tool]) + with _patch_db(pg_conn): + state = _states(pg_conn, agent_id)[f"tool:{tool}"] + assert state["credential_mode"] == "member" + assert state["account"] is None + assert state["connection"]["connector_key"] == "telegram" + + def test_admin_forced_mode_wins(self, pg_conn): + tool = str(UserToolsRepository(pg_conn).create( + OWNER, "telegram", connection_id=_connection(pg_conn), + )["id"]) + agent_id, _ = _agent(pg_conn, tools=[tool]) + ConnectorPoliciesRepository(pg_conn).upsert("telegram", credential_mode="member") + with _patch_db(pg_conn): + state = _states(pg_conn, agent_id)[f"tool:{tool}"] + assert state["credential_mode"] == "member" + assert state["account"] is None + + def test_teammates_owner_mode_tool_uses_their_account(self, pg_conn): + agent_id, team_id = _agent(pg_conn) + for member in (OWNER, OTHER): + if not TeamMembersRepository(pg_conn).is_member(member, team_id): + TeamMembersRepository(pg_conn).add_member(team_id, member) + tool = str(UserToolsRepository(pg_conn).create( + OTHER, "telegram", connection_id=_connection(pg_conn, user=OTHER), + )["id"]) + TeamResourceGrantsRepository(pg_conn).grant(team_id, "tool", tool, OTHER, OTHER) + AgentsRepository(pg_conn).update_by_id(agent_id, {"tools": [tool]}) + with _patch_db(pg_conn): + state = _states(pg_conn, agent_id)[f"tool:{tool}"] + assert state["state"] == "active" + assert state["account"] == {"user_id": OTHER, "label": OTHER} + + def test_tool_without_stored_credentials(self, pg_conn): + tool = str(UserToolsRepository(pg_conn).create(OWNER, "api_tool")["id"]) + source = str(SourcesRepository(pg_conn).create("mine", user_id=OWNER)["id"]) + agent_id, _ = _agent(pg_conn, tools=[tool], source_id=source) + states = _states(pg_conn, agent_id) + assert states[f"tool:{tool}"]["credential_mode"] is None + assert states[f"tool:{tool}"]["account"] is None + assert states[f"tool:{tool}"]["connection"] is None + assert states[f"tool:{tool}"]["owner_credential_writes"] == [] + assert states[f"source:{source}"]["owner_credential_writes"] == [] + assert states[f"source:{source}"]["credential_mode"] is None + + def test_api_tool_write_that_sends_a_saved_key(self, pg_conn): + actions = { + "create_ticket": { + "method": "POST", + "headers": {"properties": {"Authorization": {"has_value": True}}}, + }, + "list_tickets": { + "method": "GET", + "headers": {"properties": {"Authorization": {"has_value": True}}}, + }, + } + tool = str(UserToolsRepository(pg_conn).create( + OWNER, "api_tool", config={"actions": actions}, + )["id"]) + agent_id, _ = _agent(pg_conn, tools=[tool]) + assert _states(pg_conn, agent_id)[f"tool:{tool}"]["owner_credential_writes"] == ["create_ticket"] + + class TestTakeOver: def test_editor_who_can_edit_the_item_may_take_over(self, pg_conn): """An owner-lost resource the reading editor can edit is theirs to take over."""