Skip to content
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

fix(login): fixing rememberMe #31530

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

wezell
Copy link
Contributor

@wezell wezell commented Mar 3, 2025

This pull request includes several changes aimed at improving the handling of JWT tokens and removing deprecated interceptors. The most important changes include encrypting user IDs in JWT tokens, updating methods for creating and processing remember-me cookies, and removing obsolete classes.

JWT Token Enhancements:

Cookie and Session Management:

Removal of Deprecated Interceptors:

Default Auto Login Interceptor Update:

Copy link

Please use a Conventional Commit title format for this PR. For more information, see https://www.conventionalcommits.org/en/v1.0.0/

@wezell wezell changed the title fix(tests) fixing rememberMe fix(tests): fixing rememberMe Mar 3, 2025
@wezell wezell linked an issue Mar 3, 2025 that may be closed by this pull request
@wezell wezell changed the title fix(tests): fixing rememberMe fix(login): fixing rememberMe Mar 3, 2025
@wezell wezell requested a review from Copilot March 3, 2025 20:13
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR improves the login flow by enhancing JWT token encryption/decryption, updating remember-me cookie handling, and removing obsolete interceptor classes.

  • Updated DefaultAutoLoginWebInterceptor to process remember-me tokens and refresh them if needed
  • Modified JWT token generation to encrypt user IDs and updated user retrieval logic in JWToken interface
  • Removed deprecated interceptors and streamlined login flows in LoginServiceAPIFactory and LoginFactory

Reviewed Changes

File Description
dotCMS/src/main/java/com/dotcms/filters/interceptor/dotcms/DefaultAutoLoginWebInterceptor.java Updated token handling and added token refresh logic based on JWT expiration
dotCMS/src/main/java/com/dotcms/auth/providers/jwt/factories/JsonWebTokenFactory.java Adjusted expiresDate conversion for consistency with Date type
dotCMS/src/main/java/com/dotcms/auth/providers/jwt/beans/JWToken.java Added decryption for user IDs with logging on failure
dotCMS/src/main/java/com/dotcms/cms/login/LoginServiceAPIFactory.java Renamed token processing method and updated remember-me cookie creation logic
dotCMS/src/main/java/com/dotmarketing/cms/login/factories/LoginFactory.java Modified doCookieLogin method to use userId directly and log additional details
dotCMS/src/main/java/com/dotcms/filters/interceptor/jwt/JsonWebTokenInterceptor.java Removed the obsolete JWT interceptor class
dotCMS/src/main/java/com/liferay/util/LocaleUtil.java Updated locale determination logic using PortalUtil.getUser(request)
dotcms-integration/src/test/java/com/dotcms/MainSuite1a.java Removed obsolete JWT interceptor integration tests

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

dotCMS/src/main/java/com/dotmarketing/cms/login/factories/LoginFactory.java:95

  • There is an extra '+' in the log message ('id:+'); consider removing the extra '+' to improve the log clarity.
SecurityLogger.logInfo(LoginFactory.class,"Successful login name:" + user.getFullName() + " id:+" + user.getUserId() + " email:" + user.getEmailAddress());

dotCMS/src/main/java/com/dotmarketing/cms/login/factories/LoginFactory.java:88

  • [nitpick] The attribute WebKeys.CMS_USER is set multiple times in this code block; consider consolidating it to a single call to avoid redundancy.
session.setAttribute(WebKeys.CMS_USER, user);

}
// if the token was expiry date is greater than the allowed EXPIREY date, reset it
// maybe someone updated the configured MAX_AGE_DAYS
if(token.get().getExpiresDate().after(Date.from(Instant.now().plus(jwtMaxAgeInMillis, ChronoUnit.MILLIS)))) {
Copy link
Preview

Copilot AI Mar 3, 2025

Choose a reason for hiding this comment

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

The comment above this block contains a spelling error ('EXPIREY'); please correct it to 'expiry' for clarity.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
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.

Remember Me Forgets Me
1 participant