Skip to content

Commit c0df541

Browse files
committed
refactor(mfa): address third round of PR review feedback
- Remove blank lines in WebSecurityConfig import block - Add clarifying comment in MfaConfiguration for silent skip behavior - Add positive-path MFA tests verifying fullyAuthenticated: true when user has all required FactorGrantedAuthority entries
1 parent d87eb24 commit c0df541

4 files changed

Lines changed: 38 additions & 2 deletions

File tree

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ public DefaultAuthorizationManagerFactory<Object> mfaAuthorizationManagerFactory
6262
AllRequiredFactorsAuthorizationManager.Builder<Object> factorsBuilder =
6363
AllRequiredFactorsAuthorizationManager.builder();
6464

65+
// Unknown/blank factors are silently skipped here; validateMfaConfiguration() will
66+
// throw before startup completes if the configuration is invalid.
6567
for (String factor : mfaConfigProperties.getFactors()) {
6668
if (factor == null || factor.isBlank()) {
6769
continue;

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import java.util.ArrayList;
44
import java.util.Arrays;
55
import java.util.Collections;
6-
76
import java.util.List;
87
import java.util.Map;
98
import java.util.Set;
@@ -29,7 +28,6 @@
2928
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
3029
import org.springframework.security.crypto.password.PasswordEncoder;
3130
import org.springframework.security.web.SecurityFilterChain;
32-
3331
import org.springframework.security.web.access.DelegatingMissingAuthorityAccessDeniedHandler;
3432
import org.springframework.security.web.authentication.LoginUrlAuthenticationEntryPoint;
3533
import org.springframework.security.web.session.HttpSessionEventPublisher;

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,12 @@
66
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
77
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrlPattern;
88
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
9+
import java.util.List;
910
import java.util.Set;
1011
import java.util.stream.Collectors;
12+
import org.springframework.security.core.GrantedAuthority;
13+
import org.springframework.security.core.authority.FactorGrantedAuthority;
14+
import org.springframework.security.core.authority.SimpleGrantedAuthority;
1115
import org.junit.jupiter.api.DisplayName;
1216
import org.junit.jupiter.api.Test;
1317
import org.springframework.beans.factory.annotation.Autowired;
@@ -90,4 +94,18 @@ void shouldRedirectToPasswordEntryPointWhenMissingFactorAuthority() throws Excep
9094
.andExpect(status().is3xxRedirection())
9195
.andExpect(redirectedUrlPattern(mfaConfigProperties.getPasswordEntryPointUri() + "**"));
9296
}
97+
98+
@Test
99+
@DisplayName("should report fully authenticated when user has all required factor authorities")
100+
void shouldReportFullyAuthenticatedWhenUserHasAllRequiredFactorAuthorities() throws Exception {
101+
List<GrantedAuthority> authorities = List.of(
102+
new SimpleGrantedAuthority("ROLE_USER"),
103+
FactorGrantedAuthority.fromAuthority(FactorGrantedAuthority.PASSWORD_AUTHORITY));
104+
105+
mockMvc.perform(get("/user/mfa/status").with(user("user@test.com").authorities(authorities)))
106+
.andExpect(status().isOk())
107+
.andExpect(jsonPath("$.data.fullyAuthenticated").value(true))
108+
.andExpect(jsonPath("$.data.missingFactors").isEmpty())
109+
.andExpect(jsonPath("$.data.satisfiedFactors[0]").value("PASSWORD"));
110+
}
93111
}

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
77
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
88
import java.util.List;
9+
import org.springframework.security.core.GrantedAuthority;
10+
import org.springframework.security.core.authority.FactorGrantedAuthority;
11+
import org.springframework.security.core.authority.SimpleGrantedAuthority;
912
import org.junit.jupiter.api.DisplayName;
1013
import org.junit.jupiter.api.Test;
1114
import org.springframework.beans.factory.annotation.Autowired;
@@ -91,4 +94,19 @@ void shouldReportBothFactorsAsMissingForUnauthenticatedRequest() throws Exceptio
9194
.andExpect(jsonPath("$.data.missingFactors.length()").value(2))
9295
.andExpect(jsonPath("$.data.fullyAuthenticated").value(false));
9396
}
97+
98+
@Test
99+
@DisplayName("should report fully authenticated when user has all required factor authorities")
100+
void shouldReportFullyAuthenticatedWhenUserHasAllRequiredFactorAuthorities() throws Exception {
101+
List<GrantedAuthority> authorities = List.of(
102+
new SimpleGrantedAuthority("ROLE_USER"),
103+
FactorGrantedAuthority.fromAuthority(FactorGrantedAuthority.PASSWORD_AUTHORITY),
104+
FactorGrantedAuthority.fromAuthority(FactorGrantedAuthority.WEBAUTHN_AUTHORITY));
105+
106+
mockMvc.perform(get("/user/mfa/status").with(user("user@test.com").authorities(authorities)))
107+
.andExpect(status().isOk())
108+
.andExpect(jsonPath("$.data.fullyAuthenticated").value(true))
109+
.andExpect(jsonPath("$.data.missingFactors").isEmpty())
110+
.andExpect(jsonPath("$.data.satisfiedFactors.length()").value(2));
111+
}
94112
}

0 commit comments

Comments
 (0)