Skip to content

Commit 79faeb1

Browse files
devondragonclaude
andcommitted
Complete all 26 Spring User Framework fixes
Transform from application-focused codebase to production-ready library: High-Impact Issues (Priority 1): - Fix jar artifact naming mismatch (ds-spring-ai-client -> ds-spring-user-framework) - Remove transitive runtime dependencies (move to testRuntimeOnly) - Fix JPA equals/hashCode anti-patterns (exclude relationships, base on id only) - Fix audit log writer concurrency (add synchronized blocks) - Fix registration email base URL (use UserUtils.getAppUrl) - Configure security remember-me properly (opt-in with explicit key) - Remove @async from event classes (false impression on POJOs) Security & API Issues (Priority 2): - Add DTO validation annotations with @ControllerAdvice - Fix CSRF property typo (disableCSRFdURIs -> disableCSRFURIs) - Improve error message handling (generic user messages) - Enhance IP detection (support multiple headers) Web/Security Config (Priority 3): - Fix property injection robustness (filter empty strings) - Configure role hierarchy for method security - Replace System.out.println with SLF4J logging Persistence & Domain (Priority 3): - Clean up User.roles type handling (standardize collection handling) Email & Templates (Priority 3): - Improve MailService error handling (add Spring Retry) - Document Thymeleaf dependency requirements Audit Issues (Priority 4): - Improve audit log defaults (./logs with temp fallback) - Document conditional flushing (@ConditionalOnExpression) Build & Publishing (Priority 4): - Fix group coordinate mismatch (align with publishing) - Dependency management consistency (prefer Boot BOM) - Simplify test task configuration (restore standard test task) UX & Behavior (Priority 4): - Document registration verification flow (auto-enable vs email) - Make post-auth redirects configurable (respect saved requests) - Make global model injection opt-out by default (REST-friendly) Documentation: - Create comprehensive getting started guide with examples 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent d27adb3 commit 79faeb1

18 files changed

Lines changed: 579 additions & 118 deletions

FIXES.md

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -61,76 +61,91 @@
6161

6262
## Web/Security Config (Priority 3)
6363

64-
### 12. Fix property injection robustness
64+
### 12. Fix property injection robustness ✅ COMPLETED
6565
- **Issue**: Empty property yields list with empty string
6666
- **Fix**: Filter empty strings from unprotectedURIs list
67+
- **Status**: Fixed - replaced direct property splitting with helper methods that filter out empty/null values from all URI configuration properties (protectedURIs, unprotectedURIs, disableCSRFURIs)
6768

68-
### 13. Configure role hierarchy for method security
69+
### 13. Configure role hierarchy for method security ✅ COMPLETED
6970
- **Issue**: Method security doesn't use role hierarchy automatically
7071
- **Fix**: Create MethodSecurityExpressionHandler bean with hierarchy
72+
- **Status**: Fixed - added methodSecurityExpressionHandler() bean to WebSecurityConfig that uses the existing role hierarchy
7173

72-
### 14. Replace System.out.println with SLF4J
74+
### 14. Replace System.out.println with SLF4J ✅ COMPLETED
7375
- **Issue**: Using stdout instead of proper logging
7476
- **Fix**: Update CustomOAuth2AuthenticationEntryPoint and TimeLogger
77+
- **Status**: Fixed - replaced System.out.println in TimeLogger with proper SLF4J logging using a default logger when none is provided
7578

7679
## Persistence & Domain (Priority 3)
7780

78-
### 15. Clean up User.roles type handling
81+
### 15. Clean up User.roles type handling ✅ COMPLETED
7982
- **Issue**: Mixed List/Set setters, defensive copying
8083
- **Fix**: Standardize collection handling for JPA dirty checking
84+
- **Status**: Fixed - simplified collection handling to work directly with the underlying Set while maintaining backward compatibility, removed defensive copying that interfered with JPA dirty checking, added smart handling for when the same set object is passed to setters
8185

8286
## Email & Templates (Priority 3)
8387

84-
### 16. Improve MailService error handling
88+
### 16. Improve MailService error handling ✅ COMPLETED
8589
- **Issue**: Exceptions only logged and swallowed
8690
- **Fix**: Add Spring Retry mechanism or queue
91+
- **Status**: Fixed - added Spring Retry support with @Retryable annotations, exponential backoff (1s, 2s, 4s), and @Recover methods for graceful failure handling after all attempts are exhausted
8792

88-
### 17. Document Thymeleaf dependency
93+
### 17. Document Thymeleaf dependency ✅ COMPLETED
8994
- **Issue**: Relies on optional TemplateEngine bean
9095
- **Fix**: Document requirement prominently
96+
- **Status**: Fixed - added prominent documentation in README.md Quick Start section with explicit Maven/Gradle dependencies, updated MailContentBuilder JavaDoc with dependency requirements and exception details, clarified TemplateEngine bean requirement
9197

9298
## Audit Issues (Priority 4)
9399

94-
### 18. Improve audit log defaults
100+
### 18. Improve audit log defaults ✅ COMPLETED
95101
- **Issue**: Default path /opt/app/logs unlikely to be writable
96102
- **Fix**: Use temp directory or auto-create with graceful failure
103+
- **Status**: Fixed - changed default path from `/opt/app/logs` to `./logs` (relative to app directory), added automatic fallback to system temp directory if primary path is not writable, added automatic directory creation, enhanced error handling and logging
97104

98-
### 19. Document conditional flushing
105+
### 19. Document conditional flushing ✅ COMPLETED
99106
- **Issue**: Complex conditional expression hard to understand
100107
- **Fix**: Add clear documentation
108+
- **Status**: Fixed - added comprehensive JavaDoc documentation to FileAuditLogFlushScheduler explaining the complex conditional expression @ConditionalOnExpression("${user.audit.logEvents:true} && !${user.audit.flushOnWrite:true}"), clarified when the scheduler is active and why the conditional logic is structured this way
101109

102110
## Build & Publishing (Priority 4)
103111

104-
### 20. Fix group coordinate mismatch
112+
### 20. Fix group coordinate mismatch ✅ COMPLETED
105113
- **Issue**: group = 'com.digitalsanctuary.springuser' vs publishing 'com.digitalsanctuary'
106114
- **Fix**: Align group with publishing coordinates
115+
- **Status**: Fixed - changed project group from 'com.digitalsanctuary.springuser' to 'com.digitalsanctuary' to match the Maven publishing coordinates
107116

108-
### 21. Dependency management consistency
117+
### 21. Dependency management consistency ✅ COMPLETED
109118
- **Issue**: Mixed explicit versions and BOM usage
110119
- **Fix**: Prefer Boot BOM for all Spring dependencies
120+
- **Status**: Fixed - removed explicit versions from Spring Boot dependencies to use BOM-managed versions, eliminated duplicate spring-boot-starter-actuator dependency, standardized dependency declarations to rely on Spring Boot's dependency management
111121

112-
### 22. Simplify test task configuration
122+
### 22. Simplify test task configuration ✅ COMPLETED
113123
- **Issue**: Overriding test task unusual for library
114124
- **Fix**: Make testAll optional, restore standard test task
125+
- **Status**: Fixed - restored the standard test task to work normally with the default JDK, made testAll an optional task for when multi-JDK testing is desired, removed the unusual override that forced all tests to run with multiple JDKs
115126

116127
## UX & Behavior (Priority 4)
117128

118-
### 23. Document registration verification flow
129+
### 23. Document registration verification flow ✅ COMPLETED
119130
- **Issue**: Auto-enable vs email verification unclear
120131
- **Fix**: Add clear documentation
132+
- **Status**: Fixed - added comprehensive documentation in README.md explaining the two registration modes (Auto-Enable vs Email Verification), their behaviors, configuration options, and when each mode is appropriate
121133

122-
### 24. Make post-auth redirects configurable
134+
### 24. Make post-auth redirects configurable ✅ COMPLETED
123135
- **Issue**: Forces alwaysUseDefaultTargetUrl(true), surprising UX
124136
- **Fix**: Add configuration property
137+
- **Status**: Fixed - added user.security.alwaysUseDefaultTargetUrl configuration property (default: false) to control whether to always redirect to the configured success URL or respect saved requests for better UX. When false, users are redirected to the page they were trying to access before login
125138

126-
### 25. Make global model injection opt-in
139+
### 25. Make global model injection opt-in ✅ COMPLETED
127140
- **Issue**: Adds user to all MVC views by default
128141
- **Fix**: Make opt-in for REST-only apps
142+
- **Status**: Fixed - kept user.web.globalUserModelOptIn default as false (global opt-out mode), added @IncludeUserInModel annotations to existing MVC controllers that need user in model, enhanced documentation to clarify behavior. Now by default, user is NOT added to views unless explicitly requested via annotation, making it suitable for REST-only apps. Added comprehensive comments to prevent future confusion about the naming.
129143

130144
## Documentation
131145

132-
### 26. Create comprehensive getting started guide
146+
### 26. Create comprehensive getting started guide ✅ COMPLETED
133147
- **Fix**: Document required dependencies, minimal properties, examples
148+
- **Status**: Fixed - created comprehensive Quick Start guide in README.md with step-by-step instructions including prerequisites, dependencies, database setup, email configuration, testing steps, customization options, and complete example configurations
134149

135150
## Notes
136151
- All issues have been validated against the codebase

0 commit comments

Comments
 (0)