From 42577fb30244b05b02ce29d6cbb5ec3a561d045c Mon Sep 17 00:00:00 2001 From: zepatrik Date: Wed, 15 Sep 2021 18:13:05 +0200 Subject: [PATCH 1/3] fix: swagger enum definition for patchAction --- internal/httpclient/models/patch_action.go | 27 ----------- internal/httpclient/models/patch_delta.go | 56 ++++++++++++++-------- internal/relationtuple/definitions.go | 2 +- spec/api.json | 9 ++-- 4 files changed, 41 insertions(+), 53 deletions(-) delete mode 100644 internal/httpclient/models/patch_action.go diff --git a/internal/httpclient/models/patch_action.go b/internal/httpclient/models/patch_action.go deleted file mode 100644 index 63a593869..000000000 --- a/internal/httpclient/models/patch_action.go +++ /dev/null @@ -1,27 +0,0 @@ -// Code generated by go-swagger; DO NOT EDIT. - -package models - -// This file was generated by the swagger tool. -// Editing this file might prove futile when you re-run the swagger generate command - -import ( - "context" - - "github.com/go-openapi/strfmt" -) - -// PatchAction patch action -// -// swagger:model patchAction -type PatchAction string - -// Validate validates this patch action -func (m PatchAction) Validate(formats strfmt.Registry) error { - return nil -} - -// ContextValidate validates this patch action based on context it is used -func (m PatchAction) ContextValidate(ctx context.Context, formats strfmt.Registry) error { - return nil -} diff --git a/internal/httpclient/models/patch_delta.go b/internal/httpclient/models/patch_delta.go index 597724711..2a02a7efb 100644 --- a/internal/httpclient/models/patch_delta.go +++ b/internal/httpclient/models/patch_delta.go @@ -7,10 +7,12 @@ package models import ( "context" + "encoding/json" "github.com/go-openapi/errors" "github.com/go-openapi/strfmt" "github.com/go-openapi/swag" + "github.com/go-openapi/validate" ) // PatchDelta patch delta @@ -19,7 +21,8 @@ import ( type PatchDelta struct { // action - Action PatchAction `json:"action,omitempty"` + // Enum: [insert delete] + Action string `json:"action,omitempty"` // relation tuple RelationTuple *InternalRelationTuple `json:"relation_tuple,omitempty"` @@ -43,15 +46,42 @@ func (m *PatchDelta) Validate(formats strfmt.Registry) error { return nil } +var patchDeltaTypeActionPropEnum []interface{} + +func init() { + var res []string + if err := json.Unmarshal([]byte(`["insert","delete"]`), &res); err != nil { + panic(err) + } + for _, v := range res { + patchDeltaTypeActionPropEnum = append(patchDeltaTypeActionPropEnum, v) + } +} + +const ( + + // PatchDeltaActionInsert captures enum value "insert" + PatchDeltaActionInsert string = "insert" + + // PatchDeltaActionDelete captures enum value "delete" + PatchDeltaActionDelete string = "delete" +) + +// prop value enum +func (m *PatchDelta) validateActionEnum(path, location string, value string) error { + if err := validate.EnumCase(path, location, value, patchDeltaTypeActionPropEnum, true); err != nil { + return err + } + return nil +} + func (m *PatchDelta) validateAction(formats strfmt.Registry) error { if swag.IsZero(m.Action) { // not required return nil } - if err := m.Action.Validate(formats); err != nil { - if ve, ok := err.(*errors.Validation); ok { - return ve.ValidateName("action") - } + // value enum + if err := m.validateActionEnum("action", "body", m.Action); err != nil { return err } @@ -79,10 +109,6 @@ func (m *PatchDelta) validateRelationTuple(formats strfmt.Registry) error { func (m *PatchDelta) ContextValidate(ctx context.Context, formats strfmt.Registry) error { var res []error - if err := m.contextValidateAction(ctx, formats); err != nil { - res = append(res, err) - } - if err := m.contextValidateRelationTuple(ctx, formats); err != nil { res = append(res, err) } @@ -93,18 +119,6 @@ func (m *PatchDelta) ContextValidate(ctx context.Context, formats strfmt.Registr return nil } -func (m *PatchDelta) contextValidateAction(ctx context.Context, formats strfmt.Registry) error { - - if err := m.Action.ContextValidate(ctx, formats); err != nil { - if ve, ok := err.(*errors.Validation); ok { - return ve.ValidateName("action") - } - return err - } - - return nil -} - func (m *PatchDelta) contextValidateRelationTuple(ctx context.Context, formats strfmt.Registry) error { if m.RelationTuple != nil { diff --git a/internal/relationtuple/definitions.go b/internal/relationtuple/definitions.go index 7d8120fd0..ceffc410b 100644 --- a/internal/relationtuple/definitions.go +++ b/internal/relationtuple/definitions.go @@ -135,7 +135,7 @@ var ( ErrNilSubject = errors.New("subject is nil") ) -// swagger:enum +// swagger:enum patchAction type patchAction string const ( diff --git a/spec/api.json b/spec/api.json index aff342fa1..b47be3bd9 100755 --- a/spec/api.json +++ b/spec/api.json @@ -999,7 +999,11 @@ "type": "object", "properties": { "action": { - "$ref": "#/definitions/patchAction" + "type": "string", + "enum": [ + "insert", + "delete" + ] }, "relation_tuple": { "$ref": "#/definitions/InternalRelationTuple" @@ -1083,9 +1087,6 @@ } } }, - "patchAction": { - "type": "string" - }, "subject": { "type": "string" }, From 2b4a73518033f1e828559de7a09ff72fb252c929 Mon Sep 17 00:00:00 2001 From: zepatrik Date: Wed, 15 Sep 2021 18:24:01 +0200 Subject: [PATCH 2/3] test: add cases for relationtuple patch with only one tuple --- .../relationtuple/transact_server_test.go | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/internal/relationtuple/transact_server_test.go b/internal/relationtuple/transact_server_test.go index a2a98310d..c6acb0675 100644 --- a/internal/relationtuple/transact_server_test.go +++ b/internal/relationtuple/transact_server_test.go @@ -245,5 +245,65 @@ func TestWriteHandlers(t *testing.T) { require.NoError(t, err) assert.Len(t, actualRTs, 0) }) + + t.Run("case=only create", func(t *testing.T) { + nspace := addNamespace(t) + + deltas := []*relationtuple.PatchDelta{ + { + Action: relationtuple.ActionInsert, + RelationTuple: &relationtuple.InternalRelationTuple{ + Namespace: nspace.Name, + Object: "create obj", + Relation: "rel", + Subject: &relationtuple.SubjectID{ID: "create sub"}, + }, + }, + } + + body, err := json.Marshal(deltas) + require.NoError(t, err) + req, err := http.NewRequest(http.MethodPatch, ts.URL+relationtuple.RouteBase, bytes.NewBuffer(body)) + require.NoError(t, err) + resp, err := ts.Client().Do(req) + require.NoError(t, err) + assert.Equal(t, http.StatusNoContent, resp.StatusCode) + + actualRTs, _, err := reg.RelationTupleManager().GetRelationTuples(context.Background(), &relationtuple.RelationQuery{ + Namespace: nspace.Name, + }) + require.NoError(t, err) + assert.Equal(t, []*relationtuple.InternalRelationTuple{deltas[0].RelationTuple}, actualRTs) + }) + + t.Run("case=only delete", func(t *testing.T) { + nspace := addNamespace(t) + + deltas := []*relationtuple.PatchDelta{ + { + Action: relationtuple.ActionDelete, + RelationTuple: &relationtuple.InternalRelationTuple{ + Namespace: nspace.Name, + Object: "delete obj", + Relation: "rel", + Subject: &relationtuple.SubjectID{ID: "delete sub"}, + }, + }, + } + + body, err := json.Marshal(deltas) + require.NoError(t, err) + req, err := http.NewRequest(http.MethodPatch, ts.URL+relationtuple.RouteBase, bytes.NewBuffer(body)) + require.NoError(t, err) + resp, err := ts.Client().Do(req) + require.NoError(t, err) + assert.Equal(t, http.StatusNoContent, resp.StatusCode) + + actualRTs, _, err := reg.RelationTupleManager().GetRelationTuples(context.Background(), &relationtuple.RelationQuery{ + Namespace: nspace.Name, + }) + require.NoError(t, err) + assert.Equal(t, []*relationtuple.InternalRelationTuple{}, actualRTs) + }) }) } From 2131e2c7bcc750e5158189a9d4da113f986ea040 Mon Sep 17 00:00:00 2001 From: zepatrik Date: Thu, 16 Sep 2021 11:19:31 +0200 Subject: [PATCH 3/3] fix: add more input validation to relationtuple patch handler --- internal/relationtuple/transact_server.go | 12 +++++ .../relationtuple/transact_server_test.go | 48 +++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/internal/relationtuple/transact_server.go b/internal/relationtuple/transact_server.go index 1011b0a16..b9c30ee78 100644 --- a/internal/relationtuple/transact_server.go +++ b/internal/relationtuple/transact_server.go @@ -166,6 +166,18 @@ func (h *handler) patchRelations(w http.ResponseWriter, r *http.Request, _ httpr h.d.Writer().WriteError(w, r, herodot.ErrBadRequest.WithError(err.Error())) return } + for _, d := range deltas { + if d.RelationTuple == nil { + h.d.Writer().WriteError(w, r, herodot.ErrBadRequest.WithError("relation_tuple is missing")) + return + } + switch d.Action { + case ActionDelete, ActionInsert: + default: + h.d.Writer().WriteError(w, r, herodot.ErrBadRequest.WithError("unknown action "+string(d.Action))) + return + } + } if err := h.d.RelationTupleManager().TransactRelationTuples(r.Context(), internalTuplesWithAction(deltas, ActionInsert), internalTuplesWithAction(deltas, ActionDelete)); err != nil { h.d.Writer().WriteError(w, r, err) diff --git a/internal/relationtuple/transact_server_test.go b/internal/relationtuple/transact_server_test.go index c6acb0675..113e6cfb5 100644 --- a/internal/relationtuple/transact_server_test.go +++ b/internal/relationtuple/transact_server_test.go @@ -305,5 +305,53 @@ func TestWriteHandlers(t *testing.T) { require.NoError(t, err) assert.Equal(t, []*relationtuple.InternalRelationTuple{}, actualRTs) }) + + t.Run("case=valid JSON, invalid content", func(t *testing.T) { + rawJSON := ` +[ + { + "action": "insert", + "namespace":"role", + "object":"super-admin", + "relation":"member", + "subject":"role:company-admin" + } +]` + req, err := http.NewRequest(http.MethodPatch, ts.URL+relationtuple.RouteBase, bytes.NewBufferString(rawJSON)) + require.NoError(t, err) + resp, err := ts.Client().Do(req) + require.NoError(t, err) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + + defer resp.Body.Close() + errContent, err := io.ReadAll(resp.Body) + require.NoError(t, err) + assert.Contains(t, string(errContent), "relation_tuple is missing") + }) + + t.Run("case=unknown action", func(t *testing.T) { + rawJSON := ` +[ + { + "action": "unknown_action_foo", + "relation_tuple": { + "namespace":"role", + "object":"super-admin", + "relation":"member", + "subject":"role:company-admin" + } + } +]` + req, err := http.NewRequest(http.MethodPatch, ts.URL+relationtuple.RouteBase, bytes.NewBufferString(rawJSON)) + require.NoError(t, err) + resp, err := ts.Client().Do(req) + require.NoError(t, err) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + + defer resp.Body.Close() + errContent, err := io.ReadAll(resp.Body) + require.NoError(t, err) + assert.Contains(t, string(errContent), "unknown_action_foo") + }) }) }