Skip to content

Conversation

@cfsmp3
Copy link
Collaborator

@cfsmp3 cfsmp3 commented Jan 19, 2026

Summary

  • Add authentication middleware that validates user sessions
  • Validate that request body credentials match session credentials
  • Prevents users from manipulating credentials to access other users' tasks

Security Issue Addressed

Missing Authorization on API Endpoints (High) - Previously, task endpoints accepted credentials from the request body without validating them against the authenticated session. An attacker could potentially supply different credentials to access or modify other users' tasks.

Changes

  • backend/middleware/auth.go: New authentication middleware that:

    • Validates user has an authenticated session
    • For POST requests, validates credentials in body match session
    • Returns 401 Unauthorized if no session
    • Returns 403 Forbidden if credentials mismatch
    • Logs warnings for mismatch attempts
  • backend/main.go: Apply auth middleware to task endpoints

    • Auth-only: /auth/*, /api/user, /sync/logs
    • Auth + validation: /tasks, /add-task, /edit-task, /modify-task, /complete-task, /delete-task, /complete-tasks, /delete-tasks

Test plan

  • Test task operations work correctly with valid session
  • Verify unauthenticated requests are rejected (401)
  • Verify mismatched credentials are rejected (403)
  • Verify auth endpoints still work without auth middleware

🤖 Generated with Claude Code

Add middleware that validates:
1. User has an authenticated session
2. Credentials in request body match session credentials

This prevents authenticated users from manipulating credentials to
access or modify other users' tasks.

- Add middleware/auth.go with AuthMiddleware
- Apply auth middleware to all task mutation endpoints
- Rate-only endpoints: /auth/*, /api/user, /sync/logs
- Auth + rate endpoints: /tasks, /add-task, /edit-task, etc.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@github-actions
Copy link

Thank you for opening this PR!

Before a maintainer takes a look, it would be really helpful if you could walk through your changes using GitHub's review tools.

Please take a moment to:

  • Check the "Files changed" tab
  • Leave comments on any lines for functions, comments, etc. that are important, non-obvious, or may need attention
  • Clarify decisions you made or areas you might be unsure about and/or any future updates being considered.
  • Finally, submit all the comments!

More information on how to conduct a self review:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request

This helps make the review process smoother and gives us a clearer understanding of your thought process.

Once you've added your self-review, we'll continue from our side. Thank you!


// AuthMiddleware validates that the user is authenticated and that request body
// credentials match the session credentials to prevent unauthorized access.
func AuthMiddleware(store *sessions.CookieStore) func(http.Handler) http.Handler {
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems to work, code is clear as well, but the current approach probably breaks non-browser clients like Postman, or the Flutter App.

The Auth creds validation for the Backend might work for Frontend but can break for Taskwarrior Flutter App, probably we should open an issue there then, to Remove CCSync backend as a way of sync, then replace it with Taskchampion.

The app uses use Taskchampion directly (now), so probably can make it use the CCSync generated creds + the deployed Taskchampion link, bypassing the backend. Can go ahead with this i guess if it works

@cfsmp3
Copy link
Collaborator Author

cfsmp3 commented Jan 19, 2026

Closing in favor of PR #408 which uses the credential injection approach (preferred).

@cfsmp3 cfsmp3 closed this Jan 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants