-
Notifications
You must be signed in to change notification settings - Fork 4
Implement Remember Me Feature for Login #101
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
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 pull request implements the “Remember Me” feature for the login functionality by adding a frontend checkbox, updating the security configuration, and modifying the login controller behavior to set a remember me cookie upon authentication.
- Updated the login form (login.html) to include a “Remember Me” checkbox.
- Modified SecurityConfig.java to configure rememberMe support.
- Adjusted AppController.java to read the checkbox value and add a cookie for persistent login.
- Added or updated tests in JUnit5ExampleTest12.java to exercise related features.
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/test/java/net/codejava/JUnit5ExampleTest12.java | Contains tests for various search feature defaults and behaviors (not directly related to remember me). |
src/main/resources/templates/login.html | Added the Remember Me checkbox input element. |
src/main/java/net/codejava/SecurityConfig.java | Configured the rememberMe settings (key, parameter, token validity). |
src/main/java/net/codejava/AppController.java | Updated the loginPost method to conditionally add a remember me cookie when enabled. |
Files not reviewed (1)
- build_and_run_app.sh: Language not supported
javax.servlet.http.Cookie rememberMeCookie = new javax.servlet.http.Cookie("rememberMe", "true"); | ||
rememberMeCookie.setMaxAge(7 * 24 * 60 * 60); // 7 days | ||
rememberMeCookie.setHttpOnly(true); | ||
rememberMeCookie.setPath("/"); | ||
response.addCookie(rememberMeCookie); |
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.
The remember me cookie is not marked as Secure despite the PR description indicating a secure cookie. Consider adding rememberMeCookie.setSecure(true) to ensure cookie security over HTTPS.
javax.servlet.http.Cookie rememberMeCookie = new javax.servlet.http.Cookie("rememberMe", "true"); | |
rememberMeCookie.setMaxAge(7 * 24 * 60 * 60); // 7 days | |
rememberMeCookie.setHttpOnly(true); | |
rememberMeCookie.setPath("/"); | |
response.addCookie(rememberMeCookie); | |
javax.servlet.http.Cookie rememberMeCookie = new javax.servlet.http.Cookie("rememberMe", "true"); | |
rememberMeCookie.setMaxAge(7 * 24 * 60 * 60); // 7 days | |
rememberMeCookie.setHttpOnly(true); | |
rememberMeCookie.setSecure(true); // Ensure the cookie is only sent over HTTPS | |
rememberMeCookie.setPath("/"); | |
response.addCookie(rememberMeCookie); |
Copilot uses AI. Check for mistakes.
rememberMeCookie.setMaxAge(7 * 24 * 60 * 60); // 7 days | ||
rememberMeCookie.setHttpOnly(true); | ||
rememberMeCookie.setPath("/"); | ||
response.addCookie(rememberMeCookie); |
Check warning
Code scanning / CodeQL
Failure to use secure cookies Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix the issue, the secure
flag must be explicitly set on the rememberMeCookie
before adding it to the response. This ensures that the cookie is only transmitted over secure HTTPS connections. The change involves calling the setSecure(true)
method on the rememberMeCookie
object before the response.addCookie(rememberMeCookie)
line.
-
Copy modified lines R180-R185
@@ -179,7 +179,8 @@ | ||
// Set a cookie for "Remember Me" | ||
javax.servlet.http.Cookie rememberMeCookie = new javax.servlet.http.Cookie("rememberMe", "true"); | ||
rememberMeCookie.setMaxAge(7 * 24 * 60 * 60); // 7 days | ||
rememberMeCookie.setHttpOnly(true); | ||
rememberMeCookie.setPath("/"); | ||
response.addCookie(rememberMeCookie); | ||
javax.servlet.http.Cookie rememberMeCookie = new javax.servlet.http.Cookie("rememberMe", "true"); | ||
rememberMeCookie.setMaxAge(7 * 24 * 60 * 60); // 7 days | ||
rememberMeCookie.setHttpOnly(true); | ||
rememberMeCookie.setPath("/"); | ||
rememberMeCookie.setSecure(true); // Ensure the cookie is only sent over HTTPS | ||
response.addCookie(rememberMeCookie); | ||
} |
This pull request implements the "Remember Me" feature for the login functionality:
Backend Changes:
AppController.java
to set a secure cookie when the "Remember Me" checkbox is selected during login.Frontend Changes:
login.html
to include the "Remember Me" checkbox in the login form.Please review the changes and provide feedback.