Skip to content

Commit

Permalink
fix(endpoint): when looking up a endpoint we should allow org only lo…
Browse files Browse the repository at this point in the history
…okup (#16050)

* fix(endpoint): when looking up a endpoint we should allow org only lookup

In the current system the api always adds "UserID" to the filter. This only
allows the system to look up endpoints that user created. The behavior should be
that we filter based on user input and use authorizor to hide things they shouldn't see.
  • Loading branch information
lyondhill authored Nov 27, 2019
1 parent fe94c5c commit 997168c
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 65 deletions.
8 changes: 8 additions & 0 deletions authorizer/notification_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@ func (s *NotificationEndpointService) FindNotificationEndpointByID(ctx context.C

// FindNotificationEndpoints retrieves all notification endpoints that match the provided filter and then filters the list down to only the resources that are authorized.
func (s *NotificationEndpointService) FindNotificationEndpoints(ctx context.Context, filter influxdb.NotificationEndpointFilter, opt ...influxdb.FindOptions) ([]influxdb.NotificationEndpoint, int, error) {
// TODO: This is a temporary fix as to not fetch the entire collection when no filter is provided.
if !filter.UserID.Valid() && filter.OrgID == nil {
return nil, 0, &influxdb.Error{
Code: influxdb.EUnauthorized,
Msg: "cannot process a request without a org or user filter",
}
}

// TODO: we'll likely want to push this operation into the database eventually since fetching the whole list of data
// will likely be expensive.
edps, _, err := s.s.FindNotificationEndpoints(ctx, filter, opt...)
Expand Down
62 changes: 2 additions & 60 deletions authorizer/notification_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,65 +138,6 @@ func TestNotificationEndpointService_FindNotificationEndpoints(t *testing.T) {
args args
wants wants
}{
{
name: "authorized to see all notificationEndpoints",
fields: fields{
NotificationEndpointService: &mock.NotificationEndpointService{
FindNotificationEndpointsF: func(ctx context.Context, filter influxdb.NotificationEndpointFilter, opt ...influxdb.FindOptions) ([]influxdb.NotificationEndpoint, int, error) {
return []influxdb.NotificationEndpoint{
&endpoint.Slack{
Base: endpoint.Base{
ID: 1,
OrgID: 10,
},
},
&endpoint.Slack{
Base: endpoint.Base{
ID: 2,
OrgID: 10,
},
},
&endpoint.HTTP{
Base: endpoint.Base{
ID: 3,
OrgID: 11,
},
},
}, 3, nil
},
},
},
args: args{
permission: influxdb.Permission{
Action: "read",
Resource: influxdb.Resource{
Type: influxdb.OrgsResourceType,
},
},
},
wants: wants{
notificationEndpoints: []influxdb.NotificationEndpoint{
&endpoint.Slack{
Base: endpoint.Base{
ID: 1,
OrgID: 10,
},
},
&endpoint.Slack{
Base: endpoint.Base{
ID: 2,
OrgID: 10,
},
},
&endpoint.HTTP{
Base: endpoint.Base{
ID: 3,
OrgID: 11,
},
},
},
},
},
{
name: "authorized to access a single orgs notificationEndpoints",
fields: fields{
Expand Down Expand Up @@ -262,7 +203,8 @@ func TestNotificationEndpointService_FindNotificationEndpoints(t *testing.T) {
ctx := context.Background()
ctx = influxdbcontext.SetAuthorizer(ctx, &Authorizer{[]influxdb.Permission{tt.args.permission}})

edps, _, err := s.FindNotificationEndpoints(ctx, influxdb.NotificationEndpointFilter{})
oid := influxdb.ID(10)
edps, _, err := s.FindNotificationEndpoints(ctx, influxdb.NotificationEndpointFilter{OrgID: &oid})
influxdbtesting.ErrorsEqual(t, err, tt.wants.err)

if diff := cmp.Diff(edps, tt.wants.notificationEndpoints, notificationEndpointCmpOptions...); diff != "" {
Expand Down
14 changes: 9 additions & 5 deletions http/notification_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,13 +264,8 @@ func (h *NotificationEndpointHandler) handleGetNotificationEndpoint(w http.Respo
}

func decodeNotificationEndpointFilter(ctx context.Context, r *http.Request) (*influxdb.NotificationEndpointFilter, *influxdb.FindOptions, error) {
auth, err := pctx.GetAuthorizer(ctx)
if err != nil {
return nil, nil, err
}
f := &influxdb.NotificationEndpointFilter{
UserResourceMappingFilter: influxdb.UserResourceMappingFilter{
UserID: auth.GetUserID(),
ResourceType: influxdb.NotificationEndpointResourceType,
},
}
Expand All @@ -294,6 +289,15 @@ func decodeNotificationEndpointFilter(ctx context.Context, r *http.Request) (*in
} else if orgNameStr := q.Get("org"); orgNameStr != "" {
*f.Org = orgNameStr
}

if userID := q.Get("user"); userID != "" {
id, err := influxdb.IDFromString(userID)
if err != nil {
return f, opts, err
}
f.UserID = *id
}

return f, opts, err
}

Expand Down

0 comments on commit 997168c

Please sign in to comment.