PR for no-dependency cookie support?

[pcafstockf]

Thank you for your work. I'm a huge fan of OpenAPI and love this project.

I understand you desire to not introduce dependencies on additional plugins.

Would you consider a PR with a few small changes that would make it trivial for folks to add cookie validation themselves?

There would be a little more to the PR, but essentially it would boil down to this:

const cookies = [];
...
case "cookie": {
    if (this.options.cookies) {
        item.style = item.style || "form";
        cookies.push(item);
    }
    else
        console.warn("cookie parameters are not supported by Fastify");
    break;
}
...
if (cookies.length > 0) {
    schema.cookies = this.parseParams(cookies);
}

[seriousme]

Hi,

thanks for asking!

As you probably noticed fastify validation does not provide any option to validate cookies (see: https://fastify.dev/docs/latest/Reference/Validation-and-Serialization/#validation), hence I explicitly excluded cookie parameters and issue a warning to avoid people thinking it was supported somehow.

I can see your desire to expose such schema so that you can build your own parsing logic on top of it. On the other hand the specification of fastify's schema parameter is not under my control. So if I add something to it now that can come back later to haunt me in future versions of fastify.

What we could do is add something like a "cookieSchema" to the route, which could then be picked up by the handler or other middleware by those who choose to do so.

I also like to keep the console.warn in for those folks that do not have their own custom cookie validation logic.

Kind regards, Hans

[pcafstockf]

Hi,

Thanks for your response. I read your comment about submitting an issue before a PR, and so was going for brevity. Maybe a little to much 😄

Here is more detail. My goal was to not impact existing users, so I was implying that a new (optional) boolean argument be added to fastify-openapi-glue options called 'cookie' (or whatever).
If truthy, it would trigger the code above, otherwise the existing warning would still fire (but this keeps the logs clean for folks who want to use that ability).

The proposal (that I have working) is exactly as you suggested... Adding a 'cookieSchema' to the route, in a Fastify approved way.

This would literally be the entire user-land code, which could (if folks desired) even be a little plugin.

// Register the fastify-cookie plugin (or any other) for cookie parsing
fastify.register(cookie);

// Hook into the route registration process to compile cookie schemas
fastify.addHook('onRoute', (routeOptions: any) => {
  const schema = routeOptions.schema;
  // schema.cookies would be added by the modifications at the top of this PR
  if (schema?.cookies) {
    // Compile the cookie schema once and store it in the route's context
    routeOptions.config = routeOptions.config || {};
    routeOptions.config.cookieValidator = ajv.compile(schema.cookies);
  }
});

// Pre-handler hook to validate cookies using the precompiled schema
fastify.addHook('preHandler', async (request: FastifyRequest, reply: FastifyReply) => {
  // FastifyRequestContext confirms this is the supported way of doing this.
  const cookieValidator = (request.routeOptions.config as any).cookieValidator as ValidateFunction;
  if (cookieValidator) {
    const valid = cookieValidator(request.cookies);
    if (!valid) {
      reply.status(400).send({error: 'Invalid cookies', details: cookieValidator.errors});
      throw new Error('Invalid cookies');
    }
  }
});

All in all, a simple and compatible way for folks to support validating input according to an OpenAPI spec, all in a Fastify compatible way, with no impact on existing users of fastify-openapi-glue.

[seriousme]

Hi,

sounds ok to me! As you mentioned, if we add an "addCookieSchema" option to the plugin it should help people like you and not bother folks that don't implement cookie parsing.

It shouldn be too much work to implement this, although adding tests, typscript defs, docs etc always takes a bit more time. Will you submit a PR or should I add it myself ?

Kind regards, Hans

[pcafstockf]

I have working code and am happy to do a PR with tests, defs, etc. If its something you feel strongly about doing yourself, thats ok. Otherwise I'll plan on submitting a PR in the next week or two. Was thinking about including that little plugin snippet above in the docs as a "if you want to support cookie validation, you will need to do this part yourself" kind of thing?

[seriousme]

PR in a week or two sounds good to me.

Thanks ! Hans

[seriousme]

Just released as v4.7.0 on NPM

[pcafstockf]

Installed and confirmed. Thanks Hans!

[github-actions[bot]]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days'