fix: accounting for the possibility of null flows from existing realms

closes: keycloak#23980 Signed-off-by: Steve Hawkins <shawkins@redhat.com>
miquelsi · Mar 8, 2024 · 4091baf · 4091baf
1 parent 7695532
commit 4091baf
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 36 deletions.
diff --git a/...rage-private/src/main/java/org/keycloak/storage/datastore/DefaultExportImportManager.java b/...rage-private/src/main/java/org/keycloak/storage/datastore/DefaultExportImportManager.java
@@ -1288,38 +1288,35 @@ private static WebAuthnPolicy getWebAuthnPolicyPasswordless(RealmRepresentation
     }
     public static Map<String, String> importAuthenticationFlows(KeycloakSession session, RealmModel newRealm, RealmRepresentation rep) {
         Map<String, String> mappedFlows = new HashMap<>();
-        if (rep.getAuthenticationFlows() == null) {
-            // assume this is an old version being imported
-            DefaultAuthenticationFlows.migrateFlows(newRealm);
-        } else {
-            if (rep.getAuthenticatorConfig() != null) {
-                for (AuthenticatorConfigRepresentation configRep : rep.getAuthenticatorConfig()) {
-                    if (configRep.getAlias() == null) {
-                        // this can happen only during import json files from keycloak 3.4.0 and older
-                        throw new IllegalStateException("Provided realm contains authenticator config with null alias. "
-                                + "It should be resolved by adding alias to the authenticator config before exporting the realm.");
-                    }
-                    AuthenticatorConfigModel model = RepresentationToModel.toModel(configRep);
-                    newRealm.addAuthenticatorConfig(model);
-                }
-            }
-            if (rep.getAuthenticationFlows() != null) {
-                for (AuthenticationFlowRepresentation flowRep : rep.getAuthenticationFlows()) {
-                    AuthenticationFlowModel model = RepresentationToModel.toModel(flowRep);
-                    String previousId = model.getId();
-                    model = newRealm.addAuthenticationFlow(model);
-                    // store the mapped ids so that clients can reference the correct flow when importing the authenticationFlowBindingOverrides
-                    mappedFlows.put(previousId, model.getId());
+
+        if (rep.getAuthenticatorConfig() != null) {
+            for (AuthenticatorConfigRepresentation configRep : rep.getAuthenticatorConfig()) {
+                if (configRep.getAlias() == null) {
+                    // this can happen only during import json files from keycloak 3.4.0 and older
+                    throw new IllegalStateException("Provided realm contains authenticator config with null alias. "
+                            + "It should be resolved by adding alias to the authenticator config before exporting the realm.");
                 }
-                for (AuthenticationFlowRepresentation flowRep : rep.getAuthenticationFlows()) {
-                    AuthenticationFlowModel model = newRealm.getFlowByAlias(flowRep.getAlias());
-                    for (AuthenticationExecutionExportRepresentation exeRep : flowRep.getAuthenticationExecutions()) {
-                        AuthenticationExecutionModel execution = toModel(session, newRealm, model, exeRep);
-                        newRealm.addAuthenticatorExecution(execution);
-                    }
+                AuthenticatorConfigModel model = RepresentationToModel.toModel(configRep);
+                newRealm.addAuthenticatorConfig(model);
+            }
+        }
+        if (rep.getAuthenticationFlows() != null) {
+            for (AuthenticationFlowRepresentation flowRep : rep.getAuthenticationFlows()) {
+                AuthenticationFlowModel model = RepresentationToModel.toModel(flowRep);
+                String previousId = model.getId();
+                model = newRealm.addAuthenticationFlow(model);
+                // store the mapped ids so that clients can reference the correct flow when importing the authenticationFlowBindingOverrides
+                mappedFlows.put(previousId, model.getId());
+            }
+            for (AuthenticationFlowRepresentation flowRep : rep.getAuthenticationFlows()) {
+                AuthenticationFlowModel model = newRealm.getFlowByAlias(flowRep.getAlias());
+                for (AuthenticationExecutionExportRepresentation exeRep : flowRep.getAuthenticationExecutions()) {
+                    AuthenticationExecutionModel execution = toModel(session, newRealm, model, exeRep);
+                    newRealm.addAuthenticatorExecution(execution);
                 }
             }
         }
+        DefaultAuthenticationFlows.migrateFlows(newRealm);
         if (rep.getBrowserFlow() == null) {
             AuthenticationFlowModel defaultFlow = newRealm.getFlowByAlias(DefaultAuthenticationFlows.BROWSER_FLOW);
             if (defaultFlow != null) {

diff --git a/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/model/AuthenticationMapper.java b/rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/model/AuthenticationMapper.java
@@ -43,7 +43,7 @@ public static Authentication convertToModel(AuthenticationFlowModel flow, RealmM
 
         final List<String> useAsDefault = Stream.of(realm.getBrowserFlow(), realm.getRegistrationFlow(), realm.getDirectGrantFlow(),
                         realm.getResetCredentialsFlow(), realm.getClientAuthenticationFlow(), realm.getDockerAuthenticationFlow(), realm.getFirstBrokerLoginFlow())
-                .filter(f -> flow.getAlias().equals(f.getAlias())).map(AuthenticationFlowModel::getAlias).collect(Collectors.toList());
+                .filter(f -> f != null && flow.getAlias().equals(f.getAlias())).map(AuthenticationFlowModel::getAlias).collect(Collectors.toList());
 
         if (!useAsDefault.isEmpty()) {
             authentication.setUsedBy(new UsedBy(UsedBy.UsedByType.DEFAULT, useAsDefault));

diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/DefaultAuthenticationFlows.java b/server-spi-private/src/main/java/org/keycloak/models/utils/DefaultAuthenticationFlows.java
@@ -605,13 +605,15 @@ public static void firstBrokerLoginFlow(RealmModel realm, boolean migrate) {
             if (browserFlow == null) {
                 browserFlow = realm.getFlowByAlias(DefaultAuthenticationFlows.BROWSER_FLOW);
             }
-            List<AuthenticationExecutionModel> browserExecutions = new LinkedList<>();
-            KeycloakModelUtils.deepFindAuthenticationExecutions(realm, browserFlow, browserExecutions);
-            for (AuthenticationExecutionModel browserExecution : browserExecutions) {
-                if (browserExecution.isAuthenticatorFlow()){
-                    if (realm.getAuthenticationExecutionsStream(browserExecution.getFlowId())
-                            .anyMatch(e -> e.getAuthenticator().equals("auth-otp-form"))){
-                        execution.setRequirement(browserExecution.getRequirement());
+            if (browserFlow != null) {
+                List<AuthenticationExecutionModel> browserExecutions = new LinkedList<>();
+                KeycloakModelUtils.deepFindAuthenticationExecutions(realm, browserFlow, browserExecutions);
+                for (AuthenticationExecutionModel browserExecution : browserExecutions) {
+                    if (browserExecution.isAuthenticatorFlow()){
+                        if (realm.getAuthenticationExecutionsStream(browserExecution.getFlowId())
+                                .anyMatch(e -> e.getAuthenticator().equals("auth-otp-form"))){
+                            execution.setRequirement(browserExecution.getRequirement());
+                        }
                     }
                 }
             }

diff --git a/...illian/tests/base/src/test/java/org/keycloak/testsuite/exportimport/ExportImportTest.java b/...illian/tests/base/src/test/java/org/keycloak/testsuite/exportimport/ExportImportTest.java
@@ -257,6 +257,13 @@ public void testImportFromPartialExport() {
         addTestRealmToTestRealmReps("import-without-clients");
     }
 
+    @Test
+    public void testImportFromRealmWithPartialAuthenticationFlows() {
+        // import a realm with no built-in authentication flows
+        importRealmFromFile("/import/partial-authentication-flows-import.json");
+        Assert.assertTrue("Imported realm hasn't been found!", isRealmPresent("partial-authentication-flows-import"));
+    }
+
     @Test
     public void testImportWithNullAuthenticatorConfigAndNoDefaultBrowserFlow() {
         importRealmFromFile("/import/testrealm-authenticator-config-null.json");

diff --git a/...-arquillian/tests/base/src/test/resources/import/partial-authentication-flows-import.json b/...-arquillian/tests/base/src/test/resources/import/partial-authentication-flows-import.json
@@ -0,0 +1,23 @@
+{
+  "enabled": true,
+  "realm": "partial-authentication-flows-import",
+  "authenticationFlows": [
+    {
+      "alias": "X.509 browser",
+      "description": "Browser-based authentication",
+      "providerId": "basic-flow",
+      "topLevel": true,
+      "builtIn": false,
+      "authenticationExecutions": [
+        {
+          "requirement": "ALTERNATIVE",
+          "priority": 10,
+          "authenticator": "auth-cookie",
+          "authenticatorFlow": false,
+          "autheticatorFlow": false,
+          "userSetupAllowed": false
+        }
+      ]
+    }
+  ]
+}