From 2dcd05134e3fd81074236b75cf62738120e8f361 Mon Sep 17 00:00:00 2001 From: Anil Date: Tue, 7 May 2024 00:43:57 -0700 Subject: [PATCH] Improve alternative step display in FlowDiagram (#28699) (#28700) * Refactor FlowDiagram to explicitly link concurrent groupings Signed-off-by: Anil Dhurjaty * Remove subflow nodes from FlowDiagram Signed-off-by: Anil Dhurjaty * Remove disabled steps and subflows from FlowDiagram Signed-off-by: Anil Dhurjaty * Change alternative steps to be serial w/ labels Signed-off-by: Anil Dhurjaty --------- Signed-off-by: Anil Dhurjaty --- .../cypress/e2e/authentication_test.spec.ts | 11 +- .../__tests__/FlowDiagram.test.tsx | 264 +++++++++++++--- .../authentication/components/FlowDiagram.tsx | 298 +++++++++--------- 3 files changed, 373 insertions(+), 200 deletions(-) diff --git a/js/apps/admin-ui/cypress/e2e/authentication_test.spec.ts b/js/apps/admin-ui/cypress/e2e/authentication_test.spec.ts index 5e3ef71684..bad706ac78 100644 --- a/js/apps/admin-ui/cypress/e2e/authentication_test.spec.ts +++ b/js/apps/admin-ui/cypress/e2e/authentication_test.spec.ts @@ -220,16 +220,13 @@ describe("Authentication test", () => { diagramView.edgesExist([ { from: "Start", to: "Cookie" }, { from: "Cookie", to: "End" }, - { from: "Start", to: "Kerberos" }, - { from: "Kerberos", to: "End" }, - { from: "Start", to: "Identity Provider Redirector" }, + { from: "Cookie", to: "Identity Provider Redirector" }, { from: "Identity Provider Redirector", to: "End" }, - { from: "Start", to: "Start forms" }, - { from: "Start forms", to: "Username Password Form" }, + { from: "Identity Provider Redirector", to: "Username Password Form" }, { from: "Username Password Form", to: "Condition - user configured" }, { from: "Condition - user configured", to: "OTP Form" }, - { from: "Condition - user configured", to: "End forms" }, - { from: "End forms", to: "End" }, + { from: "Condition - user configured", to: "End" }, + { from: "OTP Form", to: "End" }, ]); }); }); diff --git a/js/apps/admin-ui/src/authentication/__tests__/FlowDiagram.test.tsx b/js/apps/admin-ui/src/authentication/__tests__/FlowDiagram.test.tsx index 99b2a2184e..daf6f57c2e 100644 --- a/js/apps/admin-ui/src/authentication/__tests__/FlowDiagram.test.tsx +++ b/js/apps/admin-ui/src/authentication/__tests__/FlowDiagram.test.tsx @@ -144,7 +144,7 @@ describe("", () => { const expectedEdges = [ "Edge from start to alt1", "Edge from alt1 to end", - "Edge from start to alt2", + "Edge from alt1 to alt2", "Edge from alt2 to end", ]; testHelper.expectEdgeLabels(expectedEdges); @@ -178,24 +178,15 @@ describe("", () => { const { container } = render(); const testHelper = reactFlowTester(container); + const expectedNodes = ["start", "requiredElement", "subElement", "end"]; + testHelper.expectNodeIds(expectedNodes); + const expectedEdges = [ "Edge from start to requiredElement", - "Edge from requiredElement to subflow", - "Edge from subflow to subElement", - "Edge from subElement to flow-end-subflow", - "Edge from flow-end-subflow to end", + "Edge from requiredElement to subElement", + "Edge from subElement to end", ]; testHelper.expectEdgeLabels(expectedEdges); - - const expectedNodes = [ - "start", - "requiredElement", - "subflow", - "subElement", - "flow-end-subflow", - "end", - ]; - testHelper.expectNodeIds(expectedNodes); }); it("should render a flow with a subflow with alternative steps", () => { @@ -231,22 +222,18 @@ describe("", () => { const testHelper = reactFlowTester(container); const expectedEdges = [ "Edge from start to requiredElement", - "Edge from requiredElement to subflow", - "Edge from subflow to subElement1", - "Edge from subElement1 to flow-end-subflow", - "Edge from subflow to subElement2", - "Edge from subElement2 to flow-end-subflow", - "Edge from flow-end-subflow to end", + "Edge from requiredElement to subElement1", + "Edge from subElement1 to end", + "Edge from subElement1 to subElement2", + "Edge from subElement2 to end", ]; testHelper.expectEdgeLabels(expectedEdges); const expectedNodes = [ "start", "requiredElement", - "subflow", "subElement1", "subElement2", - "flow-end-subflow", "end", ]; testHelper.expectNodeIds(expectedNodes); @@ -291,12 +278,10 @@ describe("", () => { const testHelper = reactFlowTester(container); const expectedEdges = [ "Edge from start to requiredElement", - "Edge from requiredElement to subflow", - "Edge from subflow to subElement1", - "Edge from subElement1 to flow-end-subflow", - "Edge from subflow to subElement2", - "Edge from subElement2 to flow-end-subflow", - "Edge from flow-end-subflow to finalStep", + "Edge from requiredElement to subElement1", + "Edge from subElement1 to finalStep", + "Edge from subElement1 to subElement2", + "Edge from subElement2 to finalStep", "Edge from finalStep to end", ]; testHelper.expectEdgeLabels(expectedEdges); @@ -304,10 +289,8 @@ describe("", () => { const expectedNodes = [ "start", "requiredElement", - "subflow", "subElement1", "subElement2", - "flow-end-subflow", "finalStep", "end", ]; @@ -357,6 +340,17 @@ describe("", () => { const { container } = render(); const testHelper = reactFlowTester(container); + const expectedNodes = [ + "start", + "chooseUser", + "sendReset", + "conditionOtpConfigured", + "otpForm", + "resetPassword", + "end", + ]; + testHelper.expectNodeIds(expectedNodes); + const expectedEdges = [ "Edge from start to chooseUser", "Edge from chooseUser to sendReset", @@ -437,37 +431,207 @@ describe("", () => { const { container } = render(); const testHelper = reactFlowTester(container); - const expectedEdges = [ - "Edge from start to exampleForms", - "Edge from exampleForms to usernamePasswordForm", - "Edge from usernamePasswordForm to conditionUserConfigured", - "Edge from conditionUserConfigured to conditionUserAttribute", - "Edge from conditionUserConfigured to flow-end-exampleForms", - "Edge from conditionUserAttribute to otpForm", - "Edge from conditionUserAttribute to flow-end-exampleForms", - "Edge from otpForm to confirmLink", - "Edge from confirmLink to flow-end-exampleForms", - "Edge from flow-end-exampleForms to end", - "Edge from start to conditionLoa", - "Edge from conditionLoa to reviewProfile", - "Edge from conditionLoa to end", - "Edge from reviewProfile to end", - ]; - testHelper.expectEdgeLabels(expectedEdges); const expectedNodes = [ "start", - "exampleForms", "usernamePasswordForm", "conditionUserConfigured", "conditionUserAttribute", "otpForm", "confirmLink", - "flow-end-exampleForms", "conditionLoa", "reviewProfile", "end", ]; testHelper.expectNodeIds(expectedNodes); + + const expectedEdges = [ + "Edge from start to usernamePasswordForm", + "Edge from usernamePasswordForm to conditionUserConfigured", + "Edge from conditionUserConfigured to conditionUserAttribute", + "Edge from conditionUserConfigured to end", + "Edge from conditionUserAttribute to otpForm", + "Edge from conditionUserAttribute to end", + "Edge from otpForm to confirmLink", + "Edge from confirmLink to end", + "Edge from usernamePasswordForm to conditionLoa", + "Edge from conditionLoa to reviewProfile", + "Edge from conditionLoa to end", + "Edge from reviewProfile to end", + ]; + testHelper.expectEdgeLabels(expectedEdges); + }); + + it("should render the default first broker login flow", () => { + const executionList = new ExecutionList([ + { + id: "reviewProfile", + displayName: "Review Profile", + requirement: "REQUIRED", + level: 0, + }, + { + id: "createOrLink", + displayName: "User creation or linking", + requirement: "REQUIRED", + level: 0, + }, + { + id: "createUnique", + displayName: "Create User If Unique", + requirement: "ALTERNATIVE", + level: 1, + }, + { + id: "existingAccount", + displayName: "Handle Existing Account", + requirement: "ALTERNATIVE", + level: 1, + }, + { + id: "confirmLink", + displayName: "Confirm link existing account", + requirement: "REQUIRED", + level: 2, + }, + { + id: "accountVerification", + displayName: "Account verification options", + requirement: "REQUIRED", + level: 2, + }, + { + id: "emailVerify", + displayName: "Verify existing account by Email", + requirement: "ALTERNATIVE", + level: 3, + }, + { + id: "reauthVerify", + displayName: "Verify Existing Account by Re-authentication", + requirement: "ALTERNATIVE", + level: 3, + }, + { + id: "usernamePassword", + displayName: + "Username Password Form for identity provider reauthentication", + requirement: "REQUIRED", + level: 4, + }, + { + id: "conditionalOtp", + displayName: "First broker login - Conditional OTP", + requirement: "CONDITIONAL", + level: 4, + }, + { + id: "conditionUserConfigured", + displayName: "Condition - user configured", + requirement: "REQUIRED", + level: 5, + }, + { + id: "otpForm", + displayName: "OTP Form", + requirement: "REQUIRED", + level: 5, + }, + ]); + + const { container } = render(); + + const testHelper = reactFlowTester(container); + + const expectedNodes = [ + "start", + "reviewProfile", + "createUnique", + "confirmLink", + "usernamePassword", + "conditionUserConfigured", + "otpForm", + "emailVerify", + "end", + ]; + testHelper.expectNodeIds(expectedNodes); + + const expectedEdges = [ + "Edge from start to reviewProfile", + "Edge from reviewProfile to createUnique", + "Edge from createUnique to confirmLink", + "Edge from createUnique to end", + "Edge from confirmLink to emailVerify", + "Edge from emailVerify to usernamePassword", + "Edge from usernamePassword to conditionUserConfigured", + "Edge from conditionUserConfigured to otpForm", + "Edge from conditionUserConfigured to end", + "Edge from otpForm to end", + "Edge from emailVerify to end", + ]; + testHelper.expectEdgeLabels(expectedEdges); + }); + + it("should hide disabled steps", () => { + const executionList = new ExecutionList([ + { + id: "disabled", + displayName: "Disabled", + requirement: "DISABLED", + }, + { + id: "required", + displayName: "Required", + requirement: "REQUIRED", + }, + ]); + + const { container } = render(); + + const testHelper = reactFlowTester(container); + + const expectedNodes = ["start", "required", "end"]; + testHelper.expectNodeIds(expectedNodes); + + const expectedEdges = [ + "Edge from start to required", + "Edge from required to end", + ]; + testHelper.expectEdgeLabels(expectedEdges); + }); + + it("should hide disabled subflow", () => { + const executionList = new ExecutionList([ + { + id: "required", + displayName: "Required", + requirement: "REQUIRED", + level: 0, + }, + { + id: "subflow", + displayName: "Subflow", + requirement: "DISABLED", + level: 0, + }, + { + id: "subElement", + displayName: "Sub Element", + requirement: "REQUIRED", + level: 1, + }, + ]); + + const { container } = render(); + + const testHelper = reactFlowTester(container); + const expectedNodes = ["start", "required", "end"]; + testHelper.expectNodeIds(expectedNodes); + + const expectedEdges = [ + "Edge from start to required", + "Edge from required to end", + ]; + testHelper.expectEdgeLabels(expectedEdges); }); }); diff --git a/js/apps/admin-ui/src/authentication/components/FlowDiagram.tsx b/js/apps/admin-ui/src/authentication/components/FlowDiagram.tsx index e87d14e354..7063714df7 100644 --- a/js/apps/admin-ui/src/authentication/components/FlowDiagram.tsx +++ b/js/apps/admin-ui/src/authentication/components/FlowDiagram.tsx @@ -29,19 +29,44 @@ type FlowDiagramProps = { executionList: ExecutionList; }; -type ConditionLabel = "true" | "false"; +type ConditionLabel = "true" | "false" | "success" | "attempted"; const nodeTypes = { conditional: ConditionalNode, startSubFlow: StartSubFlowNode, endSubFlow: EndSubFlowNode, -} as const; +}; -type NodeType = keyof typeof nodeTypes; +const inOutClasses = new Map([ + ["input", "keycloak__authentication__input_node"], + ["output", "keycloak__authentication__output_node"], +]); + +type NodeType = + | "conditional" + | "startSubFlow" + | "endSubFlow" + | "input" + | "output"; + +type IntermediateFlowResult = { + startId: string; + nodes: Node[]; + edges: Edge[]; + nextLinkFns: ((id: string) => Edge)[]; +}; + +function pairwise(fn: (x: T, y: T) => U, arr: T[]): U[] { + const result: U[] = []; + for (let index = 0; index < arr.length - 1; index++) { + result.push(fn(arr[index], arr[index + 1])); + } + return result; +} const isBypassable = (execution: ExpandableExecution) => execution.requirement === "ALTERNATIVE" || - execution.requirement === "DISABLED"; + execution.requirement === "CONDITIONAL"; const createEdge = ( fromNode: string, @@ -71,190 +96,177 @@ const createNode = ( return { id: ex.id!, type: nodeType, - sourcePosition: Position.Right, - targetPosition: Position.Left, + sourcePosition: nodeType === "output" ? undefined : Position.Right, + targetPosition: nodeType === "input" ? undefined : Position.Left, data: { label: ex.displayName! }, position: { x: 0, y: 0 }, + className: inOutClasses.get(nodeType || ""), }; }; -const renderSubFlowNodes = (execution: ExpandableExecution): Node[] => { - const nodes: Node[] = []; - - if (execution.requirement !== "CONDITIONAL") { - nodes.push(createNode(execution, "startSubFlow")); - - const endSubFlowId = `flow-end-${execution.id}`; - nodes.push( - createNode( - { - id: endSubFlowId, - displayName: execution.displayName!, - }, - "endSubFlow", - ), - ); - } - - return nodes.concat(renderFlowNodes(execution.executionList || [])); -}; - -const renderFlowNodes = (executionList: ExpandableExecution[]): Node[] => { - let elements: Node[] = []; - +const consecutiveBypassableFlows = ( + executionList: ExpandableExecution[], +): ExpandableExecution[] => { + const result = []; for (let index = 0; index < executionList.length; index++) { const execution = executionList[index]; - if (execution.executionList) { - elements = elements.concat(renderSubFlowNodes(execution)); - } else { - elements.push( - createNode( - execution, - providerConditionFilter(execution) ? "conditional" : undefined, - ), - ); + if (!isBypassable(execution)) { + break; } + result.push(execution); } - - return elements; + return result; }; -const renderSubFlowEdges = ( +const borderStep = ( + node: Node, + continuing: boolean = true, +): IntermediateFlowResult => ({ + startId: node.id, + nodes: [node], + edges: [], + nextLinkFns: continuing ? [(id: string) => createEdge(node.id, id)] : [], +}); + +const renderSubFlow = ( execution: ExpandableExecution, - flowEndId: string, -): { startId: string; edges: Edge[]; endId: string } => { +): IntermediateFlowResult => { if (!execution.executionList) throw new Error("Execution list is required for subflow"); - if (execution.requirement === "CONDITIONAL") { - const startId = execution.executionList![0].id!; + const graph = createGraph(createConcurrentGroupings(execution.executionList)); - return { - startId: startId, - edges: renderFlowEdges(startId, execution.executionList!, flowEndId), - endId: execution.executionList![execution.executionList!.length - 1].id!, - }; - } - const elements: Edge[] = []; - const subFlowEndId = `flow-end-${execution.id}`; - - return { - startId: execution.id!, - edges: elements.concat( - renderFlowEdges(execution.id!, execution.executionList!, subFlowEndId), - ), - endId: subFlowEndId, - }; + graph.nextLinkFns.push( + ...execution.executionList + .filter((e) => providerConditionFilter(e)) + .map((e) => (id: string) => createEdge(e.id!, id, "false")), + ); + return graph; }; -const renderFlowEdges = ( - startId: string, +const groupConcurrentSteps = ( executionList: ExpandableExecution[], - endId: string, -): Edge[] => { - let elements: Edge[] = []; - let prevExecutionId = startId; - let isLastExecutionBypassable = false; - const conditionals = []; +): ExpandableExecution[] => { + const executions = consecutiveBypassableFlows(executionList); + if (executions.length > 0) { + return executions; + } + return [executionList[0]]; +}; - for (let index = 0; index < executionList.length; index++) { - const execution = executionList[index]; - let executionId = execution.id!; - const isPrevConditional = - conditionals[conditionals.length - 1] === prevExecutionId; - const connectToPrevious = (id: string) => - elements.push( - createEdge(prevExecutionId, id, isPrevConditional ? "true" : undefined), - ); - - if (providerConditionFilter(execution)) { - conditionals.push(executionId); - } - if (startId === executionId) { - continue; - } +const createConcurrentSteps = ( + executionList: ExpandableExecution[], +): IntermediateFlowResult[] => { + if (executionList.length === 0) { + return []; + } + const executions = groupConcurrentSteps(executionList); + return executions.map((execution) => { if (execution.executionList) { - const nextRequired = - executionList.slice(index + 1).find((e) => !isBypassable(e))?.id ?? - endId; - const { - startId: subFlowStartId, - edges, - endId: subflowEndId, - } = renderSubFlowEdges(execution, nextRequired); - - connectToPrevious(subFlowStartId); - elements = elements.concat(edges); - executionId = subflowEndId; - } else { - connectToPrevious(executionId); + return renderSubFlow(execution); } - const isExecutionBypassable = isBypassable(execution); + const isConditional = providerConditionFilter(execution); + const edgeLabel = (() => { + if (isConditional) { + return "true"; + } + if (execution.requirement === "ALTERNATIVE") { + return "success"; + } + })(); - if (isExecutionBypassable) { - elements.push(createEdge(executionId, endId)); - } else { - prevExecutionId = executionId; - } + return { + startId: execution.id!, + nodes: [createNode(execution, isConditional ? "conditional" : undefined)], + edges: [], + nextLinkFns: [(id: string) => createEdge(execution.id!, id, edgeLabel)], + }; + }); +}; - isLastExecutionBypassable = isExecutionBypassable; +const createConcurrentGroupings = ( + executionList: ExpandableExecution[], +): IntermediateFlowResult[][] => { + if (executionList.length === 0) { + return []; + } + const steps = createConcurrentSteps(executionList); + return [ + steps, + ...createConcurrentGroupings(executionList.slice(steps.length)), + ]; +}; + +const createGraph = ( + groupings: IntermediateFlowResult[][], +): IntermediateFlowResult => { + const nodes: Node[] = []; + const edges: Edge[] = []; + let nextLinkFns: ((id: string) => Edge)[] = []; + + for (const group of groupings) { + nodes.push(...group.flatMap((g) => g.nodes)); + edges.push( + ...group.flatMap((g) => g.edges), + ...nextLinkFns.map((fn) => fn(group[0].startId)), + ...pairwise( + (prev, current) => + createEdge(prev.startId, current.startId, "attempted"), + group, + ), + ); + nextLinkFns = group.flatMap((g) => g.nextLinkFns); } - // subflows with conditionals automatically connect to the end, so don't do it twice - if (!isLastExecutionBypassable && conditionals.length === 0) { - elements.push(createEdge(prevExecutionId, endId)); - } - elements = elements.concat( - conditionals.map((id) => createEdge(id, endId, "false")), - ); - - return elements; + return { + startId: groupings[0][0].startId, + nodes, + edges, + nextLinkFns, + }; }; const edgeTypes: ButtonEdges = { buttonEdge: ButtonEdge, }; -function renderNodes(expandableList: ExpandableExecution[]) { - return getLayoutedNodes([ - { - id: "start", - sourcePosition: Position.Right, - type: "input", - data: { label: "Start" }, - position: { x: 0, y: 0 }, - className: "keycloak__authentication__input_node", - }, - { - id: "end", - targetPosition: Position.Left, - type: "output", - data: { label: "End" }, - position: { x: 0, y: 0 }, - className: "keycloak__authentication__output_node", - }, - ...renderFlowNodes(expandableList), - ]); -} +function renderGraph(executionList: ExpandableExecution[]): [Node[], Edge[]] { + const executionListNoDisabled = executionList.filter( + (e) => e.requirement !== "DISABLED", + ); + const groupings = [ + [borderStep(createNode({ id: "start", displayName: "Start" }, "input"))], + ...createConcurrentGroupings(executionListNoDisabled), + [ + borderStep( + createNode({ id: "end", displayName: "End" }, "output"), + false, + ), + ], + ]; -function renderEdges(expandableList: ExpandableExecution[]): Edge[] { - return getLayoutedEdges(renderFlowEdges("start", expandableList, "end")); + const { nodes, edges } = createGraph(groupings); + + return [getLayoutedNodes(nodes), getLayoutedEdges(edges)]; } export const FlowDiagram = ({ executionList: { expandableList }, }: FlowDiagramProps) => { const [expandDrawer, setExpandDrawer] = useState(false); - const initialNodes = useMemo(() => renderNodes(expandableList), []); - const initialEdges = useMemo(() => renderEdges(expandableList), []); + const [initialNodes, initialEdges] = useMemo( + () => renderGraph(expandableList), + [], + ); const [nodes, setNodes, onNodesChange] = useNodesState(initialNodes); const [edges, setEdges, onEdgesChange] = useEdgesState(initialEdges); useUpdateEffect(() => { - setNodes(renderNodes(expandableList)); - setEdges(renderEdges(expandableList)); + const [nodes, edges] = renderGraph(expandableList); + setNodes(nodes); + setEdges(edges); }, [expandableList]); const onInit = (reactFlowInstance: ReactFlowInstance) =>