From a481d4ab5da0e3fe9a0807a4718bf2230e436b32 Mon Sep 17 00:00:00 2001
From: George <me@georgemac.com>
Date: Fri, 26 Jul 2019 17:37:45 +0200
Subject: [PATCH] fix: enforce bucket authentication on task update (#14480)

---
 CHANGELOG.md            |  2 ++
 authorizer/task.go      |  7 +++--
 authorizer/task_test.go | 67 ++++++++++++++++++++++++++++++++++++++---
 3 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5a774234539..0577e5b37f6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
diff --git a/authorizer/task.go b/authorizer/task.go
index 69b4299b2b7..10fa4052b1b 100644
--- a/authorizer/task.go
+++ b/authorizer/task.go
@@ -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)
diff --git a/authorizer/task_test.go b/authorizer/task_test.go
index 0f0d2533981..7feb2647ae9 100644
--- a/authorizer/task_test.go
+++ b/authorizer/task_test.go
@@ -2,10 +2,11 @@ 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"
@@ -13,6 +14,7 @@ import (
 	"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"
 )
 
@@ -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()
@@ -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}},
@@ -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{}},