mirrors/keycloak
1
0
Fork 0
mirror of https://github.com/keycloak/keycloak.git synced 2026-01-25 16:42:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

Enable all test methods in ConcurrentLoginTest for JPA Map Storage

Browse Source 
- Tests still disabled for Hotrod and CHM
- Fixes concurrent login issues with CRDB. Verified with both PostgreSQL and CockroachDB.

Closes #12707
Closes #13210
This commit is contained in:
Stefan Guilhen
2022-10-28 16:19:02 -03:00
committed by Hynek Mlnařík
parent 4a91c07488
commit 5c2a5fac31
5 changed files with 185 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java
View File

@@ -39,6 +39,7 @@ import org.keycloak.models.UserModel;
import org.keycloak.models.UserSessionModel;
import org.keycloak.models.utils.AuthenticationFlowResolver;
import org.keycloak.models.utils.FormMessage;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.protocol.LoginProtocol;
import org.keycloak.protocol.LoginProtocol.Error;
import org.keycloak.protocol.oidc.TokenManager;
@@ -822,6 +823,13 @@ public class AuthenticationProcessor {
return ErrorPage.error(session, authenticationSession, Response.Status.BAD_REQUEST, Messages.INVALID_USER);
}
} else if (KeycloakModelUtils.isExceptionRetriable(failure)) {
// let calling code decide if whole action should be retried.
if (failure instanceof RuntimeException) {
throw (RuntimeException) failure;
} else {
throw new RuntimeException(failure);
}
} else {
ServicesLogger.LOGGER.failedAuthentication(failure);
event.error(Errors.INVALID_USER_CREDENTIALS);

125
services/src/main/java/org/keycloak/common/util/ResponseSessionTask.java Normal file
View File

@@ -0,0 +1,125 @@
/*
* Copyright 2022 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.common.util;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.Response;
import org.keycloak.models.ClientModel;
import org.keycloak.models.KeycloakContext;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionTaskWithResult;
import org.keycloak.models.RealmModel;
import org.keycloak.sessions.RootAuthenticationSessionModel;
/**
* A {@link KeycloakSessionTaskWithResult} that is aimed to be used by endpoints that want to produce a {@link Response} in
* a retriable transaction. It takes care of the boilerplate code that is common to be seen in this scenario, allowing the
* endpoint to focus on the actual code that has to be executed in a retriable manner.
* </p>
* More specifically, this task:
* <li>
* <ul>pushes the task's session into the resteasy context, restoring the original value after the task is over. This allows
* for endpoints to create new instances of themselves and inject the resteasy properties correctly;</ul>
* <ul>sets up the task's session context, based on model values found in the original session's context;</ul>
* <ul>handles {@link WebApplicationException} when it is thrown by the task.</ul>
* </li>
*
* @author <a href="mailto:sguilhen@redhat.com">Stefan Guilhen</a>
*/
public abstract class ResponseSessionTask implements KeycloakSessionTaskWithResult<Response> {
private final KeycloakSession originalSession;
/**
* Constructs a new instance of this task.
*
* @param originalSession the original {@link KeycloakSession} that was active when the task was created.
*/
public ResponseSessionTask(final KeycloakSession originalSession) {
this.originalSession = originalSession;
}
@Override
public Response run(final KeycloakSession session) {
// save the session that was originally in the resteasy context, so it can be restored once the task finishes.
KeycloakSession originalContextSession = Resteasy.getContextData(KeycloakSession.class);
try {
// set up the current session context based on the original session context.
this.setupSessionContext(session);
// push the current session into the resteasy context.
Resteasy.pushContext(KeycloakSession.class, session);
// run the actual task.
return runInternal(session);
} catch (WebApplicationException we) {
// If the exception is capable of producing a complete response, including a message entity, we return the response
// here so that the overall transaction is still committed. If the message entity is missing, we throw the exception
// so that KeycloakError handler is invoked later on to produce a valid response and the transaction is rolled back.
//
// Another reason to convert the web application exception into a response here is because some exception subtypes use
// the Keycloak session to construct the response. As a result, the conversion has to happen before the session is closed.
Response response = we.getResponse();
if (response.getEntity() != null) {
return response;
}
throw we;
} finally {
// restore original session in resteasy context.
Resteasy.pushContext(KeycloakSession.class, originalContextSession);
}
}
/**
* Sets up the context for the specified session. The original realm's context is used to determine what models
* need to be re-loaded using the current session.
*
* @param session the session whose context is to be prepared.
*/
private void setupSessionContext(final KeycloakSession session) {
if (this.originalSession == null) return;
KeycloakContext context = this.originalSession.getContext();
// setup realm model if necessary.
RealmModel realmModel = null;
if (context.getRealm() != null) {
realmModel = session.realms().getRealm(context.getRealm().getId());
session.getContext().setRealm(realmModel);
}
// setup client model if necessary.
ClientModel clientModel = null;
if (context.getClient() != null) {
clientModel = session.clients().getClientById(realmModel, context.getClient().getId());
session.getContext().setClient(clientModel);
}
// setup auth session model if necessary.
if (context.getAuthenticationSession() != null) {
RootAuthenticationSessionModel rootAuthSession = session.authenticationSessions().getRootAuthenticationSession(realmModel,
context.getAuthenticationSession().getParentSession().getId());
if (rootAuthSession != null) {
session.getContext().setAuthenticationSession(rootAuthSession.getAuthenticationSession(clientModel,
context.getAuthenticationSession().getTabId()));
}
}
}
/**
* Builds the response that is to be returned.
*
* @param session a reference the {@link KeycloakSession}.
* @return the constructed {@link Response}.
*/
public abstract Response runInternal(final KeycloakSession session);
}

26
services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java
View File

@@ -20,6 +20,7 @@ package org.keycloak.protocol.oidc.endpoints;
import org.jboss.logging.Logger;
import org.keycloak.OAuth2Constants;
import org.keycloak.authentication.AuthenticationProcessor;
import org.keycloak.common.util.ResponseSessionTask;
import org.keycloak.constants.AdapterConstants;
import org.keycloak.events.Details;
import org.keycloak.events.Errors;
@@ -30,6 +31,7 @@ import org.keycloak.models.AuthenticationFlowModel;
import org.keycloak.models.ClientModel;
import org.keycloak.models.Constants;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.protocol.AuthorizationEndpointBase;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.protocol.oidc.endpoints.request.AuthorizationEndpointRequest;
@@ -96,17 +98,22 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
event.event(EventType.LOGIN);
}
private AuthorizationEndpoint(final KeycloakSession session, final EventBuilder event, final Action action) {
this(session, event);
this.action = action;
}
@POST
@Consumes(MediaType.APPLICATION_FORM_URLENCODED)
public Response buildPost() {
logger.trace("Processing @POST request");
return process(httpRequest.getDecodedFormParameters());
return processInRetriableTransaction(httpRequest.getDecodedFormParameters());
}
@GET
public Response buildGet() {
logger.trace("Processing @GET request");
return process(session.getContext().getUri().getQueryParameters());
return processInRetriableTransaction(session.getContext().getUri().getQueryParameters());
}
/**
@@ -117,6 +124,21 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
return new DeviceEndpoint(session, event);
}
/**
* Process the request in a retriable transaction.
*/
private Response processInRetriableTransaction(final MultivaluedMap<String, String> formParameters) {
return KeycloakModelUtils.runJobInRetriableTransaction(session.getKeycloakSessionFactory(), new ResponseSessionTask(session) {
@Override
public Response runInternal(KeycloakSession session) {
// create another instance of the endpoint to isolate each run.
AuthorizationEndpoint other = new AuthorizationEndpoint(session,
new EventBuilder(session.getContext().getRealm(), session, clientConnection), action);
// process the request in the created instance.
return other.process(formParameters); }
}, 10, 100);
}
private Response process(MultivaluedMap<String, String> params) {
String clientId = AuthorizationEndpointRequestParserProcessor.getClientId(event, session, params);

27
services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java
View File

@@ -30,7 +30,7 @@ import org.keycloak.common.ClientConnection;
import org.keycloak.common.Profile;
import org.keycloak.common.constants.ServiceAccountConstants;
import org.keycloak.common.util.KeycloakUriBuilder;
import org.keycloak.common.util.Resteasy;
import org.keycloak.common.util.ResponseSessionTask;
import org.keycloak.constants.AdapterConstants;
import org.keycloak.events.Details;
import org.keycloak.events.Errors;
@@ -106,7 +106,6 @@ import javax.ws.rs.InternalServerErrorException;
import javax.ws.rs.OPTIONS;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.MultivaluedHashMap;
@@ -175,26 +174,16 @@ public class TokenEndpoint {
public Response processGrantRequest() {
// grant request needs to be run in a retriable transaction as concurrent execution of this action can lead to
// exceptions on DBs with SERIALIZABLE isolation level.
Object result = KeycloakModelUtils.runJobInRetriableTransaction(this.session.getKeycloakSessionFactory(), kcSession -> {
try {
RealmModel realmModel = kcSession.realms().getRealm(realm.getId());
kcSession.getContext().setRealm(realmModel);
// create another instance of the endpoint that will be run within the new session.
Resteasy.pushContext(KeycloakSession.class, kcSession);
TokenEndpoint other = new TokenEndpoint(session, new TokenManager(), new EventBuilder(realmModel, kcSession, clientConnection));
return KeycloakModelUtils.runJobInRetriableTransaction(session.getKeycloakSessionFactory(), new ResponseSessionTask(session) {
@Override
public Response runInternal(KeycloakSession session) {
// create another instance of the endpoint to isolate each run.
TokenEndpoint other = new TokenEndpoint(session, new TokenManager(),
new EventBuilder(session.getContext().getRealm(), session, clientConnection));
// process the request in the created instance.
return other.processGrantRequestInternal();
} catch (WebApplicationException we) {
// WebApplicationException needs to be returned and treated (rethrown) by the calling code because the new transaction
// still needs to be committed when this exception is thrown. It captures final business states that won't change when
// being retried, like an invalid code.
return we;
}
}, 10, 100);
if (WebApplicationException.class.isInstance(result)) {
throw (WebApplicationException) result;
} else {
return (Response) result;
}
}
private Response processGrantRequestInternal() {

24
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrentLoginTest.java
View File

@@ -52,6 +52,10 @@ import org.keycloak.admin.client.resource.ClientsResource;
import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.jose.jws.JWSInput;
import org.keycloak.models.UserSessionSpi;
import org.keycloak.models.map.common.AbstractMapProviderFactory;
import org.keycloak.models.map.storage.MapStorageProviderFactory;
import org.keycloak.models.map.storage.jpa.JpaMapStorageProviderFactory;
import org.keycloak.models.map.userSession.MapUserSessionProviderFactory;
import org.keycloak.models.sessions.infinispan.InfinispanUserSessionProviderFactory;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.protocol.oidc.OIDCConfigAttributes;
@@ -73,6 +77,7 @@ import org.hamcrest.Matchers;
import org.keycloak.util.JsonSerialization;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.keycloak.testsuite.util.ServerURLs.AUTH_SERVER_SSL_REQUIRED;
/**
* @author <a href="mailto:vramik@redhat.com">Vlastislav Ramik</a>
@@ -87,7 +92,14 @@ public class ConcurrentLoginTest extends AbstractConcurrencyTest {
@Before
public void beforeTest() {
// userSessionProvider is used only to prevent tests from running in certain configs, should be removed once GHI #15410 is resolved.
userSessionProvider = testingClient.server().fetch(session -> Config.getProvider(UserSessionSpi.NAME), String.class);
if (userSessionProvider.equals(MapUserSessionProviderFactory.PROVIDER_ID)) {
// append the storage provider in case of map
String mapStorageProvider = testingClient.server().fetch(session -> Config.scope(UserSessionSpi.NAME,
MapUserSessionProviderFactory.PROVIDER_ID, AbstractMapProviderFactory.CONFIG_STORAGE).get("provider"), String.class);
if (mapStorageProvider != null) userSessionProvider = userSessionProvider + "-" + mapStorageProvider;
}
createClients();
}
@@ -114,9 +126,11 @@ public class ConcurrentLoginTest extends AbstractConcurrencyTest {
@Test
public void concurrentLoginSingleUser() throws Throwable {
Assume.assumeThat("Test runs only with InfinispanUserSessionProvider",
// remove this restriction once GHI #15410 is resolved.
Assume.assumeThat("Test runs only with InfinispanUserSessionProvider or MapUserSessionProvider using JPA",
userSessionProvider,
Matchers.is(InfinispanUserSessionProviderFactory.PROVIDER_ID));
Matchers.either(equalTo(InfinispanUserSessionProviderFactory.PROVIDER_ID))
.or(equalTo(MapUserSessionProviderFactory.PROVIDER_ID + "-" + JpaMapStorageProviderFactory.PROVIDER_ID)));
log.info("*********************************************");
long start = System.currentTimeMillis();
@@ -182,9 +196,11 @@ public class ConcurrentLoginTest extends AbstractConcurrencyTest {
@Test
public void concurrentLoginMultipleUsers() throws Throwable {
Assume.assumeThat("Test runs only with InfinispanUserSessionProvider",
// remove this restriction once GHI #15410 is resolved.
Assume.assumeThat("Test runs only with InfinispanUserSessionProvider or MapUserSessionProvider using JPA",
userSessionProvider,
Matchers.is(InfinispanUserSessionProviderFactory.PROVIDER_ID));
Matchers.either(equalTo(InfinispanUserSessionProviderFactory.PROVIDER_ID))
.or(equalTo(MapUserSessionProviderFactory.PROVIDER_ID + "-" + JpaMapStorageProviderFactory.PROVIDER_ID)));
log.info("*********************************************");
long start = System.currentTimeMillis();