From 4091baf4c2225c50a5bba177db9fb5a0bea43433 Mon Sep 17 00:00:00 2001 From: Steve Hawkins Date: Wed, 6 Mar 2024 12:47:19 -0500 Subject: [PATCH] fix: accounting for the possibility of null flows from existing realms closes: #23980 Signed-off-by: Steve Hawkins --- .../datastore/DefaultExportImportManager.java | 53 +++++++++---------- .../ui/rest/model/AuthenticationMapper.java | 2 +- .../utils/DefaultAuthenticationFlows.java | 16 +++--- .../exportimport/ExportImportTest.java | 7 +++ .../partial-authentication-flows-import.json | 23 ++++++++ 5 files changed, 65 insertions(+), 36 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/test/resources/import/partial-authentication-flows-import.json diff --git a/model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultExportImportManager.java b/model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultExportImportManager.java index eb34a47efc4b..69eb2af486b7 100644 --- a/model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultExportImportManager.java +++ b/model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultExportImportManager.java @@ -1288,38 +1288,35 @@ private static WebAuthnPolicy getWebAuthnPolicyPasswordless(RealmRepresentation } public static Map importAuthenticationFlows(KeycloakSession session, RealmModel newRealm, RealmRepresentation rep) { Map 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 index 4d32cc4040f6..93c2728ecbc0 100644 --- 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 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 index ab316f54ca19..14aaf1fe23f7 100755 --- 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 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 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/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/exportimport/ExportImportTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/exportimport/ExportImportTest.java index e9b6486f92de..67d12c8d7a04 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/exportimport/ExportImportTest.java +++ b/testsuite/integration-arquillian/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/testsuite/integration-arquillian/tests/base/src/test/resources/import/partial-authentication-flows-import.json b/testsuite/integration-arquillian/tests/base/src/test/resources/import/partial-authentication-flows-import.json new file mode 100644 index 000000000000..d1da7dbaacb6 --- /dev/null +++ b/testsuite/integration-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 + } + ] + } + ] +} \ No newline at end of file