libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

Merge pull request #3510 from ssilvert/delete-subflows

Browse source 
KEYCLOAK-3681: Delete top flow doesn't remove all subflows
This commit is contained in:
Marek Posolda 2016-11-18 08:50:13 +01:00 committed by GitHub
commit b434c2b9cf
2 changed files with 37 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
services/src/main/java/org/keycloak/services/resources/admin/AuthenticationManagementResource.java
View file

@ -252,6 +252,10 @@ public class AuthenticationManagementResource {
public void deleteFlow(@PathParam("id") String id) { public void deleteFlow(@PathParam("id") String id) {
auth.requireManage(); auth.requireManage();
deleteFlow(id, true);
}
private void deleteFlow(String id, boolean isTopMostLevel) {
AuthenticationFlowModel flow = realm.getAuthenticationFlowById(id); AuthenticationFlowModel flow = realm.getAuthenticationFlowById(id);
if (flow == null) { if (flow == null) {
throw new NotFoundException("Could not find flow with id"); throw new NotFoundException("Could not find flow with id");
@ -259,18 +263,17 @@ public class AuthenticationManagementResource {
if (flow.isBuiltIn()) { if (flow.isBuiltIn()) {
throw new BadRequestException("Can't delete built in flow"); throw new BadRequestException("Can't delete built in flow");
} }
List<AuthenticationExecutionModel> executions = realm.getAuthenticationExecutions(id); List<AuthenticationExecutionModel> executions = realm.getAuthenticationExecutions(id);
for (AuthenticationExecutionModel execution : executions) { for (AuthenticationExecutionModel execution : executions) {
if(execution.getFlowId() != null) { if(execution.getFlowId() != null) {
AuthenticationFlowModel nonTopLevelFlow = realm.getAuthenticationFlowById(execution.getFlowId()); deleteFlow(execution.getFlowId(), false);
realm.removeAuthenticationFlow(nonTopLevelFlow);
} }
realm.removeAuthenticatorExecution(execution);
} }
realm.removeAuthenticationFlow(flow); realm.removeAuthenticationFlow(flow);
// Use just one event for top-level flow. Using separate events won't work properly for flows of depth 2 or bigger // Use just one event for top-level flow. Using separate events won't work properly for flows of depth 2 or bigger
adminEvent.operation(OperationType.DELETE).resourcePath(uriInfo).success(); if (isTopMostLevel) adminEvent.operation(OperationType.DELETE).resourcePath(uriInfo).success();
} }
/** /**

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

@ -37,6 +37,33 @@ import java.util.Map;
*/ */
public class FlowTest extends AbstractAuthenticationTest { public class FlowTest extends AbstractAuthenticationTest {
// KEYCLOAK-3681: Delete top flow doesn't delete all subflows
@Test
public void testRemoveSubflows() {
authMgmtResource.createFlow(newFlow("Foo", "Foo flow", "generic", true, false));
addFlowToParent("Foo", "child");
addFlowToParent("child", "grandchild");
List<AuthenticationFlowRepresentation> flows = authMgmtResource.getFlows();
AuthenticationFlowRepresentation found = findFlowByAlias("Foo", flows);
authMgmtResource.deleteFlow(found.getId());
authMgmtResource.createFlow(newFlow("Foo", "Foo flow", "generic", true, false));
addFlowToParent("Foo", "child");
// Under the old code, this would throw an error because "grandchild"
// was left in the database
addFlowToParent("child", "grandchild");
}
private void addFlowToParent(String parentAlias, String childAlias) {
Map<String, String> data = new HashMap<>();
data.put("alias", childAlias);
data.put("type", "generic");
data.put("description", childAlias + " flow");
authMgmtResource.addExecutionFlow(parentAlias, data);
}
@Test @Test
public void testAddRemoveFlow() { public void testAddRemoveFlow() {