Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ repositories {

dependencies {
// DigitalSanctuary Spring User Framework
implementation 'com.digitalsanctuary:ds-spring-user-framework:4.3.1'
implementation 'com.digitalsanctuary:ds-spring-user-framework:4.4.0'

// WebAuthn support (Passkey authentication)
implementation 'org.springframework.security:spring-security-webauthn'
Expand Down Expand Up @@ -80,7 +80,9 @@ dependencies {
developmentOnly 'org.springframework.boot:spring-boot-docker-compose'

// Utility libraries
implementation 'org.passay:passay:1.6.6'
// (passay is provided transitively by ds-spring-user-framework at the version it requires; the demo
// app does not use passay directly, so declaring/pinning it here previously forced a conflicting
// downgrade of the library's required passay version.)
implementation 'com.google.guava:guava:33.5.0-jre'
implementation 'org.hibernate.validator:hibernate-validator'

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.digitalsanctuary.spring.user.persistence.model.User;
import com.digitalsanctuary.spring.user.mail.MailService;
import com.digitalsanctuary.spring.user.service.SessionInvalidationService;
import com.digitalsanctuary.spring.user.service.TokenHasher;
import com.digitalsanctuary.spring.user.service.UserEmailService;
import com.digitalsanctuary.spring.user.service.UserVerificationService;

Expand All @@ -36,8 +37,9 @@ public CustomUserEmailService(
UserVerificationService userVerificationService,
PasswordResetTokenRepository passwordTokenRepository,
ApplicationEventPublisher eventPublisher,
SessionInvalidationService sessionInvalidationService) {
super(mailService, userVerificationService, passwordTokenRepository, eventPublisher, sessionInvalidationService);
SessionInvalidationService sessionInvalidationService,
TokenHasher tokenHasher) {
super(mailService, userVerificationService, passwordTokenRepository, eventPublisher, sessionInvalidationService, tokenHasher);
}

@Override
Expand Down
8 changes: 4 additions & 4 deletions src/main/resources/application-mfa.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
# ./gradlew bootRun --args='--spring.profiles.active=local,mfa'
#
# Notes:
# - The challenge page (user.mfa.webauthnEntryPointUri) must be in unprotectedURIs. The framework
# redirects partially-authenticated users to it; if the page itself required full authentication,
# the redirect would loop forever.
# - The challenge page (user.mfa.webauthnEntryPointUri) is auto-unprotected by the framework: it
# unprotects the configured factor entry-point URIs so the partial-auth redirect cannot loop. (No
# need to list it in unprotectedURIs by hand.)
# - The passkey registration endpoints (/webauthn/register/options, /webauthn/register) are also
# unprotected here so that a partially-authenticated user can enroll their first passkey. Spring
# Security still requires an authenticated principal to register a credential; this only relaxes
Expand All @@ -19,4 +19,4 @@ user:
mfa:
enabled: true
security:
unprotectedURIs: /,/index.html,/favicon.ico,/apple-touch-icon-precomposed.png,/css/*,/js/*,/js/user/*,/js/event/*,/js/utils/*,/img/**,/user/registration,/user/registration/passwordless,/user/resendRegistrationToken,/user/resetPassword,/user/registrationConfirm,/user/changePassword,/user/savePassword,/oauth2/authorization/*,/login,/user/login,/user/login.html,/swagger-ui.html,/swagger-ui/**,/v3/api-docs/**,/event/,/event/list.html,/event/**,/about.html,/error,/error.html,/webauthn/authenticate/**,/login/webauthn,/user/mfa/webauthn-challenge.html,/webauthn/register/options,/webauthn/register
unprotectedURIs: /,/index.html,/favicon.ico,/apple-touch-icon-precomposed.png,/css/*,/js/*,/js/user/*,/js/event/*,/js/utils/*,/img/**,/user/registration,/user/registration/passwordless,/user/resendRegistrationToken,/user/resetPassword,/user/registrationConfirm,/user/changePassword,/user/savePassword,/oauth2/authorization/*,/login,/user/login,/user/login.html,/swagger-ui.html,/swagger-ui/**,/v3/api-docs/**,/event/,/event/list.html,/event/**,/about.html,/error,/error.html,/webauthn/authenticate/**,/login/webauthn,/webauthn/register/options,/webauthn/register
2 changes: 1 addition & 1 deletion src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ user:
bcryptStrength: 12 # The bcrypt strength to use for password hashing. The higher the number, the longer it takes to hash the password. The default is 12. The minimum is 4. The maximum is 31.
testHashTime: true # If true, the test hash time will be logged to the console on startup. This is useful for determining the optimal bcryptStrength value.
defaultAction: deny # The default action for all requests. This can be either deny or allow.
unprotectedURIs: /,/index.html,/favicon.ico,/apple-touch-icon-precomposed.png,/css/*,/js/*,/js/user/*,/js/event/*,/js/utils/*,/img/**,/user/registration,/user/registration/passwordless,/user/resendRegistrationToken,/user/resetPassword,/user/registrationConfirm,/user/changePassword,/user/savePassword,/oauth2/authorization/*,/login,/user/login,/user/login.html,/swagger-ui.html,/swagger-ui/**,/v3/api-docs/**,/event/,/event/list.html,/event/**,/about.html,/error,/error.html,/webauthn/authenticate/**,/login/webauthn,/user/mfa/webauthn-challenge.html # A comma delimited list of URIs that should not be protected by Spring Security if the defaultAction is deny.
unprotectedURIs: /,/index.html,/favicon.ico,/apple-touch-icon-precomposed.png,/css/*,/js/*,/js/user/*,/js/event/*,/js/utils/*,/img/**,/user/registration,/user/registration/passwordless,/user/resendRegistrationToken,/user/resetPassword,/user/registrationConfirm,/user/changePassword,/user/savePassword,/oauth2/authorization/*,/login,/user/login,/user/login.html,/swagger-ui.html,/swagger-ui/**,/v3/api-docs/**,/event/,/event/list.html,/event/**,/about.html,/error,/error.html,/webauthn/authenticate/**,/login/webauthn # A comma delimited list of URIs that should not be protected by Spring Security if the defaultAction is deny.
protectedURIs: /protected.html # A comma delimited list of URIs that should be protected by Spring Security if the defaultAction is allow.
disableCSRFdURIs: /no-csrf-test # A comma delimited list of URIs that should not be protected by CSRF protection. This may include API endpoints that need to be called without a CSRF token.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@
/**
* Guards the consistency of the MFA configuration files themselves.
*
* The MFA entry point pages must be listed in {@code user.security.unprotectedURIs}: the framework's
* access-denied handler redirects partially-authenticated users to the entry point URI, and if that page is itself
* protected the redirect loops forever. The framework only auto-unprotects {@code /user/mfa/status}, not the entry
* point pages, so the demo config has to keep these two settings in sync by hand.
* As of the framework release containing the #313 fix, the framework auto-unprotects the configured MFA factor
* entry-point URIs ({@code user.mfa.passwordEntryPointUri} / {@code user.mfa.webauthnEntryPointUri}), so the demo no
* longer has to list the WebAuthn challenge page in {@code user.security.unprotectedURIs} by hand. These tests now only
* assert what is still the demo's own responsibility: MFA must be opt-in (disabled by default), and the passkey
* <em>enrollment</em> endpoints (which are not factor entry points) must be reachable by partially-authenticated users.
*/
@DisplayName("MFA Config Consistency Tests")
class MfaConfigConsistencyTest {
Expand Down Expand Up @@ -44,19 +45,6 @@ private static List<String> unprotectedUris(Map<String, Object> yaml) {
return Arrays.stream(uris.toString().split(",")).map(String::trim).toList();
}

@Test
@DisplayName("application.yml unprotects the configured WebAuthn challenge page")
void baseConfigUnprotectsWebauthnEntryPoint() {
Map<String, Object> yaml = loadYaml("/application.yml");
Map<String, Object> mfa = section(yaml, "user", "mfa");

String webauthnEntryPoint = String.valueOf(mfa.get("webauthnEntryPointUri"));
assertThat(webauthnEntryPoint).as("user.mfa.webauthnEntryPointUri should be configured").isNotEqualTo("null");
assertThat(unprotectedUris(yaml))
.as("the WebAuthn challenge page must be unprotected or MFA redirects loop forever")
.contains(webauthnEntryPoint);
}

@Test
@DisplayName("application.yml leaves MFA disabled by default (opt-in via the mfa profile)")
void baseConfigLeavesMfaDisabled() {
Expand All @@ -67,16 +55,16 @@ void baseConfigLeavesMfaDisabled() {
}

@Test
@DisplayName("mfa profile enables MFA and unprotects the challenge page and passkey enrollment endpoints")
void mfaProfileEnablesMfaAndUnprotectsEntryPoint() {
@DisplayName("mfa profile enables MFA and unprotects the passkey enrollment endpoints")
void mfaProfileEnablesMfaAndUnprotectsEnrollmentEndpoints() {
Map<String, Object> yaml = loadYaml("/application-mfa.yml");
Map<String, Object> mfa = section(yaml, "user", "mfa");
assertThat(mfa.get("enabled")).isEqualTo(Boolean.TRUE);

// The WebAuthn challenge page (the configured factor entry point) is now auto-unprotected by the
// framework, so it no longer needs to be listed here. The passkey ENROLLMENT endpoints are not factor
// entry points, so partially-authenticated users still need them unprotected to enroll a first passkey.
List<String> uris = unprotectedUris(yaml);
assertThat(uris).contains("/user/mfa/webauthn-challenge.html");
// Partially-authenticated users need to be able to enroll their first passkey, otherwise new
// accounts can never satisfy the WEBAUTHN factor.
assertThat(uris).contains("/webauthn/register/options", "/webauthn/register");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
Expand All @@ -16,12 +17,16 @@
import org.springframework.test.context.bean.override.mockito.MockitoBean;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.MvcResult;
import org.springframework.transaction.PlatformTransactionManager;
import org.springframework.transaction.TransactionDefinition;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.support.TransactionTemplate;
import com.digitalsanctuary.spring.demo.UserDemoApplication;
import com.digitalsanctuary.spring.user.dto.UserDto;
import com.digitalsanctuary.spring.user.mail.MailService;
import com.digitalsanctuary.spring.user.persistence.model.User;
import com.digitalsanctuary.spring.user.persistence.repository.UserRepository;
import com.digitalsanctuary.spring.user.persistence.repository.VerificationTokenRepository;
import com.digitalsanctuary.spring.user.service.UserService;
import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.persistence.EntityManager;
Expand Down Expand Up @@ -50,24 +55,61 @@ class UserApiIntegrationTestFixed {
@Autowired
private UserService userService;

@Autowired
private VerificationTokenRepository verificationTokenRepository;

@Autowired
private PlatformTransactionManager transactionManager;

@MockitoBean
private MailService mailService;

@PersistenceContext
private EntityManager entityManager;

private static final String TEST_EMAIL = "test@example.com";

private UserDto testUserDto;

@BeforeEach
void setUp() {
// Start from a clean slate. Registration (via the API and via UserService.registerNewUserAccount)
// commits the new user in its own transaction as of 4.4.0 — it does NOT roll back with the test's
// @Transactional. Without a committed cleanup, the user persisted by one test method collides with
// the next (UserAlreadyExistException / 409 Conflict).
deleteTestUserCommitted();

testUserDto = new UserDto();
testUserDto.setFirstName("Test");
testUserDto.setLastName("User");
testUserDto.setEmail("test@example.com");
testUserDto.setEmail(TEST_EMAIL);
testUserDto.setPassword("SecurePass123!");
testUserDto.setMatchingPassword("SecurePass123!");
}

@AfterEach
void tearDown() {
// Remove the user committed by this test so it cannot leak into other tests.
deleteTestUserCommitted();
}

/**
* Deletes the test user (and any verification token) in its own committed transaction. A
* REQUIRES_NEW transaction is required because the class is {@code @Transactional}: a plain delete here
* would roll back with the test and never actually remove the committed registration row.
*/
private void deleteTestUserCommitted() {
TransactionTemplate tx = new TransactionTemplate(transactionManager);
tx.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW);
tx.executeWithoutResult(status -> {
User existing = userRepository.findByEmail(TEST_EMAIL);
if (existing != null) {
verificationTokenRepository.deleteByUser(existing);
userRepository.delete(existing);
}
});
}

@Test
@DisplayName("Should successfully register new user")
void shouldRegisterNewUser() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,9 @@ void getAuthoritiesFromUser_persistedUser_loadsAuthoritiesWithLazyLoading() {
Collection<? extends GrantedAuthority> authorities = authorityService
.getAuthoritiesFromUser(fetchedUser);

// Then
assertThat(authorities).hasSize(3).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
"LOGIN_PRIVILEGE",
// Then - as of 4.4.0 the role name itself is also granted (needed for hasRole() checks)
assertThat(authorities).hasSize(4).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
"ROLE_USER", "LOGIN_PRIVILEGE",
"UPDATE_OWN_USER_PRIVILEGE", "RESET_OWN_PASSWORD_PRIVILEGE");
}

Expand All @@ -159,9 +159,10 @@ void getAuthoritiesFromUser_multipleRoles_combinesAllPrivileges() {
Collection<? extends GrantedAuthority> authorities = authorityService
.getAuthoritiesFromUser(multiRoleUser);

// Then
assertThat(authorities).hasSize(6) // 3 from user role + 3 from manager role
.extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder("LOGIN_PRIVILEGE",
// Then - 6 privileges + the two role names (ROLE_USER, ROLE_MANAGER) granted as of 4.4.0
assertThat(authorities).hasSize(8) // 3 user + 3 manager privileges + 2 role names
.extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder("ROLE_USER",
"ROLE_MANAGER", "LOGIN_PRIVILEGE",
"UPDATE_OWN_USER_PRIVILEGE",
"RESET_OWN_PASSWORD_PRIVILEGE", "ADD_USER_TO_TEAM_PRIVILEGE",
"REMOVE_USER_FROM_TEAM_PRIVILEGE",
Expand All @@ -180,9 +181,9 @@ void getAuthoritiesFromUser_adminUser_hasAllAdminPrivileges() {
// When
Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromUser(adminUser);

// Then
assertThat(authorities).hasSize(5).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
"ADMIN_PRIVILEGE",
// Then - 5 admin privileges + the ROLE_ADMIN role name granted as of 4.4.0
assertThat(authorities).hasSize(6).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
"ROLE_ADMIN", "ADMIN_PRIVILEGE",
"INVITE_USER_PRIVILEGE", "READ_USER_PRIVILEGE", "ASSIGN_MANAGER_PRIVILEGE",
"RESET_ANY_USER_PASSWORD_PRIVILEGE");
}
Expand All @@ -206,9 +207,9 @@ void getAuthoritiesFromRoles_sharedPrivileges_deduplicatesCorrectly() {
Collection<? extends GrantedAuthority> authorities = authorityService
.getAuthoritiesFromRoles(Arrays.asList(role1, role2));

// Then - Should only have 4 unique privileges (3 from user + 1 shared)
assertThat(authorities).hasSize(4).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
"LOGIN_PRIVILEGE",
// Then - 4 unique privileges (3 from user + 1 shared) + the two role names granted as of 4.4.0
assertThat(authorities).hasSize(6).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
"ROLE_TEST1", "ROLE_TEST2", "LOGIN_PRIVILEGE",
"UPDATE_OWN_USER_PRIVILEGE", "RESET_OWN_PASSWORD_PRIVILEGE", "SHARED_PRIVILEGE");
}

Expand Down Expand Up @@ -247,9 +248,11 @@ void getAuthoritiesFromRoles_separatelyLoadedRoles_worksCorrectly() {
Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromRoles(
Arrays.asList(loadedUserRole, loadedManagerRole, loadedAdminRole));

// Then - Should have all unique privileges from all three roles
assertThat(authorities).hasSize(11) // 3 + 3 + 5 unique privileges
// Then - all unique privileges from all three roles, plus the three role names granted as of 4.4.0
assertThat(authorities).hasSize(14) // 3 + 3 + 5 privileges + 3 role names
.extracting(GrantedAuthority::getAuthority).contains(
// Role names
"ROLE_USER", "ROLE_MANAGER", "ROLE_ADMIN",
// User privileges
"LOGIN_PRIVILEGE", "UPDATE_OWN_USER_PRIVILEGE",
"RESET_OWN_PASSWORD_PRIVILEGE",
Expand Down
Loading
Loading