libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases 1 Packages Activity Actions

[KEYCLOAK-9539] Race condition SecurityContextHolder.setAuthentication()

Browse source 
This is an issue with the Spring Security Keycloak Adapter relating to
 the way the Authentication is stored in the SecurityContext, causing a
 race condition in application code using that. It does not seem to
 affect actual Spring Security operation.

We had a pretty strange race condition in our application. When many
 requests were incoming at the same time, occasionally the old
 unauthenticated Authentication provided to
 KeycloakAuthenticationProvider for performing the actual authentication
 would stay the current authentication, as returned by
 SecurityContextHolder.getContext().getAuthentication(). That resulted
 in authenticated users' JavaScript requests occasionally (~1/50 given a
 large request volume) returning a 403 because the 'old' token was still
 in the context, causing Spring Security to see them as unauthenticated.

This PR resolves this issue by replacing the whole context, as suggested
 by a Spring Security contributor in jzheaux/spring-security-oauth2-resource-server#48. By default,
 SecurityContextHolder keeps the actual context object in a ThreadLocal,
 which should be safe from race-conditions. The actual Authentication
 object, however, is kept in a mere field, hence the reason for this PR.

JIRA issue: https://issues.jboss.org/browse/KEYCLOAK-9539
This commit is contained in:
Philipp Nowak 2019-02-11 19:02:18 +01:00 committed by Pedro Igor
parent 9c34cc7365
commit 39828b2c94
3 changed files with 16 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
adapters/oidc/spring-security/src/main/java/org/keycloak/adapters/springsecurity/authentication/SpringSecurityRequestAuthenticator.java
View file

@ -32,6 +32,7 @@ import org.keycloak.adapters.springsecurity.account.SimpleKeycloakAccount;
import org.keycloak.adapters.springsecurity.token.KeycloakAuthenticationToken;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder;
import javax.servlet.http.HttpServletRequest;
@ -94,7 +95,10 @@ public class SpringSecurityRequestAuthenticator extends RequestAuthenticator {
logger.debug("Completing bearer authentication. Bearer roles: {} ",roles);
SecurityContextHolder.getContext().setAuthentication(new KeycloakAuthenticationToken(account, false));
SecurityContext context = SecurityContextHolder.createEmptyContext();
context.setAuthentication(new KeycloakAuthenticationToken(account, false));
SecurityContextHolder.setContext(context);
request.setAttribute(KeycloakSecurityContext.class.getName(), securityContext);
}

14
adapters/oidc/spring-security/src/main/java/org/keycloak/adapters/springsecurity/filter/KeycloakAuthenticationProcessingFilter.java
View file

@ -49,6 +49,7 @@ import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.authentication.event.InteractiveAuthenticationSuccessEvent;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter;
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
@ -197,14 +198,15 @@ public class KeycloakAuthenticationProcessingFilter extends AbstractAuthenticati
log.debug("Authentication success using bearer token/basic authentication. Updating SecurityContextHolder to contain: {}", authResult);
}
SecurityContextHolder.getContext().setAuthentication(authResult);
// Fire event
if (this.eventPublisher != null) {
eventPublisher.publishEvent(new InteractiveAuthenticationSuccessEvent(authResult, this.getClass()));
}
SecurityContext context = SecurityContextHolder.createEmptyContext();
context.setAuthentication(authResult);
SecurityContextHolder.setContext(context);
try {
// Fire event
if (this.eventPublisher != null) {
eventPublisher.publishEvent(new InteractiveAuthenticationSuccessEvent(authResult, this.getClass()));
}
chain.doFilter(request, response);
} finally {
SecurityContextHolder.clearContext();

4
adapters/oidc/spring-security/src/main/java/org/keycloak/adapters/springsecurity/token/SpringSecurityTokenStore.java
View file

@ -105,7 +105,9 @@ public class SpringSecurityTokenStore implements AdapterTokenStore {
}
logger.debug("Saving account info {}", account);
SecurityContextHolder.getContext().setAuthentication(new KeycloakAuthenticationToken(account, true));
SecurityContext context = SecurityContextHolder.createEmptyContext();
context.setAuthentication(new KeycloakAuthenticationToken(account, true));
SecurityContextHolder.setContext(context);
}
@Override