tsed: [BUG] Multer Middleware executed before auth middleware
Describe the bug
I’m currently in progress of implementing a multer file upload and just noticed that the MulterMiddleware is executed before the auth middleware.
I debugged into here:
And you can see the auth middleware is after the multer one:
To Reproduce
My controller is simple:
@Controller('/admin')
// My Custom Auth Decorator, calling UseAuth underneath as described in docs
@AdminAuth({ permissions: [ Read ] })
export class AdminController {
...
@Put()
@AdminAuth({ permissions: [ Write ] })
public async add(@MultipartFile('image') image: any) { ... }
...
}
Expected behavior
The Auth Middleware should be executed before the Multer Middleware to not process & upload the uploaded files’s if the user hasn’t even the permission to do so.
It should be generally one of the first middlewares imo, so before validations and stuff, but i think that is already the case
Code snippets
No response
Repository URL example
No response
OS
macOS
Node version
20.9.0
Library version
7.43.0
Additional context
No response
About this issue
- Original URL
- State: closed
- Created 8 months ago
- Comments: 18 (4 by maintainers)
Commits related to this issue
- feat(common): add priority option on middleware decorator Closes: #2516 — committed to tsedio/tsed by Romakita 8 months ago
- feat(common): add priority option on middleware decorator Closes: #2516 — committed to tsedio/tsed by Romakita 8 months ago
- feat(common): add priority option on middleware decorator Closes: #2516 — committed to tsedio/tsed by Romakita 8 months ago
- feat(common): add priority option on middleware decorator Closes: #2516 — committed to tsedio/tsed by Romakita 8 months ago
- feat(common): add priority option on middleware decorator Closes: #2516 — committed to tsedio/tsed by Romakita 8 months ago
- feat(common): add priority option on middleware decorator Closes: #2516 — committed to tsedio/tsed by Romakita 8 months ago
- feat(common): add priority option on middleware decorator Closes: #2516 — committed to tsedio/tsed by Romakita 8 months ago
- feat(common): add priority option on middleware decorator Closes: #2516 — committed to tsedio/tsed by Romakita 8 months ago
Ok perfect. I found a quick win to fix this issue 😉 See you
Yes sure. I’ll try to fix that ASAP 😉
I think you’ve right, but changing that isn’t simple because the Pipe architecture (validation, desezialize, etc…) are always called per parameters just before the class/method execution, so after all middlewares added before the endpoint method.
Actually Ts.ED do that:
If we want to have a logical order execution to prevent unnecessary upload or code execution, the workflow should be
But this workflow doesn’t work, because
@MultipartFiledepend on ExpressionPipe which depend on the multer middleware.So maybe Validation shouldn’t be performed on Pipe but in a middleware (the middleware should build a schema that aggregate all schemes for each parameters in one and run the validation over the request.params/headers/body/query). Changing that will be a big challenge (and maybe a breaking change).