EYCLOAK-12741 Add name and description edit functionality to Authenti…

…cation and Execution Flows

core/src/main/java/org/keycloak/representations/idm/AuthenticationExecutionInfoRepresentation.java

@@ -30,6 +30,7 @@ public class AuthenticationExecutionInfoRepresentation implements Serializable {
     protected String requirement;
     protected String displayName;
     protected String alias;
+    protected String description;
     protected List<String> requirementChoices;
     protected Boolean configurable;
     protected Boolean authenticationFlow;
@@ -63,6 +64,14 @@ public void setAlias(String alias) {
         this.alias = alias;
     }
 
+    public String getDescription() {
+        return description;
+    }
+
+    public void setDescription(String description) {
+        this.description = description;
+    }
+
     public String getRequirement() {
         return requirement;
     }

integration/admin-client/src/main/java/org/keycloak/admin/client/resource/AuthenticationManagementResource.java

@@ -88,6 +88,11 @@ public interface AuthenticationManagementResource {
     @Consumes(MediaType.APPLICATION_JSON)
     Response copy(@PathParam("flowAlias") String flowAlias, Map<String, String> data);
 
+    @Path("/flows/{id}")
+    @PUT
+    @Consumes(MediaType.APPLICATION_JSON)
+    void updateFlow(@PathParam("id") String id, AuthenticationFlowRepresentation flow);
+
     @Path("/flows/{flowAlias}/executions/flow")
     @POST
     @Consumes(MediaType.APPLICATION_JSON)

services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java

@@ -255,6 +255,7 @@ public AuthenticationFlowRepresentation getFlow(@PathParam("id") String id) {
     @PUT
     @NoCache
     @Consumes(MediaType.APPLICATION_JSON)
+    @Produces(MediaType.APPLICATION_JSON)
     public Response updateFlow(@PathParam("id") String id, AuthenticationFlowRepresentation flow) {
         auth.realm().requireManageRealm();
 
@@ -264,10 +265,32 @@ public Response updateFlow(@PathParam("id") String id, AuthenticationFlowReprese
             return ErrorResponse.exists("Failed to update flow with empty alias name");
         }
 
+        //check if updating a correct flow
+        AuthenticationFlowModel checkFlow = realm.getAuthenticationFlowById(id);
+        if (checkFlow == null) {
+            session.getTransactionManager().setRollbackOnly();
+            throw new NotFoundException("Illegal execution");
+        }
+
+        //if a different flow with the same name does already exist, throw an exception
+        if (realm.getFlowByAlias(flow.getAlias()) != null && !checkFlow.getAlias().equals(flow.getAlias())) {
+            return ErrorResponse.exists("Flow alias name already exists");
+        }
+
+        //if the name changed
+        if (!checkFlow.getAlias().equals(flow.getAlias())) {
+            checkFlow.setAlias(flow.getAlias());
+        }
+
+        //check if the description changed
+        if (!checkFlow.getDescription().equals(flow.getDescription())) {
+            checkFlow.setDescription(flow.getDescription());
+        }
+
+        //update the flow
         flow.setId(existingFlow.getId());
         realm.updateAuthenticationFlow(RepresentationToModel.toModel(flow));
         adminEvent.operation(OperationType.UPDATE).resourcePath(session.getContext().getUri()).representation(flow).success();
-
         return Response.accepted(flow).build();
     }
 
@@ -533,6 +556,7 @@ public void recurseExecutions(AuthenticationFlowModel flow, List<AuthenticationE
                     rep.getRequirementChoices().add(AuthenticationExecutionModel.Requirement.DISABLED.name());
                 }
                 rep.setDisplayName(flowRef.getAlias());
+                rep.setDescription(flowRef.getDescription());
                 rep.setConfigurable(false);
                 rep.setId(execution.getId());
                 rep.setAuthenticationFlow(execution.isAuthenticatorFlow());
@@ -571,16 +595,16 @@ public void recurseExecutions(AuthenticationFlowModel flow, List<AuthenticationE
     }
 
     /**
-     * Update authentication executions of a flow
-     *
+     * Update authentication executions of a Flow
      * @param flowAlias Flow alias
-     * @param rep
+     * @param rep AuthenticationExecutionInfoRepresentation
      */
     @Path("/flows/{flowAlias}/executions")
     @PUT
     @NoCache
+    @Produces(MediaType.APPLICATION_JSON)
     @Consumes(MediaType.APPLICATION_JSON)
-    public void updateExecutions(@PathParam("flowAlias") String flowAlias, AuthenticationExecutionInfoRepresentation rep) {
+    public Response updateExecutions(@PathParam("flowAlias") String flowAlias, AuthenticationExecutionInfoRepresentation rep) {
         auth.realm().requireManageRealm();
 
         AuthenticationFlowModel flow = realm.getFlowByAlias(flowAlias);
@@ -599,7 +623,38 @@ public void updateExecutions(@PathParam("flowAlias") String flowAlias, Authentic
             model.setRequirement(AuthenticationExecutionModel.Requirement.valueOf(rep.getRequirement()));
             realm.updateAuthenticatorExecution(model);
             adminEvent.operation(OperationType.UPDATE).resource(ResourceType.AUTH_EXECUTION).resourcePath(session.getContext().getUri()).representation(rep).success();
+            return Response.accepted(flow).build();
         }
+
+        //executions can't have name and description updated
+        if (rep.getAuthenticationFlow() == null) { return Response.accepted(flow).build();}
+
+        //check if updating a correct flow
+        AuthenticationFlowModel checkFlow = realm.getAuthenticationFlowById(rep.getFlowId());
+        if (checkFlow == null) {
+            session.getTransactionManager().setRollbackOnly();
+            throw new NotFoundException("Illegal execution");
+        }
+
+        //if a different flow with the same name does already exist, throw an exception
+        if (realm.getFlowByAlias(rep.getDisplayName()) != null && !checkFlow.getAlias().equals(rep.getDisplayName())) {
+            return ErrorResponse.exists("Flow alias name already exists");
+        }
+
+        //if the name changed
+        if (!checkFlow.getAlias().equals(rep.getDisplayName())) {
+            checkFlow.setAlias(rep.getDisplayName());
+        }
+
+        //check if the description changed
+        if (!checkFlow.getDescription().equals(rep.getDescription())) {
+            checkFlow.setDescription(rep.getDescription());
+        }
+
+        //update the flow
+        realm.updateAuthenticationFlow(checkFlow);
+        adminEvent.operation(OperationType.UPDATE).resource(ResourceType.AUTH_EXECUTION).resourcePath(session.getContext().getUri()).representation(rep).success();
+        return Response.accepted(flow).build();
     }
 
     /**

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/ExecutionTest.java

@@ -32,6 +32,8 @@
 import org.keycloak.representations.idm.AuthenticationFlowRepresentation;
 import org.keycloak.representations.idm.AuthenticatorConfigRepresentation;
 import org.keycloak.testsuite.admin.ApiUtil;
+import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude;
+import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer;
 import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
 import org.keycloak.testsuite.util.AdminEventPaths;
 import org.keycloak.testsuite.util.AssertAdminEvents;
@@ -44,8 +46,6 @@
 import java.util.Map;
 
 import static org.hamcrest.Matchers.hasItems;
-import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude;
-import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer;
 
 /**
  * @author <a href="mailto:mstrukel@redhat.com">Marko Strukelj</a>
@@ -286,9 +286,9 @@ public void testClientFlowExecutions() {
         }
 
         // Update execution with not-existent ID - SHOULD FAIL
+        AuthenticationExecutionInfoRepresentation executionRep2 = new AuthenticationExecutionInfoRepresentation();
+        executionRep2.setId("not-existent");
         try {
-            AuthenticationExecutionInfoRepresentation executionRep2 = new AuthenticationExecutionInfoRepresentation();
-            executionRep2.setId("not-existent");
             authMgmtResource.updateExecutions("new-client-flow", executionRep2);
             Assert.fail("Not expected to update not-existent execution");
         } catch (NotFoundException nfe) {

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/authentication/FlowTest.java

@@ -19,25 +19,25 @@
 
 import org.junit.Assert;
 import org.junit.Test;
-
 import org.keycloak.events.admin.OperationType;
 import org.keycloak.events.admin.ResourceType;
 import org.keycloak.representations.idm.AuthenticationExecutionExportRepresentation;
+import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation;
 import org.keycloak.representations.idm.AuthenticationFlowRepresentation;
-import org.keycloak.testsuite.admin.ApiUtil;
 import org.keycloak.testsuite.util.AdminEventPaths;
-import org.keycloak.testsuite.util.AssertAdminEvents;
 
 import javax.ws.rs.BadRequestException;
+import javax.ws.rs.ClientErrorException;
 import javax.ws.rs.NotFoundException;
 import javax.ws.rs.core.Response;
+import javax.ws.rs.core.Response.Status;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import javax.ws.rs.core.Response.Status;
 
-import static org.hamcrest.Matchers.*;
-import static org.keycloak.testsuite.util.Matchers.*;
+import static org.hamcrest.Matchers.containsString;
+import static org.keycloak.testsuite.util.Matchers.body;
+import static org.keycloak.testsuite.util.Matchers.statusCodeIs;
 
 /**
  * @author <a href="mailto:mstrukel@redhat.com">Marko Strukelj</a>
@@ -253,6 +253,7 @@ public void testCopyFlow() {
         copyOfBrowser = authMgmtResource.getFlow(copyOfBrowser.getId());
         Assert.assertNotNull(copyOfBrowser);
         compareFlows(browser, copyOfBrowser);
+        authMgmtResource.deleteFlow(copyOfBrowser.getId());
     }
 
     @Test
@@ -275,4 +276,138 @@ public void addExecutionFlow() {
         assertAdminEvents.assertEvent(REALM_NAME, OperationType.CREATE, AdminEventPaths.authAddExecutionFlowPath("parent"), params, ResourceType.AUTH_EXECUTION_FLOW);
     }
 
+    @Test
+    //KEYCLOAK-12741
+    //test editing of authentication flows
+    public void editFlowTest() {
+        List<AuthenticationFlowRepresentation> flows;
+
+        //copy an existing one first
+        HashMap<String, String> params = new HashMap<>();
+        params.put("newName", "Copy of browser");
+        Response response = authMgmtResource.copy("browser", params);
+        assertAdminEvents.assertEvent(REALM_NAME, OperationType.CREATE, AdminEventPaths.authCopyFlowPath("browser"), params, ResourceType.AUTH_FLOW);
+        try {
+            Assert.assertEquals("Copy flow", 201, response.getStatus());
+        } finally {
+            response.close();
+        }
+
+        //load the newly copied flow
+        flows = authMgmtResource.getFlows();
+        AuthenticationFlowRepresentation testFlow = findFlowByAlias("Copy of browser", flows);
+        //Set a new unique name. Should succeed
+        testFlow.setAlias("Copy of browser2");
+        authMgmtResource.updateFlow(testFlow.getId(), testFlow);
+        assertAdminEvents.assertEvent(REALM_NAME, OperationType.UPDATE, AdminEventPaths.authEditFlowPath(testFlow.getId()), ResourceType.AUTH_FLOW);
+        flows = authMgmtResource.getFlows();
+        Assert.assertEquals("Copy of browser2", findFlowByAlias("Copy of browser2", flows).getAlias());
+
+        //Create new flow and edit the old one to have the new ones name
+        AuthenticationFlowRepresentation newFlow = newFlow("New Flow", "Test description", "basic-flow", true, false);
+        createFlow(newFlow);
+        // check that new flow is returned in a children list
+        flows = authMgmtResource.getFlows();
+        AuthenticationFlowRepresentation found = findFlowByAlias("New Flow", flows);
+
+        Assert.assertNotNull("created flow visible in parent", found);
+        compareFlows(newFlow, found);
+
+        //try to update old flow with alias that already exists
+        testFlow.setAlias("New Flow");
+        try {
+            authMgmtResource.updateFlow(found.getId(), testFlow);
+        } catch (ClientErrorException exception){
+            //expoected
+        }
+        flows = authMgmtResource.getFlows();
+
+        //name should be the same for the old Flow
+        Assert.assertEquals("Copy of browser2", findFlowByAlias("Copy of browser2", flows).getAlias());
+
+        //Only update the description
+        found.setDescription("New description");
+        authMgmtResource.updateFlow(found.getId(), found);
+        flows = authMgmtResource.getFlows();
+
+        Assert.assertEquals("New description", findFlowByAlias("New Flow", flows).getDescription());
+        assertAdminEvents.assertEvent(REALM_NAME, OperationType.UPDATE, AdminEventPaths.authEditFlowPath(found.getId()), ResourceType.AUTH_FLOW);
+
+        //Update name and description
+        found.setAlias("New Flow2");
+        found.setDescription("New description2");
+        authMgmtResource.updateFlow(found.getId(), found);
+        flows = authMgmtResource.getFlows();
+
+        Assert.assertEquals("New Flow2", findFlowByAlias("New Flow2", flows).getAlias());
+        Assert.assertEquals("New description2", findFlowByAlias("New Flow2", flows).getDescription());
+        assertAdminEvents.assertEvent(REALM_NAME, OperationType.UPDATE, AdminEventPaths.authEditFlowPath(found.getId()), ResourceType.AUTH_FLOW);
+        Assert.assertNull(findFlowByAlias("New Flow", flows));
+
+        authMgmtResource.deleteFlow(testFlow.getId());
+        authMgmtResource.deleteFlow(found.getId());
+    }
+
+    @Test
+    public void editExecutionFlowTest() {
+        HashMap<String, String> params = new HashMap<>();
+        List<AuthenticationExecutionInfoRepresentation> executionReps;
+        //create new parent flow
+        AuthenticationFlowRepresentation newFlow = newFlow("Parent-Flow", "This is a parent flow", "basic-flow", true, false);
+        createFlow(newFlow);
+
+        //create a child sub flow
+        params.put("alias", "Child-Flow");
+        params.put("description", "This is a child flow");
+        params.put("provider", "registration-page-form");
+        params.put("type", "basic-flow");
+
+        authMgmtResource.addExecutionFlow("Parent-Flow", params);
+        assertAdminEvents.assertEvent(REALM_NAME, OperationType.CREATE, AdminEventPaths.authAddExecutionFlowPath("Parent-Flow"), params, ResourceType.AUTH_EXECUTION_FLOW);
+
+        executionReps = authMgmtResource.getExecutions("Parent-Flow");
+
+        //create another with the same name of the previous one. Should fail to create
+        params = new HashMap<>();
+        params.put("alias", "Child-Flow");
+        params.put("description", "This is another child flow");
+        params.put("provider", "registration-page-form");
+        params.put("type", "basic-flow");
+
+        try {
+            authMgmtResource.addExecutionFlow("Parent-Flow", params);
+            Assert.fail("addExecutionFlow the alias already exist");
+        } catch (Exception expected) {
+            // Expected
+        }
+
+        AuthenticationExecutionInfoRepresentation found = executionReps.get(0);
+        found.setDisplayName("Parent-Flow");
+
+        try {
+            authMgmtResource.updateExecutions("Parent-Flow", found);
+        } catch (ClientErrorException exception){
+            //expected
+        }
+
+        //edit both name and description
+        found.setDisplayName("Child-Flow2");
+        found.setDescription("This is another child flow2");
+
+        authMgmtResource.updateExecutions("Parent-Flow", found);
+        assertAdminEvents.assertEvent(REALM_NAME, OperationType.UPDATE, AdminEventPaths.authUpdateExecutionPath("Parent-Flow"), ResourceType.AUTH_EXECUTION);
+        executionReps = authMgmtResource.getExecutions("Parent-Flow");
+        Assert.assertEquals("Child-Flow2", executionReps.get(0).getDisplayName());
+        Assert.assertEquals("This is another child flow2", executionReps.get(0).getDescription());
+
+        //edit only description
+        found.setDescription("This is another child flow3");
+        authMgmtResource.updateExecutions("Parent-Flow", found);
+
+        assertAdminEvents.assertEvent(REALM_NAME, OperationType.UPDATE, AdminEventPaths.authUpdateExecutionPath("Parent-Flow"), ResourceType.AUTH_EXECUTION);
+        executionReps = authMgmtResource.getExecutions("Parent-Flow");
+        Assert.assertEquals("Child-Flow2", executionReps.get(0).getDisplayName());
+        Assert.assertEquals("This is another child flow3", executionReps.get(0).getDescription());
+    }
+
 }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/AdminEventPaths.java

@@ -390,6 +390,11 @@ public static String authCopyFlowPath(String flowAlias) {
         return uri.toString();
     }
 
+    public static String authEditFlowPath(String flowId) {
+        URI uri = UriBuilder.fromUri(authMgmtBasePath()).path(AuthenticationManagementResource.class, "updateFlow")
+        .build(flowId);
+        return uri.toString();
+    }
     public static String authAddExecutionFlowPath(String flowAlias) {
         URI uri = UriBuilder.fromUri(authMgmtBasePath()).path(AuthenticationManagementResource.class, "addExecutionFlow")
                 .build(flowAlias);

themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties

@@ -1150,6 +1150,7 @@ cut=Cut
 paste=Paste
 create-group=Create group
 create-authenticator-execution=Create Authenticator Execution
+edit-flow=Edit Flow
 create-form-action-execution=Create Form Action Execution
 create-top-level-form=Create Top Level Form
 flow.alias.tooltip=Specifies display name for the flow.
@@ -1284,6 +1285,7 @@ started=Started
 logout-all-sessions=Log out all sessions
 logout=Logout
 new-name=New Name
+new-description=New Description
 ok=Ok
 attributes=Attributes
 role-mappings=Role Mappings