Validation doesn't account for ingress/egress direction and doesn't respect writeOnly/readOnly in conjunction with required

[tstirrat15]

I'd like to use this library to validate requests and responses that have readOnly and writeOnly flags in addition to required flags. Currently, the code is aware of and respects the required flag, but it isn't aware of readOnly or writeOnly.

This is correct as it stands; the spec uses a MAY around the handling of readOnly and writeOnly. I think this package would be more useful and powerful if it respected them.

I'm planning on putting up a PR that:

• Collects readOnly and writeOnly flags as a part of interpretation
• Adds a notion of direction (ingress vs egress) to validation
• Uses those flags in validation of required by omitting readOnly fields from ingress validation and writeOnly fields from egress validation

[tstirrat15]

It does appear that v3.0 and v3.1 differ on their interpretation of these keywords, at least according to this section; I'm having trouble understanding what the difference is compared to the v3 version of the same.

[tstirrat15]

This will depend on swaggest/openapi-go#145.

[vearutop]

Could you make an example on how these tags (readOnly, writeOnly) would work in practice? E.g. a sample structure(s), endpoint(s) that use it and expected validation behavior.

[tstirrat15]

Example

type ExampleObject struct {
	Foo string `json:"foo" readOnly:"true"`
	Bar string `json:"bar" writeOnly:"true"`
}

type ExampleResponse struct {
	ExampleObject `json:"example"`
}

type ExampleRequest struct {
	ExampleObject `json:"example"`
}

reflector := openapi3.Reflector{}
reflector.Spec = &openapi3.Spec{Openapi: openAPIVersion}
op, _ := reflector.NewOperationContext("POST", "/some/path")

op.AddReqStructure(ExampleRequest{})
op.AddRespStructure(ExampleResponse{})
reflector.AddOperation(op)

Behavior

NOTE: this is specifically implementing v3.1 behavior; if we also want to implement v3.0.3 behavior I can add an additional flag in the Validator that changes the behavior when a value is provided to a readOnly required field on ingress.

• field is readOnly:
  • field is required:
    • input value is provided: PASS (makes PUTs nicer, per comment in v3.1 spec)
    • input value is not provided: PASS
    • output value is provided: PASS
    • output value is not provided: fail
  • field is not required
    • input value is provided: fail (a readOnly value should not be provided)
    • input value is not provided: PASS
    • output value is provided: PASS
    • output value is not provided: PASS
• field is writeOnly:
  • field is required:
    • input value is provided: PASS
    • input value is not provided: fail
    • output value is provided: fail
    • output value is not provided: PASS
  • field is not required:
    • input value is provided: PASS
    • input value is not provided: PASS
    • output value is provided: fail
    • output value is not provided: PASS

I can translate the above into a table or into terms of the specific structs if that would help.

Fields that are only marked as readOnly will continue to work as-is.

[tstirrat15]

And I find that the v3 version of the spec provides the clearest guidance on what the behaviors should be: https://swagger.io/specification/v3/#fixed-fields-20

With the caveat that the required + readOnly case is the one that differs.

[vearutop]

Ok, makes sense, please proceed with implementation if you'd like. I think we can stick to v3.1 behavior, let's skip v3.0 quirk until someone actually needs it.

[tstirrat15]

I'm gonna have a go at it, but I hadn't realized that the recursive validation was handled by the jsonschema library, which doesn't care about readOnly and writeOnly (and jsonschema's readOnly and writeOnly behavior differs from openapi's).

[vearutop]

swaggest/rest/openapi.Collector uses openapi.Reflector to

	WalkRequestJSONSchemas(method string, cu ContentUnit, cb JSONSchemaCallback, done func(oc OperationContext)) error
	WalkResponseJSONSchemas(cu ContentUnit, cb JSONSchemaCallback, done func(oc OperationContext)) error

when sourcing JSON schemas for validation.

I think we can implement read/write behavior in swaggest/openapi-go to change exposed JSON schema according to read/write constraints (by removing or adding properties to required).

Then exported JSON schema will have no mentions of readOnly/writeOnly, but instead would be a simple "pre-rendered" version either for requests or responses.

This might be also challenging to implement, but probably easier than introducing concepts of read/write to a general JSON schema validator.

[tstirrat15]

I'm not opposed to that approach in principle. I don't think I'll have time to get to this soon, though - my work priorities just shifted and we changed our openapi schema to drop usage of readOnly and writeOnly.