-
Notifications
You must be signed in to change notification settings - Fork 9
feat: add csrf protection #522
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
1f7d991 to
34fdd11
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements CSRF (Cross-Site Request Forgery) protection for the application by adding CSRF tokens to authentication flows and protected endpoints. The implementation reorganizes middleware into separate auth and security packages, introduces a new CSRF cookie mechanism, and updates both backend and frontend code to handle CSRF tokens.
Key changes:
- Implements CSRF token generation, validation, and middleware protection
- Reorganizes middleware code into
authandsecuritysubpackages with a sharedutilspackage - Integrates CSRF token issuance in login, logout, and token refresh flows
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/http/middleware/security/csrf.go | New CSRF middleware that validates tokens for mutating HTTP methods |
| internal/http/middleware/security/csrf_cookie.go | Manages CSRF cookie lifecycle (issue, read, clear) with secure/insecure modes |
| internal/http/middleware/security/csrf_test.go | Tests for CSRF middleware validation logic |
| internal/http/middleware/security/csrf_cookie_test.go | Tests for CSRF cookie operations |
| internal/http/middleware/security/rate_limiter.go | Moved rate limiter to security package |
| internal/http/middleware/security/rate_limiter_test.go | Updated package declaration for rate limiter tests |
| internal/http/middleware/auth/auth.go | Moved authentication middleware to auth package |
| internal/http/middleware/auth/auth_cookie.go | Refactored to use shared RequireSecure utility |
| internal/http/middleware/auth/auth_cookie_test.go | Updated tests to use utils.RequireSecure |
| internal/http/middleware/utils/require_secure.go | Extracted shared logic for HTTPS detection |
| internal/http/router.go | Updated imports and integrated CSRF middleware into protected routes |
| internal/http/api/login_user.go | Added CSRF token issuance on successful login |
| internal/http/api/logout_user.go | Added CSRF token clearing on logout |
| internal/http/api/refresh_token_user.go | Added CSRF token issuance on token refresh |
| ui/leafwiki-ui/src/lib/api/auth.ts | Added CSRF token extraction from cookies and inclusion in API requests |
Comments suppressed due to low confidence (1)
internal/http/middleware/auth/auth_cookie.go:1
- The package declaration is incorrect. This file is in the
internal/http/middleware/auth/directory and is imported asauth_middleware, so it should declarepackage authinstead ofpackage middleware.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
34fdd11 to
8f41e1e
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
d699da5 to
d62aa2d
Compare
d62aa2d to
cd52c44
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
internal/http/middleware/auth/auth_cookie.go:1
- The package declaration is
package middlewarebut the file is in theinternal/http/middleware/auth/directory. The package name should beauthto match the directory structure and import alias used in router.go. This inconsistency will cause import issues.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
68762af to
25a5aea
Compare
No description provided.