diff --git a/SECURITY_AUDIT.md b/SECURITY_AUDIT.md index 3f63b48e..ed9d6278 100644 --- a/SECURITY_AUDIT.md +++ b/SECURITY_AUDIT.md @@ -9,6 +9,15 @@ This document tracks security vulnerabilities found in DocuElevate and their rem ## Recent Security Fixes +### Insecure API Endpoint Exposing Integration Credentials ✅ FIXED +**Severity:** HIGH + +**Issue:** The endpoint `GET /api/integrations/{integration_id}/credentials` exposed integration credentials (e.g. passwords, API keys) in plaintext over the API. Although requiring login, this allowed anyone with an active user session to extract the raw credentials. The frontend used this endpoint for testing integration connections. + +**Remediation:** +- Removed the `/credentials` endpoint entirely. +- Added a new `POST /api/integrations/{integration_id}/test` endpoint that securely runs connection tests server-side without returning the decrypted credentials to the client. + ### CVE-2023-36464: PyPDF2/pypdf Infinite Loop Vulnerability ✅ FIXED (2026-02-12) **Severity:** Moderate (CVSS: 5.5) diff --git a/app/api/integrations.py b/app/api/integrations.py index 8d2d9209..9e7cc5a0 100644 --- a/app/api/integrations.py +++ b/app/api/integrations.py @@ -455,31 +455,6 @@ def delete_integration( logger.info("User %s deleted integration %d", owner_id, integration_id) -@router.get("/{integration_id}/credentials", summary="Retrieve decrypted credentials for an integration") -def get_integration_credentials( - integration_id: int, - request: Request, - db: DbSession, - owner_id: CurrentOwner, -) -> dict[str, Any]: - """Return the decrypted credentials dict for a saved integration. - - This endpoint is intended for internal use by background tasks that need - to authenticate with a third-party service. Treat the response as - sensitive — it contains plaintext secrets. - """ - integration = ( - db.query(UserIntegration) - .filter(UserIntegration.id == integration_id, UserIntegration.owner_id == owner_id) - .first() - ) - if not integration: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Integration not found") - - credentials = _decode_credentials(integration.credentials) - return {"credentials": credentials or {}} - - # --------------------------------------------------------------------------- # Connection test helpers # --------------------------------------------------------------------------- @@ -608,6 +583,36 @@ _CONNECTION_TESTERS: dict[str, Any] = { # --------------------------------------------------------------------------- +@router.post("/{integration_id}/test", summary="Test a saved integration connection") +def test_saved_integration_connection( + integration_id: int, + request: Request, + db: DbSession, + owner_id: CurrentOwner, +) -> dict[str, Any]: + """Test connection for an already-saved integration.""" + integration = ( + db.query(UserIntegration) + .filter(UserIntegration.id == integration_id, UserIntegration.owner_id == owner_id) + .first() + ) + if not integration: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Integration not found") + + tester = _CONNECTION_TESTERS.get(integration.integration_type) + if tester is None: + return { + "success": False, + "message": f"Connection testing is not yet supported for '{integration.integration_type}'. " + "The integration can still be saved and will be validated on first use.", + } + + config = json.loads(integration.config) if integration.config else {} + credentials = _decode_credentials(integration.credentials) or {} + + return tester(config, credentials) + + @router.post("/test", summary="Test an integration connection without saving") def test_integration_connection( request: Request, diff --git a/frontend/templates/integrations_dashboard.html b/frontend/templates/integrations_dashboard.html index 335b6745..065a5412 100644 --- a/frontend/templates/integrations_dashboard.html +++ b/frontend/templates/integrations_dashboard.html @@ -1379,27 +1379,15 @@ function integrationsDashboard() { async testSavedIntegration(intg) { this.testingId = intg.id; try { - // Retrieve saved credentials to test - const credsResp = await fetch(`/api/integrations/${intg.id}/credentials`); - if (!credsResp.ok) { - this.showAlert('error', 'Test Failed', 'Could not retrieve saved credentials for testing.'); - return; - } - const creds = await credsResp.json(); - const resp = await fetch('/api/integrations/test', { + const resp = await fetch(`/api/integrations/${intg.id}/test`, { method: 'POST', headers: authHeaders(true), - body: JSON.stringify({ - integration_type: intg.integration_type, - config: intg.config, - credentials: creds, - }), }); const data = await resp.json(); - if (data.success) { + if (resp.ok && data.success) { this.showAlert('success', `${intg.name}: Connection OK`, data.message); } else { - this.showAlert('error', `${intg.name}: Connection Failed`, data.message); + this.showAlert('error', `${intg.name}: Connection Failed`, data.message || 'Connection test failed.'); } } catch (err) { this.showAlert('error', 'Test Failed', `Network error: ${err.message || 'Unknown error'}`); diff --git a/revert.sh b/revert.sh new file mode 100644 index 00000000..29e9f45b --- /dev/null +++ b/revert.sh @@ -0,0 +1,2 @@ +#!/bin/bash +echo "I have reset my repository somehow. Applying fixes again." diff --git a/tests/test_api_integrations.py b/tests/test_api_integrations.py index 90d1c1f3..9c2a0c88 100644 --- a/tests/test_api_integrations.py +++ b/tests/test_api_integrations.py @@ -403,32 +403,33 @@ class TestDeleteIntegration: @pytest.mark.integration -class TestGetIntegrationCredentials: - """Tests for GET /api/integrations/{id}/credentials.""" +class TestTestSavedIntegrationConnection: + """Tests for POST /api/integrations/{id}/test.""" + + @patch("app.api.integrations._CONNECTION_TESTERS") + def test_test_saved_integration_success(self, mock_testers, int_client): + """Test a saved integration successfully.""" + mock_tester = MagicMock(return_value={"success": True, "message": "OK"}) + mock_testers.get.return_value = mock_tester - def test_returns_decrypted_credentials(self, int_client): - """Credentials endpoint returns the decrypted dict.""" created = int_client.post("/api/integrations/", json=_IMAP_SOURCE).json() - resp = int_client.get(f"/api/integrations/{created['id']}/credentials") - assert resp.status_code == 200 - creds = resp.json()["credentials"] - assert creds["password"] == "s3cr3t" # noqa: S105 + resp = int_client.post(f"/api/integrations/{created['id']}/test") - def test_returns_empty_dict_when_no_credentials(self, int_client): - """No credentials stored returns empty dict.""" - payload = dict(_IMAP_SOURCE, credentials=None) - created = int_client.post("/api/integrations/", json=payload).json() - resp = int_client.get(f"/api/integrations/{created['id']}/credentials") assert resp.status_code == 200 - assert resp.json()["credentials"] == {} + assert resp.json()["success"] is True + mock_testers.get.assert_called_with("IMAP") + mock_tester.assert_called_once() + args = mock_tester.call_args[0] + assert args[0]["host"] == "imap.gmail.com" + assert args[1]["password"] == "s3cr3t" def test_not_found(self, int_client): """Non-existent integration returns 404.""" - resp = int_client.get("/api/integrations/9999/credentials") + resp = int_client.post("/api/integrations/9999/test") assert resp.status_code == 404 - def test_other_users_credentials_returns_404(self, int_client, int_session): - """Cannot retrieve another user's credentials.""" + def test_other_users_integration_returns_404(self, int_client, int_session): + """Cannot test another user's integration.""" other_integration = UserIntegration( owner_id=_OTHER_OWNER, direction="SOURCE", @@ -439,7 +440,7 @@ class TestGetIntegrationCredentials: ) int_session.add(other_integration) int_session.commit() - resp = int_client.get(f"/api/integrations/{other_integration.id}/credentials") + resp = int_client.post(f"/api/integrations/{other_integration.id}/test") assert resp.status_code == 404