Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: multiple alternative security objects in route schema #817

Closed
wants to merge 4 commits into from

Conversation

beryxz
Copy link

@beryxz beryxz commented Aug 23, 2024

Fixes #816

Update the security property of FastifySchema to allow multiple security objects in a route schema.

Checklist

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 23, 2024

please add some unit tests

@beryxz
Copy link
Author

beryxz commented Aug 23, 2024

Are these unit tests what you had in mind?

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 23, 2024

To be honest, I dont see any code change in the lib folder. So this is only about typings?

@beryxz
Copy link
Author

beryxz commented Aug 23, 2024

Yes, it's only about updating the type of the security property in the FastifySchema interface. The linked issue has some more detailed information.

@jdagerydnnsoft
Copy link

jdagerydnnsoft commented Aug 31, 2024

When this fix will be release?...

@@ -40,7 +40,7 @@ declare module 'fastify' {
consumes?: readonly string[];
produces?: readonly string[];
externalDocs?: OpenAPIV2.ExternalDocumentationObject | OpenAPIV3.ExternalDocumentationObject;
security?: ReadonlyArray<{ [securityLabel: string]: readonly string[] }>;
security?: ReadonlyArray<{ [securityLabel: string]: (readonly string[] | undefined) }>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is weird to allows undefined here.

@beryxz
Copy link
Author

beryxz commented Sep 1, 2024

As @climba03003 rightly pointed out, I also think my changes are invalid, and this issue/PR should be closed.

I looked into my problem more thoroughly, and it was not actually caused by Fastify or Typebox but by a lack of explicit typing on my end.

To give a simplified context:

const app = fastify().withTypeProvider<TypeBoxTypeProvider>();

// FastifyRequestTypebox & FastifyReplyTypebox defined as in the official README

export const SomeOperationSchema = {
    summary: 'Do some operation',
    security: [{ bearer_token: [] }, { api_key: [] }],
    response: {
        204: NoContentResponse
    },
};

async function some_operation(
    request: FastifyRequestTypebox<typeof SomeOperationSchema>,
    reply: FastifyReplyTypebox<typeof SomeOperationSchema>,
) {
    return reply.status(204).send();
}

app.get('/', { schema: SomeOperationSchema }, some_operation);

This approach didn't work as the dynamic result of typeof SomeOperationSchema doesn't satisfy the FastifySchema type:

[{ bearer_token: [] }, { api_key: [] }]

gets parsed as

({ bearer_token: never[]; test?: undefined; } | { test: never[]; bearer_token?: undefined; })[]

thus, not matching the security property type defined in FastifySchema.

I tried fixing this by explicitly setting the type of the route schema:

- export const SomeOperationSchema = {
+ export const SomeOperationSchema: FastifySchema = {

but this seems to break the typing of everything else.


What would be an appropriate solution?

@climba03003
Copy link
Member

I am closing this PR because it is related to how TypeScript works and coding styles.

What would be an appropriate solution?

  1. Place it all together to allows TypeScript infer.
import Swagger from '@fastify/swagger'
import Fastify from 'fastify'

const fastify = Fastify()

await fastify.register(Swagger)

fastify.get('/', {
  schema: {
    security: [{ api_key: [] }, { bearer: [] }]
  }
}, () => { })
  1. Make use of as const to bypass TypeScript auto reflection for array.
import Swagger from '@fastify/swagger'
import Fastify from 'fastify'

const fastify = Fastify()

await fastify.register(Swagger)

const schema = {
  security: [{ api_key: [] }, { bearer: [] }]
} as const

fastify.get('/', {
  schema
}, () => { })

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lists of Security Requirement Objects on an operation are not supported
4 participants