fix: more mini connector hardening

docsgpt/api/connector/routes.py

@@ -24,6 +24,7 @@ from docsgpt.core.settings import settings
 from docsgpt.parser.connectors.connector_creator import ConnectorCreator
 from docsgpt.storage.db.repositories.connector_sessions import (
     ConnectorSessionsRepository,
+    owns_connector_session,
 )
 from docsgpt.storage.db.repositories.sources import SourcesRepository
 from docsgpt.storage.db.session import db_readonly, db_session
@@ -102,8 +103,16 @@ 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 []
+    # The script only carries server-side values: the provider key comes from the
+    # supported-connector list rather than the request, and nothing posts without a token.
+    provider_key = next(
+        (key for key in ConnectorCreator.get_supported_connectors() if key == provider_raw.lower()), None,
+    )
+    payload = (
+        {"type": f"{provider_key}_auth_success", "session_token": session_token, "user_email": user_email}
+        if status == "success" and session_token and provider_key else None
+    )
+    target_origins = connector_allowed_origins(request.host_url) if payload 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 ""
@@ -126,26 +135,15 @@ def _render_callback_page(
         </style>
         <script>
             window.onload = function() {{
-                const status = {_js_literal(status)};
-                const sessionToken = {_js_literal(session_token)};
-                const userEmail = {_js_literal(user_email)};
-                const providerType = {_js_literal(provider_raw)};
+                const payload = {_js_literal(payload)};
                 const targetOrigins = {_js_literal(target_origins)};
 
-                if (status === "success" && window.opener) {{
-                    const payload = {{
-                        type: providerType + '_auth_success',
-                        session_token: sessionToken,
-                        user_email: userEmail
-                    }};
+                if (payload && window.opener) {{
                     targetOrigins.forEach(function(origin) {{
                         window.opener.postMessage(payload, origin);
                     }});
-
-                    setTimeout(() => window.close(), 3000);
-                }} else if (status === "cancelled" || status === "error") {{
-                    setTimeout(() => window.close(), 3000);
                 }}
+                setTimeout(() => window.close(), 3000);
             }};
         </script>
     </head>
@@ -357,7 +355,7 @@ class ConnectorFiles(Resource):
                 session = ConnectorSessionsRepository(conn).get_by_session_token(
                     session_token,
                 )
-            if not session or session.get("user_id") != user:
+            if not owns_connector_session(session, user, provider):
                 return make_response(jsonify({"success": False, "error": "Invalid or unauthorized session"}), 401)
 
             loader = ConnectorCreator.create_connector(provider, session_token)
@@ -426,7 +424,7 @@ class ConnectorValidateSession(Resource):
                 session = ConnectorSessionsRepository(conn).get_by_session_token(
                     session_token,
                 )
-            if not session or session.get("user_id") != user or not session.get("token_info"):
+            if not owns_connector_session(session, user, provider) or not session.get("token_info"):
                 return make_response(jsonify({"success": False, "error": "Invalid or expired session"}), 401)
 
             token_info = session["token_info"]
@@ -563,7 +561,7 @@ class ConnectorSync(Resource):
 
             with db_readonly() as conn:
                 session = ConnectorSessionsRepository(conn).get_by_session_token(session_token)
-            if not session or session.get("user_id") != user_id:
+            if not owns_connector_session(session, user_id, source_type):
                 return make_response(
                     jsonify({"success": False, "error": "Invalid or unauthorized session"}),
                     401,

docsgpt/api/user/sources/upload.py

@@ -24,7 +24,10 @@ from docsgpt.security.zip_archive import (
     ZipExtractionError,
     ZipExtractionLimits,
 )
-from docsgpt.storage.db.repositories.connector_sessions import ConnectorSessionsRepository
+from docsgpt.storage.db.repositories.connector_sessions import (
+    ConnectorSessionsRepository,
+    owns_connector_session,
+)
 from docsgpt.storage.db.repositories.idempotency import IdempotencyRepository
 from docsgpt.storage.db.repositories.sources import SourcesRepository
 from docsgpt.storage.db.source_config import SourceConfig
@@ -499,7 +502,7 @@ class UploadRemote(Resource):
                     )
                 with db_readonly() as conn:
                     connector_session = ConnectorSessionsRepository(conn).get_by_session_token(session_token)
-                if not connector_session or connector_session.get("user_id") != user:
+                if not owns_connector_session(connector_session, user, data["source"]):
                     if scoped_key:
                         _release_claim(scoped_key)
                     return make_response(

docsgpt/storage/db/repositories/connector_sessions.py

@@ -40,6 +40,16 @@ def _jsonb(value: Any) -> Any:
     return json.dumps(value, cls=PGNativeJSONEncoder)
 
 
+def owns_connector_session(session: Optional[dict], user_id: str, provider: Optional[str]) -> bool:
+    """Whether ``session`` belongs to ``user_id`` and was issued for ``provider`` (case-insensitive)."""
+    return bool(
+        session
+        and session.get("user_id") == user_id
+        and provider
+        and (session.get("provider") or "").lower() == provider.lower()
+    )
+
+
 class ConnectorSessionsRepository:
     def __init__(self, conn: Connection) -> None:
         self._conn = conn

tests/api/test_connector_oauth_security.py

@@ -29,12 +29,15 @@ def _encode_state(payload):
     return base64.urlsafe_b64encode(json.dumps(payload).encode()).decode()
 
 
-def _seed_session(pg_conn, user, token, provider="google_drive"):
+def _seed_session(pg_conn, user, token, provider="google_drive", token_info=None):
     from docsgpt.storage.db.repositories.connector_sessions import ConnectorSessionsRepository
 
     repo = ConnectorSessionsRepository(pg_conn)
     row = repo.upsert(user, provider, status="authorized")
-    repo.update(str(row["id"]), {"session_token": token})
+    patch_fields = {"session_token": token}
+    if token_info:
+        patch_fields["token_info"] = token_info
+    repo.update(str(row["id"]), patch_fields)
     return repo
 
 
@@ -131,6 +134,17 @@ class TestCallbackStatusPage:
             r = ConnectorCallbackStatus().get()
         assert "const targetOrigins = [];" in r.get_data(as_text=True)
 
+    def test_request_provider_never_reaches_inline_script(self, app):
+        from docsgpt.api.connector.routes import ConnectorCallbackStatus
+
+        with app.test_request_context(
+            "/api/connectors/callback-status?status=error&provider=zz-injected-provider"
+        ):
+            r = ConnectorCallbackStatus().get()
+        body = r.get_data(as_text=True)
+        script = body[body.index("<script>"):body.index("</script>")]
+        assert "zz-injected-provider" not in script
+
 
 class TestCallbackDeliversTokenSafely:
     def test_success_renders_page_without_redirecting_token(self, app, pg_conn):
@@ -166,6 +180,7 @@ class TestCallbackDeliversTokenSafely:
         assert r.status_code == 200
         assert "Location" not in r.headers
         assert token in body
+        assert '"type": "google_drive_auth_success"' in body
         assert '"https://app.example.com"' in body
         assert "'*'" not in body
         assert r.headers["Cache-Control"] == "no-store"
@@ -334,3 +349,88 @@ class TestRemoteUploadSessionOwnership:
 
         assert r.status_code == 200
         apply_mock.assert_called_once()
+
+    def test_rejects_session_issued_for_another_provider(self, app, pg_conn):
+        _seed_session(pg_conn, "u-prov-up", "st-prov-up", provider="share_point")
+        apply_mock = MagicMock(return_value=MagicMock(id="t"))
+
+        r = self._post(app, pg_conn, "u-prov-up", "st-prov-up", apply_mock)
+
+        assert r.status_code == 401
+        apply_mock.assert_not_called()
+
+
+class TestSessionProviderBinding:
+    def _files(self, app, pg_conn, user, provider, token, create_connector):
+        from docsgpt.api.connector.routes import ConnectorFiles
+
+        with _patch_db(pg_conn), patch(
+            "docsgpt.api.connector.routes.ConnectorCreator.create_connector", create_connector,
+        ), app.test_request_context(
+            "/api/connectors/files", method="POST", json={"provider": provider, "session_token": token},
+        ):
+            from flask import request
+            request.decoded_token = {"sub": user}
+            return ConnectorFiles().post()
+
+    def test_files_rejects_session_issued_for_another_provider(self, app, pg_conn):
+        _seed_session(pg_conn, "u-files-prov", "st-files-prov", provider="google_drive")
+        create_connector = MagicMock()
+
+        r = self._files(app, pg_conn, "u-files-prov", "share_point", "st-files-prov", create_connector)
+
+        assert r.status_code == 401
+        create_connector.assert_not_called()
+
+    def test_files_matches_provider_case_insensitively(self, app, pg_conn):
+        _seed_session(pg_conn, "u-files-case", "st-files-case", provider="google_drive")
+        create_connector = MagicMock(return_value=MagicMock(load_data=MagicMock(return_value=[]), next_page_token=None))
+
+        r = self._files(app, pg_conn, "u-files-case", "Google_Drive", "st-files-case", create_connector)
+
+        assert r.status_code == 200
+        create_connector.assert_called_once()
+
+    def test_validate_session_rejects_session_issued_for_another_provider(self, app, pg_conn):
+        from docsgpt.api.connector.routes import ConnectorValidateSession
+
+        _seed_session(
+            pg_conn, "u-val-prov", "st-val-prov", provider="google_drive", token_info={"access_token": "at"},
+        )
+        create_auth = MagicMock()
+        with _patch_db(pg_conn), patch(
+            "docsgpt.api.connector.routes.ConnectorCreator.create_auth", create_auth,
+        ), app.test_request_context(
+            "/api/connectors/validate-session", method="POST",
+            json={"provider": "share_point", "session_token": "st-val-prov"},
+        ):
+            from flask import request
+            request.decoded_token = {"sub": "u-val-prov"}
+            r = ConnectorValidateSession().post()
+
+        assert r.status_code == 401
+        create_auth.assert_not_called()
+
+    def test_sync_rejects_session_issued_for_another_provider(self, app, pg_conn):
+        from docsgpt.api.connector.routes import ConnectorSync
+        from docsgpt.storage.db.repositories.sources import SourcesRepository
+
+        user = "u-sync-prov"
+        _seed_session(pg_conn, user, "st-sync-prov", provider="share_point")
+        src = SourcesRepository(pg_conn).create(
+            "drive-src", user_id=user,
+            remote_data={"provider": "google_drive", "file_ids": ["f"], "folder_ids": []},
+        )
+        delay = MagicMock()
+        with _patch_db(pg_conn), patch(
+            "docsgpt.api.connector.routes.ingest_connector_task.delay", delay,
+        ), app.test_request_context(
+            "/api/connectors/sync", method="POST",
+            json={"source_id": str(src["id"]), "session_token": "st-sync-prov"},
+        ):
+            from flask import request
+            request.decoded_token = {"sub": user}
+            r = ConnectorSync().post()
+
+        assert r.status_code == 401
+        delay.assert_not_called()