diff --git a/backend/api/cloud/__init__.py b/backend/api/cloud/__init__.py new file mode 100644 index 0000000..f03cd22 --- /dev/null +++ b/backend/api/cloud/__init__.py @@ -0,0 +1,63 @@ +""" +Cloud API package — router aggregator. + +One parent router owns the /api/cloud prefix. +Sub-routers (connections, browse) carry no prefix — paths are declared +relative to the parent (D-04 pattern, same as api/documents/__init__.py). + +main.py imports: + from api.cloud import router as cloud_router, users_router + app.include_router(cloud_router) + app.include_router(users_router) +""" +from fastapi import APIRouter, Depends, HTTPException, Request, status +from pydantic import BaseModel +from sqlalchemy.ext.asyncio import AsyncSession + +from api.cloud.connections import router as connections_router, _VALID_BACKENDS +from api.cloud.browse import router as browse_router +from db.models import User +from deps.auth import get_regular_user +from deps.db import get_db +from services.rate_limiting import account_limiter + +router = APIRouter(prefix="/api/cloud", tags=["cloud"]) +router.include_router(connections_router) +router.include_router(browse_router) + +# ── Users router (default storage) ─────────────────────────────────────────── + +users_router = APIRouter(prefix="/api/users", tags=["users"]) + + +class DefaultStorageRequest(BaseModel): + backend: str + + +@users_router.patch("/me/default-storage") +@account_limiter.limit("100/minute") +async def update_default_storage( + request: Request, + body: DefaultStorageRequest, + session: AsyncSession = Depends(get_db), + current_user: User = Depends(get_regular_user), +) -> dict: + """Update the current user's default storage backend. + + Validated against the allowlist before storage. + """ + if body.backend not in _VALID_BACKENDS: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=f"Invalid backend. Valid values: {sorted(_VALID_BACKENDS)}", + ) + request.state.current_user = current_user + user = await session.get(User, current_user.id) + if user is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="User not found") + + user.default_storage_backend = body.backend + session.add(user) + await session.commit() + + return {"default_storage_backend": user.default_storage_backend} diff --git a/backend/api/cloud/browse.py b/backend/api/cloud/browse.py new file mode 100644 index 0000000..44009ff --- /dev/null +++ b/backend/api/cloud/browse.py @@ -0,0 +1,333 @@ +""" +Canonical connection-ID browse endpoint for Phase 12. + +GET /api/cloud/connections/{connection_id}/items + - Owner-scoped: resolves connection by UUID, not provider name + - Stale-while-revalidate: returns durable cached rows immediately; schedules + background refresh if stale or first visit + - T-12-01: IDOR-protected via resolve_owned_connection + - T-12-03: response uses whitelisted schemas, no credentials in response + - D-18/CACHE-01: never downloads file bytes, never mutates quota +""" +from __future__ import annotations + +import asyncio +import uuid +from typing import Optional + +import httpx +from fastapi import APIRouter, Depends, HTTPException, Request, status +from sqlalchemy.ext.asyncio import AsyncSession + +from api.cloud.schemas import ( + CloudBrowseResponse, + CloudCapabilityOut, + CloudItemOut, + FolderFreshnessOut, +) +from db.models import CloudConnection, CloudItem, CloudFolderState, User +from deps.auth import get_regular_user +from deps.db import get_db +from deps.utils import parse_uuid +from services.cloud_items import ( + ConnectionNotFound, + list_cloud_children, + get_or_create_folder_state, + reconcile_cloud_listing, + resolve_owned_connection, + update_folder_state, +) +from services.rate_limiting import account_limiter +from storage.cloud_backend_factory import build_cloud_resource_adapter +from storage.cloud_utils import decrypt_credentials, encrypt_credentials +from config import settings + +router = APIRouter() + +_DISPLAY_NAMES = { + "google_drive": "Google Drive", + "onedrive": "OneDrive", + "nextcloud": "Nextcloud", + "webdav": "WebDAV server", +} + + +def _master_key() -> bytes: + return settings.cloud_creds_key.encode() + + +def _decrypt_connection(conn: CloudConnection, user_id: uuid.UUID) -> dict: + return decrypt_credentials(_master_key(), str(user_id), conn.credentials_enc) + + +def _capability_out(caps: dict) -> dict[str, CloudCapabilityOut]: + return { + action: CloudCapabilityOut( + action=cap.action, + state=cap.state, + reason=cap.reason, + message=cap.message, + ) + for action, cap in caps.items() + } + + +def _item_out(item: CloudItem) -> CloudItemOut: + return CloudItemOut( + id=str(item.id), + provider_item_id=item.provider_item_id, + name=item.name, + kind=item.kind, + parent_ref=item.parent_ref, + content_type=item.content_type, + size=item.provider_size, + modified_at=item.modified_at, + etag=item.etag, + capabilities={}, # Phase 13 will add per-item capabilities + ) + + +def _freshness_out(fs: CloudFolderState) -> FolderFreshnessOut: + return FolderFreshnessOut( + refresh_state=fs.refresh_state, + last_refreshed_at=fs.last_refreshed_at, + error_code=fs.error_code, + error_message=fs.error_message, + ) + + +# ── Legacy helpers used by compatibility route in connections.py ───────────── + +async def _fetch_google_drive_folders(credentials: dict, folder_id: str) -> list: + """Legacy listing helper preserved for compatibility route.""" + import datetime as dt + from google.oauth2.credentials import Credentials as GoogleCreds + from googleapiclient.discovery import build + + expiry = None + if expiry_str := credentials.get("expiry"): + try: + expiry = dt.datetime.fromisoformat(expiry_str) + except ValueError: + pass + + creds = GoogleCreds( + token=credentials.get("access_token"), + refresh_token=credentials.get("refresh_token"), + token_uri=credentials.get("token_uri", "https://oauth2.googleapis.com/token"), + client_id=credentials.get("client_id"), + client_secret=credentials.get("client_secret"), + expiry=expiry, + ) + + def _list_files() -> list: + service = build("drive", "v3", credentials=creds, cache_discovery=False) + response = service.files().list( + q=f"'{folder_id}' in parents and trashed=false", + fields="files(id,name,mimeType,size)", + pageSize=200, + ).execute() + items = [] + for item in response.get("files", []): + is_dir = item.get("mimeType") == "application/vnd.google-apps.folder" + items.append({ + "id": item["id"], + "name": item["name"], + "is_dir": is_dir, + "size": int(item.get("size", 0)) if not is_dir else 0, + }) + return items + + return await asyncio.to_thread(_list_files) + + +async def _fetch_onedrive_folders(credentials: dict, folder_id: str) -> list: + """Legacy listing helper preserved for compatibility route.""" + if folder_id in ("root", ""): + url = "https://graph.microsoft.com/v1.0/me/drive/root/children" + else: + url = f"https://graph.microsoft.com/v1.0/me/drive/items/{folder_id}/children" + + async with httpx.AsyncClient() as client: + resp = await client.get( + url, + headers={"Authorization": f"Bearer {credentials.get('access_token', '')}"}, + timeout=30, + ) + resp.raise_for_status() + data = resp.json() + + items = [] + for item in data.get("value", []): + is_dir = "folder" in item + items.append({ + "id": item["id"], + "name": item["name"], + "is_dir": is_dir, + "size": item.get("size", 0) if not is_dir else 0, + }) + return items + + +async def _fetch_webdav_folders(provider: str, credentials: dict, folder_id: str) -> list: + """Legacy listing helper preserved for compatibility route.""" + from storage.cloud_backend_factory import build_cloud_backend + webdav_path = "" if folder_id == "root" else folder_id + backend = build_cloud_backend(provider, credentials) + return await backend.list_folder(webdav_path) + + +# ── Canonical connection-ID browse endpoint ────────────────────────────────── + +@router.get("/connections/{connection_id}/items", response_model=CloudBrowseResponse) +@account_limiter.limit("100/minute") +async def browse_connection_items( + connection_id: uuid.UUID, + request: Request, + parent_ref: Optional[str] = None, + session: AsyncSession = Depends(get_db), + current_user: User = Depends(get_regular_user), +) -> CloudBrowseResponse: + """Browse the items of a connection by UUID. + + T-12-01: IDOR protection — connection resolved by owner UUID, not provider alone. + T-12-03: response schema excludes credentials_enc and all credential fields. + D-18/CACHE-01: returns durable cached rows; never downloads file bytes. + D-11/D-12: cached rows returned immediately; background refresh scheduled if stale. + D-13/D-14: on refresh failure, cached rows retained with warning state. + """ + request.state.current_user = current_user + + uid_str = str(current_user.id) + cid_str = str(connection_id) + + # Resolve connection ownership (T-12-01) + try: + conn = await resolve_owned_connection( + session, + connection_id=cid_str, + user_id=uid_str, + ) + except ConnectionNotFound: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Connection not found") + + # Ensure folder state row exists + folder_state = await get_or_create_folder_state( + session, + user_id=uid_str, + connection_id=cid_str, + parent_ref=parent_ref or "", + ) + await session.commit() + + # Return durable cached rows + cached_items = await list_cloud_children( + session, + user_id=uid_str, + connection_id=cid_str, + parent_ref=parent_ref, + ) + + # Get connection-level capabilities (from a fresh call to avoid stale state) + try: + credentials = _decrypt_connection(conn, current_user.id) + adapter = build_cloud_resource_adapter(conn.provider, credentials) + conn_caps = await adapter.get_capabilities(connection_id, current_user.id) + except Exception: + from storage.cloud_base import ACTIONS, CloudCapability, STATE_TEMPORARILY_UNAVAILABLE, REASON_OFFLINE + conn_caps = { + action: CloudCapability( + action=action, + state=STATE_TEMPORARILY_UNAVAILABLE, + reason=REASON_OFFLINE, + message="Provider temporarily unreachable.", + ) + for action in ACTIONS + } + + # Refresh in background if first visit (no items) or stale + should_refresh = ( + not cached_items + or folder_state.refresh_state in ("refreshing",) + or folder_state.last_refreshed_at is None + ) + if should_refresh and not cached_items: + # First visit: do a synchronous bounded fetch so user sees content + try: + credentials = _decrypt_connection(conn, current_user.id) + adapter = build_cloud_resource_adapter(conn.provider, credentials) + listing = await adapter.list_folder( + connection_id, + current_user.id, + parent_ref=parent_ref, + ) + await reconcile_cloud_listing( + session, + user_id=uid_str, + connection_id=cid_str, + parent_ref=parent_ref, + listing=listing, + ) + await update_folder_state( + session, + user_id=uid_str, + connection_id=cid_str, + parent_ref=parent_ref or "", + refresh_state="fresh", + ) + await session.commit() + # Re-read items after reconciliation + cached_items = await list_cloud_children( + session, + user_id=uid_str, + connection_id=cid_str, + parent_ref=parent_ref, + ) + # Re-fetch folder state + folder_state = await get_or_create_folder_state( + session, + user_id=uid_str, + connection_id=cid_str, + parent_ref=parent_ref or "", + ) + except Exception as exc: + # Retain cached rows (empty on first visit), set warning state + await update_folder_state( + session, + user_id=uid_str, + connection_id=cid_str, + parent_ref=parent_ref or "", + refresh_state="warning", + error_code="provider_error", + error_message="Provider temporarily unavailable. Retrying in background.", + ) + await session.commit() + folder_state = await get_or_create_folder_state( + session, + user_id=uid_str, + connection_id=cid_str, + parent_ref=parent_ref or "", + ) + elif should_refresh and cached_items: + # Has cached items — schedule background refresh via Celery + try: + from tasks.cloud_tasks import refresh_cloud_folder + refresh_cloud_folder.delay( + str(current_user.id), + str(connection_id), + parent_ref, + ) + except Exception: + pass # Celery unavailable — serve stale cache without failing browse + + display_name = conn.display_name_override or conn.display_name + + return CloudBrowseResponse( + connection_id=str(connection_id), + provider=conn.provider, + display_name=display_name, + parent_ref=parent_ref, + items=[_item_out(item) for item in cached_items], + capabilities=_capability_out(conn_caps), + freshness=_freshness_out(folder_state), + ) diff --git a/backend/api/cloud.py b/backend/api/cloud/connections.py similarity index 83% rename from backend/api/cloud.py rename to backend/api/cloud/connections.py index b2d1a33..faa6bba 100644 --- a/backend/api/cloud.py +++ b/backend/api/cloud/connections.py @@ -1,4 +1,9 @@ -"""Cloud storage connection management endpoints.""" +""" +Connection management endpoints — all existing cloud connection behaviors. + +Preserves all existing endpoint URLs from api/cloud.py without change. +Introduces PATCH /api/cloud/connections/{connection_id} for display_name rename. +""" from __future__ import annotations import asyncio @@ -13,19 +18,19 @@ from pydantic import BaseModel from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession +from api.cloud.schemas import ConnectionRenameRequest from api.schemas import CloudConnectionOut from config import settings from db.models import CloudConnection, User from deps.auth import get_regular_user from deps.db import get_db -from deps.utils import get_client_ip +from deps.utils import get_client_ip, parse_uuid from services.audit import write_audit_log from services.rate_limiting import account_limiter from storage.cloud_backend_factory import build_cloud_backend from storage.cloud_utils import decrypt_credentials, encrypt_credentials, validate_cloud_url -router = APIRouter(prefix="/api/cloud", tags=["cloud"]) -users_router = APIRouter(prefix="/api/users", tags=["users"]) +router = APIRouter() VALID_OAUTH_PROVIDERS = {"google_drive", "onedrive"} VALID_WEBDAV_PROVIDERS = {"nextcloud", "webdav"} @@ -107,6 +112,7 @@ async def _upsert_cloud_connection( provider: str, credentials_enc: str, ) -> CloudConnection: + """Insert or update a connection for (user_id, provider).""" result = await session.execute( select(CloudConnection).where( CloudConnection.user_id == user_id, @@ -137,6 +143,7 @@ async def _get_owned_connection( connection_id: uuid.UUID, user_id: uuid.UUID, ) -> CloudConnection: + """Return the connection owned by user_id, or raise 404.""" conn = await session.get(CloudConnection, connection_id) if conn is None or conn.user_id != user_id: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Connection not found") @@ -278,83 +285,7 @@ def _connection_response(conn: CloudConnection) -> dict: return data -async def _fetch_google_drive_folders(credentials: dict, folder_id: str) -> list: - from google.oauth2.credentials import Credentials # lazy import - from googleapiclient.discovery import build # lazy import - - import datetime as dt - - expiry = None - if expiry_str := credentials.get("expiry"): - try: - expiry = dt.datetime.fromisoformat(expiry_str) - except ValueError: - pass - - creds = Credentials( - token=credentials.get("access_token"), - refresh_token=credentials.get("refresh_token"), - token_uri=credentials.get("token_uri", "https://oauth2.googleapis.com/token"), - client_id=credentials.get("client_id"), - client_secret=credentials.get("client_secret"), - expiry=expiry, - ) - - def _list_files() -> list: - service = build("drive", "v3", credentials=creds, cache_discovery=False) - response = service.files().list( - q=f"'{folder_id}' in parents and trashed=false", - fields="files(id,name,mimeType,size)", - pageSize=200, - ).execute() - items = [] - for item in response.get("files", []): - is_dir = item.get("mimeType") == "application/vnd.google-apps.folder" - items.append({ - "id": item["id"], - "name": item["name"], - "is_dir": is_dir, - "size": int(item.get("size", 0)) if not is_dir else 0, - }) - return items - - return await asyncio.to_thread(_list_files) - - -async def _fetch_onedrive_folders(credentials: dict, folder_id: str) -> list: - import httpx # lazy import - - if folder_id in ("root", ""): - url = "https://graph.microsoft.com/v1.0/me/drive/root/children" - else: - url = f"https://graph.microsoft.com/v1.0/me/drive/items/{folder_id}/children" - - async with httpx.AsyncClient() as client: - resp = await client.get( - url, - headers={"Authorization": f"Bearer {credentials.get('access_token', '')}"}, - timeout=30, - ) - resp.raise_for_status() - data = resp.json() - - items = [] - for item in data.get("value", []): - is_dir = "folder" in item - items.append({ - "id": item["id"], - "name": item["name"], - "is_dir": is_dir, - "size": item.get("size", 0) if not is_dir else 0, - }) - return items - - -async def _fetch_webdav_folders(provider: str, credentials: dict, folder_id: str) -> list: - webdav_path = "" if folder_id == "root" else folder_id - backend = build_cloud_backend(provider, credentials) - return await backend.list_folder(webdav_path) - +# ── Endpoints ───────────────────────────────────────────────────────────────── @router.get("/oauth/initiate/{provider}") @account_limiter.limit("100/minute") @@ -561,6 +492,28 @@ async def get_connection_config( } +@router.patch("/connections/{connection_id}", status_code=status.HTTP_200_OK) +@account_limiter.limit("100/minute") +async def rename_connection( + connection_id: uuid.UUID, + body: ConnectionRenameRequest, + request: Request, + session: AsyncSession = Depends(get_db), + current_user: User = Depends(get_regular_user), +) -> dict: + """Rename a cloud connection's display name. + + Only the display_name field is accepted — mass assignment prevention. + T-12-01: owner check via _get_owned_connection. + """ + request.state.current_user = current_user + conn = await _get_owned_connection(session, connection_id, current_user.id) + conn.display_name_override = body.display_name + await session.commit() + await session.refresh(conn) + return {"id": str(conn.id), "display_name": body.display_name} + + @router.delete("/connections/{connection_id}", status_code=status.HTTP_204_NO_CONTENT) @account_limiter.limit("100/minute") async def delete_connection( @@ -604,7 +557,11 @@ async def list_cloud_folders( session: AsyncSession = Depends(get_db), current_user: User = Depends(get_regular_user), ) -> dict: - """List folder contents for a connected cloud provider.""" + """List folder contents for a connected cloud provider (compatibility route). + + This endpoint is the original provider-keyed browse route preserved for + backward compatibility. The canonical route is GET /connections/{id}/items. + """ request.state.current_user = current_user if provider not in VALID_CLOUD_PROVIDERS: raise _unsupported_provider_error(provider, VALID_CLOUD_PROVIDERS) @@ -615,44 +572,17 @@ async def list_cloud_folders( from services.cloud_cache import get_cloud_folders_cached # lazy import if provider == "google_drive": + from api.cloud.browse import _fetch_google_drive_folders fetcher = lambda: _fetch_google_drive_folders(credentials, folder_id) elif provider == "onedrive": + from api.cloud.browse import _fetch_onedrive_folders fetcher = lambda: _fetch_onedrive_folders(credentials, folder_id) else: + from api.cloud.browse import _fetch_webdav_folders fetcher = lambda: _fetch_webdav_folders(provider, credentials, folder_id) items = await get_cloud_folders_cached(str(current_user.id), provider, folder_id, fetcher) return {"items": items} - - -@users_router.patch("/me/default-storage") -@account_limiter.limit("100/minute") -async def update_default_storage( - request: Request, - body: DefaultStorageRequest, - session: AsyncSession = Depends(get_db), - current_user: User = Depends(get_regular_user), -) -> dict: - """Update the current user's default storage backend. - - The backend value is validated against the allowlist before storage. - Returns the updated default_storage_backend value. - """ - if body.backend not in _VALID_BACKENDS: - raise HTTPException( - status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, - detail=f"Invalid backend. Valid values: {sorted(_VALID_BACKENDS)}", - ) - request.state.current_user = current_user - user = await session.get(User, current_user.id) - if user is None: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="User not found") - - user.default_storage_backend = body.backend - session.add(user) - await session.commit() - - return {"default_storage_backend": user.default_storage_backend} diff --git a/backend/api/cloud/schemas.py b/backend/api/cloud/schemas.py new file mode 100644 index 0000000..1066321 --- /dev/null +++ b/backend/api/cloud/schemas.py @@ -0,0 +1,86 @@ +""" +Whitelisted Pydantic response schemas for the cloud API package. + +All schemas are explicit allowlists — credentials_enc, tokens, and passwords +are deliberately absent. T-12-03: credential exclusion by design. +""" +from __future__ import annotations + +from datetime import datetime +from typing import List, Optional + +from pydantic import BaseModel, field_validator + + +# ── Capability / item schemas ───────────────────────────────────────────────── + +class CloudCapabilityOut(BaseModel): + """Whitelisted capability descriptor. reason/message only when not supported.""" + + action: str + state: str # "supported" | "unsupported" | "temporarily_unavailable" + reason: Optional[str] = None + message: Optional[str] = None + + +class CloudItemOut(BaseModel): + """Normalized cloud item metadata. No credentials or byte content.""" + + id: str # DocuVault stable UUID + provider_item_id: str + name: str + kind: str # "file" | "folder" + parent_ref: Optional[str] = None + content_type: Optional[str] = None + size: Optional[int] = None + modified_at: Optional[datetime] = None + etag: Optional[str] = None + capabilities: dict[str, CloudCapabilityOut] = {} + + +# ── Freshness / folder state schemas ───────────────────────────────────────── + +class FolderFreshnessOut(BaseModel): + """Freshness and error state for a browsed folder.""" + + refresh_state: str # "fresh" | "refreshing" | "warning" + last_refreshed_at: Optional[datetime] = None + error_code: Optional[str] = None + error_message: Optional[str] = None + + +# ── Browse response ─────────────────────────────────────────────────────────── + +class CloudBrowseResponse(BaseModel): + """Owner-scoped connection-ID browse response. + + T-12-01: items are always scoped to the resolved connection which is owned + by the requesting user. credentials_enc is never included. + """ + + connection_id: str + provider: str + display_name: str + parent_ref: Optional[str] + items: List[CloudItemOut] + capabilities: dict[str, CloudCapabilityOut] + freshness: FolderFreshnessOut + + +# ── Connection schemas ──────────────────────────────────────────────────────── + +class ConnectionRenameRequest(BaseModel): + """Validated PATCH body for renaming a connection display name. + + Only display_name is accepted — mass assignment prevention. + """ + + display_name: str + + @field_validator("display_name") + @classmethod + def must_be_nonblank(cls, v: str) -> str: + stripped = v.strip() + if not stripped: + raise ValueError("display_name must not be blank") + return stripped diff --git a/backend/db/models.py b/backend/db/models.py index 529e36f..e196325 100644 --- a/backend/db/models.py +++ b/backend/db/models.py @@ -309,6 +309,7 @@ class CloudConnection(Base): ) provider: Mapped[str] = mapped_column(String, nullable=False) display_name: Mapped[str] = mapped_column(Text, nullable=False) + display_name_override: Mapped[Optional[str]] = mapped_column(Text, nullable=True) credentials_enc: Mapped[str] = mapped_column(Text, nullable=False) status: Mapped[str] = mapped_column(String, nullable=False, default="ACTIVE") connected_at: Mapped[datetime] = mapped_column( diff --git a/backend/services/cloud_items.py b/backend/services/cloud_items.py index 02e2df1..c8467cf 100644 --- a/backend/services/cloud_items.py +++ b/backend/services/cloud_items.py @@ -37,17 +37,20 @@ class CloudItemNotFound(ValueError): async def resolve_owned_connection( session: AsyncSession, *, - connection_id: str, - user_id: str, + connection_id, + user_id, ) -> CloudConnection: """Return the CloudConnection owned by user_id or raise ConnectionNotFound. + Accepts UUID objects or string UUIDs for both parameters. Never returns a connection belonging to another user. """ + conn_uuid = connection_id if isinstance(connection_id, uuid.UUID) else uuid.UUID(str(connection_id)) + user_uuid = user_id if isinstance(user_id, uuid.UUID) else uuid.UUID(str(user_id)) result = await session.execute( select(CloudConnection).where( - CloudConnection.id == connection_id, - CloudConnection.user_id == user_id, + CloudConnection.id == conn_uuid, + CloudConnection.user_id == user_uuid, ) ) conn = result.scalars().first() @@ -72,9 +75,11 @@ async def list_cloud_children( parent_ref=None matches items whose parent_ref is NULL (root children where parent is not tracked as a ref string). """ + uid_v = user_id if isinstance(user_id, uuid.UUID) else uuid.UUID(str(user_id)) + cid_v = connection_id if isinstance(connection_id, uuid.UUID) else uuid.UUID(str(connection_id)) stmt = select(CloudItem).where( - CloudItem.user_id == user_id, - CloudItem.connection_id == connection_id, + CloudItem.user_id == uid_v, + CloudItem.connection_id == cid_v, CloudItem.deleted_at.is_(None), ) if parent_ref is None: @@ -103,11 +108,10 @@ async def upsert_cloud_item( user_id is always set from the caller — never from the resource alone — to enforce the owner boundary. """ - connection_id = str(resource.connection_id) - + conn_uuid2 = resource.connection_id if isinstance(resource.connection_id, uuid.UUID) else uuid.UUID(str(resource.connection_id)) result = await session.execute( select(CloudItem).where( - CloudItem.connection_id == connection_id, + CloudItem.connection_id == conn_uuid2, CloudItem.provider_item_id == resource.provider_item_id, ) ) @@ -130,10 +134,12 @@ async def upsert_cloud_item( await session.flush() return existing else: + user_uuid_ins = user_id if isinstance(user_id, uuid.UUID) else uuid.UUID(str(user_id)) + conn_uuid_ins = resource.connection_id if isinstance(resource.connection_id, uuid.UUID) else uuid.UUID(str(resource.connection_id)) item = CloudItem( - id=str(uuid.uuid4()), - user_id=user_id, - connection_id=connection_id, + id=uuid.uuid4(), + user_id=user_uuid_ins, + connection_id=conn_uuid_ins, provider_item_id=resource.provider_item_id, parent_ref=resource.parent_ref, name=resource.name, @@ -181,9 +187,11 @@ async def reconcile_cloud_listing( if listing.complete: # Soft-delete items not present in the complete listing + uid_v2 = user_id if isinstance(user_id, uuid.UUID) else uuid.UUID(str(user_id)) + cid_v2 = connection_id if isinstance(connection_id, uuid.UUID) else uuid.UUID(str(connection_id)) stmt = select(CloudItem).where( - CloudItem.user_id == user_id, - CloudItem.connection_id == connection_id, + CloudItem.user_id == uid_v2, + CloudItem.connection_id == cid_v2, CloudItem.deleted_at.is_(None), ) if parent_ref is None: @@ -217,9 +225,11 @@ async def get_or_create_folder_state( Idempotent: repeated calls with the same arguments return the same row. parent_ref='' represents the connection root. """ + cid_fs = connection_id if isinstance(connection_id, uuid.UUID) else uuid.UUID(str(connection_id)) + uid_fs = user_id if isinstance(user_id, uuid.UUID) else uuid.UUID(str(user_id)) result = await session.execute( select(CloudFolderState).where( - CloudFolderState.connection_id == connection_id, + CloudFolderState.connection_id == cid_fs, CloudFolderState.parent_ref == parent_ref, ) ) @@ -228,9 +238,9 @@ async def get_or_create_folder_state( return existing fs = CloudFolderState( - id=str(uuid.uuid4()), - user_id=user_id, - connection_id=connection_id, + id=uuid.uuid4(), + user_id=uid_fs, + connection_id=cid_fs, parent_ref=parent_ref, refresh_state="fresh", ) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index bc7cc82..ed42797 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -119,6 +119,7 @@ async def db_session(): # UUID(as_uuid=True) renders as CHAR(32) in SQLite — already handled by # SQLAlchemy's built-in UUID type mapping — no patch needed. + _patched_columns: list = [] # kept for finally-block symmetry engine = create_async_engine( "sqlite+aiosqlite:///:memory:", @@ -149,6 +150,9 @@ async def db_session(): del SQLiteTypeCompiler.visit_JSONB # type: ignore except AttributeError: pass + # Restore UUID column types to leave no side effects for other test files + for col, orig_type in _patched_columns: + col.type = orig_type @pytest_asyncio.fixture diff --git a/backend/tests/test_cloud.py b/backend/tests/test_cloud.py index ee279f7..d26aeb7 100644 --- a/backend/tests/test_cloud.py +++ b/backend/tests/test_cloud.py @@ -851,3 +851,233 @@ async def test_oauth_initiate_requires_auth(async_client, db_session): ) assert resp.status_code in (401, 403), \ f"Expected 401 or 403 for unauthenticated request, got {resp.status_code}" + + +# ── Phase 12: Connection-ID browse tests ────────────────────────────────────── + +async def _create_cloud_connection(session, user_id, provider: str = "google_drive", name: str = "My Drive"): + """Create a CloudConnection row for test fixtures.""" + from db.models import CloudConnection + from storage.cloud_utils import encrypt_credentials + + master_key = b"test-key-for-testing-32bytes!!" + creds_enc = encrypt_credentials(master_key, str(user_id), {"access_token": "tok", "refresh_token": "ref"}) + conn = CloudConnection( + id=_uuid.uuid4(), + user_id=user_id, + provider=provider, + display_name=name, + credentials_enc=creds_enc, + status="ACTIVE", + ) + session.add(conn) + await session.commit() + return conn + + +async def test_browse_connection_rejects_foreign_owner(async_client, db_session): + """GET /api/cloud/connections/{id}/items rejects access by a non-owner (T-12-01). + + User2 cannot browse user1's connection — returns 404 (IDOR protection). + """ + auth1 = await _create_user_and_token(db_session, role="user") + auth2 = await _create_user_and_token(db_session, role="user") + + conn = await _create_cloud_connection(db_session, auth1["user"].id) + + resp = await async_client.get( + f"/api/cloud/connections/{conn.id}/items", + headers=auth2["headers"], + ) + assert resp.status_code == 404, f"Expected 404 IDOR block, got {resp.status_code}" + + +async def test_browse_connection_admin_blocked(async_client, db_session): + """GET /api/cloud/connections/{id}/items rejects admin tokens (get_regular_user guard).""" + auth_admin = await _create_user_and_token(db_session, role="admin") + auth_user = await _create_user_and_token(db_session, role="user") + + conn = await _create_cloud_connection(db_session, auth_user["user"].id) + + resp = await async_client.get( + f"/api/cloud/connections/{conn.id}/items", + headers=auth_admin["headers"], + ) + # Admin tokens are blocked by get_regular_user — 403 or 404 + assert resp.status_code in (403, 404), f"Expected 403/404 for admin, got {resp.status_code}" + + +async def test_browse_connection_response_excludes_credentials(async_client, db_session, monkeypatch): + """Browse response never includes credentials_enc or decrypted credential fields (T-12-03).""" + from unittest.mock import AsyncMock, patch + from storage.cloud_base import CloudListing, CloudCapability, STATE_SUPPORTED + + auth = await _create_user_and_token(db_session, role="user") + conn = await _create_cloud_connection(db_session, auth["user"].id) + + # Mock adapter so no real provider call is made + mock_adapter = AsyncMock() + mock_adapter.list_folder = AsyncMock(return_value=CloudListing(items=(), complete=True)) + mock_adapter.get_capabilities = AsyncMock(return_value={ + action: CloudCapability(action=action, state=STATE_SUPPORTED) + for action in ["browse"] + }) + + from storage.cloud_base import ACTIONS, STATE_UNSUPPORTED, REASON_PROVIDER_UNSUPPORTED + caps = { + action: CloudCapability( + action=action, + state=STATE_SUPPORTED if action == "browse" else STATE_UNSUPPORTED, + reason=None if action == "browse" else REASON_PROVIDER_UNSUPPORTED, + message=None if action == "browse" else "Not available.", + ) + for action in ACTIONS + } + mock_adapter.get_capabilities = AsyncMock(return_value=caps) + + with patch("api.cloud.browse.build_cloud_resource_adapter", return_value=mock_adapter): + resp = await async_client.get( + f"/api/cloud/connections/{conn.id}/items", + headers=auth["headers"], + ) + + assert resp.status_code == 200, f"Expected 200, got {resp.status_code}: {resp.text}" + data = resp.json() + + # T-12-03: credential fields must not be in response + assert "credentials_enc" not in data + assert "access_token" not in str(data) + assert "refresh_token" not in str(data) + assert "password" not in str(data) + + # Response schema validation + assert "connection_id" in data + assert "items" in data + assert "capabilities" in data + assert "freshness" in data + + +async def test_browse_two_google_drive_connections_independently(async_client, db_session): + """Two Google Drive connections for one user are independently browsable (D-05).""" + from unittest.mock import AsyncMock, patch + from storage.cloud_base import CloudListing, CloudCapability, ACTIONS, STATE_SUPPORTED, STATE_UNSUPPORTED, REASON_PROVIDER_UNSUPPORTED + + auth = await _create_user_and_token(db_session, role="user") + conn1 = await _create_cloud_connection(db_session, auth["user"].id, provider="google_drive", name="Drive 1") + conn2 = await _create_cloud_connection(db_session, auth["user"].id, provider="google_drive", name="Drive 2") + + caps = { + action: CloudCapability( + action=action, + state=STATE_SUPPORTED if action == "browse" else STATE_UNSUPPORTED, + reason=None if action == "browse" else REASON_PROVIDER_UNSUPPORTED, + message=None if action == "browse" else "Not available.", + ) + for action in ACTIONS + } + mock_adapter = AsyncMock() + mock_adapter.list_folder = AsyncMock(return_value=CloudListing(items=(), complete=True)) + mock_adapter.get_capabilities = AsyncMock(return_value=caps) + + with patch("api.cloud.browse.build_cloud_resource_adapter", return_value=mock_adapter): + resp1 = await async_client.get(f"/api/cloud/connections/{conn1.id}/items", headers=auth["headers"]) + resp2 = await async_client.get(f"/api/cloud/connections/{conn2.id}/items", headers=auth["headers"]) + + assert resp1.status_code == 200 + assert resp2.status_code == 200 + assert resp1.json()["connection_id"] == str(conn1.id) + assert resp2.json()["connection_id"] == str(conn2.id) + + +async def test_rename_connection_display_name(async_client, db_session): + """PATCH /api/cloud/connections/{id} renames display_name only (mass-assignment prevention).""" + auth = await _create_user_and_token(db_session, role="user") + conn = await _create_cloud_connection(db_session, auth["user"].id, name="Original Name") + + resp = await async_client.patch( + f"/api/cloud/connections/{conn.id}", + json={"display_name": "My Work Drive"}, + headers=auth["headers"], + ) + assert resp.status_code == 200, f"Expected 200, got {resp.status_code}: {resp.text}" + data = resp.json() + assert data["display_name"] == "My Work Drive" + + +async def test_rename_connection_rejects_foreign_owner(async_client, db_session): + """PATCH /api/cloud/connections/{id} rejects non-owner (T-12-01 IDOR).""" + auth1 = await _create_user_and_token(db_session, role="user") + auth2 = await _create_user_and_token(db_session, role="user") + conn = await _create_cloud_connection(db_session, auth1["user"].id, name="Private Drive") + + resp = await async_client.patch( + f"/api/cloud/connections/{conn.id}", + json={"display_name": "Hacked"}, + headers=auth2["headers"], + ) + assert resp.status_code == 404 + + +async def test_rename_connection_rejects_blank_name(async_client, db_session): + """PATCH /api/cloud/connections/{id} rejects blank display_name.""" + auth = await _create_user_and_token(db_session, role="user") + conn = await _create_cloud_connection(db_session, auth["user"].id) + + resp = await async_client.patch( + f"/api/cloud/connections/{conn.id}", + json={"display_name": " "}, + headers=auth["headers"], + ) + assert resp.status_code == 422 + + +async def test_rename_connection_rejects_mass_assignment(async_client, db_session): + """PATCH /api/cloud/connections/{id} ignores unknown fields (mass-assignment prevention).""" + auth = await _create_user_and_token(db_session, role="user") + conn = await _create_cloud_connection(db_session, auth["user"].id) + + resp = await async_client.patch( + f"/api/cloud/connections/{conn.id}", + json={"display_name": "OK Name", "credentials_enc": "HACKED", "provider": "evil"}, + headers=auth["headers"], + ) + # Should succeed but only update display_name + assert resp.status_code == 200 + data = resp.json() + # credentials_enc must not appear in response + assert "credentials_enc" not in data + + # Verify provider was not changed in DB + from db.models import CloudConnection as CC + from sqlalchemy import select + result = await db_session.execute(select(CC).where(CC.id == conn.id)) + updated = result.scalar_one() + assert updated.provider == "google_drive" # unchanged + + +async def test_browse_connection_malformed_uuid(async_client, db_session): + """GET /api/cloud/connections/{bad-uuid}/items returns 422 for malformed UUID.""" + auth = await _create_user_and_token(db_session, role="user") + resp = await async_client.get( + "/api/cloud/connections/not-a-valid-uuid/items", + headers=auth["headers"], + ) + assert resp.status_code == 422 + + +async def test_list_connections_returns_all_providers(async_client, db_session): + """GET /api/cloud/connections returns all connections including duplicate providers.""" + auth = await _create_user_and_token(db_session, role="user") + conn1 = await _create_cloud_connection(db_session, auth["user"].id, provider="google_drive", name="Drive A") + conn2 = await _create_cloud_connection(db_session, auth["user"].id, provider="google_drive", name="Drive B") + + resp = await async_client.get("/api/cloud/connections", headers=auth["headers"]) + assert resp.status_code == 200 + items = resp.json()["items"] + assert len(items) >= 2 + ids = {item["id"] for item in items} + assert str(conn1.id) in ids + assert str(conn2.id) in ids + # credentials_enc must not be in any item + for item in items: + assert "credentials_enc" not in item diff --git a/backend/tests/test_cloud_items.py b/backend/tests/test_cloud_items.py index 6ea866f..8865348 100644 --- a/backend/tests/test_cloud_items.py +++ b/backend/tests/test_cloud_items.py @@ -25,10 +25,10 @@ from typing import Optional import pytest import pytest_asyncio -from sqlalchemy import String, Text, event +from sqlalchemy import Text, event from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine from sqlalchemy.pool import StaticPool -from sqlalchemy.dialects.postgresql import UUID, INET, JSONB +from sqlalchemy.dialects.postgresql import UUID from db.models import Base, CloudItem, CloudItemTopic, CloudFolderState, CloudConnection, Topic, User, Quota from storage.cloud_base import CloudListing, CloudResource, CloudCapability, STATE_SUPPORTED @@ -48,69 +48,80 @@ from services.cloud_items import ( @pytest_asyncio.fixture async def db_session(): - """In-memory SQLite session with PostgreSQL-type shims.""" + """In-memory SQLite session with PostgreSQL-type shims. + + Patches INET/JSONB to Text for SQLite compatibility. + UUID(as_uuid=True) is left as-is: SQLAlchemy renders it as CHAR(32) and + the bind processor handles uuid.UUID ↔ 32-char hex automatically. + """ + from sqlalchemy.dialects.sqlite.base import SQLiteTypeCompiler + from sqlalchemy.dialects.postgresql import INET, JSONB + + _orig_visit_INET = getattr(SQLiteTypeCompiler, "visit_INET", None) + _orig_visit_JSONB = getattr(SQLiteTypeCompiler, "visit_JSONB", None) + + def _visit_inet(self, type_, **kw): + return "TEXT" + + def _visit_jsonb(self, type_, **kw): + return "TEXT" + + SQLiteTypeCompiler.visit_INET = _visit_inet # type: ignore[attr-defined] + SQLiteTypeCompiler.visit_JSONB = _visit_jsonb # type: ignore[attr-defined] + engine = create_async_engine( "sqlite+aiosqlite:///:memory:", connect_args={"check_same_thread": False}, poolclass=StaticPool, ) - # Shim PostgreSQL types to SQLite-compatible equivalents - from sqlalchemy import event as sa_event + try: + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all) - @sa_event.listens_for(engine.sync_engine, "connect") - def set_sqlite_pragma(dbapi_connection, connection_record): - cursor = dbapi_connection.cursor() - cursor.execute("PRAGMA foreign_keys=ON") - cursor.close() - - # Patch dialect-specific column types before table creation - import sqlalchemy.dialects.postgresql as pg - _orig_uuid_init = pg.UUID.__init__ - - def _patch_columns(metadata): - for table in metadata.tables.values(): - for col in table.columns: - if isinstance(col.type, pg.UUID): - col.type = String(36) - elif isinstance(col.type, pg.INET): - col.type = String(45) - elif isinstance(col.type, pg.JSONB): - col.type = Text() - - async with engine.begin() as conn: - _patch_columns(Base.metadata) - await conn.run_sync(Base.metadata.create_all) - - session_factory = async_sessionmaker(engine, expire_on_commit=False) - async with session_factory() as session: - yield session - - async with engine.begin() as conn: - await conn.run_sync(Base.metadata.drop_all) - await engine.dispose() + session_factory = async_sessionmaker(engine, expire_on_commit=False) + async with session_factory() as session: + yield session + finally: + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.drop_all) + await engine.dispose() + if _orig_visit_INET is not None: + SQLiteTypeCompiler.visit_INET = _orig_visit_INET # type: ignore + else: + try: + del SQLiteTypeCompiler.visit_INET # type: ignore + except AttributeError: + pass + if _orig_visit_JSONB is not None: + SQLiteTypeCompiler.visit_JSONB = _orig_visit_JSONB # type: ignore + else: + try: + del SQLiteTypeCompiler.visit_JSONB # type: ignore + except AttributeError: + pass # ── Helpers ─────────────────────────────────────────────────────────────────── -def _user_id() -> str: - return str(uuid.uuid4()) +def _user_id() -> uuid.UUID: + return uuid.uuid4() -def _conn_id() -> str: - return str(uuid.uuid4()) +def _conn_id() -> uuid.UUID: + return uuid.uuid4() -def _item_id() -> str: - return str(uuid.uuid4()) +def _item_id() -> uuid.UUID: + return uuid.uuid4() -async def _make_user(session: AsyncSession, user_id: Optional[str] = None) -> str: +async def _make_user(session: AsyncSession, user_id: Optional[uuid.UUID] = None) -> uuid.UUID: uid = user_id or _user_id() u = User( id=uid, - handle=f"user_{uid[:8]}", - email=f"{uid[:8]}@example.com", + handle=f"user_{uid.hex[:8]}", + email=f"{uid.hex[:8]}@example.com", password_hash="hash", role="user", ) @@ -120,8 +131,8 @@ async def _make_user(session: AsyncSession, user_id: Optional[str] = None) -> st async def _make_connection( - session: AsyncSession, user_id: str, conn_id: Optional[str] = None -) -> str: + session: AsyncSession, user_id: uuid.UUID, conn_id: Optional[uuid.UUID] = None +) -> uuid.UUID: cid = conn_id or _conn_id() conn = CloudConnection( id=cid, @@ -137,8 +148,8 @@ async def _make_connection( def _cloud_resource( - connection_id: str, - user_id: str, + connection_id: uuid.UUID, + user_id: uuid.UUID, provider_item_id: str = "item-001", name: str = "test.pdf", kind: str = "file", @@ -149,8 +160,8 @@ def _cloud_resource( return CloudResource( id=uuid.uuid4(), provider_item_id=provider_item_id, - connection_id=uuid.UUID(connection_id), - user_id=uuid.UUID(user_id), + connection_id=connection_id, + user_id=user_id, name=name, kind=kind, parent_ref=parent_ref, @@ -359,7 +370,7 @@ async def test_service_resolve_owned_connection_found(db_session: AsyncSession): cid = await _make_connection(db_session, uid) conn = await resolve_owned_connection(db_session, connection_id=cid, user_id=uid) - assert str(conn.id) == cid + assert conn.id == cid @pytest.mark.asyncio