From 08e8de73706feba899d6d1e5d4b9338e79111bea Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 14 Sep 2026 22:29:26 +0100 Subject: [PATCH] fix: mini connector fixes --- .../Integrations/confluence-connector.mdx | 2 +- .../Integrations/google-drive-connector.mdx | 2 +- .../Integrations/sharepoint-connector.mdx | 2 +- docsgpt/api/connector/routes.py | 17 ++++-- tests/api/test_connector_oauth_security.py | 54 +++++++++++++++++++ 5 files changed, 71 insertions(+), 6 deletions(-) diff --git a/docs/content/Guides/Integrations/confluence-connector.mdx b/docs/content/Guides/Integrations/confluence-connector.mdx index 6b0625ce..d9702fd2 100644 --- a/docs/content/Guides/Integrations/confluence-connector.mdx +++ b/docs/content/Guides/Integrations/confluence-connector.mdx @@ -52,7 +52,7 @@ VITE_CONFLUENCE_CLIENT_ID=your-atlassian-client-id | `CONFLUENCE_CLIENT_ID` | Client ID from your Atlassian OAuth app | Yes | | `CONFLUENCE_CLIENT_SECRET` | Client secret from your Atlassian OAuth app | Yes | | `VITE_CONFLUENCE_CLIENT_ID` | Same Client ID, used by the frontend to show the Confluence option | Yes | -| `CONNECTOR_ALLOWED_ORIGINS` | Comma-separated frontend origins allowed to receive the sign-in result, e.g. `https://docsgpt.example.com`. Not needed when the frontend shares the API origin or runs on `localhost:5173` | When the frontend is on its own origin | +| `CONNECTOR_ALLOWED_ORIGINS` | Comma-separated frontend origins allowed to receive the sign-in result, e.g. `https://docsgpt.example.com`. Not needed when the frontend shares the API origin, or in local dev when the callback is on `localhost`/`127.0.0.1` and the frontend runs on port 5173 | When the frontend is on its own origin | ### Step 5: Restart and Use diff --git a/docs/content/Guides/Integrations/google-drive-connector.mdx b/docs/content/Guides/Integrations/google-drive-connector.mdx index 9dfcbde3..3014c784 100644 --- a/docs/content/Guides/Integrations/google-drive-connector.mdx +++ b/docs/content/Guides/Integrations/google-drive-connector.mdx @@ -50,7 +50,7 @@ VITE_GOOGLE_CLIENT_ID=your-google-client-id | `GOOGLE_CLIENT_ID` | OAuth Client ID from GCP Credentials | Yes | | `GOOGLE_CLIENT_SECRET` | OAuth Client Secret from GCP Credentials | Yes | | `VITE_GOOGLE_CLIENT_ID` | Same Client ID, used by the frontend to show the Google Drive option | Yes | -| `CONNECTOR_ALLOWED_ORIGINS` | Comma-separated frontend origins allowed to receive the sign-in result, e.g. `https://docsgpt.example.com`. Not needed when the frontend shares the API origin or runs on `localhost:5173` | When the frontend is on its own origin | +| `CONNECTOR_ALLOWED_ORIGINS` | Comma-separated frontend origins allowed to receive the sign-in result, e.g. `https://docsgpt.example.com`. Not needed when the frontend shares the API origin, or in local dev when the callback is on `localhost`/`127.0.0.1` and the frontend runs on port 5173 | When the frontend is on its own origin | Make sure to use the same Google Client ID in both backend and frontend configurations. diff --git a/docs/content/Guides/Integrations/sharepoint-connector.mdx b/docs/content/Guides/Integrations/sharepoint-connector.mdx index 0b2a7c7b..58829f38 100644 --- a/docs/content/Guides/Integrations/sharepoint-connector.mdx +++ b/docs/content/Guides/Integrations/sharepoint-connector.mdx @@ -45,7 +45,7 @@ MICROSOFT_TENANT_ID=your-azure-ad-tenant-id | `MICROSOFT_CLIENT_SECRET` | Client secret value | Yes | — | | `MICROSOFT_TENANT_ID` | Directory (tenant) ID | No | `common` | | `MICROSOFT_AUTHORITY` | Login endpoint override | No | Auto-constructed | -| `CONNECTOR_ALLOWED_ORIGINS` | Comma-separated frontend origins allowed to receive the sign-in result, e.g. `https://docsgpt.example.com` | When the frontend is on its own origin | API origin and `localhost:5173` | +| `CONNECTOR_ALLOWED_ORIGINS` | Comma-separated frontend origins allowed to receive the sign-in result, e.g. `https://docsgpt.example.com` | When the frontend is on its own origin | API origin; also `localhost:5173` when the callback is on `localhost`/`127.0.0.1` | `MICROSOFT_TENANT_ID=common` (the default) allows any Microsoft account to authenticate. Set this to your specific tenant ID in production. diff --git a/docsgpt/api/connector/routes.py b/docsgpt/api/connector/routes.py index cd32f7d6..cbb87917 100644 --- a/docsgpt/api/connector/routes.py +++ b/docsgpt/api/connector/routes.py @@ -81,8 +81,9 @@ def connector_allowed_origins(request_host_url: str) -> list[str]: if callback_origin: callback = urlsplit(callback_origin) if callback.hostname in _LOOPBACK_HOSTS: - for port in (_DEV_FRONTEND_PORT, callback.port or 80): - candidates += [f"http://localhost:{port}", f"http://127.0.0.1:{port}"] + callback_port = f":{callback.port}" if callback.port else "" + for host in ("localhost", "127.0.0.1"): + candidates += [f"http://{host}:{_DEV_FRONTEND_PORT}", f"{callback.scheme}://{host}{callback_port}"] origins: list[str] = [] for candidate in candidates: origin = _origin_of(candidate) @@ -101,6 +102,8 @@ def _render_callback_page( ): """Popup page that reports an OAuth result to the opener on allowed origins only.""" status = status if status in ("success", "error", "cancelled") else "error" + # Without a token there is no result to report, so post to no one. + target_origins = connector_allowed_origins(request.host_url) if session_token else [] provider = html.escape(provider_raw.replace("_", " ").title()) connected_as = ( f"

Connected as: {html.escape(user_email)}

" if status == "success" and user_email else "" @@ -127,7 +130,7 @@ def _render_callback_page( const sessionToken = {_js_literal(session_token)}; const userEmail = {_js_literal(user_email)}; const providerType = {_js_literal(provider_raw)}; - const targetOrigins = {_js_literal(connector_allowed_origins(request.host_url))}; + const targetOrigins = {_js_literal(target_origins)}; if (status === "success" && window.opener) {{ const payload = {{ @@ -196,6 +199,14 @@ class ConnectorAuth(Resource): auth = ConnectorCreator.create_auth(provider) authorization_url = auth.get_authorization_url(state=state) + # The popup drops results for origins outside the allowlist, which the + # user only sees as a cancelled sign-in; name the missing origin here. + request_origin = _origin_of(request.headers.get("Origin")) + if request_origin and request_origin not in connector_allowed_origins(request.host_url): + current_app.logger.warning( + f"Connector sign-in requested from {request_origin}, which cannot receive the result; " + "add it to CONNECTOR_ALLOWED_ORIGINS" + ) return make_response(jsonify({ "success": True, "authorization_url": authorization_url, diff --git a/tests/api/test_connector_oauth_security.py b/tests/api/test_connector_oauth_security.py index e4572779..897933e3 100644 --- a/tests/api/test_connector_oauth_security.py +++ b/tests/api/test_connector_oauth_security.py @@ -2,6 +2,7 @@ import base64 import json +import logging from contextlib import contextmanager from unittest.mock import MagicMock, patch @@ -82,6 +83,19 @@ class TestConnectorAllowedOrigins: assert "http://127.0.0.1:5173" in local assert "http://localhost:5173" not in public + def test_loopback_aliases_keep_callback_scheme_and_port(self): + from docsgpt.api.connector.routes import connector_allowed_origins + from docsgpt.core.settings import settings + + with patch.object(settings, "CONNECTOR_ALLOWED_ORIGINS", None), \ + patch.object(settings, "OIDC_FRONTEND_URL", None), \ + patch.object(settings, "CONNECTOR_REDIRECT_BASE_URI", "https://localhost/api/connectors/callback"): + origins = connector_allowed_origins("https://localhost/") + + assert "https://127.0.0.1" in origins + assert "http://localhost" not in origins + assert "http://127.0.0.1" not in origins + class TestCallbackStatusPage: def test_never_posts_to_wildcard_origin(self, app): @@ -108,6 +122,15 @@ class TestCallbackStatusPage: assert "attacker-supplied" not in body assert "evil@example.com" not in body + def test_tokenless_success_posts_to_no_origin(self, app): + from docsgpt.api.connector.routes import ConnectorCallbackStatus + + with app.test_request_context( + "/api/connectors/callback-status?status=success&provider=google_drive" + ): + r = ConnectorCallbackStatus().get() + assert "const targetOrigins = [];" in r.get_data(as_text=True) + class TestCallbackDeliversTokenSafely: def test_success_renders_page_without_redirecting_token(self, app, pg_conn): @@ -171,6 +194,37 @@ class TestAuthUrlReportsCallbackOrigin: assert r.status_code == 200 assert r.json["callback_origin"] == "https://api.example.com" + @pytest.mark.parametrize( + "origin, warns", + [("https://app.example.com", True), ("https://api.example.com", False), (None, False)], + ) + def test_warns_when_requesting_origin_cannot_receive_result(self, app, pg_conn, caplog, origin, warns): + from docsgpt.api.connector.routes import ConnectorAuth + from docsgpt.core.settings import settings + + fake_auth = MagicMock() + fake_auth.get_authorization_url.return_value = "https://ex/auth?state=x" + headers = {"Origin": origin} if origin else {} + + with _patch_db(pg_conn), patch( + "docsgpt.api.connector.routes.ConnectorCreator.is_supported", return_value=True, + ), patch( + "docsgpt.api.connector.routes.ConnectorCreator.create_auth", return_value=fake_auth, + ), patch.object(settings, "CONNECTOR_ALLOWED_ORIGINS", None), patch.object( + settings, "OIDC_FRONTEND_URL", None, + ), patch.object( + settings, "CONNECTOR_REDIRECT_BASE_URI", "https://api.example.com/api/connectors/callback", + ), app.test_request_context( + "/api/connectors/auth?provider=google_drive", headers=headers, base_url="https://api.example.com", + ), caplog.at_level(logging.WARNING): + from flask import request + request.decoded_token = {"sub": "u-auth-warn"} + r = ConnectorAuth().get() + + assert r.status_code == 200 + warned = any("CONNECTOR_ALLOWED_ORIGINS" in rec.getMessage() for rec in caplog.records) + assert warned is warns + class TestDisconnectOwnership: def test_requires_authentication(self, app, pg_conn):