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

How would one modify an incoming request #551

Closed
CedricVanhaverbeke opened this issue Nov 2, 2021 · 13 comments · Fixed by fastify/fastify#4084
Closed

How would one modify an incoming request #551

CedricVanhaverbeke opened this issue Nov 2, 2021 · 13 comments · Fixed by fastify/fastify#4084
Labels
good first issue Good for newcomers

Comments

@CedricVanhaverbeke
Copy link

Use case

incoming request looks like this:

{
   "params": {
    "user": "@me"
     ...
   }
   "user" : {
     // user content here
   }
   ...
}

I would like to change the user parameter to the current user's id. This one is fetched from a JWT token that is passed along with the request. Let's say req.user contains the user object.

First solution

My first solution consists of creating a plugin (with fastify-plugin) to do this:

import fp from 'fastify-plugin'

// translates every "@me" query parameter to the current user's id
const plugin = fp(function (fastify: FastifyInstance) {
  fastify.addHook('preHandler', (req) => {
    const meEntries = Object.entries(req.params).filter(([_, value]) => value === '@me')
    if (meEntries.length > 0) {

      const myIdEntries = meEntries.map(([key]) => ([key, req.user.id]))

      req.params = {
        ...req.params as Record<string, any>,
        ...Object.fromEntries(myIdEntries),
      }
    }
  })
})

But this feels not right. I assume modifying the request-object is not the way to go

Alternative ideas

  • Create a decorator to modify the request. It seems designed for this use case. However, there's no documentation about changing an existing entry in the request
  • Modifying the request and redirecting to this modified request.

Question

How should this be approached?

@CedricVanhaverbeke CedricVanhaverbeke added the help wanted Extra attention is needed label Nov 2, 2021
@zekth
Copy link
Member

zekth commented Nov 2, 2021

Meanwhile technically this is correct, it's pretty confusing in a sense. Because params are supposed to be the client parameters sent within the request and not being mutated in the request lifecycle. If your server grows a lot it can lead to confusions.

I'd rather decorate the request with a proper name like parsedUser or something else; a bit like session handling.

@simoneb
Copy link

simoneb commented Jun 10, 2022

@CedricVanhaverbeke did you manage to solve your issue?

@Mazuh
Copy link

Mazuh commented Jun 20, 2022

Let me take a look on this today.

@Mazuh
Copy link

Mazuh commented Jun 20, 2022

afaik, we have two choices:

  • document how to add custom properties more properly (and not recycling existing ones)
  • or create an interface where users can set custom properties on FastifyInstance type, like maybe a property called .custom or another hook

considering the usage of typescript, I'm thinking on the second approach.

can a maintainer give some feedback on this? I'm available to work on it in any case.

@mcollina
Copy link
Member

cc @fastify/typescript I think the latter would be amazing but I don't know if it's possible

@fox1t
Copy link
Member

fox1t commented Jun 20, 2022

The latter should be feasible with the current type definitions! (Some genetics wizardry is required, though 😁).

@Mazuh
Copy link

Mazuh commented Jun 22, 2022

I managed to make what I believe to be a more proper way to do what you're wanting with typescript.

First, I generated a new project with fastify-cli using typescript template:

fastify generate hello-fastify-ts --lang=typescript

and then I created another file on plugin folder (in this setup, anything there will be autoloaded):

import fp from 'fastify-plugin'

export default fp(async (fastify, opts) => {
  fastify.addHook('preHandler', async (req) => {
    req.authenticatedUser = {
      id: 42,
      name: 'Marcell',
      token: 'wadda.wadda.wadda',
      isAdmin: true,
    };
  })
})

interface AuthenticatedUser {
  id: number,
  name: string;
  token: string;
  isAdmin: boolean;
}

declare module 'fastify' {
  export interface FastifyRequest {
    authenticatedUser?: AuthenticatedUser;
  }
}

it works pretty well, my type validations were working just fine, I got warnings when trying to use the new custom property in a wrong way both in controller and hook, tests ok, etc etc.

so, after some thought, I don't think it'd be very useful to add a builtin API to support that, imho the current one seems very straightforward, maybe there's some place to put this example in a documentation?

@CedricVanhaverbeke, do you think this is enough for what you're looking for?

maintainers, any recommended course of action for this?

@simoneb
Copy link

simoneb commented Jun 22, 2022

I agree with your conclusions. If anything, all we need to do here is write some docs, or point at some resources, or both :)

@mcollina
Copy link
Member

Would you like to send a PR improving the docs?

@mcollina mcollina added good first issue Good for newcomers and removed help wanted Extra attention is needed labels Jun 22, 2022
@simoneb
Copy link

simoneb commented Jun 22, 2022

Would you like to send a PR improving the docs?

@Mazuh ^

@Mazuh
Copy link

Mazuh commented Jun 23, 2022

Sure! I'll be opening until EOD.

@CedricVanhaverbeke
Copy link
Author

Thanks for the effort! Looks great! Wishing you all the best :D

@alzalabany
Copy link

I managed to make what I believe to be a more proper way to do what you're wanting with typescript.

First, I generated a new project with fastify-cli using typescript template:

fastify generate hello-fastify-ts --lang=typescript

and then I created another file on plugin folder (in this setup, anything there will be autoloaded):

import fp from 'fastify-plugin'

export default fp(async (fastify, opts) => {
  fastify.addHook('preHandler', async (req) => {
    req.authenticatedUser = {
      id: 42,
      name: 'Marcell',
      token: 'wadda.wadda.wadda',
      isAdmin: true,
    };
  })
})

interface AuthenticatedUser {
  id: number,
  name: string;
  token: string;
  isAdmin: boolean;
}

declare module 'fastify' {
  export interface FastifyRequest {
    authenticatedUser?: AuthenticatedUser;
  }
}

it works pretty well, my type validations were working just fine, I got warnings when trying to use the new custom property in a wrong way both in controller and hook, tests ok, etc etc.

so, after some thought, I don't think it'd be very useful to add a builtin API to support that, imho the current one seems very straightforward, maybe there's some place to put this example in a documentation?

@CedricVanhaverbeke, do you think this is enough for what you're looking for?

maintainers, any recommended course of action for this?

one problem with this approach is typescript now will expect authenticatedUser?: AuthenticatedUser; even tho it should really be authenticatedUser: AuthenticatedUser;

anyone have anysolution for that ?

i tried

fastify.addHook('preHandler', async (req) asserts FastifyRequest & {authenticatedUser: NonNullable<FastifyRequest["authenticatedUser"]>} => { }

but that broke down my type provider

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

Successfully merging a pull request may close this issue.

7 participants