feat: add ability to define securitySchemes under components

[viktor-ku]

Hi, there is a small change that I missed when using this package: having to define my securitySchemes (as of v3 of open api)

I hope you too find it useful and it'll get merged shortly.

Any feedback would be appreciated!

Thanks

[AGalabov]

From what I see you're aiming to provide a pure object definition for the components.securitySchemes property, right? If that is the case I see no reason for putting this through the registry and binding it into the generation logic. I'll try and showcase a different approach.

// Note I think this would look better in a class/factory but a function is easier to shwocase. We can just export this as
// some sort of helper function from `zod-to-openapi`.
function generateSecuritySchemes<T extends Record<string, SecuritySchemeObject>>(securitySchemes: T) {
  return {
    securitySchemes,
    addSecurityScheme: (key: keyof T, val: string[] = []) => ({[key]: val}),
  }
}


  const generator = new OpenAPIGenerator(rootRegistry.definitions);

  const Security = generateSecuritySchemes({bearerAuth: {
    type: 'http',
    scheme: 'bearer',
    bearerFormat: 'JWT',
  }})

  registry.registerPath({
    path: '/units',
    method: 'get',
    security: [
      Security.addSecurityScheme('bearerAuth'),
    ],
    responses: {
      200: {
        mediaType: 'application/json',
        schema: Unit.array().openapi({description: 'Array of all units'})
      }
    }
  })

  generator.generateDocument({
    // the generic stuff
    components: {
      securitySchemes: Security.securitySchemes
    }
  });

In order to make this work we would only need to make sure the config is being merged correctly in generateDocument. I feel like it is not wrong to provide support for manually added components (if someone wants to do that).

@viktor-ku @georgyangelov what do you think. I'd also love to here any possible benefits of getting it through the registry, since I wrote this on the fly and haven't tested particular cases.

[viktor-ku]

It's certainly reasonable, however I think that registry could serve a bit more than just a zod schema handler. The registry just sounds like a convenient way to just add stuff to it like security schemas and it would just know what to do with it.

Maybe it's better to rename it from registry.registerSecuritySchema to like registry.addSecuritySchema implying that there is nothing to generate, just add to the registry?

And then I don't believe it's necessary to have additional boilerplate around generateDocument, I believe the registry is therefore capable of handling exactly that: knowing when to add something to the components and then doing it.

[georgyangelov]

@viktor-ku Thank you for taking the time to open a PR, it's much appreciated!

On the topic @AGalabov raised, on one hand, from a user point of view it would be nice to have the same way of registering OpenAPI stuff as with Zod schemas. On the other, something like the following is pretty simple as well (which is not currently possible):

  registry.registerPath({
    path: '/units',
    method: 'get',
    security: [
      bearerAuth: [],
    ],
    responses: { ... }
  })

  generator.generateDocument({
    ...,
    components: {
      securitySchemes: {
        bearerAuth: { type: 'http', scheme: 'bearer', bearerFormat: 'JWT' }
      }
    }
  });

I believe the concern here is purely from a maintenance point of view - if we start supporting one type of (let's call it) raw openapi object, then it'll make sense to support all of them, which I'm not sure we want to have to do, especially if they are special-cased.

So what do you both think about:

1. Make it possible for the registry to support arbitrary openapi objects that go into /components/, not just security schemes. Something like registry.registerComponent('securitySchemes', 'bearerAuth', { ... })
2. Instead of having securitySchemeRefs, responsesRefs, headersRefs, linksRefs, etc. we can make it a single implementation with something like rawComponents

This way the implementation for all the components would be the same and we won't need to support all types separately. As a bonus there could still be registerSecurityScheme which uses a more specific argument type and proxies to registerComponent. It can also provide the .security() function in the return type since that seems useful as well.

Something like this, which is your code, just some comments on the reusable version. We can add both since the registerSecurityScheme function will be very simple:

const registry = new OpenAPIRegistry();

// Will call registry.registerComponent('securitySchemes', 'bearerAuth', { ... })
const bearerAuth = registry.registerSecurityScheme('bearerAuth', {
  type: 'http',
  scheme: 'bearer',
  bearerFormat: 'JWT',
})

registry.registerPath({
  path: '/units',
  method: 'get',
  security: [
    // Same as just specifying `{ [bearerAuth.name]: [] }`
    bearerAuth.security()
  ],
  responses: { ... }
})

So to sum up, my suggestion is to just make the implementation a bit more general so that any raw components are supported, then specialize just the registerSecurityScheme method of the OpenAPIRegistry to use that and provide helpers. The code shouldn't need to change too much from what it is now. @viktor-ku @AGalabov what do you think?

[viktor-ku]

Make it possible for the registry to support arbitrary openapi objects that go into /components/, not just security schemes. Something like registry.registerComponent('securitySchemes', 'bearerAuth', { ... })

Instead of having securitySchemeRefs, responsesRefs, headersRefs, linksRefs, etc. we can make it a single implementation with something like rawComponents

This way the implementation for all the components would be the same and we won't need to support all types separately. As a bonus there could still be registerSecurityScheme which uses a more specific argument type and proxies to registerComponent. It can also provide the .security() function in the return type since that seems useful as well

I like all of this and also the latter code sample.

[georgyangelov]

@viktor-ku Could you make the change in this PR? Otherwise I'll refactor later but I won't have the time this week

[viktor-ku]

@georgyangelov sorry I've moved to trpc now, I won't have time this week either.

[georgyangelov]

I've created a separate PR for the more generic version: #28

Viktor, thank you again for sparking the discussion here