feat: added docling authentication when ibm auth is enabled#1551
Conversation
github-actions
Bot
added
backend
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis pull request extends IBM authentication support throughout the Docling document conversion flow. Auth context (user ID and JWT token) is now threaded from API endpoints through document, processor, and langflow services into DoclingService, which constructs IBM-compatible headers on all Docling API requests. New environment-based configuration enables dev-mode authentication overrides and controls TLS verification. Startup logic guards prevent unnecessary security setup in dev mode. ChangesIBM Auth Integration in Docling Upload Flow
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
github-actions
Bot
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/api/upload.py (1)
122-128:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent
user_idderivation vs the rest of this file.Line 122 uses
user.user_id if user else None, but lines 132 (owner_user_id), 44, 90, 210 all useuser.user_id if (user and not is_no_auth) else None. As a result, whenis_no_auth_mode()is true,process_upload_context(and downstream DoclingX-Tenant-Id) receives a user id whilechat_servicereceivesNone— these should be consistent.If the intent for IBM auth is to always have a tenant id when a user object exists, that's fine, but the same logic should apply to
chat_service.upload_context_chat'sownerargument. Otherwise, mirror the gated form here.♻️ Proposed alignment
- filename = file.filename or "uploaded_document" - user_id = user.user_id if user else None - - jwt_token = user.jwt_token + filename = file.filename or "uploaded_document" + + from config.settings import is_no_auth_mode + is_no_auth = is_no_auth_mode() + user_id = user.user_id if (user and not is_no_auth) else None + jwt_token = user.jwt_token doc_result = await document_service.process_upload_context( file, filename, user_id=user_id, jwt_token=jwt_token ) - from config.settings import is_no_auth_mode - is_no_auth = is_no_auth_mode() owner_user_id = user.user_id if (user and not is_no_auth) else 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/upload.py` around lines 122 - 128, The user_id passed to document_service.process_upload_context is derived unconditionally as user.user_id if user else None, which is inconsistent with other usages (owner_user_id and chat_service.upload_context_chat) that use the gated form when is_no_auth_mode() is considered; update the derivation to match the gated logic used elsewhere (e.g., set user_id = user.user_id if (user and not is_no_auth_mode()) else None) so process_upload_context and chat_service.upload_context_chat receive the same owner/tenant value; adjust the variable near where owner_user_id is computed and ensure both process_upload_context and chat_service.upload_context_chat use that consistent value.
🧹 Nitpick comments (2)
src/services/docling_service.py (2)
90-99: 💤 Low valueOptional: rename
auth_headertojwt_tokenfor consistency.Across the rest of the codebase (
document_service.py,processors.py,langflow_file_service.py,api/upload.py) this value is consistently calledjwt_token. Renaming the parameter on_get_auth_headers,upload_to_docling_direct_async,get_docling_result_async,_poll_result,convert_file, andconvert_bytestojwt_token(and constructing theAuthorizationheader inside_get_auth_headers) would remove the contract ambiguity flagged above.🤖 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/services/docling_service.py` around lines 90 - 99, Rename the auth parameter named auth_header to jwt_token across the Docling-related API to keep naming consistent and remove ambiguity: update the _get_auth_headers signature to _get_auth_headers(self, user_id: Optional[str] = None, jwt_token: Optional[str] = None) and use jwt_token to build the "Authorization" header; also rename the parameter in upload_to_docling_direct_async, get_docling_result_async, _poll_result, convert_file, and convert_bytes so they accept jwt_token and pass it into _get_auth_headers; ensure all call sites in document_service.py, processors.py, langflow_file_service.py, and api/upload.py are updated to pass jwt_token and that header construction is centralized in _get_auth_headers.
93-99: 💤 Low valueConsider logging when IBM auth is enabled but credentials are missing.
When
IBM_AUTH_ENABLEDis true and eitheruser_idorauth_headerisNone, the helper silently omits those headers and Docling Serve will return an opaque 401/403. Alogger.warning(orlogger.debugat minimum) here would make troubleshooting much easier without changing behavior.🤖 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/services/docling_service.py` around lines 93 - 99, When IBM_AUTH_ENABLED is true and you're building the headers (the block that checks auth_header and user_id and returns headers), add logging when either auth_header or user_id is missing so missing credentials are visible; specifically, inside the IBM_AUTH_ENABLED branch, if auth_header is falsy log a warning (or debug) mentioning missing Authorization, and if user_id is falsy log a warning (or debug) mentioning missing X-Tenant-Id, keeping behavior unchanged otherwise (still only set headers when values exist).
🤖 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/upload.py`:
- Around line 122-128: The user_id passed to
document_service.process_upload_context is derived unconditionally as
user.user_id if user else None, which is inconsistent with other usages
(owner_user_id and chat_service.upload_context_chat) that use the gated form
when is_no_auth_mode() is considered; update the derivation to match the gated
logic used elsewhere (e.g., set user_id = user.user_id if (user and not
is_no_auth_mode()) else None) so process_upload_context and
chat_service.upload_context_chat receive the same owner/tenant value; adjust the
variable near where owner_user_id is computed and ensure both
process_upload_context and chat_service.upload_context_chat use that consistent
value.
---
Nitpick comments:
In `@src/services/docling_service.py`:
- Around line 90-99: Rename the auth parameter named auth_header to jwt_token
across the Docling-related API to keep naming consistent and remove ambiguity:
update the _get_auth_headers signature to _get_auth_headers(self, user_id:
Optional[str] = None, jwt_token: Optional[str] = None) and use jwt_token to
build the "Authorization" header; also rename the parameter in
upload_to_docling_direct_async, get_docling_result_async, _poll_result,
convert_file, and convert_bytes so they accept jwt_token and pass it into
_get_auth_headers; ensure all call sites in document_service.py, processors.py,
langflow_file_service.py, and api/upload.py are updated to pass jwt_token and
that header construction is centralized in _get_auth_headers.
- Around line 93-99: When IBM_AUTH_ENABLED is true and you're building the
headers (the block that checks auth_header and user_id and returns headers), add
logging when either auth_header or user_id is missing so missing credentials are
visible; specifically, inside the IBM_AUTH_ENABLED branch, if auth_header is
falsy log a warning (or debug) mentioning missing Authorization, and if user_id
is falsy log a warning (or debug) mentioning missing X-Tenant-Id, keeping
behavior unchanged otherwise (still only set headers when values exist).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d2201801-3f38-4a62-822a-739883764316
📒 Files selected for processing (5)
src/api/upload.pysrc/models/processors.pysrc/services/docling_service.pysrc/services/document_service.pysrc/services/langflow_file_service.py
…eature flag is on
github-actions
Bot
added
docker
enhancement
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/config/settings.py (1)
371-379:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
docling_http_clientdoesn't honourDOCLING_SERVE_VERIFY_SSL.The
healthendpoint creates a per-requesthttpx.AsyncClient(verify=DOCLING_SERVE_VERIFY_SSL), but the long-liveddocling_http_clientpassed toDoclingServiceis constructed without averify=argument (defaults toTrue). When this client is injected,DoclingService._get_client()returns it directly and never uses its own default client initialization logic. In any deployment where Docling Serve is HTTPS with a self-signed certificate andDOCLING_SERVE_VERIFY_SSL=false, the health check will succeed while every actual conversion/polling call fromDoclingServicewill fail SSL verification.🛠 Proposed fix
self.docling_http_client = httpx.AsyncClient( timeout=httpx.Timeout( timeout=INGESTION_TIMEOUT, connect=30.0, read=INGESTION_TIMEOUT, write=30.0, pool=30.0, - ) + ), + verify=DOCLING_SERVE_VERIFY_SSL, )🤖 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 371 - 379, The long-lived AsyncClient stored as self.docling_http_client is being created without passing the DOCLING_SERVE_VERIFY_SSL setting so it always verifies SSL; update the construction in settings.py to pass verify=DOCLING_SERVE_VERIFY_SSL (or the corresponding boolean constant) to httpx.AsyncClient/httpx.Timeout initialization so DoclingService._get_client receives a client that honors the DOCLING_SERVE_VERIFY_SSL flag; alternatively, modify DoclingService._get_client to wrap or recreate the injected client with verify=DOCLING_SERVE_VERIFY_SSL when the injected client lacks that setting.
🧹 Nitpick comments (1)
src/dependencies.py (1)
124-125: 💤 Low valueMove
import base64to module level.
base64is a stdlib module and should be imported at the top of the file rather than inside a conditional branch.🤖 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/dependencies.py` around lines 124 - 125, Move the standard-library import out of the conditional and place it at module level: remove the inline "import base64" from the block that checks IBM_USERNAME and IBM_PASSWORD and add "import base64" with the other top-level imports; keep the conditional logic that uses IBM_USERNAME and IBM_PASSWORD unchanged so only the import location is modified.
🤖 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/dependencies.py`:
- Around line 122-141: The current env-var override in the top-level auth path
(creating User and setting request.state.user) will unconditionally bypass
per-request auth when IBM_USERNAME/IBM_PASSWORD are set; change this to only run
when IBM_AUTH_DEV_MODE is enabled (i.e., gate the block on IBM_AUTH_DEV_MODE in
addition to IBM_USERNAME and IBM_PASSWORD) and elevate the logger call from
logger.debug to logger.warning to make the override visible; update the block
where User is constructed and request.state.user is set (the code that builds
lh_credentials and sets jwt_token/opensearch_credentials) so it only executes
when IBM_AUTH_DEV_MODE is true.
---
Outside diff comments:
In `@src/config/settings.py`:
- Around line 371-379: The long-lived AsyncClient stored as
self.docling_http_client is being created without passing the
DOCLING_SERVE_VERIFY_SSL setting so it always verifies SSL; update the
construction in settings.py to pass verify=DOCLING_SERVE_VERIFY_SSL (or the
corresponding boolean constant) to httpx.AsyncClient/httpx.Timeout
initialization so DoclingService._get_client receives a client that honors the
DOCLING_SERVE_VERIFY_SSL flag; alternatively, modify DoclingService._get_client
to wrap or recreate the injected client with verify=DOCLING_SERVE_VERIFY_SSL
when the injected client lacks that setting.
---
Nitpick comments:
In `@src/dependencies.py`:
- Around line 124-125: Move the standard-library import out of the conditional
and place it at module level: remove the inline "import base64" from the block
that checks IBM_USERNAME and IBM_PASSWORD and add "import base64" with the other
top-level imports; keep the conditional logic that uses IBM_USERNAME and
IBM_PASSWORD unchanged so only the import location is modified.
🪄 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: f294f043-1aa4-4168-8f3c-d303b21c8302
📒 Files selected for processing (8)
.env.exampledocker-compose.ymlsrc/api/docling.pysrc/config/settings.pysrc/dependencies.pysrc/services/auth_service.pysrc/services/docling_service.pysrc/utils/opensearch_utils.py
✅ Files skipped from review due to trivial changes (1)
- src/services/auth_service.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/services/docling_service.py
github-actions
Bot
added
enhancement
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main.py (2)
262-263: ⚡ Quick winDeduplicate the inline import of
IBM_AUTH_DEV_MODE/IBM_AUTH_ENABLEDinsideinit_index.The same
from config.settings import IBM_AUTH_DEV_MODE, IBM_AUTH_ENABLEDstatement appears twice inside one function (lines 262 and 325). Move it once to the top ofinit_index.♻️ Proposed refactor
async def init_index(opensearch_client=None, admin_username: str = None): """Initialize OpenSearch index and security roles""" os_client = opensearch_client or clients.opensearch + from config.settings import IBM_AUTH_DEV_MODE, IBM_AUTH_ENABLED try: ... else: logger.info( "Index already exists, skipping creation", ... ) - from config.settings import IBM_AUTH_DEV_MODE, IBM_AUTH_ENABLED if not (IBM_AUTH_ENABLED and IBM_AUTH_DEV_MODE): ... ... else: logger.info( "Knowledge filters index already exists, skipping creation", ... ) - from config.settings import IBM_AUTH_DEV_MODE, IBM_AUTH_ENABLED if not (IBM_AUTH_ENABLED and IBM_AUTH_DEV_MODE): ...Also applies to: 325-326
🤖 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/main.py` around lines 262 - 263, The init_index function currently imports IBM_AUTH_DEV_MODE and IBM_AUTH_ENABLED inline in multiple places; remove the duplicate inline imports and place a single "from config.settings import IBM_AUTH_DEV_MODE, IBM_AUTH_ENABLED" once at the top of init_index, then update the conditional checks (the existing if not (IBM_AUTH_ENABLED and IBM_AUTH_DEV_MODE) and the later usage) to reference those imported names without re-importing them.
265-281: ⚡ Quick winBroad
Exceptioncatch flagged by ruff BLE001.Both new
try/exceptblocks (replica normalization for documents index and knowledge-filters index) catchExceptionblindly. The intent is clearly to log-and-continue on startup, which is reasonable, but ruff BLE001 fires on both. Consider narrowing to specific OpenSearch client exceptions (e.g.,opensearchpy.TransportError) or suppressing the lint rule with a targeted# noqa: BLE001comment if the broad catch is intentional.Also applies to: 327-343
🤖 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/main.py` around lines 265 - 281, The try/except around os_client.indices.get_settings / os_client.indices.put_settings is catching broad Exception (raising ruff BLE001); change the except to catch the OpenSearch client-specific error (e.g., opensearchpy.exceptions.TransportError or the appropriate client exception your OpenSearch client uses) so only client errors are swallowed and logged via logger.warning, or if broad catching is intentionally required for startup resilience, add a targeted suppression comment (# noqa: BLE001) immediately on the except line; update both the documents index block (get_settings/put_settings) and the corresponding knowledge-filters block to use the same specific-exception handling or the same noqa suppression.
🤖 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.
Nitpick comments:
In `@src/main.py`:
- Around line 262-263: The init_index function currently imports
IBM_AUTH_DEV_MODE and IBM_AUTH_ENABLED inline in multiple places; remove the
duplicate inline imports and place a single "from config.settings import
IBM_AUTH_DEV_MODE, IBM_AUTH_ENABLED" once at the top of init_index, then update
the conditional checks (the existing if not (IBM_AUTH_ENABLED and
IBM_AUTH_DEV_MODE) and the later usage) to reference those imported names
without re-importing them.
- Around line 265-281: The try/except around os_client.indices.get_settings /
os_client.indices.put_settings is catching broad Exception (raising ruff
BLE001); change the except to catch the OpenSearch client-specific error (e.g.,
opensearchpy.exceptions.TransportError or the appropriate client exception your
OpenSearch client uses) so only client errors are swallowed and logged via
logger.warning, or if broad catching is intentionally required for startup
resilience, add a targeted suppression comment (# noqa: BLE001) immediately on
the except line; update both the documents index block
(get_settings/put_settings) and the corresponding knowledge-filters block to use
the same specific-exception handling or the same noqa suppression.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 91b0c836-07cc-4a72-a711-6fc2fa789f44
📒 Files selected for processing (4)
.env.exampleflows/ingestion_flow.jsonsrc/config/settings.pysrc/main.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/config/settings.py
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
… through an exception' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
github-actions
Bot
added
enhancement
github-actions
Bot
added
ci
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
3 participants
This pull request adds support for passing user authentication information (user ID and JWT token) to the Docling document processing service, enabling per-user authorization and tenant isolation when IBM authentication is enabled. The changes propagate these credentials through the API, document service, and Docling service layers, ensuring that all relevant Docling API calls include the necessary authentication headers.
Authentication and Authorization Enhancements:
user_idandjwt_tokenparameters to document processing methods (process_upload_context,process_document_standard, etc.) and ensured they are passed through the API layer (upload_contextinsrc/api/upload.py) and service layer (src/services/document_service.py,src/models/processors.py). [1] [2] [3] [4]DoclingServicemethods (upload_to_docling_direct_async,get_docling_result_async,convert_file,convert_bytes, and the internal_poll_result) to accept and forward authentication headers to Docling Serve endpoints. [1] [2] [3] [4] [5] [6]_get_auth_headershelper inDoclingServiceto construct authentication headers only when IBM authentication is enabled.File Upload and Ingestion Updates:
Configuration and Import Cleanups:
Summary by CodeRabbit
New Features
Configuration
Behavior