diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/WorkflowDefinitionResourceIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/WorkflowDefinitionResourceIT.java index e5cccdfca07..f5e8eb93886 100644 --- a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/WorkflowDefinitionResourceIT.java +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/WorkflowDefinitionResourceIT.java @@ -9542,4 +9542,386 @@ public class WorkflowDefinitionResourceIT { LOG.warn("Cleanup error: {}", e.getMessage()); } } + + @Test + @Order(42) + void test_SelfApprovalPrevention(TestNamespace ns) throws IOException { + LOG.info("Starting test_SelfApprovalPrevention"); + + OpenMetadataClient client = SdkClients.adminClient(); + String uniqueSuffix = String.valueOf(System.currentTimeMillis()); + + // Step 1: Create three users + LOG.debug("Creating test users for self-approval prevention testing"); + + CreateUser createUser1 = + new CreateUser() + .withName("approver1_" + uniqueSuffix) + .withEmail("approver1_" + uniqueSuffix + "@example.com") + .withDisplayName("Test Approver 1") + .withIsAdmin(true); // Make user1 an admin so they can update reviewers + User user1 = client.users().create(createUser1); + LOG.debug("Created user 1: {}", user1.getName()); + + CreateUser createUser2 = + new CreateUser() + .withName("approver2_" + uniqueSuffix) + .withEmail("approver2_" + uniqueSuffix + "@example.com") + .withDisplayName("Test Approver 2"); + User user2 = client.users().create(createUser2); + LOG.debug("Created user 2: {}", user2.getName()); + + CreateUser createUser3 = + new CreateUser() + .withName("approver3_" + uniqueSuffix) + .withEmail("approver3_" + uniqueSuffix + "@example.com") + .withDisplayName("Test Approver 3"); + User user3 = client.users().create(createUser3); + LOG.debug("Created user 3: {}", user3.getName()); + + // Step 2: Create classification first (without reviewers) + LOG.debug("Creating initial classification"); + CreateClassification createClassification = + new CreateClassification() + .withName(ns.prefix("test_classification")) + .withDisplayName("Test Classification for Self-Approval") + .withDescription("Classification to test self-approval prevention"); + Classification classification = client.classifications().create(createClassification); + LOG.debug("Created initial classification: {}", classification.getName()); + + // Step 3: Create workflow for classification approval + LOG.debug("Creating workflow for classification approval testing"); + + String workflowJson = + """ + { + "name": "%s", + "displayName": "Self-Approval Prevention Test Workflow", + "description": "Workflow testing prevention of self-approval", + "trigger": { + "type": "eventBasedEntity", + "config": { + "entityTypes": ["classification"], + "events": ["Updated"], + "exclude": [], + "filter": {} + }, + "output": ["relatedEntity", "updatedBy"] + }, + "nodes": [ + { + "name": "start", + "displayName": "Start", + "type": "startEvent", + "subType": "startEvent" + }, + { + "name": "setStatusInReview", + "displayName": "Set Status In Review", + "type": "automatedTask", + "subType": "setEntityAttributeTask", + "config": { + "fieldName": "entityStatus", + "fieldValue": "In Review" + }, + "input": ["relatedEntity", "updatedBy"], + "inputNamespaceMap": { + "relatedEntity": "global", + "updatedBy": "global" + }, + "output": [] + }, + { + "name": "ApprovalTask", + "displayName": "Approval Task", + "type": "userTask", + "subType": "userApprovalTask", + "config": { + "assignees": { + "addReviewers": true, + "addOwners": false, + "candidates": [] + } + }, + "input": ["relatedEntity"], + "inputNamespaceMap": { + "relatedEntity": "global" + }, + "output": ["result"], + "branches": ["true", "false"] + }, + { + "name": "setStatusApproved", + "displayName": "Set Status Approved", + "type": "automatedTask", + "subType": "setEntityAttributeTask", + "config": { + "fieldName": "entityStatus", + "fieldValue": "Approved" + }, + "input": ["relatedEntity", "updatedBy"], + "inputNamespaceMap": { + "relatedEntity": "global", + "updatedBy": "global" + }, + "output": [] + }, + { + "name": "setStatusRejected", + "displayName": "Set Status Rejected", + "type": "automatedTask", + "subType": "setEntityAttributeTask", + "config": { + "fieldName": "entityStatus", + "fieldValue": "Rejected" + }, + "input": ["relatedEntity", "updatedBy"], + "inputNamespaceMap": { + "relatedEntity": "global", + "updatedBy": "global" + }, + "output": [] + }, + { + "name": "endApproved", + "displayName": "End Approved", + "type": "endEvent", + "subType": "endEvent" + }, + { + "name": "endRejected", + "displayName": "End Rejected", + "type": "endEvent", + "subType": "endEvent" + } + ], + "edges": [ + {"from": "start", "to": "setStatusInReview"}, + {"from": "setStatusInReview", "to": "ApprovalTask"}, + {"from": "ApprovalTask", "to": "setStatusApproved", "condition": "true"}, + {"from": "ApprovalTask", "to": "setStatusRejected", "condition": "false"}, + {"from": "setStatusApproved", "to": "endApproved"}, + {"from": "setStatusRejected", "to": "endRejected"} + ], + "config": {"storeStageStatus": false} + } + """ + .formatted("SelfApprovalPreventionWorkflow"); + + CreateWorkflowDefinition workflow; + try { + LOG.debug( + "Attempting to parse workflow JSON: {}", + workflowJson.substring(0, Math.min(500, workflowJson.length()))); + workflow = JsonUtils.readValue(workflowJson, CreateWorkflowDefinition.class); + } catch (Exception e) { + LOG.error("Failed to parse workflow JSON: {}", e.getMessage()); + LOG.error("Workflow JSON content: {}", workflowJson); + throw e; + } + + String workflowResponse = + client + .getHttpClient() + .executeForString( + HttpMethod.POST, BASE_PATH, workflow, RequestOptions.builder().build()); + + JsonNode workflowCreated = MAPPER.readTree(workflowResponse); + String workflowId = workflowCreated.get("id").asText(); + LOG.debug("Created self-approval prevention workflow: {}", workflowId); + + // Step 4: Create client for user1 and update classification (user1 making the change and is a + // reviewer) + LOG.debug("Creating admin client for user1 and updating classification with reviewers"); + + // Create admin client for user1 (the one making the change) with admin privileges + OpenMetadataClient user1Client = + SdkClients.createClient(user1.getEmail(), user1.getEmail(), new String[] {"admin"}); + + EntityReference user1Ref = + new EntityReference() + .withId(user1.getId()) + .withType("user") + .withName(user1.getName()) + .withFullyQualifiedName(user1.getFullyQualifiedName()); + + EntityReference user2Ref = + new EntityReference() + .withId(user2.getId()) + .withType("user") + .withName(user2.getName()) + .withFullyQualifiedName(user2.getFullyQualifiedName()); + + EntityReference user3Ref = + new EntityReference() + .withId(user3.getId()) + .withType("user") + .withName(user3.getName()) + .withFullyQualifiedName(user3.getFullyQualifiedName()); + + // Construct JSON patch to add reviewers and update description + String reviewersJson = + String.format( + "[" + + "{\"op\":\"replace\",\"path\":\"/description\",\"value\":\"Updated description to trigger workflow\"}," + + "{\"op\":\"add\",\"path\":\"/reviewers\",\"value\":[" + + "{\"id\":\"%s\",\"type\":\"user\",\"name\":\"%s\",\"fullyQualifiedName\":\"%s\"}," + + "{\"id\":\"%s\",\"type\":\"user\",\"name\":\"%s\",\"fullyQualifiedName\":\"%s\"}," + + "{\"id\":\"%s\",\"type\":\"user\",\"name\":\"%s\",\"fullyQualifiedName\":\"%s\"}" + + "]}]", + user1.getId(), + user1.getName(), + user1.getFullyQualifiedName(), + user2.getId(), + user2.getName(), + user2.getFullyQualifiedName(), + user3.getId(), + user3.getName(), + user3.getFullyQualifiedName()); + + // Update the classification to add reviewers (INCLUDING user1 who is making the update) + JsonNode patch = MAPPER.readTree(reviewersJson); + Classification updatedClassification = + user1Client.classifications().patch(classification.getId(), patch); + LOG.debug( + "Patched classification: {} with reviewers: [{}, {}, {}] (user1 client making update)", + updatedClassification.getName(), + user1.getName(), + user2.getName(), + user3.getName()); + + // Step 5: Wait and verify task creation + LOG.info("Waiting for workflow to process classification update and create approval task..."); + + await() + .atMost(Duration.ofSeconds(30)) + .pollInterval(Duration.ofSeconds(2)) + .untilAsserted( + () -> { + try { + String feedResponse = + user1Client + .getHttpClient() + .executeForString( + HttpMethod.GET, + "/v1/feed?type=Task", + null, + RequestOptions.builder().build()); + + JsonNode feedData = MAPPER.readTree(feedResponse); + JsonNode threads = feedData.get("data"); + + boolean foundTask = false; + boolean selfApprovalPrevented = false; + + for (JsonNode thread : threads) { + LOG.debug("Checking thread: {}", thread); + if (thread.has("task")) { + JsonNode task = thread.get("task"); + LOG.debug("Found task: {}", task); + + // Check if thread has about field (about is on the Thread, not the Task) + if (!thread.has("about") || thread.get("about") == null) { + LOG.debug("Thread missing 'about' field, skipping"); + continue; + } + + String taskAbout = thread.get("about").asText(); + LOG.debug("Thread about: {}", taskAbout); + + if (taskAbout.contains(classification.getFullyQualifiedName())) { + foundTask = true; + LOG.debug("Found matching task for classification"); + + if (!task.has("assignees") || task.get("assignees") == null) { + LOG.warn("Task missing 'assignees' field"); + continue; + } + + JsonNode assignees = task.get("assignees"); + LOG.debug("Task assignees: {}", assignees); + + // Verify that user1 (the updater) is NOT in the assignees due to + // self-approval prevention + boolean user1InAssignees = false; + boolean user2InAssignees = false; + boolean user3InAssignees = false; + + for (JsonNode assignee : assignees) { + if (assignee.has("name") && assignee.get("name") != null) { + String assigneeName = assignee.get("name").asText(); + LOG.debug("Checking assignee: {}", assigneeName); + if (user1.getName().equals(assigneeName)) { + user1InAssignees = true; + } + if (user2.getName().equals(assigneeName)) { + user2InAssignees = true; + } + if (user3.getName().equals(assigneeName)) { + user3InAssignees = true; + } + } + } + + LOG.debug( + "Task assignees analysis: user1 (updater) in assignees: {}, user2 in assignees: {}, user3 in assignees: {}", + user1InAssignees, + user2InAssignees, + user3InAssignees); + + // Self-approval prevention: user1 should NOT be in assignees, but user2, + // user3 should be + selfApprovalPrevented = + !user1InAssignees && user2InAssignees && user3InAssignees; + break; + } + } + } + + assertTrue(foundTask, "Expected to find approval task for classification"); + assertTrue( + selfApprovalPrevented, + "Self-approval prevention failed: creator should not be in assignees"); + + } catch (Exception e) { + LOG.error("Error during task verification: {}", e.getMessage()); + fail( + "Failed to verify task creation and self-approval prevention: " + + e.getMessage()); + } + }); + + LOG.info("✓ Verified that self-approval prevention is working correctly"); + + // Step 6: Cleanup + LOG.debug("Cleaning up test resources"); + + // Delete workflow + try { + client + .getHttpClient() + .executeForString( + HttpMethod.DELETE, + BASE_PATH + "/" + workflowId, + null, + RequestOptions.builder().build()); + LOG.debug("✓ Deleted workflow"); + } catch (Exception e) { + LOG.warn("Failed to delete workflow: {}", e.getMessage()); + } + + // Delete classification + Map params = new HashMap<>(); + params.put("recursive", "true"); + client.classifications().delete(classification.getId().toString(), params); + LOG.debug("✓ Deleted classification"); + + // Delete users + client.users().delete(user1.getId().toString(), params); + client.users().delete(user2.getId().toString(), params); + client.users().delete(user3.getId().toString(), params); + LOG.debug("✓ Deleted test users"); + + LOG.info("test_SelfApprovalPrevention completed successfully"); + } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/userTask/impl/SetApprovalAssigneesImpl.java b/openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/userTask/impl/SetApprovalAssigneesImpl.java index 52c45dafb4c..4d32ab9d050 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/userTask/impl/SetApprovalAssigneesImpl.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/userTask/impl/SetApprovalAssigneesImpl.java @@ -1,7 +1,9 @@ package org.openmetadata.service.governance.workflows.elements.nodes.userTask.impl; import static org.openmetadata.service.governance.workflows.Workflow.EXCEPTION_VARIABLE; +import static org.openmetadata.service.governance.workflows.Workflow.GLOBAL_NAMESPACE; import static org.openmetadata.service.governance.workflows.Workflow.RELATED_ENTITY_VARIABLE; +import static org.openmetadata.service.governance.workflows.Workflow.UPDATED_BY_VARIABLE; import static org.openmetadata.service.governance.workflows.Workflow.WORKFLOW_RUNTIME_EXCEPTION; import static org.openmetadata.service.governance.workflows.WorkflowHandler.getProcessDefinitionKeyFromId; @@ -115,6 +117,27 @@ public class SetApprovalAssigneesImpl implements JavaDelegate { List assigneeList = new ArrayList<>(assignees); + // Prevent self-approval: Remove updatedBy user from assignees list + try { + String updatedBy = + (String) varHandler.getNamespacedVariable(GLOBAL_NAMESPACE, UPDATED_BY_VARIABLE); + if (updatedBy != null && !updatedBy.trim().isEmpty()) { + String updatedByEntityLink = + new MessageParser.EntityLink("user", updatedBy).getLinkString(); + boolean removed = assigneeList.remove(updatedByEntityLink); + if (removed) { + LOG.debug( + "[Process: {}] Prevented self-approval: Removed updatedBy user '{}' from assignees", + execution.getProcessInstanceId(), + updatedBy); + } + } + } catch (Exception e) { + LOG.warn( + "Failed to retrieve updatedBy variable for self-approval prevention: {}", + e.getMessage()); + } + // Persist the list as JSON array so TaskListener can read it. // Using setVariable instead of setVariableLocal to ensure visibility across subprocess. execution.setVariable( diff --git a/openmetadata-ui/src/main/resources/ui/src/components/LineageTable/LineageTable.tsx b/openmetadata-ui/src/main/resources/ui/src/components/LineageTable/LineageTable.tsx index db7e0746ddd..aee1198174e 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/LineageTable/LineageTable.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/LineageTable/LineageTable.tsx @@ -270,14 +270,12 @@ const LineageTable: FC<{ entity: SourceType }> = ({ entity }) => { onChange={(_, value) => { handlePageChange(1); updateURLParams({ dir: value }); - }} - > + }}> {radioGroupOptions.map((option) => ( + value={option.value}> {option.label} ))} @@ -330,8 +328,7 @@ const LineageTable: FC<{ entity: SourceType }> = ({ entity }) => { }, }, }} - onClick={(event) => setImpactOnEl(event.currentTarget)} - > + onClick={(event) => setImpactOnEl(event.currentTarget)}> } @@ -341,8 +338,7 @@ const LineageTable: FC<{ entity: SourceType }> = ({ entity }) => { setImpactOnEl(null)} - > + onClose={() => setImpactOnEl(null)}> {LINEAGE_IMPACT_OPTIONS.map((option) => ( = ({ entity }) => { setSelectedImpactLevel(option.key); handlePageChange(currentPage); setImpactOnEl(null); - }} - > + }}> {option.icon} {option.label} @@ -504,15 +499,13 @@ const LineageTable: FC<{ entity: SourceType }> = ({ entity }) => { (_: string, record: SearchSourceAlias) => ( + entityType={record.entityType ?? ''}> + )}> {stringToHTML( highlightSearchText(getEntityName(record), searchValue) )} @@ -668,8 +661,7 @@ const LineageTable: FC<{ entity: SourceType }> = ({ entity }) => { to={getEntityLinkFromType( record?.fullyQualifiedName ?? '', record?.type as EntityType - )} - > + )}> {stringToHTML( highlightSearchText( Fqn.split(record?.fullyQualifiedName ?? '').pop(), @@ -694,8 +686,7 @@ const LineageTable: FC<{ entity: SourceType }> = ({ entity }) => { to={getEntityLinkFromType( record?.fullyQualifiedName ?? '', record?.type as EntityType - )} - > + )}> {stringToHTML( highlightSearchText( Fqn.split(record?.fullyQualifiedName ?? '').pop(), @@ -816,8 +807,7 @@ const LineageTable: FC<{ entity: SourceType }> = ({ entity }) => { 'lineage-card lineage-card-table' )} data-testid="lineage-card-table" - title={cardHeader} - > + title={cardHeader}> = ({ diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/CoverImageUpload/MUICoverImageUpload.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/CoverImageUpload/MUICoverImageUpload.tsx index ae274224075..99f2434977a 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/CoverImageUpload/MUICoverImageUpload.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/common/CoverImageUpload/MUICoverImageUpload.tsx @@ -11,18 +11,11 @@ * limitations under the License. */ -import { - Box, - Button, - IconButton, - Typography, - useTheme, -} from '@mui/material'; +import { Box, Button, IconButton, Typography, useTheme } from '@mui/material'; import { RefreshCcw01, Trash01, UploadCloud01 } from '@untitledui/icons'; import { useSnackbar } from 'notistack'; import { FC, useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; -import Loader from '../Loader/Loader'; import { showNotistackError } from '../../../utils/NotistackUtils'; import imageClassBase from '../../BlockEditor/Extensions/image/ImageClassBase'; import MUIFileUpload from '../FileUpload/MUIFileUpload'; @@ -30,6 +23,7 @@ import { FileUploadValue, FileValidationResult, } from '../FileUpload/MUIFileUpload.interface'; +import Loader from '../Loader/Loader'; import { MUICoverImageUploadProps } from './CoverImageUpload.interface'; const DEFAULT_MAX_SIZE_MB = 5; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/FileUpload/MUIFileUpload.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/FileUpload/MUIFileUpload.tsx index 20c4797e84c..325b16127ab 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/FileUpload/MUIFileUpload.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/common/FileUpload/MUIFileUpload.tsx @@ -36,8 +36,8 @@ import { AxiosError } from 'axios'; import { useSnackbar } from 'notistack'; import { FC, useCallback, useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; -import Loader from '../Loader/Loader'; import { showNotistackError } from '../../../utils/NotistackUtils'; +import Loader from '../Loader/Loader'; import { MUIFileUploadProps } from './MUIFileUpload.interface'; const DEFAULT_MAX_SIZE_MB = 5; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/common/atoms/actions/useDelete.tsx b/openmetadata-ui/src/main/resources/ui/src/components/common/atoms/actions/useDelete.tsx index 4d596f7303a..0fe31bd551c 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/common/atoms/actions/useDelete.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/common/atoms/actions/useDelete.tsx @@ -23,13 +23,13 @@ import { Typography, useTheme, } from '@mui/material'; -import Loader from '../../Loader/Loader'; import { Trash01 } from '@untitledui/icons'; import { useCallback, useMemo, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { deleteEntity } from '../../../../rest/miscAPI'; import { getEntityName } from '../../../../utils/EntityUtils'; import { showErrorToast, showSuccessToast } from '../../../../utils/ToastUtils'; +import Loader from '../../Loader/Loader'; interface UseDeleteConfig< T extends { id: string; name?: string; displayName?: string } diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/ColumnBulkOperations/ColumnGrid/components/ColumnGridTableRow.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/ColumnBulkOperations/ColumnGrid/components/ColumnGridTableRow.tsx index a8d5142dc28..ee5919c0220 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/ColumnBulkOperations/ColumnGrid/components/ColumnGridTableRow.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/ColumnBulkOperations/ColumnGrid/components/ColumnGridTableRow.tsx @@ -11,12 +11,7 @@ * limitations under the License. */ -import { - Checkbox, - TableCell, - TableRow, - useTheme, -} from '@mui/material'; +import { Checkbox, TableCell, TableRow, useTheme } from '@mui/material'; import React, { useMemo } from 'react'; import Loader from '../../../../components/common/Loader/Loader'; import { ColumnGridRowData } from '../ColumnGrid.interface'; diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/EntityLineageUtils.tsx b/openmetadata-ui/src/main/resources/ui/src/utils/EntityLineageUtils.tsx index a091756213c..d405b497b71 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/EntityLineageUtils.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/utils/EntityLineageUtils.tsx @@ -2037,8 +2037,7 @@ const buildLineageTableColumns = (headers: string[]): ColumnsType => { to={entityUtilClassBase.getEntityLink( getEntityType(serviceType, record.fromEntityFQN), record.fromEntityFQN - )} - > + )}> {renderCombined(record.fromEntityFQN, serviceType)} ); @@ -2060,8 +2059,7 @@ const buildLineageTableColumns = (headers: string[]): ColumnsType => { to={entityUtilClassBase.getEntityLink( getEntityType(serviceType, record.toEntityFQN), record.toEntityFQN - )} - > + )}> {renderCombined(record.toEntityFQN, serviceType)} ); @@ -2085,8 +2083,7 @@ const buildLineageTableColumns = (headers: string[]): ColumnsType => { render: (text: string) => ( + ellipsis={{ tooltip: true }}> {isEmpty(text) ? NO_DATA_PLACEHOLDER : text} ),