-
Notifications
You must be signed in to change notification settings - Fork 6.2k
Description
Describe the bug
Up to 6.5.7, you were able to create (in Kotlin) a class that implements AuthenticationManagerResolver<String> and handle potential multi-tenancy as below (below is a simplified version, but you get the gist):
@Component
class AuthManagerResolver(
@Value($$"${security.issuer1}") val issuer1: String,
@Value($$"${security.issuer2}") val issuer2: String,
) : AuthenticationManagerResolver<String> {
private val authenticationManagerCreator = { issuer: String ->
when (issuer) {
issuer1 ->
AuthenticationManager { authentication: Authentication ->
val provider =
JwtAuthenticationProvider(JwtDecoders.fromIssuerLocation(issuer))
provider.authenticate(authentication)
}
issuer2 ->
AuthenticationManager { authentication: Authentication ->
val provider =
JwtAuthenticationProvider(JwtDecoders.fromIssuerLocation(issuer))
provider.authenticate(authentication)
}
else -> throw IllegalArgumentException("Unsupported issuer: $issuer")
}
}
private val authenticationManagers: Map<String, AuthenticationManager> =
mapOf(
issuer1 to authenticationManagerCreator(issuer1),
issuer2 to authenticationManagerCreator(issuer2),
)
override fun resolve(issuer: String): AuthenticationManager? = authenticationManagers[issuer]
}with v7.0.0, there seems to be some nullability removed:
- in
resolve-> this makes sense, mostly, you would not want to return anullAuthenticationManager, although I did not see an issue with that initially, as a nullauthManagerwould fail authenticating anyway - in
AuthenticationManagerinterface -> not sure I can find where the nullability was removed, but with 7.0.0, the code above requiresJwtAuthenticationProvider.authenticateto not return nullableauthentication, which at the moment it does not.
Was it intentional to change the nullability in these classes? If so, do you suggest that in Kotlin we handle nullability by ourselves in these cases?
To Reproduce
- use Kotlin, Spring 4, Spring Security 7
- create a class as the example above
Expected behavior
For resolve, as above, I am quite happy to handle the non-presence of the issuer in the map.
For .authenticate, given the way it is structured, there should not be any need for it to return a nullable Authentication, given how it asserts that the token is not null, as per below:
@Override
public Authentication authenticate(Authentication authentication) throws AuthenticationException {
BearerTokenAuthenticationToken bearer = (BearerTokenAuthenticationToken) authentication;
Jwt jwt = getJwt(bearer);
AbstractAuthenticationToken token = this.jwtAuthenticationConverter.convert(jwt);
// This was added as of v 7.0.0
Assert.notNull(token, "token cannot be null");
if (token.getDetails() == null) {
token.setDetails(bearer.getDetails());
}
this.logger.debug("Authenticated token");
return token;
}
Sample
Will give one asap, but should be pretty straight forward 😄
Edit: https://github.com/alfonsoristorato/multi-tenancy-rep