fix(buckets): prevent returning system buckets to unauthorized users (#17117)

pull/17208/head
Gavin Cabbage 2020-03-11 13:06:08 -04:00 committed by GitHub
parent f98874566c
commit f5e8547482
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 88 additions and 38 deletions

View File

@ -2,6 +2,9 @@ package authorizer
import (
"context"
"fmt"
icontext "github.com/influxdata/influxdb/context"
"github.com/influxdata/influxdb"
"github.com/influxdata/influxdb/kit/tracing"
@ -13,12 +16,14 @@ var _ influxdb.BucketService = (*BucketService)(nil)
// against it appropriately.
type BucketService struct {
s influxdb.BucketService
u influxdb.UserResourceMappingService
}
// NewBucketService constructs an instance of an authorizing bucket serivce.
func NewBucketService(s influxdb.BucketService) *BucketService {
func NewBucketService(s influxdb.BucketService, u influxdb.UserResourceMappingService) *BucketService {
return &BucketService{
s: s,
u: u,
}
}
@ -26,22 +31,6 @@ func newBucketPermission(a influxdb.Action, orgID, id influxdb.ID) (*influxdb.Pe
return influxdb.NewPermissionAtID(id, a, influxdb.BucketsResourceType, orgID)
}
func authorizeReadBucket(ctx context.Context, orgID, id influxdb.ID) error {
span, ctx := tracing.StartSpanFromContext(ctx)
defer span.Finish()
p, err := newBucketPermission(influxdb.ReadAction, orgID, id)
if err != nil {
return err
}
if err := IsAllowed(ctx, *p); err != nil {
return err
}
return nil
}
func authorizeWriteBucket(ctx context.Context, orgID, id influxdb.ID) error {
p, err := newBucketPermission(influxdb.WriteAction, orgID, id)
if err != nil {
@ -55,6 +44,66 @@ func authorizeWriteBucket(ctx context.Context, orgID, id influxdb.ID) error {
return nil
}
func authorizeReadBucket(ctx context.Context, b *influxdb.Bucket, u influxdb.UserResourceMappingService) error {
switch b.Type {
case influxdb.BucketTypeSystem:
return authorizeReadSystemBucket(ctx, b, u)
default:
return authorizeReadUserBucket(ctx, b)
}
}
func authorizeReadUserBucket(ctx context.Context, b *influxdb.Bucket) error {
span, ctx := tracing.StartSpanFromContext(ctx)
defer span.Finish()
p, err := newBucketPermission(influxdb.ReadAction, b.OrgID, b.ID)
if err != nil {
return err
}
if err := IsAllowed(ctx, *p); err != nil {
return err
}
return nil
}
func authorizeReadSystemBucket(ctx context.Context, b *influxdb.Bucket, u influxdb.UserResourceMappingService) error {
// HACK: remove once system buckets are migrated away from hard coded values
if !b.OrgID.Valid() && (b.ID == influxdb.TasksSystemBucketID || b.ID == influxdb.MonitoringSystemBucketID) {
return nil
}
userID, err := icontext.GetUserID(ctx)
if err != nil {
return &influxdb.Error{
Code: influxdb.EUnauthorized,
Msg: fmt.Sprintf("unauthorized"),
Err: err,
}
}
ms, _, err := u.FindUserResourceMappings(ctx, influxdb.UserResourceMappingFilter{
UserID: userID,
ResourceType: influxdb.OrgsResourceType,
})
if err != nil {
return fmt.Errorf("finding organization mapping for user %s: %v", b.ID, err)
}
for _, m := range ms {
if m.ResourceID == b.OrgID {
return nil
}
}
return &influxdb.Error{
Code: influxdb.EUnauthorized,
Msg: fmt.Sprintf("unauthorized"),
}
}
// FindBucketByID checks to see if the authorizer on context has read access to the id provided.
func (s *BucketService) FindBucketByID(ctx context.Context, id influxdb.ID) (*influxdb.Bucket, error) {
span, ctx := tracing.StartSpanFromContext(ctx)
@ -65,7 +114,7 @@ func (s *BucketService) FindBucketByID(ctx context.Context, id influxdb.ID) (*in
return nil, err
}
if err := authorizeReadBucket(ctx, b.OrgID, id); err != nil {
if err := authorizeReadBucket(ctx, b, s.u); err != nil {
return nil, err
}
@ -82,7 +131,7 @@ func (s *BucketService) FindBucketByName(ctx context.Context, orgID influxdb.ID,
return nil, err
}
if err := authorizeReadBucket(ctx, b.OrgID, b.ID); err != nil {
if err := authorizeReadBucket(ctx, b, s.u); err != nil {
return nil, err
}
@ -99,7 +148,7 @@ func (s *BucketService) FindBucket(ctx context.Context, filter influxdb.BucketFi
return nil, err
}
if err := authorizeReadBucket(ctx, b.OrgID, b.ID); err != nil {
if err := authorizeReadBucket(ctx, b, s.u); err != nil {
return nil, err
}
@ -122,13 +171,7 @@ func (s *BucketService) FindBuckets(ctx context.Context, filter influxdb.BucketF
// https://github.com/golang/go/wiki/SliceTricks#filtering-without-allocating
buckets := bs[:0]
for _, b := range bs {
// HACK: remove once system buckets are migrated away from hard coded values
if b.Type == influxdb.BucketTypeSystem {
buckets = append(buckets, b)
continue
}
err := authorizeReadBucket(ctx, b.OrgID, b.ID)
err := authorizeReadBucket(ctx, b, s.u)
if err != nil && influxdb.ErrorCode(err) != influxdb.EUnauthorized {
return nil, 0, err
}

View File

@ -104,7 +104,7 @@ func TestBucketService_FindBucketByID(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := authorizer.NewBucketService(tt.fields.BucketService)
s := authorizer.NewBucketService(tt.fields.BucketService, nil)
ctx := context.Background()
ctx = influxdbcontext.SetAuthorizer(ctx, &Authorizer{[]influxdb.Permission{tt.args.permission}})
@ -189,7 +189,7 @@ func TestBucketService_FindBucket(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := authorizer.NewBucketService(tt.fields.BucketService)
s := authorizer.NewBucketService(tt.fields.BucketService, nil)
ctx := context.Background()
ctx = influxdbcontext.SetAuthorizer(ctx, &Authorizer{[]influxdb.Permission{tt.args.permission}})
@ -314,7 +314,7 @@ func TestBucketService_FindBuckets(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := authorizer.NewBucketService(tt.fields.BucketService)
s := authorizer.NewBucketService(tt.fields.BucketService, nil)
ctx := context.Background()
ctx = influxdbcontext.SetAuthorizer(ctx, &Authorizer{[]influxdb.Permission{tt.args.permission}})
@ -429,7 +429,7 @@ func TestBucketService_UpdateBucket(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := authorizer.NewBucketService(tt.fields.BucketService)
s := authorizer.NewBucketService(tt.fields.BucketService, nil)
ctx := context.Background()
ctx = influxdbcontext.SetAuthorizer(ctx, &Authorizer{tt.args.permissions})
@ -534,7 +534,7 @@ func TestBucketService_DeleteBucket(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := authorizer.NewBucketService(tt.fields.BucketService)
s := authorizer.NewBucketService(tt.fields.BucketService, nil)
ctx := context.Background()
ctx = influxdbcontext.SetAuthorizer(ctx, &Authorizer{tt.args.permissions})
@ -616,7 +616,7 @@ func TestBucketService_CreateBucket(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := authorizer.NewBucketService(tt.fields.BucketService)
s := authorizer.NewBucketService(tt.fields.BucketService, nil)
ctx := context.Background()
ctx = influxdbcontext.SetAuthorizer(ctx, &Authorizer{[]influxdb.Permission{tt.args.permission}})

View File

@ -596,7 +596,7 @@ func (m *Launcher) run(ctx context.Context) (err error) {
deps, err := influxdb.NewDependencies(
storageflux.NewReader(readservice.NewStore(m.engine)),
m.engine,
authorizer.NewBucketService(bucketSvc),
authorizer.NewBucketService(bucketSvc, userResourceSvc),
authorizer.NewOrgService(orgSvc),
authorizer.NewSecretService(secretSvc),
nil,
@ -820,7 +820,7 @@ func (m *Launcher) run(ctx context.Context) (err error) {
pkgerLogger := m.log.With(zap.String("service", "pkger"))
pkgSVC = pkger.NewService(
pkger.WithLogger(pkgerLogger),
pkger.WithBucketSVC(authorizer.NewBucketService(b.BucketService)),
pkger.WithBucketSVC(authorizer.NewBucketService(b.BucketService, b.UserResourceMappingService)),
pkger.WithCheckSVC(authorizer.NewCheckService(b.CheckService, authedURMSVC, authedOrgSVC)),
pkger.WithDashboardSVC(authorizer.NewDashboardService(b.DashboardService)),
pkger.WithLabelSVC(authorizer.NewLabelServiceWithOrg(b.LabelService, b.OrgLookupService)),

View File

@ -126,7 +126,7 @@ func NewAPIHandler(b *APIBackend, opts ...APIHandlerOptFn) *APIHandler {
h.Mount(prefixAuthorization, NewAuthorizationHandler(b.Logger, authorizationBackend))
bucketBackend := NewBucketBackend(b.Logger.With(zap.String("handler", "bucket")), b)
bucketBackend.BucketService = authorizer.NewBucketService(b.BucketService)
bucketBackend.BucketService = authorizer.NewBucketService(b.BucketService, b.UserResourceMappingService)
h.Mount(prefixBuckets, NewBucketHandler(b.Logger, bucketBackend))
checkBackend := NewCheckBackend(b.Logger.With(zap.String("handler", "check")), b)
@ -184,7 +184,7 @@ func NewAPIHandler(b *APIBackend, opts ...APIHandlerOptFn) *APIHandler {
sourceBackend := NewSourceBackend(b.Logger.With(zap.String("handler", "source")), b)
sourceBackend.SourceService = authorizer.NewSourceService(b.SourceService)
sourceBackend.BucketService = authorizer.NewBucketService(b.BucketService)
sourceBackend.BucketService = authorizer.NewBucketService(b.BucketService, b.UserResourceMappingService)
h.Mount(prefixSources, NewSourceHandler(b.Logger, sourceBackend))
h.Mount("/api/v2/swagger.json", newSwaggerLoader(b.Logger.With(zap.String("service", "swagger-loader")), b.HTTPErrorHandler))

View File

@ -133,7 +133,14 @@ func (s *Service) FindBucketByName(ctx context.Context, orgID influxdb.ID, n str
return b, err
}
// CreateSystemBuckets creates the task and monitoring system buckets for an organization
// CreateSystemBuckets for an organization
func (s *Service) CreateSystemBuckets(ctx context.Context, o *influxdb.Organization) error {
return s.kv.Update(ctx, func(tx Tx) error {
return s.createSystemBuckets(ctx, tx, o)
})
}
// createSystemBuckets creates the task and monitoring system buckets for an organization
func (s *Service) createSystemBuckets(ctx context.Context, tx Tx, o *influxdb.Organization) error {
tb := &influxdb.Bucket{
OrgID: o.ID,