diff --git a/automation/service/session.go b/automation/service/session.go index 8d7d69336..2a993b214 100644 --- a/automation/service/session.go +++ b/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), ) } diff --git a/automation/service/trigger.go b/automation/service/trigger.go index 1d7f80ef6..d853fff96 100644 --- a/automation/service/trigger.go +++ b/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) diff --git a/automation/service/workflow.go b/automation/service/workflow.go index 7f56aa10d..00eec7ca0 100644 --- a/automation/service/workflow.go +++ b/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, diff --git a/automation/types/function.go b/automation/types/function.go index 0be229d9d..9bb12e0d9 100644 --- a/automation/types/function.go +++ b/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 ( diff --git a/pkg/wfexec/session.go b/pkg/wfexec/session.go index 23b3a2c72..a2e26a9bf 100644 --- a/pkg/wfexec/session.go +++ b/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()) diff --git a/tests/workflows/exec_permissions_test.go b/tests/workflows/exec_permissions_test.go new file mode 100644 index 000000000..31dbdf6d7 --- /dev/null +++ b/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()) + }) +} diff --git a/tests/workflows/invoker_and_runner_in_scope_test.go b/tests/workflows/invoker_and_runner_in_scope_test.go index aa08314a2..610a300f8 100644 --- a/tests/workflows/invoker_and_runner_in_scope_test.go +++ b/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) }) - } diff --git a/tests/workflows/testdata/exec_permissions/users.yaml b/tests/workflows/testdata/exec_permissions/users.yaml new file mode 100644 index 000000000..8ad607285 --- /dev/null +++ b/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 diff --git a/tests/workflows/testdata/exec_permissions/workflow.yaml b/tests/workflows/testdata/exec_permissions/workflow.yaml new file mode 100644 index 000000000..9831ca304 --- /dev/null +++ b/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