Skip to content

Commit

Permalink
fix(authorizer): fix endpoint handler auth
Browse files Browse the repository at this point in the history
  • Loading branch information
affo committed Mar 11, 2020
1 parent 598ec8a commit 4e22b0b
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 62 deletions.
52 changes: 36 additions & 16 deletions authorizer/notification_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,44 @@ func NewNotificationEndpointService(
}
}

func newNotificationEndpointPermission(a influxdb.Action, orgID, id influxdb.ID) (*influxdb.Permission, error) {
return influxdb.NewPermissionAtID(id, a, influxdb.NotificationEndpointResourceType, orgID)
}

func authorizeReadNotificationEndpoint(ctx context.Context, orgID, id influxdb.ID) error {
p, err := newNotificationEndpointPermission(influxdb.ReadAction, orgID, id)
if err != nil {
return err
}

if err := IsAllowed(ctx, *p); err != nil {
return err
}

return nil
}

func authorizeWriteNotificationEndpoint(ctx context.Context, orgID, id influxdb.ID) error {
p, err := newNotificationEndpointPermission(influxdb.WriteAction, orgID, id)
if err != nil {
return err
}

if err := IsAllowed(ctx, *p); err != nil {
return err
}

return nil
}

// FindNotificationEndpointByID checks to see if the authorizer on context has read access to the id provided.
func (s *NotificationEndpointService) FindNotificationEndpointByID(ctx context.Context, id influxdb.ID) (influxdb.NotificationEndpoint, error) {
edp, err := s.s.FindNotificationEndpointByID(ctx, id)
if err != nil {
return nil, err
}

if err := authorizeReadOrg(ctx, edp.GetOrgID()); err != nil {
if err := authorizeReadNotificationEndpoint(ctx, edp.GetOrgID(), edp.GetID()); err != nil {
return nil, err
}

Expand Down Expand Up @@ -64,7 +94,7 @@ func (s *NotificationEndpointService) FindNotificationEndpoints(ctx context.Cont
// https://github.com/golang/go/wiki/SliceTricks#filtering-without-allocating
endpoints := edps[:0]
for _, edp := range edps {
err := authorizeReadOrg(ctx, edp.GetOrgID())
err := authorizeReadNotificationEndpoint(ctx, edp.GetOrgID(), edp.GetID())
if err != nil && influxdb.ErrorCode(err) != influxdb.EUnauthorized {
return nil, 0, err
}
Expand All @@ -85,19 +115,9 @@ func (s *NotificationEndpointService) CreateNotificationEndpoint(ctx context.Con
if err != nil {
return err
}

pOrg, err := newOrgPermission(influxdb.WriteAction, edp.GetOrgID())
if err != nil {
if err := IsAllowed(ctx, *p); err != nil {
return err
}

err0 := IsAllowed(ctx, *p)
err1 := IsAllowed(ctx, *pOrg)

if err0 != nil && err1 != nil {
return err0
}

return s.s.CreateNotificationEndpoint(ctx, edp, userID)
}

Expand All @@ -108,7 +128,7 @@ func (s *NotificationEndpointService) UpdateNotificationEndpoint(ctx context.Con
return nil, err
}

if err := authorizeWriteOrg(ctx, edp.GetOrgID()); err != nil {
if err := authorizeWriteNotificationEndpoint(ctx, edp.GetOrgID(), edp.GetID()); err != nil {
return nil, err
}

Expand All @@ -122,7 +142,7 @@ func (s *NotificationEndpointService) PatchNotificationEndpoint(ctx context.Cont
return nil, err
}

if err := authorizeWriteOrg(ctx, edp.GetOrgID()); err != nil {
if err := authorizeWriteNotificationEndpoint(ctx, edp.GetOrgID(), edp.GetID()); err != nil {
return nil, err
}

Expand All @@ -136,7 +156,7 @@ func (s *NotificationEndpointService) DeleteNotificationEndpoint(ctx context.Con
return nil, 0, err
}

if err := authorizeWriteOrg(ctx, edp.GetOrgID()); err != nil {
if err := authorizeWriteNotificationEndpoint(ctx, edp.GetOrgID(), edp.GetID()); err != nil {
return nil, 0, err
}

Expand Down
86 changes: 43 additions & 43 deletions authorizer/notification_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ func TestNotificationEndpointService_FindNotificationEndpointByID(t *testing.T)
},
args: args{
permission: influxdb.Permission{
Action: "read",
Action: influxdb.ReadAction,
Resource: influxdb.Resource{
Type: influxdb.OrgsResourceType,
ID: influxdbtesting.IDPtr(10),
Type: influxdb.NotificationEndpointResourceType,
OrgID: influxdbtesting.IDPtr(10),
},
},
id: 1,
Expand All @@ -92,7 +92,7 @@ func TestNotificationEndpointService_FindNotificationEndpointByID(t *testing.T)
},
args: args{
permission: influxdb.Permission{
Action: "read",
Action: influxdb.ReadAction,
Resource: influxdb.Resource{
Type: influxdb.NotificationEndpointResourceType,
ID: influxdbtesting.IDPtr(2),
Expand All @@ -102,7 +102,7 @@ func TestNotificationEndpointService_FindNotificationEndpointByID(t *testing.T)
},
wants: wants{
err: &influxdb.Error{
Msg: "read:orgs/000000000000000a is unauthorized",
Msg: "read:orgs/000000000000000a/notificationEndpoints/0000000000000001 is unauthorized",
Code: influxdb.EUnauthorized,
},
},
Expand Down Expand Up @@ -164,10 +164,10 @@ func TestNotificationEndpointService_FindNotificationEndpoints(t *testing.T) {
},
args: args{
permission: influxdb.Permission{
Action: "read",
Action: influxdb.ReadAction,
Resource: influxdb.Resource{
Type: influxdb.OrgsResourceType,
ID: influxdbtesting.IDPtr(10),
Type: influxdb.NotificationEndpointResourceType,
OrgID: influxdbtesting.IDPtr(10),
},
},
},
Expand Down Expand Up @@ -254,17 +254,17 @@ func TestNotificationEndpointService_UpdateNotificationEndpoint(t *testing.T) {
id: 1,
permissions: []influxdb.Permission{
{
Action: "write",
Action: influxdb.WriteAction,
Resource: influxdb.Resource{
Type: influxdb.OrgsResourceType,
ID: influxdbtesting.IDPtr(10),
Type: influxdb.NotificationEndpointResourceType,
OrgID: influxdbtesting.IDPtr(10),
},
},
{
Action: "read",
Action: influxdb.ReadAction,
Resource: influxdb.Resource{
Type: influxdb.OrgsResourceType,
ID: influxdbtesting.IDPtr(10),
Type: influxdb.NotificationEndpointResourceType,
OrgID: influxdbtesting.IDPtr(10),
},
},
},
Expand Down Expand Up @@ -299,17 +299,17 @@ func TestNotificationEndpointService_UpdateNotificationEndpoint(t *testing.T) {
id: 1,
permissions: []influxdb.Permission{
{
Action: "read",
Action: influxdb.ReadAction,
Resource: influxdb.Resource{
Type: influxdb.OrgsResourceType,
ID: influxdbtesting.IDPtr(10),
Type: influxdb.NotificationEndpointResourceType,
OrgID: influxdbtesting.IDPtr(10),
},
},
},
},
wants: wants{
err: &influxdb.Error{
Msg: "write:orgs/000000000000000a is unauthorized",
Msg: "write:orgs/000000000000000a/notificationEndpoints/0000000000000001 is unauthorized",
Code: influxdb.EUnauthorized,
},
},
Expand Down Expand Up @@ -375,17 +375,17 @@ func TestNotificationEndpointService_PatchNotificationEndpoint(t *testing.T) {
id: 1,
permissions: []influxdb.Permission{
{
Action: "write",
Action: influxdb.WriteAction,
Resource: influxdb.Resource{
Type: influxdb.OrgsResourceType,
ID: influxdbtesting.IDPtr(10),
Type: influxdb.NotificationEndpointResourceType,
OrgID: influxdbtesting.IDPtr(10),
},
},
{
Action: "read",
Action: influxdb.ReadAction,
Resource: influxdb.Resource{
Type: influxdb.OrgsResourceType,
ID: influxdbtesting.IDPtr(10),
Type: influxdb.NotificationEndpointResourceType,
OrgID: influxdbtesting.IDPtr(10),
},
},
},
Expand Down Expand Up @@ -420,17 +420,17 @@ func TestNotificationEndpointService_PatchNotificationEndpoint(t *testing.T) {
id: 1,
permissions: []influxdb.Permission{
{
Action: "read",
Action: influxdb.ReadAction,
Resource: influxdb.Resource{
Type: influxdb.OrgsResourceType,
ID: influxdbtesting.IDPtr(10),
Type: influxdb.NotificationEndpointResourceType,
OrgID: influxdbtesting.IDPtr(10),
},
},
},
},
wants: wants{
err: &influxdb.Error{
Msg: "write:orgs/000000000000000a is unauthorized",
Msg: "write:orgs/000000000000000a/notificationEndpoints/0000000000000001 is unauthorized",
Code: influxdb.EUnauthorized,
},
},
Expand Down Expand Up @@ -490,17 +490,17 @@ func TestNotificationEndpointService_DeleteNotificationEndpoint(t *testing.T) {
id: 1,
permissions: []influxdb.Permission{
{
Action: "write",
Action: influxdb.WriteAction,
Resource: influxdb.Resource{
Type: influxdb.OrgsResourceType,
ID: influxdbtesting.IDPtr(10),
Type: influxdb.NotificationEndpointResourceType,
OrgID: influxdbtesting.IDPtr(10),
},
},
{
Action: "read",
Action: influxdb.ReadAction,
Resource: influxdb.Resource{
Type: influxdb.OrgsResourceType,
ID: influxdbtesting.IDPtr(10),
Type: influxdb.NotificationEndpointResourceType,
OrgID: influxdbtesting.IDPtr(10),
},
},
},
Expand Down Expand Up @@ -530,17 +530,17 @@ func TestNotificationEndpointService_DeleteNotificationEndpoint(t *testing.T) {
id: 1,
permissions: []influxdb.Permission{
{
Action: "read",
Action: influxdb.ReadAction,
Resource: influxdb.Resource{
Type: influxdb.OrgsResourceType,
ID: influxdbtesting.IDPtr(10),
Type: influxdb.NotificationEndpointResourceType,
OrgID: influxdbtesting.IDPtr(10),
},
},
},
},
wants: wants{
err: &influxdb.Error{
Msg: "write:orgs/000000000000000a is unauthorized",
Msg: "write:orgs/000000000000000a/notificationEndpoints/0000000000000001 is unauthorized",
Code: influxdb.EUnauthorized,
},
},
Expand Down Expand Up @@ -592,7 +592,7 @@ func TestNotificationEndpointService_CreateNotificationEndpoint(t *testing.T) {
args: args{
orgID: 10,
permission: influxdb.Permission{
Action: "write",
Action: influxdb.WriteAction,
Resource: influxdb.Resource{
Type: influxdb.NotificationEndpointResourceType,
OrgID: influxdbtesting.IDPtr(10),
Expand All @@ -615,10 +615,10 @@ func TestNotificationEndpointService_CreateNotificationEndpoint(t *testing.T) {
args: args{
orgID: 10,
permission: influxdb.Permission{
Action: "write",
Action: influxdb.WriteAction,
Resource: influxdb.Resource{
Type: influxdb.OrgsResourceType,
ID: influxdbtesting.IDPtr(10),
Type: influxdb.NotificationEndpointResourceType,
OrgID: influxdbtesting.IDPtr(10),
},
},
},
Expand All @@ -638,7 +638,7 @@ func TestNotificationEndpointService_CreateNotificationEndpoint(t *testing.T) {
args: args{
orgID: 10,
permission: influxdb.Permission{
Action: "write",
Action: influxdb.WriteAction,
Resource: influxdb.Resource{
Type: influxdb.NotificationEndpointResourceType,
ID: influxdbtesting.IDPtr(1),
Expand Down
15 changes: 12 additions & 3 deletions http/notification_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,9 @@ var _ influxdb.NotificationEndpointService = (*NotificationEndpointService)(nil)

// FindNotificationEndpointByID returns a single notification endpoint by ID.
func (s *NotificationEndpointService) FindNotificationEndpointByID(ctx context.Context, id influxdb.ID) (influxdb.NotificationEndpoint, error) {
if !id.Valid() {
return nil, fmt.Errorf("invalid ID: please provide a valid ID")
}
var resp notificationEndpointDecoder
err := s.Client.
Get(prefixNotificationEndpoints, id.String()).
Expand Down Expand Up @@ -621,8 +624,6 @@ func (s *NotificationEndpointService) FindNotificationEndpoints(ctx context.Cont
// TODO(@jsteenb2): this is unsatisfactory, we have no way of grabbing the new notification endpoint without
// serious hacky hackertoning. Put it on the list...
func (s *NotificationEndpointService) CreateNotificationEndpoint(ctx context.Context, ne influxdb.NotificationEndpoint, userID influxdb.ID) error {
// userID is ignored here since server reads it off
// the token/auth. its a nothing burger here
var resp notificationEndpointDecoder
err := s.Client.
PostJSON(&notificationEndpointEncoder{ne: ne}, prefixNotificationEndpoints).
Expand All @@ -640,7 +641,9 @@ func (s *NotificationEndpointService) CreateNotificationEndpoint(ctx context.Con
// UpdateNotificationEndpoint updates a single notification endpoint.
// Returns the new notification endpoint after update.
func (s *NotificationEndpointService) UpdateNotificationEndpoint(ctx context.Context, id influxdb.ID, ne influxdb.NotificationEndpoint, userID influxdb.ID) (influxdb.NotificationEndpoint, error) {
// userID is ignored since userID is grabbed off the http auth set on the client
if !id.Valid() {
return nil, fmt.Errorf("invalid ID: please provide a valid ID")
}
var resp notificationEndpointDecoder
err := s.Client.
PutJSON(&notificationEndpointEncoder{ne: ne}, prefixNotificationEndpoints, id.String()).
Expand All @@ -655,6 +658,9 @@ func (s *NotificationEndpointService) UpdateNotificationEndpoint(ctx context.Con
// PatchNotificationEndpoint updates a single notification endpoint with changeset.
// Returns the new notification endpoint state after update.
func (s *NotificationEndpointService) PatchNotificationEndpoint(ctx context.Context, id influxdb.ID, upd influxdb.NotificationEndpointUpdate) (influxdb.NotificationEndpoint, error) {
if !id.Valid() {
return nil, fmt.Errorf("invalid ID: please provide a valid ID")
}
if err := upd.Valid(); err != nil {
return nil, err
}
Expand All @@ -676,6 +682,9 @@ func (s *NotificationEndpointService) PatchNotificationEndpoint(ctx context.Cont
// then see what falls out :flushed... for now returning nothing for secrets, orgID, and only returning an error. This makes
// the code/design smell super obvious imo
func (s *NotificationEndpointService) DeleteNotificationEndpoint(ctx context.Context, id influxdb.ID) ([]influxdb.SecretField, influxdb.ID, error) {
if !id.Valid() {
return nil, 0, fmt.Errorf("invalid ID: please provide a valid ID")
}
err := s.Client.
Delete(prefixNotificationEndpoints, id.String()).
Do(ctx)
Expand Down

0 comments on commit 4e22b0b

Please sign in to comment.