Make sure Cancel AIA does not remove required action from user

Signed-off-by: mposolda <mposolda@gmail.com>
(cherry picked from commit 5e0915854348c9cb95519d5d2d04b41ee97605db)
This commit is contained in:
mposolda 2025-04-11 16:14:14 +02:00 committed by Stian Thorgersen
parent f835f49065
commit b329e6e79a
6 changed files with 106 additions and 40 deletions

View File

@ -1256,13 +1256,14 @@ public class AuthenticationManager {
protected static Response executionActions(KeycloakSession session, AuthenticationSessionModel authSession,
HttpRequest request, EventBuilder event, RealmModel realm, UserModel user, Set<String> ignoredActions) {
final var kcAction = authSession.getClientNote(Constants.KC_ACTION);
final var firstApplicableRequiredAction =
final String kcAction = authSession.getClientNote(Constants.KC_ACTION);
final RequiredActionProviderModel firstApplicableRequiredAction =
getFirstApplicableRequiredAction(realm, authSession, user, kcAction, ignoredActions);
boolean kcActionExecution = kcAction != null && kcAction.equals(firstApplicableRequiredAction.getProviderId());
if (firstApplicableRequiredAction != null) {
return executeAction(session, authSession, firstApplicableRequiredAction, request, event, realm, user,
kcAction != null, ignoredActions);
kcActionExecution, ignoredActions);
}
return null;
@ -1320,9 +1321,7 @@ public class AuthenticationManager {
return context.getChallenge();
}
else if (context.getStatus() == RequiredActionContext.Status.IGNORE) {
authSession.getAuthenticatedUser().removeRequiredAction(factory.getId());
authSession.removeRequiredAction(factory.getId());
setKcActionStatus(factory.getId(), RequiredActionContext.KcActionStatus.SUCCESS, authSession);
setKcActionStatus(factory.getId(), RequiredActionContext.KcActionStatus.ERROR, authSession);
ignoredActions.add(factory.getId());
return nextActionAfterAuthentication(session, authSession, session.getContext().getConnection(), request, session.getContext().getUri(), event, ignoredActions);
}
@ -1361,7 +1360,7 @@ public class AuthenticationManager {
nonInitiatedActionAliases.addAll(user.getRequiredActionsStream().toList());
nonInitiatedActionAliases.addAll(authSession.getRequiredActions());
final var applicableNonInitiatedActions = nonInitiatedActionAliases.stream()
final Map<String, RequiredActionProviderModel> applicableNonInitiatedActions = nonInitiatedActionAliases.stream()
.map(alias -> getApplicableRequiredAction(realm, alias))
.filter(Objects::nonNull)
.filter(model -> !ignoredActions.contains(model.getProviderId()))
@ -1380,17 +1379,14 @@ public class AuthenticationManager {
}
}
final Map<String, RequiredActionProviderModel> applicableActions;
if (kcAction != null) {
applicableActions = new HashMap<>(applicableNonInitiatedActions);
applicableActions.put(kcAction.getAlias(), kcAction);
} else {
applicableActions = applicableNonInitiatedActions;
}
final var applicableActionsSorted = applicableActions.values().stream()
final List<RequiredActionProviderModel> applicableActionsSorted = applicableNonInitiatedActions.values().stream()
.sorted(RequiredActionProviderModel.RequiredActionComparator.SINGLETON)
.toList();
.collect(Collectors.toList());
// Insert "kc_action" as last action (unless present in required actions)
if (kcAction != null && !applicableNonInitiatedActions.containsKey(kcActionAlias)) {
applicableActionsSorted.add(kcAction);
}
if (logger.isDebugEnabled()) {
logger.debugv("applicable required actions (sorted): {0}",

View File

@ -1161,10 +1161,11 @@ public class LoginActionsService {
Response response;
boolean cancelled = false;
if (isCancelAppInitiatedAction(factory.getId(), authSession, context)) {
provider.initiatedActionCanceled(session, authSession);
AuthenticationManager.setKcActionStatus(factory.getId(), RequiredActionContext.KcActionStatus.CANCELLED, authSession);
context.success();
cancelled = true;
} else {
provider.processAction(context);
}
@ -1173,7 +1174,13 @@ public class LoginActionsService {
authSession.setAuthNote(AuthenticationProcessor.LAST_PROCESSED_EXECUTION, action);
}
if (context.getStatus() == RequiredActionContext.Status.SUCCESS) {
if (cancelled) {
event.clone().error(Errors.REJECTED_BY_USER);
initLoginEvent(authSession);
event.event(EventType.LOGIN);
authSession.removeAuthNote(AuthenticationProcessor.CURRENT_AUTHENTICATION_EXECUTION);
response = AuthenticationManager.nextActionAfterAuthentication(session, authSession, clientConnection, request, session.getContext().getUri(), event);
} else if (context.getStatus() == RequiredActionContext.Status.SUCCESS) {
event.clone().success();
initLoginEvent(authSession);
event.event(EventType.LOGIN);

View File

@ -13,6 +13,7 @@ import java.time.Duration;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import static org.keycloak.testsuite.util.DroneUtils.getCurrentDriver;
@ -98,6 +99,18 @@ public final class URLUtils {
return true;
}
/**
* @return action-url from the HTML code of the current page. Assumption is, that page is one of the Keycloak login pages (login theme pages)
*/
public static String getActionUrlFromCurrentPage(WebDriver driver) {
Matcher m = Pattern.compile("form action=\"([^\"]*)\"").matcher(driver.getPageSource());
if (m.find()) {
return m.group(1);
} else {
return null;
}
}
/**
* @see #sendPOSTRequestWithWebDriver(String, String)
*

View File

@ -28,6 +28,7 @@ import org.junit.Test;
import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.cookie.CookieType;
import org.keycloak.events.Details;
import org.keycloak.events.Errors;
import org.keycloak.events.EventType;
import org.keycloak.events.email.EmailEventListenerProviderFactory;
import org.keycloak.models.RealmModel;
@ -37,19 +38,25 @@ import org.keycloak.representations.idm.EventRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.representations.idm.UserSessionRepresentation;
import org.keycloak.services.resources.LoginActionsService;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.pages.LoginConfigTotpPage;
import org.keycloak.testsuite.pages.LoginPasswordUpdatePage;
import org.keycloak.testsuite.updaters.RealmAttributeUpdater;
import org.keycloak.testsuite.updaters.UserAttributeUpdater;
import org.keycloak.testsuite.util.GreenMailRule;
import org.keycloak.testsuite.util.MailUtils;
import org.keycloak.testsuite.util.OAuthClient;
import org.keycloak.testsuite.util.URLUtils;
import org.keycloak.testsuite.util.UserBuilder;
import org.keycloak.testsuite.util.SecondBrowser;
import org.openqa.selenium.Cookie;
import org.openqa.selenium.WebDriver;
import java.util.List;
import java.util.Map;
import static org.hamcrest.Matchers.contains;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
@ -77,6 +84,9 @@ public class AppInitiatedActionResetPasswordTest extends AbstractAppInitiatedAct
@Page
protected LoginPasswordUpdatePage changePasswordPage;
@Page
protected LoginConfigTotpPage totpPage;
@Drone
@SecondBrowser
private WebDriver driver2;
@ -235,6 +245,35 @@ public class AppInitiatedActionResetPasswordTest extends AbstractAppInitiatedAct
changePasswordPage.cancel();
assertKcActionStatus(CANCELLED);
events.expect(EventType.CUSTOM_REQUIRED_ACTION_ERROR)
.detail(Details.CUSTOM_REQUIRED_ACTION, UserModel.RequiredAction.UPDATE_PASSWORD.name())
.error(Errors.REJECTED_BY_USER)
.assertEvent();
events.expectLogin().assertEvent();
}
@Test
public void cancelWhenOTPRequiredAction() throws Exception {
// Add OTP required action to the user
UserResource user = ApiUtil.findUserByUsernameId(testRealm(), "test-user@localhost");
UserRepresentation userRep = user.toRepresentation();
UserBuilder.edit(userRep).requiredAction(UserModel.RequiredAction.CONFIGURE_TOTP.name());
user.update(userRep);
doAIA();
loginPage.login("test-user@localhost", "password");
// Cancel button should not be displayed
totpPage.assertCurrent();
Assert.assertFalse(totpPage.isCancelDisplayed());
// Try to manually send POST request from browser with cancel the AIA
String actionUrl = URLUtils.getActionUrlFromCurrentPage(driver);
URLUtils.sendPOSTRequestWithWebDriver(actionUrl, Map.of(LoginActionsService.CANCEL_AIA, "true"));
// Assert OTP required action still on the user
Assert.assertThat(user.toRepresentation().getRequiredActions(), contains(UserModel.RequiredAction.CONFIGURE_TOTP.name()));
}
@Test

View File

@ -29,11 +29,12 @@ import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
import org.keycloak.testsuite.AssertEvents;
import org.keycloak.testsuite.pages.AppPage;
import org.keycloak.testsuite.pages.LoginPage;
import org.keycloak.testsuite.pages.VerifyEmailPage;
import org.keycloak.testsuite.pages.TermsAndConditionsPage;
import org.keycloak.testsuite.updaters.UserAttributeUpdater;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.keycloak.testsuite.actions.AbstractAppInitiatedActionTest.SUCCESS;
/**
* @author <a href="mailto:wadahiro@gmail.com">Hiroyuki Wada</a>
@ -54,7 +55,7 @@ public class AppInitiatedActionTest extends AbstractTestRealmKeycloakTest {
protected LoginPage loginPage;
@Page
protected VerifyEmailPage verifyEmailPage;
protected TermsAndConditionsPage termsAndConditionsPage;
@Test
public void executeUnknownAction() {
@ -102,30 +103,40 @@ public class AppInitiatedActionTest extends AbstractTestRealmKeycloakTest {
}
@Test
public void executeActionWithVerifyEmailUnsupportedAIA() throws IOException {
public void executeActionWithTermsAndConditionsUnsupportedAIA() throws IOException {
RealmResource realm = testRealm();
RequiredActionProviderRepresentation model = realm.flows().getRequiredAction(UserModel.RequiredAction.VERIFY_EMAIL.name());
int prevPriority = model.getPriority();
RequiredActionProviderRepresentation termsAndConditions = realm.flows().getRequiredAction(TermsAndConditions.PROVIDER_ID);
int prevPriority = termsAndConditions.getPriority();
boolean prevEnabled = termsAndConditions.isEnabled();
try (UserAttributeUpdater userUpdater = UserAttributeUpdater
.forUserByUsername(realm, "test-user@localhost")
.setRequiredActions(UserModel.RequiredAction.VERIFY_EMAIL).update()) {
// Set max priority for verify email (AIA not supported) to be executed before update password
model.setPriority(1);
realm.flows().updateRequiredAction(UserModel.RequiredAction.VERIFY_EMAIL.name(), model);
.setRequiredActions(UserModel.RequiredAction.TERMS_AND_CONDITIONS).update()) {
// Set max priority for terms and conditions (AIA not supported) to be executed before update password
termsAndConditions.setPriority(1);
termsAndConditions.setEnabled(true);
realm.flows().updateRequiredAction(TermsAndConditions.PROVIDER_ID, termsAndConditions);
oauth.kcAction(UserModel.RequiredAction.UPDATE_PASSWORD.name()).openLoginForm();
loginPage.login("test-user@localhost", "password");
// the update password should be displayed
// Terms and conditions are displayed first (They are not AIA, but displayed as a regular action as they were first)
termsAndConditionsPage.assertCurrent();
termsAndConditionsPage.acceptTerms();
// the update password should be displayed as an AIA
passwordUpdatePage.assertCurrent();
assertTrue(passwordUpdatePage.isCancelDisplayed());
passwordUpdatePage.changePassword("password", "password");
// once the AIA password is executed the verify profile should be displayed for the login
verifyEmailPage.assertCurrent();
// once the AIA password is executed the terms and conditions should be displayed for the login
appPage.assertCurrent();
String kcActionStatus = oauth.getCurrentQuery().get("kc_action_status");
assertEquals(SUCCESS, kcActionStatus);
} finally {
model.setPriority(prevPriority);
realm.flows().updateRequiredAction(UserModel.RequiredAction.VERIFY_EMAIL.name(), model);
termsAndConditions.setPriority(prevPriority);
termsAndConditions.setEnabled(prevEnabled);
realm.flows().updateRequiredAction(TermsAndConditions.PROVIDER_ID, termsAndConditions);
}
}
}

View File

@ -242,7 +242,13 @@ public class RequiredActionPriorityTest extends AbstractTestRealmKeycloakTest {
events.expectRequiredAction(EventType.UPDATE_PASSWORD).assertEvent();
events.expectRequiredAction(EventType.UPDATE_CREDENTIAL).assertEvent();
// Second, update profile
// Second, accept terms
termsPage.assertCurrent();
termsPage.acceptTerms();
events.expectRequiredAction(EventType.CUSTOM_REQUIRED_ACTION).removeDetail(Details.REDIRECT_URI)
.detail(Details.CUSTOM_REQUIRED_ACTION, TermsAndConditions.PROVIDER_ID).assertEvent();
// Finally, update profile. Action specified by "kc_action" should be always triggered last
updateProfilePage.assertCurrent();
updateProfilePage.prepareUpdate().firstName(NEW_FIRST_NAME).lastName(NEW_LAST_NAME)
.email(NEW_EMAIL).submit();
@ -252,12 +258,6 @@ public class RequiredActionPriorityTest extends AbstractTestRealmKeycloakTest {
.detail(Details.UPDATED_EMAIL, NEW_EMAIL)
.assertEvent();
// Finally, accept terms
termsPage.assertCurrent();
termsPage.acceptTerms();
events.expectRequiredAction(EventType.CUSTOM_REQUIRED_ACTION).removeDetail(Details.REDIRECT_URI)
.detail(Details.CUSTOM_REQUIRED_ACTION, TermsAndConditions.PROVIDER_ID).assertEvent();
// Logged in
appPage.assertCurrent();
assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType());