fix: mini connector fixes

This commit is contained in:
Alex committed 2026-09-14 22:29:26 +01:00
1 parent 41b3afed14
commit 08e8de7370
5 files changed
+71 -6

No files matched your search

@@ -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
@@ -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 |
<Callout type="warning" emoji="⚠️">
Make sure to use the same Google Client ID in both backend and frontend configurations.
@@ -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` |
<Callout type="warning">
`MICROSOFT_TENANT_ID=common` (the default) allows any Microsoft account to authenticate. Set this to your specific tenant ID in production.
+14 -3
View File
@@ -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"<p>Connected as: {html.escape(user_email)}</p>" 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,
@@ -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):