Skip to content

fix: Add forwarded JWT header auth and issuer utils for api users#1697

Merged
edwinjosechittilappilly merged 7 commits into
mainfrom
fix-api-roles
May 28, 2026
Merged

fix: Add forwarded JWT header auth and issuer utils for api users#1697
edwinjosechittilappilly merged 7 commits into
mainfrom
fix-api-roles

Conversation

@edwinjosechittilappilly

@edwinjosechittilappilly edwinjosechittilappilly commented May 28, 2026

Copy link
Copy Markdown
Collaborator

Add utilities to fetch and cache issuer public keys and verify JWTs (config/utils.py), including a helper to obtain OpenSearch service tokens from an internal auth server. Expose AUTH_SERVER_URL and a per-call get_jwt_auth_header accessor in config/settings.py so the forwarded-JWT header is configurable and testable. Extend dependencies.py to accept a gateway-forwarded JWT on the /v1 (API-key) surface: verify the token against the issuer, stage JWT roles for RBAC, and make the JWT the source of identity when valid (with proper 401 behavior under RBAC). Add unit tests for the header accessor and the JWT-header auth flows.

Summary by CodeRabbit

  • New Features

    • Permission-scoped API‑key checks enforced across v1 endpoints.
    • JWT-in-header authentication with configurable header name and issuer-based JWT verification and public-key discovery.
  • Refactor

    • Modernized type annotations (PEP‑604 unions) and unified JWT role staging for consistent auth behavior.
    • Endpoints now produce clearer JSON/stream responses and standardized error payloads.
  • Tests

    • New unit tests covering permission enforcement and JWT header authentication.

Review Change Stack

Add utilities to fetch and cache issuer public keys and verify JWTs (config/utils.py), including a helper to obtain OpenSearch service tokens from an internal auth server. Expose AUTH_SERVER_URL and a per-call get_jwt_auth_header accessor in config/settings.py so the forwarded-JWT header is configurable and testable. Extend dependencies.py to accept a gateway-forwarded JWT on the /v1 (API-key) surface: verify the token against the issuer, stage JWT roles for RBAC, and make the JWT the source of identity when valid (with proper 401 behavior under RBAC). Add unit tests for the header accessor and the JWT-header auth flows.
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Implements permission-scoped API-key gating and gateway-JWT authentication for v1: adds require_api_key_permission, JWT public-key discovery/verification and role staging, updates v1 endpoints to use permission dependencies and PEP-604 typing, and adds unit/integration tests and a Ruff allowlist entry.

Changes

API Key Authentication & Permission Enforcement

Layer / File(s) Summary
Configuration & JWT infrastructure
pyproject.toml, src/config/settings.py, src/config/utils.py
Adds get_jwt_auth_header(), public-key discovery and caching, _load_public_key_from_payload, get_public_key_from_issuer, verify_jwt_from_issuer, and Ruff allowlist for dependencies.require_api_key_permission.
Permission & JWT authentication dependencies
src/dependencies.py
Adds require_api_key_permission(perm), introduces _stage_jwt_roles, refactors IBM JWT cookie processing to stage roles, and implements a JWT-in-header branch in get_api_key_user_async.
Chat endpoint permission wiring
src/api/v1/chat.py
Rewires chat endpoints to require chat:use, conversations:read:own, conversations:delete:own; modernizes ChatV1Body typing and response formatting including per-message usage.
Documents & Knowledge Endpoint Permission Wiring
src/api/v1/documents.py, src/api/v1/knowledge_filters.py
Wires documents endpoints to require knowledge:upload, knowledge:read:own, knowledge:delete:own; knowledge-filter endpoints require kf:create or kf:edit:own.
Search & Settings Endpoint Permission Wiring
src/api/v1/search.py, src/api/v1/settings.py
Search requires search:use; settings update requires config:write. Request/settings models updated to PEP-604 unions.
Models Endpoint Formatting
src/api/v1/models.py
Reformats _fetch_models error handling into multi-line JSONResponse blocks; behavior unchanged.
API Key Permission Integration Tests
tests/unit/dependencies/test_require_api_key_permission.py
Adds integration tests that seed roles/permissions and verify require_api_key_permission gating, kill-switch, and allowed/denied flows including real /v1/documents DELETE gating.
JWT Header Configuration Tests
tests/unit/config/test_jwt_auth_header.py
Adds tests for get_jwt_auth_header() default and env override.
JWT Header Auth & Role Staging Tests
tests/unit/dependencies/test_jwt_header_auth.py
Adds tests for _stage_jwt_roles and get_api_key_user_async JWT-header branch covering RBAC on/off, role extraction, missing-role and invalid-JWT 401s, and regression guards.

Sequence Diagram

sequenceDiagram
  participant Client
  participant Gateway
  participant App as OpenRAG App
  participant Issuer
  participant RBAC as RBACService
  Client->>Gateway: request to /v1 endpoint with JWT header
  Gateway->>App: forwarded JWT (get_jwt_auth_header)
  App->>App: verify_jwt_from_issuer (reads kid, iss)
  App->>Issuer: fetch issuer keys
  Issuer->>App: JWKS/PEM
  App->>App: _load_public_key_from_payload -> cache
  App->>App: jwt.decode -> claims
  App->>RBAC: rbac.get_user_permissions(user_id)
  RBAC-->>App: permission list
  App->>App: require_api_key_permission checks permission -> allow/deny
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • lucaseduoli
  • mpawlow
  • zzzming
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 46.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding JWT header authentication and issuer utilities for API users.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-api-roles

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added backend 🔷 Issues related to backend services (OpenSearch, Langflow, APIs) tests bug 🔴 Something isn't working. labels May 28, 2026
Introduce require_api_key_permission dependency to enforce RBAC for the /v1 API-key/forwarded-JWT surface (mirrors require_permission semantics and kill-switch behavior). Wire v1 endpoints to use the new gate, update several type hints to PEP 604 unions, and tidy imports. Add unit tests (tests/unit/dependencies/test_require_api_key_permission.py) to validate permission checks and kill-switch behavior. Also register the new dependency in pyproject extend-immutable-calls.
@github-actions github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels May 28, 2026
@github-actions github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels May 28, 2026
@edwinjosechittilappilly edwinjosechittilappilly marked this pull request as ready for review May 28, 2026 17:43
Copilot AI review requested due to automatic review settings May 28, 2026 17:43
@github-actions github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels May 28, 2026

This comment was marked as outdated.

Annotate chat_id_container as dict[str, str] in chat endpoint to make types explicit. Allow get_settings_endpoint to return either SettingsResponse or JSONResponse to support error responses. Change default OPENRAG_JWT_AUTH_HEADER from "X-OpenRAG-JWT" to the standard "Authorization" header so gateway-forwarded JWTs use a common header.
@github-actions github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels May 28, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/api/v1/chat.py (1)

149-149: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add type annotation to fix mypy error.

The pipeline reports a mypy error requiring a type annotation for chat_id_container.

Proposed fix
-        chat_id_container = {}
+        chat_id_container: dict[str, str | None] = {}
🤖 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/api/v1/chat.py` at line 149, Add an explicit type annotation for
chat_id_container to satisfy mypy (e.g., import Dict and Any from typing and
declare chat_id_container: Dict[str, Any] = {}); ensure the typing import (from
typing import Dict, Any) is added near other imports and replace the bare
assignment "chat_id_container = {}" with the annotated declaration so mypy
recognizes its type.
src/api/v1/settings.py (1)

69-71: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix return type mismatch flagged by mypy.

The function signature declares -> SettingsResponse but the exception handler returns JSONResponse, causing the pipeline mypy error. Either change the return type annotation to SettingsResponse | JSONResponse or raise an HTTPException instead.

Option 1: Raise HTTPException (preferred for FastAPI)
     except Exception as e:
         logger.error("Failed to get settings", error=str(e))
-        return JSONResponse({"error": "Failed to get settings"}, status_code=500)
+        raise HTTPException(status_code=500, detail="Failed to get settings")

Add HTTPException to imports:

-from fastapi import Depends
+from fastapi import Depends, HTTPException
Option 2: Update return type annotation
 async def get_settings_endpoint(
     user: User = Depends(get_api_key_user_async),
-) -> SettingsResponse:
+) -> SettingsResponse | JSONResponse:
🤖 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/api/v1/settings.py` around lines 69 - 71, The exception handler in the
function that returns SettingsResponse currently returns a JSONResponse, causing
a mypy return-type mismatch; update the handler to raise fastapi.HTTPException
instead: add HTTPException to imports and replace `return JSONResponse({"error":
"Failed to get settings"}, status_code=500)` with `raise
HTTPException(status_code=500, detail="Failed to get settings")` (keep the
existing logger.error call). This preserves the declared return type
(SettingsResponse) and follows FastAPI conventions.
🧹 Nitpick comments (2)
src/config/utils.py (1)

89-91: ⚖️ Poor tradeoff

Synchronous HTTP call may block the event loop when invoked from async context.

get_public_key_from_issuer uses a synchronous httpx.Client, but it's called from verify_jwt_from_issuer, which is invoked inside get_api_key_user_async (an async FastAPI dependency). This blocks the event loop during the HTTP fetch, degrading concurrency under load.

Consider converting to httpx.AsyncClient (and making the caller chain async) or wrapping in asyncio.to_thread() at the call site.

🤖 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/utils.py` around lines 89 - 91, get_public_key_from_issuer
currently performs a blocking HTTP call with httpx.Client which can block the
FastAPI event loop when invoked from async dependencies; change it to an async
implementation using httpx.AsyncClient (await client.get(...) and await
response.aread()/response.raise_for_status()) and propagate async changes by
making verify_jwt_from_issuer async and awaiting get_public_key_from_issuer, and
updating get_api_key_user_async to await verify_jwt_from_issuer; alternatively,
if you prefer minimal changes, keep get_public_key_from_issuer sync but call it
from verify_jwt_from_issuer via asyncio.to_thread(...) to offload the blocking
call—use the approach that matches your project's async style and ensure all
call sites await the async functions.
tests/unit/dependencies/test_require_api_key_permission.py (1)

18-19: ⚡ Quick win

Make the in-memory SQLite pooling explicit (intent clarity), not necessity.

SQLAlchemy’s default behavior for sqlite+aiosqlite:///:memory: already uses StaticPool/keeps a single in-memory connection alive, so poolclass=StaticPool isn’t strictly required to avoid state-sharing flakes; adding it just makes the test’s intent resilient to future default changes.

Proposed diff
 from fastapi import Depends, FastAPI, Request
 from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine
+from sqlalchemy.pool import StaticPool
 from sqlmodel import SQLModel
@@
     engine = create_async_engine(
         "sqlite+aiosqlite:///:memory:",
         connect_args={"check_same_thread": False},
+        poolclass=StaticPool,
     )
🤖 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 `@tests/unit/dependencies/test_require_api_key_permission.py` around lines 18 -
19, The test should make SQLite in-memory pooling explicit: import StaticPool
from sqlalchemy.pool and pass poolclass=StaticPool to create_async_engine (the
call that currently uses "sqlite+aiosqlite:///:memory:"), so the engine creation
in this test (where create_async_engine is used alongside
async_sessionmaker/SQLModel) clearly documents and enforces a single-connection
in-memory pool; update the engine construction to include poolclass=StaticPool
(and keep any existing connect_args) so intent is explicit and robust to default
changes.
🤖 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.

Outside diff comments:
In `@src/api/v1/chat.py`:
- Line 149: Add an explicit type annotation for chat_id_container to satisfy
mypy (e.g., import Dict and Any from typing and declare chat_id_container:
Dict[str, Any] = {}); ensure the typing import (from typing import Dict, Any) is
added near other imports and replace the bare assignment "chat_id_container =
{}" with the annotated declaration so mypy recognizes its type.

In `@src/api/v1/settings.py`:
- Around line 69-71: The exception handler in the function that returns
SettingsResponse currently returns a JSONResponse, causing a mypy return-type
mismatch; update the handler to raise fastapi.HTTPException instead: add
HTTPException to imports and replace `return JSONResponse({"error": "Failed to
get settings"}, status_code=500)` with `raise HTTPException(status_code=500,
detail="Failed to get settings")` (keep the existing logger.error call). This
preserves the declared return type (SettingsResponse) and follows FastAPI
conventions.

---

Nitpick comments:
In `@src/config/utils.py`:
- Around line 89-91: get_public_key_from_issuer currently performs a blocking
HTTP call with httpx.Client which can block the FastAPI event loop when invoked
from async dependencies; change it to an async implementation using
httpx.AsyncClient (await client.get(...) and await
response.aread()/response.raise_for_status()) and propagate async changes by
making verify_jwt_from_issuer async and awaiting get_public_key_from_issuer, and
updating get_api_key_user_async to await verify_jwt_from_issuer; alternatively,
if you prefer minimal changes, keep get_public_key_from_issuer sync but call it
from verify_jwt_from_issuer via asyncio.to_thread(...) to offload the blocking
call—use the approach that matches your project's async style and ensure all
call sites await the async functions.

In `@tests/unit/dependencies/test_require_api_key_permission.py`:
- Around line 18-19: The test should make SQLite in-memory pooling explicit:
import StaticPool from sqlalchemy.pool and pass poolclass=StaticPool to
create_async_engine (the call that currently uses
"sqlite+aiosqlite:///:memory:"), so the engine creation in this test (where
create_async_engine is used alongside async_sessionmaker/SQLModel) clearly
documents and enforces a single-connection in-memory pool; update the engine
construction to include poolclass=StaticPool (and keep any existing
connect_args) so intent is explicit and robust to default changes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 54f6a999-9142-4516-affe-3598c87a8b38

📥 Commits

Reviewing files that changed from the base of the PR and between bfa3a39 and 3444ca6.

📒 Files selected for processing (13)
  • pyproject.toml
  • src/api/v1/chat.py
  • src/api/v1/documents.py
  • src/api/v1/knowledge_filters.py
  • src/api/v1/models.py
  • src/api/v1/search.py
  • src/api/v1/settings.py
  • src/config/settings.py
  • src/config/utils.py
  • src/dependencies.py
  • tests/unit/config/test_jwt_auth_header.py
  • tests/unit/dependencies/test_jwt_header_auth.py
  • tests/unit/dependencies/test_require_api_key_permission.py

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Actionable comments posted: 0

@github-actions github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels May 28, 2026
Stop pinning JWT issuers via AUTH_SERVER_URL and remove the issuer allowlist. JWT verification now discovers signing keys from the token's own iss (JWKS/PEM/JWK) and trusts the issuer to publish its keys; related helper logic (k8s SA token reader, issuer allowlist helper, and get_opensearch_service_token) and the AUTH_SERVER_URL setting were removed. The default JWT header was changed to Authorization (tests updated and pinning added where needed). Update dependencies and unit tests to reflect the simplified verification flow and header default; callers relying on AUTH_SERVER_URL or the removed service-token helper must be updated accordingly.
@github-actions github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels May 28, 2026
@github-actions github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels May 28, 2026
@edwinjosechittilappilly edwinjosechittilappilly enabled auto-merge (squash) May 28, 2026 18:23
@edwinjosechittilappilly

Copy link
Copy Markdown
Collaborator Author

@coderabbitai generate a mermaid diagram on the workflow

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@edwinjosechittilappilly Here is a Mermaid diagram illustrating the authentication and authorization workflow introduced by this PR:

flowchart TD
    A([Incoming /v1 Request]) --> B{JWT header\npresent?\nget_jwt_auth_header}

    B -- No --> K[Proceed to\nAPI Key Check]
    B -- Yes --> C{AUTH_SERVER_URL\nconfigured?}

    C -- No --> K
    C -- Yes --> D[verify_jwt_from_issuer\nconfig/utils.py]

    D --> D1[Strip Bearer prefix]
    D1 --> D2[Decode unverified header\nCheck allowed alg]
    D2 --> D3[get_public_key_from_issuer\nFetch & cache issuer public key]
    D3 --> D4[jwt.decode\nVerify signature + claims\niss, sub, exp, iat]

    D4 -- Verification\nfails --> E{RBAC\nenforced?\nOPENRAG_RBAC_ENFORCE}
    E -- Yes --> F[HTTP 401\nInvalid or unverifiable JWT]
    E -- No --> K

    D4 -- Claims valid --> G[Build User identity\nfrom JWT claims]
    G --> H[_stage_jwt_roles\nextract roles from JWT claims]

    H --> I{RBAC\nenforced?}
    I -- No --> J[Skip role staging\nreturn JWT User]
    I -- Yes --> I2{Roles found\nin JWT?}
    I2 -- No roles --> F2[HTTP 401\nNo recognized roles]
    I2 -- Roles found --> I3[Store roles in\nrequest.state.jwt_roles]
    I3 --> L[_attach_db_user_id\nlink JWT identity to DB]
    L --> J

    K --> K1{API key\npresent?}
    K1 -- No --> K2[HTTP 401\nAPI key required]
    K1 -- Yes --> K3[Resolve User\nfrom API key\nget_api_key_user_async]

    J --> M[require_api_key_permission\nperm check]
    K3 --> M

    M --> N{RBAC\nenforced?}
    N -- No --> O([✅ Handler invoked])
    N -- Yes --> P[rbac.get_user_permissions]
    P --> Q{Permission\nheld?}
    Q -- Yes --> O
    Q -- No --> R[HTTP 403\npermission_denied\nrequired: perm]

    style F fill:rgba(220,50,47,0.5)
    style F2 fill:rgba(220,50,47,0.5)
    style K2 fill:rgba(220,50,47,0.5)
    style R fill:rgba(220,50,47,0.5)
    style O fill:rgba(42,161,152,0.5)
Loading

Key flows summarized:

Path Trigger Outcome
JWT-header (RBAC off) JWT present + valid, RBAC disabled User built from JWT claims, bypasses role/permission checks
JWT-header (RBAC on) JWT present + valid, RBAC enabled Roles staged, permission gate applied
JWT invalid (RBAC on) JWT present but unverifiable HTTP 401
JWT invalid (RBAC off) JWT present but unverifiable Falls through to API key check
No JWT No JWT header Falls through to API key check
Permission denied User lacks required permission HTTP 403 with required permission in body

@edwinjosechittilappilly

edwinjosechittilappilly commented May 28, 2026

Copy link
Copy Markdown
Collaborator Author

@edwinjosechittilappilly Here is a Mermaid diagram illustrating the authentication and authorization workflow introduced by this PR:

flowchart TD
    A([Incoming /v1 Request]) --> B{JWT header\npresent?\nget_jwt_auth_header}

    B -- No --> K[Proceed to\nAPI Key Check]
    B -- Yes --> C{AUTH_SERVER_URL\nconfigured?}

    C -- No --> K
    C -- Yes --> D[verify_jwt_from_issuer\nconfig/utils.py]

    D --> D1[Strip Bearer prefix]
    D1 --> D2[Decode unverified header\nCheck allowed alg]
    D2 --> D3[get_public_key_from_issuer\nFetch & cache issuer public key]
    D3 --> D4[jwt.decode\nVerify signature + claims\niss, sub, exp, iat]

    D4 -- Verification\nfails --> E{RBAC\nenforced?\nOPENRAG_RBAC_ENFORCE}
    E -- Yes --> F[HTTP 401\nInvalid or unverifiable JWT]
    E -- No --> K

    D4 -- Claims valid --> G[Build User identity\nfrom JWT claims]
    G --> H[_stage_jwt_roles\nextract roles from JWT claims]

    H --> I{RBAC\nenforced?}
    I -- No --> J[Skip role staging\nreturn JWT User]
    I -- Yes --> I2{Roles found\nin JWT?}
    I2 -- No roles --> F2[HTTP 401\nNo recognized roles]
    I2 -- Roles found --> I3[Store roles in\nrequest.state.jwt_roles]
    I3 --> L[_attach_db_user_id\nlink JWT identity to DB]
    L --> J

    K --> K1{API key\npresent?}
    K1 -- No --> K2[HTTP 401\nAPI key required]
    K1 -- Yes --> K3[Resolve User\nfrom API key\nget_api_key_user_async]

    J --> M[require_api_key_permission\nperm check]
    K3 --> M

    M --> N{RBAC\nenforced?}
    N -- No --> O([✅ Handler invoked])
    N -- Yes --> P[rbac.get_user_permissions]
    P --> Q{Permission\nheld?}
    Q -- Yes --> O
    Q -- No --> R[HTTP 403\npermission_denied\nrequired: perm]

    style F fill:rgba(220,50,47,0.5)
    style F2 fill:rgba(220,50,47,0.5)
    style K2 fill:rgba(220,50,47,0.5)
    style R fill:rgba(220,50,47,0.5)
    style O fill:rgba(42,161,152,0.5)
Loading

Unable to render rich display

@coderabbitai retry again generate the sequence diagram

@langflow-ai langflow-ai deleted a comment from coderabbitai Bot May 28, 2026
@edwinjosechittilappilly

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review the PR

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

@edwinjosechittilappilly Sure! Triggering a full review of PR #1697 now.

Also, here is the sequence diagram you requested earlier:

sequenceDiagram
    participant Client
    participant FastAPI as FastAPI /v1 Endpoint
    participant GKU as get_jwt_auth_header()<br/>(config/settings.py)
    participant VJI as verify_jwt_from_issuer()<br/>(config/utils.py)
    participant GPKI as get_public_key_from_issuer()<br/>(config/utils.py)
    participant SJR as _stage_jwt_roles()<br/>(dependencies.py)
    participant ADBU as _attach_db_user_id()<br/>(dependencies.py)
    participant RAKP as require_api_key_permission()<br/>(dependencies.py)
    participant RBAC as rbac.get_user_permissions()

    Client->>FastAPI: Request with headers

    FastAPI->>GKU: get_jwt_auth_header()
    GKU-->>FastAPI: header name (e.g. "Authorization")

    alt JWT header present AND AUTH_SERVER_URL set
        FastAPI->>VJI: verify_jwt_from_issuer(token)
        VJI->>VJI: Strip Bearer prefix
        VJI->>VJI: Decode unverified header (check alg)
        VJI->>GPKI: get_public_key_from_issuer(iss, kid)
        GPKI-->>VJI: cached/fetched public key
        VJI->>VJI: jwt.decode (verify sig + iss/sub/exp/iat)

        alt Verification fails
            VJI-->>FastAPI: None
            alt RBAC enforced
                FastAPI-->>Client: HTTP 401 Invalid or unverifiable JWT
            else RBAC off
                FastAPI->>FastAPI: Fall through to API key check
            end
        else Claims valid
            VJI-->>FastAPI: JWT claims dict
            FastAPI->>FastAPI: Build User identity from claims
            FastAPI->>SJR: _stage_jwt_roles(request, claims, user_id)
            alt RBAC enforced
                SJR->>SJR: Extract roles from JWT claim
                alt No recognized roles
                    SJR-->>Client: HTTP 401 No recognized roles
                else Roles found
                    SJR->>SJR: Store in request.state.jwt_roles
                    SJR-->>FastAPI: roles staged
                end
            else RBAC off
                SJR-->>FastAPI: noop (jwt_roles not staged)
            end
            FastAPI->>ADBU: _attach_db_user_id(user)
            ADBU-->>FastAPI: user with DB id
        end
    else No JWT header or no AUTH_SERVER_URL
        FastAPI->>FastAPI: Check API key header
        alt No API key
            FastAPI-->>Client: HTTP 401 API key required
        else API key present
            FastAPI->>FastAPI: Resolve User from API key
        end
    end

    FastAPI->>RAKP: require_api_key_permission(perm)
    alt RBAC off
        RAKP-->>FastAPI: bypass
    else RBAC enforced
        RAKP->>RBAC: get_user_permissions(user)
        RBAC-->>RAKP: permissions set
        alt Permission held
            RAKP-->>FastAPI: allowed
        else Permission missing
            RAKP-->>Client: HTTP 403 permission_denied (required: perm)
        end
    end

    FastAPI-->>Client: ✅ Handler response
Loading
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@edwinjosechittilappilly Here is a sequence diagram for the authentication and authorization workflow:

sequenceDiagram
    autonumber
    participant C as Client
    participant EP as FastAPI /v1 Endpoint
    participant DEP as get_api_key_user_async
    participant VFY as verify_jwt_from_issuer
    participant PKI as get_public_key_from_issuer<br/>(TTL Cache)
    participant SJR as _stage_jwt_roles
    participant DBA as _attach_db_user_id
    participant PERM as require_api_key_permission
    participant RBAC as RBAC Service

    C->>EP: HTTP Request (JWT header or API key)
    EP->>DEP: resolve user identity

    alt JWT header present AND AUTH_SERVER_URL configured
        DEP->>VFY: verify_jwt_from_issuer(token)
        VFY->>VFY: Strip Bearer prefix
        VFY->>VFY: Decode unverified header → check alg allowlist
        VFY->>VFY: Decode unverified claims → extract iss
        VFY->>PKI: get_public_key_from_issuer(iss, kid)
        PKI-->>VFY: cached or freshly fetched public key
        VFY->>VFY: jwt.decode (verify sig + iss/sub/exp/iat)

        alt Verification succeeds
            VFY-->>DEP: JWT claims (sub, email, …)
            DEP->>DEP: Build User from JWT claims
            DEP->>SJR: _stage_jwt_roles(request, claims, user_id)

            alt RBAC enforced (OPENRAG_RBAC_ENFORCE=true)
                SJR->>SJR: Map JWT roles claim → OpenRAG roles
                alt Roles found
                    SJR->>SJR: request.state.jwt_roles = [roles]
                    SJR-->>DEP: OK
                    DEP->>DBA: _attach_db_user_id(user)
                    DBA-->>DEP: user with DB id attached
                    DEP-->>EP: JWT User ✅
                else No recognized roles
                    SJR-->>EP: HTTP 401 – No recognized roles ❌
                end
            else RBAC off
                SJR->>SJR: skip role staging
                SJR-->>DEP: OK (roles not staged)
                DEP-->>EP: JWT User ✅
            end

        else Verification fails
            VFY-->>DEP: None
            alt RBAC enforced
                DEP-->>EP: HTTP 401 – Invalid or unverifiable JWT ❌
            else RBAC off
                Note over DEP: Fall through to API key check
                DEP->>DEP: Check API key header
                alt API key present
                    DEP-->>EP: API key User ✅
                else No API key
                    DEP-->>EP: HTTP 401 – API key required ❌
                end
            end
        end

    else No JWT header (or no AUTH_SERVER_URL)
        DEP->>DEP: Check API key header
        alt API key present
            DEP-->>EP: API key User ✅
        else No API key
            DEP-->>EP: HTTP 401 – API key required ❌
        end
    end

    EP->>PERM: require_api_key_permission(perm)

    alt RBAC enforced
        PERM->>RBAC: get_user_permissions(user)
        RBAC-->>PERM: permission set
        alt Permission held
            PERM-->>EP: OK ✅
            EP-->>C: 200 Response
        else Permission missing
            PERM-->>EP: HTTP 403 – permission_denied { required: perm } ❌
            EP-->>C: 403 Forbidden
        end
    else RBAC off
        PERM-->>EP: Bypass ✅
        EP-->>C: 200 Response
    end
Loading

Summary of key paths:

# Scenario Outcome
1 JWT present, valid, roles found (RBAC on) ✅ Roles staged → permission gate applied
2 JWT present, valid, no roles (RBAC on) ❌ HTTP 401 – No recognized roles
3 JWT present, valid (RBAC off) ✅ Identity from JWT, no role/perm check
4 JWT present, invalid (RBAC on) ❌ HTTP 401 – Invalid or unverifiable JWT
5 JWT present, invalid (RBAC off) Falls through → API key check
6 No JWT → API key present ✅ Proceeds to permission gate
7 No JWT → No API key ❌ HTTP 401 – API key required
8 Permission check fails ❌ HTTP 403 – permission_denied

(¬ᵕ¬) 🐇

@github-actions github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels May 28, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/api/v1/settings.py (1)

47-71: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Return type annotation no longer matches actual return paths.

The annotation was narrowed to SettingsResponse, but line 71 still returns JSONResponse on error. This breaks static type checking and can mislead consumers.

Either restore the union type or convert to an HTTPException for consistency with FastAPI error-handling patterns.

Option A: Restore union type
 async def get_settings_endpoint(
     user: User = Depends(get_api_key_user_async),
-) -> SettingsResponse:
+) -> SettingsResponse | JSONResponse:
Option B: Raise HTTPException (preferred)
+from fastapi import Depends, HTTPException
-from fastapi import Depends
 ...
     except Exception as e:
         logger.error("Failed to get settings", error=str(e))
-        return JSONResponse({"error": "Failed to get settings"}, status_code=500)
+        raise HTTPException(status_code=500, detail="Failed to get settings")

As per coding guidelines: src/api/**/*.py requires verifying "response model typing."

🤖 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/api/v1/settings.py` around lines 47 - 71, The get_settings_endpoint
function currently annotates its return type as SettingsResponse but returns a
JSONResponse on exception; update this to follow FastAPI error-handling by
replacing the JSONResponse return with raising fastapi.HTTPException (import
HTTPException) so the function consistently returns SettingsResponse or raises
on error, or alternatively change the signature to return
Union[SettingsResponse, JSONResponse] if you prefer Option A; locate
get_settings_endpoint, the error handling block that logs via logger.error and
uses JSONResponse, and modify it to raise HTTPException(status_code=500,
detail="Failed to get settings") while keeping the existing log of
get_openrag_config errors.
🤖 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.

Outside diff comments:
In `@src/api/v1/settings.py`:
- Around line 47-71: The get_settings_endpoint function currently annotates its
return type as SettingsResponse but returns a JSONResponse on exception; update
this to follow FastAPI error-handling by replacing the JSONResponse return with
raising fastapi.HTTPException (import HTTPException) so the function
consistently returns SettingsResponse or raises on error, or alternatively
change the signature to return Union[SettingsResponse, JSONResponse] if you
prefer Option A; locate get_settings_endpoint, the error handling block that
logs via logger.error and uses JSONResponse, and modify it to raise
HTTPException(status_code=500, detail="Failed to get settings") while keeping
the existing log of get_openrag_config errors.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 943bde5b-6e57-4b73-b524-eb86fcb9f131

📥 Commits

Reviewing files that changed from the base of the PR and between d2fcb9e and b033e52.

📒 Files selected for processing (2)
  • pyproject.toml
  • src/api/v1/settings.py

@mfortman11 mfortman11 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend 🔷 Issues related to backend services (OpenSearch, Langflow, APIs) bug 🔴 Something isn't working. lgtm tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants