fix: improve authentication flow and session handling #45
Conversation
- Add authorization check after user authentication - Fix redirect URL handling in session management - Refactor template rendering into dedicated methods - Improve logout behavior to redirect to login page - Clean up session management by deleting specific keys instead of clearing all
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull Request Overview
This PR improves the authentication flow and session handling by adding proper authorization checks after authentication, fixing redirect URL management, and refactoring template rendering methods for better code organization.
- Added authorization verification in the OAuth callback handler to ensure authenticated users are also authorized
- Fixed session management by properly deleting specific session keys and handling redirect URLs correctly
- Refactored template rendering into dedicated methods for better maintainability
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/auth/auth.go | Added authorization checks, improved session handling, refactored template rendering methods, and enhanced logout behavior |
| pkg/auth/auth_test.go | Updated test expectations to reflect authorization flow changes and moved mock setup for better organization |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| c.Redirect(http.StatusFound, "/") | ||
| } else { | ||
| c.Redirect(http.StatusFound, redirectURL.(string)) | ||
| } |
There was a problem hiding this comment.
The redirect URL is deleted from the session before checking if it's nil, but the check happens after the session is saved. This could cause issues if the session save fails. Consider moving the session.Save() call after the redirect logic.
| c.Redirect(http.StatusFound, "/") | ||
| return | ||
| } else { | ||
| c.Redirect(http.StatusFound, redirectURL.(string)) |
There was a problem hiding this comment.
Same issue as in the OAuth callback: the redirect URL is deleted and session saved before the redirect logic. If session.Save() fails, the redirect URL would be lost but the redirect might not happen correctly.
Summary
Improves the authentication flow and session handling in the auth router by adding proper authorization checks, fixing redirect URL management, and refactoring template rendering for better maintainability.
Type of Change
Related Issues
Changes Made
renderLoginandrenderUnauthorized) for better code organizationtemplatefield tologinTemplatefor clarityTesting