5.0.1: distinct WebAuthn re-auth HTTP statuses (#8) + review doc notes (#5, #6)

CHANGELOG.md

@@ -5,6 +5,7 @@ All notable changes to this project are documented here. This project follows [S
 ## [5.0.1] - Unreleased
 ### Changed
 - Reverted `Role.privileges` to `FetchType.EAGER` (it was changed to `LAZY` in 5.0.0). The 5.0.0 change made `role.getPrivileges()` throw `LazyInitializationException` when accessed outside an open transaction/session — a surprising footgun for consumers — in exchange for a negligible gain, since privileges are small, static reference data with no bulk-load path. `User.roles` remains `LAZY` (that is where the real N+1 win is), and the authentication path still loads the full `User → roles → privileges` graph in a single round trip via `UserRepository.findWithRolesByEmail`. This is a **non-breaking relaxation**: code written against 5.0.0 continues to work unchanged. See `MIGRATION.md` ("Lazy fetching of roles and privileges").
+- WebAuthn credential-management re-authentication failures now use distinct HTTP statuses so clients can tell them apart: an incorrect current password returns **401 Unauthorized** and a locked account returns **423 Locked** (both previously **400**). A missing/blank `currentPassword` field remains **400 Bad Request**. Affects `DELETE /user/webauthn/password`, `DELETE /user/webauthn/credentials/{id}`, and `PUT /user/webauthn/credentials/{id}/label`. These endpoints are new in 5.0.0; brute-force lockout behaviour is unchanged.
 
 ### Fixed
 - `user.security.trustedHosts` matching is now case-insensitive (RFC 4343). A mixed-case configured entry or forwarded host (e.g. `App.Example.Com`) now matches `app.example.com`; previously a case mismatch caused the resolver to ignore a legitimately trusted `X-Forwarded-Host` and fall back to the container's server name on the CWE-640 path.

MIGRATION.md

@@ -128,11 +128,14 @@ Affected endpoints (all require `user.webauthn.enabled=true` except where noted)
 | `/user/webauthn/credentials/{id}` | `DELETE` | Deleting a passkey requires the current password **when the account has a password**. | JSON body `{"currentPassword": "..."}` |
 | `/user/webauthn/credentials/{id}/label` | `PUT` | Renaming a passkey requires the current password **when the account has a password**. The existing body gains a `currentPassword` field. | JSON body `{"label": "...", "currentPassword": "..."}` |
 
-**Behavior when the account has a password:**
-- Missing `currentPassword` → `HTTP 400` with message *"Current password is required to change authentication methods."* — nothing is mutated.
-- Incorrect `currentPassword` → `HTTP 400` with message *"Current password is incorrect."* — nothing is mutated.
+**Behavior when the account has a password** (status codes refined in **5.0.1** — see note below):
+- Missing/blank `currentPassword` → `HTTP 400 Bad Request`, message *"Current password is required to change authentication methods."* — nothing is mutated.
+- Incorrect `currentPassword` → `HTTP 401 Unauthorized`, message *"Current password is incorrect."* — nothing is mutated.
+- Account locked (too many failed attempts) → `HTTP 423 Locked`, message *"Account is locked due to too many failed attempts…"* — nothing is mutated.
 - Correct `currentPassword` → the operation proceeds as before.
 
+> **5.0.0 → 5.0.1 note:** In 5.0.0 all three failure cases returned `HTTP 400`. As of 5.0.1 they return distinct statuses (400 missing / 401 incorrect / 423 locked) so clients can tell them apart. If you wrote a client against 5.0.0 that treats any `4xx` as "re-auth failed", no change is needed; only update it if you branched specifically on `400`.
+
 `/user/updatePassword` is unchanged: it already required and verified `oldPassword`.
 
 **Action required:** Update any client that calls the three endpoints above so that it collects the user's current password and sends it in the request body. `DELETE /user/webauthn/credentials/{id}` and `DELETE /user/webauthn/password`, which previously had no request body, now accept (and for password-holding accounts require) a JSON body carrying `currentPassword`. Existing IDOR/ownership checks and last-credential lockout protection are unchanged.

src/main/java/com/digitalsanctuary/spring/user/api/WebAuthnManagementAPI.java

@@ -16,7 +16,9 @@
 import org.springframework.web.bind.annotation.RestController;
 import com.digitalsanctuary.spring.user.audit.AuditEvent;
 import com.digitalsanctuary.spring.user.dto.WebAuthnCredentialInfo;
+import com.digitalsanctuary.spring.user.exceptions.WebAuthnAccountLockedException;
 import com.digitalsanctuary.spring.user.exceptions.WebAuthnException;
+import com.digitalsanctuary.spring.user.exceptions.WebAuthnReauthenticationException;
 import com.digitalsanctuary.spring.user.exceptions.WebAuthnUserNotFoundException;
 import com.digitalsanctuary.spring.user.persistence.model.User;
 import com.digitalsanctuary.spring.user.service.LoginAttemptService;
@@ -228,10 +230,12 @@ private User findAuthenticatedUser(UserDetails userDetails) throws WebAuthnUserN
 	 *
 	 * <p>
 	 * If the account has a password, the supplied {@code currentPassword} must be present and valid (verified via
-	 * {@link UserService#checkIfValidOldPassword(User, String)}); otherwise a {@link WebAuthnException} (HTTP 400) is
-	 * thrown <em>before</em> any mutation, preventing a session-only actor from changing the account's authentication
-	 * methods. If the account is passwordless (passkey-only) there is no current credential to verify, so this check is
-	 * a no-op — see the residual-risk note in MIGRATION.md.
+	 * {@link UserService#checkIfValidOldPassword(User, String)}) <em>before</em> any mutation, preventing a session-only
+	 * actor from changing the account's authentication methods. The failure modes map to distinct HTTP statuses so
+	 * clients can tell them apart: a missing/blank field &rarr; {@link WebAuthnException} (HTTP 400), an incorrect
+	 * password &rarr; {@link WebAuthnReauthenticationException} (HTTP 401), and a locked account &rarr;
+	 * {@link WebAuthnAccountLockedException} (HTTP 423). If the account is passwordless (passkey-only) there is no current
+	 * credential to verify, so this check is a no-op — see the residual-risk note in MIGRATION.md.
 	 * </p>
 	 *
 	 * <p>
@@ -244,23 +248,27 @@ private User findAuthenticatedUser(UserDetails userDetails) throws WebAuthnUserN
 	 *
 	 * @param user the authenticated user
 	 * @param currentPassword the current password supplied by the client (may be {@code null})
-	 * @throws WebAuthnException if the account is locked, or has a password and the supplied current password is missing or incorrect
+	 * @throws WebAuthnAccountLockedException if the account is locked (HTTP 423)
+	 * @throws WebAuthnReauthenticationException if the account has a password and the supplied current password is incorrect (HTTP 401)
+	 * @throws WebAuthnException if the account has a password and the current password is missing/blank (HTTP 400)
 	 */
 	private void requireCurrentPasswordIfSet(User user, String currentPassword) {
 		if (!userService.hasPassword(user)) {
 			// Passwordless (passkey-only) account: no current credential exists to verify. See MIGRATION.md residual-risk note.
 			return;
 		}
 		if (loginAttemptService.isLocked(user.getEmail())) {
-			throw new WebAuthnException("Account is locked due to too many failed attempts. Please try again later.");
+			// Locked account -> HTTP 423, distinct from a wrong password (401) or a missing field (400).
+			throw new WebAuthnAccountLockedException("Account is locked due to too many failed attempts. Please try again later.");
 		}
 		if (currentPassword == null || currentPassword.isBlank()) {
-			// A missing field is a client error, not a password guess, so it does not count toward lockout.
+			// A missing field is a client error (HTTP 400), not a password guess, so it does not count toward lockout.
 			throw new WebAuthnException("Current password is required to change authentication methods.");
 		}
 		if (!userService.checkIfValidOldPassword(user, currentPassword)) {
 			loginAttemptService.loginFailed(user.getEmail());
-			throw new WebAuthnException("Current password is incorrect.");
+			// A wrong password is an authentication failure -> HTTP 401.
+			throw new WebAuthnReauthenticationException("Current password is incorrect.");
 		}
 		// Successful re-authentication clears the failed-attempt counter, matching login semantics.
 		loginAttemptService.loginSucceeded(user.getEmail());

src/main/java/com/digitalsanctuary/spring/user/api/WebAuthnManagementAPIAdvice.java

@@ -6,7 +6,9 @@
 import org.springframework.web.bind.MethodArgumentNotValidException;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.RestControllerAdvice;
+import com.digitalsanctuary.spring.user.exceptions.WebAuthnAccountLockedException;
 import com.digitalsanctuary.spring.user.exceptions.WebAuthnException;
+import com.digitalsanctuary.spring.user.exceptions.WebAuthnReauthenticationException;
 import com.digitalsanctuary.spring.user.exceptions.WebAuthnUserNotFoundException;
 import com.digitalsanctuary.spring.user.util.GenericResponse;
 import jakarta.validation.ConstraintViolationException;
@@ -26,6 +28,18 @@ public ResponseEntity<GenericResponse> handleUserNotFound(WebAuthnUserNotFoundEx
 		return ResponseEntity.status(HttpStatus.NOT_FOUND).body(new GenericResponse(ex.getMessage()));
 	}
 
+	@ExceptionHandler(WebAuthnAccountLockedException.class)
+	public ResponseEntity<GenericResponse> handleAccountLocked(WebAuthnAccountLockedException ex) {
+		log.warn("WebAuthn account locked: {}", ex.getMessage());
+		return ResponseEntity.status(HttpStatus.LOCKED).body(new GenericResponse(ex.getMessage()));
+	}
+
+	@ExceptionHandler(WebAuthnReauthenticationException.class)
+	public ResponseEntity<GenericResponse> handleReauthenticationFailure(WebAuthnReauthenticationException ex) {
+		log.warn("WebAuthn re-authentication failure: {}", ex.getMessage());
+		return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(new GenericResponse(ex.getMessage()));
+	}
+
 	@ExceptionHandler(WebAuthnException.class)
 	public ResponseEntity<GenericResponse> handleWebAuthnError(WebAuthnException ex) {
 		log.warn("WebAuthn error: {}", ex.getMessage());

src/main/java/com/digitalsanctuary/spring/user/exceptions/WebAuthnAccountLockedException.java

@@ -0,0 +1,21 @@
+package com.digitalsanctuary.spring.user.exceptions;
+
+/**
+ * Thrown when a credential-altering WebAuthn operation is rejected because the account is currently locked due to too
+ * many failed authentication attempts. Maps to HTTP 423 (Locked) so clients can distinguish a temporary lockout from a
+ * wrong password (→ 401) or a malformed request (→ 400).
+ */
+public class WebAuthnAccountLockedException extends WebAuthnException {
+
+	/** Serial Version UID. */
+	private static final long serialVersionUID = 1L;
+
+	/**
+	 * Instantiates a new WebAuthn account-locked exception.
+	 *
+	 * @param message the message
+	 */
+	public WebAuthnAccountLockedException(final String message) {
+		super(message);
+	}
+}

src/main/java/com/digitalsanctuary/spring/user/exceptions/WebAuthnReauthenticationException.java

@@ -0,0 +1,22 @@
+package com.digitalsanctuary.spring.user.exceptions;
+
+/**
+ * Thrown when re-authentication for a credential-altering WebAuthn operation fails because the supplied current password
+ * is incorrect. Maps to HTTP 401 (Unauthorized) so clients can distinguish a wrong password from a malformed request
+ * (missing field → 400) or a locked account (→ 423). A missing/blank current-password field is a client error
+ * and remains a plain {@link WebAuthnException} (400).
+ */
+public class WebAuthnReauthenticationException extends WebAuthnException {
+
+	/** Serial Version UID. */
+	private static final long serialVersionUID = 1L;
+
+	/**
+	 * Instantiates a new WebAuthn re-authentication exception.
+	 *
+	 * @param message the message
+	 */
+	public WebAuthnReauthenticationException(final String message) {
+		super(message);
+	}
+}

src/main/java/com/digitalsanctuary/spring/user/persistence/model/PasswordResetToken.java

@@ -42,6 +42,7 @@ public class PasswordResetToken {
 	private String token;
 
 	/** The user. */
+	// EAGER so getUser() works on a detached token; note the loaded User's roles are still LAZY — use UserRepository.findWithRolesByEmail if you need authorities.
 	@ToString.Exclude
 	@OneToOne(targetEntity = User.class, fetch = FetchType.EAGER)
 	@JoinColumn(nullable = false, name = "user_id")

src/main/java/com/digitalsanctuary/spring/user/persistence/model/VerificationToken.java

@@ -44,6 +44,7 @@ public class VerificationToken {
 	private String token;
 
 	/** The user. */
+	// EAGER so getUser() works on a detached token; note the loaded User's roles are still LAZY — use UserRepository.findWithRolesByEmail if you need authorities.
 	@ToString.Exclude
 	@OneToOne(targetEntity = User.class, fetch = FetchType.EAGER)
 	@JoinColumn(nullable = false, name = "user_id", foreignKey = @ForeignKey(name = "FK_VERIFY_USER"))

src/main/java/com/digitalsanctuary/spring/user/persistence/repository/UserRepository.java

@@ -34,6 +34,10 @@ public interface UserRepository extends JpaRepository<User, Long> {
 	 * {@link #findByEmail(String)} remains for callers (token lookups, existence checks, lockout counters) that do not
 	 * need the authority graph.</p>
 	 *
+	 * <p>Implementation note: the two-level graph is a single JOIN FETCH, so the result set is a Cartesian product of
+	 * roles &times; privileges (Hibernate de-duplicates via the {@code Set} mappings). This is fine for the small,
+	 * bounded role/privilege graphs of a single user; it is not intended for bulk loading many users at once.</p>
+	 *
 	 * @param email the email
 	 * @return the user with roles and privileges initialized, or {@code null} if none found
 	 */

src/test/java/com/digitalsanctuary/spring/user/api/WebAuthnFeatureEnabledIntegrationTest.java

@@ -150,7 +150,7 @@ void shouldRejectDeleteWhenCurrentPasswordMissingForPasswordAccount() throws Exc
 	void shouldRejectDeleteWhenCurrentPasswordIncorrectForPasswordAccount() throws Exception {
 		String payload = "{\"currentPassword\":\"wrongPassword!\"}";
 		mockMvc.perform(delete("/user/webauthn/credentials/cred-1").with(user(TEST_EMAIL).roles("USER")).with(csrf())
-				.contentType(MediaType.APPLICATION_JSON).content(payload)).andExpect(status().isBadRequest())
+				.contentType(MediaType.APPLICATION_JSON).content(payload)).andExpect(status().isUnauthorized())
 				.andExpect(jsonPath("$.message", containsString("Current password is incorrect")));
 
 		assertThat(webAuthnCredentialRepository.findByIdWithUser("cred-1")).isPresent();
@@ -174,7 +174,7 @@ void shouldRejectRenameWhenCurrentPasswordMissingForPasswordAccount() throws Exc
 	void shouldRejectRenameWhenCurrentPasswordIncorrectForPasswordAccount() throws Exception {
 		String payload = "{\"label\":\"New Label\",\"currentPassword\":\"wrongPassword!\"}";
 		mockMvc.perform(put("/user/webauthn/credentials/cred-1/label").with(user(TEST_EMAIL).roles("USER")).with(csrf())
-				.contentType(MediaType.APPLICATION_JSON).content(payload)).andExpect(status().isBadRequest())
+				.contentType(MediaType.APPLICATION_JSON).content(payload)).andExpect(status().isUnauthorized())
 				.andExpect(jsonPath("$.message", containsString("Current password is incorrect")));
 
 		assertThat(webAuthnCredentialRepository.findByIdWithUser("cred-1"))

src/test/java/com/digitalsanctuary/spring/user/api/WebAuthnManagementAPIAdviceTest.java

@@ -0,0 +1,52 @@
+package com.digitalsanctuary.spring.user.api;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+import com.digitalsanctuary.spring.user.exceptions.WebAuthnAccountLockedException;
+import com.digitalsanctuary.spring.user.exceptions.WebAuthnException;
+import com.digitalsanctuary.spring.user.exceptions.WebAuthnReauthenticationException;
+import com.digitalsanctuary.spring.user.exceptions.WebAuthnUserNotFoundException;
+import com.digitalsanctuary.spring.user.util.GenericResponse;
+
+/**
+ * Unit tests for {@link WebAuthnManagementAPIAdvice}, verifying that each WebAuthn exception type maps to the intended
+ * HTTP status so credential-management clients can distinguish the failure modes (missing field, wrong password, locked
+ * account, unknown user).
+ */
+@DisplayName("WebAuthnManagementAPIAdvice status mapping")
+class WebAuthnManagementAPIAdviceTest {
+
+    private final WebAuthnManagementAPIAdvice advice = new WebAuthnManagementAPIAdvice();
+
+    @Test
+    @DisplayName("base WebAuthnException (e.g. missing current password) -> 400 Bad Request")
+    void baseExceptionMapsToBadRequest() {
+        ResponseEntity<GenericResponse> response = advice.handleWebAuthnError(new WebAuthnException("Current password is required"));
+        assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
+    }
+
+    @Test
+    @DisplayName("incorrect current password -> 401 Unauthorized")
+    void reauthenticationFailureMapsToUnauthorized() {
+        ResponseEntity<GenericResponse> response =
+                advice.handleReauthenticationFailure(new WebAuthnReauthenticationException("Current password is incorrect."));
+        assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
+    }
+
+    @Test
+    @DisplayName("locked account -> 423 Locked")
+    void accountLockedMapsToLocked() {
+        ResponseEntity<GenericResponse> response = advice.handleAccountLocked(new WebAuthnAccountLockedException("Account is locked"));
+        assertThat(response.getStatusCode()).isEqualTo(HttpStatus.LOCKED);
+    }
+
+    @Test
+    @DisplayName("unknown user -> 404 Not Found")
+    void userNotFoundMapsToNotFound() {
+        ResponseEntity<GenericResponse> response = advice.handleUserNotFound(new WebAuthnUserNotFoundException("User not found"));
+        assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
+    }
+}