KEYCLOAK-3295 Kerberos authenticator changed during userFederationProvider update just if it was DISABLED

services/src/main/java/org/keycloak/services/resources/admin/UserFederationProvidersResource.java

@@ -22,6 +22,7 @@ import org.jboss.resteasy.spi.ResteasyProviderFactory;
 import org.keycloak.common.constants.KerberosConstants;
 import org.keycloak.events.admin.OperationType;
 import org.keycloak.mappers.FederationConfigValidationException;
+import org.keycloak.models.AuthenticationExecutionModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserFederationProvider;
@@ -98,7 +99,8 @@ public class UserFederationProvidersResource {
     public static boolean checkKerberosCredential(KeycloakSession session, RealmModel realm, UserFederationProviderModel model) {
         String allowKerberosCfg = model.getConfig().get(KerberosConstants.ALLOW_KERBEROS_AUTHENTICATION);
         if (Boolean.valueOf(allowKerberosCfg)) {
-            CredentialHelper.setAlternativeCredential(session, CredentialRepresentation.KERBEROS, realm);
+            CredentialHelper.setOrReplaceAuthenticationRequirement(session, realm, CredentialRepresentation.KERBEROS,
+                    AuthenticationExecutionModel.Requirement.ALTERNATIVE, AuthenticationExecutionModel.Requirement.DISABLED);
             return true;
         }
 

services/src/main/java/org/keycloak/utils/CredentialHelper.java

@@ -28,6 +28,7 @@ import org.keycloak.models.AuthenticationExecutionModel;
 import org.keycloak.models.AuthenticationFlowModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
+import org.keycloak.services.ServicesLogger;
 
 /**
  * used to set an execution a state based on type.
@@ -37,25 +38,32 @@ import org.keycloak.models.RealmModel;
  */
 public class CredentialHelper {
 
+    protected static final ServicesLogger logger = ServicesLogger.ROOT_LOGGER;
+
     public static void setRequiredCredential(KeycloakSession session, String type, RealmModel realm) {
         AuthenticationExecutionModel.Requirement requirement = AuthenticationExecutionModel.Requirement.REQUIRED;
-        authenticationRequirement(session, realm, type, requirement);
+        setOrReplaceAuthenticationRequirement(session, realm, type, requirement, null);
     }
 
     public static void setAlternativeCredential(KeycloakSession session, String type, RealmModel realm) {
         AuthenticationExecutionModel.Requirement requirement = AuthenticationExecutionModel.Requirement.ALTERNATIVE;
-        authenticationRequirement(session, realm, type, requirement);
+        setOrReplaceAuthenticationRequirement(session, realm, type, requirement, null);
     }
 
-    public static void authenticationRequirement(KeycloakSession session, RealmModel realm, String type, AuthenticationExecutionModel.Requirement requirement) {
+    public static void setOrReplaceAuthenticationRequirement(KeycloakSession session, RealmModel realm, String type, AuthenticationExecutionModel.Requirement requirement, AuthenticationExecutionModel.Requirement currentRequirement) {
         for (AuthenticationFlowModel flow : realm.getAuthenticationFlows()) {
             for (AuthenticationExecutionModel execution : realm.getAuthenticationExecutions(flow.getId())) {
                 String providerId = execution.getAuthenticator();
                 ConfigurableAuthenticatorFactory factory = getConfigurableAuthenticatorFactory(session, providerId);
                 if (factory == null) continue;
                 if (type.equals(factory.getReferenceCategory())) {
-                    execution.setRequirement(requirement);
+                    if (currentRequirement == null || currentRequirement.equals(execution.getRequirement())) {
-                    realm.updateAuthenticatorExecution(execution);
+                        execution.setRequirement(requirement);
+                        realm.updateAuthenticatorExecution(execution);
+                        logger.debugf("Authenticator execution '%s' switched to '%s'", execution.getAuthenticator(), requirement.toString());
+                    } else {
+                        logger.debugf("Skip switch authenticator execution '%s' to '%s' as it's in state %s", execution.getAuthenticator(), requirement.toString(), execution.getRequirement());
+                    }
                 }
             }
         }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserFederationTest.java

@@ -271,6 +271,44 @@ public class UserFederationTest extends AbstractAdminTest {
         removeUserFederationProvider(id);
     }
 
+    @Test
+    public void testKerberosAuthenticatorChangedOnlyIfDisabled() {
+        // Change kerberos to REQUIRED
+        AuthenticationExecutionInfoRepresentation kerberosExecution = findKerberosExecution();
+        kerberosExecution.setRequirement(AuthenticationExecutionModel.Requirement.REQUIRED.toString());
+        realm.flows().updateExecutions("browser", kerberosExecution);
+        assertAdminEvents.assertEvent(realmId, OperationType.UPDATE, AdminEventPaths.authUpdateExecutionPath("browser"), kerberosExecution);
+
+        // create LDAP provider with kerberos
+        UserFederationProviderRepresentation ldapRep = UserFederationProviderBuilder.create()
+                .displayName("ldap2")
+                .providerName("ldap")
+                .priority(2)
+                .configProperty(KerberosConstants.ALLOW_KERBEROS_AUTHENTICATION, "true")
+                .build();
+        String id = createUserFederationProvider(ldapRep);
+
+        // Assert kerberos authenticator still REQUIRED
+        kerberosExecution = findKerberosExecution();
+        Assert.assertEquals(kerberosExecution.getRequirement(), AuthenticationExecutionModel.Requirement.REQUIRED.toString());
+
+        // update LDAP provider with kerberos
+        ldapRep = userFederation().get(id).toRepresentation();
+        userFederation().get(id).update(ldapRep);
+        assertAdminEvents.assertEvent(realmId, OperationType.UPDATE, AdminEventPaths.userFederationResourcePath(id), ldapRep);
+
+        // Assert kerberos authenticator still REQUIRED
+        kerberosExecution = findKerberosExecution();
+        Assert.assertEquals(kerberosExecution.getRequirement(), AuthenticationExecutionModel.Requirement.REQUIRED.toString());
+
+        // Cleanup
+        kerberosExecution.setRequirement(AuthenticationExecutionModel.Requirement.DISABLED.toString());
+        realm.flows().updateExecutions("browser", kerberosExecution);
+        assertAdminEvents.assertEvent(realmId, OperationType.UPDATE, AdminEventPaths.authUpdateExecutionPath("browser"), kerberosExecution);
+        removeUserFederationProvider(id);
+
+    }
+
 
     @Test (expected = NotFoundException.class)
     public void testLookupNotExistentProvider() {