Make sure Cancel AIA does not remove required action from user

Signed-off-by: mposolda <mposolda@gmail.com>
This commit is contained in:
mposolda 2025-04-11 16:14:14 +02:00 committed by Stian Thorgersen
parent 09bba88927
commit a78c951a5a
6 changed files with 90 additions and 28 deletions

View File

@ -1262,13 +1262,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;
@ -1326,9 +1327,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);
}
@ -1367,7 +1366,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()))
@ -1386,17 +1385,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

@ -1171,10 +1171,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);
}
@ -1183,7 +1184,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

@ -14,6 +14,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;
@ -99,6 +100,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;
@ -38,12 +39,16 @@ import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.representations.idm.UserSessionRepresentation;
import org.keycloak.services.managers.AuthenticationSessionManager;
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.URLUtils;
import org.keycloak.testsuite.util.UserBuilder;
import org.keycloak.testsuite.util.oauth.AccessTokenResponse;
import org.keycloak.testsuite.util.oauth.OAuthClient;
import org.keycloak.testsuite.util.SecondBrowser;
@ -51,7 +56,9 @@ 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;
@ -79,6 +86,9 @@ public class AppInitiatedActionResetPasswordTest extends AbstractAppInitiatedAct
@Page
protected LoginPasswordUpdatePage changePasswordPage;
@Page
protected LoginConfigTotpPage totpPage;
@Drone
@SecondBrowser
private WebDriver driver2;
@ -238,6 +248,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

@ -35,6 +35,7 @@ import org.keycloak.testsuite.util.GreenMailRule;
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>
@ -123,12 +124,18 @@ public class AppInitiatedActionTest extends AbstractTestRealmKeycloakTest {
oauth.loginForm().kcAction(UserModel.RequiredAction.UPDATE_PASSWORD.name()).open();
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 terms and conditions should be displayed for the login
termsAndConditionsPage.assertCurrent();
appPage.assertCurrent();
assertEquals(SUCCESS, oauth.parseLoginResponse().getKcActionStatus());
} finally {
termsAndConditions.setPriority(prevPriority);
termsAndConditions.setEnabled(prevEnabled);

View File

@ -239,7 +239,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();
@ -249,12 +255,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());