Repository navigation
Conversation
Fix verified RED->GREEN. DashboardFilterStateRestApi 401 on POST/PUT - @has_access_api before @Protect checks anonymous user at api.py:52
Code Review Agent Run #89b9e2Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #43525 +/- ##
==========================================
+ Coverage 78.93% 79.02% +0.09%
==========================================
Files 2877 2879 +2
Lines 165280 165669 +389
Branches 38187 38282 +95
==========================================
+ Hits 130466 130924 +458
+ Misses 32352 32268 -84
- Partials 2462 2477 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| @api | ||
| @has_access_api | ||
| @expose("/<int:pk>/filter_state", methods=("POST",)) |
There was a problem hiding this comment.
Removing has_access_api leaves @api wrapping @protect() on this route. JWT validation errors other than NoAuthorizationError (for example, an expired or malformed bearer token) now fall into @api's broad exception handler and become a 500, while the GET/DELETE routes let Flask-JWT-Extended return the authentication error. Could we remove @api from POST and PUT as well, or otherwise keep @protect() outside that broad handler?
|
|
||
| @api | ||
| @has_access_api | ||
| @expose("/<int:pk>/filter_state", methods=("POST",)) |
There was a problem hiding this comment.
The existing filter-state API tests authenticate POST/PUT with login_as_admin or login_as, so they do not exercise the bearer-token ordering this fixes. Could we add a regression that sends an access token without a browser session, asserts POST returns 201 and PUT returns 200, and would fail again if has_access_api is restored?
Remove @api from post() and put() to match get() and delete(), so JWT validation errors return through flask-jwt-extended's own handlers instead of being caught by @api's broad except and turned into a 500. Also add a regression test that authenticates POST and PUT with a bearer token and no browser session.
|
Done in df93432: removed @api from post() and put() so JWT errors return through flask-jwt-extended's own handling like GET/DELETE, and added test_post_bearer_token_auth / test_put_bearer_token_auth exercising the bearer-token path without a browser session. |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #143409Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
Superseded by #43564, which landed the same decorator fix with broader test coverage. Closing. |
2 participants
Fixes 401 on DashboardFilterStateRestApi POST and PUT.
Problem: POST at api.py:52 and PUT at 177 had
@has_access_apibefore@protect().has_access_apichecks permissions on the current user beforeprotectauthenticates the request, so the permission check runs against an anonymous user and returns 401 even for valid sessions. GET and DELETE already use only@protect()and work correctly.Fix: Remove
@has_access_apifrompost()andput()to matchget()anddelete(). This makes all four methods consistent and lets@protect()handle authentication first. Removes the now unused import.Verification: Static check shows all four methods now expose
@exposefollowed by@protect()with nohas_access_api.py_compilepasses. Diff is 3 deletions in one file.No em dashes used.