diff --git a/uw-spring-security-core/src/main/java/edu/wisc/uwss/local/LocalUserDetailsAttributesMapper.java b/uw-spring-security-core/src/main/java/edu/wisc/uwss/local/LocalUserDetailsAttributesMapper.java index db33acb8ca0dec3febf7d977069fd6220928c49c..751cde911439b9ef23622a5ead0e4d4fa88b617b 100644 --- a/uw-spring-security-core/src/main/java/edu/wisc/uwss/local/LocalUserDetailsAttributesMapper.java +++ b/uw-spring-security-core/src/main/java/edu/wisc/uwss/local/LocalUserDetailsAttributesMapper.java @@ -13,6 +13,15 @@ import edu.wisc.uwss.UWUserDetailsImpl; /** * Interface providing a mechanism to bind a row from the local users properties * file to a {@link UWUserDetails} instance. + * + * This interface is used during application initialization - not during + * authentication attempts. Since it is executed during Spring ApplicationContext initialization, + * implementations should avoid injecting other service or dao interfaces, as it may be + * affected by a race condition. + * + * If you have custom {@link UWUserDetails} that depend on services/daos to complete the model, + * you may want to consider implementing a {@link LocalUsersAuthenticationAttemptCallback}; that interface + * participates in the authentication attempt itself, not during application initialization. * * @author Nicholas Blair */ diff --git a/uw-spring-security-core/src/main/java/edu/wisc/uwss/local/LocalUserDetailsManagerImpl.java b/uw-spring-security-core/src/main/java/edu/wisc/uwss/local/LocalUserDetailsManagerImpl.java index f657dd6109ce660322e3b751260bd91e57e3aaca..e331dae10c6dbe349f90f5cf604c0aa6ffa49f3d 100644 --- a/uw-spring-security-core/src/main/java/edu/wisc/uwss/local/LocalUserDetailsManagerImpl.java +++ b/uw-spring-security-core/src/main/java/edu/wisc/uwss/local/LocalUserDetailsManagerImpl.java @@ -42,7 +42,7 @@ public class LocalUserDetailsManagerImpl implements UserDetailsManager { private final Map<String, UWUserDetails> users = new ConcurrentHashMap<>(); @Autowired(required=false) - private List<LocalUWUserDetailsCallback> callbacks = new ArrayList<>(); + private List<LocalUsersAuthenticationAttemptCallback> callbacks = new ArrayList<>(); @Autowired(required=false) private LocalUserDetailsAttributesMapper localUserDetailsAttributeMapper = new LocalUserDetailsAttributesMapper.Default(); @@ -78,7 +78,7 @@ public class LocalUserDetailsManagerImpl implements UserDetailsManager { * * @param callbacks */ - void setCallbacks(List<LocalUWUserDetailsCallback> callbacks) { + void setCallbacks(List<LocalUsersAuthenticationAttemptCallback> callbacks) { this.callbacks = callbacks; } @@ -124,7 +124,7 @@ public class LocalUserDetailsManagerImpl implements UserDetailsManager { throw new UsernameNotFoundException(username + " not found"); } UWUserDetails result = SerializationUtils.clone(user); - for(LocalUWUserDetailsCallback callback: callbacks) { + for(LocalUsersAuthenticationAttemptCallback callback: callbacks) { callback.success(result); } return result; diff --git a/uw-spring-security-core/src/main/java/edu/wisc/uwss/local/LocalUWUserDetailsCallback.java b/uw-spring-security-core/src/main/java/edu/wisc/uwss/local/LocalUsersAuthenticationAttemptCallback.java similarity index 61% rename from uw-spring-security-core/src/main/java/edu/wisc/uwss/local/LocalUWUserDetailsCallback.java rename to uw-spring-security-core/src/main/java/edu/wisc/uwss/local/LocalUsersAuthenticationAttemptCallback.java index a12b41f35286e9587dfcc1b0cebb3176747c0a8a..f862109e514a70df950f2ec50cf246bc3486d209 100644 --- a/uw-spring-security-core/src/main/java/edu/wisc/uwss/local/LocalUWUserDetailsCallback.java +++ b/uw-spring-security-core/src/main/java/edu/wisc/uwss/local/LocalUsersAuthenticationAttemptCallback.java @@ -3,13 +3,13 @@ package edu.wisc.uwss.local; import edu.wisc.uwss.UWUserDetails; /** - * Callback interface allowing downstream projects to mutate the result to be returned from - * {@link LocalUserDetailsManagerImpl#loadUserByUsername(String)}. + * Callback interface allowing downstream projects to mutate the {@link UWUserDetails} returned + * by {@link LocalUserDetailsManagerImpl#loadUserByUsername(String)} during an authentication attempt. * * Example usage: * <pre> - class MyCustomLocalUWUserDetailsCallback implements LocalUWUserDetailsCallback<MyUWUserDetailsImpl> { + class MyLocalUsersAuthenticationAttemptCallback implements LocalUsersAuthenticationAttemptCallback<MyUWUserDetailsImpl> { @Autowired private MySomethingDao somethingDao; @@ -23,9 +23,12 @@ import edu.wisc.uwss.UWUserDetails; * Consumers can register any number of Beans implementing this interface; see {@link org.springframework.core.Ordered} * if you need a particular order of execution. * + * If instead you need to modify {@link UWUserDetails} instances during application initialization only, + * see {@link LocalUserDetailsAttributesMapper}. + * * @author Nicholas Blair */ -public interface LocalUWUserDetailsCallback<T extends UWUserDetails> { +public interface LocalUsersAuthenticationAttemptCallback<T extends UWUserDetails> { /** * Callback executed upon successfully loading the {@link UWUserDetails} for a username. diff --git a/uw-spring-security-core/src/main/java/edu/wisc/uwss/preauth/PreauthenticatedUserDetailsAttributeMapper.java b/uw-spring-security-core/src/main/java/edu/wisc/uwss/preauth/PreauthenticatedUserDetailsAttributeMapper.java index 7684fdaf1683b580424367149836fdb83a2d0036..1dce635642ffcb9a533393cce02c841703488bc8 100644 --- a/uw-spring-security-core/src/main/java/edu/wisc/uwss/preauth/PreauthenticatedUserDetailsAttributeMapper.java +++ b/uw-spring-security-core/src/main/java/edu/wisc/uwss/preauth/PreauthenticatedUserDetailsAttributeMapper.java @@ -23,6 +23,9 @@ import edu.wisc.uwss.UWUserDetailsImpl; /** * Interface providing a mechanism to the attributes preauthenticated in the * {@link HttpServletRequest} to a {@link UWUserDetails} instance. + * + * This interface is executed during authentication attempts (similar to + * {@link edu.wisc.uwss.local.LocalUsersAuthenticationAttemptCallback}). * * @author Nicholas Blair */ diff --git a/uw-spring-security-core/src/test/java/edu/wisc/uwss/local/LocalUserDetailsManagerImplTest.java b/uw-spring-security-core/src/test/java/edu/wisc/uwss/local/LocalUserDetailsManagerImplTest.java index c62f96f52cfc58048525a1e8050adc031b541fd6..01c2db45ec6c23ea374aee0eaf2d01b8f05739d0 100644 --- a/uw-spring-security-core/src/test/java/edu/wisc/uwss/local/LocalUserDetailsManagerImplTest.java +++ b/uw-spring-security-core/src/test/java/edu/wisc/uwss/local/LocalUserDetailsManagerImplTest.java @@ -277,11 +277,16 @@ public class LocalUserDetailsManagerImplTest { assertFalse(manager.userExists(username)); } + /** + * Test confirming {@link LocalUsersAuthenticationAttemptCallback} is fired appropriately + * during an authentication attempt ({@link LocalUserDetailsManagerImpl#loadUserByUsername(String)}). + */ @Test public void loadUserByUsername_with_callback() { boolean visited = false; LocalUserDetailsManagerImpl manager = new LocalUserDetailsManagerImpl(); - LocalUWUserDetailsCallback callback = new LocalUWUserDetailsCallback<UWUserDetailsImpl>() { + // setup a simple callback to mutate the userDetails + LocalUsersAuthenticationAttemptCallback callback = new LocalUsersAuthenticationAttemptCallback<UWUserDetailsImpl>() { @Override public void success(UWUserDetailsImpl userDetails) { userDetails.setFirstName("something custom"); @@ -289,6 +294,7 @@ public class LocalUserDetailsManagerImplTest { }; manager.setCallbacks(Arrays.asList(callback)); + // no first name set on original user UWUserDetails user = new UWUserDetailsImpl("UW000A000", "foo", "bar", "Foo Bar", "foo@foo.wisc.edu"); // first name starts null assertNull(user.getFirstName());