diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java index 74275bbd9ae..d5bb5fa020d 100755 --- a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java +++ b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java @@ -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); diff --git a/services/src/main/java/org/keycloak/common/util/ResponseSessionTask.java b/services/src/main/java/org/keycloak/common/util/ResponseSessionTask.java new file mode 100644 index 00000000000..2c697de33d9 --- /dev/null +++ b/services/src/main/java/org/keycloak/common/util/ResponseSessionTask.java @@ -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. + *

+ * More specifically, this task: + *
  • + * + * + * + *
    • + * + * @author Stefan Guilhen + */ +public abstract class ResponseSessionTask implements KeycloakSessionTaskWithResult { + + 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); +} diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java index 4797c3d2946..4e57538e3df 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java @@ -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 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 params) { String clientId = AuthorizationEndpointRequestParserProcessor.getClientId(event, session, params); diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java index 61b412753bf..251dc72857b 100644 --- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java @@ -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() { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrentLoginTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrentLoginTest.java index e46a2358229..6d5440a4848 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrentLoginTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrentLoginTest.java @@ -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 Vlastislav Ramik @@ -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();