Default required action providers are still available after feature disabling

Closes #13189

model/legacy-private/src/main/java/org/keycloak/migration/migrators/MigrateTo12_0_0.java

@@ -22,24 +22,12 @@ import org.keycloak.migration.ModelVersion;
 import org.keycloak.models.AccountRoles;
 import org.keycloak.models.Constants;
 import org.keycloak.models.KeycloakSession;
-import org.keycloak.models.RealmModel;
+import org.keycloak.models.utils.DefaultRequiredActions;
-import org.keycloak.models.RequiredActionProviderModel;
 
 public class MigrateTo12_0_0 implements Migration {
 
     public static final ModelVersion VERSION = new ModelVersion("12.0.0");
 
-    private static void addDeleteAccountAction(RealmModel realm) {
-        RequiredActionProviderModel deleteAccount = new RequiredActionProviderModel();
-        deleteAccount.setEnabled(false);
-        deleteAccount.setAlias("delete_account");
-        deleteAccount.setName("Delete Account");
-        deleteAccount.setProviderId("delete_account");
-        deleteAccount.setDefaultAction(false);
-        deleteAccount.setPriority(60);
-        realm.addRequiredActionProvider(deleteAccount);
-    }
-
     @Override
     public void migrate(KeycloakSession session) {
         session.realms()
@@ -50,7 +38,10 @@ public class MigrateTo12_0_0 implements Migration {
           .forEach(client -> client.addRole(AccountRoles.DELETE_ACCOUNT)
           .setDescription("${role_" + AccountRoles.DELETE_ACCOUNT + "}"));
 
-        session.realms().getRealmsStream().filter(realm -> Objects.isNull(realm.getRequiredActionProviderByAlias("delete_account"))).forEach(MigrateTo12_0_0::addDeleteAccountAction);
+        session.realms()
+                .getRealmsStream()
+                .filter(realm -> Objects.isNull(realm.getRequiredActionProviderByAlias("delete_account")))
+                .forEach(DefaultRequiredActions.Action.DELETE_ACCOUNT::addAction);
     }
 
     @Override

server-spi-private/src/main/java/org/keycloak/models/utils/DefaultRequiredActions.java

@@ -22,12 +22,101 @@ import org.keycloak.models.RealmModel;
 import org.keycloak.models.RequiredActionProviderModel;
 import org.keycloak.models.UserModel;
 
+import java.util.Arrays;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
+
+import static org.keycloak.common.Profile.isFeatureEnabled;
+
 /**
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
  * @version $Revision: 1 $
  */
 public class DefaultRequiredActions {
+
+    /**
+     * Check whether the action is the default one used in a realm and is available in the application
+     * Often, the default actions can be disabled due to the fact a particular feature is disabled
+     *
+     * @param action required action
+     * @return true if the required action is the default one and is available
+     */
+    public static boolean isActionAvailable(RequiredActionProviderModel action) {
+        if (action == null) return false;
+        final Optional<Action> foundAction = Action.findByAlias(action.getAlias());
+
+        return foundAction.isPresent() && foundAction.get().isAvailable();
+    }
+
+    /**
+     * Add default required actions to the realm
+     *
+     * @param realm realm
+     */
     public static void addActions(RealmModel realm) {
+        Arrays.stream(Action.values()).forEach(f -> f.addAction(realm));
+    }
+
+    /**
+     * Add default required action to the realm
+     *
+     * @param realm  realm
+     * @param action particular required action
+     */
+    public static void addAction(RealmModel realm, Action action) {
+        Optional.ofNullable(action).ifPresent(f -> f.addAction(realm));
+    }
+
+    public enum Action {
+        VERIFY_EMAIL(UserModel.RequiredAction.VERIFY_EMAIL.name(), DefaultRequiredActions::addVerifyEmailAction),
+        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),
+        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)),
+        CONFIGURE_RECOVERY_AUTHN_CODES(UserModel.RequiredAction.CONFIGURE_RECOVERY_AUTHN_CODES.name(), DefaultRequiredActions::addRecoveryAuthnCodesAction, () -> isFeatureEnabled(Profile.Feature.RECOVERY_CODES)),
+        WEBAUTHN_REGISTER("webauthn-register", DefaultRequiredActions::addWebAuthnRegisterAction, () -> isFeatureEnabled(Profile.Feature.WEB_AUTHN)),
+        WEBAUTHN_PASSWORDLESS_REGISTER("webauthn-register-passwordless", DefaultRequiredActions::addWebAuthnPasswordlessRegisterAction, () -> isFeatureEnabled(Profile.Feature.WEB_AUTHN));
+
+        private final String alias;
+        private final Consumer<RealmModel> addAction;
+        private final Supplier<Boolean> isAvailable;
+
+        Action(String alias, Consumer<RealmModel> addAction, Supplier<Boolean> isAvailable) {
+            this.alias = alias;
+            this.addAction = addAction;
+            this.isAvailable = isAvailable;
+        }
+
+        Action(String alias, Consumer<RealmModel> addAction) {
+            this(alias, addAction, () -> true);
+        }
+
+        public String getAlias() {
+            return alias;
+        }
+
+        public void addAction(RealmModel realm) {
+            addAction.accept(realm);
+        }
+
+        public boolean isAvailable() {
+            return isAvailable.get();
+        }
+
+        public static Optional<Action> findByAlias(String alias) {
+            return Arrays.stream(Action.values())
+                    .filter(Objects::nonNull)
+                    .filter(f -> f.getAlias().equals(alias))
+                    .findFirst();
+        }
+    }
+
+    public static void addVerifyEmailAction(RealmModel realm) {
         if (realm.getRequiredActionProviderByAlias(UserModel.RequiredAction.VERIFY_EMAIL.name()) == null) {
             RequiredActionProviderModel verifyEmail = new RequiredActionProviderModel();
             verifyEmail.setEnabled(true);
@@ -37,9 +126,10 @@ public class DefaultRequiredActions {
             verifyEmail.setDefaultAction(false);
             verifyEmail.setPriority(50);
             realm.addRequiredActionProvider(verifyEmail);
-
         }
+    }
 
+    public static void addUpdateProfileAction(RealmModel realm) {
         if (realm.getRequiredActionProviderByAlias(UserModel.RequiredAction.UPDATE_PROFILE.name()) == null) {
             RequiredActionProviderModel updateProfile = new RequiredActionProviderModel();
             updateProfile.setEnabled(true);
@@ -50,7 +140,9 @@ public class DefaultRequiredActions {
             updateProfile.setPriority(40);
             realm.addRequiredActionProvider(updateProfile);
         }
+    }
 
+    public static void addConfigureTotpAction(RealmModel realm) {
         if (realm.getRequiredActionProviderByAlias(UserModel.RequiredAction.CONFIGURE_TOTP.name()) == null) {
             RequiredActionProviderModel totp = new RequiredActionProviderModel();
             totp.setEnabled(true);
@@ -61,19 +153,9 @@ public class DefaultRequiredActions {
             totp.setPriority(10);
             realm.addRequiredActionProvider(totp);
         }
+    }
 
-        if (realm.getRequiredActionProviderByAlias(UserModel.RequiredAction.CONFIGURE_RECOVERY_AUTHN_CODES.name()) == null &&
+    public static void addUpdatePasswordAction(RealmModel realm) {
-                Profile.isFeatureEnabled(Profile.Feature.RECOVERY_CODES)) {
-            RequiredActionProviderModel recoveryCodes = new RequiredActionProviderModel();
-            recoveryCodes.setEnabled(true);
-            recoveryCodes.setAlias(UserModel.RequiredAction.CONFIGURE_RECOVERY_AUTHN_CODES.name());
-            recoveryCodes.setName("Recovery Authentication Codes");
-            recoveryCodes.setProviderId(UserModel.RequiredAction.CONFIGURE_RECOVERY_AUTHN_CODES.name());
-            recoveryCodes.setDefaultAction(false);
-            recoveryCodes.setPriority(70);
-            realm.addRequiredActionProvider(recoveryCodes);
-        }
-
         if (realm.getRequiredActionProviderByAlias(UserModel.RequiredAction.UPDATE_PASSWORD.name()) == null) {
             RequiredActionProviderModel updatePassword = new RequiredActionProviderModel();
             updatePassword.setEnabled(true);
@@ -84,7 +166,9 @@ public class DefaultRequiredActions {
             updatePassword.setPriority(30);
             realm.addRequiredActionProvider(updatePassword);
         }
+    }
 
+    public static void addTermsAndConditionsAction(RealmModel realm) {
         if (realm.getRequiredActionProviderByAlias("terms_and_conditions") == null) {
             RequiredActionProviderModel termsAndConditions = new RequiredActionProviderModel();
             termsAndConditions.setEnabled(false);
@@ -95,12 +179,6 @@ public class DefaultRequiredActions {
             termsAndConditions.setPriority(20);
             realm.addRequiredActionProvider(termsAndConditions);
         }
-
-        addUpdateLocaleAction(realm);
-        addDeleteAccountAction(realm);
-        addUpdateEmailAction(realm);
-        addWebAuthnRegisterAction(realm);
-        addWebAuthnPasswordlessRegisterAction(realm);
     }
 
     public static void addDeleteAccountAction(RealmModel realm) {
@@ -129,27 +207,58 @@ public class DefaultRequiredActions {
         }
     }
 
-    public static void addUpdateEmailAction(RealmModel realm){
+    public static void addUpdateEmailAction(RealmModel realm) {
-        if (realm.getRequiredActionProviderByAlias(UserModel.RequiredAction.UPDATE_EMAIL.name()) == null
+        final String PROVIDER_ID = UserModel.RequiredAction.UPDATE_EMAIL.name();
-                && Profile.isFeatureEnabled(Profile.Feature.UPDATE_EMAIL)){
+
+        final boolean isAvailable = Action.UPDATE_EMAIL.isAvailable();
+        if (!isAvailable) return;
+
+        final RequiredActionProviderModel provider = realm.getRequiredActionProviderByAlias(PROVIDER_ID);
+        final boolean isRequiredActionActive = provider != null;
+
+        if (!isRequiredActionActive) {
             RequiredActionProviderModel updateEmail = new RequiredActionProviderModel();
             updateEmail.setEnabled(true);
-            updateEmail.setAlias(UserModel.RequiredAction.UPDATE_EMAIL.name());
+            updateEmail.setAlias(PROVIDER_ID);
             updateEmail.setName("Update Email");
-            updateEmail.setProviderId(UserModel.RequiredAction.UPDATE_EMAIL.name());
+            updateEmail.setProviderId(PROVIDER_ID);
             updateEmail.setDefaultAction(false);
             updateEmail.setPriority(70);
             realm.addRequiredActionProvider(updateEmail);
         }
     }
 
+    public static void addRecoveryAuthnCodesAction(RealmModel realm) {
+        final String PROVIDER_ID = UserModel.RequiredAction.CONFIGURE_RECOVERY_AUTHN_CODES.name();
+
+        final boolean isAvailable = Action.CONFIGURE_RECOVERY_AUTHN_CODES.isAvailable();
+        if (!isAvailable) return;
+
+        final RequiredActionProviderModel provider = realm.getRequiredActionProviderByAlias(PROVIDER_ID);
+        final boolean isRequiredActionActive = provider != null;
+
+        if (!isRequiredActionActive) {
+            RequiredActionProviderModel recoveryCodes = new RequiredActionProviderModel();
+            recoveryCodes.setEnabled(true);
+            recoveryCodes.setAlias(PROVIDER_ID);
+            recoveryCodes.setName("Recovery Authentication Codes");
+            recoveryCodes.setProviderId(PROVIDER_ID);
+            recoveryCodes.setDefaultAction(false);
+            recoveryCodes.setPriority(70);
+            realm.addRequiredActionProvider(recoveryCodes);
+        }
+    }
+
     public static void addWebAuthnRegisterAction(RealmModel realm) {
         final String PROVIDER_ID = "webauthn-register";
 
-        final boolean isWebAuthnFeatureEnabled = Profile.isFeatureEnabled(Profile.Feature.WEB_AUTHN);
+        final boolean isAvailable = Action.WEBAUTHN_REGISTER.isAvailable();
-        final boolean isRequiredActionActive = realm.getRequiredActionProviderByAlias(PROVIDER_ID) != null;
+        if (!isAvailable) return;
 
-        if (isWebAuthnFeatureEnabled && !isRequiredActionActive) {
+        final RequiredActionProviderModel provider = realm.getRequiredActionProviderByAlias(PROVIDER_ID);
+        final boolean isRequiredActionActive = provider != null;
+
+        if (!isRequiredActionActive) {
             final RequiredActionProviderModel webauthnRegister = new RequiredActionProviderModel();
             webauthnRegister.setEnabled(true);
             webauthnRegister.setAlias(PROVIDER_ID);
@@ -164,10 +273,13 @@ public class DefaultRequiredActions {
     public static void addWebAuthnPasswordlessRegisterAction(RealmModel realm) {
         final String PROVIDER_ID = "webauthn-register-passwordless";
 
-        final boolean isWebAuthnFeatureEnabled = Profile.isFeatureEnabled(Profile.Feature.WEB_AUTHN);
+        final boolean isAvailable = Action.WEBAUTHN_PASSWORDLESS_REGISTER.isAvailable();
-        final boolean isRequiredActionActive = realm.getRequiredActionProviderByAlias(PROVIDER_ID) != null;
+        if (!isAvailable) return;
 
-        if (isWebAuthnFeatureEnabled && !isRequiredActionActive) {
+        final RequiredActionProviderModel provider = realm.getRequiredActionProviderByAlias(PROVIDER_ID);
+        final boolean isRequiredActionActive = provider != null;
+
+        if (!isRequiredActionActive) {
             final RequiredActionProviderModel webauthnRegister = new RequiredActionProviderModel();
             webauthnRegister.setEnabled(true);
             webauthnRegister.setAlias(PROVIDER_ID);

services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java

@@ -59,6 +59,7 @@ import org.keycloak.models.RequiredActionProviderModel;
 import org.keycloak.models.UserConsentModel;
 import org.keycloak.models.UserModel;
 import org.keycloak.models.UserSessionModel;
+import org.keycloak.models.utils.DefaultRequiredActions;
 import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.models.utils.SessionTimeoutHelper;
 import org.keycloak.models.utils.SystemClientUtil;
@@ -95,8 +96,8 @@ import javax.ws.rs.core.UriBuilder;
 import javax.ws.rs.core.UriInfo;
 import java.io.UnsupportedEncodingException;
 import java.net.URI;
-import java.net.URLEncoder;
 import java.net.URLDecoder;
+import java.net.URLEncoder;
 import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
@@ -1353,7 +1354,8 @@ public class AuthenticationManager {
         // see if any required actions need triggering, i.e. an expired password
         realm.getRequiredActionProvidersStream()
                 .filter(RequiredActionProviderModel::isEnabled)
-                .map(model -> toRequiredActionFactory(session, model))
+                .map(model -> toRequiredActionFactory(session, model, realm))
+                .filter(Objects::nonNull)
                 .forEachOrdered(f -> evaluateRequiredAction(session, authSession, request, event, realm, user, f));
     }
 
@@ -1386,12 +1388,17 @@ public class AuthenticationManager {
         provider.evaluateTriggers(result);
     }
 
-    private static RequiredActionFactory toRequiredActionFactory(KeycloakSession session, RequiredActionProviderModel model) {
+    private static RequiredActionFactory toRequiredActionFactory(KeycloakSession session, RequiredActionProviderModel model, RealmModel realm) {
         RequiredActionFactory factory = (RequiredActionFactory) session.getKeycloakSessionFactory()
                 .getProviderFactory(RequiredActionProvider.class, model.getProviderId());
         if (factory == null) {
-            throw new RuntimeException("Unable to find factory for Required Action: "
+            if (!DefaultRequiredActions.isActionAvailable(model)) {
-                    + model.getProviderId() + " did you forget to declare it in a META-INF/services file?");
+                logger.warnf("Required action provider factory '%s' configured in the realm '%s' is not available. " +
+                        "Provider not found or feature is disabled.", model.getProviderId(), realm.getName());
+            } else {
+                throw new RuntimeException(String.format("Unable to find factory for Required Action '%s' configured in the realm '%s'. " +
+                        "Did you forget to declare it in a META-INF/services file?", model.getProviderId(), realm.getName()));
+            }
         }
         return factory;
     }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/feature/AbstractFeatureStateTest.java

@@ -0,0 +1,57 @@
+/*
+ * 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.testsuite.feature;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.keycloak.representations.idm.RealmRepresentation;
+import org.keycloak.representations.info.ServerInfoRepresentation;
+import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
+
+import java.util.Set;
+
+/**
+ * Abstract test class for asserting a state of a particular feature after enabling/disabling
+ */
+public abstract class AbstractFeatureStateTest extends AbstractTestRealmKeycloakTest {
+
+    public abstract String getFeatureProviderId();
+
+    public abstract String getFeatureSpiName();
+
+    @Test
+    public void featureEnabled() {
+        testFeatureAvailability(true);
+    }
+
+    @Test
+    public void featureDisabled() {
+        testFeatureAvailability(false);
+    }
+
+    public void testFeatureAvailability(boolean expectedAvailability) {
+        ServerInfoRepresentation serverInfo = adminClient.serverInfo().getInfo();
+        Set<String> authenticatorProviderIds = serverInfo.getProviders().get(getFeatureSpiName()).getProviders().keySet();
+        Assert.assertEquals(expectedAvailability, authenticatorProviderIds.contains(getFeatureProviderId()));
+    }
+
+    @Override
+    public void configureTestRealm(RealmRepresentation testRealm) {
+
+    }
+}

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/feature/RecoveryAuthnCodesFeatureTest.java

@@ -0,0 +1,50 @@
+/*
+ * 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.testsuite.feature;
+
+import org.junit.Test;
+import org.keycloak.authentication.AuthenticatorSpi;
+import org.keycloak.authentication.authenticators.browser.RecoveryAuthnCodesFormAuthenticatorFactory;
+import org.keycloak.common.Profile;
+import org.keycloak.testsuite.arquillian.annotation.DisableFeature;
+import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
+
+public class RecoveryAuthnCodesFeatureTest extends AbstractFeatureStateTest {
+
+    @Override
+    public String getFeatureProviderId() {
+        return RecoveryAuthnCodesFormAuthenticatorFactory.PROVIDER_ID;
+    }
+
+    @Override
+    public String getFeatureSpiName() {
+        return AuthenticatorSpi.SPI_NAME;
+    }
+
+    @Test
+    @EnableFeature(value = Profile.Feature.RECOVERY_CODES, skipRestart = true)
+    public void featureEnabled() {
+        testFeatureAvailability(true);
+    }
+
+    @Test
+    @DisableFeature(value = Profile.Feature.RECOVERY_CODES, skipRestart = true)
+    public void featureDisabled() {
+        testFeatureAvailability(false);
+    }
+}

testsuite/integration-arquillian/tests/other/webauthn/src/test/java/org/keycloak/testsuite/webauthn/WebAuthnFeatureTest.java

@@ -18,39 +18,28 @@
 
 package org.keycloak.testsuite.webauthn;
 
-import org.junit.Assert;
 import org.junit.Test;
 import org.keycloak.authentication.AuthenticatorSpi;
 import org.keycloak.authentication.authenticators.browser.WebAuthnAuthenticatorFactory;
 import org.keycloak.common.Profile;
-import org.keycloak.representations.idm.RealmRepresentation;
-import org.keycloak.representations.info.ServerInfoRepresentation;
-import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
 import org.keycloak.testsuite.arquillian.annotation.DisableFeature;
-import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
+import org.keycloak.testsuite.feature.AbstractFeatureStateTest;
 
-import java.util.Set;
+public class WebAuthnFeatureTest extends AbstractFeatureStateTest {
-
-public class WebAuthnFeatureTest extends AbstractTestRealmKeycloakTest {
 
     @Override
-    public void configureTestRealm(RealmRepresentation testRealm) {
+    public String getFeatureProviderId() {
+        return WebAuthnAuthenticatorFactory.PROVIDER_ID;
     }
 
-    @Test
+    @Override
-    public void testWebAuthnEnabled() {
+    public String getFeatureSpiName() {
-        testWebAuthnAvailability(true);
+        return AuthenticatorSpi.SPI_NAME;
     }
 
     @Test
     @DisableFeature(value = Profile.Feature.WEB_AUTHN, skipRestart = true)
-    public void testWebAuthnDisabled() {
+    public void featureDisabled() {
-        testWebAuthnAvailability(false);
+        testFeatureAvailability(false);
-    }
-
-    private void testWebAuthnAvailability(boolean expectedAvailability) {
-        ServerInfoRepresentation serverInfo = adminClient.serverInfo().getInfo();
-        Set<String> authenticatorProviderIds = serverInfo.getProviders().get(AuthenticatorSpi.SPI_NAME).getProviders().keySet();
-        Assert.assertEquals(expectedAvailability, authenticatorProviderIds.contains(WebAuthnAuthenticatorFactory.PROVIDER_ID));
     }
 }