Skip to content

Unwrap scim errors when possible for patch validation #187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,25 @@ func TestServerResourcePatchHandlerInvalidRemoveOp(t *testing.T) {
assertEqualStatusCode(t, http.StatusBadRequest, rr.Code)
}

func TestServerResourcePatchHandlerInvalidRemoveOpNoTarget(t *testing.T) {
req := httptest.NewRequest(http.MethodPatch, "/Groups/0001", strings.NewReader(`{
"schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
"Operations":[
{
"op":"remove",
"path":""
}
]
}`))
rr := httptest.NewRecorder()
newTestServer(t).ServeHTTP(rr, req)

assertEqualStatusCode(t, http.StatusBadRequest, rr.Code)
var scimErr *errors.ScimError
assertUnmarshalNoError(t, json.Unmarshal(rr.Body.Bytes(), &scimErr))
assertEqualSCIMErrors(t, &errors.ScimErrorNoTarget, scimErr)
}

func TestServerResourcePatchHandlerMapTypeSubAttribute(t *testing.T) {
recorder := httptest.NewRecorder()
newTestServer(t).ServeHTTP(recorder, httptest.NewRequest(http.MethodPatch, "/Users/0001", strings.NewReader(`{
Expand Down
6 changes: 1 addition & 5 deletions internal/patch/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package patch

import (
"github.com/elimity-com/scim/filter"
"net/http"

"github.com/elimity-com/scim/errors"
"github.com/elimity-com/scim/schema"
Expand All @@ -13,10 +12,7 @@ import (
func (v OperationValidator) validateRemove() (interface{}, error) {
// If "path" is unspecified, the operation fails with HTTP status code 400 and a "scimType" error code of "noTarget".
if v.Path == nil {
return nil, &errors.ScimError{
ScimType: errors.ScimTypeNoTarget,
Status: http.StatusBadRequest,
}
return nil, &errors.ScimErrorNoTarget
}

refAttr, err := v.getRefAttribute(v.Path.AttributePath)
Expand Down
27 changes: 16 additions & 11 deletions resource_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ package scim
import (
"bytes"
"encoding/json"
"errors"
"net/http"

"github.com/elimity-com/scim/errors"
scimErrors "github.com/elimity-com/scim/errors"
"github.com/elimity-com/scim/internal/patch"
"github.com/elimity-com/scim/optional"
"github.com/elimity-com/scim/schema"
Expand Down Expand Up @@ -86,10 +87,10 @@ func (t ResourceType) schemaWithCommon() schema.Schema {
return s
}

func (t ResourceType) validate(raw []byte) (ResourceAttributes, *errors.ScimError) {
func (t ResourceType) validate(raw []byte) (ResourceAttributes, *scimErrors.ScimError) {
var m map[string]interface{}
if err := unmarshal(raw, &m); err != nil {
return ResourceAttributes{}, &errors.ScimErrorInvalidSyntax
return ResourceAttributes{}, &scimErrors.ScimErrorInvalidSyntax
}

attributes, scimErr := t.schemaWithCommon().Validate(m)
Expand All @@ -101,7 +102,7 @@ func (t ResourceType) validate(raw []byte) (ResourceAttributes, *errors.ScimErro
extensionField := m[extension.Schema.ID]
if extensionField == nil {
if extension.Required {
return ResourceAttributes{}, &errors.ScimErrorInvalidValue
return ResourceAttributes{}, &scimErrors.ScimErrorInvalidValue
}
continue
}
Expand All @@ -118,30 +119,30 @@ func (t ResourceType) validate(raw []byte) (ResourceAttributes, *errors.ScimErro
}

// validatePatch parse and validate PATCH request.
func (t ResourceType) validatePatch(r *http.Request) ([]PatchOperation, *errors.ScimError) {
func (t ResourceType) validatePatch(r *http.Request) ([]PatchOperation, *scimErrors.ScimError) {
data, err := readBody(r)
if err != nil {
return nil, &errors.ScimErrorInvalidSyntax
return nil, &scimErrors.ScimErrorInvalidSyntax
}

var req struct {
Schemas []string
Operations []json.RawMessage
}
if err := unmarshal(data, &req); err != nil {
return nil, &errors.ScimErrorInvalidSyntax
return nil, &scimErrors.ScimErrorInvalidSyntax
}

// The body of each request MUST contain the "schemas" attribute with the URI value of
// "urn:ietf:params:scim:api:messages:2.0:PatchOp".
if len(req.Schemas) != 1 || req.Schemas[0] != "urn:ietf:params:scim:api:messages:2.0:PatchOp" {
return nil, &errors.ScimErrorInvalidValue
return nil, &scimErrors.ScimErrorInvalidValue
}

// The body of an HTTP PATCH request MUST contain the attribute "Operations",
// whose value is an array of one or more PATCH operations.
if len(req.Operations) < 1 {
return nil, &errors.ScimErrorInvalidValue
return nil, &scimErrors.ScimErrorInvalidValue
}

// Evaluation continues until all operations are successfully applied or until an error condition is encountered.
Expand All @@ -153,11 +154,15 @@ func (t ResourceType) validatePatch(r *http.Request) ([]PatchOperation, *errors.
t.getSchemaExtensions()...,
)
if err != nil {
return nil, &errors.ScimErrorInvalidPath
return nil, &scimErrors.ScimErrorInvalidPath
}
value, err := validator.Validate()
if err != nil {
return nil, &errors.ScimErrorInvalidValue
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New logic to unwrap the underlying scim error (this does assume/require underlying errors returned are pointers as best practice to work properly, which does seem to be the case).

Please let me know if any other tests should be added as I only noticed the handlers_test.go tests. 🙏🏽

var scimErr *scimErrors.ScimError
if errors.As(err, &scimErr) {
return nil, scimErr
}
return nil, &scimErrors.ScimErrorInvalidValue
}
operations = append(operations, PatchOperation{
Op: string(validator.Op),
Expand Down