fix: prevent NPE and correct process manager group condition - EXO-82244 (#456)

prevent NPE and correct process manager group condition

Author: hakermi

processes-api/src/main/java/org/exoplatform/processes/model/ProcessesFilter.java

@@ -28,5 +28,5 @@ public class ProcessesFilter {
   private String query;
   private Boolean enabled;
   private Boolean manager;
-
+  private Boolean isProcessManager;
 }

processes-services/src/main/java/org/exoplatform/processes/dao/WorkFlowDAO.java

@@ -55,17 +55,17 @@ private String  buildWorkflowQuery(ProcessesFilter processesFilter, List<String>
     String q = processesFilter.getQuery();
     Boolean enabled = processesFilter.getEnabled();
     Boolean manager = processesFilter.getManager();
-    boolean managerProcess = memberships.stream().anyMatch(m -> m.endsWith("platform/processes"));
+    boolean isProcessManager = processesFilter.getIsProcessManager();
     String query = " ( workFlow.title like '%" + q + "%' OR workFlow.description like '%" + q + "%' OR workFlow.summary like '%" + q + "%' )";
     String queryString = "SELECT DISTINCT workFlow FROM WorkFlow workFlow";
-    if (enabled != null || manager == true || managerProcess == false) {
-      if(memberships != null) {
+    if (enabled != null || Boolean.TRUE.equals(manager) || !isProcessManager) {
+      if (memberships != null) {
         if ( Boolean.FALSE.equals(manager)) {
           queryString = queryString + " LEFT JOIN workFlow.manager manager";
         }
         queryString = queryString + " LEFT JOIN workFlow.participator participator";
       }
-      if(StringUtils.isNotEmpty(q) || memberships != null || enabled != null){
+      if (StringUtils.isNotEmpty(q) || memberships != null || enabled != null){
         queryString = queryString + " WHERE";
         if (StringUtils.isNotEmpty(q)){
           queryString = queryString + query;

processes-services/src/main/java/org/exoplatform/processes/rest/ProcessesRest.java

@@ -50,6 +50,7 @@
 import org.exoplatform.services.rest.resource.ResourceContainer;
 import org.exoplatform.services.security.ConversationState;
 import org.exoplatform.services.security.Identity;
+import org.exoplatform.services.security.IdentityRegistry;
 import org.exoplatform.social.core.manager.IdentityManager;
 
 
@@ -63,6 +64,8 @@ public class ProcessesRest implements ResourceContainer {
 
   private IdentityManager  identityManager;
 
+  private IdentityRegistry identityRegistry;
+
   private ProcessesAttachmentService processesAttachmentService;
 
   private static final int           CACHE_DURATION_SECONDS      = 31536000;
@@ -71,15 +74,19 @@ public class ProcessesRest implements ResourceContainer {
 
   private static final CacheControl  ILLUSTRATION_CACHE_CONTROL  = new CacheControl();
 
+  private static final String              PROCESSES_GROUP          = "/platform/processes";
+
   static {
     ILLUSTRATION_CACHE_CONTROL.setMaxAge(CACHE_DURATION_SECONDS);
   }
 
   public ProcessesRest(ProcessesService processesService,
                        IdentityManager identityManager,
+                       IdentityRegistry identityRegistry,
                        ProcessesAttachmentService processesAttachmentService) {
     this.processesService = processesService;
     this.identityManager = identityManager;
+    this.identityRegistry = identityRegistry;
     this.processesAttachmentService = processesAttachmentService;
   }
 
@@ -133,10 +140,12 @@ public Response getWorkFlows(@Parameter(name = "Identity technical identifier",
       if (query != null) {
         filter.setQuery(query);
       }
+
       long userIdentityId = currentIdentityId;
       if (userId != null) {
         userIdentityId = userId;
       }
+      filter.setIsProcessManager(RestUtils.isProcessesGroupMember(identityManager, identityRegistry, userIdentityId));
       List<WorkFlow> workFlows = processesService.getWorkFlows(filter, offset, limit, userIdentityId);
       return Response.ok(EntityBuilder.toRestEntities(workFlows, expand)).build();
     } catch (Exception e) {

processes-services/src/main/java/org/exoplatform/processes/rest/util/RestUtils.java

@@ -17,6 +17,7 @@
 package org.exoplatform.processes.rest.util;
 
 import org.exoplatform.services.security.ConversationState;
+import org.exoplatform.services.security.IdentityRegistry;
 import org.exoplatform.social.core.identity.model.Identity;
 import org.exoplatform.social.core.manager.IdentityManager;
 
@@ -45,4 +46,18 @@ public static final long getCurrentUserIdentityId(IdentityManager identityManage
   public static boolean isProcessesGroupMember(org.exoplatform.services.security.Identity identity) {
     return identity != null && identity.isMemberOf(PROCESSES_GROUP);
   }
+
+  public static boolean isProcessesGroupMember(IdentityManager identityManager, IdentityRegistry identityRegistry, long userId) {
+    Identity identity = identityManager.getIdentity(userId);
+    if (identity == null) {
+      return false;
+    }
+
+    String remoteId = identity.getRemoteId();
+    if (remoteId == null) {
+      return false;
+    }
+
+    return isProcessesGroupMember(identityRegistry.getIdentity(remoteId));
+  }
 }

processes-services/src/main/java/org/exoplatform/processes/storage/ProcessesStorageImpl.java

@@ -585,7 +585,6 @@ public List<WorkFlow> findWorkFlows(ProcessesFilter processesFilter, long userId
           Collection<Membership> ms = organizationService.getMembershipHandler().findMembershipsByUser(userName);
           if (ms != null) {
             for (Membership membership : ms) {
-              isMemberProcessesGroup = false;
               if (membership.getGroupId().equals(PROCESSES_GROUP)) {
                 isMemberProcessesGroup = true;
               }

processes-services/src/test/java/org/exoplatform/processes/rest/EntityBuilderTest.java

@@ -74,7 +74,7 @@ public static void afterRunBare() throws Exception { // NOSONAR
   public void setUp() {
     RuntimeDelegate.setInstance(new RuntimeDelegateImpl());
     identityRegistry = mock(IdentityRegistry.class);
-    this.processesRest = new ProcessesRest(processesService, identityManager, processesAttachmentService);
+    this.processesRest = new ProcessesRest(processesService, identityManager, identityRegistry, processesAttachmentService);
   }
 
   @Test

processes-services/src/test/java/org/exoplatform/processes/rest/ProcessesRestTest.java

@@ -1,9 +1,7 @@
 package org.exoplatform.processes.rest;
 
 import static org.junit.Assert.assertEquals;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyLong;
-import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
@@ -25,6 +23,7 @@
 import javax.ws.rs.core.Response;
 import javax.ws.rs.ext.RuntimeDelegate;
 
+import org.exoplatform.services.security.IdentityRegistry;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.Test;
@@ -81,6 +80,9 @@ public class ProcessesRestTest {
   @Mock
   private ProcessesAttachmentService                                                          processesAttachmentService;
 
+  @Mock
+  private IdentityRegistry identityRegistry;
+
   private ProcessesRest                                                                       processesRest;
 
   @Mock
@@ -98,7 +100,7 @@ public static void afterRunBare() throws Exception { // NOSONAR
   @Before
   public void setUp() {
     RuntimeDelegate.setInstance(new RuntimeDelegateImpl());
-    this.processesRest = new ProcessesRest(processesService, identityManager, processesAttachmentService);
+    this.processesRest = new ProcessesRest(processesService, identityManager, identityRegistry, processesAttachmentService);
 
     ConversationState conversationState = mock(ConversationState.class);
     CONVERSATION_STATE.when(() -> ConversationState.getCurrent()).thenReturn(conversationState);
@@ -123,7 +125,7 @@ public void getWorkFlows() throws Exception {
     ENTITY_BUILDER.when(() -> EntityBuilder.toRestEntities(workFlows, null)).thenReturn(workFlowEntities);
     Response response2 = processesRest.getWorkFlows(1L, true, null, "test", null, 0, 10);
     assertEquals(response2.getStatus(), Response.Status.OK.getStatusCode());
-    when(processesService.getWorkFlows(processesFilter, 0, 10, 1L)).thenThrow(RuntimeException.class);
+    when(processesService.getWorkFlows(any(ProcessesFilter.class), anyInt(), anyInt(), anyLong())).thenThrow(RuntimeException.class);
     Response response3 = processesRest.getWorkFlows(1L, null, null, null, null, 0, 10);
     assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), response3.getStatus());
 

processes-services/src/test/java/org/exoplatform/processes/storage/ProcessesStorageImplTest.java

@@ -653,7 +653,7 @@ public void findWorkflow() throws Exception {
     newWorkFlowEntity1.setId(1L);
     newWorkFlowEntity1.setProjectId(1L);
     when(workFlowDAO.create(workFlowEntity)).thenReturn(newWorkFlowEntity);
-    ProcessesFilter filter = new ProcessesFilter("", null, null);
+    ProcessesFilter filter = new ProcessesFilter("", null, null, true);
     List<WorkFlowEntity> workFlowEntities = new ArrayList<>();
     workFlowEntities.add(newWorkFlowEntity);
     memberships = new ArrayList<>();
@@ -756,7 +756,7 @@ public void countWorkflow() {
     newWorkFlowEntity.toString();
     newWorkFlowEntity.equals(workFlow);
     when(workFlowDAO.create(workFlowEntity)).thenReturn(newWorkFlowEntity);
-    ProcessesFilter filter = new ProcessesFilter("", null, true);
+    ProcessesFilter filter = new ProcessesFilter("", null, true, true);
     when(workFlowDAO.countWorkFlows(filter)).thenReturn(1);
     when(workFlow.getIllustrativeAttachment()).thenReturn(illustrativeAttachment);
     List<CreatorIdentityEntity> identityEntities = new ArrayList<>();