fix: forward end-user JWT to MCP/API callers via gateway-placed X-OpenRAG-API-JWT header#1874
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds ChangesSecondary JWT Header and SaaS+RBAC Fail-Fast Authentication
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/config/settings.py`:
- Around line 273-290: The get_api_jwt_header() function does not normalize
empty or whitespace values of the OPENRAG_API_JWT_HEADER environment variable to
the default fallback. When the environment variable is set but contains an empty
string or only whitespace, the function returns that blank value instead of
falling back to the default "X-OpenRAG-API-JWT". Fix this by retrieving the
environment variable value, checking if it is empty or contains only whitespace
using appropriate string methods, and returning the default value in those
cases; otherwise return the retrieved environment variable value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 41667a0c-e678-4a84-bd8e-428b77a93fc9
📒 Files selected for processing (3)
src/config/settings.pysrc/dependencies.pysrc/mcp_http/server.py
| def get_api_jwt_header() -> str: | ||
| """Secondary, gateway-placed JWT header for the /v1 API/MCP surface only. | ||
|
|
||
| The primary JWT header (``get_jwt_auth_header()``, default ``Authorization``) | ||
| is stripped by FastMCP's get_http_headers() before an MCP tool call is | ||
| proxied to the underlying /v1 route, so it never reaches the /v1 auth | ||
| dependency. The gateway (Traefik) therefore authenticates the MCP/API caller | ||
| (who supplies X-Username + X-Api-Key) and injects the minted user JWT into | ||
| this add-on header instead, which FastMCP forwards verbatim. Read per-call so | ||
| tests can override via monkeypatch.setenv. | ||
|
|
||
| TRUST NOTE: this header is read in addition to Authorization on the /v1 | ||
| surface and is trusted as an identity source. It is gateway-managed: Traefik | ||
| mints and injects it, and MUST strip any client-supplied value at the edge | ||
| (standard internal-trust-header hygiene) so callers cannot forge it — | ||
| important because claims are decode-only unless ``OPENRAG_JWT_VERIFY_SIGNATURE`` | ||
| is enabled.""" | ||
| return os.getenv("OPENRAG_API_JWT_HEADER", "X-OpenRAG-API-JWT") |
There was a problem hiding this comment.
Normalize blank OPENRAG_API_JWT_HEADER values to the default.
If OPENRAG_API_JWT_HEADER is set but empty/whitespace, this returns a blank header name and the fallback JWT lookup will always miss.
Proposed fix
def get_api_jwt_header() -> str:
@@
- return os.getenv("OPENRAG_API_JWT_HEADER", "X-OpenRAG-API-JWT")
+ configured = os.getenv("OPENRAG_API_JWT_HEADER", "").strip()
+ return configured or "X-OpenRAG-API-JWT"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/config/settings.py` around lines 273 - 290, The get_api_jwt_header()
function does not normalize empty or whitespace values of the
OPENRAG_API_JWT_HEADER environment variable to the default fallback. When the
environment variable is set but contains an empty string or only whitespace, the
function returns that blank value instead of falling back to the default
"X-OpenRAG-API-JWT". Fix this by retrieving the environment variable value,
checking if it is empty or contains only whitespace using appropriate string
methods, and returning the default value in those cases; otherwise return the
retrieved environment variable value.
…nRAG-API-JWT header (#1874) * use fallback header * Notes on new header * raises http exception for missing jwt
…nRAG-API-JWT header (#1874) * use fallback header * Notes on new header * raises http exception for missing jwt
* feat: add connectors permissions for Saas (#1782) * feat: add SaaS workspace connector permissions with RBAC gates Introduce admin Connectors Permission settings (cloud-only) backed by workspace_config, enforce availability on connector APIs and OAuth init, and sync the new connectors:manage:access permission at startup without an Alembic migration. OSS/dev OSS brand bypasses policy; add dev role toggle and brand-aware proxy headers for local RBAC testing. * style: ruff autofix (auto) * address CodeRabbit comments and fix backend lint unused imports * fix backend lint * feat: workspace connector permissions with aligned SaaS policy guards Add admin Connectors Permission (workspace_config-backed) with RBAC connectors:manage:access, enforce policy on connector APIs and OAuth init, and keep the permission list independent of the live connectors tab. Consolidate frontend connector-access hooks into useGetConnectorsQuery and settings tab helpers into brand.ts. Fix Connectors Permission tab redirects (RSC dev-brand default, wait for permissionsResolved). Let explicitly enabled connectors override deployment visibility filters; add OPENRAG_DEV_CONNECTOR_POLICY for local OSS dev backend enforcement Snapshot and restore all cached connector query keys in connect/disconnect mutations so optimistic updates stay correct when policy context changes mid-mutation (brand toggle, permissions resolving). * style: ruff autofix (auto) * addressed coderabbit comments * remove mentions of dev only envs in the env.example --------- Co-authored-by: Olfa Maslah <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Edwin Jose <[email protected]> * fix: google to support WEBHOOK_BASE_URL (#1849) * Update connector.py * update env in settings * fix webhook url of sharepoint * fix: make connector webhooks work end-to-end (Drive routing, Graph subscriptions, change discovery) (#1867) * fix: alias legacy /connectors/google/webhook to google_drive and reload connections store on webhook channel lookup miss * Fix Sharepoint and Google Drive * fix: propagate deleted items from SharePoint/OneDrive webhook delta query Main's delete-event coverage (#1852) expects handle_webhook to return deleted file ids so sync_specific_files can run its deleted-at-source cleanup (get_file_content -> 404 -> delete indexed chunks). The delta implementation skipped deleted items; now deleted files propagate and deleted folders stay excluded. * use logging _config * Update test_webhook_type_alias.py * fix: forward end-user JWT to MCP/API callers via gateway-placed X-OpenRAG-API-JWT header (#1874) * use fallback header * Notes on new header * raises http exception for missing jwt * feat: Add shared flag for COS ingestion to allow documents to be indexed without an owner * style: ruff autofix (auto) * Update src/api/connectors.py Improving error return Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat: Implement shared flag for document replacement in OpenSearch queries * style: ruff autofix (auto) * refactor: Remove shared flag from document ingestion options in SDK * feat: Update resolve_shared_owner_fields to return anonymous user details for shared documents * style: ruff autofix (auto) * feat: Add anonymous delete permission for shared documents and update related functionality * feat: Update delete permission to include anonymous access for shared documents * style: ruff autofix (auto) * style: apply ruff autofix * feat: Restrict shared flag usage to ibm_cos connector in sync_connector_files method * fix: clarify behavior of documents with owner=null in DLS tests * fix button types (#1875) * refactor: no derived state (#1886) * no derivied state * coderabbit comments * OPENSEARCH_NODE_COUNT_CHECK_ENABLED as deafult flag (#1975) What changed: - Renamed the env var read by the backend to OPENSEARCH_NODE_COUNT_CHECK_ENABLED in src/config/settings.py. - Updated Docker Compose to pass OPENSEARCH_NODE_COUNT_CHECK_ENABLED in docker-compose.yml. - Added the canonical flag to the example env file in .env.example. - Documented the flag in the OpenSearch config table in docs/docs/reference/configuration.mdx. - Updated the existing readiness test wording in tests/unit/test_opensearch_wait_node_count.py. - Added a regression test that loads config.settings with OPENSEARCH_NODE_COUNT_CHECK_ENABLED=false and verifies the canonical flag is what the code reads in tests/unit/config/ test_opensearch_node_count_check_enabled.py. * chore: Segment improvements (#1971) * chore: Segment improvements (#1963) * Update static properties * and knowledge/setting actions * avoid duplicate events * coderabbit comments * fix: onboarding timeout cleanup issue (#1977) * fix the cleanup timeout issue for the embedding step * final step fix * feat: add shared parameter to connector_sync function * feat: add shared parameter to TaskProcessor and update connector_sync for permission checks * fix: improve response handling in delete_document_endpoint for better status reporting * fix: clear completeTimeoutRef after completion to prevent memory leaks * fix: update OPENSEARCH_NODE_COUNT_CHECK_ENABLED to use fallback from OPENSEARCH_NODE_COUNT_CHECK * fix: onboarding timeout cleanup issue (#1977) * fix the cleanup timeout issue for the embedding step * final step fix * fix: support big csv files on langflow-less ingestion (#1968) * fix max tokens when splitting for langflow-less ingestion * fix coderabbit picks * style: ruff autofix (auto) * added non lf ingestion test --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> * fix: upgrade OpenSearch to 3.6.0 (#1972) * upgrade OpenSearch to 3.6.0 * fixed PR review comments * fix nitpicks * Added ibm watsonx ai and langchain ibm to Langflow image (#1989) * OPENSEARCH_NODE_COUNT_CHECK_ENABLED as deafult flag (#1975) What changed: - Renamed the env var read by the backend to OPENSEARCH_NODE_COUNT_CHECK_ENABLED in src/config/settings.py. - Updated Docker Compose to pass OPENSEARCH_NODE_COUNT_CHECK_ENABLED in docker-compose.yml. - Added the canonical flag to the example env file in .env.example. - Documented the flag in the OpenSearch config table in docs/docs/reference/configuration.mdx. - Updated the existing readiness test wording in tests/unit/test_opensearch_wait_node_count.py. - Added a regression test that loads config.settings with OPENSEARCH_NODE_COUNT_CHECK_ENABLED=false and verifies the canonical flag is what the code reads in tests/unit/config/ test_opensearch_node_count_check_enabled.py. * fix: onboarding timeout cleanup issue (#1977) * fix the cleanup timeout issue for the embedding step * final step fix * fix: universal file names (#1982) * ascii support for ingest files * update the flow json * style: ruff autofix (auto) * Update langflow_headers.py * Update test_langflow_ingest_callback.py * Update test-ci.yml * Update test_onboarding_sample_docs.py * style: ruff autofix (auto) * Update test_ascii_safe_header_value.py * Update langflow_file_service.py * re trigger commit * fix: purge config_manager singleton in non-langflow ingestion test The _purge_modules() list was missing "config.config_manager", so the ConfigManager singleton retained its cached _config from a previous test. Subsequent calls to get_openrag_config() returned the old config where disable_ingest_with_langflow=False, causing the router to use the Langflow path even though DISABLE_INGEST_WITH_LANGFLOW=true was set in the environment. Adding config.config_manager to the purge list forces a fresh load on the next import, picking up the correct env-var values. * fix: use hash_id to derive expected document_id in CSV ingestion test DocumentFileProcessor ignores Docling's binary_hash field and computes document_id = hash_id(file_path) from the actual file content. The test was asserting against the literal mock value "sha-csv-integration-123" which never appears in the indexed documents, giving 0 hits and a false failure. Now the test derives expected_document_id via hash_id(csv_path) to match what the production code actually indexes. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --------- Co-authored-by: Wallgau <[email protected]> Co-authored-by: Olfa Maslah <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Edwin Jose <[email protected]> Co-authored-by: Edwin Jose <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Mike Fortman <[email protected]> Co-authored-by: Sebastián Estévez <[email protected]> Co-authored-by: Lucas Oliveira <[email protected]>
Problem
MCP tool calls against the
/v1/*surface fail with 403/401:openrag_get_settings/update_settings/list_models→ 403 (providers:read,config:write)openrag_search, knowledge-filter reads/deletes → "403 (401 Unauthorized)" / 404Root cause: every
/v1request arriving via/mcplogsjwt_present: false. FastMCP'sfrom_fastapiproxy rebuilds the request to the underlying/v1handler usingget_http_headers(), whose hardcoded exclude set containsauthorization— so the gateway-forwarded user JWT is stripped before it reaches the auth dependency. The caller then falls back to IBM lakehouse Basic credentials, which (a) carry noopenrag_roles→ RBAC 403, and (b) are rejected by OpenSearch →AuthenticationException(401).This is a proxy-layer issue, not a Traefik misconfiguration: the same JWT arrives fine (
jwt_present: true) on UI/session routes where it isn't proxied through FastMCP.Fix
Read the JWT from a secondary, gateway-placed header that FastMCP does not strip. In SaaS, the MCP/API caller still authenticates with
X-Username+X-Api-Key; Traefik exchanges those for the user JWT and injects it into the add-on header. The/v1API-key auth path reads it as a fallback toAuthorization.config/settings.py: addget_api_jwt_header()→OPENRAG_API_JWT_HEADER(defaultX-OpenRAG-API-JWT).dependencies.py(get_api_key_user_async): useAuthorization, then fall back to the add-on header; log/report whichever header actually supplied the token.mcp_http/server.py: correct the auth docstring (Authorization is stripped by the proxy; document the gateway-placed JWT header).Trust model
The add-on header is gateway-managed, not client-supplied — same trust boundary as today's
Authorization. Because claims are decode-only unlessOPENRAG_JWT_VERIFY_SIGNATURE=true, the header is trusted only because Traefik mints it. Gateway requirement: Traefik must injectX-OpenRAG-API-JWTon the/mcproute and strip any client-supplied value at the edge so it cannot be forged.Config
OPENRAG_API_JWT_HEADER(defaultX-OpenRAG-API-JWT) — name of the add-on JWT header.Testing
tests/unit/dependencies/test_jwt_header_auth.py,tests/unit/config/test_jwt_auth_header.py,tests/unit/dependencies/test_ibm_header_auth.py).X-OpenRAG-API-JWT, callopenrag_get_settingsvia MCP and confirm the[AUTH] API-key path JWT header lookuplog flips tojwt_present: true(header_nameX-OpenRAG-API-JWT), andopenrag_searchreturns results with no OpenSearch 401.Follow-ups
get_api_key_user_async.Summary by CodeRabbit
Release Notes
New Features
/v1API surface, including gateway-forwarded header fallback.Bug Fixes
Documentation
/v1handling.Tests