Fallback to no override flow when missing in client override

Closes #30765

Signed-off-by: Lex Cao <lexcao@foxmail.com>

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

@@ -16,10 +16,10 @@
  */
 package org.keycloak.models.utils;
 
+import org.jboss.logging.Logger;
 import org.keycloak.models.AuthenticationFlowBindings;
 import org.keycloak.models.AuthenticationFlowModel;
 import org.keycloak.models.ClientModel;
-import org.keycloak.models.ModelException;
 import org.keycloak.sessions.AuthenticationSessionModel;
 
 /**
@@ -28,16 +28,19 @@ import org.keycloak.sessions.AuthenticationSessionModel;
  */
 public class AuthenticationFlowResolver {
 
+    private static final Logger logger = Logger.getLogger(AuthenticationFlowResolver.class);
+
     public static AuthenticationFlowModel resolveBrowserFlow(AuthenticationSessionModel authSession) {
         AuthenticationFlowModel flow = null;
         ClientModel client = authSession.getClient();
         String clientFlow = client.getAuthenticationFlowBindingOverride(AuthenticationFlowBindings.BROWSER_BINDING);
         if (clientFlow != null) {
             flow = authSession.getRealm().getAuthenticationFlowById(clientFlow);
-            if (flow == null) {
+            if (flow != null) {
-                throw new ModelException("Client " + client.getClientId() + " has browser flow override, but this flow does not exist");
+                return flow;
             }
-            return flow;
+            logger.warnf("Client %s has browser flow override, but this flow '%s' does not exist, " +
+                    "fallback to browser flow", client.getClientId(), clientFlow);
         }
         return authSession.getRealm().getBrowserFlow();
     }
@@ -47,10 +50,11 @@ public class AuthenticationFlowResolver {
         String clientFlow = client.getAuthenticationFlowBindingOverride(AuthenticationFlowBindings.DIRECT_GRANT_BINDING);
         if (clientFlow != null) {
             flow = authSession.getRealm().getAuthenticationFlowById(clientFlow);
-            if (flow == null) {
+            if (flow != null) {
-                throw new ModelException("Client " + client.getClientId() + " has direct grant flow override, but this flow does not exist");
+                return flow;
             }
-            return flow;
+            logger.warnf("Client %s has direct grant flow override, but this flow '%s' does not exist, " +
+                    "fallback to direct grant flow", client.getClientId(), clientFlow);
         }
         return authSession.getRealm().getDirectGrantFlow();
     }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/FlowOverrideTest.java

@@ -23,6 +23,7 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.keycloak.OAuth2Constants;
+import org.keycloak.admin.client.resource.ClientResource;
 import org.keycloak.admin.client.resource.ClientsResource;
 import org.keycloak.authentication.authenticators.browser.UsernamePasswordFormFactory;
 import org.keycloak.common.Profile;
@@ -33,16 +34,19 @@ import org.keycloak.models.AuthenticationFlowModel;
 import org.keycloak.models.ClientModel;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.utils.TimeBasedOTP;
+import org.keycloak.representations.idm.AuthenticationFlowRepresentation;
 import org.keycloak.representations.idm.ClientRepresentation;
 import org.keycloak.representations.idm.RealmRepresentation;
-import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
 import org.keycloak.testsuite.AssertEvents;
+import org.keycloak.testsuite.admin.ApiUtil;
 import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
 import org.keycloak.testsuite.arquillian.annotation.UncaughtServerErrorExpected;
 import org.keycloak.testsuite.authentication.PushButtonAuthenticatorFactory;
 import org.keycloak.testsuite.pages.AppPage;
 import org.keycloak.testsuite.pages.ErrorPage;
 import org.keycloak.testsuite.pages.LoginPage;
+import org.keycloak.testsuite.util.AdminClientUtil;
+import org.keycloak.testsuite.util.FlowUtil;
 import org.keycloak.util.BasicAuthHelper;
 import org.openqa.selenium.By;
 
@@ -53,16 +57,18 @@ import jakarta.ws.rs.core.Form;
 import jakarta.ws.rs.core.HttpHeaders;
 import jakarta.ws.rs.core.Response;
 import java.util.List;
+import java.util.Map;
+import java.util.function.Consumer;
 
 import static org.junit.Assert.assertEquals;
-import org.keycloak.testsuite.util.AdminClientUtil;
+import static org.keycloak.testsuite.forms.BrowserFlowTest.revertFlows;
 
 /**
  * Test that clients can override auth flows
  *
  * @author <a href="mailto:bburke@redhat.com">Bill Burke</a>
  */
-public class FlowOverrideTest extends AbstractTestRealmKeycloakTest {
+public class FlowOverrideTest extends AbstractFlowTest {
 
     public static final String TEST_APP_DIRECT_OVERRIDE = "test-app-direct-override";
     public static final String TEST_APP_FLOW = "test-app-flow";
@@ -217,6 +223,11 @@ public class FlowOverrideTest extends AbstractTestRealmKeycloakTest {
         events.expectLogin().client("test-app-flow").detail(Details.USERNAME, "test-user@localhost").assertEvent();
     }
 
+    @Test
+    public void testRemovedFlowOverrideByClientThenFallbackToToNoOverrideBrowserFlow() {
+        testWithRemovedFlowOverrideByClient(AuthenticationFlowBindings.BROWSER_BINDING, this::testNoOverrideBrowser);
+    }
+
     // TODO remove this once DYNAMIC_SCOPES feature is enabled by default
     @Test
     @EnableFeature(value = Profile.Feature.DYNAMIC_SCOPES, skipRestart = true)
@@ -254,6 +265,11 @@ public class FlowOverrideTest extends AbstractTestRealmKeycloakTest {
         testDirectGrantNoOverride("test-app");
     }
 
+    @Test
+    public void testRemovedFlowOverrideByClientThenFallbackToNoOverrideDirectGrantFlow() {
+        testWithRemovedFlowOverrideByClient(AuthenticationFlowBindings.DIRECT_GRANT_BINDING, this::testNoOverrideBrowser);
+    }
+
     private void testDirectGrantNoOverride(String clientId) {
         Client httpClient = AdminClientUtil.createResteasyClient();
         String grantUri = oauth.getResourceOwnerPasswordCredentialGrantUrl();
@@ -371,4 +387,29 @@ public class FlowOverrideTest extends AbstractTestRealmKeycloakTest {
         Assert.assertEquals(browserFlowId, clientRep.getAuthenticationFlowBindingOverrides().get(AuthenticationFlowBindings.BROWSER_BINDING));
 
     }
+
+    private void testWithRemovedFlowOverrideByClient(String binding, Consumer<String> testNoOverride) {
+        ClientResource clientResource = ApiUtil.findClientByClientId(testRealm(), "test-app");
+        Assert.assertNotNull(clientResource);
+        ClientRepresentation clientRep = clientResource.toRepresentation();
+
+        String newFlowAlias = "Copy of Browser Flow";
+        testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session).copyBrowserFlow(newFlowAlias));
+
+        try {
+            // 1. set a flow override for client
+            AuthenticationFlowRepresentation newFlow = findFlowByAlias(newFlowAlias);
+            clientRep.setAuthenticationFlowBindingOverrides(Map.of(binding, newFlow.getId()));
+            clientResource.update(clientRep);
+
+            // 2. remove the flow
+            revertFlows(testRealm(), newFlowAlias);
+
+            // 3. login with client
+            testNoOverride.accept(clientRep.getClientId());
+        } finally {
+            clientRep.setAuthenticationFlowBindingOverrides(Map.of(binding, ""));
+            clientResource.update(clientRep);
+        }
+    }
 }