From 2feda2f373b86c3287ee3f4f97634a17db1aa97f Mon Sep 17 00:00:00 2001 From: chvostek Date: Tue, 16 Dec 2025 16:28:22 +0100 Subject: [PATCH 01/12] [NAE-2303] TaskRef Security Improvements - improve dataSet behavior check --- .../engine/workflow/service/DataService.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java b/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java index bdfbbbf90dd..f54dae68768 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java +++ b/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java @@ -232,6 +232,9 @@ public SetDataEventOutcome setData(Task task, ObjectNode values, Map> behaviorMap = dataField.getBehavior(); + if (behaviorMap == null) { + return false; + } + Set behaviorSet = behaviorMap.get(transId); + return behaviorSet != null && behaviorSet.contains(FieldBehavior.EDITABLE); + } + @Override public GetDataGroupsEventOutcome getDataGroups(String taskId, Locale locale) { return getDataGroups(taskId, locale, new HashSet<>(), 0, null); From 2d75ead14a83d56e7825a8d7c584ebf658b447cc Mon Sep 17 00:00:00 2001 From: chvostek Date: Wed, 17 Dec 2025 12:03:27 +0100 Subject: [PATCH 02/12] [NAE-2303] TaskRef Security Improvements - forbid setData for taskRef and caseRef fields on endpoint calls --- .../engine/workflow/service/DataService.java | 13 +++++++++++++ .../workflow/service/interfaces/IDataService.java | 2 ++ .../engine/workflow/web/AbstractTaskController.java | 7 ++++++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java b/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java index f54dae68768..de0c86b3433 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java +++ b/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java @@ -68,6 +68,8 @@ public class DataService implements IDataService { public static final int MONGO_ID_LENGTH = 24; + private static final Set setDataForbiddenFieldTypes = Set.of(FieldType.TASK_REF, FieldType.CASE_REF); + @Autowired protected ApplicationEventPublisher publisher; @@ -219,6 +221,11 @@ public SetDataEventOutcome setData(Task task, ObjectNode values) { @Override public SetDataEventOutcome setData(Task task, ObjectNode values, Map params) { + return setData(task, values, params, false); + } + + @Override + public SetDataEventOutcome setData(Task task, ObjectNode values, Map params, boolean applyForbiddenTypes) { Case useCase = workflowService.findOne(task.getCaseId()); IUser user = userService.getLoggedOrSystem(); @@ -230,6 +237,12 @@ public SetDataEventOutcome setData(Task task, ObjectNode values, Map { String fieldId = entry.getKey(); + if (applyForbiddenTypes) { + FieldType fieldType = useCase.getField(fieldId).getType(); + if (setDataForbiddenFieldTypes.contains(fieldType)) { + return; + } + } DataField dataField = useCase.getDataSet().get(fieldId); if (dataField != null) { if (!isDataFieldEditable(dataField, task.getTransitionId())) { diff --git a/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.java b/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.java index 48dc9244a9d..d143fe4b7ad 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.java +++ b/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.java @@ -36,6 +36,8 @@ public interface IDataService { SetDataEventOutcome setData(Task task, ObjectNode values, Map params); + SetDataEventOutcome setData(Task task, ObjectNode values, Map params, boolean applyForbiddenTypes); + FileFieldInputStream getFile(Case useCase, Task task, FileField field, boolean forPreview) throws FileNotFoundException; FileFieldInputStream getFile(Case useCase, Task task, FileField field, boolean forPreview, Map params) throws FileNotFoundException; diff --git a/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java b/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java index 890e053fcde..1c2b04d50ba 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java +++ b/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java @@ -211,7 +211,12 @@ public EntityModel getData(String taskId, Locale locale public EntityModel setData(String taskId, ObjectNode dataBody, Locale locale) { try { Map outcomes = new HashMap<>(); - dataBody.fields().forEachRemaining(it -> outcomes.put(it.getKey(), dataService.setData(it.getKey(), it.getValue().deepCopy()))); + dataBody.fields().forEachRemaining(fieldChangesEntry -> { + String taskIdToChangeWith = fieldChangesEntry.getKey(); + Task taskToChangeWith = taskService.findOne(taskIdToChangeWith); + outcomes.put(taskIdToChangeWith, dataService.setData(taskToChangeWith, + fieldChangesEntry.getValue().deepCopy(), new HashMap<>(), true)); + }); SetDataEventOutcome mainOutcome = taskService.getMainOutcome(outcomes, taskId); return EventOutcomeWithMessageResource.successMessage("Data field values have been successfully set", LocalisedEventOutcomeFactory.from(mainOutcome, LocaleContextHolder.getLocale())); From 8e0a6c4e4af2c0c54f22e30865708db688dca830 Mon Sep 17 00:00:00 2001 From: chvostek Date: Wed, 17 Dec 2025 14:48:29 +0100 Subject: [PATCH 03/12] [NAE-2303] TaskRef Security Improvements - improve security --- .../engine/workflow/service/DataService.java | 9 +++-- .../engine/workflow/service/TaskService.java | 3 ++ .../workflow/web/AbstractTaskController.java | 35 ++++++++++++++++--- .../workflow/web/PublicTaskController.java | 6 ++-- .../engine/workflow/web/TaskController.java | 6 ++-- 5 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java b/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java index de0c86b3433..d5d8db4bc37 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java +++ b/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java @@ -224,8 +224,11 @@ public SetDataEventOutcome setData(Task task, ObjectNode values, Map params, boolean applyForbiddenTypes) { + public SetDataEventOutcome setData(Task task, ObjectNode values, Map params, boolean runSafe) { Case useCase = workflowService.findOne(task.getCaseId()); IUser user = userService.getLoggedOrSystem(); @@ -237,7 +240,7 @@ public SetDataEventOutcome setData(Task task, ObjectNode values, Map { String fieldId = entry.getKey(); - if (applyForbiddenTypes) { + if (runSafe) { FieldType fieldType = useCase.getField(fieldId).getType(); if (setDataForbiddenFieldTypes.contains(fieldType)) { return; @@ -245,7 +248,7 @@ public SetDataEventOutcome setData(Task task, ObjectNode values, Map outco } } mainOutcome = outcomes.remove(key); + if (mainOutcome == null) { + return null; + } mainOutcome.addOutcomes(new ArrayList<>(outcomes.values())); return mainOutcome; } diff --git a/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java b/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java index 1c2b04d50ba..74481c9f6a4 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java +++ b/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java @@ -5,6 +5,7 @@ import com.netgrif.application.engine.elastic.service.interfaces.IElasticTaskService; import com.netgrif.application.engine.elastic.web.requestbodies.singleaslist.SingleElasticTaskSearchRequestAsList; import com.netgrif.application.engine.eventoutcomes.LocalisedEventOutcomeFactory; +import com.netgrif.application.engine.petrinet.domain.dataset.FieldType; import com.netgrif.application.engine.petrinet.domain.throwable.TransitionNotExecutableException; import com.netgrif.application.engine.workflow.domain.IllegalArgumentWithChangedFieldsException; import com.netgrif.application.engine.workflow.domain.MergeFilterOperation; @@ -16,6 +17,7 @@ import com.netgrif.application.engine.workflow.service.FileFieldInputStream; import com.netgrif.application.engine.workflow.service.interfaces.IDataService; import com.netgrif.application.engine.workflow.service.interfaces.ITaskService; +import com.netgrif.application.engine.workflow.service.interfaces.IWorkflowService; import com.netgrif.application.engine.workflow.web.requestbodies.file.FileFieldRequest; import com.netgrif.application.engine.workflow.web.requestbodies.singleaslist.SingleTaskSearchRequestAsList; import com.netgrif.application.engine.workflow.web.responsebodies.*; @@ -38,10 +40,8 @@ import org.springframework.web.multipart.MultipartFile; import java.io.FileNotFoundException; -import java.util.HashMap; -import java.util.List; -import java.util.Locale; -import java.util.Map; +import java.util.*; +import java.util.stream.Collectors; public abstract class AbstractTaskController { @@ -51,11 +51,15 @@ public abstract class AbstractTaskController { private final IDataService dataService; + private final IWorkflowService workflowService; + private final IElasticTaskService searchService; - public AbstractTaskController(ITaskService taskService, IDataService dataService, IElasticTaskService searchService) { + public AbstractTaskController(ITaskService taskService, IDataService dataService, IWorkflowService workflowService, + IElasticTaskService searchService) { this.taskService = taskService; this.dataService = dataService; + this.workflowService = workflowService; this.searchService = searchService; } @@ -210,14 +214,35 @@ public EntityModel getData(String taskId, Locale locale public EntityModel setData(String taskId, ObjectNode dataBody, Locale locale) { try { + List dataGroups = dataService.getDataGroups(taskId, locale).getData(); + Set referencedTaskIds = new HashSet<>(); + referencedTaskIds.add(taskId); + for (com.netgrif.application.engine.petrinet.domain.DataGroup dataGroup : dataGroups) { + // todo 2303 NPE + Set referencedTaskIdsByDataGroup = dataGroup.getFields().getContent().stream() + .filter(localisedField -> localisedField.getType() == FieldType.TASK_REF + && localisedField.getValue() instanceof List + && !((List) localisedField.getValue()).isEmpty()) + .map(localisedField -> (List) localisedField.getValue()) + .flatMap(List::stream) + .collect(Collectors.toSet()); + referencedTaskIds.addAll(referencedTaskIdsByDataGroup); + } Map outcomes = new HashMap<>(); dataBody.fields().forEachRemaining(fieldChangesEntry -> { String taskIdToChangeWith = fieldChangesEntry.getKey(); + if (!referencedTaskIds.contains(taskIdToChangeWith)) { + return; + } Task taskToChangeWith = taskService.findOne(taskIdToChangeWith); outcomes.put(taskIdToChangeWith, dataService.setData(taskToChangeWith, fieldChangesEntry.getValue().deepCopy(), new HashMap<>(), true)); }); SetDataEventOutcome mainOutcome = taskService.getMainOutcome(outcomes, taskId); + if (mainOutcome == null) { + Task task = taskService.findOne(taskId); + mainOutcome = new SetDataEventOutcome(workflowService.findOne(task.getCaseId()), task); + } return EventOutcomeWithMessageResource.successMessage("Data field values have been successfully set", LocalisedEventOutcomeFactory.from(mainOutcome, LocaleContextHolder.getLocale())); } catch (IllegalArgumentWithChangedFieldsException e) { diff --git a/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java b/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java index 30294e760c9..02818262191 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java +++ b/src/main/java/com/netgrif/application/engine/workflow/web/PublicTaskController.java @@ -7,6 +7,7 @@ import com.netgrif.application.engine.workflow.domain.eventoutcomes.response.EventOutcomeWithMessage; import com.netgrif.application.engine.workflow.service.interfaces.IDataService; import com.netgrif.application.engine.workflow.service.interfaces.ITaskService; +import com.netgrif.application.engine.workflow.service.interfaces.IWorkflowService; import com.netgrif.application.engine.workflow.web.requestbodies.file.FileFieldRequest; import com.netgrif.application.engine.workflow.web.requestbodies.singleaslist.SingleTaskSearchRequestAsList; import com.netgrif.application.engine.workflow.web.responsebodies.LocalisedTaskResource; @@ -49,8 +50,9 @@ public class PublicTaskController extends AbstractTaskController { private final ITaskService taskService; private final IDataService dataService; - public PublicTaskController(ITaskService taskService, IDataService dataService, IUserService userService) { - super(taskService, dataService, null); + public PublicTaskController(ITaskService taskService, IDataService dataService, IUserService userService, + IWorkflowService workflowService) { + super(taskService, dataService, workflowService, null); this.taskService = taskService; this.dataService = dataService; this.userService = userService; diff --git a/src/main/java/com/netgrif/application/engine/workflow/web/TaskController.java b/src/main/java/com/netgrif/application/engine/workflow/web/TaskController.java index b9690306c3f..593f37ce4ee 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/web/TaskController.java +++ b/src/main/java/com/netgrif/application/engine/workflow/web/TaskController.java @@ -9,6 +9,7 @@ import com.netgrif.application.engine.workflow.domain.eventoutcomes.response.EventOutcomeWithMessage; import com.netgrif.application.engine.workflow.service.interfaces.IDataService; import com.netgrif.application.engine.workflow.service.interfaces.ITaskService; +import com.netgrif.application.engine.workflow.service.interfaces.IWorkflowService; import com.netgrif.application.engine.workflow.web.requestbodies.file.FileFieldRequest; import com.netgrif.application.engine.workflow.web.requestbodies.singleaslist.SingleTaskSearchRequestAsList; import com.netgrif.application.engine.workflow.web.responsebodies.CountResponse; @@ -51,8 +52,9 @@ public class TaskController extends AbstractTaskController { public static final Logger log = LoggerFactory.getLogger(TaskController.class); - public TaskController(ITaskService taskService, IDataService dataService, IElasticTaskService searchService) { - super(taskService, dataService, searchService); + public TaskController(ITaskService taskService, IDataService dataService, IWorkflowService workflowService, + IElasticTaskService searchService) { + super(taskService, dataService, workflowService, searchService); } @Override From 339b64de38ba3d26207a99214d862a2f9a3523cc Mon Sep 17 00:00:00 2001 From: chvostek Date: Thu, 18 Dec 2025 07:41:10 +0100 Subject: [PATCH 04/12] [NAE-2303] TaskRef Security Improvements - add javadoc --- .../engine/workflow/service/DataService.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java b/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java index d5d8db4bc37..8c66f711100 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java +++ b/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java @@ -225,7 +225,15 @@ public SetDataEventOutcome setData(Task task, ObjectNode values, Map params, boolean runSafe) { From 6bb523d8a2990e7ff6cdcd5ca16b206d447084d9 Mon Sep 17 00:00:00 2001 From: chvostek Date: Thu, 18 Dec 2025 07:46:45 +0100 Subject: [PATCH 05/12] [NAE-2303] TaskRef Security Improvements - remove todo --- .../application/engine/workflow/web/AbstractTaskController.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java b/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java index 74481c9f6a4..ede4f088b1b 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java +++ b/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java @@ -218,7 +218,6 @@ public EntityModel setData(String taskId, ObjectNode da Set referencedTaskIds = new HashSet<>(); referencedTaskIds.add(taskId); for (com.netgrif.application.engine.petrinet.domain.DataGroup dataGroup : dataGroups) { - // todo 2303 NPE Set referencedTaskIdsByDataGroup = dataGroup.getFields().getContent().stream() .filter(localisedField -> localisedField.getType() == FieldType.TASK_REF && localisedField.getValue() instanceof List From 1dc1dc7cf54c81ed921362c949fc9ced63a820a7 Mon Sep 17 00:00:00 2001 From: chvostek Date: Thu, 18 Dec 2025 08:56:03 +0100 Subject: [PATCH 06/12] [NAE-2303] TaskRef Security Improvements - implement task controller tests --- .../engine/workflow/TaskControllerTest.groovy | 110 +++++++++++-- .../petriNets/task_controller_set_data.xml | 154 ++++++++++++++++++ 2 files changed, 252 insertions(+), 12 deletions(-) create mode 100644 src/test/resources/petriNets/task_controller_set_data.xml diff --git a/src/test/groovy/com/netgrif/application/engine/workflow/TaskControllerTest.groovy b/src/test/groovy/com/netgrif/application/engine/workflow/TaskControllerTest.groovy index ce970f467d6..867c387bb42 100644 --- a/src/test/groovy/com/netgrif/application/engine/workflow/TaskControllerTest.groovy +++ b/src/test/groovy/com/netgrif/application/engine/workflow/TaskControllerTest.groovy @@ -1,5 +1,7 @@ package com.netgrif.application.engine.workflow +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.databind.node.ObjectNode import com.netgrif.application.engine.TestHelper import com.netgrif.application.engine.auth.domain.Authority import com.netgrif.application.engine.auth.domain.User @@ -9,7 +11,6 @@ import com.netgrif.application.engine.auth.service.interfaces.IUserService import com.netgrif.application.engine.elastic.service.interfaces.IElasticTaskService import com.netgrif.application.engine.petrinet.domain.PetriNet import com.netgrif.application.engine.petrinet.domain.VersionType -import com.netgrif.application.engine.petrinet.domain.dataset.FileFieldValue import com.netgrif.application.engine.petrinet.domain.dataset.FileListFieldValue import com.netgrif.application.engine.petrinet.domain.roles.ProcessRole import com.netgrif.application.engine.petrinet.service.ProcessRoleService @@ -18,12 +19,12 @@ import com.netgrif.application.engine.startup.ImportHelper import com.netgrif.application.engine.startup.SuperCreator import com.netgrif.application.engine.utils.FullPageRequest import com.netgrif.application.engine.workflow.domain.Case +import com.netgrif.application.engine.workflow.domain.DataField import com.netgrif.application.engine.workflow.domain.Task import com.netgrif.application.engine.workflow.service.TaskSearchService import com.netgrif.application.engine.workflow.service.TaskService import com.netgrif.application.engine.workflow.service.interfaces.IDataService import com.netgrif.application.engine.workflow.service.interfaces.IWorkflowService -import com.netgrif.application.engine.workflow.web.PublicTaskController import com.netgrif.application.engine.workflow.web.TaskController import com.netgrif.application.engine.workflow.web.WorkflowController import com.netgrif.application.engine.workflow.web.requestbodies.TaskSearchRequest @@ -87,7 +88,9 @@ class TaskControllerTest { @Autowired private TaskController taskController - private PetriNet net + private PetriNet allDataNet + + private PetriNet setDataNet private Case useCase @@ -106,7 +109,7 @@ class TaskControllerTest { state: UserState.ACTIVE, authorities: [authorityService.getOrCreate(Authority.user)] as Set, processRoles: [] as Set)) - importNet() + importNets() } @Test @@ -118,7 +121,7 @@ class TaskControllerTest { @Test void testDeleteFile() { - Case testCase = helper.createCase("My case", net) + Case testCase = helper.createCase("My case", allDataNet) String taskId = testCase.tasks.find {it.transition == "1"}.task dataService.saveFile(taskId, "file", new MockMultipartFile("test", new byte[] {})) @@ -132,7 +135,7 @@ class TaskControllerTest { @Test void testDeleteFileByName() { - Case testCase = helper.createCase("My case", net) + Case testCase = helper.createCase("My case", allDataNet) String taskId = testCase.tasks.find {it.transition == "1"}.task dataService.saveFiles(taskId, "fileList", new MockMultipartFile[] {new MockMultipartFile("test", "test", null, new byte[] {})}) @@ -144,6 +147,79 @@ class TaskControllerTest { assert ((FileListFieldValue) testCase.dataSet["fileList"].value).namesPaths == null || ((FileListFieldValue) testCase.dataSet["fileList"].value).namesPaths.size() == 0 } + @Test + void testSetDataFieldTypeRestriction() { + Case testCase = helper.createCase("My case", setDataNet) + String taskId = testCase.tasks.find { it.transition == "testSetDataFieldTypeRestriction" }.task + + ObjectNode dataSet = populateDataset([(taskId):["taskRef_0": ["type": "taskRef", "value": [taskId]]]]) + def response = taskController.setData(taskId, dataSet, Locale.default) + assert response != null && response.content.outcome != null + assert response.content.outcome.changedFields.changedFields.isEmpty() + assert ((List) workflowService.findOne(testCase.stringId).getDataField("taskRef_0").getValue()).isEmpty() + + dataSet = populateDataset([(taskId):["caseRef_0": ["type": "caseRef", "value": [testCase.stringId]]]]) + response = taskController.setData(taskId, dataSet, Locale.default) + assert response != null && response.content.outcome != null + assert response.content.outcome.changedFields.changedFields.isEmpty() + assert ((List) workflowService.findOne(testCase.stringId).getDataField("caseRef_0").getValue()).isEmpty() + } + + @Test + void testSetDataVisibleField() { + Case testCase = helper.createCase("My case", setDataNet) + String taskId = testCase.tasks.find { it.transition == "data" }.task + + ObjectNode dataSet = populateDataset([(taskId):["text_1": ["type": "text", "value": "awd"]]]) + def response = taskController.setData(taskId, dataSet, Locale.default) + assert response != null && response.content.outcome == null + assert response.content.error != null + + // todo: test visible behavior based on parent taskRef behavior + } + + @Test + void testSetDataNestedTaskRefRestrictions() { + Case testCase1 = helper.createCase("testCase1", setDataNet) + String taskId = testCase1.tasks.find { it.transition == "testSetDataNestedTaskRefRestrictions" }.task + Case testCase2 = helper.createCase("testCase2", setDataNet) + Case testCase3 = helper.createCase("testCase3", setDataNet) + + DataField case1DataField = testCase1.getDataField("taskRef_0") + case1DataField.setValue(List.of(testCase2.tasks.find { it.transition == "testSetDataNestedTaskRefRestrictions" }.task)) + workflowService.save(testCase1) + + DataField case2DataField = testCase2.getDataField("taskRef_0") + case2DataField.setValue(List.of(testCase3.tasks.find { it.transition == "data" }.task)) + workflowService.save(testCase2) + + String nestedOtherTaskId = testCase2.tasks.find { it.transition == "data" }.task + ObjectNode dataSet = populateDataset([(nestedOtherTaskId):["text_0": ["type": "text", "value": "awd"]]]) + def response = taskController.setData(taskId, dataSet, Locale.default) + assert response != null && response.content.outcome != null + assert response.content.outcome.changedFields.changedFields.isEmpty() + assert workflowService.findOne(testCase2.stringId).getDataField("text_0").getValue() == null + + String nestedTaskId = testCase3.tasks.find { it.transition == "data" }.task + dataSet = populateDataset([(nestedTaskId):["text_0": ["type": "text", "value": "awd"]]]) + response = taskController.setData(taskId, dataSet, Locale.default) + assert response != null && response.content.outcome != null + assert !response.content.outcome.changedFields.changedFields.isEmpty() + assert workflowService.findOne(testCase3.stringId).getDataField("text_0").getValue() == "awd" + } + + @Test + void testSetDataNonReferencedField() { + Case testCase = helper.createCase("My case", setDataNet) + String taskId = testCase.tasks.find { it.transition == "testSetDataNonReferencedField" }.task + + ObjectNode dataSet = populateDataset([(taskId):["text_1": ["type": "text", "value": "awd"]]]) + def response = taskController.setData(taskId, dataSet, Locale.default) + + assert response != null && response.content.outcome == null + assert response.content.error != null + } + void testWithRoleAndUserref() { createCase() @@ -167,15 +243,19 @@ class TaskControllerTest { assert !findTasksByMongo().empty } - void importNet() { - PetriNet netOptional = helper.createNet("all_data_refs.xml", VersionType.MAJOR).get() - assert netOptional != null - net = netOptional + void importNets() { + PetriNet allDataNet = helper.createNet("all_data_refs.xml", VersionType.MAJOR).get() + assert allDataNet != null + this.allDataNet = allDataNet + + PetriNet setDataNet = helper.createNet("task_controller_set_data.xml", VersionType.MAJOR).get() + assert setDataNet != null + this.setDataNet = setDataNet } void createCase() { useCase = null - useCase = helper.createCase("My case", net) + useCase = helper.createCase("My case", allDataNet) assert useCase != null } @@ -203,7 +283,7 @@ class TaskControllerTest { } void setUserRole() { - List roles = processRoleService.findAll(net.stringId) + List roles = processRoleService.findAll(allDataNet.stringId) for (ProcessRole role : roles) { if (role.importId == "process_role") { @@ -219,4 +299,10 @@ class TaskControllerTest { Page tasks = taskService.search(taskSearchRequestList, new FullPageRequest(), userService.findByEmail(DUMMY_USER_MAIL, false).transformToLoggedUser(), new Locale("en"), false) return tasks } + + static ObjectNode populateDataset(Map>> data) { + ObjectMapper mapper = new ObjectMapper() + String json = mapper.writeValueAsString(data) + return mapper.readTree(json) as ObjectNode + } } diff --git a/src/test/resources/petriNets/task_controller_set_data.xml b/src/test/resources/petriNets/task_controller_set_data.xml new file mode 100644 index 00000000000..60743a813ad --- /dev/null +++ b/src/test/resources/petriNets/task_controller_set_data.xml @@ -0,0 +1,154 @@ + + task_controller_set_data + 1.0.0 + TCS + task_controller_set_data + device_hub + true + true + false + + taskRef_0 + + </data> + <data type="caseRef"> + <id>caseRef_0</id> + <title/> + </data> + <data type="text"> + <id>text_0</id> + <title/> + </data> + <data type="text"> + <id>text_1</id> + <title/> + </data> + <transition> + <id>testSetDataFieldTypeRestriction</id> + <x>720</x> + <y>336</y> + <label>testSetDataFieldTypeRestriction</label> + <dataGroup> + <id>testSetDataFieldTypeRestriction_0</id> + <cols>4</cols> + <layout>grid</layout> + <dataRef> + <id>taskRef_0</id> + <logic> + <behavior>editable</behavior> + </logic> + <layout> + <x>1</x> + <y>1</y> + <rows>1</rows> + <cols>2</cols> + <template>material</template> + <appearance>outline</appearance> + </layout> + </dataRef> + <dataRef> + <id>caseRef_0</id> + <logic> + <behavior>editable</behavior> + </logic> + <layout> + <x>1</x> + <y>2</y> + <rows>1</rows> + <cols>2</cols> + <template>material</template> + <appearance>outline</appearance> + </layout> + </dataRef> + </dataGroup> + </transition> + <transition> + <id>testSetDataNestedTaskRefRestrictions</id> + <x>720</x> + <y>436</y> + <label>testSetDataNestedTaskRefRestrictions</label> + <dataGroup> + <id>testSetDataNestedTaskRefRestrictions_0</id> + <cols>4</cols> + <layout>grid</layout> + <dataRef> + <id>taskRef_0</id> + <logic> + <behavior>editable</behavior> + </logic> + <layout> + <x>1</x> + <y>1</y> + <rows>1</rows> + <cols>2</cols> + <template>material</template> + <appearance>outline</appearance> + </layout> + </dataRef> + </dataGroup> + </transition> + <transition> + <id>data</id> + <x>720</x> + <y>564</y> + <label>data</label> + <dataGroup> + <id>data_0</id> + <cols>4</cols> + <layout>grid</layout> + <dataRef> + <id>text_0</id> + <logic> + <behavior>editable</behavior> + </logic> + <layout> + <x>1</x> + <y>1</y> + <rows>1</rows> + <cols>2</cols> + <template>material</template> + <appearance>outline</appearance> + </layout> + </dataRef> + <dataRef> + <id>text_1</id> + <logic> + <behavior>visible</behavior> + </logic> + <layout> + <x>1</x> + <y>2</y> + <rows>1</rows> + <cols>2</cols> + <template>material</template> + <appearance>outline</appearance> + </layout> + </dataRef> + </dataGroup> + </transition> + <transition> + <id>testSetDataNonReferencedField</id> + <x>720</x> + <y>664</y> + <label>testSetDataNonReferencedField</label> + <dataGroup> + <id>data_0</id> + <cols>4</cols> + <layout>grid</layout> + <dataRef> + <id>text_0</id> + <logic> + <behavior>editable</behavior> + </logic> + <layout> + <x>1</x> + <y>1</y> + <rows>1</rows> + <cols>2</cols> + <template>material</template> + <appearance>outline</appearance> + </layout> + </dataRef> + </dataGroup> + </transition> +</document> \ No newline at end of file From c0cb354c2a2d899dc3b7b2100316743a160ff04b Mon Sep 17 00:00:00 2001 From: chvostek <chvostek@netgrif.com> Date: Fri, 19 Dec 2025 08:12:13 +0100 Subject: [PATCH 07/12] [NAE-2303] TaskRef Security Improvements - handle possible NPE in DataService --- .../application/engine/workflow/service/DataService.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java b/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java index 8c66f711100..2078aef801a 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java +++ b/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java @@ -249,8 +249,11 @@ public SetDataEventOutcome setData(Task task, ObjectNode values, Map<String, Str values.fields().forEachRemaining(entry -> { String fieldId = entry.getKey(); if (runSafe) { - FieldType fieldType = useCase.getField(fieldId).getType(); - if (setDataForbiddenFieldTypes.contains(fieldType)) { + Field<?> field = useCase.getField(fieldId); + if (field == null) { + throw new IllegalArgumentException("Such field with id [" + fieldId + "] does not exist in petri net [" + useCase.getPetriNetId() + "]"); + } + if (setDataForbiddenFieldTypes.contains(field.getType())) { return; } } From 87916b25b403298a04286b595bd71b75896e6616 Mon Sep 17 00:00:00 2001 From: chvostek <chvostek@netgrif.com> Date: Fri, 19 Dec 2025 08:12:50 +0100 Subject: [PATCH 08/12] [NAE-2303] TaskRef Security Improvements - rename parameter in IDataService --- .../engine/workflow/service/interfaces/IDataService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.java b/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.java index d143fe4b7ad..3f3f11724a6 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.java +++ b/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.java @@ -36,7 +36,7 @@ public interface IDataService { SetDataEventOutcome setData(Task task, ObjectNode values, Map<String, String> params); - SetDataEventOutcome setData(Task task, ObjectNode values, Map<String, String> params, boolean applyForbiddenTypes); + SetDataEventOutcome setData(Task task, ObjectNode values, Map<String, String> params, boolean runSafe); FileFieldInputStream getFile(Case useCase, Task task, FileField field, boolean forPreview) throws FileNotFoundException; From 629ae6f32248e85bfa7c7ddca2cd58f00a6fe05f Mon Sep 17 00:00:00 2001 From: chvostek <chvostek@netgrif.com> Date: Fri, 19 Dec 2025 08:17:44 +0100 Subject: [PATCH 09/12] [NAE-2303] TaskRef Security Improvements - handle possible NPE in AbstractTaskController - fix typo --- .../workflow/web/AbstractTaskController.java | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java b/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java index ede4f088b1b..0e295542652 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java +++ b/src/main/java/com/netgrif/application/engine/workflow/web/AbstractTaskController.java @@ -238,10 +238,7 @@ public EntityModel<EventOutcomeWithMessage> setData(String taskId, ObjectNode da fieldChangesEntry.getValue().deepCopy(), new HashMap<>(), true)); }); SetDataEventOutcome mainOutcome = taskService.getMainOutcome(outcomes, taskId); - if (mainOutcome == null) { - Task task = taskService.findOne(taskId); - mainOutcome = new SetDataEventOutcome(workflowService.findOne(task.getCaseId()), task); - } + mainOutcome = handleMainSetDataEventOutcome(mainOutcome, taskId); return EventOutcomeWithMessageResource.successMessage("Data field values have been successfully set", LocalisedEventOutcomeFactory.from(mainOutcome, LocaleContextHolder.getLocale())); } catch (IllegalArgumentWithChangedFieldsException e) { @@ -259,6 +256,7 @@ public EntityModel<EventOutcomeWithMessage> saveFile(String taskId, MultipartFil Map<String, SetDataEventOutcome> outcomes = new HashMap<>(); outcomes.put(dataBody.getParentTaskId(), dataService.saveFile(dataBody.getParentTaskId(), dataBody.getFieldId(), multipartFile)); SetDataEventOutcome mainOutcome = taskService.getMainOutcome(outcomes, taskId); + mainOutcome = handleMainSetDataEventOutcome(mainOutcome, taskId); return EventOutcomeWithMessageResource.successMessage("Data field values have been successfully set", LocalisedEventOutcomeFactory.from(mainOutcome, LocaleContextHolder.getLocale())); } catch (IllegalArgumentWithChangedFieldsException e) { @@ -291,7 +289,8 @@ public EntityModel<EventOutcomeWithMessage> deleteFile(String taskId, String fie Map<String, SetDataEventOutcome> outcomes = new HashMap<>(); outcomes.put(taskId, dataService.deleteFile(taskId, fieldId)); SetDataEventOutcome mainOutcome = taskService.getMainOutcome(outcomes, taskId); - return EventOutcomeWithMessageResource.successMessage("Data field values have been sucessfully set", + mainOutcome = handleMainSetDataEventOutcome(mainOutcome, taskId); + return EventOutcomeWithMessageResource.successMessage("Data field values have been successfully set", LocalisedEventOutcomeFactory.from(mainOutcome, LocaleContextHolder.getLocale())); } @@ -299,7 +298,8 @@ public EntityModel<EventOutcomeWithMessage> saveFiles(String taskId, MultipartFi Map<String, SetDataEventOutcome> outcomes = new HashMap<>(); outcomes.put(requestBody.getParentTaskId(), dataService.saveFiles(requestBody.getParentTaskId(), requestBody.getFieldId(), multipartFiles)); SetDataEventOutcome mainOutcome = taskService.getMainOutcome(outcomes, taskId); - return EventOutcomeWithMessageResource.successMessage("Data field values have been sucessfully set", + mainOutcome = handleMainSetDataEventOutcome(mainOutcome, taskId); + return EventOutcomeWithMessageResource.successMessage("Data field values have been successfully set", LocalisedEventOutcomeFactory.from(mainOutcome, LocaleContextHolder.getLocale())); } @@ -323,7 +323,8 @@ public EntityModel<EventOutcomeWithMessage> deleteNamedFile(String taskId, Strin Map<String, SetDataEventOutcome> outcomes = new HashMap<>(); outcomes.put(taskId, dataService.deleteFileByName(taskId, fieldId, name)); SetDataEventOutcome mainOutcome = taskService.getMainOutcome(outcomes, taskId); - return EventOutcomeWithMessageResource.successMessage("Data field values have been sucessfully set", + mainOutcome = handleMainSetDataEventOutcome(mainOutcome, taskId); + return EventOutcomeWithMessageResource.successMessage("Data field values have been successfully set", LocalisedEventOutcomeFactory.from(mainOutcome, LocaleContextHolder.getLocale())); } @@ -339,4 +340,13 @@ public ResponseEntity<Resource> getFilePreview(String taskId, String fieldId) th .headers(headers) .body(fileFieldInputStream != null ? new InputStreamResource(fileFieldInputStream.getInputStream()) : null); } + + protected SetDataEventOutcome handleMainSetDataEventOutcome(SetDataEventOutcome mainOutcome, String taskId) { + if (mainOutcome == null) { + Task task = taskService.findOne(taskId); + return new SetDataEventOutcome(workflowService.findOne(task.getCaseId()), task); + } else { + return mainOutcome; + } + } } From 383bae4f649374fc55861d5bc457ac9ba0564768 Mon Sep 17 00:00:00 2001 From: chvostek <chvostek@netgrif.com> Date: Fri, 19 Dec 2025 08:19:15 +0100 Subject: [PATCH 10/12] [NAE-2303] TaskRef Security Improvements - rename method in TaskControllerTest --- .../engine/workflow/TaskControllerTest.groovy | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/groovy/com/netgrif/application/engine/workflow/TaskControllerTest.groovy b/src/test/groovy/com/netgrif/application/engine/workflow/TaskControllerTest.groovy index 867c387bb42..ebbd492060b 100644 --- a/src/test/groovy/com/netgrif/application/engine/workflow/TaskControllerTest.groovy +++ b/src/test/groovy/com/netgrif/application/engine/workflow/TaskControllerTest.groovy @@ -152,13 +152,13 @@ class TaskControllerTest { Case testCase = helper.createCase("My case", setDataNet) String taskId = testCase.tasks.find { it.transition == "testSetDataFieldTypeRestriction" }.task - ObjectNode dataSet = populateDataset([(taskId):["taskRef_0": ["type": "taskRef", "value": [taskId]]]]) + ObjectNode dataSet = populateNestedDataset([(taskId):["taskRef_0": ["type": "taskRef", "value": [taskId]]]]) def response = taskController.setData(taskId, dataSet, Locale.default) assert response != null && response.content.outcome != null assert response.content.outcome.changedFields.changedFields.isEmpty() assert ((List<String>) workflowService.findOne(testCase.stringId).getDataField("taskRef_0").getValue()).isEmpty() - dataSet = populateDataset([(taskId):["caseRef_0": ["type": "caseRef", "value": [testCase.stringId]]]]) + dataSet = populateNestedDataset([(taskId):["caseRef_0": ["type": "caseRef", "value": [testCase.stringId]]]]) response = taskController.setData(taskId, dataSet, Locale.default) assert response != null && response.content.outcome != null assert response.content.outcome.changedFields.changedFields.isEmpty() @@ -170,7 +170,7 @@ class TaskControllerTest { Case testCase = helper.createCase("My case", setDataNet) String taskId = testCase.tasks.find { it.transition == "data" }.task - ObjectNode dataSet = populateDataset([(taskId):["text_1": ["type": "text", "value": "awd"]]]) + ObjectNode dataSet = populateNestedDataset([(taskId):["text_1": ["type": "text", "value": "awd"]]]) def response = taskController.setData(taskId, dataSet, Locale.default) assert response != null && response.content.outcome == null assert response.content.error != null @@ -194,14 +194,14 @@ class TaskControllerTest { workflowService.save(testCase2) String nestedOtherTaskId = testCase2.tasks.find { it.transition == "data" }.task - ObjectNode dataSet = populateDataset([(nestedOtherTaskId):["text_0": ["type": "text", "value": "awd"]]]) + ObjectNode dataSet = populateNestedDataset([(nestedOtherTaskId):["text_0": ["type": "text", "value": "awd"]]]) def response = taskController.setData(taskId, dataSet, Locale.default) assert response != null && response.content.outcome != null assert response.content.outcome.changedFields.changedFields.isEmpty() assert workflowService.findOne(testCase2.stringId).getDataField("text_0").getValue() == null String nestedTaskId = testCase3.tasks.find { it.transition == "data" }.task - dataSet = populateDataset([(nestedTaskId):["text_0": ["type": "text", "value": "awd"]]]) + dataSet = populateNestedDataset([(nestedTaskId):["text_0": ["type": "text", "value": "awd"]]]) response = taskController.setData(taskId, dataSet, Locale.default) assert response != null && response.content.outcome != null assert !response.content.outcome.changedFields.changedFields.isEmpty() @@ -213,7 +213,7 @@ class TaskControllerTest { Case testCase = helper.createCase("My case", setDataNet) String taskId = testCase.tasks.find { it.transition == "testSetDataNonReferencedField" }.task - ObjectNode dataSet = populateDataset([(taskId):["text_1": ["type": "text", "value": "awd"]]]) + ObjectNode dataSet = populateNestedDataset([(taskId):["text_1": ["type": "text", "value": "awd"]]]) def response = taskController.setData(taskId, dataSet, Locale.default) assert response != null && response.content.outcome == null @@ -300,7 +300,7 @@ class TaskControllerTest { return tasks } - static ObjectNode populateDataset(Map<String, Map<String, Map<String, String>>> data) { + static ObjectNode populateNestedDataset(Map<String, Map<String, Map<String, String>>> data) { ObjectMapper mapper = new ObjectMapper() String json = mapper.writeValueAsString(data) return mapper.readTree(json) as ObjectNode From cc304888a77026e1cd787587e9ae0f2544bd66ad Mon Sep 17 00:00:00 2001 From: chvostek <chvostek@netgrif.com> Date: Fri, 19 Dec 2025 08:48:24 +0100 Subject: [PATCH 11/12] [NAE-2303] TaskRef Security Improvements - change the parameter type in TaskControllerTest --- .../application/engine/workflow/TaskControllerTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/groovy/com/netgrif/application/engine/workflow/TaskControllerTest.groovy b/src/test/groovy/com/netgrif/application/engine/workflow/TaskControllerTest.groovy index ebbd492060b..e30f8af7f85 100644 --- a/src/test/groovy/com/netgrif/application/engine/workflow/TaskControllerTest.groovy +++ b/src/test/groovy/com/netgrif/application/engine/workflow/TaskControllerTest.groovy @@ -300,7 +300,7 @@ class TaskControllerTest { return tasks } - static ObjectNode populateNestedDataset(Map<String, Map<String, Map<String, String>>> data) { + static ObjectNode populateNestedDataset(Map<String, Map<String, Map<String, Object>>> data) { ObjectMapper mapper = new ObjectMapper() String json = mapper.writeValueAsString(data) return mapper.readTree(json) as ObjectNode From 881548b9d8c7385cab1709781d722abc0b572937 Mon Sep 17 00:00:00 2001 From: chvostek <chvostek@netgrif.com> Date: Fri, 19 Dec 2025 09:46:34 +0100 Subject: [PATCH 12/12] [NAE-2303] TaskRef Security Improvements - rename parameter --- .../application/engine/workflow/service/DataService.java | 8 ++++---- .../engine/workflow/service/interfaces/IDataService.java | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java b/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java index 2078aef801a..c97e5e0546e 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java +++ b/src/main/java/com/netgrif/application/engine/workflow/service/DataService.java @@ -230,13 +230,13 @@ public SetDataEventOutcome setData(Task task, ObjectNode values, Map<String, Str * @param task the task object of which the data are updated * @param values information about how to update the data fields * @param params additional information to be injected to the action delegate context - * @param runSafe if set to true, additional validations are going to be applied when updating the data fields. If + * @param runStrict if set to true, additional validations are going to be applied when updating the data fields. If * set to false, minimal restrictions are considered. * * @return outcome containing Case, Task and changes that have been made. * */ @Override - public SetDataEventOutcome setData(Task task, ObjectNode values, Map<String, String> params, boolean runSafe) { + public SetDataEventOutcome setData(Task task, ObjectNode values, Map<String, String> params, boolean runStrict) { Case useCase = workflowService.findOne(task.getCaseId()); IUser user = userService.getLoggedOrSystem(); @@ -248,7 +248,7 @@ public SetDataEventOutcome setData(Task task, ObjectNode values, Map<String, Str SetDataEventOutcome outcome = new SetDataEventOutcome(useCase, task); values.fields().forEachRemaining(entry -> { String fieldId = entry.getKey(); - if (runSafe) { + if (runStrict) { Field<?> field = useCase.getField(fieldId); if (field == null) { throw new IllegalArgumentException("Such field with id [" + fieldId + "] does not exist in petri net [" + useCase.getPetriNetId() + "]"); @@ -259,7 +259,7 @@ public SetDataEventOutcome setData(Task task, ObjectNode values, Map<String, Str } DataField dataField = useCase.getDataSet().get(fieldId); if (dataField != null) { - if (runSafe && !isDataFieldEditable(dataField, task.getTransitionId())) { + if (runStrict && !isDataFieldEditable(dataField, task.getTransitionId())) { throw new IllegalArgumentException("Cannot edit data field [" + fieldId + "], which is not editable on transition [" + task.getTransitionId() + "]."); } Field field = useCase.getPetriNet().getField(fieldId).get(); diff --git a/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.java b/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.java index 3f3f11724a6..a2cb4211f8b 100644 --- a/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.java +++ b/src/main/java/com/netgrif/application/engine/workflow/service/interfaces/IDataService.java @@ -36,7 +36,7 @@ public interface IDataService { SetDataEventOutcome setData(Task task, ObjectNode values, Map<String, String> params); - SetDataEventOutcome setData(Task task, ObjectNode values, Map<String, String> params, boolean runSafe); + SetDataEventOutcome setData(Task task, ObjectNode values, Map<String, String> params, boolean runStrict); FileFieldInputStream getFile(Case useCase, Task task, FileField field, boolean forPreview) throws FileNotFoundException;