Skip to content

Commit

Permalink
fix: enforce bucket authentication on task update
Browse files Browse the repository at this point in the history
  • Loading branch information
GeorgeMac committed Jul 26, 2019
1 parent a961947 commit 5df8973
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 7 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

### Bug Fixes

1. [14480](https://github.com/influxdata/influxdb/pull/14480): Fix authentication when updating a task with invalid org or bucket.

## v2.0.0-alpha.16 [2019-07-25]

### Bug Fixes
Expand Down
7 changes: 5 additions & 2 deletions authorizer/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,11 @@ func (ts *taskServiceValidator) UpdateTask(ctx context.Context, id platform.ID,
return nil, err
}

if err := ts.validateBucket(ctx, task.Flux, task.OrganizationID, loggerFields...); err != nil {
return nil, err
// given an update to the task flux definition
if upd.Flux != nil {
if err := ts.validateBucket(ctx, *upd.Flux, task.OrganizationID, loggerFields...); err != nil {
return nil, err
}
}

return ts.TaskService.UpdateTask(ctx, id, upd)
Expand Down
67 changes: 62 additions & 5 deletions authorizer/task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,19 @@ package authorizer_test

import (
"context"
"errors"
"fmt"
"testing"

"github.com/influxdata/influxdb"
platform "github.com/influxdata/influxdb"
"github.com/influxdata/influxdb/authorizer"
pctx "github.com/influxdata/influxdb/context"
"github.com/influxdata/influxdb/http"
"github.com/influxdata/influxdb/inmem"
"github.com/influxdata/influxdb/mock"
_ "github.com/influxdata/influxdb/query/builtin"
"github.com/influxdata/influxdb/task/backend"
"github.com/pkg/errors"
"go.uber.org/zap/zaptest"
)

Expand Down Expand Up @@ -113,8 +115,9 @@ from(bucket:"holder") |> range(start:-5m) |> to(bucket:"holder", org:"thing")`,

func TestValidations(t *testing.T) {
var (
taskID influxdb.ID = 0x7456
runID influxdb.ID = 0x402
taskID = influxdb.ID(0x7456)
runID = influxdb.ID(0x402)
otherOrg = &influxdb.Organization{Name: "other_org"}
)

inmem := inmem.NewService()
Expand All @@ -129,10 +132,24 @@ func TestValidations(t *testing.T) {
if err != nil {
t.Fatal(err)
}
orgID := r.Org.ID
validTaskService := authorizer.NewTaskService(zaptest.NewLogger(t), mockTaskService(orgID, taskID, runID), inmem)

if err := inmem.CreateOrganization(context.Background(), otherOrg); err != nil {
t.Fatal(err)
}

otherBucket := &influxdb.Bucket{
Name: "other_bucket",
OrgID: otherOrg.ID,
}

if err = inmem.CreateBucket(context.Background(), otherBucket); err != nil {
t.Fatal(err)
}

var (
orgID = r.Org.ID
validTaskService = authorizer.NewTaskService(zaptest.NewLogger(t), mockTaskService(orgID, taskID, runID), inmem)

// Read all tasks in org.
orgReadAllTaskPermissions = []influxdb.Permission{
{Action: influxdb.ReadAction, Resource: influxdb.Resource{Type: influxdb.TasksResourceType, OrgID: &orgID}},
Expand Down Expand Up @@ -363,6 +380,46 @@ from(bucket:"cows") |> range(start:-5m) |> to(bucket:"cows", org:"thing")`
return nil
},
},
{
name: "UpdateTask with bad org",
auth: &influxdb.Authorization{Status: "active", Permissions: orgWriteAllTaskBucketPermissions},
check: func(ctx context.Context, svc influxdb.TaskService) error {
var (
flux = `option task = {
name: "my_task",
every: 1s,
}
from(bucket:"cows") |> range(start:-5m) |> to(bucket:"other_bucket", org:"other_org")`
_, err = svc.UpdateTask(ctx, taskID, influxdb.TaskUpdate{
Flux: &flux,
})
)

perr, ok := err.(*influxdb.Error)
if !ok {
return fmt.Errorf("expected platform error, got %q of type %T", err, err)
}

if perr.Code != influxdb.EInvalid {
return fmt.Errorf(`expected "invalid", got %q`, perr.Code)
}

if perr.Msg != "Failed to authorize." {
return fmt.Errorf(`expected "Failed to authorize.", got %q`, perr.Msg)
}

cerr, ok := errors.Cause(perr.Err).(*platform.Error)
if !ok {
return fmt.Errorf("expected platform error, got %q of type %T", perr.Err, perr.Err)
}

if cerr.Code != influxdb.ENotFound {
return fmt.Errorf(`expected "not found", got %q`, perr.Code)
}

return nil
},
},
{
name: "DeleteTask missing auth",
auth: &influxdb.Authorization{Permissions: []influxdb.Permission{}},
Expand Down

0 comments on commit 5df8973

Please sign in to comment.