fix(webauthn): apply PR #256 review fixes (#258)

Apply all 14 review fixes that were missing from the squash merge:

- Safe-parse AuthenticatorTransport enums, default label, trim-before-length
- Fix TOCTOU race in last-credential protection with in-transaction recount
- Add ON DELETE CASCADE to WebAuthn foreign keys in schema
- Add WebAuthnPreDeleteEventListener for cleanup on user deletion
- Add WebAuthnAuthenticationToken for passkey session distinction
- Change transports to List<String> in credential response DTO
- Add @notblank on path variable parameters in management API
- Add @ConditionalOnProperty to WebAuthnManagementAPIAdvice
- Change WebAuthnException to extend RuntimeException
- Use SecureRandom for user handle generation
- Use @transactional(propagation = MANDATORY) on credential operations
- Update all tests for new behavior

Author: devondragon

db-scripts/mariadb-schema.sql

@@ -106,7 +106,7 @@ CREATE TABLE `user_entities` (
   PRIMARY KEY (`id`),
   UNIQUE KEY `UK_user_entities_name` (`name`),
   KEY `FK_user_entities_user` (`user_account_id`),
-  CONSTRAINT `FK_user_entities_user` FOREIGN KEY (`user_account_id`) REFERENCES `user_account` (`id`)
+  CONSTRAINT `FK_user_entities_user` FOREIGN KEY (`user_account_id`) REFERENCES `user_account` (`id`) ON DELETE CASCADE
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;
 
 CREATE TABLE `user_credentials` (
@@ -126,5 +126,5 @@ CREATE TABLE `user_credentials` (
   `label` VARCHAR(64) NOT NULL,
   PRIMARY KEY (`credential_id`),
   KEY `FK_user_credentials_entity` (`user_entity_user_id`),
-  CONSTRAINT `FK_user_credentials_entity` FOREIGN KEY (`user_entity_user_id`) REFERENCES `user_entities` (`id`)
+  CONSTRAINT `FK_user_credentials_entity` FOREIGN KEY (`user_entity_user_id`) REFERENCES `user_entities` (`id`) ON DELETE CASCADE
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci;

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

@@ -13,7 +13,6 @@
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
 import com.digitalsanctuary.spring.user.dto.WebAuthnCredentialInfo;
-import com.digitalsanctuary.spring.user.exceptions.WebAuthnException;
 import com.digitalsanctuary.spring.user.exceptions.WebAuthnUserNotFoundException;
 import com.digitalsanctuary.spring.user.persistence.model.User;
 import com.digitalsanctuary.spring.user.service.UserService;
@@ -96,9 +95,9 @@ public ResponseEntity<Boolean> hasCredentials(@AuthenticationPrincipal UserDetai
 	 * @return ResponseEntity with success message or error
 	 */
 	@PutMapping("/credentials/{id}/label")
-	public ResponseEntity<GenericResponse> renameCredential(@PathVariable @Size(max = 512) String id,
+	public ResponseEntity<GenericResponse> renameCredential(@PathVariable @NotBlank @Size(max = 512) String id,
 			@RequestBody @Valid RenameCredentialRequest request,
-			@AuthenticationPrincipal UserDetails userDetails) throws WebAuthnException {
+			@AuthenticationPrincipal UserDetails userDetails) {
 		User user = findAuthenticatedUser(userDetails);
 		credentialManagementService.renameCredential(id, request.label(), user);
 		return ResponseEntity.ok(new GenericResponse("Passkey renamed successfully"));
@@ -121,8 +120,8 @@ public ResponseEntity<GenericResponse> renameCredential(@PathVariable @Size(max
 	 * @return ResponseEntity with success message or error
 	 */
 	@DeleteMapping("/credentials/{id}")
-	public ResponseEntity<GenericResponse> deleteCredential(@PathVariable @Size(max = 512) String id,
-			@AuthenticationPrincipal UserDetails userDetails) throws WebAuthnException {
+	public ResponseEntity<GenericResponse> deleteCredential(@PathVariable @NotBlank @Size(max = 512) String id,
+			@AuthenticationPrincipal UserDetails userDetails) {
 		User user = findAuthenticatedUser(userDetails);
 		credentialManagementService.deleteCredential(id, user);
 		return ResponseEntity.ok(new GenericResponse("Passkey deleted successfully"));

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

@@ -1,5 +1,6 @@
 package com.digitalsanctuary.spring.user.api;
 
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.web.bind.MethodArgumentNotValidException;
@@ -15,6 +16,7 @@
  * Centralized exception handling for WebAuthn credential management endpoints.
  */
 @RestControllerAdvice(assignableTypes = WebAuthnManagementAPI.class)
+@ConditionalOnProperty(name = "user.webauthn.enabled", havingValue = "true", matchIfMissing = false)
 @Slf4j
 public class WebAuthnManagementAPIAdvice {
 

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

@@ -1,6 +1,7 @@
 package com.digitalsanctuary.spring.user.dto;
 
 import java.time.Instant;
+import java.util.List;
 import lombok.AllArgsConstructor;
 import lombok.Builder;
 import lombok.Data;
@@ -28,7 +29,7 @@ public class WebAuthnCredentialInfo {
 	private Instant lastUsed;
 
 	/** Supported transports (usb, nfc, ble, internal). */
-	private String transports;
+	private List<String> transports;
 
 	/** Whether credential is backup-eligible (synced passkey). */
 	private Boolean backupEligible;

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

@@ -4,7 +4,7 @@
  * Exception thrown for WebAuthn-related errors.
  *
  * <p>
- * This is a checked exception used to signal WebAuthn-specific business logic errors such as:
+ * This is an unchecked exception used to signal WebAuthn-specific business logic errors such as:
  * </p>
  * <ul>
  * <li>Attempting to delete the last passkey when user has no password</li>
@@ -13,7 +13,7 @@
  * <li>User not found during credential operations</li>
  * </ul>
  */
-public class WebAuthnException extends Exception {
+public class WebAuthnException extends RuntimeException {
 
 	/** Serial Version UID. */
 	private static final long serialVersionUID = 1L;

src/main/java/com/digitalsanctuary/spring/user/listener/WebAuthnPreDeleteEventListener.java

@@ -0,0 +1,49 @@
+package com.digitalsanctuary.spring.user.listener;
+
+import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
+import org.springframework.context.event.EventListener;
+import org.springframework.stereotype.Component;
+import com.digitalsanctuary.spring.user.event.UserPreDeleteEvent;
+import com.digitalsanctuary.spring.user.persistence.model.WebAuthnUserEntity;
+import com.digitalsanctuary.spring.user.persistence.repository.WebAuthnCredentialRepository;
+import com.digitalsanctuary.spring.user.persistence.repository.WebAuthnUserEntityRepository;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+
+/**
+ * Listens for {@link UserPreDeleteEvent} and cleans up all WebAuthn data for the user being deleted.
+ *
+ * <p>
+ * Deletes all credentials associated with the user's WebAuthn entity, then deletes the entity itself.
+ * This listener is synchronous so that failures cause the enclosing transaction to roll back, preventing
+ * orphaned user accounts.
+ * </p>
+ */
+@Slf4j
+@Component
+@ConditionalOnProperty(name = "user.webauthn.enabled", havingValue = "true", matchIfMissing = false)
+@RequiredArgsConstructor
+public class WebAuthnPreDeleteEventListener {
+
+	private final WebAuthnCredentialRepository credentialRepository;
+	private final WebAuthnUserEntityRepository userEntityRepository;
+
+	/**
+	 * Handle user pre-delete by removing all WebAuthn credentials and the user entity.
+	 *
+	 * @param event the user pre-delete event
+	 */
+	@EventListener
+	public void onUserPreDelete(UserPreDeleteEvent event) {
+		Long userId = event.getUserId();
+		log.debug("Cleaning up WebAuthn data for user {}", userId);
+
+		userEntityRepository.findByUserId(userId).ifPresent(this::deleteUserEntityAndCredentials);
+	}
+
+	private void deleteUserEntityAndCredentials(WebAuthnUserEntity userEntity) {
+		credentialRepository.findByUserEntity(userEntity).forEach(credentialRepository::delete);
+		userEntityRepository.delete(userEntity);
+		log.info("Deleted WebAuthn data for user entity {}", userEntity.getName());
+	}
+}

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

@@ -1,10 +1,13 @@
 package com.digitalsanctuary.spring.user.persistence.repository;
 
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.stereotype.Repository;
+import org.springframework.transaction.annotation.Propagation;
 import org.springframework.transaction.annotation.Transactional;
 import com.digitalsanctuary.spring.user.dto.WebAuthnCredentialInfo;
 import com.digitalsanctuary.spring.user.persistence.model.WebAuthnCredential;
@@ -59,6 +62,7 @@ public long countCredentials(Long userId) {
 	 * @param userId the user ID
 	 * @return count of locked credential rows
 	 */
+	@Transactional(propagation = Propagation.MANDATORY)
 	public long lockAndCountCredentials(Long userId) {
 		return credentialRepository.lockCredentialIdsByUserEntityUserId(userId).size();
 	}
@@ -135,9 +139,18 @@ private Optional<WebAuthnCredential> findCredentialForUser(String credentialId,
 	 * @return the DTO
 	 */
 	private WebAuthnCredentialInfo toCredentialInfo(WebAuthnCredential entity) {
+		List<String> transportList = parseTransportList(entity.getAuthenticatorTransports());
 		return WebAuthnCredentialInfo.builder().id(entity.getCredentialId()).label(entity.getLabel())
 				.created(entity.getCreated()).lastUsed(entity.getLastUsed())
-				.transports(entity.getAuthenticatorTransports()).backupEligible(entity.isBackupEligible())
+				.transports(transportList).backupEligible(entity.isBackupEligible())
 				.backupState(entity.isBackupState()).build();
 	}
+
+	private List<String> parseTransportList(String transports) {
+		if (transports == null || transports.isEmpty()) {
+			return Collections.emptyList();
+		}
+		return Arrays.stream(transports.split(",")).map(String::trim).filter(s -> !s.isEmpty())
+				.collect(Collectors.toList());
+	}
 }

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

@@ -1,10 +1,11 @@
 package com.digitalsanctuary.spring.user.persistence.repository;
 
-import java.nio.ByteBuffer;
+import java.security.SecureRandom;
 import java.util.Base64;
 import java.util.Optional;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.context.annotation.Primary;
+import org.springframework.dao.DataIntegrityViolationException;
 import org.springframework.security.web.webauthn.api.Bytes;
 import org.springframework.security.web.webauthn.api.ImmutablePublicKeyCredentialUserEntity;
 import org.springframework.security.web.webauthn.api.PublicKeyCredentialUserEntity;
@@ -45,6 +46,7 @@ public PublicKeyCredentialUserEntity findById(Bytes id) {
 	}
 
 	@Override
+	@Transactional
 	public PublicKeyCredentialUserEntity findByUsername(String username) {
 		// Handle edge cases that can occur during login
 		if (username == null || username.isEmpty() || "anonymousUser".equals(username)) {
@@ -65,8 +67,13 @@ public PublicKeyCredentialUserEntity findByUsername(String username) {
 			return null;
 		}
 
-		// Create WebAuthn user entity for this application user
-		return createUserEntity(user);
+		// Create WebAuthn user entity for this application user, handling concurrent creation
+		try {
+			return createUserEntity(user);
+		} catch (DataIntegrityViolationException e) {
+			log.debug("Concurrent WebAuthn user entity creation for {}, retrying lookup", username);
+			return webAuthnUserEntityRepository.findByName(username).map(this::toSpringSecurityEntity).orElse(null);
+		}
 	}
 
 	@Override
@@ -107,7 +114,9 @@ public void delete(Bytes id) {
 	 */
 	@Transactional
 	public PublicKeyCredentialUserEntity createUserEntity(User user) {
-		Bytes userId = new Bytes(longToBytes(user.getId()));
+		byte[] randomHandle = new byte[64];
+		new SecureRandom().nextBytes(randomHandle);
+		Bytes userId = new Bytes(randomHandle);
 		String idStr = Base64.getUrlEncoder().withoutPadding().encodeToString(userId.getBytes());
 		String displayName = user.getFullName();
 
@@ -148,13 +157,4 @@ private PublicKeyCredentialUserEntity toSpringSecurityEntity(WebAuthnUserEntity
 				.displayName(entity.getDisplayName()).build();
 	}
 
-	/**
-	 * Convert Long ID to byte array for WebAuthn user ID.
-	 *
-	 * @param value the Long value to convert
-	 * @return byte array representation of the Long
-	 */
-	private byte[] longToBytes(Long value) {
-		return ByteBuffer.allocate(Long.BYTES).putLong(value).array();
-	}
 }

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

@@ -1,7 +1,6 @@
 package com.digitalsanctuary.spring.user.security;
 
 import java.io.IOException;
-import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContext;
 import org.springframework.security.core.context.SecurityContextHolder;
@@ -73,7 +72,7 @@ public void onAuthenticationSuccess(HttpServletRequest request, HttpServletRespo
 			UserDetails userDetails = userDetailsService.loadUserByUsername(username);
 
 			// Create new authentication with DSUserDetails as principal, preserving authorities
-			Authentication convertedAuth = UsernamePasswordAuthenticationToken.authenticated(userDetails, null,
+			Authentication convertedAuth = new WebAuthnAuthenticationToken(userDetails,
 					authentication.getAuthorities());
 
 			// Update SecurityContext with the converted authentication

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

@@ -0,0 +1,49 @@
+package com.digitalsanctuary.spring.user.security;
+
+import java.util.Collection;
+import org.springframework.security.authentication.AbstractAuthenticationToken;
+import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.userdetails.UserDetails;
+
+/**
+ * Authentication token representing a successful WebAuthn/passkey authentication.
+ *
+ * <p>
+ * This token replaces the use of {@link org.springframework.security.authentication.UsernamePasswordAuthenticationToken}
+ * for WebAuthn logins, making it possible to distinguish passkey-based sessions from password-based sessions
+ * in security logic and audit trails.
+ * </p>
+ *
+ * <p>
+ * The principal is the application's {@link UserDetails} (typically {@code DSUserDetails}), and
+ * {@link #getCredentials()} returns {@code null} because no password is involved in WebAuthn authentication.
+ * </p>
+ */
+public class WebAuthnAuthenticationToken extends AbstractAuthenticationToken {
+
+	private static final long serialVersionUID = 1L;
+
+	private final UserDetails principal;
+
+	/**
+	 * Creates a new WebAuthn authentication token.
+	 *
+	 * @param principal the authenticated user details
+	 * @param authorities the granted authorities
+	 */
+	public WebAuthnAuthenticationToken(UserDetails principal, Collection<? extends GrantedAuthority> authorities) {
+		super(authorities);
+		this.principal = principal;
+		setAuthenticated(true);
+	}
+
+	@Override
+	public Object getCredentials() {
+		return null;
+	}
+
+	@Override
+	public Object getPrincipal() {
+		return principal;
+	}
+}