fix: correct decorator order in prompt-service so auth_required is enforced#1964
fix: correct decorator order in prompt-service so auth_required is enforced#1964alpakalee wants to merge 1 commit into
Conversation
…forced Python applies decorators bottom-up. Placing @auth_required above @route caused Flask to register the raw handler function — the auth wrapper was never invoked on incoming requests. Swap the order in all three affected controllers so the authenticated wrapper is what Flask stores in its URL map. Affected files: - controllers/extraction.py (POST /extract) - controllers/indexing.py (POST /index) - controllers/answer_prompt.py (POST /answer-prompt)
|
|
Summary by CodeRabbit
WalkthroughThree Flask controller routes have their decorators reordered: the route decorator ( ChangesFlask Route Decorator Order
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
|
| Filename | Overview |
|---|---|
| prompt-service/src/unstract/prompt_service/controllers/extraction.py | Decorator order corrected so Flask registers the auth wrapper; endpoint is now named wrapper due to missing functools.wraps in auth_required. |
| prompt-service/src/unstract/prompt_service/controllers/indexing.py | Decorator order corrected; same functools.wraps concern applies as in extraction.py. |
| prompt-service/src/unstract/prompt_service/controllers/answer_prompt.py | Decorator order corrected; same functools.wraps concern applies as in extraction.py. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming POST request] --> B{Flask URL Map}
B --> C[auth_required wrapper]
C --> D{Token present and valid?}
D -- No --> E[Return 401]
D -- Yes --> F[Original route handler]
F --> G[Return response]
style E fill:#f66,color:#fff
style G fill:#6a6,color:#fff
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
prompt-service/src/unstract/prompt_service/controllers/extraction.py:21-23
**Missing `functools.wraps` in `auth_required` changes Flask endpoint names**
Now that `@route` is applied after `@AuthHelper.auth_required`, Flask registers the inner `wrapper` function (returned by `auth_required`) rather than the original handler. Because `auth.py`'s `auth_required` does not call `@functools.wraps(func)` on its wrapper, the three endpoints are now registered under the endpoint name `"wrapper"` instead of `"extract"`, `"index"`, and `"prompt_processor"`. Each lives in a separate blueprint so there is no immediate collision here, but if a second `@auth_required`-decorated route is ever added to any of these blueprints the app will crash at startup with a duplicate-endpoint `AssertionError`. Adding `@functools.wraps(func)` to the `wrapper` inside `AuthHelper.auth_required` in `helpers/auth.py` fixes this for all three controllers at once.
Reviews (1): Last reviewed commit: "fix: correct decorator order in prompt-s..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @extraction_bp.route("/extract", methods=["POST"]) | ||
| @AuthHelper.auth_required | ||
| def extract() -> Any: |
There was a problem hiding this comment.
functools.wraps in auth_required changes Flask endpoint names
Now that @route is applied after @AuthHelper.auth_required, Flask registers the inner wrapper function (returned by auth_required) rather than the original handler. Because auth.py's auth_required does not call @functools.wraps(func) on its wrapper, the three endpoints are now registered under the endpoint name "wrapper" instead of "extract", "index", and "prompt_processor". Each lives in a separate blueprint so there is no immediate collision here, but if a second @auth_required-decorated route is ever added to any of these blueprints the app will crash at startup with a duplicate-endpoint AssertionError. Adding @functools.wraps(func) to the wrapper inside AuthHelper.auth_required in helpers/auth.py fixes this for all three controllers at once.
Prompt To Fix With AI
This is a comment left during a code review.
Path: prompt-service/src/unstract/prompt_service/controllers/extraction.py
Line: 21-23
Comment:
**Missing `functools.wraps` in `auth_required` changes Flask endpoint names**
Now that `@route` is applied after `@AuthHelper.auth_required`, Flask registers the inner `wrapper` function (returned by `auth_required`) rather than the original handler. Because `auth.py`'s `auth_required` does not call `@functools.wraps(func)` on its wrapper, the three endpoints are now registered under the endpoint name `"wrapper"` instead of `"extract"`, `"index"`, and `"prompt_processor"`. Each lives in a separate blueprint so there is no immediate collision here, but if a second `@auth_required`-decorated route is ever added to any of these blueprints the app will crash at startup with a duplicate-endpoint `AssertionError`. Adding `@functools.wraps(func)` to the `wrapper` inside `AuthHelper.auth_required` in `helpers/auth.py` fixes this for all three controllers at once.
How can I resolve this? If you propose a fix, please make it concise.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
prompt-service/src/unstract/prompt_service/controllers/indexing.py (1)
35-36: ⚡ Quick winConsider adding a test to verify auth enforcement.
To prevent regression and confirm the fix works as intended, consider adding an integration test that verifies these endpoints return 401 when called without a valid Authorization header.
Example test structure:
def test_endpoint_requires_auth(client): """Verify endpoint returns 401 without auth header.""" response = client.post('/index', json={...}) assert response.status_code == 401🤖 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 `@prompt-service/src/unstract/prompt_service/controllers/indexing.py` around lines 35 - 36, Add an integration test that exercises the indexing endpoint handler decorated by indexing_bp.route("/index", methods=["POST"]) and protected by AuthHelper.auth_required: create a test (e.g., test_index_endpoint_requires_auth) that uses the test client to POST to "/index" without an Authorization header and asserts response.status_code == 401; also add a complementary positive test that includes a valid Authorization header to confirm successful auth handling. Ensure the test file imports the app/test client fixture and targets the route guarded by AuthHelper.auth_required so future regressions to auth enforcement are caught.
🤖 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 `@prompt-service/src/unstract/prompt_service/controllers/indexing.py`:
- Around line 35-36: Add an integration test that exercises the indexing
endpoint handler decorated by indexing_bp.route("/index", methods=["POST"]) and
protected by AuthHelper.auth_required: create a test (e.g.,
test_index_endpoint_requires_auth) that uses the test client to POST to "/index"
without an Authorization header and asserts response.status_code == 401; also
add a complementary positive test that includes a valid Authorization header to
confirm successful auth handling. Ensure the test file imports the app/test
client fixture and targets the route guarded by AuthHelper.auth_required so
future regressions to auth enforcement are caught.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 24b979fc-c32d-4c8e-883d-37f365b29bf9
📒 Files selected for processing (3)
prompt-service/src/unstract/prompt_service/controllers/answer_prompt.pyprompt-service/src/unstract/prompt_service/controllers/extraction.pyprompt-service/src/unstract/prompt_service/controllers/indexing.py
2 participants
What
Swaps the
@auth_required/@routedecorator order in threeprompt-servicecontrollersso that Flask registers the authenticated wrapper, not the raw handler function.
Why
Python applies stacked decorators bottom-up. The previous order placed
@auth_requiredon top:
Flask finalises its URL map at import time when
@routeis applied.@auth_requiredisapplied afterward, updating only the module-level variable — the URL map already holds the
raw function. As a result, the auth wrapper is never invoked on any incoming request.
Correct order (route first, then auth):
How
controllers/extraction.pycontrollers/indexing.pycontrollers/answer_prompt.pyCan this PR break any existing features. If yes, please list possible items. If no, please explain why.
No. The same auth logic is applied — this fix makes it actually run for the first time.
No behavioural changes beyond enforcing the token check that was already implemented.
Database Migrations
N/A
Env Config
N/A
Relevant Docs
N/A
Related Issues or PRs
N/A
Dependencies Versions
N/A
Notes on Testing
platform-serviceandx2text-servicein this repo already use the correct order(
@routeabove auth). This change bringsprompt-servicein line with the same pattern.Checklist
I have read and understood the Contribution Guidelines.