Check the authentication config exists before returning its reference

Closes keycloak#34888 Signed-off-by: rmartinc <[email protected]>
ebremer · Nov 18, 2024 · 8d559d5 · 8d559d5
1 parent 8ebe900
commit 8d559d5
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 3 deletions.
diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java
@@ -930,7 +930,9 @@ public static AuthenticationExecutionExportRepresentation toRepresentation(Keycl
         AuthenticationExecutionExportRepresentation rep = new AuthenticationExecutionExportRepresentation();
         if (model.getAuthenticatorConfig() != null) {
             AuthenticatorConfigModel config = new DeployedConfigurationsManager(session).getAuthenticatorConfig(realm, model.getAuthenticatorConfig());
-            rep.setAuthenticatorConfig(config.getAlias());
+            if (config != null) {
+                rep.setAuthenticatorConfig(config.getAlias());
+            }
         }
         rep.setAuthenticator(model.getAuthenticator());
         rep.setAuthenticatorFlow(model.isAuthenticatorFlow());

diff --git a/...src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java b/...src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java
@@ -663,6 +663,19 @@ public List<AuthenticationExecutionInfoRepresentation> getExecutions(@Parameter(
         return result;
     }
 
+    private String getAuthenticationConfig(String flowAlias, AuthenticationExecutionModel model) {
+        if (model.getAuthenticatorConfig() == null) {
+            return null;
+        }
+        AuthenticatorConfigModel config = new DeployedConfigurationsManager(session).getAuthenticatorConfig(realm, model.getAuthenticatorConfig());
+        if (config == null) {
+            logger.warnf("Authenticator configuration '%s' is missing for execution '%s' (%s) in flow '%s'",
+                    model.getAuthenticatorConfig(), model.getId(), model.getAuthenticator(), flowAlias);
+            return null;
+        }
+        return config.getId();
+    }
+
     public void recurseExecutions(AuthenticationFlowModel flow, List<AuthenticationExecutionInfoRepresentation> result, int level) {
         AtomicInteger index = new AtomicInteger(0);
         realm.getAuthenticationExecutionsStream(flow.getId()).forEachOrdered(execution -> {
@@ -682,7 +695,7 @@ public void recurseExecutions(AuthenticationFlowModel flow, List<AuthenticationE
                     rep.getRequirementChoices().add(AuthenticationExecutionModel.Requirement.REQUIRED.name());
                     rep.getRequirementChoices().add(AuthenticationExecutionModel.Requirement.DISABLED.name());
                     rep.setProviderId(execution.getAuthenticator());
-                    rep.setAuthenticationConfig(execution.getAuthenticatorConfig());
+                    rep.setAuthenticationConfig(getAuthenticationConfig(flow.getAlias(), execution));
                 } else if (AuthenticationFlow.CLIENT_FLOW.equals(flowRef.getProviderId())) {
                     rep.getRequirementChoices().add(AuthenticationExecutionModel.Requirement.ALTERNATIVE.name());
                     rep.getRequirementChoices().add(AuthenticationExecutionModel.Requirement.REQUIRED.name());
@@ -732,7 +745,7 @@ public void recurseExecutions(AuthenticationFlowModel flow, List<AuthenticationE
                 }
 
                 rep.setProviderId(providerId);
-                rep.setAuthenticationConfig(execution.getAuthenticatorConfig());
+                rep.setAuthenticationConfig(getAuthenticationConfig(flow.getAlias(), execution));
                 result.add(rep);
             }
         });

diff --git a/...se/src/test/java/org/keycloak/testsuite/admin/authentication/AuthenticatorConfigTest.java b/...se/src/test/java/org/keycloak/testsuite/admin/authentication/AuthenticatorConfigTest.java
@@ -23,6 +23,9 @@
 import org.keycloak.authentication.authenticators.broker.IdpDetectExistingBrokerUserAuthenticatorFactory;
 import org.keycloak.events.admin.OperationType;
 import org.keycloak.events.admin.ResourceType;
+import org.keycloak.models.AuthenticatorConfigModel;
+import org.keycloak.models.RealmModel;
+import org.keycloak.representations.idm.AuthenticationExecutionExportRepresentation;
 import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation;
 import org.keycloak.representations.idm.AuthenticationFlowRepresentation;
 import org.keycloak.representations.idm.AuthenticatorConfigInfoRepresentation;
@@ -221,6 +224,36 @@ public void testDuplicateAuthenticatorConfigAlias() {
         Assert.assertEquals(404, nfe.getResponse().getStatus());
     }
 
+    @Test
+    public void testMissingConfig() {
+        AuthenticatorConfigRepresentation cfg = newConfig("foo", IdpCreateUserIfUniqueAuthenticatorFactory.REQUIRE_PASSWORD_UPDATE_AFTER_REGISTRATION, "true");
+        final String cfgId = createConfig(executionId, cfg);
+        final String realmId = testRealmId;
+        AuthenticatorConfigRepresentation cfgRep = authMgmtResource.getAuthenticatorConfig(cfgId);
+        Assert.assertNotNull(cfgRep);
+
+        testingClient.server().run(session -> {
+            // emulating a broken config id, remove the config but do not remove the link in the authenticator
+            RealmModel realm = session.realms().getRealm(realmId);
+            AuthenticatorConfigModel config = realm.getAuthenticatorConfigById(cfgId);
+            realm.removeAuthenticatorConfig(config);
+        });
+
+        // check the flow can be read and execution has no config
+        AuthenticationFlowRepresentation flow = authMgmtResource.getFlow(flowId);
+        AuthenticationExecutionExportRepresentation execExport = flow.getAuthenticationExecutions().stream()
+                .filter(ae -> IdpCreateUserIfUniqueAuthenticatorFactory.PROVIDER_ID.equals(ae.getAuthenticator()))
+                .findAny()
+                .orElse(null);
+        Assert.assertNotNull(execExport);
+        Assert.assertNull(execExport.getAuthenticatorConfig());
+
+        // check the execution can be read with no configuration assigned
+        AuthenticationExecutionInfoRepresentation execInfo = findExecutionByProvider(
+                IdpCreateUserIfUniqueAuthenticatorFactory.PROVIDER_ID, authMgmtResource.getExecutions("firstBrokerLogin2"));
+        Assert.assertNull(execInfo.getAuthenticationConfig());
+    }
+
     private String createConfig(String executionId, AuthenticatorConfigRepresentation cfg) {
         try (Response resp = authMgmtResource.newExecutionConfig(executionId, cfg)) {
             Assert.assertEquals(201, resp.getStatus());