Fixes #11883 - A user can resolve a task created by him (#11998)

* Fixes #11883 - A user can resolve a task created by him

* Test failure fixes

openmetadata-service/src/main/java/org/openmetadata/service/exception/CatalogExceptionMessage.java

@@ -151,6 +151,10 @@ public final class CatalogExceptionMessage {
     return String.format("Principal: CatalogPrincipal{name='%s'} operations %s not allowed", user, operations);
   }
 
+  public static String taskOperationNotAllowed(String user, String operations) {
+    return String.format("Principal: CatalogPrincipal{name='%s'} operations %s not allowed", user, operations);
+  }
+
   public static String entityIsNotEmpty(String entityType) {
     return String.format("%s is not empty", entityType);
   }

openmetadata-service/src/main/java/org/openmetadata/service/formatter/decorators/MessageDecorator.java

@@ -14,6 +14,7 @@
 package org.openmetadata.service.formatter.decorators;
 
 import java.util.LinkedList;
+import org.apache.commons.lang3.StringUtils;
 import org.bitbucket.cowwoc.diffmatchpatch.DiffMatchPatch;
 import org.openmetadata.schema.type.ChangeEvent;
 
@@ -44,6 +45,7 @@ public interface MessageDecorator<T> {
 
   default String getPlaintextDiff(String oldValue, String newValue) {
     // create a configured DiffRowGenerator
+    oldValue = oldValue == null ? StringUtils.EMPTY : oldValue;
     String addMarker = this.httpAddMarker();
     String removeMarker = this.httpRemoveMarker();
 

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

@@ -25,7 +25,6 @@ import static org.openmetadata.service.exception.CatalogExceptionMessage.ANNOUNC
 import static org.openmetadata.service.exception.CatalogExceptionMessage.ANNOUNCEMENT_OVERLAP;
 import static org.openmetadata.service.exception.CatalogExceptionMessage.entityNotFound;
 import static org.openmetadata.service.util.EntityUtil.compareEntityReference;
-import static org.openmetadata.service.util.EntityUtil.populateEntityReferences;
 import static org.openmetadata.service.util.RestUtil.DELETED_TEAM_DISPLAY;
 import static org.openmetadata.service.util.RestUtil.DELETED_TEAM_NAME;
 import static org.openmetadata.service.util.RestUtil.DELETED_USER_DISPLAY;
@@ -72,6 +71,7 @@ import org.openmetadata.schema.type.ThreadType;
 import org.openmetadata.schema.utils.EntityInterfaceUtil;
 import org.openmetadata.service.Entity;
 import org.openmetadata.service.ResourceRegistry;
+import org.openmetadata.service.exception.CatalogExceptionMessage;
 import org.openmetadata.service.exception.EntityNotFoundException;
 import org.openmetadata.service.formatter.decorators.FeedMessageDecorator;
 import org.openmetadata.service.formatter.decorators.MessageDecorator;
@@ -80,7 +80,7 @@ import org.openmetadata.service.resources.feeds.FeedResource;
 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.Authorizer;
+import org.openmetadata.service.security.AuthorizationException;
 import org.openmetadata.service.security.policyevaluator.SubjectCache;
 import org.openmetadata.service.util.EntityUtil;
 import org.openmetadata.service.util.FullyQualifiedName;
@@ -173,6 +173,7 @@ public class FeedRepository {
 
     // Add mentions to field relationship table
     storeMentions(thread, thread.getMessage());
+    populateAssignees(thread);
     return thread;
   }
 
@@ -226,23 +227,17 @@ public class FeedRepository {
       // Add a default message to the Task thread with updated description/tag
       TaskDetails task = thread.getTask();
       TaskType type = task.getType();
-      String oldValue = StringUtils.EMPTY;
+      if (EntityUtil.isDescriptionTask(type)) {
-      if (List.of(TaskType.RequestDescription, TaskType.UpdateDescription).contains(type)) {
-        if (task.getOldValue() != null) {
-          oldValue = task.getOldValue();
-        }
         message =
             String.format(
                 "Resolved the Task with Description - %s",
-                feedMessageFormatter.getPlaintextDiff(oldValue, task.getNewValue()));
+                feedMessageFormatter.getPlaintextDiff(task.getOldValue(), task.getNewValue()));
-      } else if (List.of(TaskType.RequestTag, TaskType.UpdateTag).contains(type)) {
+      } else if (EntityUtil.isTagTask(type)) {
-        List<TagLabel> tags;
+        String oldValue =
-        if (task.getOldValue() != null) {
+            task.getOldValue() != null
-          tags = JsonUtils.readObjects(task.getOldValue(), TagLabel.class);
+                ? getTagFQNs(JsonUtils.readObjects(task.getOldValue(), TagLabel.class))
-          oldValue = getTagFQNs(tags);
+                : StringUtils.EMPTY;
-        }
+        String newValue = getTagFQNs(JsonUtils.readObjects(task.getNewValue(), TagLabel.class));
-        tags = JsonUtils.readObjects(task.getNewValue(), TagLabel.class);
-        String newValue = getTagFQNs(tags);
         message =
             String.format(
                 "Resolved the Task with Tag(s) - %s", feedMessageFormatter.getPlaintextDiff(oldValue, newValue));
@@ -600,39 +595,39 @@ public class FeedRepository {
     return new PatchResponse<>(Status.OK, updatedHref, change);
   }
 
-  public void checkPermissionsForResolveTask(Thread thread, SecurityContext securityContext, Authorizer authorizer)
+  public void checkPermissionsForResolveTask(Thread thread, boolean closeTask, SecurityContext securityContext)
       throws IOException {
-    if (!thread.getType().equals(ThreadType.Task)) {
+    String userName = securityContext.getUserPrincipal().getName();
-      return; // Nothing to resolve
+    User user = SubjectCache.getInstance().getUser(userName);
-    }
     EntityLink about = EntityLink.parse(thread.getAbout());
     EntityReference aboutRef = EntityUtil.validateEntityLink(about);
-
+    if (Boolean.TRUE.equals(user.getIsAdmin())) {
-    // Get owner for the addressed to Entity
+      return; // Allow admin resolve/close task
-    EntityReference owner = Entity.getOwner(aboutRef);
-
-    String userName = securityContext.getUserPrincipal().getName();
-    User loggedInUser = SubjectCache.getInstance().getUser(userName);
-    List<EntityReference> teams =
-        populateEntityReferences(
-            dao.relationshipDAO()
-                .findFrom(loggedInUser.getId().toString(), Entity.USER, Relationship.HAS.ordinal(), Entity.TEAM),
-            Entity.TEAM);
-    List<String> teamNames = teams.stream().map(EntityReference::getName).collect(Collectors.toList());
-
-    // check if logged-in user satisfies any of the following
-    // - Creator of the task
-    // - logged-in user or the teams they belong to were assigned the task
-    // - logged-in user or the teams they belong to, owns the entity that the task is about
-    List<EntityReference> assignees = thread.getTask().getAssignees();
-    if (!thread.getCreatedBy().equals(userName)
-        && assignees.stream().noneMatch(assignee -> assignee.getName().equals(userName))
-        && assignees.stream().noneMatch(assignee -> teamNames.contains(assignee.getName()))
-        && !owner.getName().equals(userName)
-        && !teamNames.contains(owner.getName())) {
-      // Only admins or bots can close or resolve task other than the above-mentioned users
-      authorizer.authorizeAdmin(securityContext);
     }
+
+    // Allow if user is an assignee of the resolve/close task
+    // Allow if user is the owner of the resource for which task is created to resolve/close task
+    // 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))
+        || owner.getName().equals(userName)
+        || closeTask && thread.getCreatedBy().equals(userName)) {
+      return;
+    }
+
+    // Allow if user belongs to a team that has task assigned to it
+    // Allow if user belongs to a team if owner of the resource against which task is created
+    List<EntityReference> teams = user.getTeams();
+    List<String> teamNames = teams.stream().map(EntityReference::getName).collect(Collectors.toList());
+    if (assignees.stream().anyMatch(assignee -> teamNames.contains(assignee.getName()))
+        && teamNames.contains(owner.getName())) {
+      return;
+    }
+
+    // Finally, operation is not allowed - throw exception
+    throw new AuthorizationException(
+        CatalogExceptionMessage.taskOperationNotAllowed(userName, closeTask ? "closeTask" : "resolveTask"));
   }
 
   private void validateAnnouncement(AnnouncementDetails announcementDetails) {

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

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

openmetadata-service/src/test/java/org/openmetadata/service/secrets/ExternalSecretsManagerTest.java

@@ -101,11 +101,9 @@ public abstract class ExternalSecretsManagerTest {
     // Encrypt private key and ensure it is indeed encrypted
     secretsManager.encryptAuthenticationMechanism("bot", actualAuthMechanism);
     assertNotEquals(privateKey, getPrivateKey(actualAuthMechanism));
-    System.out.println("XXX privateKey encrypted is " + getPrivateKey(actualAuthMechanism));
 
     // Decrypt private key and ensure it is decrypted
     secretsManager.decryptAuthenticationMechanism("bot", actualAuthMechanism);
-    System.out.println("XXX privateKey decrypted is " + getPrivateKey(actualAuthMechanism));
     assertEquals(privateKey, getPrivateKey(actualAuthMechanism));
   }
 
@@ -130,12 +128,10 @@ public abstract class ExternalSecretsManagerTest {
 
     // Encrypt the pipeline and make sure it is secret key encrypted
     secretsManager.encryptIngestionPipeline(actualIngestionPipeline);
-    System.out.println("XXX encrypted aws secret access key is " + getAwsSecretAccessKey(actualIngestionPipeline));
     assertNotEquals(secretKey, getAwsSecretAccessKey(actualIngestionPipeline));
 
     // Decrypt the pipeline and make sure the secret key is decrypted
     secretsManager.decryptIngestionPipeline(actualIngestionPipeline);
-    System.out.println("XXX decrypted aws secret access key is " + getAwsSecretAccessKey(actualIngestionPipeline));
     assertEquals(secretKey, getAwsSecretAccessKey(actualIngestionPipeline));
     assertEquals(expectedIngestionPipeline, actualIngestionPipeline);
   }