Skip to content

Commit

Permalink
MF-527 - Fix group rights in Remove service methods
Browse files Browse the repository at this point in the history
  • Loading branch information
majabirmancevic committed Oct 1, 2024
1 parent 6292bd7 commit 2c78380
Show file tree
Hide file tree
Showing 16 changed files with 58 additions and 52 deletions.
2 changes: 1 addition & 1 deletion consumers/notifiers/api/http/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func removeNotifiersEndpoint(svc notifiers.Service) endpoint.Endpoint {
return nil, err
}

if err := svc.RemoveNotifiers(ctx, req.token, req.groupID, req.NotifierIDs...); err != nil {
if err := svc.RemoveNotifiers(ctx, req.token, req.NotifierIDs...); err != nil {
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion consumers/notifiers/api/http/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ func runRemoveNotifiersTest(t *testing.T, validContacts []string) {
req := testRequest{
client: ts.Client(),
method: http.MethodPatch,
url: fmt.Sprintf("%s/groups/%s/notifiers", ts.URL, groupID),
url: fmt.Sprintf("%s/notifiers", ts.URL),
token: tc.auth,
contentType: tc.contentType,
body: strings.NewReader(body),
Expand Down
14 changes: 5 additions & 9 deletions consumers/notifiers/api/http/requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
)

const (
minLen = 1
maxLimitSize = 100
maxNameSize = 254
nameOrder = "name"
Expand Down Expand Up @@ -89,7 +90,7 @@ func (req createNotifiersReq) validate() error {
return apiutil.ErrMissingGroupID
}

if len(req.Notifiers) == 0 {
if len(req.Notifiers) < minLen {
return apiutil.ErrEmptyList
}

Expand All @@ -107,7 +108,7 @@ func (req createNotifierReq) validate() error {
return apiutil.ErrNameSize
}

if len(req.Contacts) == 0 {
if len(req.Contacts) < minLen {
return apiutil.ErrEmptyList
}

Expand Down Expand Up @@ -135,15 +136,14 @@ func (req updateNotifierReq) validate() error {
return apiutil.ErrNameSize
}

if len(req.Contacts) == 0 {
if len(req.Contacts) < minLen {
return apiutil.ErrEmptyList
}

return nil
}

type removeNotifiersReq struct {
groupID string
token string
NotifierIDs []string `json:"notifier_ids,omitempty"`
}
Expand All @@ -153,11 +153,7 @@ func (req removeNotifiersReq) validate() error {
return apiutil.ErrBearerToken
}

if req.groupID == "" {
return apiutil.ErrMissingGroupID
}

if len(req.NotifierIDs) == 0 {
if len(req.NotifierIDs) < minLen {
return apiutil.ErrEmptyList
}

Expand Down
5 changes: 2 additions & 3 deletions consumers/notifiers/api/http/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func MakeHandler(tracer opentracing.Tracer, svc notifiers.Service, logger log.Lo
encodeResponse,
opts...,
))
r.Patch("/groups/:id/notifiers", kithttp.NewServer(
r.Patch("/notifiers", kithttp.NewServer(
kitot.TraceServer(tracer, "remove_notifiers")(removeNotifiersEndpoint(svc)),
decodeRemoveNotifiers,
encodeResponse,
Expand Down Expand Up @@ -155,8 +155,7 @@ func decodeRemoveNotifiers(_ context.Context, r *http.Request) (interface{}, err
}

req := removeNotifiersReq{
token: apiutil.ExtractBearerToken(r),
groupID: bone.GetValue(r, idKey),
token: apiutil.ExtractBearerToken(r),
}

if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions consumers/notifiers/api/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (lm *loggingMiddleware) UpdateNotifier(ctx context.Context, token string, n
return lm.svc.UpdateNotifier(ctx, token, notifier)
}

func (lm *loggingMiddleware) RemoveNotifiers(ctx context.Context, token, groupID string, id ...string) (err error) {
func (lm *loggingMiddleware) RemoveNotifiers(ctx context.Context, token string, id ...string) (err error) {
defer func(begin time.Time) {
message := fmt.Sprintf("Method remove_notifiers took %s to complete", time.Since(begin))
if err != nil {
Expand All @@ -89,7 +89,7 @@ func (lm *loggingMiddleware) RemoveNotifiers(ctx context.Context, token, groupID
lm.logger.Info(fmt.Sprintf("%s without errors.", message))
}(time.Now())

return lm.svc.RemoveNotifiers(ctx, token, groupID, id...)
return lm.svc.RemoveNotifiers(ctx, token, id...)
}

func (lm *loggingMiddleware) Consume(msg interface{}) (err error) {
Expand Down
4 changes: 2 additions & 2 deletions consumers/notifiers/api/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,13 @@ func (ms *metricsMiddleware) UpdateNotifier(ctx context.Context, token string, n
return ms.svc.UpdateNotifier(ctx, token, notifier)
}

func (ms *metricsMiddleware) RemoveNotifiers(ctx context.Context, token, groupID string, id ...string) error {
func (ms *metricsMiddleware) RemoveNotifiers(ctx context.Context, token string, id ...string) error {
defer func(begin time.Time) {
ms.counter.With("method", "remove_notifiers").Add(1)
ms.latency.With("method", "remove_notifiers").Observe(time.Since(begin).Seconds())
}(time.Now())

return ms.svc.RemoveNotifiers(ctx, token, groupID, id...)
return ms.svc.RemoveNotifiers(ctx, token, id...)
}

func (ms *metricsMiddleware) Consume(msg interface{}) error {
Expand Down
14 changes: 10 additions & 4 deletions consumers/notifiers/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type Service interface {

// RemoveNotifiers removes the notifiers identified with the provided IDs, that
// belongs to the user identified by the provided key.
RemoveNotifiers(ctx context.Context, token, groupID string, id ...string) error
RemoveNotifiers(ctx context.Context, token string, id ...string) error

consumers.Consumer
}
Expand Down Expand Up @@ -174,9 +174,15 @@ func (ns *notifierService) UpdateNotifier(ctx context.Context, token string, not
return ns.notifierRepo.Update(ctx, notifier)
}

func (ns *notifierService) RemoveNotifiers(ctx context.Context, token, groupID string, ids ...string) error {
if _, err := ns.things.Authorize(ctx, &protomfx.AuthorizeReq{Token: token, Object: groupID, Subject: things.GroupSub, Action: things.Editor}); err != nil {
return errors.Wrap(errors.ErrAuthorization, err)
func (ns *notifierService) RemoveNotifiers(ctx context.Context, token string, ids ...string) error {
for _, id := range ids {
notifier, err := ns.notifierRepo.RetrieveByID(ctx, id)
if err != nil {
return err
}
if _, err := ns.things.Authorize(ctx, &protomfx.AuthorizeReq{Token: token, Object: notifier.GroupID, Subject: things.GroupSub, Action: things.Editor}); err != nil {
return errors.Wrap(errors.ErrAuthorization, err)
}
}

if err := ns.notifierRepo.Remove(ctx, ids...); err != nil {
Expand Down
14 changes: 7 additions & 7 deletions consumers/notifiers/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,12 @@ func runRemoveNotifiersTest(t *testing.T, validContacts []string) {
token string
err error
}{
{
desc: "remove notifier with wrong credentials",
id: nf.ID,
token: wrongValue,
err: errors.ErrAuthentication,
},
{
desc: "remove existing notifier",
id: nf.ID,
Expand All @@ -458,16 +464,10 @@ func runRemoveNotifiersTest(t *testing.T, validContacts []string) {
token: token,
err: errors.ErrNotFound,
},
{
desc: "remove notifier with wrong credentials",
id: nf.ID,
token: wrongValue,
err: errors.ErrAuthentication,
},
}

for _, tc := range cases {
err := svc.RemoveNotifiers(context.Background(), tc.token, groupID, tc.id)
err := svc.RemoveNotifiers(context.Background(), tc.token, tc.id)
assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %s got %s\n", tc.desc, tc.err, err))
}
}
2 changes: 1 addition & 1 deletion webhooks/api/http/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func removeWebhooksEndpoint(svc webhooks.Service) endpoint.Endpoint {
return nil, err
}

if err := svc.RemoveWebhooks(ctx, req.token, req.groupID, req.WebhookIDs...); err != nil {
if err := svc.RemoveWebhooks(ctx, req.token, req.WebhookIDs...); err != nil {
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion webhooks/api/http/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ func TestRemoveWebhooks(t *testing.T) {
req := testRequest{
client: ts.Client(),
method: http.MethodPatch,
url: fmt.Sprintf("%s/groups/%s/webhooks", ts.URL, groupID),
url: fmt.Sprintf("%s/webhooks", ts.URL),
token: tc.auth,
contentType: tc.contentType,
body: strings.NewReader(body),
Expand Down
6 changes: 3 additions & 3 deletions webhooks/api/http/requests.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
)

const (
minLen = 1
maxLimitSize = 100
maxNameSize = 254
nameOrder = "name"
Expand Down Expand Up @@ -49,7 +50,7 @@ func (req createWebhooksReq) validate() error {
return apiutil.ErrMissingGroupID
}

if len(req.Webhooks) <= 0 {
if len(req.Webhooks) < minLen {
return apiutil.ErrEmptyList
}

Expand Down Expand Up @@ -153,7 +154,6 @@ func (req updateWebhookReq) validate() error {
}

type removeWebhooksReq struct {
groupID string
token string
WebhookIDs []string `json:"webhook_ids,omitempty"`
}
Expand All @@ -163,7 +163,7 @@ func (req removeWebhooksReq) validate() error {
return apiutil.ErrBearerToken
}

if len(req.WebhookIDs) < 1 {
if len(req.WebhookIDs) < minLen {
return apiutil.ErrEmptyList
}

Expand Down
5 changes: 2 additions & 3 deletions webhooks/api/http/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func MakeHandler(tracer opentracing.Tracer, svc webhooks.Service, logger log.Log
encodeResponse,
opts...,
))
r.Patch("/groups/:id/webhooks", kithttp.NewServer(
r.Patch("/webhooks", kithttp.NewServer(
kitot.TraceServer(tracer, "remove_webhooks")(removeWebhooksEndpoint(svc)),
decodeRemoveWebhooks,
encodeResponse,
Expand Down Expand Up @@ -153,8 +153,7 @@ func decodeRemoveWebhooks(_ context.Context, r *http.Request) (interface{}, erro
}

req := removeWebhooksReq{
token: apiutil.ExtractBearerToken(r),
groupID: bone.GetValue(r, idKey),
token: apiutil.ExtractBearerToken(r),
}

if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions webhooks/api/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (lm *loggingMiddleware) UpdateWebhook(ctx context.Context, token string, we
return lm.svc.UpdateWebhook(ctx, token, webhook)
}

func (lm *loggingMiddleware) RemoveWebhooks(ctx context.Context, token, groupID string, id ...string) (err error) {
func (lm *loggingMiddleware) RemoveWebhooks(ctx context.Context, token string, id ...string) (err error) {
defer func(begin time.Time) {
message := fmt.Sprintf("Method remove_webhooks took %s to complete", time.Since(begin))
if err != nil {
Expand All @@ -89,7 +89,7 @@ func (lm *loggingMiddleware) RemoveWebhooks(ctx context.Context, token, groupID
lm.logger.Info(fmt.Sprintf("%s without errors.", message))
}(time.Now())

return lm.svc.RemoveWebhooks(ctx, token, groupID, id...)
return lm.svc.RemoveWebhooks(ctx, token, id...)
}

func (lm *loggingMiddleware) Consume(message interface{}) (err error) {
Expand Down
4 changes: 2 additions & 2 deletions webhooks/api/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,13 @@ func (ms *metricsMiddleware) UpdateWebhook(ctx context.Context, token string, we
return ms.svc.UpdateWebhook(ctx, token, webhook)
}

func (ms *metricsMiddleware) RemoveWebhooks(ctx context.Context, token, groupID string, id ...string) error {
func (ms *metricsMiddleware) RemoveWebhooks(ctx context.Context, token string, id ...string) error {
defer func(begin time.Time) {
ms.counter.With("method", "remove_webhooks").Add(1)
ms.latency.With("method", "remove_webhooks").Observe(time.Since(begin).Seconds())
}(time.Now())

return ms.svc.RemoveWebhooks(ctx, token, groupID, id...)
return ms.svc.RemoveWebhooks(ctx, token, id...)
}

func (ms *metricsMiddleware) Consume(message interface{}) error {
Expand Down
14 changes: 10 additions & 4 deletions webhooks/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type Service interface {

// RemoveWebhooks removes the webhooks identified with the provided IDs, that
// belongs to the user identified by the provided key.
RemoveWebhooks(ctx context.Context, token, groupID string, id ...string) error
RemoveWebhooks(ctx context.Context, token string, id ...string) error

consumers.Consumer
}
Expand Down Expand Up @@ -150,9 +150,15 @@ func (ws *webhooksService) UpdateWebhook(ctx context.Context, token string, webh
return ws.webhooks.Update(ctx, webhook)
}

func (ws *webhooksService) RemoveWebhooks(ctx context.Context, token, groupID string, ids ...string) error {
if _, err := ws.things.Authorize(ctx, &protomfx.AuthorizeReq{Token: token, Object: groupID, Subject: things.GroupSub, Action: things.Editor}); err != nil {
return errors.Wrap(errors.ErrAuthorization, err)
func (ws *webhooksService) RemoveWebhooks(ctx context.Context, token string, ids ...string) error {
for _, id := range ids {
webhook, err := ws.webhooks.RetrieveByID(ctx, id)
if err != nil {
return err
}
if _, err := ws.things.Authorize(ctx, &protomfx.AuthorizeReq{Token: token, Object: webhook.GroupID, Subject: things.GroupSub, Action: things.Editor}); err != nil {
return errors.Wrap(errors.ErrAuthorization, err)
}
}

if err := ws.webhooks.Remove(ctx, ids...); err != nil {
Expand Down
14 changes: 7 additions & 7 deletions webhooks/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,12 @@ func TestRemoveWebhooks(t *testing.T) {
token string
err error
}{
{
desc: "remove webhook with wrong credentials",
id: wh.ID,
token: wrongValue,
err: errors.ErrAuthentication,
},
{
desc: "remove existing webhook",
id: wh.ID,
Expand All @@ -330,16 +336,10 @@ func TestRemoveWebhooks(t *testing.T) {
token: token,
err: errors.ErrNotFound,
},
{
desc: "remove webhook with wrong credentials",
id: wh.ID,
token: wrongValue,
err: errors.ErrAuthentication,
},
}

for _, tc := range cases {
err := svc.RemoveWebhooks(context.Background(), tc.token, groupID, tc.id)
err := svc.RemoveWebhooks(context.Background(), tc.token, tc.id)
assert.True(t, errors.Contains(err, tc.err), fmt.Sprintf("%s: expected %s got %s\n", tc.desc, tc.err, err))
}
}
Expand Down

0 comments on commit 2c78380

Please sign in to comment.