diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UserFederationProvidersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UserFederationProvidersResource.java index eff1de7edd..3afa1e99af 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UserFederationProvidersResource.java +++ b/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; } diff --git a/services/src/main/java/org/keycloak/utils/CredentialHelper.java b/services/src/main/java/org/keycloak/utils/CredentialHelper.java index ab2a17e9c0..7db006ad56 100755 --- a/services/src/main/java/org/keycloak/utils/CredentialHelper.java +++ b/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); - realm.updateAuthenticatorExecution(execution); + if (currentRequirement == null || currentRequirement.equals(execution.getRequirement())) { + 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()); + } } } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserFederationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserFederationTest.java index 90b14a2108..2018e27ff5 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserFederationTest.java +++ b/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() {