Skip to content

Uses the same authentication decorator on all modules#1004

Open
poupounetjoyeux wants to merge 3 commits intocrocodilestick:mainfrom
poupounetjoyeux:main
Open

Uses the same authentication decorator on all modules#1004
poupounetjoyeux wants to merge 3 commits intocrocodilestick:mainfrom
poupounetjoyeux:main

Conversation

@poupounetjoyeux
Copy link

@poupounetjoyeux poupounetjoyeux commented Feb 1, 2026

Make decarotar name more reliable
Remove some code duplication regarding anonymous check
Makes KOSync using standard decorators

@crocodilestick
Copy link
Owner

Hey, just did a review of this and there are a few issues:

  1. KOSync endpoints lost CSRF exemption. With global CSRF enabled, PUT /kosync/syncs/progress will be rejected without a CSRF token, breaking KOReader sync. This is visible at kosync.py:368-470
  2. Unauthorized KOSync requests now return the HTTPBasicAuth default response instead of the JSON error format promised by KOSync.
    • basic_auth_required returns auth.auth_error_callback directly, bypassing the KOSync JSON error handlers and the documented {"error": 2001, "message": "Unauthorized"} behavior.
    • See usermanagement.py:143-166 and KOSync routes in kosync.py:368-470

If you can resolve these i'll take another look

@crocodilestick crocodilestick added the in progress A fix is currently being worked on / is in testing label Feb 2, 2026
@poupounetjoyeux
Copy link
Author

Hello @crocodilestick,

Thank you to have take the time starting the review!

I repushed (and rebase) regarding your comments with a dedicated error handler for KOSync
Just deployed my custom image and it seems to works as expected 👍

Don't hesitate if you see other things to update

Add an error handler parameter to generate proper errors on KOSync side
Move functions location and add headers for more reliability
Make new require_kosync_enabled method a decorator
@poupounetjoyeux
Copy link
Author

Hello @crocodilestick

I just rebased and deployed it locally, all is OK 👌

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

Labels

in progress A fix is currently being worked on / is in testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants