From d705841a00ec37184818aca618f82501a2b81560 Mon Sep 17 00:00:00 2001 From: Jeffrey Smith II Date: Mon, 13 Jun 2022 15:52:28 -0400 Subject: [PATCH] feat: error when creating v1 auth with a nonexistent bucket id (#23422) * feat: error when creating v1 auth with a nonexistent bucket id * fix: only check for buckets * refactor: test cleanup for clarity --- cmd/influxd/upgrade/security_test.go | 14 +++ v1/authorization/error.go | 7 ++ v1/authorization/storage_authorization.go | 12 +++ .../storage_authorization_test.go | 87 +++++++++++++++++++ 4 files changed, 120 insertions(+) diff --git a/cmd/influxd/upgrade/security_test.go b/cmd/influxd/upgrade/security_test.go index e8b895d7f5..1c779e90c6 100644 --- a/cmd/influxd/upgrade/security_test.go +++ b/cmd/influxd/upgrade/security_test.go @@ -3,6 +3,7 @@ package upgrade import ( "context" "errors" + "fmt" "reflect" "sort" "testing" @@ -202,6 +203,19 @@ func TestUpgradeSecurity(t *testing.T) { userID: oResp.Auth.UserID, } + for k, v := range tc.db2ids { + for i, id := range v { + b := &influxdb.Bucket{ + ID: id, + Name: fmt.Sprintf("%s_%d", k, id), + OrgID: targetOptions.orgID, + } + err := tenantSvc.CreateBucket(context.Background(), b) + require.NoError(t, err) + tc.db2ids[k][i] = b.ID + } + } + // fill in expected permissions now that we know IDs for _, want := range tc.want { for _, user := range tc.users { diff --git a/v1/authorization/error.go b/v1/authorization/error.go index 8247169e03..a9a658f274 100644 --- a/v1/authorization/error.go +++ b/v1/authorization/error.go @@ -38,6 +38,13 @@ var ( Code: errors.EConflict, Msg: "token already exists", } + + // ErrBucketNotFound is used when attempting to create an authorization + // with a bucket id that does not exist + ErrBucketNotFound = &errors.Error{ + Code: errors.ENotFound, + Msg: "bucket not found when creating auth", + } ) // ErrInvalidAuthIDError is used when a service was provided an invalid ID. diff --git a/v1/authorization/storage_authorization.go b/v1/authorization/storage_authorization.go index 50688b696d..f0635f65e7 100644 --- a/v1/authorization/storage_authorization.go +++ b/v1/authorization/storage_authorization.go @@ -10,6 +10,7 @@ import ( "github.com/influxdata/influxdb/v2/kit/platform/errors" "github.com/influxdata/influxdb/v2/kv" jsonp "github.com/influxdata/influxdb/v2/pkg/jsonparser" + "github.com/influxdata/influxdb/v2/tenant" ) func authIndexKey(n string) []byte { @@ -68,6 +69,17 @@ func (s *Store) CreateAuthorization(ctx context.Context, tx kv.Tx, a *influxdb.A a.ID = id } + ts := tenant.NewStore(s.kvStore) + for _, p := range a.Permissions { + if p.Resource.ID == nil || p.Resource.Type != influxdb.BucketsResourceType { + continue + } + _, err := ts.GetBucket(ctx, tx, *p.Resource.ID) + if err == tenant.ErrBucketNotFound { + return ErrBucketNotFound + } + } + if err := s.uniqueAuthToken(ctx, tx, a); err != nil { return ErrTokenAlreadyExistsError } diff --git a/v1/authorization/storage_authorization_test.go b/v1/authorization/storage_authorization_test.go index 206c8575ec..4c10beb47b 100644 --- a/v1/authorization/storage_authorization_test.go +++ b/v1/authorization/storage_authorization_test.go @@ -12,7 +12,9 @@ import ( "github.com/influxdata/influxdb/v2/kv" "github.com/influxdata/influxdb/v2/kv/migration/all" "github.com/influxdata/influxdb/v2/pkg/pointer" + "github.com/influxdata/influxdb/v2/tenant" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" ) @@ -225,6 +227,91 @@ func TestAuth(t *testing.T) { } } +func TestAuthBucketNotExists(t *testing.T) { + store := inmem.NewKVStore() + if err := all.Up(context.Background(), zaptest.NewLogger(t), store); err != nil { + t.Fatal(err) + } + + ts, err := NewStore(store) + require.NoError(t, err) + + bucketID := platform.ID(1) + tenant := tenant.NewStore(store) + err = tenant.Update(context.Background(), func(tx kv.Tx) error { + err := tenant.CreateBucket(context.Background(), tx, &influxdb.Bucket{ + ID: bucketID, + OrgID: platform.ID(10), + Name: "testbucket", + }) + if err != nil { + return err + } + + b, err := tenant.GetBucketByName(context.Background(), tx, platform.ID(10), "testbucket") + if err != nil { + return err + } + + bucketID = b.ID + + return nil + }) + require.NoError(t, err) + + perm1, err := influxdb.NewPermissionAtID( + bucketID, + influxdb.ReadAction, + influxdb.BucketsResourceType, + platform.ID(10), + ) + require.NoError(t, err) + + perm2, err := influxdb.NewPermissionAtID( + platform.ID(2), + influxdb.ReadAction, + influxdb.BucketsResourceType, + platform.ID(10), + ) + require.NoError(t, err) + + err = ts.Update(context.Background(), func(tx kv.Tx) error { + err = ts.CreateAuthorization(context.Background(), tx, &influxdb.Authorization{ + ID: platform.ID(1), + Token: "buckettoken", + OrgID: platform.ID(10), + UserID: platform.ID(4), + Status: influxdb.Active, + Permissions: []influxdb.Permission{ + *perm1, + }, + }) + + return err + }) + + require.NoErrorf(t, err, "Authorization creating should have succeeded") + + err = ts.Update(context.Background(), func(tx kv.Tx) error { + err = ts.CreateAuthorization(context.Background(), tx, &influxdb.Authorization{ + ID: platform.ID(1), + Token: "buckettoken", + OrgID: platform.ID(10), + UserID: platform.ID(4), + Status: influxdb.Active, + Permissions: []influxdb.Permission{ + *perm2, + }, + }) + + return err + }) + + if err == nil || err != ErrBucketNotFound { + t.Fatalf("Authorization creating should have failed with ErrBucketNotFound [Error]: %v", err) + } +} + func Test_filterAuthorizationsFn(t *testing.T) { var ( otherID = platform.ID(999)