refactor(mfa): address code review feedback on MFA implementation

- Change MfaStatusResponse from @DaTa to @value for immutability
- Use method-injected Authentication instead of SecurityContextHolder
- Remove duplicate FACTOR_AUTHORITY_MAP from MfaAPI, delegate to
  MfaConfiguration.mapFactorToAuthority() (now public)
- Replace switch/case in WebSecurityConfig.setupMfa() with HashMap lookup
- Add default AccessDeniedHandlerImpl fallback for non-MFA access denied
- Add null/blank guards in mfaAuthorizationManagerFactory and validator
- Remove redundant @propertysource from MfaConfiguration (already loaded
  by WebAuthnConfiguration per PR #264 consolidation)
- Normalize requiredFactors to uppercase for consistent API response
  casing across requiredFactors, satisfiedFactors, and missingFactors
- Fix inaccurate JavaDoc on validateMfaConfiguration

Author: devondragon

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

@@ -3,18 +3,16 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
-import java.util.Map;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.http.ResponseEntity;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.GrantedAuthority;
-import org.springframework.security.core.authority.FactorGrantedAuthority;
-import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
 import com.digitalsanctuary.spring.user.dto.MfaStatusResponse;
 import com.digitalsanctuary.spring.user.security.MfaConfigProperties;
+import com.digitalsanctuary.spring.user.security.MfaConfiguration;
 import com.digitalsanctuary.spring.user.util.JSONResponse;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
@@ -36,13 +34,6 @@
 @RequiredArgsConstructor
 public class MfaAPI {
 
-	/**
-	 * Mapping from user-facing factor names to Spring Security authority strings.
-	 */
-	private static final Map<String, String> FACTOR_AUTHORITY_MAP = Map.of(
-			"PASSWORD", FactorGrantedAuthority.PASSWORD_AUTHORITY,
-			"WEBAUTHN", FactorGrantedAuthority.WEBAUTHN_AUTHORITY);
-
 	private final MfaConfigProperties mfaConfigProperties;
 
 	/**
@@ -52,12 +43,13 @@ public class MfaAPI {
 	 * accessible to partially-authenticated users (added to unprotected URIs when MFA is enabled).
 	 * </p>
 	 *
+	 * @param authentication the current authentication, injected by Spring MVC (may be null)
 	 * @return a ResponseEntity containing the MFA status
 	 */
 	@GetMapping("/status")
-	public ResponseEntity<JSONResponse> getMfaStatus() {
-		List<String> requiredFactors = mfaConfigProperties.getFactors();
-		Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+	public ResponseEntity<JSONResponse> getMfaStatus(Authentication authentication) {
+		List<String> requiredFactors = mfaConfigProperties.getFactors().stream()
+				.map(String::toUpperCase).toList();
 
 		List<String> satisfiedFactors = new ArrayList<>();
 		List<String> missingFactors = new ArrayList<>();
@@ -66,15 +58,18 @@ public ResponseEntity<JSONResponse> getMfaStatus() {
 			Collection<? extends GrantedAuthority> authorities = authentication.getAuthorities();
 
 			for (String factor : requiredFactors) {
-				String authorityString = FACTOR_AUTHORITY_MAP.get(factor.toUpperCase());
+				String normalized = factor.toUpperCase();
+				String authorityString = MfaConfiguration.mapFactorToAuthority(normalized);
 				if (authorityString != null && hasAuthority(authorities, authorityString)) {
-					satisfiedFactors.add(factor);
+					satisfiedFactors.add(normalized);
 				} else {
-					missingFactors.add(factor);
+					missingFactors.add(normalized);
 				}
 			}
 		} else {
-			missingFactors.addAll(requiredFactors);
+			for (String factor : requiredFactors) {
+				missingFactors.add(factor.toUpperCase());
+			}
 		}
 
 		MfaStatusResponse status = MfaStatusResponse.builder()

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

@@ -2,7 +2,7 @@
 
 import java.util.List;
 import lombok.Builder;
-import lombok.Data;
+import lombok.Value;
 
 /**
  * Response DTO for the MFA status endpoint.
@@ -13,22 +13,22 @@
  *
  * @see com.digitalsanctuary.spring.user.api.MfaAPI
  */
-@Data
+@Value
 @Builder
 public class MfaStatusResponse {
 
 	/** Whether MFA is enabled on the server. */
-	private boolean mfaEnabled;
+	boolean mfaEnabled;
 
 	/** The list of required factor names (e.g., PASSWORD, WEBAUTHN). */
-	private List<String> requiredFactors;
+	List<String> requiredFactors;
 
 	/** The list of factor names that the current session has satisfied. */
-	private List<String> satisfiedFactors;
+	List<String> satisfiedFactors;
 
 	/** The list of factor names that the current session has not yet satisfied. */
-	private List<String> missingFactors;
+	List<String> missingFactors;
 
 	/** Whether the current session has satisfied all required factors. */
-	private boolean fullyAuthenticated;
+	boolean fullyAuthenticated;
 }

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

@@ -7,7 +7,6 @@
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
-import org.springframework.context.annotation.PropertySource;
 import org.springframework.context.event.ContextRefreshedEvent;
 import org.springframework.context.event.EventListener;
 import org.springframework.security.authorization.AllRequiredFactorsAuthorizationManager;
@@ -36,7 +35,6 @@
  */
 @Slf4j
 @Configuration
-@PropertySource("classpath:config/dsspringuserconfig.properties")
 @EnableConfigurationProperties(MfaConfigProperties.class)
 @RequiredArgsConstructor
 public class MfaConfiguration {
@@ -65,6 +63,9 @@ public DefaultAuthorizationManagerFactory<Object> mfaAuthorizationManagerFactory
 				AllRequiredFactorsAuthorizationManager.builder();
 
 		for (String factor : mfaConfigProperties.getFactors()) {
+			if (factor == null || factor.isBlank()) {
+				continue;
+			}
 			String authority = FACTOR_AUTHORITY_MAP.get(factor.toUpperCase());
 			if (authority != null) {
 				factorsBuilder.requireFactor(RequiredFactor.withAuthority(authority).build());
@@ -81,7 +82,8 @@ public DefaultAuthorizationManagerFactory<Object> mfaAuthorizationManagerFactory
 	}
 
 	/**
-	 * Validates MFA configuration on application startup. Runs only when MFA is enabled.
+	 * Validates MFA configuration on application startup. Performs validation only when MFA is enabled; returns
+	 * immediately otherwise.
 	 *
 	 * @param event the context refreshed event
 	 */
@@ -100,6 +102,10 @@ public void validateMfaConfiguration(ContextRefreshedEvent event) {
 		}
 
 		for (String factor : factors) {
+			if (factor == null || factor.isBlank()) {
+				throw new IllegalStateException(
+						"MFA factors list contains a null or blank entry. Check user.mfa.factors configuration.");
+			}
 			if (!FACTOR_AUTHORITY_MAP.containsKey(factor.toUpperCase())) {
 				throw new IllegalStateException(
 						"Unknown MFA factor: '" + factor + "'. Supported factors: " + FACTOR_AUTHORITY_MAP.keySet());
@@ -120,6 +126,9 @@ public void validateMfaConfiguration(ContextRefreshedEvent event) {
 
 	/**
 	 * Resolves the configured factor names to Spring Security authority strings.
+	 * <p>
+	 * Package-private for testing via {@link MfaConfigurationTest}.
+	 * </p>
 	 *
 	 * @return list of Spring Security authority strings
 	 */
@@ -140,7 +149,10 @@ List<String> resolveFactorAuthorities() {
 	 * @param factorName the factor name (e.g., "PASSWORD", "WEBAUTHN")
 	 * @return the corresponding Spring Security authority string, or null if unknown
 	 */
-	static String mapFactorToAuthority(String factorName) {
+	public static String mapFactorToAuthority(String factorName) {
+		if (factorName == null || factorName.isBlank()) {
+			return null;
+		}
 		return FACTOR_AUTHORITY_MAP.get(factorName.toUpperCase());
 	}
 }

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

@@ -3,7 +3,9 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
 import org.springframework.beans.factory.annotation.Value;
@@ -27,6 +29,7 @@
 import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
 import org.springframework.security.crypto.password.PasswordEncoder;
 import org.springframework.security.web.SecurityFilterChain;
+import org.springframework.security.web.access.AccessDeniedHandlerImpl;
 import org.springframework.security.web.access.DelegatingMissingAuthorityAccessDeniedHandler;
 import org.springframework.security.web.authentication.LoginUrlAuthenticationEntryPoint;
 import org.springframework.security.web.session.HttpSessionEventPublisher;
@@ -260,28 +263,21 @@ private void setupMfa(HttpSecurity http) throws Exception {
 		DelegatingMissingAuthorityAccessDeniedHandler.Builder handlerBuilder =
 				DelegatingMissingAuthorityAccessDeniedHandler.builder();
 
+		Map<String, String> factorToUri = new HashMap<>();
+		factorToUri.put("PASSWORD", mfaConfigProperties.getPasswordEntryPointUri());
+		factorToUri.put("WEBAUTHN", mfaConfigProperties.getWebauthnEntryPointUri());
+
 		for (String factor : mfaConfigProperties.getFactors()) {
 			String authority = MfaConfiguration.mapFactorToAuthority(factor);
-			if (authority == null) {
-				continue;
-			}
-			switch (factor.toUpperCase()) {
-				case "PASSWORD":
-					handlerBuilder.addEntryPointFor(
-							new LoginUrlAuthenticationEntryPoint(mfaConfigProperties.getPasswordEntryPointUri()),
-							authority);
-					break;
-				case "WEBAUTHN":
-					handlerBuilder.addEntryPointFor(
-							new LoginUrlAuthenticationEntryPoint(mfaConfigProperties.getWebauthnEntryPointUri()),
-							authority);
-					break;
-				default:
-					break;
+			String uri = factorToUri.get(factor.toUpperCase());
+			if (authority != null && uri != null) {
+				handlerBuilder.addEntryPointFor(new LoginUrlAuthenticationEntryPoint(uri), authority);
 			}
 		}
 
-		http.exceptionHandling(handling -> handling.accessDeniedHandler(handlerBuilder.build()));
+		DelegatingMissingAuthorityAccessDeniedHandler handler = handlerBuilder.build();
+		handler.setDefaultAccessDeniedHandler(new AccessDeniedHandlerImpl());
+		http.exceptionHandling(handling -> handling.accessDeniedHandler(handler));
 		log.info("MFA configured with access denied handler for factors: {}", mfaConfigProperties.getFactors());
 	}