5.0.0: Tier 2 security & library-citizenship remediation (breaking) + Spring Boot 4.1.0#318
Merged
Conversation
…ATION sections Begin the Tier 2 / 5.0.0 breaking-change work off the 4.4.0 tag. Seeds the [5.0.0] CHANGELOG section and a 'Migrating to 5.0.x' guide with the prominent reverse-proxy ACTION REQUIRED notice (Task 1.3). Notes that audit-log JSON-per-line (Task 4.3) is intentionally dropped (injection already fixed via sanitization in 4.4.0).
…owlist to prevent reset poisoning (H2, CWE-640)
…ken, VerificationToken) Adds @column(nullable=false, unique=true) to the token field of both PasswordResetToken and VerificationToken. Token values have been hashed at rest since 4.4.0, so the fixed-length values are safe to index. Includes a @databasetest (H2 slice) that verifies the constraint is enforced on flush for both tables. Updates CHANGELOG.md (Breaking Changes) and MIGRATION.md (DDL migration guidance for Flyway/Liquibase users).
…vent account enumeration
…move/set password, passkey delete/rename)
…multi-node startup
… safe toString
…tached-entity hazards
…ss-level constraint as 400
…move empty api.data package - Move GlobalValidationExceptionHandler from ...exception (singular) to ...exceptions (plural), updating its package declaration. The ...exception package is removed. - Move GlobalValidationExceptionHandlerScopeTest to match the new package location. - Delete the empty, unreferenced src/main/.../api/data/Response.java (0 bytes) and its directory. - Document in CHANGELOG.md (Breaking Changes) and MIGRATION.md (Package consolidation subsection).
…tion); namespace library bean names
Incorporates dependabot PR #317 into the 5.0.0 line. Library starters remain compileOnly; full check (939 tests) passes against Spring Boot 4.1.0.
Contributor
There was a problem hiding this comment.
Pull request overview
Major 5.0.0 (breaking) remediation release focused on tightening security boundaries (host-header poisoning defense, anti-enumeration responses, re-auth for credential-altering ops), improving “library citizenship” (scoped @ControllerAdvice, additive message bundles, namespaced bean names), and addressing performance/data correctness (LAZY roles/privileges with entity-graph on auth paths, schema uniqueness constraints), while updating validation/docs/tests and verifying Spring Boot 4.1.0 compatibility.
Changes:
- Introduces canonical app-url resolution (
user.security.appUrl/user.security.trustedHosts) and updates email-link generation paths to avoid forwarded-host poisoning. - Makes roles/privileges LAZY with a new
@EntityGraphrepository finder for auth, plus schema uniqueness constraints and startup idempotence under multi-node races. - Refactors events to carry ids/scalars (not entities), scopes validation advice to library controllers, and makes message bundle registration additive via an
EnvironmentPostProcessor.
Reviewed changes
Copilot reviewed 76 out of 78 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/resources/test-messages/consumer-bundle.properties | Test consumer i18n bundle for additive message-source verification. |
| src/test/java/com/digitalsanctuary/spring/user/util/AppUrlResolverTest.java | Unit coverage for app-url resolution and forwarded-host allowlisting behavior. |
| src/test/java/com/digitalsanctuary/spring/user/UserConfigurationToggleTest.java | Verifies opt-out toggles for async/retry/scheduling/method-security nested configs. |
| src/test/java/com/digitalsanctuary/spring/user/service/UserUpdateIntegrationTest.java | Adjusts role setup in tests to respect new unique role-name constraints. |
| src/test/java/com/digitalsanctuary/spring/user/service/UserServiceTest.java | Updates tests for id/scalar-based events and auth authority sourcing changes. |
| src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java | Adds coverage for id-based registration email sending (entity reload). |
| src/test/java/com/digitalsanctuary/spring/user/service/TokenHashingSecurityTest.java | Updates tests for constructor changes and email-service dependencies. |
| src/test/java/com/digitalsanctuary/spring/user/service/DSUserDetailsServiceTest.java | Switches to entity-graph finder in auth path tests. |
| src/test/java/com/digitalsanctuary/spring/user/service/DSOidcUserServiceTest.java | Updates OAuth/OIDC flows for entity-graph finder + scalar event payloads. |
| src/test/java/com/digitalsanctuary/spring/user/service/DSOidcUserServiceRegistrationGuardTest.java | Updates registration-guard tests for new user lookup method. |
| src/test/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserServiceTest.java | Updates OAuth2 flows for entity-graph lookup + scalar event payloads. |
| src/test/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserServiceRegistrationGuardTest.java | Updates registration-guard tests for new user lookup method. |
| src/test/java/com/digitalsanctuary/spring/user/roles/RolePrivilegeSetupServiceTest.java | Adds race-handling tests and updates persistence interactions to saveAndFlush. |
| src/test/java/com/digitalsanctuary/spring/user/persistence/repository/UserRepositoryEntityGraphTest.java | DB-slice regression guard for LAZY roles/privileges + bounded queries via entity graph. |
| src/test/java/com/digitalsanctuary/spring/user/persistence/model/TokenUniquenessTest.java | DB-slice tests for UNIQUE+NOT NULL token constraints. |
| src/test/java/com/digitalsanctuary/spring/user/persistence/model/RolePrivilegeUniquenessTest.java | DB-slice tests for UNIQUE+NOT NULL role/privilege name constraints. |
| src/test/java/com/digitalsanctuary/spring/user/persistence/model/EntityEqualityTest.java | Validates id-based equality/hashCode and secret-safe toString. |
| src/test/java/com/digitalsanctuary/spring/user/MessageSourceEnvironmentPostProcessorTest.java | Verifies additive message basename merge behavior and runtime resolution. |
| src/test/java/com/digitalsanctuary/spring/user/listener/WebAuthnPreDeleteEventListenerTest.java | Updates to new UserPreDeleteEvent scalar payload. |
| src/test/java/com/digitalsanctuary/spring/user/listener/RegistrationListenerTest.java | Updates registration listener tests for scalar OnRegistrationCompleteEvent. |
| src/test/java/com/digitalsanctuary/spring/user/gdpr/GdprExportServiceTest.java | Adds coverage for exporting roles via entity-graph reload. |
| src/test/java/com/digitalsanctuary/spring/user/gdpr/GdprDeletionServiceTest.java | Updates to new UserPreDeleteEvent scalar payload. |
| src/test/java/com/digitalsanctuary/spring/user/gdpr/ConsentAuditServiceTest.java | Updates to new ConsentChangedEvent scalar payload. |
| src/test/java/com/digitalsanctuary/spring/user/exceptions/GlobalValidationExceptionHandlerScopeTest.java | Verifies @ControllerAdvice(assignableTypes=...) scoping to library controllers. |
| src/test/java/com/digitalsanctuary/spring/user/event/UserPreDeleteEventTest.java | Updates event tests for scalar payloads. |
| src/test/java/com/digitalsanctuary/spring/user/event/OnRegistrationCompleteEventTest.java | Updates event tests for scalar payloads + builder semantics. |
| src/test/java/com/digitalsanctuary/spring/user/event/ConsentChangedEventTest.java | Updates event tests for scalar payloads. |
| src/test/java/com/digitalsanctuary/spring/user/architecture/SelfProxiedMethodVisibilityTest.java | Expands “self-proxy” visibility checks to role/privilege REQUIRES_NEW methods. |
| src/test/java/com/digitalsanctuary/spring/user/api/WebAuthnManagementAPITest.java | Adds tests for current-password requirement on credential-altering operations. |
| src/test/java/com/digitalsanctuary/spring/user/api/WebAuthnFeatureEnabledIntegrationTest.java | Integration coverage for current-password requirements + password encoding realism. |
| src/test/java/com/digitalsanctuary/spring/user/api/UserAPIUnitTest.java | Updates API unit tests for uniform registration/resend responses + app-url resolver. |
| src/test/java/com/digitalsanctuary/spring/user/api/UserApiTest.java | Updates integration tests for uniform responses and validation handler behavior. |
| src/test/java/com/digitalsanctuary/spring/user/api/UserAPIRegistrationGuardTest.java | Updates wiring for app-url resolver dependency in registration guard tests. |
| src/main/resources/META-INF/spring.factories | Registers EnvironmentPostProcessor for additive message basenames. |
| src/main/resources/META-INF/additional-spring-configuration-metadata.json | Adds metadata for appUrl/trustedHosts and feature-toggle properties. |
| src/main/resources/config/dsspringuserconfig.properties | Removes message-bundle override; documents new appUrl/trustedHosts properties. |
| src/main/java/com/digitalsanctuary/spring/user/util/UserUtils.java | Deprecates unsafe getAppUrl in favor of AppUrlResolver. |
| src/main/java/com/digitalsanctuary/spring/user/util/AppUrlResolver.java | New resolver to avoid forwarded-host poisoning and produce stable base URLs. |
| src/main/java/com/digitalsanctuary/spring/user/UserConfiguration.java | Converts entry point to @AutoConfiguration and adds per-feature opt-out toggles. |
| src/main/java/com/digitalsanctuary/spring/user/service/UserService.java | Namespaces bean name; updates events + auth authority sourcing for LAZY roles. |
| src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java | Adds id-based registration email overload that reloads entity transactionally. |
| src/main/java/com/digitalsanctuary/spring/user/service/DSUserDetailsService.java | Uses entity-graph repository finder on authentication load. |
| src/main/java/com/digitalsanctuary/spring/user/service/DSOidcUserService.java | Uses entity-graph lookup + publishes scalar registration events. |
| src/main/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserService.java | Uses entity-graph lookup + publishes scalar registration events. |
| src/main/java/com/digitalsanctuary/spring/user/security/UserSecurityBeansAutoConfiguration.java | Adds default AppUrlResolver bean (backs off if consumer defines one). |
| src/main/java/com/digitalsanctuary/spring/user/roles/RolePrivilegeSetupService.java | Makes startup role/privilege setup idempotent under unique-constraint races. |
| src/main/java/com/digitalsanctuary/spring/user/persistence/repository/UserRepository.java | Adds findWithRolesByEmail with @EntityGraph for auth/authority graph loading. |
| src/main/java/com/digitalsanctuary/spring/user/persistence/model/WebAuthnUserEntity.java | Switches away from Lombok @Data; id-based equality and secret-safe toString. |
| src/main/java/com/digitalsanctuary/spring/user/persistence/model/WebAuthnCredential.java | Switches away from Lombok @Data; key-based equality and secret-safe toString. |
| src/main/java/com/digitalsanctuary/spring/user/persistence/model/VerificationToken.java | Adds UNIQUE+NOT NULL token column + id-based equality and secret-safe toString. |
| src/main/java/com/digitalsanctuary/spring/user/persistence/model/User.java | Switches roles to LAZY; id-only equality; secret-safe toString. |
| src/main/java/com/digitalsanctuary/spring/user/persistence/model/Role.java | Makes privileges LAZY; adds unique/not-null constraint on role name. |
| src/main/java/com/digitalsanctuary/spring/user/persistence/model/Privilege.java | Adds unique/not-null constraint on privilege name. |
| src/main/java/com/digitalsanctuary/spring/user/persistence/model/PasswordResetToken.java | Adds UNIQUE+NOT NULL token column + id-based equality and secret-safe toString. |
| src/main/java/com/digitalsanctuary/spring/user/persistence/model/PasswordHistoryEntry.java | Switches away from Lombok @Data; id-based equality and secret-safe toString. |
| src/main/java/com/digitalsanctuary/spring/user/MessageSourceEnvironmentPostProcessor.java | New additive message-bundle registration via env post-processor. |
| src/main/java/com/digitalsanctuary/spring/user/listener/RegistrationListener.java | Switches to scalar event payloads and id-based email dispatch. |
| src/main/java/com/digitalsanctuary/spring/user/gdpr/GdprExportService.java | Reloads user via entity-graph finder to export roles safely with LAZY fetching. |
| src/main/java/com/digitalsanctuary/spring/user/gdpr/GdprDeletionService.java | Publishes scalar UserPreDeleteEvent instead of entity-carrying event. |
| src/main/java/com/digitalsanctuary/spring/user/gdpr/ConsentAuditService.java | Publishes scalar ConsentChangedEvent instead of entity-carrying event. |
| src/main/java/com/digitalsanctuary/spring/user/exceptions/GlobalValidationExceptionHandler.java | New scoped validation advice (library controllers only) + global error support. |
| src/main/java/com/digitalsanctuary/spring/user/exception/GlobalValidationExceptionHandler.java | Removes old global (application-wide) validation advice (package consolidation). |
| src/main/java/com/digitalsanctuary/spring/user/event/UserPreDeleteEvent.java | Converts event payload to userId/userEmail scalars (no JPA entity). |
| src/main/java/com/digitalsanctuary/spring/user/event/OnRegistrationCompleteEvent.java | Converts event payload to userId/userEmail/userEnabled scalars (no JPA entity). |
| src/main/java/com/digitalsanctuary/spring/user/event/ConsentChangedEvent.java | Converts event payload to userId/userEmail scalars (no JPA entity). |
| src/main/java/com/digitalsanctuary/spring/user/controller/UserPageController.java | Namespaces controller bean name to reduce collisions. |
| src/main/java/com/digitalsanctuary/spring/user/controller/UserActionController.java | Namespaces controller bean name to reduce collisions. |
| src/main/java/com/digitalsanctuary/spring/user/audit/AuditMailAutoConfiguration.java | Namespaces MailService bean name to reduce collisions. |
| src/main/java/com/digitalsanctuary/spring/user/api/WebAuthnManagementAPI.java | Namespaces API bean; requires current password for credential-altering operations when set. |
| src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java | Uniform 200 responses for registration/resend; uses AppUrlResolver for email links. |
| src/main/java/com/digitalsanctuary/spring/user/api/MfaAPI.java | Namespaces API bean name to reduce collisions. |
| src/main/java/com/digitalsanctuary/spring/user/api/GdprAPI.java | Namespaces API bean name to reduce collisions. |
| MIGRATION.md | Adds detailed 5.0.x migration guidance for breaking changes and remediation steps. |
| gradle.properties | Bumps version to 5.0.0-SNAPSHOT. |
| db-scripts/mariadb-schema.sql | Updates schema DDL for new UNIQUE/NOT NULL constraints. |
| CHANGELOG.md | Adds 5.0.0 (Unreleased) entry documenting breaking changes and fixes. |
| build.gradle | Updates Spring Boot plugin version to 4.1.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+39
to
+42
| public AppUrlResolver(String configuredAppUrl, List<String> trustedHosts) { | ||
| this.configuredAppUrl = (configuredAppUrl == null || configuredAppUrl.isBlank()) ? null : configuredAppUrl.trim(); | ||
| this.trustedHosts = trustedHosts == null ? List.of() : trustedHosts.stream().map(String::trim).toList(); | ||
| } |
Comment on lines
+55
to
+63
| String fwdHost = request.getHeader("X-Forwarded-Host"); | ||
| boolean useForwarded = fwdHost != null && !fwdHost.isEmpty() && trustedHosts.contains(stripPort(fwdHost)); | ||
| if (fwdHost != null && !fwdHost.isEmpty() && !useForwarded) { | ||
| log.warn("AppUrlResolver: ignoring untrusted X-Forwarded-Host '{}' (not in user.security.trustedHosts)", fwdHost); | ||
| } | ||
|
|
||
| String scheme = useForwarded ? headerOr(request, "X-Forwarded-Proto", request.getScheme()) : request.getScheme(); | ||
| String host = useForwarded ? stripPort(fwdHost) : request.getServerName(); | ||
| int port = useForwarded ? forwardedPort(request, scheme) : request.getServerPort(); |
Comment on lines
100
to
108
| for (Map.Entry<String, List<String>> entry : rolesAndPrivilegesConfig.getRolesAndPrivileges().entrySet()) { | ||
| final String roleName = entry.getKey(); | ||
| final List<String> privileges = entry.getValue(); | ||
| if (roleName != null && privileges != null) { | ||
| final Set<Privilege> privilegeSet = new HashSet<>(); | ||
| for (String privilegeName : privileges) { | ||
| privilegeSet.add(getOrCreatePrivilege(privilegeName)); | ||
| getOrCreatePrivilege(privilegeName); | ||
| } | ||
| getOrCreateRole(roleName, privilegeSet); | ||
| getOrCreateRole(roleName, new HashSet<>(privileges)); | ||
| } |
…d denials Complete the account-enumeration hardening (AC 4.2) and close an audit gap surfaced in PR review. - POST /user/registration/passwordless no longer returns HTTP 409 with an explicit "account already exists" message on a duplicate email. It now returns the same success-shaped HTTP 200 a genuine registration produces, making the two cases indistinguishable to the caller. The true outcome is still recorded server-side via the audit event. This mirrors the form-path fix already applied to /user/registration. - Both registration paths (form and passwordless) now emit an audit event on RegistrationDeniedException, matching the trail already produced for the duplicate-email path. Previously a guard denial produced only an INFO log. Adds UserAPIRegistrationGuardTest coverage asserting the uniform passwordless response and that no registration event is published for an existing email.
…p guard Extend the ds-prefix bean namespacing (AC 3.6) to the high-collision service beans that still used Spring's derived default names. With Spring Boot 4's default spring.main.allow-bean-definition-overriding=false, a consumer bean of the same default name would fail context startup with a BeanDefinitionOverrideException. Renamed: UserEmailService, DSUserDetailsService, LoginAttemptService, SessionInvalidationService, PasswordPolicyService, AuthorityService, RolePrivilegeSetupService, MailContentBuilder (all now ds-prefixed). Verified no by-name references (@Qualifier/@Resource/@DependsOn/SpEL/getBean) exist; all injection is by-type and unaffected. Also marks RolePrivilegeSetupService.alreadySetup volatile so the one-time-setup guard is visible across threads under concurrent context refresh (e.g. parallel test execution).
…lver Three robustness/hardening fixes on the request-derived email-link base URL (CWE-640 path): - X-Forwarded-Host may be a comma-separated list across a multi-proxy chain (RFC 7230); the client-facing host is the first value. Match the allow-list against that first value so legitimate ALB+nginx style chains are honored instead of silently falling back to the container server name. - Validate X-Forwarded-Proto to http/https only. A misconfigured or compromised trusted proxy can no longer inject an arbitrary scheme (e.g. javascript) into security-sensitive email links; invalid values fall back to the request scheme. - Sanitize attacker-controlled header values (CR/LF/tab) before logging. X-Forwarded-Port is likewise read as first-of-list. Adds tests for the multi-valued host match and the invalid-proto fallback.
…nges The current-password check guarding credential-altering operations (passkey delete/rename, password removal) verifies a bcrypt password and is therefore an authentication surface, but it did not participate in account lockout. A session-holding actor could make unlimited password guesses against these endpoints without ever tripping LoginAttemptService. requireCurrentPasswordIfSet now: - rejects an already-locked account up front, - reports each wrong password via loginFailed (locking the account once the configured threshold is reached), and - resets the failed-attempt counter via loginSucceeded on success. A missing currentPassword field is treated as a client error, not a guess, so it does not count toward lockout. Adds tests for the locked-account rejection, the failed-attempt report, and the success-path counter reset.
Move MessageSourceEnvironmentPostProcessor registration from the legacy META-INF/spring.factories to META-INF/spring/org.springframework.boot.env.EnvironmentPostProcessor.imports, the mechanism Spring Boot now prefers and the same style already used for the library's AutoConfiguration.imports. No behavior change; the post-processor class is unchanged.
…GELOG/MIGRATION - Add a warning on each id-based entity (User, PasswordResetToken, VerificationToken, PasswordHistoryEntry, WebAuthnUserEntity) noting that identity-based equals/hashCode makes two transient (unsaved, id == null) instances compare equal, so unsaved entities must not be used as Set/Map keys. Behavior is unchanged and bounded (entities are loaded from the DB before collection use); the comment prevents future misuse. - CHANGELOG: record the WebAuthn lockout integration, the AppUrlResolver X-Forwarded hardening, the registration-denial audit events, and extend the anti-enumeration entry to the passwordless path. - MIGRATION: add /user/registration/passwordless to the anti-enumeration tables and add the newly namespaced service beans to the bean-name table.
…g-slash contract AppUrlResolver's Javadoc and resolveAppUrl's @return tag both promise "no trailing slash", but configuredAppUrl was returned verbatim after only .trim(). A consumer setting user.security.appUrl=https://app.example.com/ would silently produce double slashes in security email links (appUrl + "/user/registrationConfirm?token=..." → "https://app.example.com//user/..."). Strip all trailing slashes from configuredAppUrl in the constructor so the contract is enforced at construction time regardless of how many slashes the consumer accidentally appended. Adds two-case test (single and triple slash).
The onApplicationEvent loop iterated raw config-supplied privilege names and passed them directly to getOrCreatePrivilege without a null/blank check. With the NOT NULL constraint now enforced on privilege.name, a misconfigured null or blank entry in user.roles-and-privileges would fail startup with a DataIntegrityViolationException (and a blank would attempt to persist an empty string, which likewise violates the constraint). Add a null/blank guard consistent with the existing roleName null check, log a warning so operators can fix their configuration, and filter the same blanks out of the privilege set passed to getOrCreateRole so the persisted associations match what was actually inserted.
# Conflicts: # gradle.properties
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| String fwdHost = firstHeaderValue(request.getHeader("X-Forwarded-Host")); | ||
| boolean useForwarded = fwdHost != null && !fwdHost.isEmpty() && trustedHosts.contains(stripPort(fwdHost)); | ||
| if (fwdHost != null && !fwdHost.isEmpty() && !useForwarded) { | ||
| log.warn("AppUrlResolver: ignoring untrusted X-Forwarded-Host '{}' (not in user.security.trustedHosts)", sanitizeForLog(fwdHost)); |
| try { | ||
| return Integer.parseInt(portHeader.trim()); | ||
| } catch (NumberFormatException e) { | ||
| log.warn("AppUrlResolver: ignoring non-numeric X-Forwarded-Port '{}'", sanitizeForLog(portHeader)); |
| if ("http".equalsIgnoreCase(proto) || "https".equalsIgnoreCase(proto)) { | ||
| return proto; | ||
| } | ||
| log.warn("AppUrlResolver: ignoring invalid X-Forwarded-Proto '{}', falling back to request scheme", sanitizeForLog(proto)); |
Code Review — PR #318 (5.0.0 Tier 2 Remediation)This is a substantial, well-structured major release. The 12 tasks are clearly separated into atomic commits, the MIGRATION.md and CHANGELOG.md entries are thorough, and the 939-test pass + 103/103 Playwright green gives good coverage confidence. The notes below are mostly actionable bugs and a handful of design observations; nothing that blocks the work — just items worth addressing before tagging 5.0.0. Bugs / Correctness Issues1. this.trustedHosts = trustedHosts.stream().map(String::trim).toList();
// ...
boolean useForwarded = trustedHosts.contains(stripPort(fwdHost));
this.trustedHosts = trustedHosts.stream().map(s -> s.trim().toLowerCase(Locale.ROOT)).toList();
// and in resolve():
boolean useForwarded = trustedHosts.contains(stripPort(fwdHost).toLowerCase(Locale.ROOT));2. Inconsistent uniform-messaging on passwordless registration (UserAPI.java) Task 4.2 goal is identical responses for success and duplicate-email paths. The form registration path returns:
But the passwordless duplicate-email path returns:
These differ in wording. A response-body scanner would still distinguish the two endpoints even if the status codes match. Use the same hedged message for both, or make both equally ambiguous. 3. Missing
4. Test resource file for The integration sub-test Design / Performance Observations5. @EntityGraph(attributePaths = {"roles", "roles.privileges"})
Optional<User> findWithRolesByEmail(String email);A two-level 6. With 7. log.warn("Privilege '{}' not found, skipping assignment to role '{}'", privName, roleName);This message says "usually a typo", but during multi-node startup it can trigger transiently when node B resolves privileges before node A's 8.
These are meaningfully different to a client but the exception type and status code are the same. The client cannot distinguish "wrong password" from "no passkey registered" without parsing the message string. This is a UX issue not a security issue (the brute-force protection is correctly wired regardless). If you ever want to document a stable API contract for these endpoints, finer error codes (e.g. 401 or a structured error body) would help consumers. 9. Missing test: multi-proxy comma-separated The implementation correctly takes Minor Style Notes
Summary
The overall implementation quality is high — the |
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements Tier 2 (5.0.0) of the 2026-06 remediation plan — the breaking-change / needs-a-heads-up items — branched off the
4.4.0tag. Each task is one atomic commit with spec + code-quality review and CHANGELOG/MIGRATION entries. A final holistic review found no task-interaction bugs.Versioned 5.0.0 (SemVer-honest for the breaking surface); Spring Boot compatibility is documented separately in MIGRATION rather than tied to the major.
What's included (12 tasks)
Security
user.security.appUrl/trustedHosts, ignoringX-Forwarded-Host(CWE-640).Performance / data
User→roles→privileges with@EntityGraphon the auth path.Model / events
equals/hashCode(drop@Data); secrets excluded fromtoString.@Asyncdetached-entity hazards).Library citizenship
@ControllerAdvicescoped to library controllers; 400 (not 500) for class-level@PasswordMatches.exception→exceptions; removed emptyapi.data.EnvironmentPostProcessor); namespaced bean names.@AutoConfigurationentry point with toggleable@Enable*(user.{async,retry,scheduling,method-security}.enabled, default true).Dependencies
Intentionally NOT included
Breaking changes
All consumer-affecting changes are documented in
MIGRATION.md("Migrating to 5.0.x") with remediation steps, and listed under Breaking Changes inCHANGELOG.md. Manual-schema (ddl-auto=validate/Flyway) consumers:db-scripts/mariadb-schema.sqlupdated with the new unique constraints.Validation
./gradlew clean check— 939 tests, 0 failures (on Spring Boot 4.1.0).5.0.0-SNAPSHOT. The upgrade required exactly the 3 consumer migrations documented in MIGRATION (event constructor,getUserId(),UserEmailServiceconstructor) — confirming the migration guide is accurate.Notes for review
/registration/passwordlessstill returns 409 on an existing email (4.2 covered/registration+/resendRegistrationToken); passwordless accounts can't require a current credential for credential changes (no step-up infra). Both noted in CHANGELOG/MIGRATION.4.4.0tag;gradle.propertiesis5.0.0-SNAPSHOT.