Skip to content
Snippets Groups Projects
Commit 54b95e3c authored by Nicholas Blair's avatar Nicholas Blair
Browse files

Merge branch 'userdetailsservice-callback' into 'master'

feature: a callback interface allowing customization of UserDetailsService#loadUserByUsername

This pull request adds a callback interface that allows downstream projects to participate in one of the core Spring Security components of the `local-users` profile: the UserDetailsService.

During an authentication attempt, `UserDetailsService#loadUserByUsername` is used by Spring Security to first check if a User object exists for the username in the credentials. If no User object is found, no further credential check takes place; if a User object is found, other Spring Security components go about comparing the provided credentials in the authentication attempt to that object.

We have a use case in DoIT Number that is driving the need for this. DoIT Number has a custom `UWUserDetails` class that has some additional fields stored behind a DAO. If we didn't have this customization, DoIT Number would need to sub-class `LocalUserDetailsManagerImpl`, then somehow exclude that bean from the UWSpringSecurityConfiguration - not trivially possible.

The existing `LocalUserDetailsAttributesMapper` interface has a lifecycle that's not conducive to this type of request. Implementations of that interface are executed during application startup - and it is possible that the DAO may not be fully constructed at the time it's queried. We need a callback that fires at time of authentication attempt - not startup.
 
With this pull request, DoIT Number will simply have to register a Spring Bean as follows to query that DAO and attach the necessary data to their custom `UWUserDetails` class as part of `UserDetailsService#loadUserByUsername`:

```
@Component
class DNumberLocalUWUserDetailsCallback implements LocalUWUserDetailsCallback<DNumberUserDetailsImpl> {

  @Autowired
  private ControlDao controlDao;

  public void success(DNumberUserDetailsImpl userDetails) {
    userDetails.setControls(controlDao.getControls(userDetails.getUsername()));
  }
}
```

This type of feature is only needed for `local-users` and not for `preauth`. The `PreauthenticatedUserDetailsAttributeMapper` interface has a lifecycle already similar to LocalUWUserDetailsCallback (firing on authentication attempt, not startup). 

Notify @alundholm 

See merge request !12
parents 32ab34dc 1e9fbbdd
No related branches found
No related tags found
1 merge request!12feature: a callback interface allowing customization of UserDetailsService#loadUserByUsername
......@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>edu.wisc.uwss</groupId>
<artifactId>uw-spring-security</artifactId>
<version>1.1.1-SNAPSHOT</version>
<version>1.2.0-SNAPSHOT</version>
<packaging>pom</packaging>
<name>UW Spring Security Parent</name>
<description>Parent project for module to integrate Spring Security with UW authentication mechanism.</description>
......
......@@ -3,7 +3,7 @@
<parent>
<groupId>edu.wisc.uwss</groupId>
<artifactId>uw-spring-security</artifactId>
<version>1.1.1-SNAPSHOT</version>
<version>1.2.0-SNAPSHOT</version>
</parent>
<artifactId>uw-spring-security-config</artifactId>
<name>UW Spring Security Configuration</name>
......
......@@ -3,7 +3,7 @@
<parent>
<groupId>edu.wisc.uwss</groupId>
<artifactId>uw-spring-security</artifactId>
<version>1.1.1-SNAPSHOT</version>
<version>1.2.0-SNAPSHOT</version>
</parent>
<artifactId>uw-spring-security-core</artifactId>
<name>UW Spring Security Core</name>
......
......@@ -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
*/
......
......@@ -3,6 +3,8 @@
*/
package edu.wisc.uwss.local;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Properties;
......@@ -39,6 +41,8 @@ public class LocalUserDetailsManagerImpl implements UserDetailsManager {
private Logger logger = LoggerFactory.getLogger(this.getClass());
private final Map<String, UWUserDetails> users = new ConcurrentHashMap<>();
@Autowired(required=false)
private List<LocalUsersAuthenticationAttemptCallback> callbacks = new ArrayList<>();
@Autowired(required=false)
private LocalUserDetailsAttributesMapper localUserDetailsAttributeMapper = new LocalUserDetailsAttributesMapper.Default();
......@@ -68,6 +72,16 @@ public class LocalUserDetailsManagerImpl implements UserDetailsManager {
public void setDemoUsers(@Qualifier("demo-users") Properties properties) {
this.properties = properties;
}
/**
* Visible for testing.
*
* @param callbacks
*/
void setCallbacks(List<LocalUsersAuthenticationAttemptCallback> callbacks) {
this.callbacks = callbacks;
}
/**
* Add a user. Does nothing if the argument is null or the {@link UWUserDetailsImpl#getUsername()} is blank.
*
......@@ -109,7 +123,11 @@ public class LocalUserDetailsManagerImpl implements UserDetailsManager {
if(user == null) {
throw new UsernameNotFoundException(username + " not found");
}
return SerializationUtils.clone(user);
UWUserDetails result = SerializationUtils.clone(user);
for(LocalUsersAuthenticationAttemptCallback callback: callbacks) {
callback.success(result);
}
return result;
}
/**
* @param user
......
package edu.wisc.uwss.local;
import edu.wisc.uwss.UWUserDetails;
/**
* Callback interface allowing downstream projects to mutate the {@link UWUserDetails} returned
* by {@link LocalUserDetailsManagerImpl#loadUserByUsername(String)} during an authentication attempt.
*
* Example usage:
*
<pre>
class MyLocalUsersAuthenticationAttemptCallback implements LocalUsersAuthenticationAttemptCallback<MyUWUserDetailsImpl> {
@Autowired
private MySomethingDao somethingDao;
public void success(MyUWUserDetailsImpl userDetails) {
userDetails.setMySomethingField(somethingDao.getSomethingsForUser(userDetails.getUsername());
}
}
</pre>
*
* 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 LocalUsersAuthenticationAttemptCallback<T extends UWUserDetails> {
/**
* Callback executed upon successfully loading the {@link UWUserDetails} for a username.
*
* @param userDetails the userDetails to modify
*/
void success(T userDetails);
}
......@@ -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
*/
......
......@@ -10,6 +10,7 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
import java.util.Properties;
......@@ -30,7 +31,7 @@ import edu.wisc.uwss.UWUserDetailsImpl;
public class LocalUserDetailsManagerImplTest {
/**
* Verify {@link LocalUserDetailsServiceImpl#loadUserByUsername(String)} throws
* Verify {@link LocalUserDetailsManagerImpl#loadUserByUsername(String)} throws
* {@link UsernameNotFoundException} for an unknown user.
*/
@Test(expected=UsernameNotFoundException.class)
......@@ -275,5 +276,32 @@ public class LocalUserDetailsManagerImplTest {
manager.deleteUser(username);
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() {
LocalUserDetailsManagerImpl manager = new LocalUserDetailsManagerImpl();
// setup a simple callback to mutate the userDetails
LocalUsersAuthenticationAttemptCallback callback = new LocalUsersAuthenticationAttemptCallback<UWUserDetailsImpl>() {
@Override
public void success(UWUserDetailsImpl userDetails) {
userDetails.setFirstName("something custom");
}
};
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());
manager.addDemoUser(user);
UWUserDetails result = manager.loadUserByUsername("foo");
// observe firstname modified by the callback
assertEquals("something custom", result.getFirstName());
}
}
\ No newline at end of file
......@@ -3,7 +3,7 @@
<parent>
<groupId>edu.wisc.uwss</groupId>
<artifactId>uw-spring-security</artifactId>
<version>1.1.1-SNAPSHOT</version>
<version>1.2.0-SNAPSHOT</version>
</parent>
<artifactId>uw-spring-security-sample-war</artifactId>
<name>UW Spring Security Sample War</name>
......
......@@ -3,7 +3,7 @@
<parent>
<groupId>edu.wisc.uwss</groupId>
<artifactId>uw-spring-security</artifactId>
<version>1.1.1-SNAPSHOT</version>
<version>1.2.0-SNAPSHOT</version>
</parent>
<artifactId>uw-spring-security-web</artifactId>
<name>UW Spring Security Web</name>
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment