Commit ef9e5b42dbec1204f6e1d178ecf5bbc779f66205

Authored by zichun
1 parent 4f0e8bc9

feat(workflow): propagate HTTP initiator principal into TaskHandler

Before this commit, every TaskHandler saw a fixed `workflow-engine`
System principal via `ctx.principal()` because there was no plumbing
from the REST caller down to the dispatcher. A printing-shop
quote-to-job-card handler (or any real business workflow) needs to
know the actual user who kicked off the process so audit columns and
role-based logic behave correctly.

## Mechanism

The chain is: Spring Security populates `SecurityContextHolder` →
`PrincipalContextFilter` mirrors it into `AuthorizationContext`
(already existed) → `WorkflowService.startProcess` reads the bound
`AuthorizedPrincipal` and stashes two reserved process variables
(`__vibeerp_initiator_id`, `__vibeerp_initiator_username`) before
calling `RuntimeService.startProcessInstanceByKey` →
`DispatchingJavaDelegate` reads them back off each `DelegateExecution`
when constructing the `DelegateTaskContext` → handler sees a real
`Principal.User` from `ctx.principal()`.

When the process is started outside an HTTP request (e.g. a future
Quartz-scheduled process, or a signal fired by a PBC event
subscriber), `AuthorizationContext.current()` is null, no initiator
variables are written, and the dispatcher falls back to the
`Principal.System("workflow-engine")` principal. A corrupt initiator
id (e.g. a non-UUID string) also falls back to the system principal
rather than failing the task, so a stale variable can't brick a
running workflow.

## Reserved variable hygiene

The `__vibeerp_` prefix is reserved framework plumbing. Two
consequences wired in this commit:

- `DispatchingJavaDelegate` strips keys starting with `__vibeerp_`
  from the variable snapshot handed to the handler (via
  `WorkflowTask.variables`), so handler code cannot accidentally
  depend on the initiator id through the wrong door — it must use
  `ctx.principal()`.
- `WorkflowService.startProcess` and `getInstanceVariables` strip
  the same prefix from their HTTP response payloads so REST callers
  never see the plumbing either.

The prefix constant lives on `DispatchingJavaDelegate.RESERVED_VAR_PREFIX`
so there is exactly one source of truth. The two initiator variable
names are public constants on `WorkflowService` — tests, future
plug-in code, and any custom handlers that genuinely need the raw
ids (e.g. a security-audit task) can depend on the stable symbols
instead of hard-coded strings.

## PingTaskHandler as the executable witness

`PingTaskHandler` now writes a `pingedBy` output variable with a
principal label (`user:<username>`, `system:<name>`, or
`plugin:<pluginId>`) and logs it. That makes the end-to-end smoke
test trivially assertable:

```
POST /api/v1/workflow/process-instances
     {"processDefinitionKey":"vibeerp-workflow-ping"}
  (as admin user, with valid JWT)
  → {"processInstanceId": "...", "ended": true,
     "variables": {
       "pong": true,
       "pongAt": "...",
       "correlationId": "...",
       "pingedBy": "user:admin"
     }}
```

Note the RESPONSE does NOT contain `__vibeerp_initiator_id` or
`__vibeerp_initiator_username` — the reserved-var filter in the
service layer hides them. The handler-side log line confirms
`principal='user:admin'` in the service-task execution thread.

## Tests

- 3 new tests in `DispatchingJavaDelegateTest`:
  * `resolveInitiator` returns a User principal when both vars set
  * falls back to system principal when id var is missing
  * falls back to system principal when id var is corrupt
    (non-UUID string)
- Updated `variables given to the handler are a defensive copy` to
  also assert that reserved `__vibeerp_*` keys are stripped from
  the task's variable snapshot.
- Updated `PingTaskHandlerTest`:
  * rename to "writes pong plus timestamp plus correlation id plus
    user principal label"
  * new test for the System-principal branch producing
    `pingedBy=system:workflow-engine`
- Total framework unit tests: 265 (was 261), all green.

## Non-goals (still parking lot)

- Plug-in-contributed TaskHandler registration via the PF4J loader
  walking child contexts for TaskHandler beans and calling
  `TaskHandlerRegistry.register`. The seam exists on the registry;
  the loader integration is the next chunk, and unblocks REF.1.
- Propagation of the full role set (not just id+username) into the
  TaskContext. Handlers don't currently see the initiator's roles.
  Can be added as a third reserved variable when a handler actually
  needs it — YAGNI for now.
- BPMN user tasks / signals / timers — engine supports them but we
  have no HTTP surface for them yet.
platform/platform-workflow/src/main/kotlin/org/vibeerp/platform/workflow/DispatchingJavaDelegate.kt
... ... @@ -11,6 +11,29 @@ import java.util.Locale
11 11 import java.util.UUID
12 12  
13 13 /**
  14 + * Internal helper: read the reserved initiator variables off a
  15 + * [DelegateExecution] and return a [Principal.User] when both are
  16 + * present, otherwise the fallback workflow-engine System principal.
  17 + * Lives at file scope (not as a method on the class) so the unit
  18 + * tests can exercise it directly.
  19 + */
  20 +internal fun resolveInitiator(execution: DelegateExecution): Principal {
  21 + val rawId = execution.getVariable(WorkflowService.INITIATOR_ID_VAR) as? String
  22 + val rawUsername = execution.getVariable(WorkflowService.INITIATOR_USERNAME_VAR) as? String
  23 + if (rawId.isNullOrBlank() || rawUsername.isNullOrBlank()) {
  24 + return DispatchingJavaDelegate.SYSTEM_PRINCIPAL
  25 + }
  26 + return try {
  27 + Principal.User(id = Id(UUID.fromString(rawId)), username = rawUsername)
  28 + } catch (ex: IllegalArgumentException) {
  29 + // Corrupted initiator-id variable → don't fail the task, just
  30 + // fall back to the system principal and let the audit pick up
  31 + // the anomaly downstream.
  32 + DispatchingJavaDelegate.SYSTEM_PRINCIPAL
  33 + }
  34 +}
  35 +
  36 +/**
14 37 * The single Flowable-facing bridge: one Spring bean that every BPMN
15 38 * service task in the framework delegates to, via
16 39 * `flowable:delegateExpression="${taskDispatcher}"`.
... ... @@ -54,11 +77,20 @@ class DispatchingJavaDelegate(
54 77 taskKey = key,
55 78 processInstanceId = execution.processInstanceId,
56 79 // Copy so the handler cannot mutate Flowable's internal map.
57   - variables = HashMap(execution.variables),
  80 + // Reserved __vibeerp_* variables (initiator id, username) are
  81 + // stripped: they are framework plumbing, not business data,
  82 + // and handlers must not depend on them. The principal goes
  83 + // through ctx.principal() instead.
  84 + variables = execution.variables.filterKeys { !it.startsWith(RESERVED_VAR_PREFIX) },
58 85 )
59 86 val ctx = DelegateTaskContext(
60 87 execution = execution,
61   - principalSupplier = { SYSTEM_PRINCIPAL },
  88 + // Read the reserved initiator variables the controller
  89 + // stashed at process-start time. Falls back to the
  90 + // workflow-engine System principal if the process was
  91 + // started outside an HTTP request (e.g. by a background
  92 + // job once the scheduler lands).
  93 + principalSupplier = { resolveInitiator(execution) },
62 94 locale = Locale.ROOT,
63 95 )
64 96  
... ... @@ -73,16 +105,25 @@ class DispatchingJavaDelegate(
73 105  
74 106 companion object {
75 107 /**
  108 + * Variable-name prefix the framework reserves for its own
  109 + * plumbing. Handlers must not use this prefix for their own
  110 + * variables. The dispatcher strips these out of the task's
  111 + * variable snapshot so handler code doesn't see them.
  112 + */
  113 + internal const val RESERVED_VAR_PREFIX: String = "__vibeerp_"
  114 +
  115 + /**
76 116 * Fixed identity the workflow engine runs as when it executes a
77   - * service task outside of a direct human request. The id is a stable
78   - * v5-style constant so audit rows over time compare equal. Plugging
79   - * in a real per-user principal will be a follow-up chunk; the
80   - * seam exists here.
  117 + * service task outside of a direct human request — e.g. a
  118 + * background-scheduled process instance once the scheduler
  119 + * lands, or a process whose initiator id variable was lost or
  120 + * corrupted. The id is a stable constant so audit rows over
  121 + * time compare equal.
81 122 */
82 123 private val WORKFLOW_ENGINE_ID: UUID =
83 124 UUID.fromString("00000000-0000-0000-0000-0000000f10a1")
84 125  
85   - private val SYSTEM_PRINCIPAL: Principal = Principal.System(
  126 + internal val SYSTEM_PRINCIPAL: Principal = Principal.System(
86 127 id = Id(WORKFLOW_ENGINE_ID),
87 128 name = "workflow-engine",
88 129 )
... ...
platform/platform-workflow/src/main/kotlin/org/vibeerp/platform/workflow/WorkflowService.kt
... ... @@ -5,6 +5,7 @@ import org.flowable.engine.RuntimeService
5 5 import org.flowable.engine.runtime.ProcessInstance
6 6 import org.slf4j.LoggerFactory
7 7 import org.springframework.stereotype.Service
  8 +import org.vibeerp.platform.security.authz.AuthorizationContext
8 9  
9 10 /**
10 11 * Thin facade over Flowable's [RuntimeService] + [RepositoryService] used
... ... @@ -41,8 +42,21 @@ class WorkflowService(
41 42 ): StartedProcessInstance {
42 43 require(processDefinitionKey.isNotBlank()) { "processDefinitionKey must not be blank" }
43 44  
  45 + // Principal propagation: stash the authenticated user's id +
  46 + // username as reserved process variables so that every service
  47 + // task dispatched downstream sees the real initiator via
  48 + // TaskContext.principal() — instead of the framework's fallback
  49 + // `workflow-engine` System principal. The two keys are prefixed
  50 + // with `__vibeerp_` so handler code must not use those names for
  51 + // its own state; DispatchingJavaDelegate reads them back when
  52 + // constructing DelegateTaskContext.
  53 + val authorized = AuthorizationContext.current()
44 54 val sanitized: Map<String, Any> = buildMap {
45 55 variables.forEach { (k, v) -> if (v != null) put(k, v) }
  56 + if (authorized != null) {
  57 + put(INITIATOR_ID_VAR, authorized.id)
  58 + put(INITIATOR_USERNAME_VAR, authorized.username)
  59 + }
46 60 }
47 61 val instance: ProcessInstance = if (businessKey.isNullOrBlank()) {
48 62 runtimeService.startProcessInstanceByKey(processDefinitionKey, sanitized)
... ... @@ -52,7 +66,7 @@ class WorkflowService(
52 66  
53 67 // A synchronous end-to-end process returns `ended == true` here; a
54 68 // process that blocks on a user task returns `ended == false`.
55   - val resultVars = if (instance.isEnded) {
  69 + val rawVars = if (instance.isEnded) {
56 70 // For a finished synchronous process Flowable clears the runtime
57 71 // row, so the variables on the returned instance are all we can
58 72 // see — plus any that the delegate set before completion.
... ... @@ -60,6 +74,8 @@ class WorkflowService(
60 74 } else {
61 75 runtimeService.getVariables(instance.id) ?: emptyMap()
62 76 }
  77 + // Strip reserved framework variables before returning to the caller.
  78 + val resultVars = rawVars.filterKeys { !it.startsWith(DispatchingJavaDelegate.RESERVED_VAR_PREFIX) }
63 79  
64 80 log.info(
65 81 "started process '{}' instanceId='{}' ended={} vars={}",
... ... @@ -95,14 +111,16 @@ class WorkflowService(
95 111 /**
96 112 * Fetch the variables currently attached to a process instance. Throws
97 113 * [NoSuchElementException] if the instance does not exist (the
98   - * controller maps that to a 404).
  114 + * controller maps that to a 404). Reserved `__vibeerp_*` framework
  115 + * plumbing variables are filtered out so HTTP callers never see them.
99 116 */
100 117 fun getInstanceVariables(processInstanceId: String): Map<String, Any?> {
101 118 val instance = runtimeService.createProcessInstanceQuery()
102 119 .processInstanceId(processInstanceId)
103 120 .singleResult()
104 121 ?: throw NoSuchElementException("no active process instance with id '$processInstanceId'")
105   - return runtimeService.getVariables(instance.id) ?: emptyMap()
  122 + return (runtimeService.getVariables(instance.id) ?: emptyMap())
  123 + .filterKeys { !it.startsWith(DispatchingJavaDelegate.RESERVED_VAR_PREFIX) }
106 124 }
107 125  
108 126 /**
... ... @@ -120,6 +138,22 @@ class WorkflowService(
120 138 resourceName = it.resourceName,
121 139 )
122 140 }
  141 +
  142 + companion object {
  143 + /**
  144 + * Reserved process variable name: the UUID-string id of the
  145 + * principal who started the process instance. Read by
  146 + * [DispatchingJavaDelegate] when constructing the TaskContext
  147 + * so every handler sees the real initiator.
  148 + */
  149 + const val INITIATOR_ID_VAR: String = "__vibeerp_initiator_id"
  150 +
  151 + /**
  152 + * Reserved process variable name: the username of the principal
  153 + * who started the process instance.
  154 + */
  155 + const val INITIATOR_USERNAME_VAR: String = "__vibeerp_initiator_username"
  156 + }
123 157 }
124 158  
125 159 data class StartedProcessInstance(
... ...
platform/platform-workflow/src/main/kotlin/org/vibeerp/platform/workflow/builtin/PingTaskHandler.kt
... ... @@ -2,6 +2,7 @@ package org.vibeerp.platform.workflow.builtin
2 2  
3 3 import org.slf4j.LoggerFactory
4 4 import org.springframework.stereotype.Component
  5 +import org.vibeerp.api.v1.security.Principal
5 6 import org.vibeerp.api.v1.workflow.TaskContext
6 7 import org.vibeerp.api.v1.workflow.TaskHandler
7 8 import org.vibeerp.api.v1.workflow.WorkflowTask
... ... @@ -32,10 +33,20 @@ class PingTaskHandler : TaskHandler {
32 33 override fun key(): String = KEY
33 34  
34 35 override fun execute(task: WorkflowTask, ctx: TaskContext) {
35   - log.info("PingTaskHandler invoked for processInstanceId='{}'", task.processInstanceId)
  36 + val principalLabel = when (val p = ctx.principal()) {
  37 + is Principal.User -> "user:${p.username}"
  38 + is Principal.System -> "system:${p.name}"
  39 + is Principal.PluginPrincipal -> "plugin:${p.pluginId}"
  40 + }
  41 + log.info(
  42 + "PingTaskHandler invoked for processInstanceId='{}' principal='{}'",
  43 + task.processInstanceId,
  44 + principalLabel,
  45 + )
36 46 ctx.set("pong", true)
37 47 ctx.set("pongAt", Instant.now().toString())
38 48 ctx.set("correlationId", ctx.correlationId())
  49 + ctx.set("pingedBy", principalLabel)
39 50 }
40 51  
41 52 companion object {
... ...
platform/platform-workflow/src/test/kotlin/org/vibeerp/platform/workflow/DispatchingJavaDelegateTest.kt
... ... @@ -13,9 +13,11 @@ import io.mockk.slot
13 13 import io.mockk.verify
14 14 import org.flowable.engine.delegate.DelegateExecution
15 15 import org.junit.jupiter.api.Test
  16 +import org.vibeerp.api.v1.security.Principal
16 17 import org.vibeerp.api.v1.workflow.TaskContext
17 18 import org.vibeerp.api.v1.workflow.TaskHandler
18 19 import org.vibeerp.api.v1.workflow.WorkflowTask
  20 +import java.util.UUID
19 21  
20 22 class DispatchingJavaDelegateTest {
21 23  
... ... @@ -60,13 +62,18 @@ class DispatchingJavaDelegateTest {
60 62 }
61 63  
62 64 @Test
63   - fun `variables given to the handler are a defensive copy`() {
  65 + fun `variables given to the handler are a defensive copy without reserved vars`() {
64 66 val handler = mockk<TaskHandler>()
65 67 every { handler.key() } returns "copy.key"
66 68  
67   - val originalVars = mutableMapOf<String, Any>("k" to "v")
  69 + val originalVars = mutableMapOf<String, Any>(
  70 + "k" to "v",
  71 + WorkflowService.INITIATOR_ID_VAR to "11111111-1111-1111-1111-111111111111",
  72 + WorkflowService.INITIATOR_USERNAME_VAR to "alice",
  73 + )
68 74 val taskSlot = slot<WorkflowTask>()
69 75 every { handler.execute(capture(taskSlot), any()) } just Runs
  76 + every { handler.execute(capture(taskSlot), any()) } just Runs
70 77  
71 78 val registry = TaskHandlerRegistry(listOf(handler))
72 79 val delegate = DispatchingJavaDelegate(registry)
... ... @@ -76,6 +83,9 @@ class DispatchingJavaDelegateTest {
76 83 every { execution.processInstanceId } returns "pi-copy"
77 84 every { execution.processDefinitionId } returns "pd-copy"
78 85 every { execution.variables } returns originalVars
  86 + every { execution.getVariable(WorkflowService.INITIATOR_ID_VAR) } returns
  87 + "11111111-1111-1111-1111-111111111111"
  88 + every { execution.getVariable(WorkflowService.INITIATOR_USERNAME_VAR) } returns "alice"
79 89  
80 90 delegate.execute(execution)
81 91  
... ... @@ -83,5 +93,46 @@ class DispatchingJavaDelegateTest {
83 93 // handler received, because the dispatcher copies the variable map.
84 94 originalVars["k"] = "mutated"
85 95 assertThat(taskSlot.captured.variables["k"] as String).isEqualTo("v")
  96 + // Reserved vars are stripped — handler code must reach the
  97 + // initiator via ctx.principal(), not via variables.
  98 + assertThat(taskSlot.captured.variables.containsKey(WorkflowService.INITIATOR_ID_VAR))
  99 + .isEqualTo(false)
  100 + assertThat(taskSlot.captured.variables.containsKey(WorkflowService.INITIATOR_USERNAME_VAR))
  101 + .isEqualTo(false)
  102 + }
  103 +
  104 + @Test
  105 + fun `resolveInitiator returns user principal when both vars are set`() {
  106 + val execution = mockk<DelegateExecution>()
  107 + val uuid = "22222222-2222-2222-2222-222222222222"
  108 + every { execution.getVariable(WorkflowService.INITIATOR_ID_VAR) } returns uuid
  109 + every { execution.getVariable(WorkflowService.INITIATOR_USERNAME_VAR) } returns "bob"
  110 +
  111 + val resolved = resolveInitiator(execution)
  112 + assertThat(resolved).isInstanceOf(Principal.User::class)
  113 + val user = resolved as Principal.User
  114 + assertThat(user.username).isEqualTo("bob")
  115 + assertThat(user.id.value).isEqualTo(UUID.fromString(uuid))
  116 + }
  117 +
  118 + @Test
  119 + fun `resolveInitiator falls back to system principal when id missing`() {
  120 + val execution = mockk<DelegateExecution>()
  121 + every { execution.getVariable(WorkflowService.INITIATOR_ID_VAR) } returns null
  122 + every { execution.getVariable(WorkflowService.INITIATOR_USERNAME_VAR) } returns "no-id"
  123 +
  124 + val resolved = resolveInitiator(execution)
  125 + assertThat(resolved).isInstanceOf(Principal.System::class)
  126 + }
  127 +
  128 + @Test
  129 + fun `resolveInitiator falls back to system principal when id is corrupt`() {
  130 + val execution = mockk<DelegateExecution>()
  131 + every { execution.getVariable(WorkflowService.INITIATOR_ID_VAR) } returns "not-a-uuid"
  132 + every { execution.getVariable(WorkflowService.INITIATOR_USERNAME_VAR) } returns "eve"
  133 +
  134 + val resolved = resolveInitiator(execution)
  135 + assertThat(resolved).isInstanceOf(Principal.System::class)
  136 + assertThat((resolved as Principal.System).name).isEqualTo("workflow-engine")
86 137 }
87 138 }
... ...
platform/platform-workflow/src/test/kotlin/org/vibeerp/platform/workflow/builtin/PingTaskHandlerTest.kt
... ... @@ -8,8 +8,11 @@ import io.mockk.just
8 8 import io.mockk.mockk
9 9 import io.mockk.verify
10 10 import org.junit.jupiter.api.Test
  11 +import org.vibeerp.api.v1.core.Id
  12 +import org.vibeerp.api.v1.security.Principal
11 13 import org.vibeerp.api.v1.workflow.TaskContext
12 14 import org.vibeerp.api.v1.workflow.WorkflowTask
  15 +import java.util.UUID
13 16  
14 17 class PingTaskHandlerTest {
15 18  
... ... @@ -19,10 +22,11 @@ class PingTaskHandlerTest {
19 22 }
20 23  
21 24 @Test
22   - fun `execute writes pong plus timestamp plus correlation id`() {
  25 + fun `execute writes pong plus timestamp plus correlation id plus user principal label`() {
23 26 val ctx = mockk<TaskContext>()
24 27 every { ctx.set(any(), any()) } just Runs
25 28 every { ctx.correlationId() } returns "corr-42"
  29 + every { ctx.principal() } returns Principal.User(id = Id(UUID.randomUUID()), username = "alice")
26 30  
27 31 val handler = PingTaskHandler()
28 32 handler.execute(
... ... @@ -37,5 +41,21 @@ class PingTaskHandlerTest {
37 41 verify(exactly = 1) { ctx.set("pong", true) }
38 42 verify(exactly = 1) { ctx.set("pongAt", any()) }
39 43 verify(exactly = 1) { ctx.set("correlationId", "corr-42") }
  44 + verify(exactly = 1) { ctx.set("pingedBy", "user:alice") }
  45 + }
  46 +
  47 + @Test
  48 + fun `execute labels a system principal with its name`() {
  49 + val ctx = mockk<TaskContext>()
  50 + every { ctx.set(any(), any()) } just Runs
  51 + every { ctx.correlationId() } returns "corr-9"
  52 + every { ctx.principal() } returns Principal.System(id = Id(UUID.randomUUID()), name = "workflow-engine")
  53 +
  54 + PingTaskHandler().execute(
  55 + task = WorkflowTask("vibeerp.workflow.ping", "pi-sys", emptyMap()),
  56 + ctx = ctx,
  57 + )
  58 +
  59 + verify(exactly = 1) { ctx.set("pingedBy", "system:workflow-engine") }
40 60 }
41 61 }
... ...