diff --git a/platform/platform-workflow/src/main/kotlin/org/vibeerp/platform/workflow/DispatchingJavaDelegate.kt b/platform/platform-workflow/src/main/kotlin/org/vibeerp/platform/workflow/DispatchingJavaDelegate.kt index 73eb05b..6ec7656 100644 --- a/platform/platform-workflow/src/main/kotlin/org/vibeerp/platform/workflow/DispatchingJavaDelegate.kt +++ b/platform/platform-workflow/src/main/kotlin/org/vibeerp/platform/workflow/DispatchingJavaDelegate.kt @@ -11,6 +11,29 @@ import java.util.Locale import java.util.UUID /** + * Internal helper: read the reserved initiator variables off a + * [DelegateExecution] and return a [Principal.User] when both are + * present, otherwise the fallback workflow-engine System principal. + * Lives at file scope (not as a method on the class) so the unit + * tests can exercise it directly. + */ +internal fun resolveInitiator(execution: DelegateExecution): Principal { + val rawId = execution.getVariable(WorkflowService.INITIATOR_ID_VAR) as? String + val rawUsername = execution.getVariable(WorkflowService.INITIATOR_USERNAME_VAR) as? String + if (rawId.isNullOrBlank() || rawUsername.isNullOrBlank()) { + return DispatchingJavaDelegate.SYSTEM_PRINCIPAL + } + return try { + Principal.User(id = Id(UUID.fromString(rawId)), username = rawUsername) + } catch (ex: IllegalArgumentException) { + // Corrupted initiator-id variable → don't fail the task, just + // fall back to the system principal and let the audit pick up + // the anomaly downstream. + DispatchingJavaDelegate.SYSTEM_PRINCIPAL + } +} + +/** * The single Flowable-facing bridge: one Spring bean that every BPMN * service task in the framework delegates to, via * `flowable:delegateExpression="${taskDispatcher}"`. @@ -54,11 +77,20 @@ class DispatchingJavaDelegate( taskKey = key, processInstanceId = execution.processInstanceId, // Copy so the handler cannot mutate Flowable's internal map. - variables = HashMap(execution.variables), + // Reserved __vibeerp_* variables (initiator id, username) are + // stripped: they are framework plumbing, not business data, + // and handlers must not depend on them. The principal goes + // through ctx.principal() instead. + variables = execution.variables.filterKeys { !it.startsWith(RESERVED_VAR_PREFIX) }, ) val ctx = DelegateTaskContext( execution = execution, - principalSupplier = { SYSTEM_PRINCIPAL }, + // Read the reserved initiator variables the controller + // stashed at process-start time. Falls back to the + // workflow-engine System principal if the process was + // started outside an HTTP request (e.g. by a background + // job once the scheduler lands). + principalSupplier = { resolveInitiator(execution) }, locale = Locale.ROOT, ) @@ -73,16 +105,25 @@ class DispatchingJavaDelegate( companion object { /** + * Variable-name prefix the framework reserves for its own + * plumbing. Handlers must not use this prefix for their own + * variables. The dispatcher strips these out of the task's + * variable snapshot so handler code doesn't see them. + */ + internal const val RESERVED_VAR_PREFIX: String = "__vibeerp_" + + /** * Fixed identity the workflow engine runs as when it executes a - * service task outside of a direct human request. The id is a stable - * v5-style constant so audit rows over time compare equal. Plugging - * in a real per-user principal will be a follow-up chunk; the - * seam exists here. + * service task outside of a direct human request — e.g. a + * background-scheduled process instance once the scheduler + * lands, or a process whose initiator id variable was lost or + * corrupted. The id is a stable constant so audit rows over + * time compare equal. */ private val WORKFLOW_ENGINE_ID: UUID = UUID.fromString("00000000-0000-0000-0000-0000000f10a1") - private val SYSTEM_PRINCIPAL: Principal = Principal.System( + internal val SYSTEM_PRINCIPAL: Principal = Principal.System( id = Id(WORKFLOW_ENGINE_ID), name = "workflow-engine", ) diff --git a/platform/platform-workflow/src/main/kotlin/org/vibeerp/platform/workflow/WorkflowService.kt b/platform/platform-workflow/src/main/kotlin/org/vibeerp/platform/workflow/WorkflowService.kt index 19fb308..18fabf8 100644 --- a/platform/platform-workflow/src/main/kotlin/org/vibeerp/platform/workflow/WorkflowService.kt +++ b/platform/platform-workflow/src/main/kotlin/org/vibeerp/platform/workflow/WorkflowService.kt @@ -5,6 +5,7 @@ import org.flowable.engine.RuntimeService import org.flowable.engine.runtime.ProcessInstance import org.slf4j.LoggerFactory import org.springframework.stereotype.Service +import org.vibeerp.platform.security.authz.AuthorizationContext /** * Thin facade over Flowable's [RuntimeService] + [RepositoryService] used @@ -41,8 +42,21 @@ class WorkflowService( ): StartedProcessInstance { require(processDefinitionKey.isNotBlank()) { "processDefinitionKey must not be blank" } + // Principal propagation: stash the authenticated user's id + + // username as reserved process variables so that every service + // task dispatched downstream sees the real initiator via + // TaskContext.principal() — instead of the framework's fallback + // `workflow-engine` System principal. The two keys are prefixed + // with `__vibeerp_` so handler code must not use those names for + // its own state; DispatchingJavaDelegate reads them back when + // constructing DelegateTaskContext. + val authorized = AuthorizationContext.current() val sanitized: Map = buildMap { variables.forEach { (k, v) -> if (v != null) put(k, v) } + if (authorized != null) { + put(INITIATOR_ID_VAR, authorized.id) + put(INITIATOR_USERNAME_VAR, authorized.username) + } } val instance: ProcessInstance = if (businessKey.isNullOrBlank()) { runtimeService.startProcessInstanceByKey(processDefinitionKey, sanitized) @@ -52,7 +66,7 @@ class WorkflowService( // A synchronous end-to-end process returns `ended == true` here; a // process that blocks on a user task returns `ended == false`. - val resultVars = if (instance.isEnded) { + val rawVars = if (instance.isEnded) { // For a finished synchronous process Flowable clears the runtime // row, so the variables on the returned instance are all we can // see — plus any that the delegate set before completion. @@ -60,6 +74,8 @@ class WorkflowService( } else { runtimeService.getVariables(instance.id) ?: emptyMap() } + // Strip reserved framework variables before returning to the caller. + val resultVars = rawVars.filterKeys { !it.startsWith(DispatchingJavaDelegate.RESERVED_VAR_PREFIX) } log.info( "started process '{}' instanceId='{}' ended={} vars={}", @@ -95,14 +111,16 @@ class WorkflowService( /** * Fetch the variables currently attached to a process instance. Throws * [NoSuchElementException] if the instance does not exist (the - * controller maps that to a 404). + * controller maps that to a 404). Reserved `__vibeerp_*` framework + * plumbing variables are filtered out so HTTP callers never see them. */ fun getInstanceVariables(processInstanceId: String): Map { val instance = runtimeService.createProcessInstanceQuery() .processInstanceId(processInstanceId) .singleResult() ?: throw NoSuchElementException("no active process instance with id '$processInstanceId'") - return runtimeService.getVariables(instance.id) ?: emptyMap() + return (runtimeService.getVariables(instance.id) ?: emptyMap()) + .filterKeys { !it.startsWith(DispatchingJavaDelegate.RESERVED_VAR_PREFIX) } } /** @@ -120,6 +138,22 @@ class WorkflowService( resourceName = it.resourceName, ) } + + companion object { + /** + * Reserved process variable name: the UUID-string id of the + * principal who started the process instance. Read by + * [DispatchingJavaDelegate] when constructing the TaskContext + * so every handler sees the real initiator. + */ + const val INITIATOR_ID_VAR: String = "__vibeerp_initiator_id" + + /** + * Reserved process variable name: the username of the principal + * who started the process instance. + */ + const val INITIATOR_USERNAME_VAR: String = "__vibeerp_initiator_username" + } } data class StartedProcessInstance( diff --git a/platform/platform-workflow/src/main/kotlin/org/vibeerp/platform/workflow/builtin/PingTaskHandler.kt b/platform/platform-workflow/src/main/kotlin/org/vibeerp/platform/workflow/builtin/PingTaskHandler.kt index cfa403f..9160410 100644 --- a/platform/platform-workflow/src/main/kotlin/org/vibeerp/platform/workflow/builtin/PingTaskHandler.kt +++ b/platform/platform-workflow/src/main/kotlin/org/vibeerp/platform/workflow/builtin/PingTaskHandler.kt @@ -2,6 +2,7 @@ package org.vibeerp.platform.workflow.builtin import org.slf4j.LoggerFactory import org.springframework.stereotype.Component +import org.vibeerp.api.v1.security.Principal import org.vibeerp.api.v1.workflow.TaskContext import org.vibeerp.api.v1.workflow.TaskHandler import org.vibeerp.api.v1.workflow.WorkflowTask @@ -32,10 +33,20 @@ class PingTaskHandler : TaskHandler { override fun key(): String = KEY override fun execute(task: WorkflowTask, ctx: TaskContext) { - log.info("PingTaskHandler invoked for processInstanceId='{}'", task.processInstanceId) + val principalLabel = when (val p = ctx.principal()) { + is Principal.User -> "user:${p.username}" + is Principal.System -> "system:${p.name}" + is Principal.PluginPrincipal -> "plugin:${p.pluginId}" + } + log.info( + "PingTaskHandler invoked for processInstanceId='{}' principal='{}'", + task.processInstanceId, + principalLabel, + ) ctx.set("pong", true) ctx.set("pongAt", Instant.now().toString()) ctx.set("correlationId", ctx.correlationId()) + ctx.set("pingedBy", principalLabel) } companion object { diff --git a/platform/platform-workflow/src/test/kotlin/org/vibeerp/platform/workflow/DispatchingJavaDelegateTest.kt b/platform/platform-workflow/src/test/kotlin/org/vibeerp/platform/workflow/DispatchingJavaDelegateTest.kt index 6e3d94e..4dbff1b 100644 --- a/platform/platform-workflow/src/test/kotlin/org/vibeerp/platform/workflow/DispatchingJavaDelegateTest.kt +++ b/platform/platform-workflow/src/test/kotlin/org/vibeerp/platform/workflow/DispatchingJavaDelegateTest.kt @@ -13,9 +13,11 @@ import io.mockk.slot import io.mockk.verify import org.flowable.engine.delegate.DelegateExecution import org.junit.jupiter.api.Test +import org.vibeerp.api.v1.security.Principal import org.vibeerp.api.v1.workflow.TaskContext import org.vibeerp.api.v1.workflow.TaskHandler import org.vibeerp.api.v1.workflow.WorkflowTask +import java.util.UUID class DispatchingJavaDelegateTest { @@ -60,13 +62,18 @@ class DispatchingJavaDelegateTest { } @Test - fun `variables given to the handler are a defensive copy`() { + fun `variables given to the handler are a defensive copy without reserved vars`() { val handler = mockk() every { handler.key() } returns "copy.key" - val originalVars = mutableMapOf("k" to "v") + val originalVars = mutableMapOf( + "k" to "v", + WorkflowService.INITIATOR_ID_VAR to "11111111-1111-1111-1111-111111111111", + WorkflowService.INITIATOR_USERNAME_VAR to "alice", + ) val taskSlot = slot() every { handler.execute(capture(taskSlot), any()) } just Runs + every { handler.execute(capture(taskSlot), any()) } just Runs val registry = TaskHandlerRegistry(listOf(handler)) val delegate = DispatchingJavaDelegate(registry) @@ -76,6 +83,9 @@ class DispatchingJavaDelegateTest { every { execution.processInstanceId } returns "pi-copy" every { execution.processDefinitionId } returns "pd-copy" every { execution.variables } returns originalVars + every { execution.getVariable(WorkflowService.INITIATOR_ID_VAR) } returns + "11111111-1111-1111-1111-111111111111" + every { execution.getVariable(WorkflowService.INITIATOR_USERNAME_VAR) } returns "alice" delegate.execute(execution) @@ -83,5 +93,46 @@ class DispatchingJavaDelegateTest { // handler received, because the dispatcher copies the variable map. originalVars["k"] = "mutated" assertThat(taskSlot.captured.variables["k"] as String).isEqualTo("v") + // Reserved vars are stripped — handler code must reach the + // initiator via ctx.principal(), not via variables. + assertThat(taskSlot.captured.variables.containsKey(WorkflowService.INITIATOR_ID_VAR)) + .isEqualTo(false) + assertThat(taskSlot.captured.variables.containsKey(WorkflowService.INITIATOR_USERNAME_VAR)) + .isEqualTo(false) + } + + @Test + fun `resolveInitiator returns user principal when both vars are set`() { + val execution = mockk() + val uuid = "22222222-2222-2222-2222-222222222222" + every { execution.getVariable(WorkflowService.INITIATOR_ID_VAR) } returns uuid + every { execution.getVariable(WorkflowService.INITIATOR_USERNAME_VAR) } returns "bob" + + val resolved = resolveInitiator(execution) + assertThat(resolved).isInstanceOf(Principal.User::class) + val user = resolved as Principal.User + assertThat(user.username).isEqualTo("bob") + assertThat(user.id.value).isEqualTo(UUID.fromString(uuid)) + } + + @Test + fun `resolveInitiator falls back to system principal when id missing`() { + val execution = mockk() + every { execution.getVariable(WorkflowService.INITIATOR_ID_VAR) } returns null + every { execution.getVariable(WorkflowService.INITIATOR_USERNAME_VAR) } returns "no-id" + + val resolved = resolveInitiator(execution) + assertThat(resolved).isInstanceOf(Principal.System::class) + } + + @Test + fun `resolveInitiator falls back to system principal when id is corrupt`() { + val execution = mockk() + every { execution.getVariable(WorkflowService.INITIATOR_ID_VAR) } returns "not-a-uuid" + every { execution.getVariable(WorkflowService.INITIATOR_USERNAME_VAR) } returns "eve" + + val resolved = resolveInitiator(execution) + assertThat(resolved).isInstanceOf(Principal.System::class) + assertThat((resolved as Principal.System).name).isEqualTo("workflow-engine") } } diff --git a/platform/platform-workflow/src/test/kotlin/org/vibeerp/platform/workflow/builtin/PingTaskHandlerTest.kt b/platform/platform-workflow/src/test/kotlin/org/vibeerp/platform/workflow/builtin/PingTaskHandlerTest.kt index cfe45d9..3dfeea8 100644 --- a/platform/platform-workflow/src/test/kotlin/org/vibeerp/platform/workflow/builtin/PingTaskHandlerTest.kt +++ b/platform/platform-workflow/src/test/kotlin/org/vibeerp/platform/workflow/builtin/PingTaskHandlerTest.kt @@ -8,8 +8,11 @@ import io.mockk.just import io.mockk.mockk import io.mockk.verify import org.junit.jupiter.api.Test +import org.vibeerp.api.v1.core.Id +import org.vibeerp.api.v1.security.Principal import org.vibeerp.api.v1.workflow.TaskContext import org.vibeerp.api.v1.workflow.WorkflowTask +import java.util.UUID class PingTaskHandlerTest { @@ -19,10 +22,11 @@ class PingTaskHandlerTest { } @Test - fun `execute writes pong plus timestamp plus correlation id`() { + fun `execute writes pong plus timestamp plus correlation id plus user principal label`() { val ctx = mockk() every { ctx.set(any(), any()) } just Runs every { ctx.correlationId() } returns "corr-42" + every { ctx.principal() } returns Principal.User(id = Id(UUID.randomUUID()), username = "alice") val handler = PingTaskHandler() handler.execute( @@ -37,5 +41,21 @@ class PingTaskHandlerTest { verify(exactly = 1) { ctx.set("pong", true) } verify(exactly = 1) { ctx.set("pongAt", any()) } verify(exactly = 1) { ctx.set("correlationId", "corr-42") } + verify(exactly = 1) { ctx.set("pingedBy", "user:alice") } + } + + @Test + fun `execute labels a system principal with its name`() { + val ctx = mockk() + every { ctx.set(any(), any()) } just Runs + every { ctx.correlationId() } returns "corr-9" + every { ctx.principal() } returns Principal.System(id = Id(UUID.randomUUID()), name = "workflow-engine") + + PingTaskHandler().execute( + task = WorkflowTask("vibeerp.workflow.ping", "pi-sys", emptyMap()), + ctx = ctx, + ) + + verify(exactly = 1) { ctx.set("pingedBy", "system:workflow-engine") } } }