Restrict starting processes to candidate starters (Activiti#4181)

* Activiti#4180 Restrict starting processes to candidate starters * fix codacy warnings * fix failing tests * fix license header * java doc typo fix * add static property EVERYONE_GROUP for * group * rename candidateStarter from exists to defined
RaphaelCSS · Dec 21, 2022 · 41b6075 · 41b6075
1 parent 0c6311a
commit 41b6075
Show file tree

Hide file tree

Showing 15 changed files with 227 additions and 31 deletions.
diff --git a/...-process-runtime-impl/src/main/java/org/activiti/runtime/api/impl/ProcessRuntimeImpl.java b/...-process-runtime-impl/src/main/java/org/activiti/runtime/api/impl/ProcessRuntimeImpl.java
@@ -15,6 +15,7 @@
  */
 package org.activiti.runtime.api.impl;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -67,14 +68,15 @@
 import org.activiti.runtime.api.model.impl.APIProcessInstanceConverter;
 import org.activiti.runtime.api.model.impl.APIVariableInstanceConverter;
 import org.activiti.runtime.api.query.impl.PageImpl;
-import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.transaction.annotation.Transactional;
 
 @PreAuthorize("hasRole('ACTIVITI_USER')")
 public class ProcessRuntimeImpl implements ProcessRuntime {
 
+    private static final String EVERYONE_GROUP = "*";
+
     private final RepositoryService repositoryService;
 
     private final APIProcessDefinitionConverter processDefinitionConverter;
@@ -99,9 +101,6 @@ public class ProcessRuntimeImpl implements ProcessRuntime {
 
     private final SecurityManager securityManager;
 
-    @Value("${activiti.candidateStarters.enabled:false}")
-    private boolean candidateStartersEnabled;
-
     public ProcessRuntimeImpl(RepositoryService repositoryService,
                               APIProcessDefinitionConverter processDefinitionConverter,
                               RuntimeService runtimeService,
@@ -147,11 +146,9 @@ public ProcessDefinition processDefinition(String processDefinitionId) {
     }
 
     private ProcessDefinitionQuery createProcessDefinitionQueryWithAccessCheck() {
-        ProcessDefinitionQuery processDefinitionQuery = repositoryService.createProcessDefinitionQuery();
-        if (candidateStartersEnabled) {
-            processDefinitionQuery.startableByUser(securityManager.getAuthenticatedUserId());
-        }
-        return processDefinitionQuery;
+        return repositoryService.createProcessDefinitionQuery()
+                                .startableByUser(securityManager.getAuthenticatedUserId())
+                                .startableByGroups(getCurrentUserGroupsIncludingEveryOneGroup());
     }
 
     private Optional<org.activiti.engine.repository.ProcessDefinition> findLatestProcessDefinition(ProcessDefinitionQuery processDefinitionQuery) {
@@ -555,4 +552,9 @@ private boolean isATaskAssigneeOrACandidate(String processInstanceId) {
         return taskQuery.count() > 0;
     }
 
+    private List<String> getCurrentUserGroupsIncludingEveryOneGroup() {
+        List<String> groups = new ArrayList<>(securityManager.getAuthenticatedUserGroups());
+        groups.add(EVERYONE_GROUP);
+        return groups;
+    }
 }
diff --git a/...cess-runtime-impl/src/test/java/org/activiti/runtime/api/impl/ProcessRuntimeImplTest.java b/...cess-runtime-impl/src/test/java/org/activiti/runtime/api/impl/ProcessRuntimeImplTest.java
@@ -155,6 +155,8 @@ public void updateShouldBeAbleToUpdateNameBusinessKey() {
 
     @Test
     public void should_getProcessDefinitionById_when_appVersionIsNull() {
+        doReturn("user").when(securityManager).getAuthenticatedUserId();
+
         String processDefinitionId = "processDefinitionId";
         String processDefinitionKey = "processDefinitionKey";
 
@@ -179,6 +181,8 @@ public void should_getProcessDefinitionById_when_appVersionIsNull() {
 
     @Test
     public void should_throwActivitiUnprocessableEntryException_when_processDefinitionAppVersionDiffersFromCurrentDeploymentVersion() {
+        doReturn("user").when(securityManager).getAuthenticatedUserId();
+
         String processDefinitionId = "processDefinitionId";
         ProcessDefinitionEntityImpl processDefinition = new ProcessDefinitionEntityImpl();
         processDefinition.setId(processDefinitionId);
@@ -207,6 +211,8 @@ public void should_throwActivitiUnprocessableEntryException_when_processDefiniti
 
     @Test
     public void should_throwActivitiObjectNotFoundException_when_canReadFalse() {
+        doReturn("user").when(securityManager).getAuthenticatedUserId();
+
         String processDefinitionId = "processDefinitionId";
         String processDefinitionKey = "processDefinitionKey";
         ProcessDefinitionEntityImpl processDefinition = new ProcessDefinitionEntityImpl();

diff --git a/...tiviti-bpmn-converter/src/main/java/org/activiti/bpmn/converter/parser/ProcessParser.java b/...tiviti-bpmn-converter/src/main/java/org/activiti/bpmn/converter/parser/ProcessParser.java
@@ -42,12 +42,20 @@ public Process parse(XMLStreamReader xtr, BpmnModel model) throws Exception {
       if (StringUtils.isNotEmpty(xtr.getAttributeValue(null, ATTRIBUTE_PROCESS_EXECUTABLE))) {
         process.setExecutable(Boolean.parseBoolean(xtr.getAttributeValue(null, ATTRIBUTE_PROCESS_EXECUTABLE)));
       }
+
       String candidateUsersString = xtr.getAttributeValue(ACTIVITI_EXTENSIONS_NAMESPACE, ATTRIBUTE_PROCESS_CANDIDATE_USERS);
+      if (candidateUsersString != null) {
+          process.setCandidateStarterUsersDefined(true);
+      }
       if (StringUtils.isNotEmpty(candidateUsersString)) {
         List<String> candidateUsers = BpmnXMLUtil.parseDelimitedList(candidateUsersString);
         process.setCandidateStarterUsers(candidateUsers);
       }
+
       String candidateGroupsString = xtr.getAttributeValue(ACTIVITI_EXTENSIONS_NAMESPACE, ATTRIBUTE_PROCESS_CANDIDATE_GROUPS);
+      if (candidateGroupsString != null) {
+          process.setCandidateStarterGroupsDefined(true);
+      }
       if (StringUtils.isNotEmpty(candidateGroupsString)) {
         List<String> candidateGroups = BpmnXMLUtil.parseDelimitedList(candidateGroupsString);
         process.setCandidateStarterGroups(candidateGroups);

diff --git a/activiti-core/activiti-bpmn-model/src/main/java/org/activiti/bpmn/model/Process.java b/activiti-core/activiti-bpmn-model/src/main/java/org/activiti/bpmn/model/Process.java
@@ -38,6 +38,8 @@ public class Process extends BaseElement implements FlowElementsContainer, HasEx
   protected List<String> candidateStarterGroups = new ArrayList<String>();
   protected List<EventListener> eventListeners = new ArrayList<EventListener>();
   protected Map<String, FlowElement> flowElementMap = new LinkedHashMap<String, FlowElement>();
+  protected boolean candidateStarterUsersDefined;
+  protected boolean candidateStarterGroupsDefined;
 
   // Added during process definition parsing
   protected FlowElement initialFlowElement;
@@ -274,6 +276,22 @@ public void setCandidateStarterGroups(List<String> candidateStarterGroups) {
     this.candidateStarterGroups = candidateStarterGroups;
   }
 
+  public boolean isCandidateStarterUsersDefined() {
+    return candidateStarterUsersDefined;
+  }
+
+  public void setCandidateStarterUsersDefined(boolean candidateStarterUsersDefined) {
+    this.candidateStarterUsersDefined = candidateStarterUsersDefined;
+  }
+
+  public boolean isCandidateStarterGroupsDefined() {
+    return candidateStarterGroupsDefined;
+  }
+
+  public void setCandidateStarterGroupsDefined(boolean candidateStarterGroupsDefined) {
+    this.candidateStarterGroupsDefined = candidateStarterGroupsDefined;
+  }
+
   public List<EventListener> getEventListeners() {
     return eventListeners;
   }

diff --git a/...re/activiti-engine/src/main/java/org/activiti/engine/impl/ProcessDefinitionQueryImpl.java b/...re/activiti-engine/src/main/java/org/activiti/engine/impl/ProcessDefinitionQueryImpl.java
@@ -59,6 +59,7 @@ public class ProcessDefinitionQueryImpl extends AbstractQuery<ProcessDefinitionQ
     private boolean latest;
     private SuspensionState suspensionState;
     private String authorizationUserId;
+    private List<String> authorizationGroups;
     private String procDefId;
     private String tenantId;
     private String tenantIdLike;
@@ -297,13 +298,17 @@ public ProcessDefinitionQuery eventSubscription(String eventType,
     }
 
     public List<String> getAuthorizationGroups() {
+        if (authorizationGroups != null) {
+            return authorizationGroups;
+        }
         // Similar behaviour as the TaskQuery.taskCandidateUser() which
         // includes the groups the candidate
         // user is part of
         if (authorizationUserId != null) {
             UserGroupManager userGroupManager = Context.getProcessEngineConfiguration().getUserGroupManager();
             if (userGroupManager != null) {
-                return userGroupManager.getUserGroups(authorizationUserId);
+                authorizationGroups = userGroupManager.getUserGroups(authorizationUserId);
+                return authorizationGroups;
             } else {
                 log.warn("No UserGroupManager set on ProcessEngineConfiguration. Tasks queried only where user is directly related, not through groups.");
             }
@@ -491,4 +496,9 @@ public ProcessDefinitionQueryImpl startableByUser(String userId) {
         this.authorizationUserId = userId;
         return this;
     }
+
+    public ProcessDefinitionQuery startableByGroups(List<String> groupIds) {
+        authorizationGroups = groupIds;
+        return this;
+    }
 }
diff --git a/...iti-engine/src/main/java/org/activiti/engine/impl/bpmn/deployer/BpmnDeploymentHelper.java b/...iti-engine/src/main/java/org/activiti/engine/impl/bpmn/deployer/BpmnDeploymentHelper.java
@@ -165,7 +165,7 @@ public void updateTimersAndEvents(ProcessDefinitionEntity processDefinition,
     timerManager.scheduleTimers(processDefinition, process);
   }
 
-  enum ExpressionType {
+  protected enum ExpressionType {
     USER, GROUP
   }
 

diff --git a/.../activiti-engine/src/main/java/org/activiti/engine/repository/ProcessDefinitionQuery.java b/.../activiti-engine/src/main/java/org/activiti/engine/repository/ProcessDefinitionQuery.java
@@ -15,6 +15,7 @@
  */
 package org.activiti.engine.repository;
 
+import java.util.List;
 import java.util.Set;
 
 import org.activiti.engine.ActivitiIllegalArgumentException;
@@ -140,10 +141,16 @@ public interface ProcessDefinitionQuery extends Query<ProcessDefinitionQuery, Pr
     ProcessDefinitionQuery processDefinitionResourceNameLike(String resourceNameLike);
 
     /**
-     * Only selects process definitions which given userId is authoriezed to start
+     * Only selects process definitions which given userId is authorized to start
      */
     ProcessDefinitionQuery startableByUser(String userId);
 
+    /**
+     * Only selects process definitions which given group members are authorized to start
+     * If not set and startableByUser is set, the groups of that user will be used
+     */
+    ProcessDefinitionQuery startableByGroups(List<String> groupIds);
+
     /**
      * Only selects process definitions which are suspended
      */

diff --git a/...e/activiti-engine/src/main/resources/org/activiti/db/mapping/entity/ProcessDefinition.xml b/...e/activiti-engine/src/main/resources/org/activiti/db/mapping/entity/ProcessDefinition.xml
@@ -252,10 +252,16 @@
       <if test="eventSubscriptionType != null">
       	and (EVT.EVENT_TYPE_ = #{eventSubscriptionType} and EVT.EVENT_NAME_ = #{eventSubscriptionName})
       </if>
-      <if test="authorizationUserId != null">
-        AND (exists (select ID_  from ${prefix}ACT_RU_IDENTITYLINK  IDN where IDN.PROC_DEF_ID_ = RES.ID_ and IDN.USER_ID_ = #{authorizationUserId})
+      <if test="authorizationUserId != null  || (authorizationGroups != null &amp;&amp; authorizationGroups.size() &gt; 0)">
+        AND (
+        <if test="authorizationUserId != null" >
+         exists (select ID_  from ${prefix}ACT_RU_IDENTITYLINK  IDN where IDN.PROC_DEF_ID_ = RES.ID_ and IDN.USER_ID_ = #{authorizationUserId})
+          <if test="authorizationGroups != null &amp;&amp; authorizationGroups.size() &gt; 0">
+          OR
+          </if>
+        </if>
         <if test="authorizationGroups != null &amp;&amp; authorizationGroups.size() &gt; 0">
-         OR exists (select ID_ from ${prefix}ACT_RU_IDENTITYLINK  IDN where IDN.PROC_DEF_ID_ = RES.ID_ and IDN.GROUP_ID_ IN
+         exists (select ID_ from ${prefix}ACT_RU_IDENTITYLINK  IDN where IDN.PROC_DEF_ID_ = RES.ID_ and IDN.GROUP_ID_ IN
             <foreach item="group" index="index" collection="authorizationGroups"
                      open="(" separator="," close=")">
               #{group}

diff --git a/...ine/src/test/java/org/activiti/engine/test/api/repository/ProcessDefinitionQueryTest.java b/...ine/src/test/java/org/activiti/engine/test/api/repository/ProcessDefinitionQueryTest.java
@@ -348,8 +348,39 @@ public void testQueryWithNullArgs() {
   }
 
   public void testQueryWithEmptyIdSet() {
-    List<ProcessDefinition> processDefinitionList = repositoryService.createProcessDefinitionQuery().processDefinitionIds(new HashSet<>(0)).list();
+    List<ProcessDefinition> processDefinitionList = repositoryService.createProcessDefinitionQuery()
+        .processDefinitionIds(new HashSet<>(0)).list();
     assertThat(processDefinitionList).isNotEmpty();
   }
 
+  public void testQueryWithNoCandidateStarters() {
+    List<ProcessDefinition> processDefinitionList = repositoryService.createProcessDefinitionQuery()
+        .startableByUser("user1").list();
+    assertThat(processDefinitionList).isEmpty();
+  }
+
+  public void testQueryWithCandidateStarterUser() {
+    String processDefinitionId = repositoryService.createProcessDefinitionQuery()
+        .processDefinitionKey("one")
+        .latestVersion()
+        .singleResult()
+        .getId();
+    repositoryService.addCandidateStarterUser(processDefinitionId,"user1");
+    List<ProcessDefinition> processDefinitionList = repositoryService.createProcessDefinitionQuery()
+        .startableByUser("user1").list();
+    assertThat(processDefinitionList).hasSize(1);
+  }
+
+  public void testQueryWithCandidateStarterGroup() {
+    String processDefinitionId = repositoryService.createProcessDefinitionQuery().
+        processDefinitionKey("one")
+        .latestVersion()
+        .singleResult()
+        .getId();
+    repositoryService.addCandidateStarterGroup(processDefinitionId,"group1");
+    List<ProcessDefinition> processDefinitionList = repositoryService.createProcessDefinitionQuery()
+        .startableByGroups(List.of("group1")).list();
+    assertThat(processDefinitionList).hasSize(1);
+  }
+
 }
diff --git a/...starter/src/main/java/org/activiti/spring/boot/CandidateStartersDeploymentConfigurer.java b/...starter/src/main/java/org/activiti/spring/boot/CandidateStartersDeploymentConfigurer.java
@@ -0,0 +1,46 @@
+/*
+ * Copyright 2010-2020 Alfresco Software, Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.activiti.spring.boot;
+
+import org.activiti.bpmn.model.Process;
+import org.activiti.engine.impl.bpmn.deployer.BpmnDeploymentHelper;
+import org.activiti.engine.impl.context.Context;
+import org.activiti.engine.impl.persistence.entity.ProcessDefinitionEntity;
+import org.activiti.spring.SpringProcessEngineConfiguration;
+
+import java.util.List;
+
+public class CandidateStartersDeploymentConfigurer implements ProcessEngineConfigurationConfigurer {
+
+    private static final String EVERYONE_GROUP = "*";
+
+    @Override
+    public void configure(SpringProcessEngineConfiguration processEngineConfiguration) {
+        processEngineConfiguration.setBpmnDeploymentHelper(new CandidateStartersDeploymentHelper());
+    }
+
+    public class CandidateStartersDeploymentHelper extends BpmnDeploymentHelper {
+        @Override
+        public void addAuthorizationsForNewProcessDefinition(Process process, ProcessDefinitionEntity processDefinition) {
+            super.addAuthorizationsForNewProcessDefinition(process, processDefinition);
+            if (process != null &&
+                !process.isCandidateStarterUsersDefined() &&
+                !process.isCandidateStarterGroupsDefined()) {
+                addAuthorizationsFromIterator(Context.getCommandContext(), List.of(EVERYONE_GROUP), processDefinition, ExpressionType.GROUP);
+            }
+        }
+    }
+}
diff --git a/...g-boot-starter/src/main/java/org/activiti/spring/boot/ProcessEngineAutoConfiguration.java b/...g-boot-starter/src/main/java/org/activiti/spring/boot/ProcessEngineAutoConfiguration.java
@@ -61,7 +61,6 @@
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.AutoConfigureAfter;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
-import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.context.annotation.Bean;
@@ -220,7 +219,6 @@ public ProcessDeployedEventProducer processDeployedEventProducer(RepositoryServi
 
     @Bean
     @ConditionalOnMissingBean
-    @ConditionalOnProperty("activiti.candidateStarters.enabled")
     public ProcessCandidateStartersEventProducer processCandidateStartersEventProducer(RepositoryService repositoryService,
                                                                                        @Autowired(required = false) List<ProcessRuntimeEventListener<ProcessCandidateStarterUserAddedEvent>> candidateStarterUserListeners,
                                                                                        @Autowired(required = false) List<ProcessRuntimeEventListener<ProcessCandidateStarterGroupAddedEvent>> candidateStarterGroupListeners) {
@@ -295,4 +293,10 @@ public ApplicationDeployedEventProducer applicationDeployedEventProducer(Reposit
                 eventPublisher);
     }
 
+    @Bean
+    @ConditionalOnMissingBean
+    public CandidateStartersDeploymentConfigurer candidateStartersDeploymentConfigurer() {
+        return new CandidateStartersDeploymentConfigurer();
+    }
+
 }
diff --git a/...tarter/src/test/java/org/activiti/spring/boot/process/ProcessCandidateStarterEventIT.java b/...tarter/src/test/java/org/activiti/spring/boot/process/ProcessCandidateStarterEventIT.java
@@ -25,7 +25,6 @@
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.context.SpringBootTest;
-import org.springframework.test.context.TestPropertySource;
 
 import java.util.List;
 
@@ -34,7 +33,6 @@
 
 
 @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.MOCK)
-@TestPropertySource(properties = "activiti.candidateStarters.enabled=true")
 public class ProcessCandidateStarterEventIT {
 
     @Autowired