From 062841a059226d32657c59dca67003e12182bf45 Mon Sep 17 00:00:00 2001 From: stianst Date: Mon, 4 Nov 2019 12:53:29 +0100 Subject: [PATCH] KEYCLOAK-11898 Refactor AIA implementation --- .../authentication/RequiredActionContext.java | 17 +-- .../java/org/keycloak/models/Constants.java | 2 + .../RequiredActionContextResult.java | 5 - .../FreeMarkerLoginFormsProvider.java | 3 +- .../protocol/oidc/OIDCLoginProtocol.java | 5 + .../managers/AuthenticationManager.java | 140 ++++++++++-------- .../resources/LoginActionsService.java | 34 ++--- .../testsuite/pages/LoginConfigTotpPage.java | 9 ++ .../pages/LoginPasswordUpdatePage.java | 10 ++ .../pages/LoginUpdateProfilePage.java | 9 ++ .../AbstractAppInitiatedActionTest.java | 40 +++-- .../actions/AppInitiatedActionCancelTest.java | 72 --------- .../AppInitiatedActionResetPasswordTest.java | 80 +++++++++- .../actions/AppInitiatedActionRoleTest.java | 62 -------- .../AppInitiatedActionTotpSetupTest.java | 43 ++---- .../AppInitiatedActionUpdateProfileTest.java | 13 +- .../RequiredActionResetPasswordTest.java | 5 + .../actions/RequiredActionTotpSetupTest.java | 1 + .../RequiredActionUpdateProfileTest.java | 3 + 19 files changed, 263 insertions(+), 290 deletions(-) delete mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionCancelTest.java delete mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionRoleTest.java diff --git a/server-spi-private/src/main/java/org/keycloak/authentication/RequiredActionContext.java b/server-spi-private/src/main/java/org/keycloak/authentication/RequiredActionContext.java index 559a0eda60..adbac97c38 100755 --- a/server-spi-private/src/main/java/org/keycloak/authentication/RequiredActionContext.java +++ b/server-spi-private/src/main/java/org/keycloak/authentication/RequiredActionContext.java @@ -37,12 +37,17 @@ import java.net.URI; * @version $Revision: 1 $ */ public interface RequiredActionContext { - public static enum Status { + enum Status { CHALLENGE, SUCCESS, IGNORE, - FAILURE, - CANCELED_AIA + FAILURE + } + + enum KcActionStatus { + SUCCESS, + CANCELLED, + ERROR } /** @@ -139,11 +144,5 @@ public interface RequiredActionContext { * */ void ignore(); - - /** - * Mark application-initiated action as canceled by the user. - * - */ - void cancelAIA(); } diff --git a/server-spi-private/src/main/java/org/keycloak/models/Constants.java b/server-spi-private/src/main/java/org/keycloak/models/Constants.java index 6326ee4098..c8fdf81502 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/Constants.java +++ b/server-spi-private/src/main/java/org/keycloak/models/Constants.java @@ -69,6 +69,8 @@ public final class Constants { public static final String KEY = "key"; public static final String KC_ACTION = "kc_action"; + public static final String KC_ACTION_STATUS = "kc_action_status"; + public static final String KC_ACTION_EXECUTING = "kc_action_executing"; public static final int KC_ACTION_MAX_AGE = 300; public static final String IS_AIA_REQUEST = "IS_AIA_REQUEST"; diff --git a/services/src/main/java/org/keycloak/authentication/RequiredActionContextResult.java b/services/src/main/java/org/keycloak/authentication/RequiredActionContextResult.java index 53d54ab534..f79323443a 100755 --- a/services/src/main/java/org/keycloak/authentication/RequiredActionContextResult.java +++ b/services/src/main/java/org/keycloak/authentication/RequiredActionContextResult.java @@ -136,11 +136,6 @@ public class RequiredActionContextResult implements RequiredActionContext { public void ignore() { status = Status.IGNORE; } - - @Override - public void cancelAIA() { - status = Status.CANCELED_AIA; - } @Override public URI getActionUrl(String code) { diff --git a/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java b/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java index 01399bffa5..b549989cfd 100755 --- a/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java +++ b/services/src/main/java/org/keycloak/forms/login/freemarker/FreeMarkerLoginFormsProvider.java @@ -66,7 +66,6 @@ import java.util.*; import static org.keycloak.models.UserModel.RequiredAction.UPDATE_PASSWORD; -import static org.keycloak.services.managers.AuthenticationManager.IS_AIA_REQUEST; /** * @author Stian Thorgersen @@ -180,7 +179,7 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider { attributes.put("statusCode", status.getStatusCode()); } - if (authenticationSession != null && authenticationSession.getClientNote(IS_AIA_REQUEST) != null) { + if (authenticationSession != null && authenticationSession.getClientNote(Constants.KC_ACTION_EXECUTING) != null) { attributes.put("isAppInitiatedAction", true); } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java index 132116309b..ea018dc21c 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java @@ -203,6 +203,11 @@ public class OIDCLoginProtocol implements LoginProtocol { String nonce = authSession.getClientNote(OIDCLoginProtocol.NONCE_PARAM); clientSessionCtx.setAttribute(OIDCLoginProtocol.NONCE_PARAM, nonce); + String kcActionStatus = authSession.getClientNote(Constants.KC_ACTION_STATUS); + if (kcActionStatus != null) { + redirectUri.addParam(Constants.KC_ACTION_STATUS, kcActionStatus); + } + // Standard or hybrid flow String code = null; if (responseType.hasResponseType(OIDCResponseType.CODE)) { diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java index ed2cfddd72..c6b8eeeb14 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -129,8 +129,6 @@ public class AuthenticationManager { public static final String KEYCLOAK_REMEMBER_ME = "KEYCLOAK_REMEMBER_ME"; public static final String KEYCLOAK_LOGOUT_PROTOCOL = "KEYCLOAK_LOGOUT_PROTOCOL"; private static final TokenTypeCheck VALIDATE_IDENTITY_COOKIE = new TokenTypeCheck(TokenUtil.TOKEN_TYPE_KEYCLOAK_ID); - public static final String IS_AIA_REQUEST = LOGIN_SESSION_NOTE_ADDITIONAL_REQ_PARAMS_PREFIX + Constants.IS_AIA_REQUEST; - public static final String IS_SILENT_CANCEL = LOGIN_SESSION_NOTE_ADDITIONAL_REQ_PARAMS_PREFIX + Constants.AIA_SILENT_CANCEL; public static boolean isSessionValid(RealmModel realm, UserSessionModel userSession) { if (userSession == null) { @@ -904,6 +902,11 @@ public class AuthenticationManager { return authSession.getRequiredActions().iterator().next(); } + String kcAction = authSession.getClientNote(Constants.KC_ACTION); + if (kcAction != null) { + return kcAction; + } + if (client.isConsentRequired()) { UserConsentModel grantedConsent = getEffectiveGrantedConsent(session, authSession); @@ -1056,43 +1059,78 @@ public class AuthenticationManager { List sortedRequiredActions = sortRequiredActionsByPriority(realm, requiredActions); for (RequiredActionProviderModel model : sortedRequiredActions) { - RequiredActionFactory factory = (RequiredActionFactory)session.getKeycloakSessionFactory().getProviderFactory(RequiredActionProvider.class, model.getProviderId()); - if (factory == null) { - throw new RuntimeException("Unable to find factory for Required Action: " + model.getProviderId() + " did you forget to declare it in a META-INF/services file?"); - } - RequiredActionContextResult context = new RequiredActionContextResult(authSession, realm, event, session, request, user, factory); - RequiredActionProvider actionProvider = null; - try { - actionProvider = createRequiredAction(context); - } catch (AuthenticationFlowException e) { - if (e.getResponse() != null) { - return e.getResponse(); - } - throw e; - } - actionProvider.requiredActionChallenge(context); - - if (context.getStatus() == RequiredActionContext.Status.FAILURE) { - LoginProtocol protocol = context.getSession().getProvider(LoginProtocol.class, context.getAuthenticationSession().getProtocol()); - protocol.setRealm(context.getRealm()) - .setHttpHeaders(context.getHttpRequest().getHttpHeaders()) - .setUriInfo(context.getUriInfo()) - .setEventBuilder(event); - Response response = protocol.sendError(context.getAuthenticationSession(), Error.CONSENT_DENIED); - event.error(Errors.REJECTED_BY_USER); + Response response = executeAction(session, authSession, model, request, event, realm, user, false); + if (response != null) { return response; } - else if (context.getStatus() == RequiredActionContext.Status.CHALLENGE) { - authSession.setAuthNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION, model.getProviderId()); - return context.getChallenge(); + } + + String kcAction = authSession.getClientNote(Constants.KC_ACTION); + if (kcAction != null) { + for (RequiredActionProviderModel m : realm.getRequiredActionProviders()) { + if (m.getProviderId().equals(kcAction)) { + return executeAction(session, authSession, m, request, event, realm, user, true); + } } - else if (context.getStatus() == RequiredActionContext.Status.SUCCESS) { - event.clone().event(EventType.CUSTOM_REQUIRED_ACTION).detail(Details.CUSTOM_REQUIRED_ACTION, factory.getId()).success(); - // don't have to perform the same action twice, so remove it from both the user and session required actions - authSession.getAuthenticatedUser().removeRequiredAction(factory.getId()); - authSession.removeRequiredAction(factory.getId()); + + logger.debugv("Requested action {0} not configured for realm", kcAction); + setKcActionStatus(kcAction, RequiredActionContext.KcActionStatus.ERROR, authSession); + } + + return null; + } + + private static Response executeAction(KeycloakSession session, AuthenticationSessionModel authSession, RequiredActionProviderModel model, + HttpRequest request, EventBuilder event, RealmModel realm, UserModel user, boolean kcActionExecution) { + RequiredActionFactory factory = (RequiredActionFactory) session.getKeycloakSessionFactory().getProviderFactory(RequiredActionProvider.class, model.getProviderId()); + if (factory == null) { + throw new RuntimeException("Unable to find factory for Required Action: " + model.getProviderId() + " did you forget to declare it in a META-INF/services file?"); + } + RequiredActionContextResult context = new RequiredActionContextResult(authSession, realm, event, session, request, user, factory); + RequiredActionProvider actionProvider = null; + try { + actionProvider = createRequiredAction(context); + } catch (AuthenticationFlowException e) { + if (e.getResponse() != null) { + return e.getResponse(); + } + throw e; + } + + if (kcActionExecution) { + if (actionProvider.initiatedActionSupport() == InitiatedActionSupport.NOT_SUPPORTED) { + logger.debugv("Requested action {0} does not support being invoked with kc_action", factory.getId()); + setKcActionStatus(factory.getId(), RequiredActionContext.KcActionStatus.ERROR, authSession); + return null; + } else { + authSession.setClientNote(Constants.KC_ACTION_EXECUTING, factory.getId()); } } + + actionProvider.requiredActionChallenge(context); + + if (context.getStatus() == RequiredActionContext.Status.FAILURE) { + LoginProtocol protocol = context.getSession().getProvider(LoginProtocol.class, context.getAuthenticationSession().getProtocol()); + protocol.setRealm(context.getRealm()) + .setHttpHeaders(context.getHttpRequest().getHttpHeaders()) + .setUriInfo(context.getUriInfo()) + .setEventBuilder(event); + Response response = protocol.sendError(context.getAuthenticationSession(), Error.CONSENT_DENIED); + event.error(Errors.REJECTED_BY_USER); + return response; + } + else if (context.getStatus() == RequiredActionContext.Status.CHALLENGE) { + authSession.setAuthNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION, model.getProviderId()); + return context.getChallenge(); + } + else if (context.getStatus() == RequiredActionContext.Status.SUCCESS) { + event.clone().event(EventType.CUSTOM_REQUIRED_ACTION).detail(Details.CUSTOM_REQUIRED_ACTION, factory.getId()).success(); + // don't have to perform the same action twice, so remove it from both the user and session required actions + authSession.getAuthenticatedUser().removeRequiredAction(factory.getId()); + authSession.removeRequiredAction(factory.getId()); + setKcActionStatus(factory.getId(), RequiredActionContext.KcActionStatus.SUCCESS, authSession); + } + return null; } @@ -1143,37 +1181,11 @@ public class AuthenticationManager { public void ignore() { throw new RuntimeException("Not allowed to call ignore() within evaluateTriggers()"); } - - @Override - public void cancelAIA() { - throw new RuntimeException("Not allowed to call cancelAIA() within evaluateTriggers()"); - } }; - evaluateApplicationInitiatedActionTrigger(session, provider, model, authSession); provider.evaluateTriggers(result); } } - - // Determine if provider is being requested as an Application-Initiated Action - // If so, add it to the authSession. - private static void evaluateApplicationInitiatedActionTrigger(final KeycloakSession session, - final RequiredActionProvider provider, - final RequiredActionProviderModel model, - final AuthenticationSessionModel authSession - ) { - if (provider.initiatedActionSupport() == InitiatedActionSupport.NOT_SUPPORTED) return; - - String aia = authSession.getClientNote(Constants.KC_ACTION); - if (aia == null) return; - - // make sure you are evaluating the action that was requested - if (!aia.equalsIgnoreCase(model.getProviderId())) return; - - authSession.addRequiredAction(model.getProviderId()); - authSession.removeClientNote(Constants.KC_ACTION); // keep this from being executed twice - authSession.setClientNote(IS_AIA_REQUEST, "true"); - } public static AuthResult verifyIdentityToken(KeycloakSession session, RealmModel realm, UriInfo uriInfo, ClientConnection connection, boolean checkActive, boolean checkTokenType, boolean isCookie, String tokenString, HttpHeaders headers, Predicate... additionalChecks) { @@ -1266,4 +1278,12 @@ public class AuthenticationManager { } } + public static void setKcActionStatus(String executedProviderId, RequiredActionContext.KcActionStatus status, AuthenticationSessionModel authSession) { + if (executedProviderId.equals(authSession.getClientNote(Constants.KC_ACTION))) { + authSession.setClientNote(Constants.KC_ACTION_STATUS, status.name().toLowerCase()); + authSession.removeClientNote(Constants.KC_ACTION); + authSession.removeClientNote(Constants.KC_ACTION_EXECUTING); + } + } + } diff --git a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java index 2762144a8a..5a2a623a69 100755 --- a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java +++ b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java @@ -101,8 +101,6 @@ import java.net.URI; import java.util.Map; import static org.keycloak.authentication.actiontoken.DefaultActionToken.ACTION_TOKEN_BASIC_CHECKS; -import static org.keycloak.services.managers.AuthenticationManager.IS_AIA_REQUEST; -import static org.keycloak.services.managers.AuthenticationManager.IS_SILENT_CANCEL; /** * @author Stian Thorgersen @@ -993,9 +991,10 @@ public class LoginActionsService { Response response; - if (isCancelAppInitiatedAction(authSession, context)) { + if (isCancelAppInitiatedAction(factory.getId(), authSession, context)) { provider.initiatedActionCanceled(session, authSession); - context.cancelAIA(); + AuthenticationManager.setKcActionStatus(factory.getId(), RequiredActionContext.KcActionStatus.CANCELLED, authSession); + context.success(); } else { provider.processAction(context); } @@ -1011,16 +1010,13 @@ public class LoginActionsService { authSession.removeRequiredAction(factory.getId()); authSession.getAuthenticatedUser().removeRequiredAction(factory.getId()); authSession.removeAuthNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION); + AuthenticationManager.setKcActionStatus(factory.getId(), RequiredActionContext.KcActionStatus.SUCCESS, authSession); response = AuthenticationManager.nextActionAfterAuthentication(session, authSession, clientConnection, request, session.getContext().getUri(), event); } else if (context.getStatus() == RequiredActionContext.Status.CHALLENGE) { response = context.getChallenge(); } else if (context.getStatus() == RequiredActionContext.Status.FAILURE) { response = interruptionResponse(context, authSession, action, Error.CONSENT_DENIED); - } else if (isSilentAIACancel(authSession, context)) { - response = interruptionResponse(context, authSession, action, Error.CANCELLED_AIA_SILENT); - } else if (context.getStatus() == RequiredActionContext.Status.CANCELED_AIA) { - response = interruptionResponse(context, authSession, action, Error.CANCELLED_AIA); } else { throw new RuntimeException("Unreachable"); } @@ -1041,21 +1037,13 @@ public class LoginActionsService { return protocol.sendError(authSession, error); } - private boolean isCancelAppInitiatedAction(AuthenticationSessionModel authSession, RequiredActionContextResult context) { - MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); - - boolean userRequestedCancelAIA = formData.getFirst(CANCEL_AIA) != null; - boolean isAIARequest = authSession.getClientNote(IS_AIA_REQUEST) != null; - - return isAIARequest && userRequestedCancelAIA; - } - - private boolean isSilentAIACancel(AuthenticationSessionModel authSession, RequiredActionContextResult context) { - String silentCancel = authSession.getClientNote(IS_SILENT_CANCEL); - boolean isSilentCancel = "true".equalsIgnoreCase(silentCancel); - boolean isAIACancel = isCancelAppInitiatedAction(authSession, context); - - return isSilentCancel && isAIACancel; + private boolean isCancelAppInitiatedAction(String providerId, AuthenticationSessionModel authSession, RequiredActionContextResult context) { + if (providerId.equals(authSession.getClientNote(Constants.KC_ACTION_EXECUTING))) { + MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); + boolean userRequestedCancelAIA = formData.getFirst(CANCEL_AIA) != null; + return userRequestedCancelAIA; + } + return false; } } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginConfigTotpPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginConfigTotpPage.java index 72d9a1b501..b88232b715 100755 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginConfigTotpPage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginConfigTotpPage.java @@ -16,6 +16,7 @@ */ package org.keycloak.testsuite.pages; +import org.openqa.selenium.NoSuchElementException; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.FindBy; @@ -82,4 +83,12 @@ public class LoginConfigTotpPage extends AbstractPage { return loginErrorMessage.getText(); } + public boolean isCancelDisplayed() { + try { + return cancelAIAButton.isDisplayed(); + } catch (NoSuchElementException e) { + return false; + } + } + } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginPasswordUpdatePage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginPasswordUpdatePage.java index 9d27d2596a..ae63ba9036 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginPasswordUpdatePage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginPasswordUpdatePage.java @@ -16,6 +16,7 @@ */ package org.keycloak.testsuite.pages; +import org.openqa.selenium.NoSuchElementException; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.FindBy; @@ -68,4 +69,13 @@ public class LoginPasswordUpdatePage extends LanguageComboboxAwarePage { public String getFeedbackMessage() { return feedbackMessage.getText(); } + + public boolean isCancelDisplayed() { + try { + return cancelAIAButton.isDisplayed(); + } catch (NoSuchElementException e) { + return false; + } + } + } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginUpdateProfilePage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginUpdateProfilePage.java index ed4716ec6f..fb81c22e56 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginUpdateProfilePage.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/pages/LoginUpdateProfilePage.java @@ -17,6 +17,7 @@ package org.keycloak.testsuite.pages; +import org.openqa.selenium.NoSuchElementException; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.FindBy; @@ -91,4 +92,12 @@ public class LoginUpdateProfilePage extends AbstractPage { throw new UnsupportedOperationException(); } + public boolean isCancelDisplayed() { + try { + return cancelAIAButton.isDisplayed(); + } catch (NoSuchElementException e) { + return false; + } + } + } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AbstractAppInitiatedActionTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AbstractAppInitiatedActionTest.java index f4bbcfa267..0d94efc4cb 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AbstractAppInitiatedActionTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AbstractAppInitiatedActionTest.java @@ -16,11 +16,11 @@ */ package org.keycloak.testsuite.actions; -import javax.ws.rs.core.UriBuilder; +import org.apache.http.NameValuePair; +import org.apache.http.client.utils.URLEncodedUtils; import org.jboss.arquillian.graphene.page.Page; import org.junit.Assert; import org.junit.Rule; - import org.keycloak.protocol.oidc.OIDCLoginProtocolService; import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; import org.keycloak.testsuite.AssertEvents; @@ -29,6 +29,11 @@ import org.keycloak.testsuite.pages.AppPage.RequestType; import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.util.WaitUtils; +import javax.ws.rs.core.UriBuilder; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.List; + /** * @author Stan Silvert */ @@ -50,13 +55,8 @@ public abstract class AbstractAppInitiatedActionTest extends AbstractTestRealmKe } protected void doAIA() { - doAIA(false); - } - - protected void doAIA(boolean silentCancel) { UriBuilder builder = OIDCLoginProtocolService.authUrl(authServerPage.createUriBuilder()); String uri = builder.queryParam("kc_action", this.aiaAction) - .queryParam("silent_cancel", Boolean.toString(silentCancel)) .queryParam("response_type", "code") .queryParam("client_id", "test-app") .queryParam("scope", "openid") @@ -65,15 +65,25 @@ public abstract class AbstractAppInitiatedActionTest extends AbstractTestRealmKe driver.navigate().to(uri); WaitUtils.waitForPageToLoad(); } - - protected void assertRedirectSuccess() { + + protected void assertKcActionStatus(String expectedStatus) { Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); - } - - protected void assertCancelMessage() { - String url = this.driver.getCurrentUrl(); - Assert.assertTrue("Expected 'error=interaction_required' in url", url.contains("error=interaction_required")); - Assert.assertTrue("Expected 'error_description=User+cancelled+aplication-initiated+action.' in url", url.contains("error_description=User+cancelled+aplication-initiated+action.")); + + URI url = null; + try { + url = new URI(this.driver.getCurrentUrl()); + } catch (URISyntaxException e) { + throw new RuntimeException(e); + } + List pairs = URLEncodedUtils.parse(url, "UTF-8"); + String kcActionStatus = null; + for (NameValuePair p : pairs) { + if (p.getName().equals("kc_action_status")) { + kcActionStatus = p.getValue(); + break; + } + } + Assert.assertEquals(expectedStatus, kcActionStatus); } protected void assertSilentCancelMessage() { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionCancelTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionCancelTest.java deleted file mode 100644 index 861a69f7de..0000000000 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionCancelTest.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Copyright 2019 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.actions; - -import org.jboss.arquillian.graphene.page.Page; -import org.junit.Test; -import org.keycloak.models.UserModel; -import org.keycloak.representations.idm.RealmRepresentation; -import org.keycloak.testsuite.pages.LoginUpdateProfileEditUsernameAllowedPage; - -/** - * Test makes sure that sending a cancel signal does not remove a non-AIA - * required action - * - * @author Stan Silvert - */ -public class AppInitiatedActionCancelTest extends AbstractAppInitiatedActionTest { - - @Page - protected LoginUpdateProfileEditUsernameAllowedPage updateProfilePage; - - public AppInitiatedActionCancelTest() { - super("update_profile"); - } - - @Override - public void configureTestRealm(RealmRepresentation testRealm) { - ActionUtil.addRequiredActionForUser(testRealm, "test-user@localhost", UserModel.RequiredAction.UPDATE_PROFILE.name()); - } - - @Test - // Verify that sending a "cancel" does not remove the required action. - public void cancelUpdateProfile() { - doAIA(); - loginPage.login("test-user@localhost", "password"); - updateProfilePage.assertCurrent(); - updateProfilePage.cancel(); - assertRedirectSuccess(); - assertCancelMessage(); - - appPage.logout(); - - loginPage.open(); - loginPage.assertCurrent(); - loginPage.login("test-user@localhost", "password"); - updateProfilePage.assertCurrent(); - } - - @Test - public void silentCancelUpdateProfile() { - doAIA(true); - loginPage.login("test-user@localhost", "password"); - updateProfilePage.assertCurrent(); - updateProfilePage.cancel(); - assertRedirectSuccess(); - assertSilentCancelMessage(); - } -} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionResetPasswordTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionResetPasswordTest.java index 4259f43a85..865956ddfd 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionResetPasswordTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionResetPasswordTest.java @@ -17,23 +17,31 @@ package org.keycloak.testsuite.actions; import org.jboss.arquillian.graphene.page.Page; +import org.junit.After; import org.junit.Assert; import org.junit.Rule; import org.junit.Test; +import org.keycloak.admin.client.resource.UserResource; import org.keycloak.events.EventType; +import org.keycloak.models.UserModel; import org.keycloak.representations.idm.EventRepresentation; import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.pages.AppPage.RequestType; import org.keycloak.testsuite.pages.LoginPasswordUpdatePage; import org.keycloak.testsuite.util.GreenMailRule; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + /** * @author Stan Silvert */ public class AppInitiatedActionResetPasswordTest extends AbstractAppInitiatedActionTest { public AppInitiatedActionResetPasswordTest() { - super("update_password"); + super(UserModel.RequiredAction.UPDATE_PASSWORD.name()); } @Override @@ -47,19 +55,28 @@ public class AppInitiatedActionResetPasswordTest extends AbstractAppInitiatedAct @Page protected LoginPasswordUpdatePage changePasswordPage; + @After + public void after() { + ApiUtil.resetUserPassword(testRealm().users().get(findUser("test-user@localhost").getId()), "password", false); + } @Test - public void tempPassword() throws Exception { - doAIA(); - + public void resetPassword() throws Exception { + loginPage.open(); loginPage.login("test-user@localhost", "password"); + events.expectLogin().assertEvent(); + + doAIA(); + changePasswordPage.assertCurrent(); + assertTrue(changePasswordPage.isCancelDisplayed()); + changePasswordPage.changePassword("new-password", "new-password"); events.expectRequiredAction(EventType.UPDATE_PASSWORD).assertEvent(); - Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + assertKcActionStatus("success"); EventRepresentation loginEvent = events.expectLogin().assertEvent(); @@ -72,7 +89,31 @@ public class AppInitiatedActionResetPasswordTest extends AbstractAppInitiatedAct events.expectLogin().assertEvent(); } - + + @Test + public void resetPasswordRequiresReAuth() throws Exception { + loginPage.open(); + loginPage.login("test-user@localhost", "password"); + + events.expectLogin().assertEvent(); + + setTimeOffset(350); + + // Should prompt for re-authentication + doAIA(); + + loginPage.assertCurrent(); + loginPage.login("test-user@localhost", "password"); + + changePasswordPage.assertCurrent(); + assertTrue(changePasswordPage.isCancelDisplayed()); + + changePasswordPage.changePassword("new-password", "new-password"); + + events.expectRequiredAction(EventType.UPDATE_PASSWORD).assertEvent(); + assertKcActionStatus("success"); + } + @Test public void cancelChangePassword() throws Exception { doAIA(); @@ -82,8 +123,31 @@ public class AppInitiatedActionResetPasswordTest extends AbstractAppInitiatedAct changePasswordPage.assertCurrent(); changePasswordPage.cancel(); - assertRedirectSuccess(); - assertCancelMessage(); + assertKcActionStatus("cancelled"); + } + + @Test + public void resetPasswordUserHasUpdatePasswordRequiredAction() throws Exception { + loginPage.open(); + loginPage.login("test-user@localhost", "password"); + + UserResource userResource = testRealm().users().get(findUser("test-user@localhost").getId()); + UserRepresentation userRep = userResource.toRepresentation(); + userRep.getRequiredActions().add(UserModel.RequiredAction.UPDATE_PASSWORD.name()); + userResource.update(userRep); + + events.expectLogin().assertEvent(); + + doAIA(); + + changePasswordPage.assertCurrent(); + assertFalse(changePasswordPage.isCancelDisplayed()); + + changePasswordPage.changePassword("new-password", "new-password"); + + events.expectRequiredAction(EventType.UPDATE_PASSWORD).assertEvent(); + + assertKcActionStatus("success"); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionRoleTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionRoleTest.java deleted file mode 100644 index b948fb26cd..0000000000 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionRoleTest.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright 2019 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.actions; - -import java.util.List; -import org.jboss.arquillian.graphene.page.Page; -import org.junit.Test; -import org.keycloak.models.AccountRoles; -import org.keycloak.representations.idm.RealmRepresentation; -import org.keycloak.representations.idm.RoleRepresentation; -import org.keycloak.testsuite.pages.ErrorPage; - -/** - * - * @author Stan Silvert - */ -public class AppInitiatedActionRoleTest extends AbstractAppInitiatedActionTest { - - @Page - protected ErrorPage errorPage; - - public AppInitiatedActionRoleTest() { - super("update_profile"); - } - - @Override - public void configureTestRealm(RealmRepresentation testRealm) { - List roleList = testRealm.getRoles().getClient().get("test-app"); - - RoleRepresentation manageAccountRole = null; - for (RoleRepresentation role : roleList) { - if (role.getName().equals(AccountRoles.MANAGE_ACCOUNT)) { - manageAccountRole = role; - break; - } - } - - roleList.remove(manageAccountRole); - } - - @Test - public void roleNotSetTest() { - loginPage.open(); - loginPage.login("test-user@localhost", "password"); - doAIA(); - errorPage.assertCurrent(); - } -} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionTotpSetupTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionTotpSetupTest.java index 2694a6e1c6..4b67c4b5ac 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionTotpSetupTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionTotpSetupTest.java @@ -25,6 +25,7 @@ import org.keycloak.events.Details; import org.keycloak.events.EventType; import org.keycloak.models.AuthenticationExecutionModel; import org.keycloak.models.UserCredentialModel; +import org.keycloak.models.UserModel; import org.keycloak.models.utils.HmacOTP; import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation; @@ -51,7 +52,7 @@ import static org.junit.Assert.assertTrue; public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionTest { public AppInitiatedActionTotpSetupTest() { - super("configure_totp"); + super(UserModel.RequiredAction.CONFIGURE_TOTP.name()); } @Override @@ -64,7 +65,7 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT for (AuthenticationExecutionInfoRepresentation execution : adminClient.realm("test").flows().getExecutions("browser")) { String providerId = execution.getProviderId(); if ("auth-otp-form".equals(providerId)) { - execution.setRequirement(AuthenticationExecutionModel.Requirement.REQUIRED.name()); + execution.setRequirement(AuthenticationExecutionModel.Requirement.OPTIONAL.name()); adminClient.realm("test").flows().updateExecutions("browser", execution); } } @@ -111,8 +112,8 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT events.poll(); // skip to totp event String authSessionId = events.expectRequiredAction(EventType.UPDATE_TOTP).user(userId).detail(Details.USERNAME, "setuptotp").assertEvent() .getDetails().get(Details.CODE_ID); - - assertRedirectSuccess(); + + assertKcActionStatus("success"); events.expectLogin().user(userId).session(authSessionId).detail(Details.USERNAME, "setuptotp").assertEvent(); } @@ -125,9 +126,8 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT totpPage.assertCurrent(); totpPage.cancel(); - - assertRedirectSuccess(); - assertCancelMessage(); + + assertKcActionStatus("cancelled"); } @Test @@ -297,8 +297,8 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT String authSessionId = events.expectRequiredAction(EventType.UPDATE_TOTP).assertEvent() .getDetails().get(Details.CODE_ID); - assertRedirectSuccess(); - + assertKcActionStatus("success"); + EventRepresentation loginEvent = events.expectLogin().session(authSessionId).assertEvent(); oauth.openLogout(); @@ -310,8 +310,6 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT loginTotpPage.login(totp.generateTOTP(totpSecret)); - assertRedirectSuccess(); - events.expectLogin().assertEvent(); } @@ -333,7 +331,7 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT totpPage.configure(totp.generateTOTP(totpCode)); // After totp config, user should be on the app page - assertRedirectSuccess(); + assertKcActionStatus("success"); events.poll(); events.expectRequiredAction(EventType.UPDATE_TOTP).user(userId).detail(Details.USERNAME, "setuptotp2").assertEvent(); @@ -375,17 +373,6 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT // Try to login loginPage.open(); loginPage.login("setupTotp2", "password2"); - - // Since the authentificator was removed, it has to be set up again - totpPage.assertCurrent(); - totpPage.configure(totp.generateTOTP(totpPage.getTotpSecret())); - - String sessionId = events.expectRequiredAction(EventType.UPDATE_TOTP).user(userId).detail(Details.USERNAME, "setupTotp2").assertEvent() - .getDetails().get(Details.CODE_ID); - - assertRedirectSuccess(); - - events.expectLogin().user(userId).session(sessionId).detail(Details.USERNAME, "setupTotp2").assertEvent(); } @Test @@ -416,7 +403,7 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT String sessionId = events.expectRequiredAction(EventType.UPDATE_TOTP).assertEvent() .getDetails().get(Details.CODE_ID); - assertRedirectSuccess(); + assertKcActionStatus("success"); EventRepresentation loginEvent = events.expectLogin().session(sessionId).assertEvent(); @@ -431,7 +418,7 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT assertEquals(8, token.length()); loginTotpPage.login(token); - assertRedirectSuccess(); + assertKcActionStatus(null); events.expectLogin().assertEvent(); @@ -469,7 +456,7 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT .getDetails().get(Details.CODE_ID); //RequestType reqType = appPage.getRequestType(); - assertRedirectSuccess(); + assertKcActionStatus("success"); EventRepresentation loginEvent = events.expectLogin().session(sessionId).assertEvent(); oauth.openLogout(); @@ -481,7 +468,7 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT String token = otpgen.generateHOTP(totpSecret, 1); loginTotpPage.login(token); - assertRedirectSuccess(); + assertKcActionStatus(null); events.expectLogin().assertEvent(); @@ -506,7 +493,7 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT loginTotpPage.assertCurrent(); loginTotpPage.login(token); - assertRedirectSuccess(); + assertKcActionStatus(null); events.expectLogin().assertEvent(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionUpdateProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionUpdateProfileTest.java index 1d9fc7fd71..c628c28d1d 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionUpdateProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionUpdateProfileTest.java @@ -23,6 +23,7 @@ import org.junit.Before; import org.junit.Test; import org.keycloak.events.Details; import org.keycloak.events.EventType; +import org.keycloak.models.UserModel; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; @@ -37,7 +38,7 @@ import org.keycloak.testsuite.util.UserBuilder; public class AppInitiatedActionUpdateProfileTest extends AbstractAppInitiatedActionTest { public AppInitiatedActionUpdateProfileTest() { - super("update_profile"); + super(UserModel.RequiredAction.UPDATE_PROFILE.name()); } @Page @@ -85,7 +86,7 @@ public class AppInitiatedActionUpdateProfileTest extends AbstractAppInitiatedAct events.expectRequiredAction(EventType.UPDATE_PROFILE).assertEvent(); events.expectLogin().assertEvent(); - assertRedirectSuccess(); + assertKcActionStatus("success"); // assert user is really updated in persistent store UserRepresentation user = ActionUtil.findUserWithAdminClient(adminClient, "test-user@localhost"); @@ -113,7 +114,7 @@ public class AppInitiatedActionUpdateProfileTest extends AbstractAppInitiatedAct events.expectRequiredAction(EventType.UPDATE_EMAIL).detail(Details.PREVIOUS_EMAIL, "test-user@localhost").detail(Details.UPDATED_EMAIL, "new@email.com").assertEvent(); events.expectRequiredAction(EventType.UPDATE_PROFILE).assertEvent(); - assertRedirectSuccess(); + assertKcActionStatus("success"); // assert user is really updated in persistent store UserRepresentation user = ActionUtil.findUserWithAdminClient(adminClient, "test-user@localhost"); @@ -132,8 +133,8 @@ public class AppInitiatedActionUpdateProfileTest extends AbstractAppInitiatedAct updateProfilePage.assertCurrent(); updateProfilePage.cancel(); - assertRedirectSuccess(); - assertCancelMessage(); + assertKcActionStatus("cancelled"); + // assert nothing was updated in persistent store UserRepresentation user = ActionUtil.findUserWithAdminClient(adminClient, "test-user@localhost"); @@ -164,7 +165,7 @@ public class AppInitiatedActionUpdateProfileTest extends AbstractAppInitiatedAct .removeDetail(Details.CONSENT) .assertEvent(); - assertRedirectSuccess(); + assertKcActionStatus("success"); events.expectLogin().detail(Details.USERNAME, "john-doh@localhost").user(userId).assertEvent(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionResetPasswordTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionResetPasswordTest.java index 59f88fe2a0..1e3ae0fe16 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionResetPasswordTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionResetPasswordTest.java @@ -33,6 +33,9 @@ import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.LoginPasswordUpdatePage; import org.keycloak.testsuite.util.GreenMailRule; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + /** * @author Stian Thorgersen */ @@ -66,6 +69,8 @@ public class RequiredActionResetPasswordTest extends AbstractTestRealmKeycloakTe loginPage.login("test-user@localhost", "password"); changePasswordPage.assertCurrent(); + assertFalse(changePasswordPage.isCancelDisplayed()); + changePasswordPage.changePassword("new-password", "new-password"); events.expectRequiredAction(EventType.UPDATE_PASSWORD).assertEvent(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionTotpSetupTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionTotpSetupTest.java index 22215764d2..d758b39b66 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionTotpSetupTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionTotpSetupTest.java @@ -131,6 +131,7 @@ public class RequiredActionTotpSetupTest extends AbstractTestRealmKeycloakTest { String userId = events.expectRegister("setupTotp", "email@mail.com").assertEvent().getUserId(); assertTrue(totpPage.isCurrent()); + assertFalse(totpPage.isCancelDisplayed()); totpPage.configure(totp.generateTOTP(totpPage.getTotpSecret())); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java index d829d5bc70..71e24d2618 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java @@ -38,6 +38,8 @@ import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.LoginUpdateProfileEditUsernameAllowedPage; import org.keycloak.testsuite.util.UserBuilder; +import static org.junit.Assert.assertFalse; + /** * @author Stian Thorgersen */ @@ -92,6 +94,7 @@ public class RequiredActionUpdateProfileTest extends AbstractTestRealmKeycloakTe loginPage.login("test-user@localhost", "password"); updateProfilePage.assertCurrent(); + assertFalse(updateProfilePage.isCancelDisplayed()); updateProfilePage.update("New first", "New last", "new@email.com", "test-user@localhost");