From 4d8d6f8cd85ed1f8f03201e6b4c9164197004bac Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 3 Apr 2023 13:23:38 +0200 Subject: [PATCH] Preserve authentication flow IDs after import closes #9564 --- .../datastore/LegacyExportImportManager.java | 2 -- .../map/datastore/MapExportImportManager.java | 2 -- .../models/utils/RepresentationToModel.java | 1 + .../exportimport/ExportImportTest.java | 25 +++++++++++++++++ .../import/import-without-roles.json | 28 +++++++++---------- 5 files changed, 40 insertions(+), 18 deletions(-) diff --git a/model/legacy-private/src/main/java/org/keycloak/storage/datastore/LegacyExportImportManager.java b/model/legacy-private/src/main/java/org/keycloak/storage/datastore/LegacyExportImportManager.java index 2bf8bfaca8..e03b62b55c 100644 --- a/model/legacy-private/src/main/java/org/keycloak/storage/datastore/LegacyExportImportManager.java +++ b/model/legacy-private/src/main/java/org/keycloak/storage/datastore/LegacyExportImportManager.java @@ -1271,9 +1271,7 @@ public class LegacyExportImportManager implements ExportImportManager { if (rep.getAuthenticationFlows() != null) { for (AuthenticationFlowRepresentation flowRep : rep.getAuthenticationFlows()) { AuthenticationFlowModel model = RepresentationToModel.toModel(flowRep); - // make sure new id is generated for new AuthenticationFlowModel instance String previousId = model.getId(); - model.setId(null); 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()); diff --git a/model/map/src/main/java/org/keycloak/models/map/datastore/MapExportImportManager.java b/model/map/src/main/java/org/keycloak/models/map/datastore/MapExportImportManager.java index 4c4cfa911e..e1ea8c3662 100644 --- a/model/map/src/main/java/org/keycloak/models/map/datastore/MapExportImportManager.java +++ b/model/map/src/main/java/org/keycloak/models/map/datastore/MapExportImportManager.java @@ -1454,9 +1454,7 @@ public class MapExportImportManager implements ExportImportManager { if (rep.getAuthenticationFlows() != null) { for (AuthenticationFlowRepresentation flowRep : rep.getAuthenticationFlows()) { AuthenticationFlowModel model = RepresentationToModel.toModel(flowRep); - // make sure new id is generated for new AuthenticationFlowModel instance String previousId = model.getId(); - model.setId(null); 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()); diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java index 7f62779da9..75eb0896da 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java @@ -946,6 +946,7 @@ public class RepresentationToModel { public static AuthenticatorConfigModel toModel(AuthenticatorConfigRepresentation rep) { AuthenticatorConfigModel model = new AuthenticatorConfigModel(); + model.setId(rep.getId()); model.setAlias(rep.getAlias()); model.setConfig(removeEmptyString(rep.getConfig())); return model; 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 bbec812adb..a01df00356 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 @@ -31,6 +31,7 @@ import org.keycloak.exportimport.dir.DirExportProvider; import org.keycloak.exportimport.dir.DirExportProviderFactory; import org.keycloak.exportimport.singlefile.SingleFileExportProviderFactory; import org.keycloak.models.UserModel; +import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation; import org.keycloak.representations.idm.ComponentRepresentation; import org.keycloak.representations.idm.KeysMetadataRepresentation; import org.keycloak.representations.idm.RealmEventsConfigRepresentation; @@ -375,6 +376,8 @@ public class ExportImportTest extends AbstractKeycloakTest { testingClient.testing().exportImport().setAction(ExportImportConfig.ACTION_EXPORT); testingClient.testing().exportImport().setRealmName("test"); + String[] authFlowObjectIdsBeforeImport = getSomeAuthenticationFlowsObjectIds(); + testingClient.testing().exportImport().runExport(); List components = adminClient.realm("test").components().query(); @@ -418,6 +421,9 @@ public class ExportImportTest extends AbstractKeycloakTest { assertTrue(testRealmRealm.users().search("user-requiredWebAuthn").get(0) .getRequiredActions().get(0).equals(WebAuthnRegisterFactory.PROVIDER_ID)); + String[] authFlowObjectIdsAfterImport = getSomeAuthenticationFlowsObjectIds(); + // Test that IDs of authentication-flows (both top level and nested) and authenticationConfiguration was preserved + Assert.assertArrayEquals(authFlowObjectIdsBeforeImport, authFlowObjectIdsAfterImport); List componentsImported = adminClient.realm("test").components().query(); assertComponents(components, componentsImported); @@ -478,6 +484,25 @@ public class ExportImportTest extends AbstractKeycloakTest { } } + + // Get IDs of some objects (top authentication flow, nested authentication flow, authentication config) to be able to test if IDs are same after re-import + private String[] getSomeAuthenticationFlowsObjectIds() { + String firstBrokerLoginFlowID = adminClient.realm("test").flows().getFlows().stream() + .filter(flow -> "first broker login".equals(flow.getAlias())) + .findFirst().get().getId(); + + List authExecutions = adminClient.realm("test").flows().getExecutions("User creation or linking"); + Assert.assertEquals("idp-create-user-if-unique", authExecutions.get(0).getProviderId()); + + String authConfigId = authExecutions.get(0).getAuthenticationConfig(); + Assert.assertEquals("create unique user config", adminClient.realm("test").flows().getAuthenticatorConfig(authConfigId).getAlias()); + + String handleExistingAccountSubflowId = authExecutions.get(1).getFlowId(); + Assert.assertEquals("Handle Existing Account", adminClient.realm("test").flows().getFlow(handleExistingAccountSubflowId).getAlias()); + + return new String[] {firstBrokerLoginFlowID, handleExistingAccountSubflowId, authConfigId }; + } + private void clearExportImportProperties() { // Clear export/import properties after test Properties systemProps = System.getProperties(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/import/import-without-roles.json b/testsuite/integration-arquillian/tests/base/src/test/resources/import/import-without-roles.json index fb3b87f7fb..5f0c80091e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/import/import-without-roles.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/import/import-without-roles.json @@ -830,7 +830,7 @@ "supportedLocales": [], "authenticationFlows": [ { - "id": "aed29d4f-aba7-4992-a600-18c0a28c1fc3", + "id": "bed29d4f-aba7-4992-a600-18c0a28c1fc3", "alias": "Handle Existing Account", "description": "Handle what to do if there is existing account with same email/username like authenticated identity provider", "providerId": "basic-flow", @@ -861,7 +861,7 @@ ] }, { - "id": "d8b8f564-6d56-4171-ba36-a8922c6eae49", + "id": "e8b8f564-6d56-4171-ba36-a8922c6eae49", "alias": "Verify Existing Account by Re-authentication", "description": "Reauthentication of existing account", "providerId": "basic-flow", @@ -885,7 +885,7 @@ ] }, { - "id": "73dcb1e4-2c7c-4494-825d-f2677cbc114c", + "id": "83dcb1e4-2c7c-4494-825d-f2677cbc114c", "alias": "browser", "description": "browser based authentication", "providerId": "basic-flow", @@ -923,7 +923,7 @@ ] }, { - "id": "a0a80dc3-d473-468e-b6e8-f1d306c21360", + "id": "b0a80dc3-d473-468e-b6e8-f1d306c21360", "alias": "clients", "description": "Base authentication for clients", "providerId": "client-flow", @@ -954,7 +954,7 @@ ] }, { - "id": "91882f46-54be-4738-847a-32e849d53240", + "id": "a1882f46-54be-4738-847a-32e849d53240", "alias": "direct grant", "description": "OpenID Connect Resource Owner Grant", "providerId": "basic-flow", @@ -985,7 +985,7 @@ ] }, { - "id": "b727a208-587c-4f27-8f48-ba2a0d4effdd", + "id": "c727a208-587c-4f27-8f48-ba2a0d4effdd", "alias": "docker auth", "description": "Used by Docker clients to authenticate against the IDP", "providerId": "basic-flow", @@ -1002,7 +1002,7 @@ ] }, { - "id": "5a6ac775-4000-4ccf-9271-6cb599297d4b", + "id": "6a6ac775-4000-4ccf-9271-6cb599297d4b", "alias": "first broker login", "description": "Actions taken after first broker login with identity provider account, which is not yet linked to any Keycloak account", "providerId": "basic-flow", @@ -1035,7 +1035,7 @@ ] }, { - "id": "1a84808d-e0c7-4759-aee8-cf9229542429", + "id": "2a84808d-e0c7-4759-aee8-cf9229542429", "alias": "forms", "description": "Username, password, otp and other auth forms.", "providerId": "basic-flow", @@ -1059,7 +1059,7 @@ ] }, { - "id": "717f990a-1c46-464c-9051-5e0ae39d63db", + "id": "817f990a-1c46-464c-9051-5e0ae39d63db", "alias": "registration", "description": "registration flow", "providerId": "basic-flow", @@ -1077,7 +1077,7 @@ ] }, { - "id": "166fca50-7b69-4cd4-80eb-a569e87ff8a2", + "id": "266fca50-7b69-4cd4-80eb-a569e87ff8a2", "alias": "registration form", "description": "registration form", "providerId": "form-flow", @@ -1115,7 +1115,7 @@ ] }, { - "id": "a516cb39-8f6d-4d08-ac82-236377be6500", + "id": "b516cb39-8f6d-4d08-ac82-236377be6500", "alias": "reset credentials", "description": "Reset credentials for a user if they forgot their password or something", "providerId": "basic-flow", @@ -1153,7 +1153,7 @@ ] }, { - "id": "8b9ae730-11e0-451f-b693-e32f09415e42", + "id": "9b9ae730-11e0-451f-b693-e32f09415e42", "alias": "saml ecp", "description": "SAML ECP Profile Authentication Flow", "providerId": "basic-flow", @@ -1172,14 +1172,14 @@ ], "authenticatorConfig": [ { - "id": "a6d38dcd-7b53-4991-b4eb-c866ce3c5e70", + "id": "b6d38dcd-7b53-4991-b4eb-c866ce3c5e70", "alias": "create unique user config", "config": { "require.password.update.after.registration": "false" } }, { - "id": "7408f503-b929-422f-b52b-277cebda44ba", + "id": "8408f503-b929-422f-b52b-277cebda44ba", "alias": "review profile config", "config": { "update.profile.on.first.login": "missing"