Change id of TermsAndConditions required actions to uppercase

Closes #9991
This commit is contained in:
Michal Hajas 2022-11-29 12:55:12 +01:00 committed by Pedro Igor
parent 2ed162d8c7
commit de7dd77aeb
14 changed files with 55 additions and 32 deletions

View file

@ -114,6 +114,7 @@ import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
import static org.keycloak.models.utils.DefaultRequiredActions.getDefaultRequiredActionCaseInsensitively;
import static org.keycloak.models.utils.RepresentationToModel.createCredentials;
import static org.keycloak.models.utils.RepresentationToModel.createFederatedIdentities;
import static org.keycloak.models.utils.RepresentationToModel.createGroups;
@ -869,11 +870,7 @@ public class LegacyExportImportManager implements ExportImportManager {
}
if (userRep.getRequiredActions() != null) {
for (String requiredAction : userRep.getRequiredActions()) {
try {
user.addRequiredAction(UserModel.RequiredAction.valueOf(requiredAction.toUpperCase()));
} catch (IllegalArgumentException iae) {
user.addRequiredAction(requiredAction);
}
user.addRequiredAction(getDefaultRequiredActionCaseInsensitively(requiredAction));
}
}
createCredentials(userRep, session, newRealm, user, false);

View file

@ -123,6 +123,7 @@ import java.util.stream.Collectors;
import static org.keycloak.models.map.storage.QueryParameters.withCriteria;
import static org.keycloak.models.map.storage.criteria.DefaultModelCriteria.criteria;
import static org.keycloak.models.utils.DefaultRequiredActions.getDefaultRequiredActionCaseInsensitively;
import static org.keycloak.models.utils.RepresentationToModel.createCredentials;
import static org.keycloak.models.utils.RepresentationToModel.createFederatedIdentities;
import static org.keycloak.models.utils.RepresentationToModel.createGroups;
@ -1138,11 +1139,7 @@ public class MapExportImportManager implements ExportImportManager {
}
if (userRep.getRequiredActions() != null) {
for (String requiredAction : userRep.getRequiredActions()) {
try {
user.addRequiredAction(UserModel.RequiredAction.valueOf(requiredAction.toUpperCase()));
} catch (IllegalArgumentException iae) {
user.addRequiredAction(requiredAction);
}
user.addRequiredAction(getDefaultRequiredActionCaseInsensitively(requiredAction));
}
}
createCredentials(userRep, session, newRealm, user, false);

View file

@ -74,7 +74,7 @@ public class DefaultRequiredActions {
UPDATE_PROFILE(UserModel.RequiredAction.UPDATE_PROFILE.name(), DefaultRequiredActions::addUpdateProfileAction),
CONFIGURE_TOTP(UserModel.RequiredAction.CONFIGURE_TOTP.name(), DefaultRequiredActions::addConfigureTotpAction),
UPDATE_PASSWORD(UserModel.RequiredAction.UPDATE_PASSWORD.name(), DefaultRequiredActions::addUpdatePasswordAction),
TERMS_AND_CONDITIONS("terms_and_conditions", DefaultRequiredActions::addTermsAndConditionsAction),
TERMS_AND_CONDITIONS(UserModel.RequiredAction.TERMS_AND_CONDITIONS.name(), DefaultRequiredActions::addTermsAndConditionsAction),
DELETE_ACCOUNT("delete_account", DefaultRequiredActions::addDeleteAccountAction),
UPDATE_USER_LOCALE("update_user_locale", DefaultRequiredActions::addUpdateLocaleAction),
UPDATE_EMAIL(UserModel.RequiredAction.UPDATE_EMAIL.name(), DefaultRequiredActions::addUpdateEmailAction, () -> isFeatureEnabled(Profile.Feature.UPDATE_EMAIL)),
@ -169,12 +169,12 @@ public class DefaultRequiredActions {
}
public static void addTermsAndConditionsAction(RealmModel realm) {
if (realm.getRequiredActionProviderByAlias("terms_and_conditions") == null) {
if (realm.getRequiredActionProviderByAlias(UserModel.RequiredAction.TERMS_AND_CONDITIONS.name()) == null) {
RequiredActionProviderModel termsAndConditions = new RequiredActionProviderModel();
termsAndConditions.setEnabled(false);
termsAndConditions.setAlias("terms_and_conditions");
termsAndConditions.setAlias(UserModel.RequiredAction.TERMS_AND_CONDITIONS.name());
termsAndConditions.setName("Terms and Conditions");
termsAndConditions.setProviderId("terms_and_conditions");
termsAndConditions.setProviderId(UserModel.RequiredAction.TERMS_AND_CONDITIONS.name());
termsAndConditions.setDefaultAction(false);
termsAndConditions.setPriority(20);
realm.addRequiredActionProvider(termsAndConditions);
@ -290,4 +290,23 @@ public class DefaultRequiredActions {
realm.addRequiredActionProvider(webauthnRegister);
}
}
/**
* Checks whether given {@code providerId} case insensitively matches any of {@link UserModel.RequiredAction} enum
* and if yes, it returns the value in correct form.
* <p/>
* This is necessary to stay backward compatible with older deployments where not all provider factories had ids
* in uppercase. This means that storage can contain some values in incorrect letter-case.
*
* @param providerId the required actions providerId
* @return providerId with correct letter-case, or the original value if it doesn't match any
* of {@link UserModel.RequiredAction}
*/
public static String getDefaultRequiredActionCaseInsensitively(String providerId) {
try {
return UserModel.RequiredAction.valueOf(providerId.toUpperCase()).name();
} catch (IllegalArgumentException iae) {
return providerId;
}
}
}

View file

@ -21,11 +21,8 @@ import org.keycloak.provider.ProviderEvent;
import org.keycloak.storage.SearchableModelField;
import java.util.Comparator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
/**

View file

@ -36,6 +36,8 @@ import javax.ws.rs.core.Response;
import javax.ws.rs.core.UriBuilder;
import javax.ws.rs.core.UriInfo;
import static org.keycloak.models.utils.DefaultRequiredActions.getDefaultRequiredActionCaseInsensitively;
/**
*
* @author hmlnarik
@ -115,13 +117,13 @@ public class ExecuteActionsActionTokenHandler extends AbstractActionTokenHandler
KeycloakSessionFactory sessionFactory = tokenContext.getSession().getKeycloakSessionFactory();
return token.getRequiredActions().stream()
.map(actionName -> realm.getRequiredActionProviderByAlias(actionName)) // get realm-specific model from action name and filter out irrelevant
.map(realm::getRequiredActionProviderByAlias) // get realm-specific model from action name and filter out irrelevant
.filter(Objects::nonNull)
.filter(RequiredActionProviderModel::isEnabled)
.map(RequiredActionProviderModel::getProviderId) // get provider ID from model
.map(providerId -> (RequiredActionFactory) sessionFactory.getProviderFactory(RequiredActionProvider.class, providerId))
.map(providerId -> (RequiredActionFactory) sessionFactory.getProviderFactory(RequiredActionProvider.class, getDefaultRequiredActionCaseInsensitively(providerId)))
.filter(Objects::nonNull)
.noneMatch(RequiredActionFactory::isOneTimeAction);

View file

@ -22,6 +22,7 @@ import org.keycloak.authentication.*;
import org.keycloak.common.util.Time;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionFactory;
import org.keycloak.models.UserModel;
import javax.ws.rs.core.Response;
import java.util.Arrays;
@ -31,7 +32,7 @@ import java.util.Arrays;
* @version $Revision: 1 $
*/
public class TermsAndConditions implements RequiredActionProvider, RequiredActionFactory {
public static final String PROVIDER_ID = "terms_and_conditions";
public static final String PROVIDER_ID = UserModel.RequiredAction.TERMS_AND_CONDITIONS.name();
public static final String USER_ATTRIBUTE = PROVIDER_ID;
@Override

View file

@ -112,6 +112,7 @@ import java.util.stream.Stream;
import static org.keycloak.common.util.ServerCookie.SameSiteAttributeValue;
import static org.keycloak.models.UserSessionModel.CORRESPONDING_SESSION_ID;
import static org.keycloak.models.utils.DefaultRequiredActions.getDefaultRequiredActionCaseInsensitively;
import static org.keycloak.protocol.oidc.grants.device.DeviceGrantType.isOAuth2DeviceVerificationFlow;
import static org.keycloak.services.util.CookieHelper.getCookie;
import static org.keycloak.utils.LockObjectsForModification.lockUserSessionsForModification;
@ -1291,7 +1292,8 @@ public class AuthenticationManager {
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());
RequiredActionFactory factory = (RequiredActionFactory) session.getKeycloakSessionFactory()
.getProviderFactory(RequiredActionProvider.class, getDefaultRequiredActionCaseInsensitively(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?");
}
@ -1402,7 +1404,7 @@ public class AuthenticationManager {
private static RequiredActionFactory toRequiredActionFactory(KeycloakSession session, RequiredActionProviderModel model, RealmModel realm) {
RequiredActionFactory factory = (RequiredActionFactory) session.getKeycloakSessionFactory()
.getProviderFactory(RequiredActionProvider.class, model.getProviderId());
.getProviderFactory(RequiredActionProvider.class, getDefaultRequiredActionCaseInsensitively(model.getProviderId()));
if (factory == null) {
if (!DefaultRequiredActions.isActionAvailable(model)) {
logger.warnf("Required action provider factory '%s' configured in the realm '%s' is not available. " +

View file

@ -102,6 +102,7 @@ import java.net.URI;
import java.util.Map;
import static org.keycloak.authentication.actiontoken.DefaultActionToken.ACTION_TOKEN_BASIC_CHECKS;
import static org.keycloak.models.utils.DefaultRequiredActions.getDefaultRequiredActionCaseInsensitively;
/**
* @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@ -992,7 +993,7 @@ public class LoginActionsService {
event.event(EventType.CUSTOM_REQUIRED_ACTION);
event.detail(Details.CUSTOM_REQUIRED_ACTION, action);
RequiredActionFactory factory = (RequiredActionFactory)session.getKeycloakSessionFactory().getProviderFactory(RequiredActionProvider.class, action);
RequiredActionFactory factory = (RequiredActionFactory)session.getKeycloakSessionFactory().getProviderFactory(RequiredActionProvider.class, getDefaultRequiredActionCaseInsensitively(action));
if (factory == null) {
ServicesLogger.LOGGER.actionProviderNull();
event.error(Errors.INVALID_CODE);

View file

@ -16,6 +16,7 @@
*/
package org.keycloak.testsuite.auth.page.login;
import org.keycloak.models.UserModel;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.FindBy;
@ -38,7 +39,7 @@ public class TermsAndConditions extends RequiredActions {
@Override
public String getActionId() {
return "terms_and_conditions";
return UserModel.RequiredAction.TERMS_AND_CONDITIONS.name();
}
public void acceptTerms() {

View file

@ -134,7 +134,7 @@ public class RequiredActionPriorityTest extends AbstractTestRealmKeycloakTest {
// After Changing the priority, the order will be:
// UpdatePassword -> UpdateProfile -> TermsAndConditions
testRealm().flows().raiseRequiredActionPriority(UserModel.RequiredAction.UPDATE_PASSWORD.name());
testRealm().flows().lowerRequiredActionPriority("terms_and_conditions");
testRealm().flows().lowerRequiredActionPriority(UserModel.RequiredAction.TERMS_AND_CONDITIONS.name());
// Login
loginPage.open();

View file

@ -26,6 +26,7 @@ import org.keycloak.authentication.requiredactions.TermsAndConditions;
import org.keycloak.events.Details;
import org.keycloak.events.Errors;
import org.keycloak.events.EventType;
import org.keycloak.models.UserModel;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.RequiredActionProviderRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
@ -72,9 +73,9 @@ public class TermsAndConditionsTest extends AbstractTestRealmKeycloakTest {
UserBuilder.edit(user).requiredAction(TermsAndConditions.PROVIDER_ID);
adminClient.realm("test").users().get(user.getId()).update(user);
RequiredActionProviderRepresentation rep = adminClient.realm("test").flows().getRequiredAction("terms_and_conditions");
RequiredActionProviderRepresentation rep = adminClient.realm("test").flows().getRequiredAction(UserModel.RequiredAction.TERMS_AND_CONDITIONS.name());
rep.setEnabled(true);
adminClient.realm("test").flows().updateRequiredAction("terms_and_conditions", rep);
adminClient.realm("test").flows().updateRequiredAction(UserModel.RequiredAction.TERMS_AND_CONDITIONS.name(), rep);
}
@Test
@ -141,9 +142,9 @@ public class TermsAndConditionsTest extends AbstractTestRealmKeycloakTest {
@Test
// KEYCLOAK-3192
public void termsDisabled() {
RequiredActionProviderRepresentation rep = adminClient.realm("test").flows().getRequiredAction("terms_and_conditions");
RequiredActionProviderRepresentation rep = adminClient.realm("test").flows().getRequiredAction(UserModel.RequiredAction.TERMS_AND_CONDITIONS.name());
rep.setEnabled(false);
adminClient.realm("test").flows().updateRequiredAction("terms_and_conditions", rep);
adminClient.realm("test").flows().updateRequiredAction(UserModel.RequiredAction.TERMS_AND_CONDITIONS.name(), rep);
loginPage.open();

View file

@ -46,11 +46,11 @@ public class RequiredActionsTest extends AbstractAuthenticationTest {
List<RequiredActionProviderRepresentation> expected = new ArrayList<>();
addRequiredAction(expected, "CONFIGURE_TOTP", "Configure OTP", true, false, null);
addRequiredAction(expected, "TERMS_AND_CONDITIONS", "Terms and Conditions", false, false, null);
addRequiredAction(expected, "UPDATE_PASSWORD", "Update Password", true, false, null);
addRequiredAction(expected, "UPDATE_PROFILE", "Update Profile", true, false, null);
addRequiredAction(expected, "VERIFY_EMAIL", "Verify Email", true, false, null);
addRequiredAction(expected, "delete_account", "Delete Account", false, false, null);
addRequiredAction(expected, "terms_and_conditions", "Terms and Conditions", false, false, null);
addRequiredAction(expected, "update_user_locale", "Update User Locale", true, false, null);
addRequiredAction(expected, "webauthn-register", "Webauthn Register", true, false, null);
addRequiredAction(expected, "webauthn-register-passwordless", "Webauthn Register Passwordless", true, false, null);

View file

@ -25,7 +25,9 @@ import org.keycloak.admin.client.resource.ClientResource;
import org.keycloak.admin.client.resource.IdentityProviderResource;
import org.keycloak.admin.client.resource.RoleResource;
import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.authentication.requiredactions.TermsAndConditions;
import org.keycloak.events.admin.OperationType;
import org.keycloak.models.UserModel;
import org.keycloak.partialimport.PartialImportResult;
import org.keycloak.partialimport.PartialImportResults;
import org.keycloak.representations.idm.AdminEventRepresentation;
@ -58,6 +60,8 @@ import java.util.Set;
import static org.hamcrest.CoreMatchers.hasItem;
import static org.hamcrest.CoreMatchers.startsWith;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
@ -245,7 +249,7 @@ public class PartialImportTest extends AbstractAuthTest {
private void addUsersWithTermsAndConditions() {
List<UserRepresentation> users = new ArrayList<>();
List<String> requiredActions = new ArrayList<>();
requiredActions.add("terms_and_conditions");
requiredActions.add(UserModel.RequiredAction.TERMS_AND_CONDITIONS.name());
for (int i = 0; i < NUM_ENTITIES; i++) {
UserRepresentation user = createUserRepresentation(USER_PREFIX + i, USER_PREFIX + i + "@foo.com", "foo", "bar", true);
@ -476,6 +480,7 @@ public class PartialImportTest extends AbstractAuthTest {
UserRepresentation user = userRsc.toRepresentation();
assertTrue(user.getUsername().startsWith(USER_PREFIX));
Assert.assertTrue(userIds.contains(id));
assertThat(user.getRequiredActions(), contains(TermsAndConditions.PROVIDER_ID));
}
}

View file

@ -18,7 +18,7 @@ public class RequiredActions extends Authentication {
public final static String DEFAULT = ".defaultAction";
public final static String CONFIGURE_TOTP = "CONFIGURE_TOTP";
public final static String UPDATE_PROFILE = "UPDATE_PROFILE";
public final static String TERMS_AND_CONDITIONS = "terms_and_conditions";
public final static String TERMS_AND_CONDITIONS = "TERMS_AND_CONDITIONS";
public final static String UPDATE_PASSWORD = "UPDATE_PASSWORD";
public final static String VERIFY_EMAIL = "VERIFY_EMAIL";