Skip to content

Commit d27adb3

Browse files
devondragonclaude
andcommitted
Enhance security and IP detection in Spring User Framework
- Improve OAuth2 error handling: replaced exposed exception details with generic user-friendly messages, added proper internal logging - Enhance IP detection: support for additional headers (X-Real-IP, CF-Connecting-IP, True-Client-IP) commonly used by CDNs and load balancers - Add comprehensive DTO validation with GlobalValidationExceptionHandler - Remove security vulnerabilities from authentication entry point 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent d00e649 commit d27adb3

10 files changed

Lines changed: 113 additions & 18 deletions

File tree

FIXES.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,21 +39,25 @@
3939

4040
## Security & API Issues (Priority 2)
4141

42-
### 8. Add DTO validation annotations
42+
### 8. Add DTO validation annotations ✅ COMPLETED
4343
- **Issue**: UserDto and PasswordDto lack bean validation
4444
- **Fix**: Add @NotBlank, @Email, password constraints, create @ControllerAdvice
45+
- **Status**: Fixed - added validation annotations to UserDto and PasswordDto fields, created GlobalValidationExceptionHandler for consistent API error responses
4546

46-
### 9. Fix CSRF property typo
47+
### 9. Fix CSRF property typo ✅ COMPLETED
4748
- **Issue**: Property name contains odd "d" - 'disableCSRFdURIs'
4849
- **Fix**: Rename to 'disableCSRFURIs'
50+
- **Status**: Fixed - corrected property name in WebSecurityConfig, configuration metadata, properties files, and test configuration
4951

50-
### 10. Improve error message handling
52+
### 10. Improve error message handling ✅ COMPLETED
5153
- **Issue**: CustomOAuth2AuthenticationEntryPoint exposes exception details
5254
- **Fix**: Use generic user messages, log details internally
55+
- **Status**: Fixed - replaced exposed exception messages with generic user-friendly messages, detailed exceptions logged internally for debugging, removed debug print statements
5356

54-
### 11. Enhance IP detection
57+
### 11. Enhance IP detection ✅ COMPLETED
5558
- **Issue**: Only honors X-Forwarded-For header
5659
- **Fix**: Support X-Real-IP, CF-Connecting-IP, True-Client-IP
60+
- **Status**: Fixed - enhanced UserUtils.getClientIP() to check multiple headers in order of preference (X-Forwarded-For, X-Real-IP, CF-Connecting-IP, True-Client-IP) with proper validation and fallback
5761

5862
## Web/Security Config (Priority 3)
5963

src/main/java/com/digitalsanctuary/spring/user/dto/PasswordDto.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.digitalsanctuary.spring.user.dto;
22

3+
import jakarta.validation.constraints.NotBlank;
4+
import jakarta.validation.constraints.Size;
35
import lombok.Data;
46

57
/**
@@ -10,9 +12,12 @@
1012
public class PasswordDto {
1113

1214
/** The old password. */
15+
@NotBlank(message = "Current password is required")
1316
private String oldPassword;
1417

1518
/** The new password. */
19+
@NotBlank(message = "New password is required")
20+
@Size(min = 8, max = 128, message = "Password must be between 8 and 128 characters")
1621
private String newPassword;
1722

1823
/** The token. */

src/main/java/com/digitalsanctuary/spring/user/dto/UserDto.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package com.digitalsanctuary.spring.user.dto;
22

3+
import jakarta.validation.constraints.Email;
4+
import jakarta.validation.constraints.NotBlank;
5+
import jakarta.validation.constraints.Size;
36
import lombok.Data;
47

58
/**
@@ -10,18 +13,28 @@
1013
public class UserDto {
1114

1215
/** The first name. */
16+
@NotBlank(message = "First name is required")
17+
@Size(max = 50, message = "First name must not exceed 50 characters")
1318
private String firstName;
1419

1520
/** The last name. */
21+
@NotBlank(message = "Last name is required")
22+
@Size(max = 50, message = "Last name must not exceed 50 characters")
1623
private String lastName;
1724

1825
/** The password. */
26+
@NotBlank(message = "Password is required")
27+
@Size(min = 8, max = 128, message = "Password must be between 8 and 128 characters")
1928
private String password;
2029

2130
/** The matching password. */
31+
@NotBlank(message = "Password confirmation is required")
2232
private String matchingPassword;
2333

2434
/** The email. */
35+
@NotBlank(message = "Email is required")
36+
@Email(message = "Please provide a valid email address")
37+
@Size(max = 100, message = "Email must not exceed 100 characters")
2538
private String email;
2639

2740
/** The role. */
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package com.digitalsanctuary.spring.user.exception;
2+
3+
import java.util.HashMap;
4+
import java.util.Map;
5+
6+
import org.springframework.http.HttpStatus;
7+
import org.springframework.http.ResponseEntity;
8+
import org.springframework.validation.FieldError;
9+
import org.springframework.web.bind.MethodArgumentNotValidException;
10+
import org.springframework.web.bind.annotation.ControllerAdvice;
11+
import org.springframework.web.bind.annotation.ExceptionHandler;
12+
13+
import lombok.extern.slf4j.Slf4j;
14+
15+
/**
16+
* Global exception handler for validation errors. This class handles validation exceptions
17+
* and returns appropriate error responses for API endpoints.
18+
*/
19+
@Slf4j
20+
@ControllerAdvice
21+
public class GlobalValidationExceptionHandler {
22+
23+
/**
24+
* Handles validation errors from @Valid annotations on request bodies.
25+
*
26+
* @param ex the MethodArgumentNotValidException
27+
* @return a ResponseEntity containing validation error details
28+
*/
29+
@ExceptionHandler(MethodArgumentNotValidException.class)
30+
public ResponseEntity<Map<String, Object>> handleValidationExceptions(MethodArgumentNotValidException ex) {
31+
log.warn("Validation error occurred: {}", ex.getMessage());
32+
33+
Map<String, Object> response = new HashMap<>();
34+
Map<String, String> errors = new HashMap<>();
35+
36+
ex.getBindingResult().getAllErrors().forEach((error) -> {
37+
String fieldName = ((FieldError) error).getField();
38+
String errorMessage = error.getDefaultMessage();
39+
errors.put(fieldName, errorMessage);
40+
});
41+
42+
response.put("success", false);
43+
response.put("code", 400);
44+
response.put("message", "Validation failed");
45+
response.put("errors", errors);
46+
47+
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(response);
48+
}
49+
}

src/main/java/com/digitalsanctuary/spring/user/security/CustomOAuth2AuthenticationEntryPoint.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,16 @@ public CustomOAuth2AuthenticationEntryPoint(AuthenticationFailureHandler failure
4343
@Override
4444
public void commence(HttpServletRequest request, HttpServletResponse response, AuthenticationException authException) throws IOException,
4545
ServletException {
46-
log.debug("CustomOAuth2AuthenticationEntryPoint.commence:" + "called with authException: {}", authException);
47-
System.out.println("CustomOAuth2AuthenticationEntryPoint.commence() called with authException: " + authException);
46+
// Log detailed exception information internally for debugging
47+
log.warn("OAuth2 authentication failed: {}", authException.getMessage(), authException);
48+
4849
if (authException instanceof OAuth2AuthenticationException && failureHandler != null) {
4950
// Use the failure handler to handle the exception and perform the redirect
5051
failureHandler.onAuthenticationFailure(request, response, authException);
5152
} else {
52-
// For other exceptions, redirect to the login page
53-
System.out.println("CustomOAuth2AuthenticationEntryPoint.commence() setting error.message: " + authException.getMessage());
54-
request.getSession().setAttribute("error.message", authException.getMessage());
53+
// For other exceptions, redirect to the login page with a generic error message
54+
String userFriendlyMessage = "Authentication failed. Please try again.";
55+
request.getSession().setAttribute("error.message", userFriendlyMessage);
5556
response.sendRedirect(redirectURL);
5657
}
5758
}

src/main/java/com/digitalsanctuary/spring/user/security/WebSecurityConfig.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public class WebSecurityConfig {
6464
@Value("#{'${user.security.unprotectedURIs}'.split(',')}")
6565
private String[] unprotectedURIsArray;
6666

67-
@Value("#{'${user.security.disableCSRFdURIs}'.split(',')}")
67+
@Value("#{'${user.security.disableCSRFURIs}'.split(',')}")
6868
private String[] disableCSRFURIsArray;
6969

7070
@Value("${user.security.loginPageURI}")

src/main/java/com/digitalsanctuary/spring/user/util/UserUtils.java

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,39 @@ private UserUtils() {
1515
}
1616

1717
/**
18-
* Get the client's IP address.
18+
* Get the client's IP address by checking various headers commonly used by proxies, load balancers, and CDNs.
19+
*
20+
* Checks headers in order of preference:
21+
* 1. X-Forwarded-For (standard proxy header)
22+
* 2. X-Real-IP (nginx and other reverse proxies)
23+
* 3. CF-Connecting-IP (Cloudflare)
24+
* 4. True-Client-IP (Akamai, Cloudflare Enterprise)
25+
* 5. Falls back to request.getRemoteAddr()
1926
*
2027
* @param request The HttpServletRequest object.
2128
* @return The client's IP address as a String.
2229
*/
2330
public static String getClientIP(HttpServletRequest request) {
24-
String xfHeader = request.getHeader("X-Forwarded-For");
25-
if (xfHeader != null) {
26-
return xfHeader.split(",")[0];
31+
// Array of header names to check in order of preference
32+
String[] ipHeaders = {
33+
"X-Forwarded-For",
34+
"X-Real-IP",
35+
"CF-Connecting-IP",
36+
"True-Client-IP"
37+
};
38+
39+
for (String header : ipHeaders) {
40+
String ip = request.getHeader(header);
41+
if (ip != null && !ip.isEmpty() && !"unknown".equalsIgnoreCase(ip)) {
42+
// X-Forwarded-For can contain multiple IPs, take the first one
43+
if ("X-Forwarded-For".equals(header)) {
44+
return ip.split(",")[0].trim();
45+
}
46+
return ip.trim();
47+
}
2748
}
49+
50+
// Fall back to remote address if no proxy headers found
2851
return request.getRemoteAddr();
2952
}
3053

src/main/resources/META-INF/additional-spring-configuration-metadata.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,9 @@
106106
"description": "A description for 'user.security.registrationURI'"
107107
},
108108
{
109-
"name": "user.security.disableCSRFdURIs",
109+
"name": "user.security.disableCSRFURIs",
110110
"type": "java.lang.String",
111-
"description": "A description for 'user.security.disableCSRFdURIs'"
111+
"description": "A comma-delimited list of URIs that should not be protected by CSRF protection"
112112
},
113113
{
114114
"name": "user.security.registrationPendingURI",

src/main/resources/config/dsspringuserconfig.properties

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ user.security.unprotectedURIs=/,/index.html,/favicon.ico,/css/*,/js/*,/img/*,/us
5757
# A comma delimited list of URIs that should be protected by Spring Security if the defaultAction is allow.
5858
user.security.protectedURIs=/protected.html
5959
# A comma delimited list of URIs that should not be protected by CSRF protection. This may include API endpoints that need to be called without a CSRF token.
60-
user.security.disableCSRFdURIs=/no-csrf-test
60+
user.security.disableCSRFURIs=/no-csrf-test
6161

6262
# The URI for the login page.
6363
user.security.loginPageURI=/user/login.html

src/test/resources/application-test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ user:
9090
defaultAction: deny # The default action for all requests. This can be either deny or allow.
9191
unprotectedURIs: /,/index.html,/favicon.ico,/css/*,/js/*,/img/*,/user/registration,/user/resendRegistrationToken,/user/resetPassword,/user/registrationConfirm,/user/changePassword,/user/savePassword,/oauth2/authorization/*,/login # A comma delimited list of URIs that should not be protected by Spring Security if the defaultAction is deny.
9292
protectedURIs: /protected.html # A comma delimited list of URIs that should be protected by Spring Security if the defaultAction is allow.
93-
disableCSRFdURIs: /no-csrf-test # A comma delimited list of URIs that should not be protected by CSRF protection. This may include API endpoints that need to be called without a CSRF token.
93+
disableCSRFURIs: /no-csrf-test # A comma delimited list of URIs that should not be protected by CSRF protection. This may include API endpoints that need to be called without a CSRF token.
9494

9595
# URIs for known user related pages. You can change these paths to your own pages if you want.
9696
loginPageURI: /user/login.html # The URI for the login page.

0 commit comments

Comments
 (0)