[KEYCLOAK-13187] - Concurrency issue when refreshing tokens and updating security context state

This commit is contained in:
Pedro Igor 2020-03-09 10:54:19 -03:00 committed by Stian Thorgersen
parent 020ffd37ee
commit acfbdf6b0e
8 changed files with 346 additions and 43 deletions

View file

@ -114,51 +114,63 @@ public class RefreshableKeycloakSecurityContext extends KeycloakSecurityContext
if (log.isTraceEnabled()) {
log.trace("Doing refresh");
}
AccessTokenResponse response = null;
try {
response = ServerRequest.invokeRefresh(deployment, refreshToken);
} catch (IOException e) {
log.error("Refresh token failure", e);
return false;
} catch (ServerRequest.HttpFailure httpFailure) {
log.error("Refresh token failure status: " + httpFailure.getStatus() + " " + httpFailure.getError());
return false;
}
if (log.isTraceEnabled()) {
log.trace("received refresh response");
}
String tokenString = response.getToken();
AccessToken token = null;
try {
AdapterTokenVerifier.VerifiedTokens tokens = AdapterTokenVerifier.verifyTokens(tokenString, response.getIdToken(), deployment);
token = tokens.getAccessToken();
log.debug("Token Verification succeeded!");
} catch (VerificationException e) {
log.error("failed verification of token");
return false;
}
// If the TTL is greater-or-equal to the expire time on the refreshed token, have to abort or go into an infinite refresh loop
if (!isTokenTimeToLiveSufficient(token)) {
log.error("failed to refresh the token with a longer time-to-live than the minimum");
return false;
}
if (response.getNotBeforePolicy() > deployment.getNotBefore()) {
deployment.updateNotBefore(response.getNotBeforePolicy());
}
this.token = token;
if (response.getRefreshToken() != null) {
if (log.isTraceEnabled()) {
log.trace("Setup new refresh token to the security context");
// block requests if the refresh token herein stored is already being used to refresh the token so that subsequent requests
// can use the last refresh token issued by the server. Note that this will only work for deployments using the session store
// and, when running in a cluster, sticky sessions must be used.
//
synchronized (this) {
if (checkActive) {
log.trace("Checking whether token has been refreshed in another thread already.");
if (isActive() && isTokenTimeToLiveSufficient(this.token)) return true;
}
AccessTokenResponse response;
try {
response = ServerRequest.invokeRefresh(deployment, refreshToken);
} catch (IOException e) {
log.error("Refresh token failure", e);
return false;
} catch (ServerRequest.HttpFailure httpFailure) {
log.error("Refresh token failure status: " + httpFailure.getStatus() + " " + httpFailure.getError());
return false;
}
if (log.isTraceEnabled()) {
log.trace("received refresh response");
}
String tokenString = response.getToken();
AccessToken token = null;
try {
AdapterTokenVerifier.VerifiedTokens tokens = AdapterTokenVerifier.verifyTokens(tokenString, response.getIdToken(), deployment);
token = tokens.getAccessToken();
log.debug("Token Verification succeeded!");
} catch (VerificationException e) {
log.error("failed verification of token");
return false;
}
// If the TTL is greater-or-equal to the expire time on the refreshed token, have to abort or go into an infinite refresh loop
if (!isTokenTimeToLiveSufficient(token)) {
log.error("failed to refresh the token with a longer time-to-live than the minimum");
return false;
}
if (response.getNotBeforePolicy() > deployment.getNotBefore()) {
deployment.updateNotBefore(response.getNotBeforePolicy());
}
this.token = token;
if (response.getRefreshToken() != null) {
if (log.isTraceEnabled()) {
log.trace("Setup new refresh token to the security context");
}
this.refreshToken = response.getRefreshToken();
}
this.tokenString = tokenString;
if (tokenStore != null) {
tokenStore.refreshCallback(this);
}
this.refreshToken = response.getRefreshToken();
}
this.tokenString = tokenString;
if (tokenStore != null) {
tokenStore.refreshCallback(this);
}
return true;
}

View file

@ -0,0 +1,40 @@
/*
* Copyright 2016 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.keycloak.testsuite.adapter.page;
import org.jboss.arquillian.container.test.api.OperateOnDeployment;
import org.jboss.arquillian.test.api.ArquillianResource;
import java.net.URL;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class TokenRefreshPage extends AbstractShowTokensPage {
public static final String DEPLOYMENT_NAME = "token-refresh";
@ArquillianResource
@OperateOnDeployment(DEPLOYMENT_NAME)
private URL url;
@Override
public URL getInjectedUrl() {
return url;
}
}

View file

@ -17,7 +17,15 @@
package org.keycloak.testsuite.adapter.servlet;
import org.apache.commons.io.FileUtils;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.client.utils.URLEncodedUtils;
import org.apache.http.impl.client.BasicCookieStore;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.impl.cookie.BasicClientCookie;
import org.apache.http.util.EntityUtils;
import org.jboss.arquillian.container.test.api.Deployment;
import org.jboss.arquillian.drone.api.annotation.Drone;
import org.jboss.arquillian.graphene.page.Page;
@ -30,6 +38,7 @@ import org.junit.Test;
import org.keycloak.OAuth2Constants;
import org.keycloak.adapters.OIDCAuthenticationError;
import org.keycloak.admin.client.resource.ClientResource;
import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.common.util.Time;
import org.keycloak.constants.AdapterConstants;
import org.keycloak.events.Details;
@ -64,6 +73,7 @@ import org.keycloak.testsuite.adapter.page.SecurePortal;
import org.keycloak.testsuite.adapter.page.SecurePortalRewriteRedirectUri;
import org.keycloak.testsuite.adapter.page.SecurePortalWithCustomSessionConfig;
import org.keycloak.testsuite.adapter.page.TokenMinTTLPage;
import org.keycloak.testsuite.adapter.page.TokenRefreshPage;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.arquillian.annotation.AppServerContainer;
import org.keycloak.testsuite.utils.arquillian.ContainerConstants;
@ -96,10 +106,14 @@ import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@ -171,6 +185,8 @@ public class DemoServletsAdapterTest extends AbstractServletsAdapterTest {
@Page
private TokenMinTTLPage tokenMinTTLPage;
@Page
private TokenRefreshPage tokenRefreshPage;
@Page
private OAuthGrant oAuthGrantPage;
@Page
protected Applications applicationsPage;
@ -262,6 +278,11 @@ public class DemoServletsAdapterTest extends AbstractServletsAdapterTest {
return servletDeployment(TokenMinTTLPage.DEPLOYMENT_NAME, AdapterActionsFilter.class, AbstractShowTokensServlet.class, TokenMinTTLServlet.class, ErrorServlet.class);
}
@Deployment(name = TokenRefreshPage.DEPLOYMENT_NAME)
protected static WebArchive tokenRefresh() {
return servletDeployment(TokenRefreshPage.DEPLOYMENT_NAME, AdapterActionsFilter.class, AbstractShowTokensServlet.class, TokenMinTTLServlet.class, ErrorServlet.class);
}
@Deployment(name = BasicAuth.DEPLOYMENT_NAME)
protected static WebArchive basicAuth() {
return servletDeployment(BasicAuth.DEPLOYMENT_NAME, BasicAuthServlet.class);
@ -755,6 +776,58 @@ public class DemoServletsAdapterTest extends AbstractServletsAdapterTest {
setAdapterAndServerTimeOffset(0, tokenMinTTLPage.toString());
}
@Test
public void testTokenConcurrentRefresh() {
RealmResource demoRealm = adminClient.realm("demo");
RealmRepresentation demo = demoRealm.toRepresentation();
demo.setAccessTokenLifespan(2);
demo.setRevokeRefreshToken(true);
demo.setRefreshTokenMaxReuse(0);
demoRealm.update(demo);
// Login
tokenRefreshPage.navigateTo();
assertTrue(testRealmLoginPage.form().isUsernamePresent());
assertCurrentUrlStartsWithLoginUrlOf(testRealmPage);
testRealmLoginPage.form().login("bburke@redhat.com", "password");
assertCurrentUrlEquals(tokenRefreshPage);
// Revert times
setAdapterAndServerTimeOffset(5, tokenRefreshPage.toString());
BasicCookieStore cookieStore = new BasicCookieStore();
BasicClientCookie jsessionid = new BasicClientCookie("JSESSIONID", driver.manage().getCookieNamed("JSESSIONID").getValue());
jsessionid.setDomain("localhost");
jsessionid.setPath("/");
cookieStore.addCookie(jsessionid);
ExecutorService executor = Executors.newWorkStealingPool();
CompletableFuture future = CompletableFuture.completedFuture(null);
try {
for (int i = 0; i < 5; i++) {
future = CompletableFuture.allOf(future, CompletableFuture.runAsync(() -> {
try (CloseableHttpClient client = HttpClientBuilder.create().setDefaultCookieStore(cookieStore)
.build()) {
HttpUriRequest request = new HttpGet(tokenRefreshPage.getInjectedUrl().toString());
try (CloseableHttpResponse httpResponse = client.execute(request)) {
assertTrue("Token not refreshed", EntityUtils.toString(httpResponse.getEntity()).contains("accessToken"));
}
} catch (Exception e) {
throw new RuntimeException(e);
}
}, executor));
}
future.join();
} finally {
executor.shutdownNow();
}
}
// Tests forwarding of parameters like "prompt"
@Test
public void testOIDCParamsForwarding() {

View file

@ -323,6 +323,16 @@
],
"secret": "password"
},
{
"clientId": "token-refresh",
"enabled": true,
"adminUrl": "/token-refresh",
"baseUrl": "/token-refresh",
"redirectUris": [
"/token-refresh/*"
],
"secret": "password"
},
{
"clientId": "third-party",
"enabled": true,

View file

@ -0,0 +1,20 @@
<!--
~ Copyright 2016 Red Hat, Inc. and/or its affiliates
~ and other contributors as indicated by the @author tags.
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<Context path="/token-refresh">
<Valve className="org.keycloak.adapters.tomcat.KeycloakAuthenticatorValve"/>
</Context>

View file

@ -0,0 +1,46 @@
<?xml version="1.0"?>
<!--
~ Copyright 2016 Red Hat, Inc. and/or its affiliates
~ and other contributors as indicated by the @author tags.
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<!DOCTYPE Configure PUBLIC "-//Mort Bay Consulting//DTD Configure//EN" "http://www.eclipse.org/jetty/configure_9_0.dtd">
<Configure class="org.eclipse.jetty.webapp.WebAppContext">
<Get name="securityHandler">
<Set name="authenticator">
<New class="org.keycloak.adapters.jetty.KeycloakJettyAuthenticator">
<!--
<Set name="adapterConfig">
<New class="org.keycloak.representations.adapters.config.AdapterConfig">
<Set name="realm">tomcat</Set>
<Set name="resource">customer-portal</Set>
<Set name="authServerUrl">http://localhost:8180/auth</Set>
<Set name="sslRequired">external</Set>
<Set name="credentials">
<Map>
<Entry>
<Item>secret</Item>
<Item>password</Item>
</Entry>
</Map>
</Set>
<Set name="realmKey">MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCrVrCuTtArbgaZzL1hvh0xtL5mc7o0NqPVnYXkLvgcwiC3BjLGw1tGEGoJaXDuSaRllobm53JBhjx33UNv+5z/UMG4kytBWxheNVKnL6GgqlNabMaFfPLPCF8kAgKnsi79NMo+n6KnSY8YeUmec/p2vjO2NjsSAVcWEQMVhJ31LwIDAQAB</Set>
</New>
</Set>
-->
</New>
</Set>
</Get>
</Configure>

View file

@ -0,0 +1,9 @@
{
"realm": "demo",
"resource": "token-refresh",
"auth-server-url": "http://localhost:8180/auth",
"ssl-required" : "external",
"credentials": {
"secret": "password"
}
}

View file

@ -0,0 +1,93 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Copyright 2016 Red Hat, Inc. and/or its affiliates
~ and other contributors as indicated by the @author tags.
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->
<web-app xmlns="http://java.sun.com/xml/ns/javaee"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://java.sun.com/xml/ns/javaee http://java.sun.com/xml/ns/javaee/web-app_3_0.xsd"
version="3.0">
<module-name>token-refresh</module-name>
<filter>
<filter-name>AdapterActionsFilter</filter-name>
<filter-class>org.keycloak.testsuite.adapter.filter.AdapterActionsFilter</filter-class>
</filter>
<servlet>
<servlet-name>Servlet</servlet-name>
<servlet-class>org.keycloak.testsuite.adapter.servlet.TokenMinTTLServlet</servlet-class>
</servlet>
<servlet>
<servlet-name>Error Servlet</servlet-name>
<servlet-class>org.keycloak.testsuite.adapter.servlet.ErrorServlet</servlet-class>
</servlet>
<filter-mapping>
<filter-name>AdapterActionsFilter</filter-name>
<url-pattern>/*</url-pattern>
</filter-mapping>
<servlet-mapping>
<servlet-name>Servlet</servlet-name>
<url-pattern>/*</url-pattern>
</servlet-mapping>
<servlet-mapping>
<servlet-name>Error Servlet</servlet-name>
<url-pattern>/error.html</url-pattern>
</servlet-mapping>
<security-constraint>
<web-resource-collection>
<web-resource-name>Users</web-resource-name>
<url-pattern>/*</url-pattern>
</web-resource-collection>
<auth-constraint>
<role-name>user</role-name>
</auth-constraint>
</security-constraint>
<security-constraint>
<web-resource-collection>
<web-resource-name>Errors</web-resource-name>
<url-pattern>/error.html</url-pattern>
</web-resource-collection>
</security-constraint>
<security-constraint>
<web-resource-collection>
<web-resource-name>Unsecured</web-resource-name>
<url-pattern>/unsecured/*</url-pattern>
</web-resource-collection>
</security-constraint>
<login-config>
<auth-method>KEYCLOAK</auth-method>
<realm-name>test</realm-name>
<form-login-config>
<form-login-page>/error.html</form-login-page>
<form-error-page>/error.html</form-error-page>
</form-login-config>
</login-config>
<security-role>
<role-name>admin</role-name>
</security-role>
<security-role>
<role-name>user</role-name>
</security-role>
</web-app>