From 8c2fe9452f42d3502a421d1a33fa704b95f40194 Mon Sep 17 00:00:00 2001 From: shlomo Date: Tue, 31 Jan 2023 18:36:18 +0200 Subject: [PATCH] fix: optional read only and write only validations (#689) --- .github/docs/openapi3.txt | 2 + openapi3/issue689_test.go | 107 ++++++++++++++++ openapi3/schema.go | 4 +- openapi3/schema_validation_settings.go | 22 +++- openapi3filter/issue689_test.go | 168 +++++++++++++++++++++++++ openapi3filter/options.go | 6 + openapi3filter/validate_request.go | 5 +- openapi3filter/validate_response.go | 5 +- 8 files changed, 310 insertions(+), 9 deletions(-) create mode 100644 openapi3/issue689_test.go create mode 100644 openapi3filter/issue689_test.go diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt index 28819d73a..3ce7ed959 100644 --- a/.github/docs/openapi3.txt +++ b/.github/docs/openapi3.txt @@ -113,6 +113,8 @@ type SchemaRefs []*SchemaRef type SchemaValidationOption func(*schemaValidationSettings) func DefaultsSet(f func()) SchemaValidationOption func DisablePatternValidation() SchemaValidationOption + func DisableReadOnlyValidation() SchemaValidationOption + func DisableWriteOnlyValidation() SchemaValidationOption func EnableFormatValidation() SchemaValidationOption func FailFast() SchemaValidationOption func MultiErrors() SchemaValidationOption diff --git a/openapi3/issue689_test.go b/openapi3/issue689_test.go new file mode 100644 index 000000000..cafbadfac --- /dev/null +++ b/openapi3/issue689_test.go @@ -0,0 +1,107 @@ +package openapi3_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" +) + +func TestIssue689(t *testing.T) { + t.Parallel() + + tests := [...]struct { + name string + schema *openapi3.Schema + value map[string]interface{} + opts []openapi3.SchemaValidationOption + checkErr require.ErrorAssertionFunc + }{ + // read-only + { + name: "read-only property succeeds when read-only validation is disabled", + schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{ + "foo": {Type: "boolean", ReadOnly: true}}), + value: map[string]interface{}{"foo": true}, + opts: []openapi3.SchemaValidationOption{ + openapi3.VisitAsRequest(), + openapi3.DisableReadOnlyValidation()}, + checkErr: require.NoError, + }, + { + name: "non read-only property succeeds when read-only validation is disabled", + schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{ + "foo": {Type: "boolean", ReadOnly: false}}), + opts: []openapi3.SchemaValidationOption{ + openapi3.VisitAsRequest()}, + value: map[string]interface{}{"foo": true}, + checkErr: require.NoError, + }, + { + name: "read-only property fails when read-only validation is enabled", + schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{ + "foo": {Type: "boolean", ReadOnly: true}}), + opts: []openapi3.SchemaValidationOption{ + openapi3.VisitAsRequest()}, + value: map[string]interface{}{"foo": true}, + checkErr: require.Error, + }, + { + name: "non read-only property succeeds when read-only validation is enabled", + schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{ + "foo": {Type: "boolean", ReadOnly: false}}), + opts: []openapi3.SchemaValidationOption{ + openapi3.VisitAsRequest()}, + value: map[string]interface{}{"foo": true}, + checkErr: require.NoError, + }, + // write-only + { + name: "write-only property succeeds when write-only validation is disabled", + schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{ + "foo": {Type: "boolean", WriteOnly: true}}), + value: map[string]interface{}{"foo": true}, + opts: []openapi3.SchemaValidationOption{ + openapi3.VisitAsResponse(), + openapi3.DisableWriteOnlyValidation()}, + checkErr: require.NoError, + }, + { + name: "non write-only property succeeds when write-only validation is disabled", + schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{ + "foo": {Type: "boolean", WriteOnly: false}}), + opts: []openapi3.SchemaValidationOption{ + openapi3.VisitAsResponse()}, + value: map[string]interface{}{"foo": true}, + checkErr: require.NoError, + }, + { + name: "write-only property fails when write-only validation is enabled", + schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{ + "foo": {Type: "boolean", WriteOnly: true}}), + opts: []openapi3.SchemaValidationOption{ + openapi3.VisitAsResponse()}, + value: map[string]interface{}{"foo": true}, + checkErr: require.Error, + }, + { + name: "non write-only property succeeds when write-only validation is enabled", + schema: openapi3.NewSchema().WithProperties(map[string]*openapi3.Schema{ + "foo": {Type: "boolean", WriteOnly: false}}), + opts: []openapi3.SchemaValidationOption{ + openapi3.VisitAsResponse()}, + value: map[string]interface{}{"foo": true}, + checkErr: require.NoError, + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + err := test.schema.VisitJSON(test.value, test.opts...) + test.checkErr(t, err) + }) + } +} diff --git a/openapi3/schema.go b/openapi3/schema.go index 4e2bf1419..5f867ff44 100644 --- a/openapi3/schema.go +++ b/openapi3/schema.go @@ -1787,8 +1787,8 @@ func (schema *Schema) visitJSONObject(settings *schemaValidationSettings, value sort.Strings(properties) for _, propName := range properties { propSchema := schema.Properties[propName] - reqRO := settings.asreq && propSchema.Value.ReadOnly - repWO := settings.asrep && propSchema.Value.WriteOnly + reqRO := settings.asreq && propSchema.Value.ReadOnly && !settings.readOnlyValidationDisabled + repWO := settings.asrep && propSchema.Value.WriteOnly && !settings.writeOnlyValidationDisabled if value[propName] == nil { if dlft := propSchema.Value.Default; dlft != nil && !reqRO && !repWO { diff --git a/openapi3/schema_validation_settings.go b/openapi3/schema_validation_settings.go index 5a28c8d8d..17aad2fa7 100644 --- a/openapi3/schema_validation_settings.go +++ b/openapi3/schema_validation_settings.go @@ -8,11 +8,13 @@ import ( type SchemaValidationOption func(*schemaValidationSettings) type schemaValidationSettings struct { - failfast bool - multiError bool - asreq, asrep bool // exclusive (XOR) fields - formatValidationEnabled bool - patternValidationDisabled bool + failfast bool + multiError bool + asreq, asrep bool // exclusive (XOR) fields + formatValidationEnabled bool + patternValidationDisabled bool + readOnlyValidationDisabled bool + writeOnlyValidationDisabled bool onceSettingDefaults sync.Once defaultsSet func() @@ -47,6 +49,16 @@ func DisablePatternValidation() SchemaValidationOption { return func(s *schemaValidationSettings) { s.patternValidationDisabled = true } } +// DisableReadOnlyValidation setting makes Validate not return an error when validating properties marked as read-only +func DisableReadOnlyValidation() SchemaValidationOption { + return func(s *schemaValidationSettings) { s.readOnlyValidationDisabled = true } +} + +// DisableWriteOnlyValidation setting makes Validate not return an error when validating properties marked as write-only +func DisableWriteOnlyValidation() SchemaValidationOption { + return func(s *schemaValidationSettings) { s.writeOnlyValidationDisabled = true } +} + // DefaultsSet executes the given callback (once) IFF schema validation set default values. func DefaultsSet(f func()) SchemaValidationOption { return func(s *schemaValidationSettings) { s.defaultsSet = f } diff --git a/openapi3filter/issue689_test.go b/openapi3filter/issue689_test.go new file mode 100644 index 000000000..592d53f74 --- /dev/null +++ b/openapi3filter/issue689_test.go @@ -0,0 +1,168 @@ +package openapi3filter + +import ( + "io" + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" + "github.com/getkin/kin-openapi/routers/gorillamux" +) + +func TestIssue689(t *testing.T) { + loader := openapi3.NewLoader() + ctx := loader.Context + spec := ` + openapi: 3.0.0 + info: + version: 1.0.0 + title: Sample API + paths: + /items: + put: + requestBody: + content: + application/json: + schema: + properties: + testWithReadOnly: + readOnly: true + type: boolean + testNoReadOnly: + type: boolean + type: object + responses: + '200': + description: OK + get: + responses: + '200': + description: OK + content: + application/json: + schema: + properties: + testWithWriteOnly: + writeOnly: true + type: boolean + testNoWriteOnly: + type: boolean +`[1:] + + doc, err := loader.LoadFromData([]byte(spec)) + require.NoError(t, err) + + err = doc.Validate(ctx) + require.NoError(t, err) + + router, err := gorillamux.NewRouter(doc) + require.NoError(t, err) + + tests := []struct { + name string + options *Options + body string + method string + checkErr require.ErrorAssertionFunc + }{ + // read-only + { + name: "non read-only property is added to request when validation enabled", + body: `{"testNoReadOnly": true}`, + method: http.MethodPut, + checkErr: require.NoError, + }, + { + name: "non read-only property is added to request when validation disabled", + body: `{"testNoReadOnly": true}`, + method: http.MethodPut, + options: &Options{ + ExcludeReadOnlyValidations: true, + }, + checkErr: require.NoError, + }, + { + name: "read-only property is added to requests when validation enabled", + body: `{"testWithReadOnly": true}`, + method: http.MethodPut, + checkErr: require.Error, + }, + { + name: "read-only property is added to requests when validation disabled", + body: `{"testWithReadOnly": true}`, + method: http.MethodPut, + options: &Options{ + ExcludeReadOnlyValidations: true, + }, + checkErr: require.NoError, + }, + // write-only + { + name: "non write-only property is added to request when validation enabled", + body: `{"testNoWriteOnly": true}`, + method: http.MethodGet, + checkErr: require.NoError, + }, + { + name: "non write-only property is added to request when validation disabled", + body: `{"testNoWriteOnly": true}`, + method: http.MethodGet, + options: &Options{ + ExcludeWriteOnlyValidations: true, + }, + checkErr: require.NoError, + }, + { + name: "write-only property is added to requests when validation enabled", + body: `{"testWithWriteOnly": true}`, + method: http.MethodGet, + checkErr: require.Error, + }, + { + name: "write-only property is added to requests when validation disabled", + body: `{"testWithWriteOnly": true}`, + method: http.MethodGet, + options: &Options{ + ExcludeWriteOnlyValidations: true, + }, + checkErr: require.NoError, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + httpReq, err := http.NewRequest(test.method, "/items", strings.NewReader(test.body)) + require.NoError(t, err) + httpReq.Header.Set("Content-Type", "application/json") + require.NoError(t, err) + + route, pathParams, err := router.FindRoute(httpReq) + require.NoError(t, err) + + requestValidationInput := &RequestValidationInput{ + Request: httpReq, + PathParams: pathParams, + Route: route, + Options: test.options, + } + + if test.method == http.MethodGet { + responseValidationInput := &ResponseValidationInput{ + RequestValidationInput: requestValidationInput, + Status: 200, + Header: httpReq.Header, + Body: io.NopCloser(strings.NewReader(test.body)), + Options: test.options, + } + err = ValidateResponse(ctx, responseValidationInput) + + } else { + err = ValidateRequest(ctx, requestValidationInput) + } + test.checkErr(t, err) + }) + } +} diff --git a/openapi3filter/options.go b/openapi3filter/options.go index 14c35d5da..4ea9e9907 100644 --- a/openapi3filter/options.go +++ b/openapi3filter/options.go @@ -15,6 +15,12 @@ type Options struct { // Set ExcludeResponseBody so ValidateResponse skips response body validation ExcludeResponseBody bool + // Set ExcludeReadOnlyValidations so ValidateRequest skips read-only validations + ExcludeReadOnlyValidations bool + + // Set ExcludeWriteOnlyValidations so ValidateResponse skips write-only validations + ExcludeWriteOnlyValidations bool + // Set IncludeResponseStatus so ValidateResponse fails on response // status not defined in OpenAPI spec IncludeResponseStatus bool diff --git a/openapi3filter/validate_request.go b/openapi3filter/validate_request.go index 7245cbe03..a8106a7c8 100644 --- a/openapi3filter/validate_request.go +++ b/openapi3filter/validate_request.go @@ -272,7 +272,7 @@ func ValidateRequestBody(ctx context.Context, input *RequestValidationInput, req } defaultsSet := false - opts := make([]openapi3.SchemaValidationOption, 0, 3) // 3 potential opts here + opts := make([]openapi3.SchemaValidationOption, 0, 4) // 4 potential opts here opts = append(opts, openapi3.VisitAsRequest()) if !options.SkipSettingDefaults { opts = append(opts, openapi3.DefaultsSet(func() { defaultsSet = true })) @@ -283,6 +283,9 @@ func ValidateRequestBody(ctx context.Context, input *RequestValidationInput, req if options.customSchemaErrorFunc != nil { opts = append(opts, openapi3.SetSchemaErrorMessageCustomizer(options.customSchemaErrorFunc)) } + if options.ExcludeReadOnlyValidations { + opts = append(opts, openapi3.DisableReadOnlyValidation()) + } // Validate JSON with the schema if err := contentType.Schema.Value.VisitJSON(value, opts...); err != nil { diff --git a/openapi3filter/validate_response.go b/openapi3filter/validate_response.go index c1be31928..08ea4e19d 100644 --- a/openapi3filter/validate_response.go +++ b/openapi3filter/validate_response.go @@ -63,13 +63,16 @@ func ValidateResponse(ctx context.Context, input *ResponseValidationInput) error return &ResponseError{Input: input, Reason: "response has not been resolved"} } - opts := make([]openapi3.SchemaValidationOption, 0, 2) + opts := make([]openapi3.SchemaValidationOption, 0, 3) // 3 potential options here if options.MultiError { opts = append(opts, openapi3.MultiErrors()) } if options.customSchemaErrorFunc != nil { opts = append(opts, openapi3.SetSchemaErrorMessageCustomizer(options.customSchemaErrorFunc)) } + if options.ExcludeWriteOnlyValidations { + opts = append(opts, openapi3.DisableWriteOnlyValidation()) + } headers := make([]string, 0, len(response.Headers)) for k := range response.Headers {