From f2efa515e77235f8d4bb7b6ec6c5c38421a368ab Mon Sep 17 00:00:00 2001 From: Dmitrii Prisacari Date: Wed, 20 Mar 2024 10:21:27 +0000 Subject: [PATCH 1/2] fix: validate schema ID in the provided resource --- handlers_test.go | 25 ++-- schema/schema.go | 29 ++++ schema/schema_test.go | 233 ++++++++++++++++++++----------- schema/testdata/schema_test.json | 2 +- 4 files changed, 191 insertions(+), 98 deletions(-) diff --git a/handlers_test.go b/handlers_test.go index 9d7e6c2..2be2313 100644 --- a/handlers_test.go +++ b/handlers_test.go @@ -433,32 +433,32 @@ func TestServerResourcePostHandlerValid(t *testing.T) { tests := []struct { name string target string - body io.Reader + body string expectedUserName string expectedExternalID interface{} }{ { name: "Users post request without version", target: "/Users", - body: strings.NewReader(`{"id": "other", "userName": "test1", "externalId": "external_test1"}`), + body: `{"id": "other", "userName": "test1", "externalId": "external_test1","schemas":["urn:ietf:params:scim:schemas:core:2.0:User"]}`, expectedUserName: "test1", expectedExternalID: "external_test1", }, { name: "Users post request with version", target: "/v2/Users", - body: strings.NewReader(`{"id": "other", "userName": "test2", "externalId": "external_test2"}`), + body: `{"id": "other", "userName": "test2", "externalId": "external_test2","schemas":["urn:ietf:params:scim:schemas:core:2.0:User"]}`, expectedUserName: "test2", expectedExternalID: "external_test2", }, { name: "Users post request without externalId", target: "/v2/Users", - body: strings.NewReader(`{"id": "other", "userName": "test3"}`), + body: `{"id": "other", "userName": "test3","schemas":["urn:ietf:params:scim:schemas:core:2.0:User"]}`, expectedUserName: "test3", expectedExternalID: nil, }, { name: "Users post request with immutable attribute", target: "/v2/Users", - body: strings.NewReader(`{"id": "other", "userName": "test3", "immutableThing": "test"}`), + body: `{"id": "other", "userName": "test3", "immutableThing": "test","schemas":["urn:ietf:params:scim:schemas:core:2.0:User"]}`, expectedUserName: "test3", expectedExternalID: nil, }, @@ -466,7 +466,7 @@ func TestServerResourcePostHandlerValid(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - req := httptest.NewRequest(http.MethodPost, test.target, test.body) + req := httptest.NewRequest(http.MethodPost, test.target, strings.NewReader(test.body)) rr := httptest.NewRecorder() newTestServer(t).ServeHTTP(rr, req) @@ -496,7 +496,8 @@ func TestServerResourcePostHandlerValid(t *testing.T) { } func TestServerResourcePutHandlerNotFound(t *testing.T) { - req := httptest.NewRequest(http.MethodPut, "/Users/9999", strings.NewReader(`{"userName": "other"}`)) + reqBody := `{"userName": "other","schemas":["urn:ietf:params:scim:schemas:core:2.0:User"]}` + req := httptest.NewRequest(http.MethodPut, "/Users/9999", strings.NewReader(reqBody)) rr := httptest.NewRecorder() newTestServer(t).ServeHTTP(rr, req) @@ -520,26 +521,26 @@ func TestServerResourcePutHandlerValid(t *testing.T) { tests := []struct { name string target string - body io.Reader + body string expectedUserName string expectedExternalID interface{} }{ { name: "Users put request", target: "/v2/Users/0002", - body: strings.NewReader(`{"id": "other", "userName": "test2", "externalId": "external_test2"}`), + body: `{"id": "other", "userName": "test2", "externalId": "external_test2","schemas":["urn:ietf:params:scim:schemas:core:2.0:User"]}`, expectedUserName: "test2", expectedExternalID: "external_test2", }, { name: "Users put request without externalId", target: "/Users/0003", - body: strings.NewReader(`{"id": "other", "userName": "test3"}`), + body: `{"id": "other", "userName": "test3","schemas":["urn:ietf:params:scim:schemas:core:2.0:User"]}`, expectedUserName: "test3", expectedExternalID: nil, }, { name: "Users put request with immutable attribute", target: "/Users/0003", - body: strings.NewReader(`{"id": "other", "userName": "test3", "immutableThing": "test"}`), + body: `{"id": "other", "userName": "test3", "immutableThing": "test","schemas":["urn:ietf:params:scim:schemas:core:2.0:User"]}`, expectedUserName: "test3", expectedExternalID: nil, }, @@ -547,7 +548,7 @@ func TestServerResourcePutHandlerValid(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - req := httptest.NewRequest(http.MethodPut, test.target, test.body) + req := httptest.NewRequest(http.MethodPut, test.target, strings.NewReader(test.body)) rr := httptest.NewRecorder() newTestServer(t).ServeHTTP(rr, req) diff --git a/schema/schema.go b/schema/schema.go index 6556633..2698dd5 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -127,12 +127,41 @@ func (s Schema) getRawAttributes() []map[string]interface{} { return attributes } +func (s Schema) validateSchemaID(resource map[string]interface{}) *errors.ScimError { + resourceSchemas, present := resource["schemas"] + if !present { + return &errors.ScimErrorInvalidSyntax + } + + resourceSchemasSlice, ok := resourceSchemas.([]interface{}) + if !ok { + return &errors.ScimErrorInvalidSyntax + } + + var schemaFound bool + for _, v := range resourceSchemasSlice { + if v == s.ID { + schemaFound = true + break + } + } + if !schemaFound { + return &errors.ScimErrorInvalidSyntax + } + + return nil +} + func (s Schema) validate(resource interface{}, checkMutability bool) (map[string]interface{}, *errors.ScimError) { core, ok := resource.(map[string]interface{}) if !ok { return nil, &errors.ScimErrorInvalidSyntax } + if err := s.validateSchemaID(core); err != nil { + return nil, err + } + attributes := make(map[string]interface{}) for _, attribute := range s.Attributes { var hit interface{} diff --git a/schema/schema_test.go b/schema/schema_test.go index c9faacc..3ae25f9 100644 --- a/schema/schema_test.go +++ b/schema/schema_test.go @@ -9,7 +9,7 @@ import ( ) var testSchema = Schema{ - ID: "empty", + ID: "test-schema-id", Name: optional.NewString("test"), Description: optional.String{}, Attributes: []CoreAttribute{ @@ -93,7 +93,8 @@ func TestJSONMarshalling(t *testing.T) { return } if normalizedActual != normalizedExpected { - t.Errorf("schema output by MarshalJSON did not match the expected output. want %s, got %s", normalizedExpected, normalizedActual) + t.Errorf("schema output by MarshalJSON did not match the expected output."+ + "\nWant: %s\nGot: %s", normalizedExpected, normalizedActual) } } @@ -107,6 +108,7 @@ func TestResourceInvalid(t *testing.T) { func TestValidValidation(t *testing.T) { for _, test := range []map[string]interface{}{ { + "schemas": []interface{}{"test-schema-id"}, "required": "present", "booleans": []interface{}{ true, @@ -131,121 +133,182 @@ func TestValidValidation(t *testing.T) { } func TestValidationInvalid(t *testing.T) { - for _, test := range []map[string]interface{}{ - { // missing required field - "field": "present", - "booleans": []interface{}{ - true, + tests := []struct { + name string + resource map[string]interface{} + }{ + { + name: "missing required field", + resource: map[string]interface{}{ + "schemas": []interface{}{"test-schema-id"}, + "field": "present", + "booleans": []interface{}{true}, }, }, - { // missing required multivalued field - "required": "present", - "booleans": []interface{}{}, - }, - { // wrong type element of slice - "required": "present", - "booleans": []interface{}{ - "present", + { + name: "missing required multivalued field", + resource: map[string]interface{}{ + "schemas": []interface{}{"test-schema-id"}, + "required": "present", + "booleans": []interface{}{}, }, }, - { // duplicate names - "required": "present", - "Required": "present", - "booleans": []interface{}{ - true, + { + name: "wrong type element of slice", + resource: map[string]interface{}{ + "schemas": []interface{}{"test-schema-id"}, + "required": "present", + "booleans": []interface{}{"present"}, }, }, - { // wrong string type - "required": true, - "booleans": []interface{}{ - true, + { + name: "duplicate names", + resource: map[string]interface{}{ + "schemas": []interface{}{"test-schema-id"}, + "required": "present", + "Required": "present", + "booleans": []interface{}{true}, }, }, - { // wrong complex type - "required": "present", - "complex": "present", - "booleans": []interface{}{ - true, + { + name: "wrong string type", + resource: map[string]interface{}{ + "schemas": []interface{}{"test-schema-id"}, + "required": true, + "booleans": []interface{}{true}, }, }, - { // wrong complex element type - "required": "present", - "booleans": []interface{}{ - true, + { + name: "wrong complex type", + resource: map[string]interface{}{ + "schemas": []interface{}{"test-schema-id"}, + "required": "present", + "complex": "present", + "booleans": []interface{}{true}, }, - "complex": []interface{}{ - "present", + }, + { + name: "wrong complex element type", + resource: map[string]interface{}{ + "schemas": []interface{}{"test-schema-id"}, + "required": "present", + "booleans": []interface{}{true}, + "complex": []interface{}{ + "present", + }, }, }, - { // duplicate complex element names - "required": "present", - "booleans": []interface{}{ - true, + { + name: "duplicate complex element names", + resource: map[string]interface{}{ + "schemas": []interface{}{"test-schema-id"}, + "required": "present", + "booleans": []interface{}{true}, + "complex": []interface{}{ + map[string]interface{}{ + "sub": "present", + "Sub": "present", + }, + }, }, - "complex": []interface{}{ - map[string]interface{}{ - "sub": "present", - "Sub": "present", + }, + { + name: "wrong type complex element", + resource: map[string]interface{}{ + "schemas": []interface{}{"test-schema-id"}, + "required": "present", + "booleans": []interface{}{true}, + "complex": []interface{}{ + map[string]interface{}{ + "sub": true, + }, }, }, }, - { // wrong type complex element - "required": "present", - "booleans": []interface{}{ - true, + { + name: "invalid type binary", + resource: map[string]interface{}{ + "schemas": []interface{}{"test-schema-id"}, + "required": "present", + "booleans": []interface{}{true}, + "binary": true, }, - "complex": []interface{}{ - map[string]interface{}{ - "sub": true, - }, + }, + { + name: "invalid type dateTime", + resource: map[string]interface{}{ + "schemas": []interface{}{"test-schema-id"}, + "required": "present", + "booleans": []interface{}{true}, + "dateTime": "04:56:22Z2008-01-23T", }, }, - { // invalid type binary - "required": "present", - "booleans": []interface{}{ - true, + { + name: "invalid type integer", + resource: map[string]interface{}{ + "schemas": []interface{}{"test-schema-id"}, + "required": "present", + "booleans": []interface{}{true}, + "integer": 1.1, }, - "binary": true, }, - { // invalid type dateTime - "required": "present", - "booleans": []interface{}{ - true, + { + name: "invalid type decimal", + resource: map[string]interface{}{ + "schemas": []interface{}{"test-schema-id"}, + "required": "present", + "booleans": []interface{}{true}, + "decimal": "1.1", }, - "dateTime": "04:56:22Z2008-01-23T", }, - { // invalid type integer - "required": "present", - "booleans": []interface{}{ - true, + { + name: "invalid type integer (json.Number)", + resource: map[string]interface{}{ + "schemas": []interface{}{"test-schema-id"}, + "required": "present", + "booleans": []interface{}{true}, + "integerNumber": json.Number("1.1"), }, - "integer": 1.1, }, - { // invalid type decimal - "required": "present", - "booleans": []interface{}{ - true, + { + name: "invalid type decimal (json.Number)", + resource: map[string]interface{}{ + "schemas": []interface{}{"test-schema-id"}, + "required": "present", + "booleans": []interface{}{true}, + "decimalNumber": json.Number("fail"), }, - "decimal": "1.1", }, - { // invalid type integer (json.Number) - "required": "present", - "booleans": []interface{}{ - true, + { + name: "missing 'schemas' attribute", + resource: map[string]interface{}{ + "required": "present", + "booleans": []interface{}{true}, }, - "integerNumber": json.Number("1.1"), }, - { // invalid type decimal (json.Number) - "required": "present", - "booleans": []interface{}{ - true, + { + name: "'schemas' attribute is not an array of strings", + resource: map[string]interface{}{ + "schemas": "test-schema-id", + "required": "present", + "booleans": []interface{}{true}, }, - "decimalNumber": json.Number("fail"), }, - } { - if _, scimErr := testSchema.Validate(test); scimErr == nil { - t.Errorf("invalid resource expected") - } + { + name: "wrong 'schemas' name", + resource: map[string]interface{}{ + "schemas": []interface{}{"wrong_schema_name"}, + "required": "present", + "booleans": []interface{}{true}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, scimErr := testSchema.Validate(tt.resource); scimErr == nil { + t.Errorf("invalid resource expected") + } + }) } } diff --git a/schema/testdata/schema_test.json b/schema/testdata/schema_test.json index bbb68db..3e2de56 100644 --- a/schema/testdata/schema_test.json +++ b/schema/testdata/schema_test.json @@ -122,6 +122,6 @@ ], "schemas": ["urn:ietf:params:scim:schemas:core:2.0:Schema"], "description": "", - "id": "empty", + "id": "test-schema-id", "name": "test" } From b3524fdb2bd84567d5493428dc7b055f6fd8b0fd Mon Sep 17 00:00:00 2001 From: Dmitrii Prisacari Date: Wed, 20 Mar 2024 12:20:44 +0000 Subject: [PATCH 2/2] refactor: rearrange code to fix linting issues --- schema/schema.go | 50 ++++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/schema/schema.go b/schema/schema.go index 2698dd5..d5b58ac 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -127,31 +127,6 @@ func (s Schema) getRawAttributes() []map[string]interface{} { return attributes } -func (s Schema) validateSchemaID(resource map[string]interface{}) *errors.ScimError { - resourceSchemas, present := resource["schemas"] - if !present { - return &errors.ScimErrorInvalidSyntax - } - - resourceSchemasSlice, ok := resourceSchemas.([]interface{}) - if !ok { - return &errors.ScimErrorInvalidSyntax - } - - var schemaFound bool - for _, v := range resourceSchemasSlice { - if v == s.ID { - schemaFound = true - break - } - } - if !schemaFound { - return &errors.ScimErrorInvalidSyntax - } - - return nil -} - func (s Schema) validate(resource interface{}, checkMutability bool) (map[string]interface{}, *errors.ScimError) { core, ok := resource.(map[string]interface{}) if !ok { @@ -193,3 +168,28 @@ func (s Schema) validate(resource interface{}, checkMutability bool) (map[string } return attributes, nil } + +func (s Schema) validateSchemaID(resource map[string]interface{}) *errors.ScimError { + resourceSchemas, present := resource["schemas"] + if !present { + return &errors.ScimErrorInvalidSyntax + } + + resourceSchemasSlice, ok := resourceSchemas.([]interface{}) + if !ok { + return &errors.ScimErrorInvalidSyntax + } + + var schemaFound bool + for _, v := range resourceSchemasSlice { + if v == s.ID { + schemaFound = true + break + } + } + if !schemaFound { + return &errors.ScimErrorInvalidSyntax + } + + return nil +}