Small-fixes-improvements #209

devondragon · 2025-09-02T04:26:13Z

No description provided.

- Fix jar artifact naming from ds-spring-ai-client to ds-spring-user-framework - Move transitive runtime dependencies to test scope (devtools, mariadb, postgresql) - Fix JPA equals/hashCode anti-patterns in Role and Privilege entities - Fix audit log writer concurrency with synchronized methods - Fix AuthorityServiceTest by adding IDs to test Privilege objects 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>

- Fix registration email base URL in UserAPI.publishRegistrationEvent - Configure security remember-me as opt-in with explicit key requirement - Remove misleading @async annotations from event POJO classes These changes improve email link generation, security configuration, and remove confusing annotations that have no effect on POJOs. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>

- 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 <[email protected]>

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 <[email protected]>

…te resetPassword method

- Fix NPE in getUserByPasswordResetToken by adding null checks - Normalize emails to lowercase for consistent database lookups - Fix MailService to use consistent constructor injection - Add @ToString.Exclude to password fields to prevent logging sensitive data - Add password match validation with custom annotation @PasswordMatches - Update configuration metadata with correct types and missing properties - Add comprehensive tests for password validation These changes improve null safety, prevent sensitive data exposure in logs, ensure data consistency, and provide better IDE support through corrected configuration metadata. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>

…obalUserModelInterceptor

…ity utilities - Created OAuth2UserTestDataBuilder and OidcUserTestDataBuilder test fixtures - Provides realistic test data for Google, Facebook OAuth2 providers - Supports Keycloak OIDC testing with proper claims and tokens - Follows builder pattern for flexible test data creation - Added DSOAuth2UserServiceTest with 15 tests - Tests Google and Facebook OAuth2 authentication flows - Covers user creation, updates, provider conflicts - Validates error handling for unsupported providers - Tests real business logic, not just mock interactions - Added DSOidcUserServiceTest with 14 tests - Tests Keycloak OIDC authentication flows - Validates OIDC-specific user extraction methods - Tests DSUserDetails integration with OIDC tokens - Covers provider conflict scenarios - Added UserUtilsTest with 29 tests - Comprehensive IP extraction from proxy headers - Tests security-critical header priority ordering - Validates URL building with various configurations - Tests edge cases and security considerations Increased test count from 193 to 251 (58 new tests) All tests passing with 100% success rate 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>

- Created LoginHelperServiceTest with 15 tests covering critical authentication flow - Tests last activity date tracking on login - Tests automatic user unlocking after lockout duration - Tests DSUserDetails creation with proper authorities - Tests handling of locked, disabled, and OAuth2 users - Tests role and privilege assignment - Tests edge cases like null dates and rapid successive logins - All tests verify real business logic, not just mock interactions - Tests provide valuable coverage for authentication helper operations - Ensures proper integration with LoginAttemptService and AuthorityService Test count increased from 251 to 266 (15 new tests) All tests passing with 100% success rate 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>

- Added 24 tests covering mail sending functionality - Tests for simple and template-based messages - Tests for async behavior and retry mechanisms - Tests for error handling and recovery methods - Tests for edge cases with special characters and null values - All tests pass and provide real value testing actual business logic

- Added 17 tests covering logout success handling - Tests for audit event creation and publishing - Tests for IP address extraction from various headers - Tests for target URL resolution logic - Tests for exception handling scenarios - Integration tests for complete logout flow - All tests pass and provide real business value

- Added 15 tests for RolePrivilegeSetupService covering role and privilege initialization - Tests for handling empty/null configurations and mixed existing/new entities - Tests for database transaction handling and privilege reuse across roles - Added 21 tests for CustomOAuth2AuthenticationEntryPoint - Tests for OAuth2 and non-OAuth2 exception handling - Tests for failure handler delegation and redirect behavior - Tests for session handling and URL configuration - All tests pass and provide real value testing actual business logic

Security Fixes: - Add email validation and normalization for OAuth2/OIDC authentication - Remove session IDs from debug logs to prevent sensitive data exposure - Implement proxy-aware URL building with X-Forwarded headers support Bug Fixes: - Add password match validation in user registration - Fix NPE protection in getUserByPasswordResetToken - Remove problematic @ConditionalOnMissingBean annotations that conflicted with Spring Boot auto-configuration - Ensure URL building maintains backward compatibility by always including ports Code Quality Improvements: - Replace string concatenation with parameterized logging throughout - Add comprehensive JavaDoc documentation to JSONResponse class - Improve error messages for OAuth2 authentication failures - Add proper email normalization (lowercase) for OAuth2/OIDC providers Testing: - All tests passing after fixes - Fixed UserUtils URL tests to work with proxy-aware implementation - Fixed UserUpdateIntegrationTest by removing bean configuration conflicts 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>

Remove extra closing </p> tag that was causing JavaDoc generation to fail. The </pre> tag should not be followed by </p> as it's already within a properly closed paragraph block. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>

Copilot

Pull Request Overview

This PR contains small fixes and improvements across multiple components including configuration updates, test coverage additions, and minor code corrections.

Fixes typos and improves configuration property documentation
Adds comprehensive test coverage for multiple service classes and utilities
Corrects property name inconsistencies and adds missing validation components

Reviewed Changes

Copilot reviewed 50 out of 51 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/test/resources/application-test.yml	Updates audit log path and fixes CSRF URI property name
src/test/java/*/Test.java	Adds comprehensive test classes for password validation, utilities, services, and security components
src/test/java/*/fixtures/.java	Adds test data builders for OAuth2 and OIDC user testing
src/main/resources/config/dsspringuserconfig.properties	Improves property documentation and fixes typos
src/main/resources/META-INF/additional-spring-configuration-metadata.json	Corrects property names and improves type definitions
src/main/java/com/digitalsanctuary/spring/user/web/*.java	Improves documentation and annotations
src/main/java/com/digitalsanctuary/spring/user/validation/*.java	Adds missing password validation classes
src/main/java/com/digitalsanctuary/spring/user/util/UserUtils.java	Enhances IP and URL extraction logic

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-03T20:10:36Z

src/test/java/com/digitalsanctuary/spring/user/service/AuthorityServiceTest.java

    void getAuthoritiesFromRoles_mixedCasePrivileges_preservesCase() {
        // Given
        Privilege lowerPrivilege = new Privilege("read_privilege");
+        lowerPrivilege.setId(1L);


[nitpick] These ID assignments should be grouped together or extracted to a setup method to improve code organization and reduce duplication of the ID assignment pattern.

Copilot · 2025-09-03T20:10:36Z

src/test/java/com/digitalsanctuary/spring/user/service/AuthorityServiceTest.java

        Privilege lowerPrivilege = new Privilege("read_privilege");
+        lowerPrivilege.setId(1L);
        Privilege upperPrivilege = new Privilege("WRITE_PRIVILEGE");
+        upperPrivilege.setId(2L);


[nitpick] These ID assignments should be grouped together or extracted to a setup method to improve code organization and reduce duplication of the ID assignment pattern.

Copilot · 2025-09-03T20:10:37Z

src/test/java/com/digitalsanctuary/spring/user/service/AuthorityServiceTest.java

        Privilege upperPrivilege = new Privilege("WRITE_PRIVILEGE");
+        upperPrivilege.setId(2L);
        Privilege mixedPrivilege = new Privilege("Delete_Privilege");
+        mixedPrivilege.setId(3L);


[nitpick] These ID assignments should be grouped together or extracted to a setup method to improve code organization and reduce duplication of the ID assignment pattern.

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

Copilot · 2025-09-03T20:10:37Z

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

+			if (colonIndex > 0) {
+				host = host.substring(0, colonIndex);


Consider using colonIndex != -1 instead of colonIndex > 0 to handle edge case where colon is the first character, and validate the extracted host is not empty.

Suggested change

if (colonIndex > 0) {

host = host.substring(0, colonIndex);

if (colonIndex != -1) {

String extractedHost = host.substring(0, colonIndex);

if (!extractedHost.isEmpty()) {

host = extractedHost;

} else {

// If extracted host is empty, fall back to server name

host = request.getServerName();

}

…java Co-authored-by: Copilot <[email protected]>

…er checks

devondragon and others added 5 commits September 1, 2025 18:59

Add PasswordResetRequestDto for password reset functionality and upda…

1336559

…te resetPassword method

Copilot AI review requested due to automatic review settings September 2, 2025 04:26

devondragon self-assigned this Sep 2, 2025

devondragon added the enhancement New feature or request label Sep 2, 2025

This comment was marked as outdated.

Sign in to view

devondragon and others added 11 commits September 1, 2025 22:39

Refactor WebSecurityConfig for improved readability and efficiency

e851ead

Add nullability annotations to preHandle and postHandle methods in Gl…

9137119

…obalUserModelInterceptor

Update README and LoginHelperServiceTest for clarity and version bump

c806854

devondragon requested a review from Copilot September 3, 2025 20:09

Copilot AI reviewed Sep 3, 2025

View reviewed changes

devondragon and others added 2 commits September 3, 2025 14:11

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

d01a724

…java Co-authored-by: Copilot <[email protected]>

Refactor UserUtils: Improve JavaDoc formatting and streamline IP head…

07b2703

…er checks

devondragon merged commit 033706a into main Sep 3, 2025
3 checks passed

devondragon deleted the small-fixes-improvements branch September 3, 2025 20:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Small-fixes-improvements #209

Small-fixes-improvements #209

Uh oh!

devondragon commented Sep 2, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 3, 2025

Uh oh!

Copilot AI Sep 3, 2025

Uh oh!

Copilot AI Sep 3, 2025

Uh oh!

Uh oh!

Copilot AI Sep 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-			if (colonIndex > 0) {
-				host = host.substring(0, colonIndex);
+			if (colonIndex != -1) {
+				String extractedHost = host.substring(0, colonIndex);
+				if (!extractedHost.isEmpty()) {
+					host = extractedHost;
+				} else {
+					// If extracted host is empty, fall back to server name
+					host = request.getServerName();
+				}

Small-fixes-improvements #209

Small-fixes-improvements #209

Uh oh!

Conversation

devondragon commented Sep 2, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants