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

TypeScript Handler Type Incompatibility with AuthenticatedRequest in Fastify #1058

Open
kaanoz1 opened this issue Sep 16, 2024 · 3 comments
Open
Labels
help wanted Extra attention is needed question Further information is requested

Comments

@kaanoz1
Copy link

kaanoz1 commented Sep 16, 2024

For more details and context, you can view my codebase here: https://github.com/Prag1974/writings-backend

I am building a web API using Fastify with TypeScript. My project utilizes the following packages:

{
  "name": "books",
  "version": "1.0.0",
  "dependencies": {
    "@fastify/cookie": "^9.4.0",
    "@fastify/passport": "^2.5.0",
    "@fastify/session": "^10.9.0",
    "bcrypt": "^5.1.1",
    "dotenv": "^16.4.5",
    "fastify": "^4.28.1",
    "nodemon": "^3.1.4",
    "passport-local": "^1.0.0",
    "pg": "^8.12.0",
    "ts-node": "^10.9.2",
    "zod": "^3.23.8"
  },
  "devDependencies": {
    "@eslint/js": "^9.9.1",
    "@types/bcrypt": "^5.0.2",
    "@types/node": "^22.4.0",
    "@types/passport-local": "^1.0.38",
    "@types/pg": "^8.11.6",
    "@typescript-eslint/eslint-plugin": "^8.2.0",
    "@typescript-eslint/parser": "^8.2.0",
    "eslint": "^9.9.1",
    "globals": "^15.9.0",
    "typescript": "^5.5.4",
    "typescript-eslint": "^8.2.0"
  }
}

Problem Description

I have set up route (./src/session/route.ts) that require authentication for all endpoints located in it and have added a preHandler hook for these routes to verify if the user is authenticated. Since the preHandler ensures authentication, request.user should always be defined in the subsequent route handlers.

Here is the mentioned route:

import type { FastifyInstance, HookHandlerDoneFunction } from "fastify";
//Other imports...

export default function sessionRoute(
  server: FastifyInstance,
  _opts: unknown,
  done: HookHandlerDoneFunction
): void {
  //Session

  server.addHook("preHandler", checkAuthentication); /* Here is the authentication hook. I added that hook to not violate DRY principle, since all endpoints require authentication, all endpoints have to own the authentication check section.
  
  server.post("/logout", { handler: logout });

  //Collections

  server.get("/collection/get", {
    preValidation: validateFunction({ BodyParams: getCollectionSchema }),
    handler: getCollection,
  });

//Other endpoints requiring authentication to response.

  server.post("/note/create", {
    preValidation: validateFunction({ BodyParams: createNoteSchema }),
    handler: createNote,
  });

  done();
}

Here is the declaration of checkAuthentication function

export function checkAuthentication(
  request: FastifyRequest,
  response: FastifyReply,
  done: HookHandlerDoneFunction
): void | unknown {
  if (!request.user)
    return response.code(HTTP_UNAUTHORIZED_CODE).send(NotLoggedInResponse);

  done();
}

So, all endpoints located in the route ensure that request.user is not optional. But declaration made in FastifyRequst still claims that request.user might be nullish (normally). But I did want to prevail that situation by creating a new interface which user property definitely defined and I would be, finally, able to use it by not concerning about it if it is nullish, since I already checked that it is not.

export interface AuthenticatedRequest<RouteGeneric extends RouteGenericInterface = RouteGenericInterface> 
extends FastifyRequest<RouteGeneric> {
  user: User; // user is definitely defined.
}

Afterwards, I used that interface in my handler function declarations e.g:

import type { FastifyReply } from "fastify";
import {
  HTTP_ACCEPTED_CODE,
  HTTP_INTERNAL_SERVER_ERROR_CODE,
  InternalServerErrorResponse,
} from "../../../libs/utility/types/utility";
import type { z } from "zod";
import type { createNoteSchema } from "../types/createNoteSchema";
import type { AuthenticatedRequest } from "../types/utility";
import type { User } from "../../../libs/session/passport/type";

export const createNote = async (
  request: AuthenticatedRequest<{ Body: z.infer<typeof createNoteSchema> }>, //Behold the type!
  response: FastifyReply
): Promise<FastifyReply> => {
  try {
    //TODO: Implement the rest. For now, this endpoint's purpose is only testing!

    const User: User = request.user; /*This line is not giving error as request parameter with the type AuthenticatedUser. However, if I switch the type of request parameter to FastifyRequest, it is yelling for type discrepancy, as you can guess, with the error message:
 Type 'PassportUser | undefined' is not assignable to type 'User'.
 Type 'undefined' is not assignable to type 'User'.ts(2322)
 */

    return response.code(HTTP_ACCEPTED_CODE).send({ user: request.user });
  } catch (error) {
    console.error(error);
    return response
      .status(HTTP_INTERNAL_SERVER_ERROR_CODE)
      .send(InternalServerErrorResponse);
  }
};

That all works, there was no problem until my endpoint declarations are yelling:

Screenshot from 2024-09-16 02-24-49

Here is the one of the error messages from my endpoints:

      Types of parameters 'request' and 'request' are incompatible.
        Type 'FastifyRequest<RouteGenericInterface, RawServerDefault, IncomingMessage, FastifySchema, FastifyTypeProviderDefault, unknown, FastifyBaseLogger, ResolveFastifyRequestType<...>>' is not assignable to type 'AuthenticatedRequest<{ Body: { /*some body parameters*/ }; }>'.
          Types of property 'user' are incompatible.
            Type 'PassportUser | undefined' is not assignable to type 'User'.
              Type 'undefined' is not assignable to type 'User'.ts(2769)

The problem is how can I solve this situation? Because I am ensure that request.user is not null or undefined as I already checked it before, hence, I wanna use it freely. Here is the some AI advices to solve that situation and the reason behind why I didn't want to use them:

What I've Tried

1. Type Assertions Inside Handlers

Using a type assertion within the handler:

export const logout = async (
  request: FastifyRequest<{ Reply: PositiveResponse | NegativeResponse }>,
  response: FastifyReply
): Promise<void> => {
  const authRequest = request as AuthenticatedRequest;
  // Use authRequest.user
};

Why this doesn't meet my requirements: I want to avoid type assertions inside handlers to keep the code clean and maintain adherence to coding standards. It still violates DRY and damaging the readability as well as memory.

2. Non-Null Assertions

Using the non-null assertion operator:

const userId = request.user!.id;

Why this doesn't meet my requirements: Non-null assertions are discouraged by ESLint rules and general coding practices I follow. Also it is require to use ! operator whenver I am supposed to access user object. It comes an unnecessary usage. I guess there is another solution to meet this problem.

3. Wrapping Handlers with Higher-Order Functions

Creating a higher-order function to wrap the handler:

function withAuthenticatedRequest(handler) {
  return (request, reply) => {
    return handler(request as AuthenticatedRequest, reply);
  };
}

export const logout = withAuthenticatedRequest(async (request, response) => {
  // Handler logic
});

Why this doesn't meet my requirements: Wrapping each handler function individually violates the DRY principle and reduces code readability.

Constraints and Requirements

I aim to achieve the following:

  1. Use AuthenticatedRequest directly in my route handlers that require authentication, ensuring that request.user is recognized as defined by TypeScript.
  2. Avoid using type assertions or non-null assertions (e.g., request.user as User or request.user!) inside handler functions, to maintain code readability and adhere to ESLint and coding standards.
  3. Avoid wrapping handler functions with higher-order functions or custom registration functions, as this violates the DRY principle and affects code readability and coherence.
  4. Avoid modifying the global FastifyRequest interface, since doing so would incorrectly affect routes that do not require authentication.
  5. Register routes normally, without any additional wrappers or changes to how handlers are defined.

As I mentioned before, you can check my code: https://github.com/Prag1974/writings-backend

I understand that this is a complex issue involving TypeScript's type system and Fastify's handler expectations. If there are any recommended practices or patterns within Fastify to handle such cases, I'd be grateful to learn about them.

Independently of this goal I envisaged (declaring a new type with property "user" and use it coherently and without any problem.), can you suggest another solution or solution path for this problem? What other solutions can I apply to this problem without violating coding principles, without disrupting readability and overall structure?

I hope I have made my problem clear. If there is anything you don't completely understand, please contact me or provide an answer to this issue. I appreciate any guidance or solutions that can help resolve this issue while adhering to the outlined constraints. Thank you in advance for your assistance! Lastly, please pardon my English.

@kaanoz1 kaanoz1 added the help wanted Extra attention is needed label Sep 16, 2024
@dosubot dosubot bot added the question Further information is requested label Sep 16, 2024
@marcoturi
Copy link

I didn't check the code, but something like this should work:


import type { FastifyInstance, HookHandlerDoneFunction } from "fastify";
import { AuthenticatedRequest } from "./authenticated-request"; // Import conditional type

type AuthenticatedRequest<Body extends object = {}> =  FastifyRequest<Body> & { user: User }

export default function sessionRoute(
  server: FastifyInstance,
  _opts: unknown,
  done: HookHandlerDoneFunction): void {

  // ... existing code

  server.get("/collection/get", {
    preValidation: validateFunction({ BodyParams: getCollectionSchema }),
    handler: (request: AuthenticatedRequest<{ Body: z.infer<typeof createNoteSchema> }>, response: FastifyReply) => {
      // Use request.user with type safety
      const userId = request.user.id; // No type assertion needed
      // ... rest of handler logic
    },
  });
}

@kaanoz1
Copy link
Author

kaanoz1 commented Oct 14, 2024

I didn't check the code, but something like this should work:


import type { FastifyInstance, HookHandlerDoneFunction } from "fastify";
import { AuthenticatedRequest } from "./authenticated-request"; // Import conditional type

type AuthenticatedRequest<Body extends object = {}> =  FastifyRequest<Body> & { user: User }

export default function sessionRoute(
  server: FastifyInstance,
  _opts: unknown,
  done: HookHandlerDoneFunction): void {

  // ... existing code

  server.get("/collection/get", {
    preValidation: validateFunction({ BodyParams: getCollectionSchema }),
    handler: (request: AuthenticatedRequest<{ Body: z.infer<typeof createNoteSchema> }>, response: FastifyReply) => {
      // Use request.user with type safety
      const userId = request.user.id; // No type assertion needed
      // ... rest of handler logic
    },
  });
}

It did not worked. There is still type incompatibility error. Error message vscode produce:

Type '(request: AuthenticatedRequest<{ Body: z.infer; }>, response: FastifyReply) => Promise' is not assignable to type 'RouteHandlerMethod<RawServerDefault, IncomingMessage, ServerResponse, RouteGenericInterface, unknown, FastifySchema, FastifyTypeProviderDefault, FastifyBaseLogger>'.
Types of parameters 'request' and 'request' are incompatible.

@AaronPorts
Copy link

AaronPorts commented Oct 17, 2024

Also didn't find a solution and just use patch-package for user?: PassportUser in FastifyRequest interface to make it required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants