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

Order of routes matters #152

Open
Christian24 opened this issue May 20, 2017 · 7 comments
Open

Order of routes matters #152

Christian24 opened this issue May 20, 2017 · 7 comments
Labels
status: awaiting answer Awaiting answer from the author of issue or PR.

Comments

@Christian24
Copy link

Hello,

awesome project. I have an issue though. My JSON-Controller looks like this:

import {Router, Request, Response, NextFunction} from 'express';
import {getEntityManager, Repository} from "typeorm";
import {User} from "../models/user";
import {authService} from "../services/authservice";

import {getConnectionManager} from "typeorm";
import {JsonController} from "routing-controllers/decorator/JsonController";
import {Authorized, Body, Get, Post, Res} from "routing-controllers";


interface authData {
    password: string;
    username: string;
    email?: string;
}
@JsonController("/users")
export class UsersController {
    private userRepository: Repository<User>;

    constructor() {
        this.userRepository = getConnectionManager().get().getRepository(User);
    }


    @Post("/sign_up")
   public async   signUp(@Body() authData: authData, @Res() res: Response) {
        if (authData.username && authData.email && authData.password) {
            let user = await  this.userRepository.findOneById(authData.username);
            const count = await this.userRepository.count();
            if (!user) {
                user = new User();
                user.username = authData.username;
                user.email = authData.email;
                user = await authService.setHashedPassword(user, authData.password);
                user.admnin = count === 0;


                this.userRepository.persist(user);
                res.send(user);
            }
            else {
                res.send("Already exists");
            }
        } else {
            res.send("Incomplete");
        }
    }


    @Post("/sign_in")
   public async signIn(@Body() authData: authData, @Res() res: Response) {
        if (authData.username && authData.password) {
            let user = await  this.userRepository.findOneById(authData.username);

            if (user) {
                console.log(user);
                const valid = await authService.checkPassword(authData.password, user);
                if (valid) {
                   authService.setTokenForUser(res, user).then(() => {
                        res.send("logged in");
                    });

                } else {
                    res.send("Wrong information");
                }
            } else {
                res.send("Not found");
            }
        } else {
            res.send("Incomplete");
        }
    }
    @Authorized()
    @Get("/auto")
    public async checkAuthentication(@Res() res: Response) {
        res.send("Authorized");
    }

}

This crashes on the sign_in route. Cannot POST /users/sign_in. Funny enough if I change the order to have the GET route at the top of the class like this:

import {Router, Request, Response, NextFunction} from 'express';
import {getEntityManager, Repository} from "typeorm";
import {User} from "../models/user";
import {authService} from "../services/authservice";

import {getConnectionManager} from "typeorm";
import {JsonController} from "routing-controllers/decorator/JsonController";
import {Authorized, Body, Get, Post, Res} from "routing-controllers";


interface authData {
    password: string;
    username: string;
    email?: string;
}
@JsonController("/users")
export class UsersController {
    private userRepository: Repository<User>;

    constructor() {
        this.userRepository = getConnectionManager().get().getRepository(User);
    }
    @Authorized()
    @Get("/auto")
    public async checkAuthentication(@Res() res: Response) {
        res.send("Authorized");
    }

    @Post("/sign_up")
   public async   signUp(@Body() authData: authData, @Res() res: Response) {
        if (authData.username && authData.email && authData.password) {
            let user = await  this.userRepository.findOneById(authData.username);
            const count = await this.userRepository.count();
            if (!user) {
                user = new User();
                user.username = authData.username;
                user.email = authData.email;
                user = await authService.setHashedPassword(user, authData.password);
                user.admnin = count === 0;


                this.userRepository.persist(user);
                res.send(user);
            }
            else {
                res.send("Already exists");
            }
        } else {
            res.send("Incomplete");
        }
    }


    @Post("/sign_in")
   public async signIn(@Body() authData: authData, @Res() res: Response) {
        if (authData.username && authData.password) {
            let user = await  this.userRepository.findOneById(authData.username);

            if (user) {
                console.log(user);
                const valid = await authService.checkPassword(authData.password, user);
                if (valid) {
                   authService.setTokenForUser(res, user).then(() => {
                        res.send("logged in");
                    });

                } else {
                    res.send("Wrong information");
                }
            } else {
                res.send("Not found");
            }
        } else {
            res.send("Incomplete");
        }
    }


}

everything works fine. Is there something I am missing or is this a bug?

Thanks a lot,
Christian

@pleerock
Copy link
Contributor

sounds strange, does it work if you remove @Authorized() decorator?

@Christian24
Copy link
Author

No, that doesn't solve it. Only the order seems to solve it. Interestingly enough it only seems to crash in signIn. SignIn is being called though.

setTokenForUser is defined like this:

 setTokenForUser: (res: Response, user: User) => {
        return new Promise((resolve, reject) => {
            const jwtClaimSet: JWTClaimSet = { name: user.username, email: user.email };
            jwt.sign(jwtClaimSet, config.authentication.secret, { algorithm: 'HS256' },
                (err: Error, token: string) => {
                    if (err) {
                        reject('Could not create jwtToken!')
                        console.log(err);
                    } else {
                        console.log("Hello world");
                        res.cookie(config.authentication.cookiename, token);
                        console.log(token);
                        resolve();
                    }
                });
        });
    }

It seems to crash on the cookie function call. The console.log is still being executed.

@pleerock
Copy link
Contributor

This should not be possible. Can you please setup a git repo with a minimal reproduction of your problem, then I can checkout, debug and solve your problem.

@Christian24
Copy link
Author

There you go: https://github.com/Christian24/routingcontrollersissue/tree/master

Thanks a lot.

@MichalLytek
Copy link
Contributor

MichalLytek commented Sep 5, 2017

@Christian24
Can you do a MINIMAL example? One simple controller with 2-3 simple routes, one service if needed, not the entire jungle which is hard to understand.

For me it looks like you've forgot to return promise somewhere in you hell and because of next fallthrough the next route is catched as it also match to the pattern.

@fernandolguevara
Copy link

fernandolguevara commented Dec 5, 2018

@pleerock @19majkel94 same problem here!!!

I have two endpoints on controller 'UserController'

@Get('/me')
@Get('/:userId')

both marked as @Authorized

some requests executes /:userId with 'me' as param

@github-actions
Copy link

Stale issue message

@github-actions github-actions bot added the status: awaiting answer Awaiting answer from the author of issue or PR. label Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: awaiting answer Awaiting answer from the author of issue or PR.
Development

No branches or pull requests

5 participants