From 7f31d5ad0387b91e642fea1ab7d1d668d5809f04 Mon Sep 17 00:00:00 2001 From: Oleg Miagkov Date: Thu, 25 Jun 2026 13:12:56 +0400 Subject: [PATCH 1/7] feat(cloud): workspace bootstrap, resolution & management API (C2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second slice of Track C — the workspace-context layer that makes the C1 models usable. - CLOUD_MODE config flag (single|team, validated; default single). - services/workspace_service.py: get_or_create_personal_workspace (idempotent single-mode default), list_accessible_workspaces (owned ∪ member), create_workspace. - core/permissions.py: get_current_workspace dependency — single mode resolves the caller's auto-created Personal workspace; team mode requires ?workspace_id and enforces >= viewer access. Extracted a shared _current_user_id helper (reused by require_workspace_access). - api/routes/workspaces.py: GET /api/workspaces (list with role), GET /api/workspaces/current, POST /api/workspaces; registered in main.py. - Personal-workspace bootstrap hooked into register + login (idempotent; backfills pre-C2 accounts). - Tests: bootstrap idempotency, owned∪member listing, single/team resolution (+400/403), and an end-to-end HTTP test of the router. Restores the owner-access test's membership-absence assertion dropped by the C1 squash. 11 workspace tests pass; existing auth/user route tests (49) unaffected; ruff clean. No schema change: per-resource scoping (repos/runs) lands as each gains DB persistence (C3/C6); today only 'repositories' is a DB model and has no CRUD routes yet. Co-Authored-By: Claude Opus 4.8 --- apps/web-backend/api/models/workspace.py | 30 ++++ apps/web-backend/api/routes/users.py | 7 + apps/web-backend/api/routes/workspaces.py | 91 +++++++++++ apps/web-backend/core/config.py | 8 + apps/web-backend/core/permissions.py | 51 +++++- apps/web-backend/main.py | 13 +- .../web-backend/services/workspace_service.py | 82 ++++++++++ apps/web-backend/tests/test_workspaces.py | 148 ++++++++++++++++++ 8 files changed, 427 insertions(+), 3 deletions(-) create mode 100644 apps/web-backend/api/routes/workspaces.py create mode 100644 apps/web-backend/services/workspace_service.py diff --git a/apps/web-backend/api/models/workspace.py b/apps/web-backend/api/models/workspace.py index b3f63ecb1..9758a1830 100644 --- a/apps/web-backend/api/models/workspace.py +++ b/apps/web-backend/api/models/workspace.py @@ -10,6 +10,7 @@ from datetime import UTC, datetime from core.database import Base +from pydantic import BaseModel, ConfigDict, Field from sqlalchemy import ( CheckConstraint, Column, @@ -89,3 +90,32 @@ def __repr__(self) -> str: f"" ) + + +# Pydantic models for API requests and responses + + +class WorkspaceCreateRequest(BaseModel): + """Request model for creating a workspace (team mode).""" + + name: str = Field(..., min_length=1, max_length=255, description="Workspace name") + + +class WorkspaceResponse(BaseModel): + """Response model for a workspace, including the caller's role in it.""" + + id: int = Field(..., description="Workspace ID") + name: str = Field(..., description="Workspace name") + role: str = Field(..., description="Caller's role: owner/editor/viewer") + created_at: datetime = Field(..., description="Creation timestamp") + + model_config = ConfigDict(from_attributes=True) + + +class WorkspaceListResponse(BaseModel): + """Response model for the list of workspaces the caller can access.""" + + workspaces: list[WorkspaceResponse] = Field( + default_factory=list, description="Accessible workspaces" + ) + cloud_mode: str = Field(..., description="Deployment mode: single or team") diff --git a/apps/web-backend/api/routes/users.py b/apps/web-backend/api/routes/users.py index 6af93dbd4..325caca1c 100644 --- a/apps/web-backend/api/routes/users.py +++ b/apps/web-backend/api/routes/users.py @@ -9,6 +9,7 @@ from core.database import get_db from core.security import create_access_token from fastapi import APIRouter, Depends, HTTPException, status +from services.workspace_service import get_or_create_personal_workspace from sqlalchemy.orm import Session from api.models.user import ( @@ -105,6 +106,9 @@ async def register_user( logger.info("New user registered: id=%s", user.id) + # Bootstrap the user's Personal workspace (single-mode default). + get_or_create_personal_workspace(db, user.id) + return _build_token_response(user) @@ -157,4 +161,7 @@ async def login_user( logger.info("User logged in: %s", user.email) + # Ensure a Personal workspace exists (idempotent; backfills pre-C2 accounts). + get_or_create_personal_workspace(db, user.id) + return _build_token_response(user) diff --git a/apps/web-backend/api/routes/workspaces.py b/apps/web-backend/api/routes/workspaces.py new file mode 100644 index 000000000..d927eefed --- /dev/null +++ b/apps/web-backend/api/routes/workspaces.py @@ -0,0 +1,91 @@ +""" +Workspace management API (Track C — cloud multitenancy). + +Lets the frontend list the workspaces a user can access, resolve the current one +(the workspace switcher), and create workspaces in team mode. Access is enforced +via core/permissions.py; the single-mode "Personal" workspace is auto-created. +""" + +import logging + +from core.config import settings +from core.database import get_db +from core.permissions import ( + WorkspaceRole, + get_current_workspace, + user_role_in_workspace, +) +from core.security import require_auth +from fastapi import APIRouter, Depends, HTTPException, status +from services.workspace_service import create_workspace, list_accessible_workspaces +from sqlalchemy.orm import Session + +from api.models.workspace import ( + Workspace, + WorkspaceCreateRequest, + WorkspaceListResponse, + WorkspaceResponse, +) + +logger = logging.getLogger(__name__) + +router = APIRouter(prefix="/api/workspaces", tags=["workspaces"]) + + +def _require_user_id(auth: dict) -> int: + """Return the integer user id from the token claims, or raise 403.""" + sub = auth.get("sub") + if sub is None or not str(sub).isdigit(): + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="No authenticated user in token", + ) + return int(sub) + + +def _to_response(workspace: Workspace, role: WorkspaceRole) -> WorkspaceResponse: + """Build a WorkspaceResponse including the caller's role string.""" + return WorkspaceResponse( + id=workspace.id, + name=workspace.name, + role=role.value, + created_at=workspace.created_at, + ) + + +@router.get("", response_model=WorkspaceListResponse) +async def list_workspaces( + auth: dict = Depends(require_auth), + db: Session = Depends(get_db), +) -> WorkspaceListResponse: + """List every workspace the caller can access, with their role in each.""" + user_id = _require_user_id(auth) + items: list[WorkspaceResponse] = [] + for ws in list_accessible_workspaces(db, user_id): + role = user_role_in_workspace(db, user_id, ws.id) or WorkspaceRole.VIEWER + items.append(_to_response(ws, role)) + return WorkspaceListResponse(workspaces=items, cloud_mode=settings.CLOUD_MODE) + + +@router.get("/current", response_model=WorkspaceResponse) +async def current_workspace( + workspace: Workspace = Depends(get_current_workspace), + auth: dict = Depends(require_auth), + db: Session = Depends(get_db), +) -> WorkspaceResponse: + """Resolve the current workspace (Personal in single mode; ?workspace_id in team).""" + user_id = _require_user_id(auth) + role = user_role_in_workspace(db, user_id, workspace.id) or WorkspaceRole.VIEWER + return _to_response(workspace, role) + + +@router.post("", response_model=WorkspaceResponse, status_code=status.HTTP_201_CREATED) +async def create_workspace_endpoint( + request: WorkspaceCreateRequest, + auth: dict = Depends(require_auth), + db: Session = Depends(get_db), +) -> WorkspaceResponse: + """Create a workspace owned by the caller (used in team mode).""" + user_id = _require_user_id(auth) + workspace = create_workspace(db, owner_id=user_id, name=request.name) + return _to_response(workspace, WorkspaceRole.OWNER) diff --git a/apps/web-backend/core/config.py b/apps/web-backend/core/config.py index 5eb5f66bd..df84a1730 100644 --- a/apps/web-backend/core/config.py +++ b/apps/web-backend/core/config.py @@ -29,6 +29,10 @@ def __init__(self): self.PORT: int = int(os.getenv("PORT", "8000")) self.DEBUG: bool = os.getenv("DEBUG", "false").lower() == "true" + # Cloud deployment mode: "single" (one auto-created "Personal" workspace + # per user) or "team" (many workspaces, explicit workspace_id per request). + self.CLOUD_MODE: str = os.getenv("CLOUD_MODE", "single").strip().lower() + # CORS configuration cors_origins = os.getenv("CORS_ORIGINS", "") self.CORS_ORIGINS: list[str] = [ @@ -92,6 +96,10 @@ def _validate(self): "SECRET_KEY must be set to a secure value in production. " "Set DEBUG=false only when SECRET_KEY is properly configured." ) + if self.CLOUD_MODE not in ("single", "team"): + raise ValueError( + f"CLOUD_MODE must be 'single' or 'team', got {self.CLOUD_MODE!r}." + ) @lru_cache diff --git a/apps/web-backend/core/permissions.py b/apps/web-backend/core/permissions.py index de1e24b4b..6f07f63ea 100644 --- a/apps/web-backend/core/permissions.py +++ b/apps/web-backend/core/permissions.py @@ -11,8 +11,10 @@ from api.models.workspace import Workspace, WorkspaceUser from fastapi import Depends, HTTPException, status +from services.workspace_service import get_or_create_personal_workspace from sqlalchemy.orm import Session +from core.config import settings from core.database import get_db from core.security import require_auth @@ -80,6 +82,12 @@ def check_workspace_access( return role is not None and role_satisfies(role, required_role) +def _current_user_id(auth: dict) -> int | None: + """Extract the integer user id from JWT claims (``sub`` is ``str(user.id)``).""" + sub = auth.get("sub") + return int(sub) if sub is not None and str(sub).isdigit() else None + + def require_workspace_access(required_role: WorkspaceRole = WorkspaceRole.VIEWER): """FastAPI dependency factory. @@ -93,8 +101,7 @@ def dependency( auth: dict = Depends(require_auth), db: Session = Depends(get_db), ) -> WorkspaceRole: - sub = auth.get("sub") - user_id = int(sub) if sub is not None and str(sub).isdigit() else None + user_id = _current_user_id(auth) role = ( user_role_in_workspace(db, user_id, workspace_id) if user_id is not None @@ -108,3 +115,43 @@ def dependency( return role return dependency + + +def get_current_workspace( + workspace_id: int | None = None, + auth: dict = Depends(require_auth), + db: Session = Depends(get_db), +) -> Workspace: + """Resolve the workspace for the current request. + + - **single mode** (``CLOUD_MODE=single``): the caller's auto-created + "Personal" workspace (``workspace_id`` is ignored). Created on first use. + - **team mode**: the workspace named by the ``workspace_id`` query parameter, + which the caller must be able to access (>= viewer) — else 400/403. + + Returns the resolved ``Workspace`` ORM object. + """ + user_id = _current_user_id(auth) + if user_id is None: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="No authenticated user in token", + ) + + if settings.CLOUD_MODE == "single": + return get_or_create_personal_workspace(db, user_id) + + if workspace_id is None: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="workspace_id is required in team mode", + ) + workspace = db.query(Workspace).filter(Workspace.id == workspace_id).first() + if workspace is None or not check_workspace_access( + db, user_id, workspace_id, WorkspaceRole.VIEWER + ): + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Insufficient workspace permissions", + ) + return workspace diff --git a/apps/web-backend/main.py b/apps/web-backend/main.py index e6c4b6efd..5cd7e61f4 100644 --- a/apps/web-backend/main.py +++ b/apps/web-backend/main.py @@ -92,7 +92,17 @@ async def lifespan(app: FastAPI): ) # Import and register API routes -from api.routes import agents, auth, files, git, specs, tasks, usage, users +from api.routes import ( + agents, + auth, + files, + git, + specs, + tasks, + usage, + users, + workspaces, +) from api.websocket import router as websocket_router app.include_router(agents.router) @@ -103,6 +113,7 @@ async def lifespan(app: FastAPI): app.include_router(tasks.router) app.include_router(usage.router) app.include_router(users.router) +app.include_router(workspaces.router) app.include_router(websocket_router) # Only expose test routes in debug/development mode diff --git a/apps/web-backend/services/workspace_service.py b/apps/web-backend/services/workspace_service.py new file mode 100644 index 000000000..4375341ff --- /dev/null +++ b/apps/web-backend/services/workspace_service.py @@ -0,0 +1,82 @@ +""" +Workspace bootstrap and listing helpers (Track C — cloud multitenancy). + +In single-user mode every user gets one auto-created "Personal" workspace that +owns their specs/runs/repos; in team mode users own and join many. These are the +deterministic, unit-testable helpers used by the workspace routes and the +``get_current_workspace`` dependency in ``core/permissions.py``. + +Owner access derives from ``Workspace.owner_id`` (see core/permissions.py), so a +Personal workspace needs no membership row — listing unions owned workspaces with +``WorkspaceUser`` memberships rather than relying on a redundant owner row. +""" + +import logging + +from api.models.workspace import Workspace, WorkspaceUser +from sqlalchemy import or_ +from sqlalchemy.orm import Session + +logger = logging.getLogger(__name__) + +# Name of the auto-created single-user workspace. +PERSONAL_WORKSPACE_NAME = "Personal" + + +def get_or_create_personal_workspace(db: Session, user_id: int) -> Workspace: + """Return the user's Personal workspace, creating it if absent (idempotent). + + This is the single-mode default workspace. If the user already owns one named + ``PERSONAL_WORKSPACE_NAME`` the oldest such workspace is reused, so repeated + calls (e.g. on every login) never create duplicates. + """ + existing = ( + db.query(Workspace) + .filter( + Workspace.owner_id == user_id, + Workspace.name == PERSONAL_WORKSPACE_NAME, + ) + .order_by(Workspace.id.asc()) + .first() + ) + if existing is not None: + return existing + + workspace = Workspace(name=PERSONAL_WORKSPACE_NAME, owner_id=user_id) + db.add(workspace) + db.commit() + db.refresh(workspace) + logger.info( + "Bootstrapped Personal workspace id=%s for user_id=%s", workspace.id, user_id + ) + return workspace + + +def list_accessible_workspaces(db: Session, user_id: int) -> list[Workspace]: + """Workspaces the user can access: those they own plus those they're a member of.""" + member_ids = db.query(WorkspaceUser.workspace_id).filter( + WorkspaceUser.user_id == user_id + ) + return ( + db.query(Workspace) + .filter( + or_( + Workspace.owner_id == user_id, + Workspace.id.in_(member_ids), + ) + ) + .order_by(Workspace.id.asc()) + .all() + ) + + +def create_workspace(db: Session, owner_id: int, name: str) -> Workspace: + """Create a workspace owned by ``owner_id`` (team mode). Owner access is implicit.""" + workspace = Workspace(name=name, owner_id=owner_id) + db.add(workspace) + db.commit() + db.refresh(workspace) + logger.info( + "Created workspace id=%s name=%r owner_id=%s", workspace.id, name, owner_id + ) + return workspace diff --git a/apps/web-backend/tests/test_workspaces.py b/apps/web-backend/tests/test_workspaces.py index 2c719ea89..1b6417809 100644 --- a/apps/web-backend/tests/test_workspaces.py +++ b/apps/web-backend/tests/test_workspaces.py @@ -8,12 +8,20 @@ import pytest from api.models.user import User from api.models.workspace import Workspace, WorkspaceUser +from core.config import settings from core.permissions import ( WorkspaceRole, check_workspace_access, + get_current_workspace, role_satisfies, user_role_in_workspace, ) +from fastapi import HTTPException +from services.workspace_service import ( + create_workspace, + get_or_create_personal_workspace, + list_accessible_workspaces, +) from sqlalchemy.exc import IntegrityError @@ -115,6 +123,17 @@ def test_workspace_owner_has_owner_access_without_membership(test_db): test_db.commit() test_db.refresh(workspace) + # Precondition for this test's meaning: the owner has NO membership row, so + # access must come from Workspace.owner_id (not an auto-created membership). + assert ( + test_db.query(WorkspaceUser) + .filter( + WorkspaceUser.workspace_id == workspace.id, + WorkspaceUser.user_id == owner.id, + ) + .first() + is None + ) assert user_role_in_workspace(test_db, owner.id, workspace.id) == WorkspaceRole.OWNER assert check_workspace_access(test_db, owner.id, workspace.id, WorkspaceRole.OWNER) @@ -134,3 +153,132 @@ def test_invalid_role_is_rejected_by_db(test_db): with pytest.raises(IntegrityError): test_db.commit() test_db.rollback() + + +# --------------------------------------------------------------------------- +# C2 — bootstrap, listing, and the get_current_workspace resolver +# --------------------------------------------------------------------------- + + +def test_get_or_create_personal_workspace_is_idempotent(test_db): + user = _make_user(test_db, "personal@test.com") + + first = get_or_create_personal_workspace(test_db, user.id) + second = get_or_create_personal_workspace(test_db, user.id) + + assert first.id == second.id + assert first.name == "Personal" + assert first.owner_id == user.id + # No duplicate Personal workspace created on the second call. + owned = test_db.query(Workspace).filter(Workspace.owner_id == user.id).all() + assert len(owned) == 1 + + +def test_list_accessible_workspaces_owned_and_member(test_db): + user = _make_user(test_db, "lister@test.com") + owned = create_workspace(test_db, owner_id=user.id, name="Mine") + + other = _make_user(test_db, "lister-other@test.com") + member_ws = create_workspace(test_db, owner_id=other.id, name="Theirs") + test_db.add( + WorkspaceUser(workspace_id=member_ws.id, user_id=user.id, role="viewer") + ) + test_db.commit() + + # A workspace the user neither owns nor belongs to must not appear. + create_workspace(test_db, owner_id=other.id, name="Hidden") + + accessible = {w.id for w in list_accessible_workspaces(test_db, user.id)} + assert accessible == {owned.id, member_ws.id} + + +def test_get_current_workspace_single_mode(test_db, monkeypatch): + monkeypatch.setattr(settings, "CLOUD_MODE", "single") + user = _make_user(test_db, "single@test.com") + + # Resolves (and creates) the Personal workspace; workspace_id is ignored. + ws = get_current_workspace( + workspace_id=None, auth={"sub": str(user.id)}, db=test_db + ) + assert ws.name == "Personal" + assert ws.owner_id == user.id + + # Idempotent: a second resolve returns the same workspace. + again = get_current_workspace( + workspace_id=999, auth={"sub": str(user.id)}, db=test_db + ) + assert again.id == ws.id + + +def test_get_current_workspace_team_mode(test_db, monkeypatch): + monkeypatch.setattr(settings, "CLOUD_MODE", "team") + owner = _make_user(test_db, "team-owner@test.com") + workspace = create_workspace(test_db, owner_id=owner.id, name="Team") + + # The owner can resolve their workspace by id. + got = get_current_workspace( + workspace_id=workspace.id, auth={"sub": str(owner.id)}, db=test_db + ) + assert got.id == workspace.id + + # team mode requires workspace_id -> 400 when omitted. + with pytest.raises(HTTPException) as missing: + get_current_workspace( + workspace_id=None, auth={"sub": str(owner.id)}, db=test_db + ) + assert missing.value.status_code == 400 + + # A non-member is denied -> 403. + outsider = _make_user(test_db, "team-outsider@test.com") + with pytest.raises(HTTPException) as denied: + get_current_workspace( + workspace_id=workspace.id, auth={"sub": str(outsider.id)}, db=test_db + ) + assert denied.value.status_code == 403 + + +def test_workspaces_api_endpoints(test_db, monkeypatch): + """End-to-end HTTP test of the workspaces router (single mode).""" + monkeypatch.setattr(settings, "CLOUD_MODE", "single") + from core.database import get_db + from core.security import require_auth + from fastapi.testclient import TestClient + from main import app + + user = _make_user(test_db, "apiuser@test.com") + + def _override_db(): + yield test_db + + def _override_auth(): + return {"sub": str(user.id), "email": user.email} + + app.dependency_overrides[get_db] = _override_db + app.dependency_overrides[require_auth] = _override_auth + try: + client = TestClient(app) + + # No workspaces owned yet. + resp = client.get("/api/workspaces") + assert resp.status_code == 200 + assert resp.json()["workspaces"] == [] + assert resp.json()["cloud_mode"] == "single" + + # Resolving the current workspace bootstraps "Personal" (owner role). + resp = client.get("/api/workspaces/current") + assert resp.status_code == 200 + assert resp.json()["name"] == "Personal" + assert resp.json()["role"] == "owner" + + # Creating a workspace returns it with owner role. + resp = client.post("/api/workspaces", json={"name": "Team X"}) + assert resp.status_code == 201 + assert resp.json()["name"] == "Team X" + assert resp.json()["role"] == "owner" + + # The list now contains both workspaces. + resp = client.get("/api/workspaces") + names = {w["name"] for w in resp.json()["workspaces"]} + assert names == {"Personal", "Team X"} + finally: + app.dependency_overrides.clear() From 8b8666c0108dd04e78133e11e78442dc561db965 Mon Sep 17 00:00:00 2001 From: Oleg Miagkov Date: Thu, 25 Jun 2026 13:25:41 +0400 Subject: [PATCH 2/7] =?UTF-8?q?fix(cloud):=20address=20C2=20review=20?= =?UTF-8?q?=E2=80=94=20log-injection,=20N+1,=20redundant=20lookups=20(C2?= =?UTF-8?q?=20review)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - workspace_service.create_workspace: stop logging the user-supplied name (CodeQL log-injection); log id + owner_id only. - workspaces.list_workspaces: replace the per-row user_role_in_workspace call (N+1) with a single membership query; owners derived from owner_id. - permissions.get_current_workspace (team mode): check access first, fetch the workspace row only on success (no wasted lookup on denial). - workspaces routes: drop redundant return-type annotations (keep response_model= in the decorator, matching the rest of the codebase) to clear the SonarCloud duplication hint. Skipped (with reason): Annotated[...] deps (whole codebase uses = Depends; trivial/stylistic) and a unique-constraint for concurrent Personal-workspace creation (needs a migration, out of C2's no-schema scope; low-probability + reads self-heal). 11 tests pass; ruff clean. Co-Authored-By: Claude Opus 4.8 --- apps/web-backend/api/routes/workspaces.py | 22 ++++++++++++++----- apps/web-backend/core/permissions.py | 10 ++++++--- .../web-backend/services/workspace_service.py | 5 ++--- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/apps/web-backend/api/routes/workspaces.py b/apps/web-backend/api/routes/workspaces.py index d927eefed..15c8dcdb3 100644 --- a/apps/web-backend/api/routes/workspaces.py +++ b/apps/web-backend/api/routes/workspaces.py @@ -25,6 +25,7 @@ WorkspaceCreateRequest, WorkspaceListResponse, WorkspaceResponse, + WorkspaceUser, ) logger = logging.getLogger(__name__) @@ -57,12 +58,23 @@ def _to_response(workspace: Workspace, role: WorkspaceRole) -> WorkspaceResponse async def list_workspaces( auth: dict = Depends(require_auth), db: Session = Depends(get_db), -) -> WorkspaceListResponse: +): """List every workspace the caller can access, with their role in each.""" user_id = _require_user_id(auth) + workspaces = list_accessible_workspaces(db, user_id) + # Resolve roles with a single membership query (avoids an N+1 over + # user_role_in_workspace): owners are detected from owner_id directly. + member_roles = { + wu.workspace_id: wu.role + for wu in db.query(WorkspaceUser).filter(WorkspaceUser.user_id == user_id) + } items: list[WorkspaceResponse] = [] - for ws in list_accessible_workspaces(db, user_id): - role = user_role_in_workspace(db, user_id, ws.id) or WorkspaceRole.VIEWER + for ws in workspaces: + role = ( + WorkspaceRole.OWNER + if ws.owner_id == user_id + else WorkspaceRole(member_roles.get(ws.id, WorkspaceRole.VIEWER.value)) + ) items.append(_to_response(ws, role)) return WorkspaceListResponse(workspaces=items, cloud_mode=settings.CLOUD_MODE) @@ -72,7 +84,7 @@ async def current_workspace( workspace: Workspace = Depends(get_current_workspace), auth: dict = Depends(require_auth), db: Session = Depends(get_db), -) -> WorkspaceResponse: +): """Resolve the current workspace (Personal in single mode; ?workspace_id in team).""" user_id = _require_user_id(auth) role = user_role_in_workspace(db, user_id, workspace.id) or WorkspaceRole.VIEWER @@ -84,7 +96,7 @@ async def create_workspace_endpoint( request: WorkspaceCreateRequest, auth: dict = Depends(require_auth), db: Session = Depends(get_db), -) -> WorkspaceResponse: +): """Create a workspace owned by the caller (used in team mode).""" user_id = _require_user_id(auth) workspace = create_workspace(db, owner_id=user_id, name=request.name) diff --git a/apps/web-backend/core/permissions.py b/apps/web-backend/core/permissions.py index 6f07f63ea..9a79534e5 100644 --- a/apps/web-backend/core/permissions.py +++ b/apps/web-backend/core/permissions.py @@ -146,10 +146,14 @@ def get_current_workspace( status_code=status.HTTP_400_BAD_REQUEST, detail="workspace_id is required in team mode", ) + # Check access first; only fetch the workspace row once access is granted. + if not check_workspace_access(db, user_id, workspace_id, WorkspaceRole.VIEWER): + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Insufficient workspace permissions", + ) workspace = db.query(Workspace).filter(Workspace.id == workspace_id).first() - if workspace is None or not check_workspace_access( - db, user_id, workspace_id, WorkspaceRole.VIEWER - ): + if workspace is None: # pragma: no cover - access check already proved existence raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail="Insufficient workspace permissions", diff --git a/apps/web-backend/services/workspace_service.py b/apps/web-backend/services/workspace_service.py index 4375341ff..c39f50bf1 100644 --- a/apps/web-backend/services/workspace_service.py +++ b/apps/web-backend/services/workspace_service.py @@ -76,7 +76,6 @@ def create_workspace(db: Session, owner_id: int, name: str) -> Workspace: db.add(workspace) db.commit() db.refresh(workspace) - logger.info( - "Created workspace id=%s name=%r owner_id=%s", workspace.id, name, owner_id - ) + # Do not log the user-supplied name (log-injection); id + owner suffice. + logger.info("Created workspace id=%s owner_id=%s", workspace.id, owner_id) return workspace From 9c03dfe33982e2385a0293cf9128f94abdfb0357 Mon Sep 17 00:00:00 2001 From: Oleg Miagkov Date: Thu, 25 Jun 2026 13:44:54 +0400 Subject: [PATCH 3/7] feat(cloud): workspace member management API (C2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rounds out the workspace API with team-member management — and the first real consumer of C1's require_workspace_access dependency (listing needs >= viewer, mutations need owner). - Endpoints (api/routes/workspaces.py): GET /api/workspaces/{id}/members (owner first), POST (add by user_id+role), PATCH /{user_id} (change role), DELETE /{user_id} (remove). Validates: unknown workspace/user -> 404, adding the owner -> 400, duplicate member -> 409. - Service (services/workspace_service.py): list_workspace_members, get_membership, add_member, update_member_role, remove_member. - Models: WorkspaceMemberAddRequest/UpdateRequest (role validated via Literal) + member response schemas. - Tests: service CRUD + an end-to-end HTTP flow (add/list/dup-409/owner-400/update/non-owner-403/viewer-can-list/remove-204). 13 workspace tests pass; ruff clean. Co-Authored-By: Claude Opus 4.8 --- apps/web-backend/api/models/workspace.py | 32 +++++ apps/web-backend/api/routes/workspaces.py | 119 +++++++++++++++++- .../web-backend/services/workspace_service.py | 59 +++++++++ apps/web-backend/tests/test_workspaces.py | 102 +++++++++++++++ 4 files changed, 311 insertions(+), 1 deletion(-) diff --git a/apps/web-backend/api/models/workspace.py b/apps/web-backend/api/models/workspace.py index 9758a1830..c009b17b5 100644 --- a/apps/web-backend/api/models/workspace.py +++ b/apps/web-backend/api/models/workspace.py @@ -8,6 +8,7 @@ """ from datetime import UTC, datetime +from typing import Literal from core.database import Base from pydantic import BaseModel, ConfigDict, Field @@ -119,3 +120,34 @@ class WorkspaceListResponse(BaseModel): default_factory=list, description="Accessible workspaces" ) cloud_mode: str = Field(..., description="Deployment mode: single or team") + + +class WorkspaceMemberAddRequest(BaseModel): + """Request model for adding a member to a workspace.""" + + user_id: int = Field(..., description="Id of the user to add") + role: Literal["owner", "editor", "viewer"] = Field( + "viewer", description="Role to grant" + ) + + +class WorkspaceMemberUpdateRequest(BaseModel): + """Request model for changing a member's role.""" + + role: Literal["owner", "editor", "viewer"] = Field(..., description="New role") + + +class WorkspaceMemberResponse(BaseModel): + """Response model for a single workspace member.""" + + user_id: int = Field(..., description="Member user id") + email: str = Field(..., description="Member email") + role: str = Field(..., description="Member role: owner/editor/viewer") + + +class WorkspaceMemberListResponse(BaseModel): + """Response model for the members of a workspace.""" + + members: list[WorkspaceMemberResponse] = Field( + default_factory=list, description="Workspace members (owner first)" + ) diff --git a/apps/web-backend/api/routes/workspaces.py b/apps/web-backend/api/routes/workspaces.py index 15c8dcdb3..e1696afa2 100644 --- a/apps/web-backend/api/routes/workspaces.py +++ b/apps/web-backend/api/routes/workspaces.py @@ -13,17 +13,31 @@ from core.permissions import ( WorkspaceRole, get_current_workspace, + require_workspace_access, user_role_in_workspace, ) from core.security import require_auth from fastapi import APIRouter, Depends, HTTPException, status -from services.workspace_service import create_workspace, list_accessible_workspaces +from services.workspace_service import ( + add_member, + create_workspace, + get_membership, + list_accessible_workspaces, + list_workspace_members, + remove_member, + update_member_role, +) from sqlalchemy.orm import Session +from api.models.user import User from api.models.workspace import ( Workspace, WorkspaceCreateRequest, WorkspaceListResponse, + WorkspaceMemberAddRequest, + WorkspaceMemberListResponse, + WorkspaceMemberResponse, + WorkspaceMemberUpdateRequest, WorkspaceResponse, WorkspaceUser, ) @@ -101,3 +115,106 @@ async def create_workspace_endpoint( user_id = _require_user_id(auth) workspace = create_workspace(db, owner_id=user_id, name=request.name) return _to_response(workspace, WorkspaceRole.OWNER) + + +# --------------------------------------------------------------------------- +# Member management (team mode) — the first real consumer of +# require_workspace_access: listing needs >= viewer, mutations need owner. +# --------------------------------------------------------------------------- + + +def _member_response(user_id: int, email: str, role: str) -> WorkspaceMemberResponse: + return WorkspaceMemberResponse(user_id=user_id, email=email, role=role) + + +@router.get("/{workspace_id}/members", response_model=WorkspaceMemberListResponse) +async def list_members( + workspace_id: int, + _role: WorkspaceRole = Depends(require_workspace_access(WorkspaceRole.VIEWER)), + db: Session = Depends(get_db), +): + """List a workspace's members (owner first). Requires >= viewer access.""" + workspace = db.query(Workspace).filter(Workspace.id == workspace_id).first() + if workspace is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="Workspace not found" + ) + members = [_member_response(workspace.owner_id, workspace.owner.email, "owner")] + members.extend( + _member_response(wu.user_id, wu.user.email, wu.role) + for wu in list_workspace_members(db, workspace_id) + ) + return WorkspaceMemberListResponse(members=members) + + +@router.post( + "/{workspace_id}/members", + response_model=WorkspaceMemberResponse, + status_code=status.HTTP_201_CREATED, +) +async def add_workspace_member( + workspace_id: int, + request: WorkspaceMemberAddRequest, + _role: WorkspaceRole = Depends(require_workspace_access(WorkspaceRole.OWNER)), + db: Session = Depends(get_db), +): + """Add a member to a workspace. Requires owner access.""" + workspace = db.query(Workspace).filter(Workspace.id == workspace_id).first() + if workspace is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="Workspace not found" + ) + if request.user_id == workspace.owner_id: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="The workspace owner already has access", + ) + user = db.query(User).filter(User.id == request.user_id).first() + if user is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="User not found" + ) + if get_membership(db, workspace_id, request.user_id) is not None: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, detail="User is already a member" + ) + membership = add_member(db, workspace_id, request.user_id, request.role) + return _member_response(membership.user_id, user.email, membership.role) + + +@router.patch( + "/{workspace_id}/members/{user_id}", response_model=WorkspaceMemberResponse +) +async def update_workspace_member( + workspace_id: int, + user_id: int, + request: WorkspaceMemberUpdateRequest, + _role: WorkspaceRole = Depends(require_workspace_access(WorkspaceRole.OWNER)), + db: Session = Depends(get_db), +): + """Change a member's role. Requires owner access.""" + membership = get_membership(db, workspace_id, user_id) + if membership is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="Membership not found" + ) + membership = update_member_role(db, membership, request.role) + return _member_response(membership.user_id, membership.user.email, membership.role) + + +@router.delete( + "/{workspace_id}/members/{user_id}", status_code=status.HTTP_204_NO_CONTENT +) +async def remove_workspace_member( + workspace_id: int, + user_id: int, + _role: WorkspaceRole = Depends(require_workspace_access(WorkspaceRole.OWNER)), + db: Session = Depends(get_db), +): + """Remove a member from a workspace. Requires owner access.""" + membership = get_membership(db, workspace_id, user_id) + if membership is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="Membership not found" + ) + remove_member(db, membership) diff --git a/apps/web-backend/services/workspace_service.py b/apps/web-backend/services/workspace_service.py index c39f50bf1..81b749541 100644 --- a/apps/web-backend/services/workspace_service.py +++ b/apps/web-backend/services/workspace_service.py @@ -79,3 +79,62 @@ def create_workspace(db: Session, owner_id: int, name: str) -> Workspace: # Do not log the user-supplied name (log-injection); id + owner suffice. logger.info("Created workspace id=%s owner_id=%s", workspace.id, owner_id) return workspace + + +def list_workspace_members(db: Session, workspace_id: int) -> list[WorkspaceUser]: + """Return the explicit membership rows for a workspace (excludes the owner).""" + return ( + db.query(WorkspaceUser) + .filter(WorkspaceUser.workspace_id == workspace_id) + .order_by(WorkspaceUser.id.asc()) + .all() + ) + + +def get_membership( + db: Session, workspace_id: int, user_id: int +) -> WorkspaceUser | None: + """Return the user's membership row in the workspace, or None.""" + return ( + db.query(WorkspaceUser) + .filter( + WorkspaceUser.workspace_id == workspace_id, + WorkspaceUser.user_id == user_id, + ) + .first() + ) + + +def add_member( + db: Session, workspace_id: int, user_id: int, role: str +) -> WorkspaceUser: + """Insert a membership row (callers validate uniqueness/owner/user first).""" + membership = WorkspaceUser( + workspace_id=workspace_id, user_id=user_id, role=role + ) + db.add(membership) + db.commit() + db.refresh(membership) + logger.info( + "Added member user_id=%s role=%s to workspace_id=%s", + user_id, + role, + workspace_id, + ) + return membership + + +def update_member_role( + db: Session, membership: WorkspaceUser, role: str +) -> WorkspaceUser: + """Update a membership's role in place.""" + membership.role = role + db.commit() + db.refresh(membership) + return membership + + +def remove_member(db: Session, membership: WorkspaceUser) -> None: + """Delete a membership row.""" + db.delete(membership) + db.commit() diff --git a/apps/web-backend/tests/test_workspaces.py b/apps/web-backend/tests/test_workspaces.py index 1b6417809..1e6967593 100644 --- a/apps/web-backend/tests/test_workspaces.py +++ b/apps/web-backend/tests/test_workspaces.py @@ -18,9 +18,14 @@ ) from fastapi import HTTPException from services.workspace_service import ( + add_member, create_workspace, + get_membership, get_or_create_personal_workspace, list_accessible_workspaces, + list_workspace_members, + remove_member, + update_member_role, ) from sqlalchemy.exc import IntegrityError @@ -282,3 +287,100 @@ def _override_auth(): assert names == {"Personal", "Team X"} finally: app.dependency_overrides.clear() + + +# --------------------------------------------------------------------------- +# C2 (cont.) — workspace member management +# --------------------------------------------------------------------------- + + +def test_member_service_crud(test_db): + owner = _make_user(test_db, "ms-owner@test.com") + ws = create_workspace(test_db, owner_id=owner.id, name="WS") + member = _make_user(test_db, "ms-member@test.com") + + m = add_member(test_db, ws.id, member.id, "viewer") + assert m.role == "viewer" + assert get_membership(test_db, ws.id, member.id) is not None + assert [x.user_id for x in list_workspace_members(test_db, ws.id)] == [member.id] + + update_member_role(test_db, m, "editor") + assert get_membership(test_db, ws.id, member.id).role == "editor" + + remove_member(test_db, m) + assert get_membership(test_db, ws.id, member.id) is None + assert list_workspace_members(test_db, ws.id) == [] + + +def test_workspace_members_api(test_db, monkeypatch): + """End-to-end HTTP test of member management + require_workspace_access.""" + monkeypatch.setattr(settings, "CLOUD_MODE", "team") + from core.database import get_db + from core.security import require_auth + from fastapi.testclient import TestClient + from main import app + + owner = _make_user(test_db, "mapi-owner@test.com") + member = _make_user(test_db, "mapi-member@test.com") + workspace = create_workspace(test_db, owner_id=owner.id, name="Team") + base = f"/api/workspaces/{workspace.id}/members" + + # A mutable holder lets us switch the acting user mid-test. + auth_holder = {"sub": str(owner.id), "email": owner.email} + + def _override_db(): + yield test_db + + def _override_auth(): + return auth_holder + + app.dependency_overrides[get_db] = _override_db + app.dependency_overrides[require_auth] = _override_auth + try: + client = TestClient(app) + + # Initially only the owner is a member. + resp = client.get(base) + assert resp.status_code == 200 + assert [m["role"] for m in resp.json()["members"]] == ["owner"] + + # Owner adds a member. + resp = client.post(base, json={"user_id": member.id, "role": "viewer"}) + assert resp.status_code == 201 + assert resp.json() == { + "user_id": member.id, + "email": member.email, + "role": "viewer", + } + + # Duplicate add -> 409; adding the owner -> 400. + assert client.post(base, json={"user_id": member.id}).status_code == 409 + assert ( + client.post(base, json={"user_id": owner.id, "role": "editor"}).status_code + == 400 + ) + + # Listing now shows owner + member. + resp = client.get(base) + assert {m["email"] for m in resp.json()["members"]} == { + owner.email, + member.email, + } + + # Owner promotes the member to editor. + resp = client.patch(f"{base}/{member.id}", json={"role": "editor"}) + assert resp.status_code == 200 + assert resp.json()["role"] == "editor" + + # A non-owner (the editor) cannot mutate members -> 403, but can list. + auth_holder["sub"] = str(member.id) + assert client.post(base, json={"user_id": owner.id}).status_code == 403 + assert client.get(base).status_code == 200 + + # Owner removes the member. + auth_holder["sub"] = str(owner.id) + assert client.delete(f"{base}/{member.id}").status_code == 204 + resp = client.get(base) + assert [m["role"] for m in resp.json()["members"]] == ["owner"] + finally: + app.dependency_overrides.clear() From 957baf0d9e330585f2fac2c81e2aa6732e6296aa Mon Sep 17 00:00:00 2001 From: Oleg Miagkov Date: Thu, 25 Jun 2026 13:46:24 +0400 Subject: [PATCH 4/7] perf(cloud): eager-load member user to avoid N+1 in member listing (C2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit list_workspace_members now joinedload's WorkspaceUser.user so the members route reads wu.user.email without a per-row query — same fix as list_workspaces. Co-Authored-By: Claude Opus 4.8 --- apps/web-backend/services/workspace_service.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/web-backend/services/workspace_service.py b/apps/web-backend/services/workspace_service.py index 81b749541..7bcfa482a 100644 --- a/apps/web-backend/services/workspace_service.py +++ b/apps/web-backend/services/workspace_service.py @@ -15,7 +15,7 @@ from api.models.workspace import Workspace, WorkspaceUser from sqlalchemy import or_ -from sqlalchemy.orm import Session +from sqlalchemy.orm import Session, joinedload logger = logging.getLogger(__name__) @@ -82,9 +82,13 @@ def create_workspace(db: Session, owner_id: int, name: str) -> Workspace: def list_workspace_members(db: Session, workspace_id: int) -> list[WorkspaceUser]: - """Return the explicit membership rows for a workspace (excludes the owner).""" + """Return the explicit membership rows for a workspace (excludes the owner). + + Eager-loads the related user so callers can read ``wu.user`` without an N+1. + """ return ( db.query(WorkspaceUser) + .options(joinedload(WorkspaceUser.user)) .filter(WorkspaceUser.workspace_id == workspace_id) .order_by(WorkspaceUser.id.asc()) .all() From c365a7e72401881589d705b6eb989588425824dd Mon Sep 17 00:00:00 2001 From: Oleg Miagkov Date: Thu, 25 Jun 2026 13:53:56 +0400 Subject: [PATCH 5/7] refactor(cloud): extract repeated 403 detail constant (Sonar S1192) The 'Insufficient workspace permissions' literal is now used 3x in permissions.py; hoist it to a module constant. (The remaining Sonar S8410 'use Annotated for deps' hints are left as-is: the whole web-backend uses = Depends(...), per the convention CodeRabbit recorded as a repo learning.) Co-Authored-By: Claude Opus 4.8 --- apps/web-backend/core/permissions.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/apps/web-backend/core/permissions.py b/apps/web-backend/core/permissions.py index 9a79534e5..d32227c4f 100644 --- a/apps/web-backend/core/permissions.py +++ b/apps/web-backend/core/permissions.py @@ -32,6 +32,9 @@ class WorkspaceRole(str, Enum): WorkspaceRole.OWNER: 2, } +# Reused 403 detail for workspace permission failures (avoid a duplicated literal). +_INSUFFICIENT_PERMISSIONS = "Insufficient workspace permissions" + def role_satisfies(actual: "WorkspaceRole | str", required: "WorkspaceRole | str") -> bool: """True when ``actual`` grants at least the privilege of ``required``.""" @@ -110,7 +113,7 @@ def dependency( if role is None or not role_satisfies(role, required_role): raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, - detail="Insufficient workspace permissions", + detail=_INSUFFICIENT_PERMISSIONS, ) return role @@ -150,12 +153,12 @@ def get_current_workspace( if not check_workspace_access(db, user_id, workspace_id, WorkspaceRole.VIEWER): raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, - detail="Insufficient workspace permissions", + detail=_INSUFFICIENT_PERMISSIONS, ) workspace = db.query(Workspace).filter(Workspace.id == workspace_id).first() if workspace is None: # pragma: no cover - access check already proved existence raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, - detail="Insufficient workspace permissions", + detail=_INSUFFICIENT_PERMISSIONS, ) return workspace From 89d0505da58912666cb82ce3271c6211636b0bf0 Mon Sep 17 00:00:00 2001 From: Oleg Miagkov Date: Thu, 25 Jun 2026 14:00:14 +0400 Subject: [PATCH 6/7] =?UTF-8?q?fix(cloud):=20clear=20CodeQL=20=E2=80=94=20?= =?UTF-8?q?drop=20tainted=20member=20log=20+=20no=20side-effects=20in=20as?= =?UTF-8?q?serts=20(C2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - workspace_service.add_member: remove the info log (user_id/role/workspace_id are request-derived → CodeQL log-injection; member-change audit is C3/C7). - tests: extract client.post/get/delete calls out of assert expressions so python -O can't strip the action (CodeQL py/side-effect-in-assert). 13 tests pass; ruff clean. Co-Authored-By: Claude Opus 4.8 --- apps/web-backend/services/workspace_service.py | 8 ++------ apps/web-backend/tests/test_workspaces.py | 18 ++++++++++-------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/apps/web-backend/services/workspace_service.py b/apps/web-backend/services/workspace_service.py index 7bcfa482a..36a8c6452 100644 --- a/apps/web-backend/services/workspace_service.py +++ b/apps/web-backend/services/workspace_service.py @@ -119,12 +119,8 @@ def add_member( db.add(membership) db.commit() db.refresh(membership) - logger.info( - "Added member user_id=%s role=%s to workspace_id=%s", - user_id, - role, - workspace_id, - ) + # No info log here: user_id/role/workspace_id are request-derived (CodeQL + # log-injection). Membership changes belong in the audit story (C3/C7). return membership diff --git a/apps/web-backend/tests/test_workspaces.py b/apps/web-backend/tests/test_workspaces.py index 1e6967593..9ed876b07 100644 --- a/apps/web-backend/tests/test_workspaces.py +++ b/apps/web-backend/tests/test_workspaces.py @@ -354,11 +354,10 @@ def _override_auth(): } # Duplicate add -> 409; adding the owner -> 400. - assert client.post(base, json={"user_id": member.id}).status_code == 409 - assert ( - client.post(base, json={"user_id": owner.id, "role": "editor"}).status_code - == 400 - ) + dup = client.post(base, json={"user_id": member.id}) + assert dup.status_code == 409 + add_owner = client.post(base, json={"user_id": owner.id, "role": "editor"}) + assert add_owner.status_code == 400 # Listing now shows owner + member. resp = client.get(base) @@ -374,12 +373,15 @@ def _override_auth(): # A non-owner (the editor) cannot mutate members -> 403, but can list. auth_holder["sub"] = str(member.id) - assert client.post(base, json={"user_id": owner.id}).status_code == 403 - assert client.get(base).status_code == 200 + denied = client.post(base, json={"user_id": owner.id}) + assert denied.status_code == 403 + listed = client.get(base) + assert listed.status_code == 200 # Owner removes the member. auth_holder["sub"] = str(owner.id) - assert client.delete(f"{base}/{member.id}").status_code == 204 + removed = client.delete(f"{base}/{member.id}") + assert removed.status_code == 204 resp = client.get(base) assert [m["role"] for m in resp.json()["members"]] == ["owner"] finally: From 6ece55040e715395c107893bf79c389cff911b4b Mon Sep 17 00:00:00 2001 From: Oleg Miagkov Date: Thu, 25 Jun 2026 14:41:11 +0400 Subject: [PATCH 7/7] feat(cloud): unique Personal workspace per owner + race-safe bootstrap (C2) Closes the concurrent-duplicate edge case flagged in review (previously deferred). Adds a partial unique index uq_personal_workspace_per_owner on workspaces(owner_id) WHERE name='Personal' (model __table_args__ + migration 004; SQLite + Postgres partial-WHERE). get_or_create_personal_workspace is now truly idempotent under concurrency: the race loser catches IntegrityError and reuses the winner's workspace. Test asserts a second 'Personal' is rejected while other names stay unconstrained. Note: this adds the first C2 migration (004), upgrading the slice from no-schema to a single additive index. Co-Authored-By: Claude Opus 4.8 --- apps/web-backend/api/models/workspace.py | 14 ++++++ .../versions/004_unique_personal_workspace.py | 47 +++++++++++++++++++ .../web-backend/services/workspace_service.py | 32 +++++++++---- apps/web-backend/tests/test_workspaces.py | 16 +++++++ 4 files changed, 100 insertions(+), 9 deletions(-) create mode 100644 apps/web-backend/migrations/versions/004_unique_personal_workspace.py diff --git a/apps/web-backend/api/models/workspace.py b/apps/web-backend/api/models/workspace.py index c009b17b5..f7ba4d753 100644 --- a/apps/web-backend/api/models/workspace.py +++ b/apps/web-backend/api/models/workspace.py @@ -17,9 +17,11 @@ Column, DateTime, ForeignKey, + Index, Integer, String, UniqueConstraint, + text, ) from sqlalchemy.orm import relationship @@ -28,6 +30,18 @@ class Workspace(Base): """A tenant boundary owning specs/runs/repositories, with role-based members.""" __tablename__ = "workspaces" + __table_args__ = ( + # At most one "Personal" workspace per owner (partial unique index) — this + # makes get_or_create_personal_workspace race-safe under concurrent + # register/login. Other workspace names are unconstrained. + Index( + "uq_personal_workspace_per_owner", + "owner_id", + unique=True, + sqlite_where=text("name = 'Personal'"), + postgresql_where=text("name = 'Personal'"), + ), + ) id = Column(Integer, primary_key=True, index=True) name = Column(String(255), nullable=False) diff --git a/apps/web-backend/migrations/versions/004_unique_personal_workspace.py b/apps/web-backend/migrations/versions/004_unique_personal_workspace.py new file mode 100644 index 000000000..d82e2eba7 --- /dev/null +++ b/apps/web-backend/migrations/versions/004_unique_personal_workspace.py @@ -0,0 +1,47 @@ +"""Enforce one Personal workspace per owner (partial unique index) + +Revision ID: 004 +Revises: 003 +Create Date: 2026-06-25 00:00:00.000000 + +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +# Alembic requires these module-level identifiers +__all__ = [ + "revision", + "down_revision", + "branch_labels", + "depends_on", + "upgrade", + "downgrade", +] + +# revision identifiers, used by Alembic. +revision: str = "004" +down_revision: str | None = "003" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + +_INDEX_NAME = "uq_personal_workspace_per_owner" + + +def upgrade() -> None: + """Add a partial unique index: at most one 'Personal' workspace per owner.""" + op.create_index( + _INDEX_NAME, + "workspaces", + ["owner_id"], + unique=True, + sqlite_where=sa.text("name = 'Personal'"), + postgresql_where=sa.text("name = 'Personal'"), + ) + + +def downgrade() -> None: + """Drop the partial unique index.""" + op.drop_index(_INDEX_NAME, table_name="workspaces") diff --git a/apps/web-backend/services/workspace_service.py b/apps/web-backend/services/workspace_service.py index 36a8c6452..14f1a475a 100644 --- a/apps/web-backend/services/workspace_service.py +++ b/apps/web-backend/services/workspace_service.py @@ -15,6 +15,7 @@ from api.models.workspace import Workspace, WorkspaceUser from sqlalchemy import or_ +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session, joinedload logger = logging.getLogger(__name__) @@ -23,14 +24,9 @@ PERSONAL_WORKSPACE_NAME = "Personal" -def get_or_create_personal_workspace(db: Session, user_id: int) -> Workspace: - """Return the user's Personal workspace, creating it if absent (idempotent). - - This is the single-mode default workspace. If the user already owns one named - ``PERSONAL_WORKSPACE_NAME`` the oldest such workspace is reused, so repeated - calls (e.g. on every login) never create duplicates. - """ - existing = ( +def _find_personal_workspace(db: Session, user_id: int) -> Workspace | None: + """Return the user's oldest workspace named PERSONAL_WORKSPACE_NAME, or None.""" + return ( db.query(Workspace) .filter( Workspace.owner_id == user_id, @@ -39,12 +35,30 @@ def get_or_create_personal_workspace(db: Session, user_id: int) -> Workspace: .order_by(Workspace.id.asc()) .first() ) + + +def get_or_create_personal_workspace(db: Session, user_id: int) -> Workspace: + """Return the user's Personal workspace, creating it if absent (idempotent). + + This is the single-mode default workspace. A partial unique index + (``uq_personal_workspace_per_owner``) guarantees one per owner, so concurrent + register/login calls cannot create duplicates: the loser of the race catches + the IntegrityError and reuses the winner's workspace. + """ + existing = _find_personal_workspace(db, user_id) if existing is not None: return existing workspace = Workspace(name=PERSONAL_WORKSPACE_NAME, owner_id=user_id) db.add(workspace) - db.commit() + try: + db.commit() + except IntegrityError: + db.rollback() + winner = _find_personal_workspace(db, user_id) + if winner is None: # pragma: no cover - the unique index guarantees one + raise + return winner db.refresh(workspace) logger.info( "Bootstrapped Personal workspace id=%s for user_id=%s", workspace.id, user_id diff --git a/apps/web-backend/tests/test_workspaces.py b/apps/web-backend/tests/test_workspaces.py index 9ed876b07..9f38ec755 100644 --- a/apps/web-backend/tests/test_workspaces.py +++ b/apps/web-backend/tests/test_workspaces.py @@ -179,6 +179,22 @@ def test_get_or_create_personal_workspace_is_idempotent(test_db): assert len(owned) == 1 +def test_personal_workspace_unique_per_owner(test_db): + owner = _make_user(test_db, "uniq@test.com") + get_or_create_personal_workspace(test_db, owner.id) + + # A second "Personal" for the same owner violates the partial unique index. + test_db.add(Workspace(name="Personal", owner_id=owner.id)) + with pytest.raises(IntegrityError): + test_db.commit() + test_db.rollback() + + # The partial index only covers "Personal" rows; other names are unconstrained. + create_workspace(test_db, owner_id=owner.id, name="Team A") + create_workspace(test_db, owner_id=owner.id, name="Team B") + assert test_db.query(Workspace).filter(Workspace.owner_id == owner.id).count() == 3 + + def test_list_accessible_workspaces_owned_and_member(test_db): user = _make_user(test_db, "lister@test.com") owned = create_workspace(test_db, owner_id=user.id, name="Mine")