fix: escape passkey notification label and defer registration event to after-commit

CONFIG.md

@@ -160,7 +160,7 @@ user:
 
 **Reserved authority names.** Do not name a role or privilege `FACTOR_*` in `user.roles-and-privileges`. Spring Security uses that prefix for factor authorities, and a plain authority with such a name is indistinguishable from a real factor by name: it satisfies MFA enforcement without the factor ever being completed, and it shadows the genuine factor in a step-up freshness check. Startup fails when such a name is configured while MFA or step-up is enabled, and logs an error otherwise.
 
-**Custom implementations.** A `StepUpService` bean supplied by your application takes precedence over the built-in one, whatever `enabled` is set to. Implement the SPI to require TOTP, a hardware token, or any other proof.
+**Custom implementations.** A `StepUpService` bean supplied by your application always replaces the built-in one (the built-in bean backs off when yours is present). Implement the SPI to require TOTP, a hardware token, or any other proof. Where that bean is consulted depends on the endpoint: `POST /user/setPassword` uses it whenever the bean is present, whatever `enabled` is set to (this is the original SPI surface from 5.3.1). The passkey delete, rename, and remove-password endpoints consult it only when `user.security.stepUp.enabled=true`, so an application that adopted the SPI for `setPassword` alone does not have step-up newly enforced on those endpoints on upgrade. To gate every credential-altering operation through your bean, set `enabled=true`.
 
 - **Allow Without Step-Up (`user.security.allowInitialPasswordSetWithoutStepUp`)**: When no `StepUpService` bean is present at all, `setPassword` is **disabled** (`HTTP 403`) unless this is `true`, which restores the previous session-only behavior. Default: `false`.
 

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

@@ -1,8 +1,9 @@
 package com.digitalsanctuary.spring.user.listener;
 
 import org.springframework.context.ApplicationEventPublisher;
-import org.springframework.context.ApplicationListener;
 import org.springframework.stereotype.Component;
+import org.springframework.transaction.event.TransactionPhase;
+import org.springframework.transaction.event.TransactionalEventListener;
 import com.digitalsanctuary.spring.user.audit.AuditEvent;
 import com.digitalsanctuary.spring.user.event.WebAuthnCredentialRegisteredEvent;
 import com.digitalsanctuary.spring.user.security.WebAuthnConfigProperties;
@@ -24,7 +25,7 @@
 @Slf4j
 @Component
 @RequiredArgsConstructor
-public class WebAuthnCredentialRegistrationListener implements ApplicationListener<WebAuthnCredentialRegisteredEvent> {
+public class WebAuthnCredentialRegistrationListener {
 
     private final UserEmailService userEmailService;
     private final ApplicationEventPublisher eventPublisher;
@@ -33,15 +34,29 @@ public class WebAuthnCredentialRegistrationListener implements ApplicationListen
     /**
      * Audits the enrollment and, unless disabled, emails the account owner.
      *
+     * <p>
+     * Runs after the enrollment transaction commits. The credential is written through a {@code @Transactional}
+     * {@code save()} that publishes this event before the commit, so a commit failure (for example a label longer
+     * than the column) would otherwise send a notification and record an audit entry for a registration that never
+     * persisted. {@code fallbackExecution = true} keeps the listener firing if the event is ever published outside a
+     * transaction.
+     * </p>
+     *
      * @param event the registration event
      */
-    @Override
-    public void onApplicationEvent(WebAuthnCredentialRegisteredEvent event) {
+    @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT, fallbackExecution = true)
+    public void onCredentialRegistered(WebAuthnCredentialRegisteredEvent event) {
         // Audit first and unconditionally: the notification is a courtesy the operator can switch off, and a mail
-        // outage must not cost us the security-relevant record of the enrollment.
-        eventPublisher.publishEvent(AuditEvent.builder().source(this).user(event.getUser())
-                .action("PasskeyRegistration").actionStatus("Success")
-                .message("Passkey registered: " + event.getLabel()).build());
+        // outage must not cost us the security-relevant record of the enrollment. Because this runs after commit, an
+        // exception here would be swallowed by Spring's transactional-listener adapter with no trace, so log it in this
+        // class rather than lose the record silently.
+        try {
+            eventPublisher.publishEvent(AuditEvent.builder().source(this).user(event.getUser())
+                    .action("PasskeyRegistration").actionStatus("Success")
+                    .message("Passkey registered: " + event.getLabel()).build());
+        } catch (RuntimeException e) {
+            log.error("Failed to record passkey registration audit event for user {}", event.getUser().getId(), e);
+        }
 
         if (!webAuthnConfigProperties.isNotifyOnRegistration()) {
             return;

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

@@ -0,0 +1,65 @@
+package com.digitalsanctuary.spring.user.security;
+
+import java.io.IOException;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
+import org.springframework.security.access.AccessDeniedException;
+import org.springframework.security.authorization.AuthorizationDeniedException;
+import org.springframework.security.web.access.AccessDeniedHandler;
+import com.digitalsanctuary.spring.user.exceptions.WebAuthnStepUpRequiredException;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import lombok.NonNull;
+import lombok.RequiredArgsConstructor;
+
+/**
+ * Renders a step-up denial on the passkey enrollment endpoint the way the credential-management endpoints do: HTTP
+ * 401 with a JSON body carrying the {@code step-up-required} error code, so a client can launch its login ceremony
+ * and retry rather than receiving a bare 403 it cannot interpret.
+ *
+ * <p>
+ * The enrollment gate is a filter-chain {@code authorizeHttpRequests} rule, so its denial is raised before any
+ * controller runs and never reaches {@code WebAuthnManagementAPIAdvice}. Only a freshness denial (an
+ * {@link AuthorizationDeniedException}) is treated as step-up; anything else that can deny the endpoint, such as a
+ * CSRF failure, is passed to the delegate so it keeps its normal 403.
+ * </p>
+ *
+ * <p>
+ * The step-up classification is by exception type, and it is sound only because the freshness gate is the sole
+ * authorization rule on {@code POST /webauthn/register}. If another {@code authorizeHttpRequests} rule (a role or
+ * scope check) is ever added to that path, its denial is also an {@link AuthorizationDeniedException} and would be
+ * mislabeled as step-up, telling the client to re-authenticate when re-authentication cannot help. Keep this handler
+ * scoped to a path whose only authorization rule is the freshness gate.
+ * </p>
+ */
+@RequiredArgsConstructor
+public class StepUpEnrollmentAccessDeniedHandler implements AccessDeniedHandler {
+
+    /** Handles denials on the endpoint that are not the freshness gate (for example, CSRF). Required (non-null). */
+    @NonNull
+    private final AccessDeniedHandler delegate;
+
+    @Override
+    public void handle(HttpServletRequest request, HttpServletResponse response, AccessDeniedException accessDeniedException)
+            throws IOException, ServletException {
+        if (!(accessDeniedException instanceof AuthorizationDeniedException)) {
+            delegate.handle(request, response, accessDeniedException);
+            return;
+        }
+        // A prior filter having already committed the response would make setStatus/setContentType silently no-ops and
+        // append the JSON to whatever was flushed, so bail rather than emit a malformed body.
+        if (response.isCommitted()) {
+            return;
+        }
+        response.setStatus(HttpStatus.UNAUTHORIZED.value());
+        response.setContentType(MediaType.APPLICATION_JSON_VALUE);
+        response.setCharacterEncoding("UTF-8");
+        // The message and error code are compile-time constants with no characters that need JSON escaping, so the
+        // body is assembled directly. This keeps the handler independent of whichever Jackson version the consuming
+        // application ships, matching the {message, error} shape GenericResponse serializes for the sibling endpoints.
+        response.getWriter().write("{\"message\":\"Recent authentication is required to add a passkey. "
+                + "Please verify with your passkey or password and retry.\",\"error\":\"" + WebAuthnStepUpRequiredException.ERROR_CODE
+                + "\"}");
+    }
+}

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

@@ -4,6 +4,7 @@
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -22,10 +23,15 @@
 import org.springframework.security.core.userdetails.UserDetailsService;
 import org.springframework.security.web.AuthenticationEntryPoint;
 import org.springframework.security.web.SecurityFilterChain;
+import org.springframework.security.web.access.AccessDeniedHandler;
+import org.springframework.security.web.access.AccessDeniedHandlerImpl;
 import org.springframework.security.web.access.DelegatingMissingAuthorityAccessDeniedHandler;
+import org.springframework.security.web.access.RequestMatcherDelegatingAccessDeniedHandler;
 import org.springframework.security.web.authentication.LoginUrlAuthenticationEntryPoint;
 import org.springframework.security.web.authentication.rememberme.PersistentTokenRepository;
 import org.springframework.security.web.savedrequest.RequestCache;
+import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher;
+import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.security.web.webauthn.authentication.WebAuthnAuthenticationFilter;
 import com.digitalsanctuary.spring.user.service.DSOAuth2UserService;
 import com.digitalsanctuary.spring.user.service.DSOidcUserService;
@@ -168,10 +174,11 @@ public SecurityFilterChain buildSecurityFilterChain(HttpSecurity http, SessionRe
 			setupWebAuthn(http);
 		}
 
-		// Configure MFA if enabled
-		if (mfaConfigProperties.isEnabled()) {
-			setupMfa(http);
-		}
+		// Configure MFA if enabled. setupMfa returns its missing-authority handler instead of installing it, so the
+		// WebAuthn enrollment step-up handler below can compose over it. The access-denied handler is installed once,
+		// after both features have been configured.
+		DelegatingMissingAuthorityAccessDeniedHandler mfaAccessDeniedHandler =
+				mfaConfigProperties.isEnabled() ? setupMfa() : null;
 
 		// Close Spring Security's built-in WebAuthn credential-delete endpoint (DELETE /webauthn/register/{id}),
 		// registered by http.webAuthn(...). It deletes a passkey after checking only credential ownership, bypassing
@@ -198,6 +205,29 @@ public SecurityFilterChain buildSecurityFilterChain(HttpSecurity http, SessionRe
 			}
 		}
 
+		// Install the access-denied handler once, composing the feature handlers so each path keeps its correct denial
+		// response. The enrollment gate denies in the filter chain, before any controller, so its denial never reaches
+		// WebAuthnManagementAPIAdvice; render it the way the credential-management endpoints render step-up (HTTP 401
+		// with the step-up-required error code) instead of a bare 403 the client cannot interpret. This must be built
+		// explicitly rather than with a single defaultAccessDeniedHandlerFor mapping: Spring collapses a lone mapping to
+		// the handler alone and drops the matcher (ExceptionHandlingConfigurer.createDefaultAccessDeniedHandler), which
+		// would make the step-up handler the app-wide default; and when MFA sets its own handler, the mapping is ignored
+		// outright. So compose a RequestMatcherDelegatingAccessDeniedHandler: enrollment POST -> step-up handler,
+		// everything else -> the MFA missing-authority handler when configured, else a plain 403.
+		AccessDeniedHandler baseAccessDeniedHandler =
+				mfaAccessDeniedHandler != null ? mfaAccessDeniedHandler : new AccessDeniedHandlerImpl();
+		if (webAuthnConfigProperties.isEnabled() && stepUpConfigProperties.isEnabled()) {
+			RequestMatcher enrollmentPost =
+					PathPatternRequestMatcher.withDefaults().matcher(HttpMethod.POST, "/webauthn/register");
+			LinkedHashMap<RequestMatcher, AccessDeniedHandler> deniedHandlers = new LinkedHashMap<>();
+			deniedHandlers.put(enrollmentPost, new StepUpEnrollmentAccessDeniedHandler(baseAccessDeniedHandler));
+			AccessDeniedHandler enrollmentAwareHandler =
+					new RequestMatcherDelegatingAccessDeniedHandler(deniedHandlers, baseAccessDeniedHandler);
+			http.exceptionHandling(handling -> handling.accessDeniedHandler(enrollmentAwareHandler));
+		} else if (mfaAccessDeniedHandler != null) {
+			http.exceptionHandling(handling -> handling.accessDeniedHandler(mfaAccessDeniedHandler));
+		}
+
 		// Configure authorization rules based on the default action
 		String defaultAction = userSecurityConfig.getDefaultAction();
 		if (DEFAULT_ACTION_DENY.equals(defaultAction)) {
@@ -263,14 +293,15 @@ private void setupWebAuthn(HttpSecurity http) throws Exception {
 	/**
 	 * Setup MFA specific configuration.
 	 * <p>
-	 * Configures a {@link DelegatingMissingAuthorityAccessDeniedHandler} that redirects partially-authenticated users to
-	 * the appropriate factor login page when they are missing a required factor authority.
+	 * Builds a {@link DelegatingMissingAuthorityAccessDeniedHandler} that redirects partially-authenticated users to the
+	 * appropriate factor login page when they are missing a required factor authority. The caller installs the returned
+	 * handler (composing it with the WebAuthn enrollment step-up handler when both features are enabled), so this method
+	 * does not touch {@code http} itself.
 	 * </p>
 	 *
-	 * @param http the http security object to configure
-	 * @throws Exception the exception
+	 * @return the missing-authority access-denied handler for the configured factors
 	 */
-	private void setupMfa(HttpSecurity http) throws Exception {
+	private DelegatingMissingAuthorityAccessDeniedHandler setupMfa() {
 		DelegatingMissingAuthorityAccessDeniedHandler.Builder handlerBuilder =
 				DelegatingMissingAuthorityAccessDeniedHandler.builder();
 
@@ -287,8 +318,8 @@ private void setupMfa(HttpSecurity http) throws Exception {
 		}
 
 		DelegatingMissingAuthorityAccessDeniedHandler handler = handlerBuilder.build();
-		http.exceptionHandling(handling -> handling.accessDeniedHandler(handler));
 		log.info("MFA configured with access denied handler for factors: {}", mfaConfigProperties.getFactors());
+		return handler;
 	}
 
 	/**

src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java

@@ -17,6 +17,7 @@
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
+import org.springframework.web.util.HtmlUtils;
 import com.digitalsanctuary.spring.user.audit.AuditEvent;
 import com.digitalsanctuary.spring.user.mail.MailService;
 import com.digitalsanctuary.spring.user.persistence.model.PasswordResetToken;
@@ -193,7 +194,10 @@ public void sendPasskeyRegisteredNotification(final User user, final String labe
         }
         Map<String, Object> variables = new HashMap<>();
         variables.put("user", user);
-        variables.put("label", label != null ? label : "Passkey");
+        // The label is client-supplied at enrollment and the template renders it unescaped (th:utext, so the
+        // message's own <strong> markup survives). Escape it here so a crafted label cannot inject HTML into the
+        // very email that warns the owner of an unrecognized passkey.
+        variables.put("label", HtmlUtils.htmlEscape(label != null ? label : "Passkey"));
         mailService.sendTemplateMessage(user.getEmail(), "New passkey added to your account", variables,
                 "mail/webauthn-credential-registered.html");
     }

src/test/java/com/digitalsanctuary/spring/user/listener/WebAuthnCredentialRegistrationListenerTest.java

@@ -10,6 +10,8 @@
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentCaptor;
 import org.springframework.context.ApplicationEventPublisher;
+import org.springframework.transaction.event.TransactionPhase;
+import org.springframework.transaction.event.TransactionalEventListener;
 import com.digitalsanctuary.spring.user.audit.AuditEvent;
 import com.digitalsanctuary.spring.user.event.WebAuthnCredentialRegisteredEvent;
 import com.digitalsanctuary.spring.user.persistence.model.User;
@@ -49,7 +51,7 @@ private WebAuthnCredentialRegistrationListener listener() {
     @Test
     @DisplayName("should record an audit event when a passkey is registered")
     void shouldRecordAuditEventWhenPasskeyRegistered() {
-        listener().onApplicationEvent(event());
+        listener().onCredentialRegistered(event());
 
         ArgumentCaptor<AuditEvent> captor = ArgumentCaptor.forClass(AuditEvent.class);
         verify(eventPublisher).publishEvent(captor.capture());
@@ -61,7 +63,7 @@ void shouldRecordAuditEventWhenPasskeyRegistered() {
     @Test
     @DisplayName("should notify the account owner by email when a passkey is registered")
     void shouldNotifyOwnerWhenPasskeyRegistered() {
-        listener().onApplicationEvent(event());
+        listener().onCredentialRegistered(event());
 
         verify(userEmailService).sendPasskeyRegisteredNotification(user, "Work Laptop");
     }
@@ -72,7 +74,7 @@ void shouldStillAuditWhenNotificationDisabled() {
         // The email is a courtesy the operator may not want; the audit trail is not optional.
         config.setNotifyOnRegistration(false);
 
-        listener().onApplicationEvent(event());
+        listener().onCredentialRegistered(event());
 
         verify(userEmailService, never()).sendPasskeyRegisteredNotification(any(), any());
         verify(eventPublisher).publishEvent(any(AuditEvent.class));
@@ -85,8 +87,23 @@ void shouldAuditEvenWhenNotificationFails() {
         org.mockito.Mockito.doThrow(new RuntimeException("smtp down")).when(userEmailService)
                 .sendPasskeyRegisteredNotification(any(), any());
 
-        listener().onApplicationEvent(event());
+        listener().onCredentialRegistered(event());
 
         verify(eventPublisher).publishEvent(any(AuditEvent.class));
     }
+
+    @Test
+    @DisplayName("should react only after the enrollment transaction commits")
+    void shouldReactAfterCommit() throws NoSuchMethodException {
+        // The credential is written through a @Transactional save() that publishes the event before commit. Reacting
+        // after commit stops a rolled-back registration (e.g. a label too long for the column) from emailing the
+        // owner and recording an audit entry for a passkey that never persisted.
+        TransactionalEventListener annotation = WebAuthnCredentialRegistrationListener.class
+                .getMethod("onCredentialRegistered", WebAuthnCredentialRegisteredEvent.class)
+                .getAnnotation(TransactionalEventListener.class);
+
+        assertThat(annotation).isNotNull();
+        assertThat(annotation.phase()).isEqualTo(TransactionPhase.AFTER_COMMIT);
+        assertThat(annotation.fallbackExecution()).isTrue();
+    }
 }