Fix invoker/runner mixup & move exec access-control check higher

automation/service/session.go

@@ -269,7 +269,7 @@ func (svc *session) Resume(sessionID, stateID uint64, i auth.Identifiable, input
 //
 // We need initial context for the session because we want to catch all cancellations or timeouts from there
 // and not from any potential HTTP requests or similar temporary context that can prematurely destroy a workflow session
-func (svc *session) spawn(g *wfexec.Graph, workflowID uint64, trace bool, callStack []uint64, invoker, runner auth.Identifiable) (ses *types.Session) {
+func (svc *session) spawn(g *wfexec.Graph, workflowID uint64, trace bool, callStack []uint64, runner, invoker auth.Identifiable) (ses *types.Session) {
 	s := &spawn{
 		workflowID: workflowID,
 		session:    make(chan *wfexec.Session, 1),
@@ -316,9 +316,15 @@ func (svc *session) Watch(ctx context.Context) {
 				}
 
 				if svc.opt.ExecDebug {
+					log := svc.log.
+						Named("exec").
+						With(zap.Uint64("workflowID", s.workflowID)).
+						With(zap.Uint64("runnerID", s.runner.Identity())).
+						With(zap.Uint64s("runnerRoles", s.runner.Roles()))
+
 					opts = append(
 						opts,
-						wfexec.SetLogger(svc.log.Named("exec").With(zap.Uint64("workflowID", s.workflowID))),
+						wfexec.SetLogger(log),
 						wfexec.SetDumpStacktraceOnPanic(true),
 					)
 				}

automation/service/trigger.go

@@ -560,11 +560,6 @@ func (svc *trigger) registerTriggers(wf *types.Workflow, runAs auth.Identifiable
 			continue
 		}
 
-		if t.EventType == "onManual" {
-			// skip onManual trigger registration,
-			// we'll handle them directly
-		}
-
 		var (
 			cnstr eventbus.ConstraintMatcher
 			ops   = make([]eventbus.HandlerRegOp, 0, len(t.Constraints)+2)

automation/service/workflow.go

@@ -559,6 +559,10 @@ func (svc *workflow) Exec(ctx context.Context, workflowID uint64, p types.Workfl
 
 		wap.setWorkflow(wf)
 
+		if !svc.ac.CanExecuteWorkflow(ctx, wf) {
+			return WorkflowErrNotAllowedToExecute()
+		}
+
 		if !wf.Enabled && !p.Trace {
 			return WorkflowErrDisabled()
 		}
@@ -612,7 +616,6 @@ func (svc *workflow) Exec(ctx context.Context, workflowID uint64, p types.Workfl
 			p.EventType = "onTrace"
 		}
 
-		//wait, err = svc.session.Start(g, ssp)
 		wait, err = svc.exec(ctx, wf, p)
 
 		if err != nil {
@@ -700,12 +703,6 @@ func (svc *workflow) exec(ctx context.Context, wf *types.Workflow, p types.Workf
 	// merge workflow scope with the input
 	scope = wf.Scope.MustMerge(p.Input)
 
-	// User (either invoker or one set in the security descriptor) MUST have
-	// permissions to execute this workflow
-	if !svc.ac.CanExecuteWorkflow(ctx, wf) {
-		return nil, WorkflowErrNotAllowedToExecute()
-	}
-
 	return svc.session.Start(ctx, g, types.SessionStartParams{
 		Invoker: intAuth.GetIdentityFromContext(ctx),
 		Runner:  runAs,

automation/types/function.go

@@ -3,11 +3,12 @@ package types
 import (
 	"context"
 	"fmt"
+	"time"
+
 	"github.com/cortezaproject/corteza-server/pkg/expr"
 	"github.com/cortezaproject/corteza-server/pkg/logger"
 	"github.com/cortezaproject/corteza-server/pkg/wfexec"
 	"go.uber.org/zap"
-	"time"
 )
 
 type (

pkg/wfexec/session.go

@@ -626,12 +626,7 @@ func (s *Session) exec(ctx context.Context, log *zap.Logger, st *State) (nxt []*
 			// push logger to context but raise the stacktrace level to panic
 			// to prevent overly verbose traces
 			ctx = logger.ContextWithValue(ctx, log)
-
-			// Context received in exec() wil not have the identity we're expecting
-			// so we need to pull it from state owner and add it to new context
-			// that is set to step exec function
-			stepCtx := auth.SetIdentityToContext(ctx, st.owner)
-			stepCtx = SetContextCallStack(stepCtx, s.callStack)
+			stepCtx := SetContextCallStack(ctx, s.callStack)
 
 			result, st.err = st.step.Exec(stepCtx, st.MakeRequest())
 

tests/workflows/exec_permissions_test.go

@@ -0,0 +1,62 @@
+package workflows
+
+import (
+	"context"
+	"testing"
+
+	"github.com/cortezaproject/corteza-server/automation/service"
+	"github.com/cortezaproject/corteza-server/automation/types"
+	"github.com/cortezaproject/corteza-server/pkg/auth"
+	"github.com/cortezaproject/corteza-server/pkg/rbac"
+	"github.com/cortezaproject/corteza-server/tests/helpers"
+	"github.com/stretchr/testify/require"
+)
+
+func Test_exec_permissions(t *testing.T) {
+	var (
+		ctx = bypassRBAC(context.Background())
+		req = require.New(t)
+	)
+
+	req.NoError(defStore.TruncateUsers(ctx))
+	req.NoError(defStore.TruncateRoles(ctx))
+	req.NoError(defStore.TruncateRoleMembers(ctx))
+	req.NoError(defStore.TruncateRbacRules(ctx))
+
+	loadNewScenario(ctx, t)
+
+	// user that the workflow is configured to use for run-as
+	execAllowed, err := defStore.LookupUserByHandle(ctx, "exec-allowed")
+	req.NoError(err)
+
+	// user that the workflow is configured to use for run-as
+	execDenied, err := defStore.LookupUserByHandle(ctx, "exec-denied")
+	req.NoError(err)
+
+	// invokers group with permissions to execute workflow
+	executors, err := defStore.LookupRoleByHandle(ctx, "executors")
+	req.NoError(err)
+
+	//err = defStore.CreateRoleMember(ctx, &sysTypes.RoleMember{UserID: wfInvoker.ID, RoleID: wfInvokers.ID})
+	//req.NoError(err)
+
+	execAllowed.SetRoles(executors.ID)
+
+	helpers.UpdateRBAC(
+		executors.ID,
+	)
+
+	rbac.Global().Reload(ctx)
+
+	t.Run("exec allowed", func(t *testing.T) {
+		ctx = auth.SetIdentityToContext(ctx, execAllowed)
+		_, _ = mustExecWorkflow(ctx, t, "wf", types.WorkflowExecParams{})
+	})
+
+	t.Run("exec denied", func(t *testing.T) {
+		req = require.New(t)
+		ctx = auth.SetIdentityToContext(ctx, execDenied)
+		_, _, err = execWorkflow(ctx, "wf", types.WorkflowExecParams{})
+		req.ErrorIs(err, service.WorkflowErrNotAllowedToExecute())
+	})
+}

tests/workflows/invoker_and_runner_in_scope_test.go

@@ -85,5 +85,4 @@ func Test_invoker_and_runner_in_scope(t *testing.T) {
 		req.Equal(aux.Runner.Handle, wfRunner.Handle)
 		req.Equal(aux.Invoker.Handle, wfInvoker.Handle)
 	})
-
 }

tests/workflows/testdata/exec_permissions/users.yaml

@@ -0,0 +1,8 @@
+users:
+  exec-allowed: exec-allowed@cortezaproject.org
+  exec-denied: exec-denied@cortezaproject.org
+
+roles:
+  executors:
+    members:
+      - exec-allowed

tests/workflows/testdata/exec_permissions/workflow.yaml

@@ -0,0 +1,16 @@
+workflows:
+  wf:
+    enabled: true
+    trace: true
+    triggers:
+      - enabled: true
+        stepID: 1
+
+    steps:
+      - stepID: 1
+        kind: termination
+
+    paths: []
+    allow:
+      executors:
+        - execute