Skip to content

Commit

Permalink
fix(labels): add check for write permissions to create label (#17174)
Browse files Browse the repository at this point in the history
  • Loading branch information
AlirieGray authored Mar 12, 2020
1 parent cdbf532 commit 991002d
Show file tree
Hide file tree
Showing 11 changed files with 272 additions and 238 deletions.
8 changes: 6 additions & 2 deletions authorizer/label.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ func (s *LabelService) FindLabels(ctx context.Context, filter influxdb.LabelFilt
// FindResourceLabels retrieves all labels belonging to the filtering resource if the authorizer on context has read access to it.
// Then it filters the list down to only the labels that are authorized.
func (s *LabelService) FindResourceLabels(ctx context.Context, filter influxdb.LabelMappingFilter) ([]*influxdb.Label, error) {
if err := filter.ResourceType.Valid(); err != nil {
return nil, err
}

if s.orgSvc == nil {
return nil, errors.New("failed to find orgSvc")
}
Expand Down Expand Up @@ -167,9 +171,9 @@ func (s *LabelService) FindResourceLabels(ctx context.Context, filter influxdb.L
return labels, nil
}

// CreateLabel checks to see if the authorizer on context has read access to the new label's org.
// CreateLabel checks to see if the authorizer on context has write access to the new label's org.
func (s *LabelService) CreateLabel(ctx context.Context, l *influxdb.Label) error {
if err := authorizeReadOrg(ctx, l.OrgID); err != nil {
if err := authorizeWriteOrg(ctx, l.OrgID); err != nil {
return err
}

Expand Down
36 changes: 31 additions & 5 deletions authorizer/label_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ func TestLabelService_CreateLabel(t *testing.T) {
wants wants
}{
{
name: "authorized to create label",
name: "unauthorized to create label with read only permission",
fields: fields{
LabelService: &mock.LabelService{
CreateLabelFn: func(ctx context.Context, l *influxdb.Label) error {
Expand All @@ -527,11 +527,14 @@ func TestLabelService_CreateLabel(t *testing.T) {
},
},
wants: wants{
err: nil,
err: &influxdb.Error{
Msg: "write:orgs/020f755c3c083000 is unauthorized",
Code: influxdb.EUnauthorized,
},
},
},
{
name: "unauthorized to create label",
name: "unauthorized to create label with incomplete write permission",
fields: fields{
LabelService: &mock.LabelService{
CreateLabelFn: func(ctx context.Context, b *influxdb.Label) error {
Expand All @@ -541,19 +544,42 @@ func TestLabelService_CreateLabel(t *testing.T) {
},
args: args{
permission: influxdb.Permission{
Action: "read",
Action: "write",
Resource: influxdb.Resource{
Type: influxdb.LabelsResourceType,
},
},
},
wants: wants{
err: &influxdb.Error{
Msg: "read:orgs/020f755c3c083000 is unauthorized",
Msg: "write:orgs/020f755c3c083000 is unauthorized",
Code: influxdb.EUnauthorized,
},
},
},

{
name: "authorized to create label",
fields: fields{
LabelService: &mock.LabelService{
CreateLabelFn: func(ctx context.Context, l *influxdb.Label) error {
return nil
},
},
},
args: args{
permission: influxdb.Permission{
Action: "write",
Resource: influxdb.Resource{
ID: influxdbtesting.IDPtr(orgOneInfluxID),
Type: influxdb.OrgsResourceType,
},
},
},
wants: wants{
err: nil,
},
},
}

for _, tt := range tests {
Expand Down
2 changes: 2 additions & 0 deletions http/api_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ func NewAPIHandler(b *APIBackend, opts ...APIHandlerOptFn) *APIHandler {

bucketBackend := NewBucketBackend(b.Logger.With(zap.String("handler", "bucket")), b)
bucketBackend.BucketService = authorizer.NewBucketService(b.BucketService, b.UserResourceMappingService)
bucketBackend.LabelService = authorizer.NewLabelServiceWithOrg(b.LabelService, b.OrgLookupService)
h.Mount(prefixBuckets, NewBucketHandler(b.Logger, bucketBackend))

checkBackend := NewCheckBackend(b.Logger.With(zap.String("handler", "check")), b)
Expand Down Expand Up @@ -166,6 +167,7 @@ func NewAPIHandler(b *APIBackend, opts ...APIHandlerOptFn) *APIHandler {
orgBackend := NewOrgBackend(b.Logger.With(zap.String("handler", "org")), b)
orgBackend.OrganizationService = authorizer.NewOrgService(b.OrganizationService)
orgBackend.SecretService = authorizer.NewSecretService(b.SecretService)
orgBackend.LabelService = authorizer.NewLabelServiceWithOrg(b.LabelService, b.OrgLookupService)
h.Mount(prefixOrganizations, NewOrgHandler(b.Logger, orgBackend))

scraperBackend := NewScraperBackend(b.Logger.With(zap.String("handler", "scraper")), b)
Expand Down
7 changes: 4 additions & 3 deletions http/bucket_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ type bucketsResponse struct {
func newBucketsResponse(ctx context.Context, opts influxdb.FindOptions, f influxdb.BucketFilter, bs []*influxdb.Bucket, labelService influxdb.LabelService) *bucketsResponse {
rs := make([]*bucketResponse, 0, len(bs))
for _, b := range bs {
labels, _ := labelService.FindResourceLabels(ctx, influxdb.LabelMappingFilter{ResourceID: b.ID})
labels, _ := labelService.FindResourceLabels(ctx, influxdb.LabelMappingFilter{ResourceID: b.ID, ResourceType: influxdb.BucketsResourceType})
rs = append(rs, NewBucketResponse(b, labels))
}
return &bucketsResponse{
Expand Down Expand Up @@ -405,7 +405,7 @@ func (h *BucketHandler) handleGetBucket(w http.ResponseWriter, r *http.Request)
return
}

labels, err := h.LabelService.FindResourceLabels(ctx, influxdb.LabelMappingFilter{ResourceID: b.ID})
labels, err := h.LabelService.FindResourceLabels(ctx, influxdb.LabelMappingFilter{ResourceID: b.ID, ResourceType: influxdb.BucketsResourceType})
if err != nil {
h.api.Err(w, err)
return
Expand Down Expand Up @@ -600,7 +600,8 @@ func (h *BucketHandler) handlePatchBucket(w http.ResponseWriter, r *http.Request
// TODO: should move to service to encapsulate labels and what any other dependencies. Future
// work for service definition
labels, err := h.LabelService.FindResourceLabels(r.Context(), influxdb.LabelMappingFilter{
ResourceID: b.ID,
ResourceID: b.ID,
ResourceType: influxdb.BucketsResourceType,
})
if err != nil {
h.api.Err(w, err)
Expand Down
8 changes: 4 additions & 4 deletions http/check_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func (h *CheckHandler) newChecksResponse(ctx context.Context, chks []influxdb.Ch
Links: newPagingLinks(prefixChecks, opts, f, len(chks)),
}
for _, chk := range chks {
labels, _ := labelService.FindResourceLabels(ctx, influxdb.LabelMappingFilter{ResourceID: chk.GetID()})
labels, _ := labelService.FindResourceLabels(ctx, influxdb.LabelMappingFilter{ResourceID: chk.GetID(), ResourceType: influxdb.ChecksResourceType})
cr, err := h.newCheckResponse(ctx, chk, labels)
if err != nil {
h.log.Info("Failed to retrieve task associated with check", zap.String("checkID", chk.GetID().String()))
Expand Down Expand Up @@ -334,7 +334,7 @@ func (h *CheckHandler) handleGetCheck(w http.ResponseWriter, r *http.Request) {
}
h.log.Debug("Check retrieved", zap.String("check", fmt.Sprint(chk)))

labels, err := h.LabelService.FindResourceLabels(ctx, influxdb.LabelMappingFilter{ResourceID: chk.GetID()})
labels, err := h.LabelService.FindResourceLabels(ctx, influxdb.LabelMappingFilter{ResourceID: chk.GetID(), ResourceType: influxdb.ChecksResourceType})
if err != nil {
h.HandleHTTPError(ctx, err, w)
return
Expand Down Expand Up @@ -613,7 +613,7 @@ func (h *CheckHandler) handlePutCheck(w http.ResponseWriter, r *http.Request) {
return
}

labels, err := h.LabelService.FindResourceLabels(ctx, influxdb.LabelMappingFilter{ResourceID: c.GetID()})
labels, err := h.LabelService.FindResourceLabels(ctx, influxdb.LabelMappingFilter{ResourceID: c.GetID(), ResourceType: influxdb.ChecksResourceType})
if err != nil {
h.HandleHTTPError(ctx, err, w)
return
Expand Down Expand Up @@ -648,7 +648,7 @@ func (h *CheckHandler) handlePatchCheck(w http.ResponseWriter, r *http.Request)
return
}

labels, err := h.LabelService.FindResourceLabels(ctx, influxdb.LabelMappingFilter{ResourceID: chk.GetID()})
labels, err := h.LabelService.FindResourceLabels(ctx, influxdb.LabelMappingFilter{ResourceID: chk.GetID(), ResourceType: influxdb.ChecksResourceType})
if err != nil {
h.HandleHTTPError(ctx, err, w)
return
Expand Down
Loading

0 comments on commit 991002d

Please sign in to comment.