Skip to content

Commit

Permalink
fix(api): don't change workflow permission for as-code (#6090)
Browse files Browse the repository at this point in the history
  • Loading branch information
fsamin committed Feb 16, 2022
1 parent 67f9425 commit b9151b4
Show file tree
Hide file tree
Showing 8 changed files with 395 additions and 15 deletions.
4 changes: 4 additions & 0 deletions engine/api/ascode.go
Expand Up @@ -130,6 +130,10 @@ func (api *API) postPerformImportAsCodeHandler() service.Handler {
key := vars[permProjectKey]
uuid := vars["uuid"]

if isService(ctx) {
return sdk.ErrForbidden
}

if uuid == "" {
return sdk.NewErrorFrom(sdk.ErrWrongRequest, "invalid given operation uuid")
}
Expand Down
2 changes: 1 addition & 1 deletion engine/api/workflow/dao.go
Expand Up @@ -352,7 +352,7 @@ func Insert(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.St
return sdk.WrapError(err, "Unable to update workflow")
}
} else {
log.Debug(ctx, "postWorkflowHandler> inherit permissions from project")
log.Debug(ctx, "inherit permissions from project")
for _, gp := range proj.ProjectGroups {
if err := group.AddWorkflowGroup(ctx, db, w, gp); err != nil {
return sdk.WrapError(err, "Cannot add group %s", gp.Group.Name)
Expand Down
2 changes: 1 addition & 1 deletion engine/api/workflow/repository.go
Expand Up @@ -125,7 +125,7 @@ func extractWorkflow(ctx context.Context, db *gorp.DbMap, store cache.Store, p *
if err != nil {
return nil, allMsgs, err
}
msgPush, workflowPushed, _, secrets, err := Push(ctx, db, store, p, data, opt, consumer, decryptFunc)
msgPush, workflowPushed, _, secrets, err := Push(ctx, db, store, p, data, opt, &consumer, decryptFunc)
// Filter workflow push message if generated from template
for i := range msgPush {
if wti != nil && msgPush[i].ID == sdk.MsgWorkflowDeprecatedVersion.ID {
Expand Down
30 changes: 27 additions & 3 deletions engine/api/workflow/workflow_importer.go
Expand Up @@ -18,7 +18,7 @@ import (
)

//Import is able to create a new workflow and all its components
func Import(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.Store, proj sdk.Project, oldW, w *sdk.Workflow, u sdk.Identifiable, force bool, msgChan chan<- sdk.Message) error {
func Import(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.Store, proj sdk.Project, oldW, w *sdk.Workflow, u sdk.Identifiable, opts ImportOptions, msgChan chan<- sdk.Message) error {
ctx, end := telemetry.Span(ctx, "workflow.Import")
defer end()

Expand All @@ -29,6 +29,30 @@ func Import(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.St
w.WorkflowData.Node.Context = &sdk.NodeContext{}
}

// If the import is not done by a direct user (ie. from a hook or if the content is coming from a repository)
// We don't take permission in account and we only keep permission of the oldWorkflow or projet permission
if opts.HookUUID != "" || opts.RepositoryName != "" {
log.Info(ctx, "Import is perform from 'as-code', we don't take groups in account (hookUUID=%q, repository=%q)", opts.HookUUID, opts.RepositoryName)
// reset permissions at the workflow level
w.Groups = nil
if oldW != nil {
w.Groups = oldW.Groups
}
// reset permissions at the node level
w.VisitNode(func(n *sdk.Node, w *sdk.Workflow) {
n.Groups = nil
if oldW != nil {
oldN := oldW.WorkflowData.NodeByName(n.Name)
if oldN != nil {
n.Groups = oldN.Groups
}
}
})
} else {
// The import is triggered by a user, we have to check the groups
// FIXME: call the same function than the handlers
}

// create the workflow if not exists
if oldW == nil {
if err := Insert(ctx, db, store, proj, w); err != nil {
Expand All @@ -44,11 +68,11 @@ func Import(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store cache.St
w.Icon = oldW.Icon
}

if !force {
if !opts.Force {
return sdk.NewErrorFrom(sdk.ErrAlreadyExist, "workflow exists")
}

if force && oldW != nil && oldW.FromRepository != "" && w.FromRepository == "" {
if opts.Force && oldW != nil && oldW.FromRepository != "" && w.FromRepository == "" {
if err := detachResourceFromRepository(db, proj.ID, oldW, msgChan); err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion engine/api/workflow/workflow_importer_test.go
Expand Up @@ -483,7 +483,7 @@ func TestImport(t *testing.T) {
}
}

if err := workflow.Import(context.TODO(), db, cache, *proj, wf, tt.args.w, u, tt.args.force, nil); err != nil {
if err := workflow.Import(context.TODO(), db, cache, *proj, wf, tt.args.w, u, workflow.ImportOptions{Force: tt.args.force}, nil); err != nil {
if !tt.wantErr {
t.Errorf("Import() error = %v, wantErr %v", err, tt.wantErr)
} else {
Expand Down
2 changes: 1 addition & 1 deletion engine/api/workflow/workflow_parser.go
Expand Up @@ -204,7 +204,7 @@ func ParseAndImport(ctx context.Context, db gorpmapper.SqlExecutorWithTx, store
}
}(&msgList)

globalError := Import(ctx, db, store, proj, oldW, w, u, opts.Force, msgChan)
globalError := Import(ctx, db, store, proj, oldW, w, u, opts, msgChan)
close(msgChan)
done.Wait()

Expand Down

0 comments on commit b9151b4

Please sign in to comment.