From aec6263f6abe0c5925d59ab930d349f83712c4e2 Mon Sep 17 00:00:00 2001 From: Paulo Date: Tue, 10 Nov 2026 19:37:25 +0200 Subject: [PATCH] DRU-1135 -- Limit a team's app to its team and the admins --- backend/druks/api/dashboard.py | 7 ++- backend/druks/api/exceptions.py | 8 +++ backend/druks/api/runs.py | 17 ++++-- backend/druks/api/server.py | 7 ++- backend/druks/apps/loader.py | 3 +- backend/druks/chat/bots/routes.py | 8 ++- .../contrib/software_factory/subscribers.py | 8 ++- .../contrib/software_factory/workflows.py | 36 +++++++++---- backend/druks/exceptions.py | 4 ++ backend/druks/mcp/gateway/routes.py | 7 ++- backend/druks/mcp/server.py | 24 +++++++-- backend/druks/teams/dependencies.py | 53 ++++++++++++++++++ backend/druks/teams/models.py | 16 +++++- backend/druks/workflows.py | 38 ++++++++++++- .../test_build_attribution.py | 39 ++++++++++++++ .../software_factory/test_build_dispatch.py | 41 +++++++++++++- backend/tests/test_agent_routes.py | 34 ++++++++++++ backend/tests/test_apps.py | 18 +++++++ backend/tests/test_dashboard.py | 12 +++++ backend/tests/test_durable_sdk.py | 54 ++++++++++++++++++- backend/tests/test_mcp_endpoint.py | 22 ++++++++ backend/tests/test_teams.py | 24 +++++++++ backend/tests/test_whatsapp.py | 42 +++++++++++---- docs/chat.md | 6 +-- docs/concepts.md | 5 ++ docs/configuration.md | 6 +++ 26 files changed, 498 insertions(+), 41 deletions(-) create mode 100644 backend/druks/teams/dependencies.py diff --git a/backend/druks/api/dashboard.py b/backend/druks/api/dashboard.py index 4f2a3979e..c4f56e881 100644 --- a/backend/druks/api/dashboard.py +++ b/backend/druks/api/dashboard.py @@ -28,6 +28,7 @@ from druks.durable.models import Artifact, Run from druks.events.models import Event from druks.settings import get_settings +from druks.teams.dependencies import schedule_app_access PREVIEW_SIZE = 4 router = APIRouter(prefix="/api/dashboard", tags=["dashboard"]) @@ -192,7 +193,11 @@ async def list_current_schedules(session: SessionDep, response: Response) -> Das return DashboardSchedules(rows=rows) -@router.post("/schedules/{kind}/run", status_code=202) +@router.post( + "/schedules/{kind}/run", + status_code=202, + dependencies=[Depends(schedule_app_access)], +) async def run_schedule(kind: str) -> dict[str, str]: workflow = workflows.get(kind) if not workflow or not workflow.every: diff --git a/backend/druks/api/exceptions.py b/backend/druks/api/exceptions.py index 6c641d516..e09d870d2 100644 --- a/backend/druks/api/exceptions.py +++ b/backend/druks/api/exceptions.py @@ -44,6 +44,14 @@ def __init__(self, run_id: str) -> None: super().__init__(f"No run {run_id}.") +class AppPermissionDenied(AgentApiError): + status_code = 403 + code = "APP_PERMISSION_DENIED" + + def __init__(self, app: str) -> None: + super().__init__(f"This account cannot use app {app!r}.") + + class RunNotActive(AgentApiError): status_code = 409 code = "RUN_NOT_ACTIVE" diff --git a/backend/druks/api/runs.py b/backend/druks/api/runs.py index 04eeaa1f6..fc05158f0 100644 --- a/backend/druks/api/runs.py +++ b/backend/druks/api/runs.py @@ -1,10 +1,11 @@ from typing import Annotated -from fastapi import APIRouter, Body, HTTPException, Path, status +from fastapi import APIRouter, Body, Depends, HTTPException, Path, status from druks.accounts.context import current_account_id from druks.api.dependencies import SessionDep from druks.api.exceptions import ( + AppPermissionDenied, RunNotActive, RunNotFailed, RunNotFound, @@ -19,11 +20,16 @@ from druks.events.models import Event from druks.notifications.exceptions import AnswerNotAllowedError, InvalidChoiceError from druks.notifications.services import validate_in_app_answer +from druks.teams.dependencies import run_app_access router = APIRouter(prefix="/api/runs", tags=["runs"]) -@router.post("/{run}/resume", status_code=status.HTTP_204_NO_CONTENT) +@router.post( + "/{run}/resume", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(run_app_access)], +) async def resume_run( session: SessionDep, run_id: Annotated[str, Path(alias="run")], body: ResumeRequest ) -> None: @@ -60,9 +66,12 @@ async def resume_run( operation_id="cancel_run", tags=["agent"], openapi_extra={"x-idempotent": True}, + dependencies=[Depends(run_app_access)], response_model=CancelRunResponse, response_model_by_alias=True, - responses=agent_error_responses(RunNotFound("run-123"), RunNotActive("run-123")), + responses=agent_error_responses( + AppPermissionDenied("app-name"), RunNotFound("run-123"), RunNotActive("run-123") + ), ) async def cancel_run( session: SessionDep, @@ -111,9 +120,11 @@ async def cancel_run( operation_id="retry_run", tags=["agent"], openapi_extra={"x-destructive": False}, + dependencies=[Depends(run_app_access)], response_model=RetryRunResponse, response_model_by_alias=True, responses=agent_error_responses( + AppPermissionDenied("app-name"), RunNotFound("run-123"), RunNotFailed("run-123"), SubjectBusy("run-456"), diff --git a/backend/druks/api/server.py b/backend/druks/api/server.py index 3c4e610cb..1b364ec06 100644 --- a/backend/druks/api/server.py +++ b/backend/druks/api/server.py @@ -38,7 +38,7 @@ from druks.durable.engine import configure_engine, init_dbos, launch, shutdown from druks.durable.exceptions import AgentCallNotFound from druks.events.routes import router as events_router -from druks.exceptions import ObjectNotFound, ValidationError +from druks.exceptions import ObjectNotFound, PermissionDenied, ValidationError from druks.files.routes import router as files_router from druks.harnesses.exceptions import AgentConfigError, CatalogError from druks.harnesses.routes import router as providers_router @@ -187,6 +187,11 @@ async def _validation_error_handler(request: Request, exc: ValidationError) -> J return await _http_exception_handler(request, HTTPException(422, str(exc))) +@app.exception_handler(PermissionDenied) +async def _permission_denied_handler(request: Request, exc: PermissionDenied) -> JSONResponse: + return await _http_exception_handler(request, HTTPException(403, str(exc))) + + @app.exception_handler(PageReadError) async def _page_read_error_handler(request: Request, exc: PageReadError) -> JSONResponse: logging.getLogger(__name__).exception("page read failed: %s", exc) diff --git a/backend/druks/apps/loader.py b/backend/druks/apps/loader.py index 8b8f8e740..05471f31d 100644 --- a/backend/druks/apps/loader.py +++ b/backend/druks/apps/loader.py @@ -230,6 +230,7 @@ def mount(api: "FastAPI", app: type[App], modules: list["ModuleType"]) -> None: from fastapi import Depends from druks.accounts.dependencies import current_account + from druks.teams.dependencies import app_access prefix = f"/api/{app.name}" for router in app.get_routers(modules): @@ -237,7 +238,7 @@ def mount(api: "FastAPI", app: type[App], modules: list["ModuleType"]) -> None: router, prefix=prefix, tags=[app.name], - dependencies=[Depends(current_account)], + dependencies=[Depends(current_account), Depends(app_access(app.name))], ) dist = app.frontend_dist() if dist: diff --git a/backend/druks/chat/bots/routes.py b/backend/druks/chat/bots/routes.py index e366ab2f3..4fe4f61cd 100644 --- a/backend/druks/chat/bots/routes.py +++ b/backend/druks/chat/bots/routes.py @@ -12,6 +12,7 @@ from druks.apps.loader import get_app from druks.chat.enums import BotAccess from druks.chat.models import Conversation +from druks.teams.dependencies import pairing_app_access from .constants import ONE_TIME_CODE_TTL_SECONDS from .schemas import OneTimeCodeResponse, ResumeConversationResponse @@ -67,6 +68,7 @@ async def open_one_time_code( "/connections/{connection_id}/pairing", response_model=OneTimeCodeResponse, response_model_by_alias=True, + dependencies=[Depends(pairing_app_access)], ) async def pair( session: SessionDep, @@ -81,7 +83,11 @@ async def pair( raise HTTPException(404, "Connection not found.") -@router.delete("/connections/{connection_id}/pairing", status_code=204) +@router.delete( + "/connections/{connection_id}/pairing", + status_code=204, + dependencies=[Depends(pairing_app_access)], +) async def unpair( session: SessionDep, connection_id: str, diff --git a/backend/druks/contrib/software_factory/subscribers.py b/backend/druks/contrib/software_factory/subscribers.py index dbec5b76d..7ea236a74 100644 --- a/backend/druks/contrib/software_factory/subscribers.py +++ b/backend/druks/contrib/software_factory/subscribers.py @@ -9,6 +9,7 @@ from druks.contrib.software_factory.ticketing.enums import TicketStatus from druks.contrib.software_factory.workflows import Build, Profile, PullRequestReview from druks.db import atomic +from druks.exceptions import PermissionDenied from druks.models import Base from druks.services.exceptions import ServiceNotConnectedError from druks.signals import publish, subscribe @@ -153,7 +154,12 @@ async def ticket_transition_drives_the_funnel(*, payload: dict) -> None: if payload["source"] == settings.tracker and payload["status"] == trigger: try: await Build.dispatch(ticket=payload) - except (TicketAlreadyMerged, TicketNotRoutable, ServiceNotConnectedError) as error: + except ( + TicketAlreadyMerged, + TicketNotRoutable, + ServiceNotConnectedError, + PermissionDenied, + ) as error: # The delivery succeeded. A raise would answer 5xx, and the tracker would deliver # the same ticket again. logger.info("Ticket %s opens no build: %s", payload["identifier"], error) diff --git a/backend/druks/contrib/software_factory/workflows.py b/backend/druks/contrib/software_factory/workflows.py index f25182e04..e0448f463 100644 --- a/backend/druks/contrib/software_factory/workflows.py +++ b/backend/druks/contrib/software_factory/workflows.py @@ -14,12 +14,14 @@ from druks.contrib.software_factory.ticketing.enums import TicketStatus from druks.core.services import Github from druks.db import db_session +from druks.exceptions import PermissionDenied from druks.mcp.inbound import get_druks_mcp_server from druks.sandbox import SandboxMcpServer, SandboxSecret from druks.sandbox.layout import get_related_root, get_work_root from druks.services.exceptions import ServiceNotConnectedError from druks.settings import get_settings from druks.skills.models import Skill +from druks.teams.models import Team from druks.workflows import FatalError, Workflow, step from druks.workspaces import RepoWorkspace @@ -174,16 +176,30 @@ async def dispatch(cls, *, ticket: dict) -> str: await item.announce("build.rejected", reason=str(error)) raise account_id = None - if tracker := await SoftwareFactory.get_tracker(): - async with tracker: - account_id = await tracker.get_account_id(ticket["actor_id"]) - account_id = account_id or await tracker.get_account_id(ticket["assignee_id"]) - return await cls.start( - subject=item, - account_id=account_id, - assignee_email=ticket["assignee_email"], - assignee_name=ticket["assignee_name"], - ) + try: + if tracker := await SoftwareFactory.get_tracker(): + async with tracker: + account_id = await tracker.get_account_id(ticket["actor_id"]) + + if ( + ticket["actor_id"] + and not account_id + and not await Team.allows(db_session(), None, cls.app) + ): + raise PermissionDenied( + "The person who moved the ticket has no Druks account and so " + f"cannot use app {cls.app!r}." + ) + account_id = account_id or await tracker.get_account_id(ticket["assignee_id"]) + return await cls.start( + subject=item, + account_id=account_id, + assignee_email=ticket["assignee_email"], + assignee_name=ticket["assignee_name"], + ) + except PermissionDenied as error: + await item.announce("build.rejected", reason=str(error)) + raise async def run_multistep( self, diff --git a/backend/druks/exceptions.py b/backend/druks/exceptions.py index f3e79ff02..e59f026c8 100644 --- a/backend/druks/exceptions.py +++ b/backend/druks/exceptions.py @@ -15,6 +15,10 @@ class ValidationError(DruksError): """A person's input is refused. The message says why.""" +class PermissionDenied(DruksError): + """An account may not do this. The message says why.""" + + class DetachedRowError(DruksError): """A row was used outside the session that loaded it.""" diff --git a/backend/druks/mcp/gateway/routes.py b/backend/druks/mcp/gateway/routes.py index d0149b81d..e2c9b0d72 100644 --- a/backend/druks/mcp/gateway/routes.py +++ b/backend/druks/mcp/gateway/routes.py @@ -6,9 +6,10 @@ from druks.accounts.dependencies import current_account from druks.accounts.models import Account from druks.api.dependencies import SessionDep -from druks.api.exceptions import RunNotFound, agent_error_responses +from druks.api.exceptions import AppPermissionDenied, RunNotFound, agent_error_responses from druks.mcp.gateway import exceptions as gate_errors from druks.mcp.gateway import schemas, services +from druks.teams.dependencies import run_app_access # Docstrings here are the derived tool descriptions and operation_id is the # tool name — renaming one is a break, never a refactor side effect. @@ -20,7 +21,9 @@ operation_id="get_gate", response_model=schemas.GateResponse, response_model_by_alias=True, + dependencies=[Depends(run_app_access)], responses=agent_error_responses( + AppPermissionDenied("app-name"), RunNotFound("run-123"), gate_errors.GateNotOpen("run-123"), ), @@ -43,7 +46,9 @@ async def get_gate( openapi_extra={"x-destructive": False, "x-idempotent": True}, response_model=schemas.GateAnswerResponse, response_model_by_alias=True, + dependencies=[Depends(run_app_access)], responses=agent_error_responses( + AppPermissionDenied("app-name"), gate_errors.InvalidGateAnswer("unknown control 'merge'"), gate_errors.GateAnswerNotAllowed("Only the number's admin can answer this run."), RunNotFound("run-123"), diff --git a/backend/druks/mcp/server.py b/backend/druks/mcp/server.py index 7e268cdd9..a17532985 100644 --- a/backend/druks/mcp/server.py +++ b/backend/druks/mcp/server.py @@ -20,13 +20,14 @@ from mcp.types import CallToolRequestParams, ToolAnnotations from druks.accounts.exceptions import InvalidPatError -from druks.accounts.models import PersonalAccessToken +from druks.accounts.models import Account, PersonalAccessToken from druks.api.exceptions import AgentApiError, InvalidToolArguments, ToolNotAvailable from druks.apps.exceptions import AppBotError from druks.apps.loader import iter_apps from druks.apps.registry import bots from druks.database import session_scope from druks.mcp.exceptions import InvalidAgentToolError +from druks.teams.models import Team _INSTRUCTIONS = """\ Druks coordinates durable agent runs over shared work items. Start with @@ -300,13 +301,22 @@ def _annotate( tool.auth = _visible_while(preconditions) -def _is_visible(context: AuthContext) -> bool: +async def _is_visible(app_names: set[str], context: AuthContext) -> bool: # A limited key lists and calls only the tools it names. Any other key gets the # Druks toolkit: the agent tools, never a tool only a Bot or a browser call calls. allowed = context.token.claims["allowed_tools"] if allowed is not None: - return context.component.name in allowed - return "agent" in context.component.tags + is_visible = context.component.name in allowed + else: + is_visible = "agent" in context.component.tags + if not is_visible: + return False + app = next((tag for tag in context.component.tags if tag in app_names), "") + if not app: + return True + async with session_scope() as session: + account = await Account.get(session, id=context.token.claims["account_id"]) + return await Team.allows(session, account, app) def create_mcp_app(api: FastAPI) -> StarletteWithLifespan: @@ -339,7 +349,11 @@ def create_mcp_app(api: FastAPI) -> StarletteWithLifespan: providers=[provider], instructions=_INSTRUCTIONS, auth=PatTokenVerifier(), - middleware=[AuthMiddleware(auth=_is_visible), ToolErrors(), StrictInputValidation()], + middleware=[ + AuthMiddleware(auth=partial(_is_visible, app_names)), + ToolErrors(), + StrictInputValidation(), + ], ) # Derivation primed app.openapi()'s cache mid-assembly; drop it. api.openapi_schema = None diff --git a/backend/druks/teams/dependencies.py b/backend/druks/teams/dependencies.py new file mode 100644 index 000000000..047a5c904 --- /dev/null +++ b/backend/druks/teams/dependencies.py @@ -0,0 +1,53 @@ +from collections.abc import Awaitable, Callable + +from fastapi import Depends, Request + +from druks.accounts.dependencies import current_account, current_session_account +from druks.accounts.models import Account +from druks.api.dependencies import SessionDep +from druks.api.exceptions import AppPermissionDenied +from druks.apps.registry import workflows +from druks.chat.bots.service import get_bot_connection +from druks.durable.models import Run +from druks.teams.models import Team + +# The gates below read a path value from the request. A path parameter in a gate's +# signature would replace the route's own one, and its description, in the schema +# that the MCP tools come from. + + +def app_access(app: str) -> Callable[..., Awaitable[None]]: + """The gate in front of every route of one app.""" + + async def access(session: SessionDep, account: Account = Depends(current_account)) -> None: + if not await Team.allows(session, account, app): + raise AppPermissionDenied(app) + + return access + + +async def run_app_access( + request: Request, session: SessionDep, account: Account = Depends(current_account) +) -> None: + """The gate for a route that acts on a run. An unknown run is the route's to answer.""" + run = await session.get(Run, request.path_params["run"]) + if run and not await Team.allows(session, account, workflows.get(run.kind).app): + raise AppPermissionDenied(workflows.get(run.kind).app) + + +async def schedule_app_access( + request: Request, session: SessionDep, account: Account = Depends(current_account) +) -> None: + """The gate for a schedule. An unknown kind is the route's to answer.""" + workflow = workflows.get(request.path_params["kind"]) + if workflow and not await Team.allows(session, account, workflow.app): + raise AppPermissionDenied(workflow.app) + + +async def pairing_app_access( + request: Request, session: SessionDep, account: Account = Depends(current_session_account) +) -> None: + """The gate for a number's pairing. An unknown connection is the route's to answer.""" + connection = await get_bot_connection(session, request.path_params["connection_id"]) + if connection and not await Team.allows(session, account, connection.identity["app"]): + raise AppPermissionDenied(connection.identity["app"]) diff --git a/backend/druks/teams/models.py b/backend/druks/teams/models.py index 3247b823b..dba77ce6e 100644 --- a/backend/druks/teams/models.py +++ b/backend/druks/teams/models.py @@ -5,7 +5,7 @@ from druks.accounts.enums import AccountKind from druks.accounts.models import Account -from druks.apps.loader import iter_apps +from druks.apps.loader import get_app, iter_apps from druks.core.models import Uuid7Pk from druks.exceptions import ValidationError from druks.models import Base, Model @@ -40,6 +40,20 @@ class Team(Model, Uuid7Pk, ordering=("name",)): async def list_all(cls, session: AsyncSession) -> list["Team"]: return list(await session.scalars(sqlalchemy.select(cls).order_by(cls.name))) + @classmethod + async def allows(cls, session: AsyncSession, account: Account | None, app: str) -> bool: + """Whether the account can use the app. None means a person without an account.""" + if not app or get_app(app).builtin: + return True + if account and (account.is_admin or account.kind != AccountKind.OPERATOR): + return True + teams = list(await session.scalars(sqlalchemy.select(cls).where(cls.apps.contains([app])))) + if not teams: + return True + if account: + return any(member.id == account.id for team in teams for member in team.members) + return False + @classmethod async def create( cls, session: AsyncSession, *, name: str, member_ids: list[str], apps: list[str] diff --git a/backend/druks/workflows.py b/backend/druks/workflows.py index ef593dd47..5d22525c1 100644 --- a/backend/druks/workflows.py +++ b/backend/druks/workflows.py @@ -84,6 +84,7 @@ SubjectSummary, ) from druks.events.models import Event +from druks.exceptions import PermissionDenied from druks.harnesses.exceptions import HarnessError, HarnessSandboxProvisioningError from druks.models import StoredSubject, snake_name from druks.notifications.outbox import send_notification @@ -94,6 +95,7 @@ from druks.sandbox.models import SandboxIdentity, SecretRef from druks.sandbox.templates import get_template_id from druks.signals import publish +from druks.teams.models import Team from druks.user_settings.models import SettingsOverride from druks.workspaces import Workspace @@ -1242,6 +1244,20 @@ async def start( cls._validate_subject(subject) async with bound_session() as session: account = await Account.get_for_run(session, account_id or current_account_id.get()) + if DBOS.workflow_id: + # A replay runs the caller's body again. The step keeps the first answer, + # so a later change to a team cannot refuse a run that started. + async def _access() -> bool: + async with step_session() as access_session: + return await Team.allows(access_session, account, cls.app) + + has_access = await DBOS.run_step_async( + StepOptions(name=f"{cls.kind}.start_access"), _access + ) + else: + has_access = await Team.allows(session, account, cls.app) + if not has_access: + raise PermissionDenied(f"Account {account.username!r} cannot use app {cls.app!r}.") account_id = account.id wire: dict[str, Any] = {} if cls._run_input_model: @@ -1413,8 +1429,26 @@ async def _run_instance( subject: dict[str, Any] | None = None, input: dict[str, Any] | None = None, ) -> Any: - async with step_session() as session: - account = await Account.get_for_run(session, (input or {}).get(_ACCOUNT_INPUT_KEY)) + input = input or {} + account_id = input.get(_ACCOUNT_INPUT_KEY) + if account_id: + async with step_session() as session: + account = await Account.get_for_run(session, account_id) + else: + # Only a scheduled tick arrives with no account. The step keeps the account + # that passed, so a replay does not check again. + async def _account() -> str: + async with step_session() as session: + account = await Account.get_for_run(session, None) + if await Team.allows(session, account, cls.app): + return account.id + raise PermissionDenied(f"Account {account.username!r} cannot use app {cls.app!r}.") + + account_id = await DBOS.run_step_async( + StepOptions(name=f"{cls.kind}.scheduled_access"), _account + ) + async with step_session() as session: + account = await Account.get(session, id=account_id) instance, run_kwargs = _bind_instance(cls, subject, input, account=account) instance._workflow_id = DBOS.workflow_id # type: ignore[assignment] token = current_workflow.set(instance) diff --git a/backend/tests/software_factory/test_build_attribution.py b/backend/tests/software_factory/test_build_attribution.py index 64ae36104..664c132ea 100644 --- a/backend/tests/software_factory/test_build_attribution.py +++ b/backend/tests/software_factory/test_build_attribution.py @@ -11,9 +11,13 @@ from druks.contrib.software_factory.workflows import Build from druks.core.webhooks.jira import JiraEvents from druks.core.webhooks.linear import LinearEvents +from druks.events.models import Event +from druks.exceptions import PermissionDenied from druks.secrets.enums import IdentityStatus from druks.secrets.models import VaultSecret +from druks.teams.models import Team from druks.testing import make_settings +from sqlalchemy import select from software_factory.factories import make_test_work_item @@ -162,6 +166,41 @@ async def test_unconnected_actor_and_assignee_use_the_default_account(druks_db, assert not started[0]["account_id"] +async def test_ticket_move_without_an_account_does_not_fall_back(druks_db, started): + await Account.get_or_create(druks_db, "admin@example.com") + await Team.create(druks_db, name="Builders", member_ids=[], apps=["software_factory"]) + await connect_service( + "linear", + identity={"actor": "druks", "workspace": "Company"}, + secrets={"api_key": "key", "webhook_secret": "hook"}, + ) + item = await make_test_work_item(repo="company/app", source="linear", title="A change") + + with pytest.raises(PermissionDenied): + await Build.dispatch( + ticket={ + "source": item.source, + "ticket_id": "issue-uuid-2", + "identifier": item.ticket_key, + "status": "Ready", + "title": item.title, + "url": f"https://tracker.example/{item.ticket_key}", + "project_name": "company/app", + "labels": [], + "team": "team-1", + "actor_id": "person-without-account", + "assignee_id": None, + "assignee_email": None, + "assignee_name": None, + } + ) + + assert not started + [event] = list(await druks_db.scalars(select(Event).where(Event.type == "build.rejected"))) + assert event.type == "build.rejected" + assert event.subject_id == str(item.id) + + async def test_builtin_ticket_uses_its_account_id(druks_db, monkeypatch, started): assignee = await Account.get_or_create(druks_db, "github-boss") project = await Project.create(name="Company") diff --git a/backend/tests/software_factory/test_build_dispatch.py b/backend/tests/software_factory/test_build_dispatch.py index 49d7a3b20..4a90a1b8a 100644 --- a/backend/tests/software_factory/test_build_dispatch.py +++ b/backend/tests/software_factory/test_build_dispatch.py @@ -3,20 +3,27 @@ import pytest from conftest import connect_service +from druks.accounts.models import Account from druks.contrib.software_factory import subscribers # noqa: F401 (the import registers them) from druks.contrib.software_factory.app import SoftwareFactory from druks.contrib.software_factory.contracts import ReviewWork -from druks.contrib.software_factory.enums import Resolution +from druks.contrib.software_factory.enums import Resolution, Status from druks.contrib.software_factory.exceptions import TicketAlreadyMerged, TicketNotRoutable from druks.contrib.software_factory.workflows import Build +from druks.durable.models import Run from druks.events.models import Event +from druks.exceptions import PermissionDenied from druks.services.exceptions import ServiceNotConnectedError from druks.signals import publish +from druks.teams.models import Team from druks.testing import seed_run +from druks.workflows import Workflow from sqlalchemy import select from software_factory.factories import make_test_work_item, seed_build_run +_WORKFLOW_START = Workflow.start.__func__ + async def _connect_github() -> None: await connect_service( @@ -112,6 +119,38 @@ async def fake_start(cls, **kwargs): assert not started +async def test_ticket_move_outside_the_team_announces_and_starts_no_build( + druks_db, monkeypatch +) -> None: + await _connect_github() + await Account.get_or_create(druks_db, "admin@example.com") + member = await Account.get_or_create(druks_db, "member@example.com") + await Team.create(druks_db, name="Builders", member_ids=[], apps=["software_factory"]) + await SoftwareFactory.override_setting("tracker", "druks") + item = await make_test_work_item(repo="o/r", title="t", ticket_key="ACME-13", source="druks") + monkeypatch.setattr(Workflow, "start", classmethod(_WORKFLOW_START)) + ticket = _ticket( + item, + source="druks", + status=Status.READY_FOR_AGENT.label, + actor_id=member.id, + ) + + with pytest.raises(PermissionDenied): + await Build.dispatch(ticket=ticket) + await publish("ticket.transitioned", payload=ticket) + + events = list( + await druks_db.scalars( + select(Event).where(Event.type == "build.rejected", Event.subject_id == str(item.id)) + ) + ) + assert len(events) == 2 + assert not await Run.list_for_subject( + druks_db, item.subject_type, str(item.id), kind=Build.kind + ) + + async def test_dispatch_refuses_a_merged_ticket_before_the_identity_check( druks_db, monkeypatch ) -> None: diff --git a/backend/tests/test_agent_routes.py b/backend/tests/test_agent_routes.py index 1c96bebd3..2966a0deb 100644 --- a/backend/tests/test_agent_routes.py +++ b/backend/tests/test_agent_routes.py @@ -15,6 +15,7 @@ from druks.durable.models import AgentCall, Run from druks.durable.reads import read_transcript_chunk from druks.mcp.gateway import services +from druks.teams.models import Team from druks.testing import configure_app_for_test, make_settings, seed_call, seed_run from druks_field_notes.models import Note from druks_field_notes.workflows import Summarize @@ -352,6 +353,39 @@ async def test_agent_errors_share_one_shape(client: TestClient, druks_db): assert body["retryable"] is True +async def test_a_member_cannot_act_on_a_run_from_a_teams_app(client: TestClient, druks_db): + await Account.get_or_create(druks_db, "admin@example.com") + await Account.get_or_create(druks_db, "op@example.com") + await Team.create(druks_db, name="Notes", member_ids=[], apps=["field_notes"]) + note = await Note.create(body="restricted run") + run = await _park(druks_db, note) + answered = await seed_run(druks_db, kind=Summarize.kind, subject=note) + answered.input_requested_at = datetime.now(UTC) + answered.answer_parked_at = answered.input_requested_at + await druks_db.flush() + + requests = [ + client.post(f"/api/runs/{run.id}/resume", json={"control": "approve"}), + client.post(f"/api/runs/{run.id}/cancel", json={"reason": "stop"}), + client.post(f"/api/runs/{run.id}/retry"), + client.get(f"/api/gates/{run.id}"), + client.post( + f"/api/gates/{run.id}/answer", + json={"parkedAt": run.input_requested_at.isoformat(), "control": "approve"}, + ), + client.post( + f"/api/gates/{answered.id}/answer", + json={ + "parkedAt": answered.input_requested_at.isoformat(), + "control": "approve", + }, + ), + ] + + assert [response.status_code for response in requests] == [403] * 6 + assert {response.json()["code"] for response in requests} == {"APP_PERMISSION_DENIED"} + + def test_missing_agent_call_uses_the_unified_shape(client: TestClient, druks_db): response = client.get("/api/agent-calls/missing") diff --git a/backend/tests/test_apps.py b/backend/tests/test_apps.py index 4694b254d..edf8b2829 100644 --- a/backend/tests/test_apps.py +++ b/backend/tests/test_apps.py @@ -3,10 +3,13 @@ import druks.redis import pytest +from conftest import IDENTITY_HEADER, header_client +from druks.accounts.models import Account from druks.apps import App, loader from druks.apps.exceptions import AppSubjectContractError from druks.apps.loader import iter_apps, load, register_apps from druks.durable.schemas import SubjectSummary +from druks.teams.models import Team from druks.workflows import Subject from fastapi import APIRouter, FastAPI from fastapi.testclient import TestClient @@ -117,6 +120,21 @@ def discover(cls) -> list[ModuleType]: assert client.get("/health/ping").status_code == 404 +async def test_app_routes_allow_the_team_and_admin_and_refuse_other_members(tmp_path, druks_db): + admin = await Account.get_or_create(druks_db, "admin@example.com") + allowed = await Account.get_or_create(druks_db, "allowed@example.com") + refused = await Account.get_or_create(druks_db, "refused@example.com") + await Team.create(druks_db, name="Field notes", member_ids=[allowed.id], apps=["field_notes"]) + + with header_client(tmp_path) as client: + responses = [ + client.get("/api/field_notes/notes", headers={IDENTITY_HEADER: account.username}) + for account in (refused, allowed, admin) + ] + + assert [response.status_code for response in responses] == [403, 200, 200] + + def _routes_module(name: str, **routers: APIRouter) -> ModuleType: module = ModuleType(f"{name}.routes") module.__dict__.update(routers) diff --git a/backend/tests/test_dashboard.py b/backend/tests/test_dashboard.py index 03cf3dd12..136c18b99 100644 --- a/backend/tests/test_dashboard.py +++ b/backend/tests/test_dashboard.py @@ -9,6 +9,7 @@ from druks.durable.dbos_state import workflow_status from druks.durable.models import Artifact, Run from druks.events.models import Event +from druks.teams.models import Team from druks.testing import configure_app_for_test, make_settings, seed_call, seed_run from druks.user_settings.models import SettingsOverride from druks_field_notes.models import Note @@ -162,6 +163,17 @@ async def test_run_now_rejects_unknown_unscheduled_and_unavailable_workflows(cli assert client.post(f"/api/dashboard/schedules/{Summarize.kind}/run").status_code == 503 +async def test_run_now_refuses_a_member_outside_the_apps_team(client, druks_db, monkeypatch): + await Account.get_or_create(druks_db, "admin@example.com") + await Account.get_or_create(druks_db, "op@example.com") + await Team.create(druks_db, name="Notes", member_ids=[], apps=["field_notes"]) + monkeypatch.setattr(Summarize, "every", "0 9 * * *") + + response = client.post(f"/api/dashboard/schedules/{Summarize.kind}/run") + + assert response.status_code == 403 + + @pytest.mark.parametrize("has_artifact", [True, False]) async def test_artifact_title_comes_from_the_latest_call(client, druks_db, has_artifact): note = await Note.create(body="Confirm the delivery date") diff --git a/backend/tests/test_durable_sdk.py b/backend/tests/test_durable_sdk.py index 92cf759b6..7fd27e7e4 100644 --- a/backend/tests/test_durable_sdk.py +++ b/backend/tests/test_durable_sdk.py @@ -35,7 +35,7 @@ from druks.durable.enums import AgentCallStatus, WorkflowEvent from druks.durable.models import AgentCall, Artifact from druks.events.models import Event -from druks.exceptions import SessionNotBoundError +from druks.exceptions import PermissionDenied, SessionNotBoundError from druks.models import StoredSubject from druks.sandbox.datastructures import AgentResult from druks.secrets.datastructures import Audience @@ -43,6 +43,7 @@ from druks.secrets.models import VaultSecret from druks.settings import Settings, configure_settings, get_settings from druks.signals import subscribe +from druks.teams.models import Team from druks.testing import init_db from druks.user_settings.models import InstallationSettings, SettingsOverride from druks.workflows import Gate, OperatorReply, Subject, Workflow, step, task @@ -358,6 +359,7 @@ async def run_multistep(self) -> None: ParentFlow=ParentFlow, PolicyFlow=PolicyFlow, DispatchingFlow=DispatchingFlow, + DailySweep=DailySweep, record_task=record_task, retry_task=retry_task, scheduled_task=scheduled_task, @@ -1031,6 +1033,56 @@ async def test_scheduled_tick_fires_dispatch_not_run(runtime): assert run.subject_key == "W-313131" +async def test_team_access_refuses_new_and_scheduled_runs(runtime): + session = get_session(runtime.engine) + try: + account = await Account.get_default(session) + team = await Team.create(session, name="Restricted", member_ids=[], apps=["field_notes"]) + await session.commit() + existing_runs = await session.scalar( + select(func.count()) + .select_from(Run) + .where(Run.kind.in_((runtime.SubjectFlow.kind, runtime.DailySweep.kind))) + ) + finally: + await session.close() + + previous_subject_app = runtime.SubjectFlow.app + previous_schedule_app = runtime.DailySweep.app + runtime.SubjectFlow.app = "field_notes" + runtime.DailySweep.app = "field_notes" + scheduled_runs = SINK.count("swept") + try: + with pytest.raises(PermissionDenied): + await runtime.SubjectFlow.start(subject=Widget(id=7), account_id=account.id) + + _, entry = next(row for row in _workflow_schedules if row[0].kind == "daily_sweep") + with pytest.raises(PermissionDenied): + await entry(datetime.now(UTC), None) + + assert SINK.count("swept") == scheduled_runs + session = get_session(runtime.engine) + try: + run_count = await session.scalar( + select(func.count()) + .select_from(Run) + .where(Run.kind.in_((runtime.SubjectFlow.kind, runtime.DailySweep.kind))) + ) + finally: + await session.close() + assert run_count == existing_runs + finally: + runtime.SubjectFlow.app = previous_subject_app + runtime.DailySweep.app = previous_schedule_app + session = get_session(runtime.engine) + try: + stored = await Team.get(session, id=team.id) + await stored.delete() + await session.commit() + finally: + await session.close() + + async def test_scheduled_dispatch_must_be_nullary(runtime): with pytest.raises(WorkflowError, match="nullary"): diff --git a/backend/tests/test_mcp_endpoint.py b/backend/tests/test_mcp_endpoint.py index 08a90b540..35946becb 100644 --- a/backend/tests/test_mcp_endpoint.py +++ b/backend/tests/test_mcp_endpoint.py @@ -32,6 +32,7 @@ from druks.mcp.server import create_mcp_app from druks.secrets.enums import SecretKind from druks.secrets.models import VaultSecret +from druks.teams.models import Team from druks.testing import asgi_client, configure_app_for_test, make_settings from druks.usage.models import UsageScrape from druks_field_notes.models import Note @@ -304,6 +305,27 @@ async def test_a_ticket_token_reaches_only_its_tools(druks_db, app, account): assert refused.is_error +async def test_a_member_cannot_list_or_call_tools_from_a_teams_app(druks_db, app): + await Account.get_or_create(druks_db, "admin@example.com") + member = await Account.get_or_create(druks_db, "member@example.com") + await Team.create(druks_db, name="Notes", member_ids=[], apps=["field_notes"]) + _, token = await PersonalAccessToken.create( + druks_db, + account_id=member.id, + name="notes", + allowed_tools=["field_notes_jot_note"], + ) + + async with live(app), _client(app, token) as client: + names = {tool.name for tool in await client.list_tools()} + refused = await client.call_tool( + "field_notes_jot_note", {"body": "Hidden note"}, raise_on_error=False + ) + + assert "field_notes_jot_note" not in names + assert refused.is_error + + async def test_board_ticket_tools_follow_the_tracker_setting(app, pat_token): project = await Project.create(name="Acme") repo = await ProjectRepo.create(project_id=project.id, full_name="acme/widget") diff --git a/backend/tests/test_teams.py b/backend/tests/test_teams.py index abac47c81..b4fc0f8b2 100644 --- a/backend/tests/test_teams.py +++ b/backend/tests/test_teams.py @@ -4,6 +4,30 @@ from conftest import IDENTITY_HEADER, header_client from druks.accounts.enums import AccountKind from druks.accounts.models import Account +from druks.teams.models import Team + + +async def test_team_allows_admins_members_open_apps_bots_and_built_in_apps(druks_db): + admin = await Account.get_or_create(druks_db, "admin@example.com") + first = await Account.get_or_create(druks_db, "first@example.com") + second = await Account.get_or_create(druks_db, "second@example.com") + outsider = await Account.get_or_create(druks_db, "outsider@example.com") + bot = await Account.create_for_bot(druks_db, AccountKind.BOT) + await Team.create(druks_db, name="First", member_ids=[first.id], apps=["field_notes"]) + await Team.create(druks_db, name="Second", member_ids=[second.id], apps=["field_notes"]) + druks_db.add(Team(name="Stored mistake", members=[], apps=["core"])) + await druks_db.flush() + + assert await Team.allows(druks_db, admin, "field_notes") + assert await Team.allows(druks_db, outsider, "software_factory") + assert await Team.allows(druks_db, first, "field_notes") + assert await Team.allows(druks_db, second, "field_notes") + assert not await Team.allows(druks_db, outsider, "field_notes") + assert await Team.allows(druks_db, outsider, "core") + assert await Team.allows(druks_db, bot, "field_notes") + assert not await Team.allows(druks_db, None, "field_notes") + assert await Team.allows(druks_db, None, "software_factory") + assert await Team.allows(druks_db, None, "core") async def test_admin_creates_lists_and_replaces_a_team(tmp_path: Path, druks_db): diff --git a/backend/tests/test_whatsapp.py b/backend/tests/test_whatsapp.py index a5069c36a..1622e259e 100644 --- a/backend/tests/test_whatsapp.py +++ b/backend/tests/test_whatsapp.py @@ -57,6 +57,7 @@ from druks.secrets.enums import SecretKind from druks.secrets.models import VaultSecret from druks.settings import Urls +from druks.teams.models import Team from druks.testing import asgi_client, configure_app_for_test, make_settings, seed_run from druks.user_settings import reads from druks.user_settings.models import InstallationSettings, SettingsOverride @@ -780,18 +781,18 @@ async def test_a_bot_tool_serves_only_the_callers_own_conversation(druks_db, tmp assert own.json() == {"user": ANA, "ticket": "T-1"} -def test_a_bot_tool_is_never_in_the_operator_toolkit(): - def visible(allowed_tools, name, tags): +async def test_a_bot_tool_is_never_in_the_operator_toolkit(): + async def visible(allowed_tools, name, tags): token = SimpleNamespace(claims={"allowed_tools": allowed_tools}) - return _is_visible( - SimpleNamespace(token=token, component=SimpleNamespace(name=name, tags=tags)) + return await _is_visible( + set(), SimpleNamespace(token=token, component=SimpleNamespace(name=name, tags=tags)) ) - assert not visible(None, "helpdesk_get_ticket", {"bot", "helpdesk"}) - assert visible(None, "get_usage", {"agent"}) - assert visible(["helpdesk_get_ticket"], "helpdesk_get_ticket", {"bot", "helpdesk"}) - assert not visible(["helpdesk_get_ticket"], "get_usage", {"agent"}) - assert not visible([], "get_usage", {"agent"}) + assert not await visible(None, "helpdesk_get_ticket", {"bot", "helpdesk"}) + assert await visible(None, "get_usage", {"agent"}) + assert await visible(["helpdesk_get_ticket"], "helpdesk_get_ticket", {"bot", "helpdesk"}) + assert not await visible(["helpdesk_get_ticket"], "get_usage", {"agent"}) + assert not await visible([], "get_usage", {"agent"}) def test_a_bot_under_another_name_or_an_unknown_tool_stops_boot(helpdesk): @@ -877,6 +878,29 @@ async def test_member_cannot_open_a_public_number_code(druks_db, tmp_path, helpd assert response.status_code == 403 +@pytest.mark.parametrize("helpdesk", [BotAccess.PRIVATE], indirect=True) +async def test_member_cannot_pair_a_private_number_from_a_teams_app(druks_db, tmp_path, helpdesk): + await Account.get_or_create(druks_db, "admin@example.com") + member = await Account.get_or_create(druks_db, "member@example.com") + connection = await link( + druks_db, + await bot_account(druks_db), + app="helpdesk", + access=BotAccess.PRIVATE, + ) + await Team.create(druks_db, name="Helpdesk", member_ids=[], apps=["helpdesk"]) + 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}/pairing", + 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) diff --git a/docs/chat.md b/docs/chat.md index 83219a36e..5b85c28a0 100644 --- a/docs/chat.md +++ b/docs/chat.md @@ -584,9 +584,9 @@ that are not linked yet. It also skips a number that a TwiML App or a SIP trunk handles, because Twilio ignores the voice URL of such a number. A phone number has no admin, and no agent turn runs on a call. When a call -starts a run that asks a question, any operator can answer it in the app that -owns the run. The Dashboard does not show these runs, because they belong to -the number's bot account. +starts a run that asks a question, an account that can use the app can answer +it. The Dashboard does not show these runs, because they belong to the number's +bot account. **Remove** clears the number's voice URL in Twilio, so Druks stops answering its calls. The number and its calls stay as history. If you link the number diff --git a/docs/concepts.md b/docs/concepts.md index 9f71a4f35..fe0f2a032 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -316,6 +316,11 @@ A team is a named group of people with a list of apps. A person can be on more than one team, and more than one team can list the same app. An admin manages teams in **Settings → Teams**. +An admin can use every app, and every person can use an app that no team lists. +A built-in app is always open, also when a team lists it by mistake. A person +can use a listed app when a team that lists it has that person. Druks refuses +all other people and starts a run only under an account that can use the app. + In `none` mode, Druks has no authentication and exactly one operator account. The first completed provider connection creates this account. Public `/_external` routes stay outside the identity gate. These routes include diff --git a/docs/configuration.md b/docs/configuration.md index 05f5ef2ff..a546ad184 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -213,6 +213,8 @@ The first account becomes the default account, including in header and JWT modes Unattended calls use its subscriptions. The flag grants no extra permissions. Calls with an explicit account use that account's subscriptions. Missing subscriptions fail the call. API keys belong to the installation. +The unattended account must be able to use each app that runs on a schedule. +If it is a member, an admin adds it to a team that lists each scheduled app. 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 @@ -643,6 +645,10 @@ does not match. A GitHub person has the account that their [GitHub sign-in](#github) links. Two matching accounts do not match. A **druks** ticket build uses the account of the ticket assignee. +When a team lists Software Factory, a ticket move by a person who cannot use it, +or who has no Druks account, opens no build. Druks does not use the assignee or +the default account instead. + ### GitHub Issues as the tracker GitHub has no status field, so the GitHub tracker reads the five status settings