stormpath-sdk-java
stormpath-sdk-java copied to clipboard
Authentication in Spring Security does not support specifying an `accountStore`
StormpathAuthenticationProvider#createAuthenticationRequest is missing inAccountStore(AccountStore)
protected AuthenticationRequest createAuthenticationRequest(Authentication authentication) {
String username = (String) authentication.getPrincipal();
String password = (String) authentication.getCredentials();
return UsernamePasswordRequest.builder().setUsernameOrEmail(username).setPassword(password).build();
}
@mrioan which milestone should this be added to? RC9?
@lhazlewood it already has the milestone 1.0.RC9. There was an user impacted by it so we decided to add it as part of this release. I think it is already too late to postpone it as Matt is already working on it. But if you feel otherwise we can still do so...
Copying here @lhazlewood 's comments from #541
When the AccountStoreResolver interface was created, it was to provide a way for the application developer to inspect the request (in whatever way they wanted) to determine which AccountStore (if any) should be targeted during an authentication attempt. It is not a requirement to target a specific account store, but definitely recommended in multi-tenant applications to avoid Stormpath's account store iteration logic in the API server, which could be very slow for many (dozens, hundreds) of account stores mapped to an application.
The only practical default implementations that we can provide - that I can think of at least - is a DisabledAccountStoreResolver which never assumes a targeted account store (and is our default configured instance), and a yet-to-be-implemented TenantAccountStoreResolver implementation that uses the OrganizationNameKeyResolver to find out which organization/tenant is currently in scope for a given request. Once that is discovered, that can be used to return the Organization instance (which implements AccountStore), satisfying the AccountStoreResolver contract.
App developers are free of course to provide their own implementation of AccountStoreResolver if either of our implementations are not sufficient.
Some code has already been implemented for this issue. It is available in the branch called spring-security-accountstore-rebased
What's missing:
- Add an IT in
spring-security-webmvc
module (not the example) to test that the authentication object has the account store or something similar that validates that this fix is working. Tests are already available inMinimalStormpathSpringSecurityWebMvcConfigurationIT
. - I did not thoroughly test everything. We should check that login and logout works ok in Spring and Spring Boot with and without Spring Security
- My code is not overriding
UsernamePasswordAuthenticationFilter
, I simply put our newStormpathSpringSecurityUsernamePasswordAuthenticationFilter
before that filter in the chain (so we end up having 2UsernamePasswordAuthenticationFilter
). I am sure this is not the best approach, we should override it somehow - Add javadoc
@mrioan I asked Rob Winch (Spring Security Lead) about the StormpathSpringSecurityUsernamePasswordAuthenticationFilter
and it seems your approach is OK. See https://github.com/spring-projects/spring-security/pull/241#issuecomment-205604994