fix: Authorization checker is executer after middlewares
Description
The authorization checker is executed after the middlewares (beforeMiddlewares).
Minimal code-snippet showcasing the problem
import 'reflect-metadata';
import { /*createExpressServer,*/ createKoaServer, JsonController, Authorized, Get, Action, UseBefore } from 'routing-controllers';
import { KoaBeforMiddleware } from './KoaMiddleware';
function KoaBeforMiddleware(context: any, next: (err?: any) => Promise<any>): Promise<any> {
console.log('do something before execution...');
return next()
.then(() => {
console.log('do something after execution');
})
.catch(error => {
console.log('error handling is also here');
});
}
@JsonController()
export class APIController {
@Authorized()
@Get('/items')
@UseBefore(KoaBeforMiddleware)
async getItems() {
return [];
}
}
const authorizationChecker = async (action: Action, roles: string[]) => {
console.log('authorizationChecker')
const token = action.request.headers["authorization"];
return token ? true : false;
};
// createExpressServer({ authorizationChecker }).listen(3000);
createKoaServer({ authorizationChecker }).listen(3001);
Expected behavior
authorizationChecker is called before the middleware
Actual behavior
authorizationChecker is called after the middleware
Hopefully this gets merged soon. I just ran into this issue myself.
@killix @attilaorosz Hello! What ended up happening with this issue? This seems like a good change, but I can see the legacy PR was closed without too much explanation? Was it breaking the other middlewares?
As an interim fix, I think I will replace authorizationChecker with a middleware of higher priority to my current middlewares. Could a good solution be to add the priority key that currently lives on @Middleware to this in some way?
@Johoseph Seems like the author of the PR didn't finish the changes (missing tests). This could indeed be a trivial change and if someone could pick it up I'd happy to merge it. I'm personally against the auth checker concept in general, I think the base framework should not be responsible for these kind of checks, that's why middlewares are a thing in the first place. Your solution of creating a simple middleware for this with a higher priority is exactly what I would do myself.