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

preHandler types conflict with package @fastify/auth #5

Closed
2 tasks done
TheWashiba opened this issue May 27, 2022 · 19 comments
Closed
2 tasks done

preHandler types conflict with package @fastify/auth #5

TheWashiba opened this issue May 27, 2022 · 19 comments

Comments

@TheWashiba
Copy link

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.0.0-rc.3

Plugin version

No response

Node.js version

16.x

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

Ubuntu 20.04

Description

I'm having TypeScript issues in the route preHandler when the package @fastify/auth ^3.0.0 is being used altogether. I'm unsure if this an issue of that package or this, but I've began experiencing issues once .withTypeProvider<TypeBoxTypeProvider>() is called. The error received:

Type 'preHandlerHookHandler<Server, IncomingMessage, ServerResponse, RouteGenericInterface, unknown, FastifySchema, FastifyTypeProviderDefault, ResolveFastifyRequestType<...>, FastifyLoggerInstance>' is not assignable to type 'preHandlerHookHandler<Server, IncomingMessage, ServerResponse, RouteGenericInterface, unknown, FastifySchema, TypeBoxTypeProvider, ResolveFastifyRequestType<...>, FastifyLoggerInstance> | preHandlerHookHandler<...>[] | undefined'.

Steps to Reproduce

Install fastify@4, @fastify/auth@3 and .1.0-beta.1 version of this plugin in a TypeScript environment. Calling the fastify.auth([]) in the route preHandler will give a type error.

Expected Behavior

No response

@mcollina
Copy link
Member

cc @fastify/typescript could you take a look?

@mcollina
Copy link
Member

Could you create a repo with the setup that is not working?

@TheWashiba
Copy link
Author

@mcollina
Copy link
Member

thx, I'll take a look asap

@mcollina
Copy link
Member

@fastify/typescript can you take a look as well? I'm kind of unsure what's the problem.

@mcollina
Copy link
Member

@kibertoad @fox1t @Eomm could you take a look?

@kibertoad
Copy link
Member

on it

@TheWashiba
Copy link
Author

Any news about it?

@fox1t
Copy link
Member

fox1t commented Jun 16, 2022

First of all, sorry for the late response. I've just looked into this, and it turns out that the issue is indeed the .withTypeProvider<TypeBoxTypeProvider>() call.

`@fastify/auth arguments fastify instance with auth method that is a preHandlerHookHandler that uses the default type provider https://github.com/fastify/fastify-auth/blob/master/auth.d.ts#L18
This clashes with your fastify instance that uses TypeBox TypeProvider.

Unfortunately, at the moment, I don't have a solution. I will look more deeply into this and return if anything comes to mind.

@mcollina
Copy link
Member

cc @sinclairzx81

@sinclairzx81
Copy link
Contributor

@mcollina @TheWashiba Hi. Had a brief look at this the other day. The issue appears to be caused by the server.auth([]) function returning a function signature that is globally typed to use the FastifyTypeProvider (not the TypeBoxTypeProvider as configured). As such the assignment of server.auth([]) on the preHandler is the primary source of this type error as the Provider types are incompatible (as mentioned by @fox1t )

Unfortunately, because the auth() function is globally typed (using module augmentation here), and because the plugin is patching the auth() function at runtime on the fastify instance, there doesn't appear to be many options available to resolve the local typing at the provider, as TypeScript is unaware of any type modifications made by the patch.

Workaround

In the interim, the following should offer a workaround if using Type Providers (albeit a not ideal solution with the any). @TheWashiba this code was derived from your repro repository. You can preview the typing on this located here.

import { Type } from '@sinclair/typebox'
import { TypeBoxTypeProvider } from '@fastify/type-provider-typebox'
import fastifyAuth from '@fastify/auth'
import fastify from 'fastify'
import fp from 'fastify-plugin'

// --------------------------------------------------------------------------------
// Plugin
// --------------------------------------------------------------------------------

export const plugin = fp(
  (fastify) => {
    const instance = fastify.withTypeProvider<TypeBoxTypeProvider>();

    instance.route({
      method: 'GET',
      url: '/plugin',
      schema: {
        body: Type.Object({
          a: Type.Number(),
          b: Type.Number()
        })
      },
      preHandler: instance.auth([]) as any, // todo: review global module augmentation of the server auth() function.
      handler: async (req, res) => { 
        const { a, b } = req.body 
      },
    });
  },
  {
    name: 'example',
  }
);

// --------------------------------------------------------------------------------
// Index
// --------------------------------------------------------------------------------

const server = fastify().withTypeProvider<TypeBoxTypeProvider>();
server.register(fastifyAuth);
server.register(plugin)
server.route({
  method: 'GET',
  url: '/',
  schema: {
    body: Type.Object({
      x: Type.Number(),
      y: Type.Number()
    })
  },
  preHandler: server.auth([]) as any, // todo: review global module augmentation of the server auth() function.
  handler: async (req, res) => {
    const { x, y } = req.body
  },
});

Possible Next Steps

I think probably the best way forward is spending some time taking a look at the way the current server.auth([]) function definitions (potentially avoiding global module augmentation if possible). I think any investigations into this would likely be applicable for other plugin modules that augment the underlying FastifyInstance. So would make for good documentation for plugin authors writing TS definitions.

In terms of TypeScript, the ideal way to allow types to propagate would be to chain calls to register(), as follows.

const server = fastify()
 .withTypeProvider<TypeBoxTypeProvider>() // remap to generic TypeBoxTypeProvider
 .register(fastifyAuth)                   // augment instance with `.auth()` function.
 .register(plugin)                        // augment with user defined functionality.
 .route({                                 // route: configured with the above
   method: 'GET',
   url: '/',
   schema: {
     body: Type.Object({
       x: Type.Number(),
       y: Type.Number()
     })
   },
   preHandler: server.auth([]),
   handler: a sync (req, res) => {
     const {  x, y } = req.body
   }
  });

Although there may be additional options to explore with the following PR's

fastify/fastify#3952 - Global augmentation of FastifyTypeProviderDefault
fastify/fastify#4034 - Propagate generics from FastifyRegister

Hope that helps!
S

@mcollina
Copy link
Member

@fox1t @climba03003 could any of you take ownership of this? it looks like it's the final step for our TS definitions.

@climba03003
Copy link
Member

climba03003 commented Jun 22, 2022

The next steps solution cannot be implemented without a breaking change.
As a quick but not a good fix, we can loosen the type on fastify-auth to preHandlerHookHandler<any, any, any, any, any, any, any, any, any> or do the similar thing on fastify-websocket.

declare module 'fastify' {
  interface FastifyInstance<RawServer, RawRequest, RawReply, Logger, TypeProvider> {
    auth(
      functions: FastifyAuthFunction[],
      options?: {
        relation?: 'and' | 'or',
        run?: 'all'
      }
    ): preHandlerHookHandler<RawServer, RawRequest, RawReply, RouteGenericInterface, ContextConfigDefault, FastifySchema, TypeProvider>;
  }
}

@mcollina
Copy link
Member

The next steps solution cannot be implemented without a breaking change.

Maybe open a tracking issue on Fastify? It could be the basis of Fastify v5.


Regarding the fix on fastify-auth, go for it. It seems the generic one is better, but that's my 2 cents.

@mcollina
Copy link
Member

The change in fastify landed. I would like to close this and move the discussion about the follow-up in Fastify proper. Could you open such an issue @climba03003?

@irg1008
Copy link

irg1008 commented Jan 26, 2023

Hi, I have searched and searched but It seems there is no clear solution to this specific problem yet.

Do you know a way to avoid having to put "as any" everywhere?

@nahtnam
Copy link

nahtnam commented Feb 18, 2023

Hi, just wanted to add onto this ticket here, I'm also unable to get it working without using as any. Here are the relevant snippets of code. Am I doing something wrong?

export const fastify = FastifyServer({}).withTypeProvider<ZodTypeProvider>();

fastify.decorate('verifyServerSecret', async (request: FastifyRequest) => {
  if (request.headers.authorization !== `Bearer ${env.SERVER_SECRET}`) {
    throw new UNAUTHORIZED("You don't have access to this endpoint");
  }
});

fastify.register(fastifyAuth);

declare module 'fastify' {
  interface FastifyInstance {
    verifyServerSecret: FastifyAuthFunction;  // technically this typing is wrong because I use `async/await` but it still works
  }
}

export type FastifyInstance = typeof fastify;

And then used like so:

fastify.route({
    method: 'POST',
    url: '/test',
    schema: {
      body: z.object({
        record: z.object({
          id: z.string(),
        }),
      }),
      response: {
        200: z.object({}),
      },
    },
    preHandler: fastify.auth([fastify.verifyServerSecret]), // adding `as any` here fixes it but doesn't seem right
    async handler(req) {
      const {
        record: { id },
      } = req.body; // ERROR: typed incorrectly :(

      return {};
    },
  });

@nahtnam
Copy link

nahtnam commented Feb 18, 2023

Ideally I want to get more complicated and do something like this:

fastify.decorate('authenticateUser', async (request: FastifyRequest) => {
  // check headers and fetch user
  req.user = {} as User
});

where req.user is correctly typed:

fastify.route({
    // ...
    preHandler: fastify.auth([fastify.authenticateUser]),
    async handler(req) {
      // req.user works and is correctly typed but only on routes that define the `preHandler`
    },
  });

it this a possibility?

@ekoeryanto
Copy link

Hi, to prevent my self confused again.
the solution is only to cast to never or any the prehandler as it impact nothing

fastify.route({
    // ...
    preHandler: [] as never,
    // ...
  });

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

No branches or pull requests

9 participants