feat(12-06): connection-ID native lifecycle — always INSERT, PUT credentials, connectionsFor multi-account

- Replace _upsert_cloud_connection with _insert_cloud_connection (always inserts new UUID row) - Add PUT /connections/{id}/credentials endpoint (owner-scoped, SSRF + health-check, password-preserve) - SettingsCloudTab: connectionsFor() renders all same-provider connections with Add account row - CloudCredentialModal.submit: calls updateWebDavCredentials on edit, connectWebDav on create - utils.js: FastAPI validation arrays normalised to concise field messages (Rule 2) - Backend tests: same-provider independence + IDOR negative test for credential update
2026-06-21 22:29:59 +02:00
parent 97c30c3a15
commit 731b65ecdd
7 changed files with 405 additions and 192 deletions
@@ -106,38 +106,29 @@ def _webdav_credentials(body: WebDAVConnectRequest) -> dict[str, str]:
    }


-async def _upsert_cloud_connection(
+async def _insert_cloud_connection(
    session: AsyncSession,
    user_id: uuid.UUID,
    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,
-            CloudConnection.provider == provider,
-        )
+    """Always insert a new connection row — never merges same-provider rows."""
+    conn = CloudConnection(
+        id=uuid.uuid4(),
+        user_id=user_id,
+        provider=provider,
+        display_name=_DISPLAY_NAMES.get(provider, provider),
+        credentials_enc=credentials_enc,
+        status="ACTIVE",
    )
-    conn = result.scalar_one_or_none()
-
-    if conn is not None:
-        conn.credentials_enc = credentials_enc
-        conn.status = "ACTIVE"
-    else:
-        conn = CloudConnection(
-            id=uuid.uuid4(),
-            user_id=user_id,
-            provider=provider,
-            display_name=_DISPLAY_NAMES.get(provider, provider),
-            credentials_enc=credentials_enc,
-            status="ACTIVE",
-        )
-        session.add(conn)
-
+    session.add(conn)
    return conn


+# Keep legacy alias used by OAuth callback (OAuth creates one connection per flow)
+_upsert_cloud_connection = _insert_cloud_connection
+
+
 async def _get_owned_connection(
    session: AsyncSession,
    connection_id: uuid.UUID,
@@ -424,7 +415,7 @@ async def connect_webdav(

    credentials_enc = encrypt_credentials(_master_key(), str(current_user.id), credentials)

-    conn = await _upsert_cloud_connection(session, current_user.id, body.provider, credentials_enc)
+    conn = await _insert_cloud_connection(session, current_user.id, body.provider, credentials_enc)
    await session.flush()

    _ip = get_client_ip(request)
@@ -492,6 +483,102 @@ async def get_connection_config(
    }


+class WebDAVCredentialUpdateRequest(BaseModel):
+    server_url: Optional[str] = None
+    username: Optional[str] = None
+    password: Optional[str] = None  # omit to keep existing
+
+
+@router.put("/connections/{connection_id}/credentials", status_code=status.HTTP_200_OK)
+@account_limiter.limit("100/minute")
+async def update_webdav_credentials(
+    connection_id: uuid.UUID,
+    body: WebDAVCredentialUpdateRequest,
+    request: Request,
+    session: AsyncSession = Depends(get_db),
+    current_user: User = Depends(get_regular_user),
+) -> dict:
+    """Update credentials for an existing WebDAV/Nextcloud connection (owner-scoped).
+
+    Only changes the fields provided. Omitting password preserves the existing secret.
+    Re-validates the URL (SSRF gate) and runs a health-check before committing.
+    T-12-06-01: owner scope via _get_owned_connection.
+    T-12-06-04: re-runs validate_cloud_url and health check before storing.
+    """
+    request.state.current_user = current_user
+    conn = await _get_owned_connection(session, connection_id, current_user.id)
+
+    if conn.provider not in VALID_WEBDAV_PROVIDERS:
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail="Credential update is only available for WebDAV/Nextcloud connections",
+        )
+
+    # Decrypt current credentials so we can merge with the submitted partial update
+    try:
+        current_creds = _decrypt_connection(conn, current_user.id)
+    except Exception:
+        raise HTTPException(
+            status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
+            detail="Failed to decrypt existing credentials",
+        )
+
+    merged = {
+        "server_url": body.server_url if body.server_url is not None else current_creds.get("server_url", ""),
+        "username": body.username if body.username is not None else current_creds.get("username", ""),
+        "password": body.password if body.password else current_creds.get("password", ""),
+    }
+
+    try:
+        validate_cloud_url(merged["server_url"])
+    except ValueError as exc:
+        raise HTTPException(
+            status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+            detail=f"Invalid server URL: {exc}",
+        ) from exc
+
+    try:
+        backend = build_cloud_backend(conn.provider, merged)
+    except ValueError as exc:
+        raise HTTPException(
+            status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+            detail=f"Invalid credentials: {exc}",
+        ) from exc
+
+    try:
+        ok = await backend.health_check()
+        if not ok:
+            raise HTTPException(
+                status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+                detail="Connection test failed — check server URL and credentials",
+            )
+    except HTTPException:
+        raise
+    except Exception as exc:
+        raise HTTPException(
+            status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+            detail=f"Connection test failed: {exc}",
+        ) from exc
+
+    conn.credentials_enc = encrypt_credentials(_master_key(), str(current_user.id), merged)
+    conn.status = "ACTIVE"
+
+    _ip = get_client_ip(request)
+    await write_audit_log(
+        session,
+        event_type="cloud.credentials_updated",
+        user_id=current_user.id,
+        actor_id=current_user.id,
+        resource_id=conn.id,
+        ip_address=_ip,
+        metadata_={"provider": conn.provider},
+    )
+    await session.commit()
+    await session.refresh(conn)
+
+    return CloudConnectionOut.model_validate(conn).model_dump()
+
+
@router.patch("/connections/{connection_id}", status_code=status.HTTP_200_OK)
@account_limiter.limit("100/minute")
 async def rename_connection(
@@ -328,6 +328,64 @@ async def test_nextcloud_connect_persists(async_client, db_session):
    assert "credentials_enc" not in data


+async def test_same_provider_connections_are_independent(async_client, db_session):
+    """Two POSTs for nextcloud create two distinct UUIDs with independent credentials.
+
+    Regression for Phase 12 UAT gap: _upsert_cloud_connection previously merged
+    same-provider connections into one row, preventing multi-account setups.
+    """
+    from unittest.mock import patch, AsyncMock
+
+    auth = await _create_user_and_token(db_session, role="user")
+
+    with patch("api.cloud.connections.validate_cloud_url", return_value=None), \
+         patch("storage.webdav_backend.validate_cloud_url", return_value=None), \
+         patch("storage.nextcloud_backend.validate_cloud_url", return_value=None), \
+         patch("asyncio.to_thread", new_callable=AsyncMock, return_value=True):
+
+        resp1 = await async_client.post(
+            "/api/cloud/connections/webdav",
+            json={"server_url": "https://nc1.example.com/remote.php/dav", "username": "alice", "password": "pass1", "provider": "nextcloud"},
+            headers=auth["headers"],
+        )
+        resp2 = await async_client.post(
+            "/api/cloud/connections/webdav",
+            json={"server_url": "https://nc2.example.com/remote.php/dav", "username": "bob", "password": "pass2", "provider": "nextcloud"},
+            headers=auth["headers"],
+        )
+
+    assert resp1.status_code == 201, resp1.text
+    assert resp2.status_code == 201, resp2.text
+    id1 = resp1.json()["id"]
+    id2 = resp2.json()["id"]
+    assert id1 != id2, "Two same-provider connections must produce distinct UUIDs"
+
+    # Both are returned in the list
+    list_resp = await async_client.get("/api/cloud/connections", headers=auth["headers"])
+    assert list_resp.status_code == 200
+    ids = [c["id"] for c in list_resp.json()["items"]]
+    assert id1 in ids and id2 in ids
+
+
+async def test_credential_update_wrong_owner_returns_404(async_client, db_session, cloud_connection_factory):
+    """PUT /connections/{id}/credentials with another user's connection returns 404.
+
+    T-12-06-01: IDOR protection — owner check via _get_owned_connection.
+    """
+    owner = await _create_user_and_token(db_session, role="user")
+    attacker = await _create_user_and_token(db_session, role="user")
+
+    conn = await cloud_connection_factory(db_session, owner["user"].id, provider="nextcloud")
+
+    resp = await async_client.put(
+        f"/api/cloud/connections/{conn.id}/credentials",
+        json={"server_url": "https://evil.example.com/dav", "username": "hacker", "password": "x"},
+        headers=attacker["headers"],
+    )
+    assert resp.status_code == 404
+    assert "credentials_enc" not in resp.text
+
+
 async def test_webdav_connect_validates(async_client, db_session, monkeypatch):
    """POST /api/cloud/connections/webdav with localhost URL returns 422 (SSRF blocked)."""
    auth = await _create_user_and_token(db_session, role="user")