Check assignee permissions before updating the entity description/tags (#15582)

* Check assignee permissions before updating the entity description/tags

* make assignee field enabled even if entity already has owner

* minor fix

---------

Co-authored-by: Ashish Gupta <ashish@getcollate.io>

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/FeedRepository.java

@@ -64,6 +64,7 @@ import org.openmetadata.schema.type.ChangeEvent;
 import org.openmetadata.schema.type.EntityReference;
 import org.openmetadata.schema.type.EventType;
 import org.openmetadata.schema.type.Include;
+import org.openmetadata.schema.type.MetadataOperation;
 import org.openmetadata.schema.type.Post;
 import org.openmetadata.schema.type.Reaction;
 import org.openmetadata.schema.type.Relationship;
@@ -85,6 +86,9 @@ import org.openmetadata.service.resources.feeds.FeedUtil;
 import org.openmetadata.service.resources.feeds.MessageParser;
 import org.openmetadata.service.resources.feeds.MessageParser.EntityLink;
 import org.openmetadata.service.security.AuthorizationException;
+import org.openmetadata.service.security.Authorizer;
+import org.openmetadata.service.security.policyevaluator.OperationContext;
+import org.openmetadata.service.security.policyevaluator.ResourceContext;
 import org.openmetadata.service.util.EntityUtil;
 import org.openmetadata.service.util.FullyQualifiedName;
 import org.openmetadata.service.util.JsonUtils;
@@ -786,11 +790,12 @@ public class FeedRepository {
   }
 
   public void checkPermissionsForResolveTask(
-      Thread thread, boolean closeTask, SecurityContext securityContext) {
+      Authorizer authorizer, Thread thread, boolean closeTask, SecurityContext securityContext) {
     String userName = securityContext.getUserPrincipal().getName();
     User user = Entity.getEntityByName(USER, userName, TEAMS_FIELD, NON_DELETED);
     EntityLink about = EntityLink.parse(thread.getAbout());
     EntityReference aboutRef = EntityUtil.validateEntityLink(about);
+    ThreadContext threadContext = getThreadContext(thread);
     if (Boolean.TRUE.equals(user.getIsAdmin())) {
       return; // Allow admin resolve/close task
     }
@@ -800,9 +805,25 @@ public class FeedRepository {
     // Allow if user created the task to close task (and not resolve task)
     EntityReference owner = Entity.getOwner(aboutRef);
     List<EntityReference> assignees = thread.getTask().getAssignees();
-    if (assignees.stream().anyMatch(assignee -> assignee.getName().equals(userName))
+    if (owner.getName().equals(userName) || closeTask && thread.getCreatedBy().equals(userName)) {
-        || owner.getName().equals(userName)
+      return;
-        || closeTask && thread.getCreatedBy().equals(userName)) {
+    }
+
+    // Allow if user is an assignee of the task and if the assignee has permissions to update the
+    // entity
+    if (assignees.stream().anyMatch(assignee -> assignee.getName().equals(userName))) {
+      // If entity does not exist, this is a create operation, else update operation
+      ResourceContext resourceContext =
+          new ResourceContext<>(aboutRef.getType(), aboutRef.getId(), null);
+      if (EntityUtil.isDescriptionTask(threadContext.getTaskWorkflow().getTaskType())) {
+        OperationContext operationContext =
+            new OperationContext(aboutRef.getType(), MetadataOperation.EDIT_DESCRIPTION);
+        authorizer.authorize(securityContext, operationContext, resourceContext);
+      } else if (EntityUtil.isTagTask(threadContext.getTaskWorkflow().getTaskType())) {
+        OperationContext operationContext =
+            new OperationContext(aboutRef.getType(), MetadataOperation.EDIT_TAGS);
+        authorizer.authorize(securityContext, operationContext, resourceContext);
+      }
       return;
     }
 

openmetadata-service/src/main/java/org/openmetadata/service/resources/feeds/FeedResource.java

@@ -297,7 +297,7 @@ public class FeedResource {
           String id,
       @Valid ResolveTask resolveTask) {
     Thread task = dao.getTask(Integer.parseInt(id));
-    dao.checkPermissionsForResolveTask(task, false, securityContext);
+    dao.checkPermissionsForResolveTask(authorizer, task, false, securityContext);
     return dao.resolveTask(uriInfo, task, securityContext.getUserPrincipal().getName(), resolveTask)
         .toResponse();
   }
@@ -326,7 +326,7 @@ public class FeedResource {
           String id,
       @Valid CloseTask closeTask) {
     Thread task = dao.getTask(Integer.parseInt(id));
-    dao.checkPermissionsForResolveTask(task, true, securityContext);
+    dao.checkPermissionsForResolveTask(authorizer, task, true, securityContext);
     return dao.closeTask(uriInfo, task, securityContext.getUserPrincipal().getName(), closeTask)
         .toResponse();
   }

openmetadata-ui/src/main/resources/ui/cypress/common/TaskUtils.ts

@@ -31,7 +31,10 @@ export type TaskDetails = {
   schemaName?: string;
 };
 
-export const verifyTaskDetails = (regexPattern: RegExp) => {
+export const verifyTaskDetails = (
+  regexPattern: RegExp,
+  taskAssignee?: string
+) => {
   cy.get('#task-panel').should('be.visible');
   cy.get('[data-testid="task-title"]')
     .invoke('text')
@@ -43,7 +46,7 @@ export const verifyTaskDetails = (regexPattern: RegExp) => {
 
   cy.get('[data-testid="owner-link"]').should('contain', owner);
 
-  cy.get(`[data-testid="${assignee}"]`).should('be.visible');
+  cy.get(`[data-testid="${taskAssignee ?? assignee}"]`).should('be.visible');
 };
 
 export const editAssignee = () => {

openmetadata-ui/src/main/resources/ui/cypress/e2e/Flow/Task.spec.ts

@@ -16,7 +16,11 @@ import {
   toastNotification,
   verifyResponseStatusCode,
 } from '../../common/common';
-import { createEntityTable, hardDeleteService } from '../../common/EntityUtils';
+import {
+  createEntityTable,
+  deleteUserEntity,
+  hardDeleteService,
+} from '../../common/EntityUtils';
 import {
   createAndUpdateDescriptionTask,
   createDescriptionTask,
@@ -26,8 +30,12 @@ import {
 import { visitEntityDetailsPage } from '../../common/Utils/Entity';
 import { getToken } from '../../common/Utils/LocalStorage';
 import { addOwner } from '../../common/Utils/Owner';
-import { DATA_ASSETS } from '../../constants/constants';
+import { DATA_ASSETS, uuid } from '../../constants/constants';
-import { DATABASE_SERVICE } from '../../constants/EntityConstant';
+import {
+  DATABASE_SERVICE,
+  USER_DETAILS,
+  USER_NAME,
+} from '../../constants/EntityConstant';
 import { SERVICE_CATEGORIES } from '../../constants/service.constants';
 
 const ENTITY_TABLE = {
@@ -39,30 +47,137 @@ const ENTITY_TABLE = {
   entityType: 'Table',
 };
 
+const POLICY_DETAILS = {
+  name: `cy-data-viewAll-policy-${uuid()}`,
+  rules: [
+    {
+      name: 'viewRuleAllowed',
+      resources: ['All'],
+      operations: ['ViewAll'],
+      effect: 'allow',
+    },
+    {
+      effect: 'deny',
+      name: 'editNotAllowed',
+      operations: ['EditAll'],
+      resources: ['All'],
+    },
+  ],
+};
+const ROLE_DETAILS = {
+  name: `cy-data-viewAll-role-${uuid()}`,
+  policies: [POLICY_DETAILS.name],
+};
+
+const TEAM_DETAILS = {
+  name: 'viewAllTeam',
+  displayName: 'viewAllTeam',
+  teamType: 'Group',
+};
+
 describe('Task flow should work', { tags: 'DataAssets' }, () => {
+  const data = {
+    user: { id: '' },
+    policy: { id: '' },
+    role: { id: '' },
+    team: { id: '' },
+  };
+
   before(() => {
     cy.login();
-    cy.getAllLocalStorage().then((data) => {
+    cy.getAllLocalStorage().then((storageData) => {
-      const token = getToken(data);
+      const token = getToken(storageData);
 
       createEntityTable({
         token,
         ...DATABASE_SERVICE,
         tables: [DATABASE_SERVICE.entity],
       });
+
+      // Create ViewAll Policy
+      cy.request({
+        method: 'POST',
+        url: `/api/v1/policies`,
+        headers: { Authorization: `Bearer ${token}` },
+        body: POLICY_DETAILS,
+      }).then((policyResponse) => {
+        data.policy = policyResponse.body;
+
+        // Create ViewAll Role
+        cy.request({
+          method: 'POST',
+          url: `/api/v1/roles`,
+          headers: { Authorization: `Bearer ${token}` },
+          body: ROLE_DETAILS,
+        }).then((roleResponse) => {
+          data.role = roleResponse.body;
+
+          // Create a new user
+          cy.request({
+            method: 'POST',
+            url: `/api/v1/users/signup`,
+            headers: { Authorization: `Bearer ${token}` },
+            body: USER_DETAILS,
+          }).then((userResponse) => {
+            data.user = userResponse.body;
+
+            // create team
+            cy.request({
+              method: 'GET',
+              url: `/api/v1/teams/name/Organization`,
+              headers: { Authorization: `Bearer ${token}` },
+            }).then((teamResponse) => {
+              cy.request({
+                method: 'POST',
+                url: `/api/v1/teams`,
+                headers: { Authorization: `Bearer ${token}` },
+                body: {
+                  ...TEAM_DETAILS,
+                  parents: [teamResponse.body.id],
+                  users: [userResponse.body.id],
+                  defaultRoles: [roleResponse.body.id],
+                },
+              }).then((teamResponse) => {
+                data.team = teamResponse.body;
+              });
+            });
+          });
+        });
+      });
     });
   });
 
   after(() => {
     cy.login();
-    cy.getAllLocalStorage().then((data) => {
+    cy.getAllLocalStorage().then((storageData) => {
-      const token = getToken(data);
+      const token = getToken(storageData);
 
       hardDeleteService({
         token,
         serviceFqn: ENTITY_TABLE.serviceName,
         serviceType: SERVICE_CATEGORIES.DATABASE_SERVICES,
       });
+
+      // Clean up for the created data
+      deleteUserEntity({ token, id: data.user.id });
+
+      cy.request({
+        method: 'DELETE',
+        url: `/api/v1/teams/${data.team.id}?hardDelete=true&recursive=true`,
+        headers: { Authorization: `Bearer ${token}` },
+      });
+
+      cy.request({
+        method: 'DELETE',
+        url: `/api/v1/policies/${data.policy.id}?hardDelete=true&recursive=true`,
+        headers: { Authorization: `Bearer ${token}` },
+      });
+
+      cy.request({
+        method: 'DELETE',
+        url: `/api/v1/roles/${data.role.id}?hardDelete=true&recursive=true`,
+        headers: { Authorization: `Bearer ${token}` },
+      });
     });
   });
 
@@ -193,7 +308,7 @@ describe('Task flow should work', { tags: 'DataAssets' }, () => {
     });
   });
 
-  it('Asignee field should be disabled for owned entity tasks', () => {
+  it('Assignee field should not be disabled for owned entity tasks', () => {
     interceptURL(
       'GET',
       `/api/v1/${ENTITY_TABLE.entity}/name/*`,
@@ -213,17 +328,49 @@ describe('Task flow should work', { tags: 'DataAssets' }, () => {
     cy.wait('@getEntityDetails').then((res) => {
       const entity = res.response.body;
 
-      // create description task and verify asignee field to have owner
+      createDescriptionTask({
-      // and should be disbaled
+        ...ENTITY_TABLE,
-
+        assignee: USER_NAME,
-      createDescriptionTask(
+        term: entity.displayName ?? entity.name,
-        {
+      });
-          ...ENTITY_TABLE,
-          assignee: 'Adam Rodriguez',
-          term: entity.displayName ?? entity.name,
-        },
-        true
-      );
     });
   });
+
+  it(`should throw error for not having edit permission for viewAll user`, () => {
+    // logout for the admin user
+    cy.logout();
+
+    // login to viewAll user
+    cy.login(USER_DETAILS.email, USER_DETAILS.password);
+
+    interceptURL(
+      'GET',
+      `/api/v1/${ENTITY_TABLE.entity}/name/*`,
+      'getEntityDetails'
+    );
+
+    visitEntityDetailsPage({
+      term: ENTITY_TABLE.term,
+      serviceName: ENTITY_TABLE.serviceName,
+      entity: ENTITY_TABLE.entity,
+    });
+
+    cy.get('[data-testid="activity_feed"]').click();
+
+    cy.get('[data-menu-id*="tasks"]').click();
+
+    // verify the task details
+    verifyTaskDetails(/#(\d+) Request to update description for/, USER_NAME);
+
+    cy.get(`[data-testid="${USER_NAME}"]`).should('be.visible');
+
+    // Accept the description suggestion which is created
+    cy.get('.ant-btn-compact-first-item').contains('Accept Suggestion').click();
+
+    verifyResponseStatusCode('@taskResolve', 403);
+
+    toastNotification(
+      `Principal: CatalogPrincipal{name='${USER_NAME}'} operation EditDescription denied by role ${ROLE_DETAILS.name}, policy ${POLICY_DETAILS.name}, rule editNotAllowed`
+    );
+  });
 });

openmetadata-ui/src/main/resources/ui/src/components/Entity/Task/TaskTab/TaskTab.component.tsx

@@ -83,6 +83,7 @@ import { getNameFromFQN } from '../../../../utils/CommonUtils';
 import EntityLink from '../../../../utils/EntityLink';
 import { getEntityFQN } from '../../../../utils/FeedUtils';
 import { checkPermission } from '../../../../utils/PermissionsUtils';
+import { getErrorText } from '../../../../utils/StringsUtils';
 import {
   fetchOptions,
   generateOptions,
@@ -266,7 +267,9 @@ export const TaskTab = ({
         rest.onAfterClose?.();
         rest.onUpdateEntityDetails?.();
       })
-      .catch((err: AxiosError) => showErrorToast(err));
+      .catch((err: AxiosError) =>
+        showErrorToast(getErrorText(err, t('server.unexpected-error')))
+      );
   };
 
   const onTaskResolve = () => {
@@ -429,7 +432,9 @@ export const TaskTab = ({
       rest.onAfterClose?.();
       setShowEditTaskModel(false);
     } catch (error) {
-      showErrorToast(error as AxiosError);
+      showErrorToast(
+        getErrorText(error as AxiosError, t('server.unexpected-error'))
+      );
     } finally {
       setIsActionLoading(false);
     }

openmetadata-ui/src/main/resources/ui/src/pages/TasksPage/RequestDescriptionPage/RequestDescriptionPage.tsx

@@ -240,7 +240,6 @@ const RequestDescription = () => {
                     },
                   ]}>
                   <Assignees
-                    disabled={Boolean(entityData.owner)}
                     options={options}
                     value={assignees}
                     onChange={setAssignees}

openmetadata-ui/src/main/resources/ui/src/pages/TasksPage/RequestTagPage/RequestTagPage.tsx

@@ -229,7 +229,6 @@ const RequestTag = () => {
                     },
                   ]}>
                   <Assignees
-                    disabled={Boolean(entityData.owner)}
                     options={options}
                     value={assignees}
                     onChange={setAssignees}

openmetadata-ui/src/main/resources/ui/src/pages/TasksPage/UpdateDescriptionPage/UpdateDescriptionPage.tsx

@@ -257,7 +257,6 @@ const UpdateDescription = () => {
                     },
                   ]}>
                   <Assignees
-                    disabled={Boolean(entityData.owner)}
                     options={options}
                     value={assignees}
                     onChange={setAssignees}

openmetadata-ui/src/main/resources/ui/src/pages/TasksPage/UpdateTagPage/UpdateTagPage.tsx

@@ -269,7 +269,6 @@ const UpdateTag = () => {
                     },
                   ]}>
                   <Assignees
-                    disabled={Boolean(entityData.owner)}
                     options={options}
                     value={assignees}
                     onChange={setAssignees}