Response validation

[ronkorving]

Hi,

First of all, amazing library! This is exactly what I was looking for all this time, giving total control, yet offering very useful features for OpenAPI 3. Just needed to say that, this-is-great :)

I was wondering about HTTP response validation though. Do you think there is a reasonable approach to validate HTTP responses against the OpenAPI definition? Perhaps too much overhead in production, but would be very useful during development and while running a test suite.

Would love to hear your thoughts. Thanks!

[anttiviljami]

Yes, we sure could add Ajv validation for output, but I would disable it by default. I've had output validation enabled using other libraries but found that it's not actually desireable to throw errors for API users because the API response doesn't conform to the spec.

How would you like to use this feature in your tests? Do you have a proposal?

[TanukiSharp]

We could use this feature during tests in order to centralize the source of truth on the OpenAPI spec, and free the user from writing the output validation logic related to the schema in his/her (all) unit tests.

I was also thinking about having a way to keep output validation even in production for some operations, especially on the body, in order to ensure that security critical values that are not supposed to be disclosed, are not disclosed unintentionally because of a potential bug or else. This may only concern highly security constrained environments, but why not having a mechanism that support all scenarios.

To this end, I was think about having an output validation handler in the openapi-backend constructor options instead of a flag, so if the handler is unset (null or undefined) then it would mean false (never perform output validation), having the handler returning true would be equivalent to have a flag set to true (always perform output validation), but this approach could allow much more granular decision taking. For example to validate output of only a subset of APIs, like the critical ones.

What do you think about that ? Thanks for your input.

[anttiviljami]

If it's a handler, wouldn't it make sense to register it with .register() like you do with other handlers like validationFail or notFound?

In the constructor, we should instruct OpenAPIBackend whether to prepare Ajv validations for responses too. Right now there's https://github.com/anttiviljami/openapi-backend/blob/master/DOCS.md#parameter-optsvalidate for input validation. Perhaps opts.validateResponses boolean ?

Let's say we also add a new handler, say responseValidationFail, which when registered can be used to check any operations with the prepared Ajv validation scheme and similarly to validationFail adds the validation errors to the context object.

The validator (and OpenAPIBackend) will also get a new method similar to .validateRequest(req). Something like .validateResponse(res, operationId)

Would this work? I'm still unsure how this should be used in the test suite.

[TanukiSharp]

About the handler I'm talking about, sorry if the term confused you, it is not a handler in openapi-backend term but a predicate function, acting like a "dynamic flag".

To speak with your terms, opts.validateResponses would not be a boolean, but what I called the handler, so a more appropriate term should be a predicate function to decide whether to apply output validation or not, with a finer granularity.

So more something like opts.validateResponsePredicate and then:

• Never perform output validation:

opts.validateResponsePredicate = null;

or

opts.validateResponsePredicate = () => false;

• Always perform output validation:

opts.validateResponsePredicate = () => true;

• Perform selective output validation: (pseudocode)

opts.validateResponsePredicate = (req, res) => {
    if (req.path.startsWith('/credentials/') || !!res.headers['x-security-critical']) {
        return true;
    }
    return false;
};

So this process only decides what to validate, then as you said comes a responseValidationFail handler for those where the predicate returned true and the Ajv validation failed. And yes this one has to be registered like the others via .register in order to keep it consistent with your way of doing things. I like it.

we should instruct OpenAPIBackend whether to prepare Ajv validations for responses too

I agree, and I think that's the only drawback of the predicate approach, because having the predicate function defined does not necessarily mean output validation has to be performed, for example in the case the predicate is () => false;, so maybe having an additional flag that overrides the whole feature ? But still there may be an inconsistency having the flag set to true and the predicate always returning false. When one wants to shoot itself in the foot...

Not sure what is the best approach, but I definitely think having a predicate to filter which output we want to validate or not, is a good thing, and doing it via a JS function is by far the simplest way. A flag gives too few control, and we certainly don't want to implement a rules engine (format, parser, runtime, unit tests, etc...)

About the .validateResponse(res, operationId) function, yes I'm thinking about having this function too, and much like .validateRequest(req), exposing it for convenience but still having it called internally based on the flag and the predicate function returned value.

Let me know if what I say is still not clear, or if you don't like the predicate approach ^^

Thanks.

[anttiviljami]

I'm wondering if the predicate should be applied during runtime (to decide whether to use a pre-built schema to validate), or during initalization (to decide whether to build the schema for an operation in the first place). I'm leaning towards the first option. Then we could have a separate flag to instruct OpenAPI backend to skip the schema building for responses entirely.

[TanukiSharp]

Do you mean applying the predicate to all operations at the initialization, and building a map of operations on which to apply the output validation ?
That's a great idea. Indeed it's pointless to keep applying the predicate on something that is predictable at init time and will never change at runtime.

Do you plan on implementing it, or do you want me to make a pull request ?

[anttiviljami]

Do you mean applying the predicate to all operations at the initialization, and building a map of operations on which to apply the output validation ?

Yes. That's current how the input validation works. We pre-compile the schemas during init and then use them during runtime. If no schema was built, there's no validation work to be done.

I think the response validation should probably work that way too.

However, right now there's no great way to disable the input validation for a single operation. All you can do is decide not to care about the validation result in the validationFail handler and pass the request on to the operation handler if you really need to.

I think using predicate functions like this is a great idea for exactly that task! Let's add predicate function opts for both input and response validation so users can skip validation steps during runtime. The schemas should be still be compiled for each operation, I think.

I'm currently working on some other minor changes related to input validation. I'll push a minor bump probably today.

If it's ok to you, I'll do the initial version and have you review the feature? I just feel like things are still in flux related to validation and I don't want you to do unnecessary work while big API changes happen. :)

[TanukiSharp]

Sure that perfectly makes sense you kick start the thing if you are performing other changes as well related with this.

For output validation, originally I was thinking about applying the predicate for each request because we may want to prevent the response to reach the net also based on values in the body, but that starts to get a bit too hairy, and anyway kind of out of OpenAPI scope.

Nonetheless, I was still thinking on building the validation function only once (per operation) at startup.

So basically that would mean, instead of creating a map of validation functions for all operations, you could add to the map only the function for which the predicate said true, and then at runtime considering operations not in the map is not a mistake but means a validation skip.

Looking forward to your update anyway, thanks for your time and your good work :)

[anttiviljami]

Hi @TanukiSharp & @ronkorving ! I've now implemented this feature (without docs) in this PR: #12

Wanna check it out?

Couple of changes to my initial plan:

1. I realised we can't do response validation in OpenAPI.handleRequest because the return value of the request handler is not uniform between frameworks. Some don't in fact even include the response value as part of their return value, but instead manipulate some framework-specific interface.
2. To solve problem 1 I've added a new handler called postResponseHandler, which gets passed the return value of the operation handler as part of the context object context.response, as well as the pre-built response validator function context.responseValidator. You can use either validator.validateResponse or context.responseValidator to validate your response in the postResponseHandler. (or in fact even in the operation handler if you like!)

Here's an example of how I imagine you might use this:

function postResponseHandler(c) {
  const valid = api.validator.validateResponse(c.response, c.operation.operationId);
  // alternatively, 
  const valid = c.responseValidator(c.response);
  if (valid.errors) {
    // return some error message here
   return { err: 'validation fail', errors: valid.errors };
  } else {
    return c.response;
  }
}
api.register({ postResponseHandler });

See the commit message for more info or ask me if something feels weird.

[anttiviljami]

This feature is now live in openapi-backend@1.6.0 Check out the example here: https://github.com/anttiviljami/openapi-backend#response-validation