joshuaslate / mern-starter

A simple starter/seed project for a fullstack JavaScript app using MongoDB, Express, React, and Node.
http://slatepeak.com
Apache License 2.0
536 stars 179 forks source link

api/auth/login always shows "Unauthorized" response when invalid email/password is used #24

Open ketanbhardwaj opened 7 years ago

ketanbhardwaj commented 7 years ago

When I try to pass wrong email/password then the user.comparePassword function in passport.js always returns "Unauthorized" as response. I could not find any way to fix it. Can you help me with this?

screenshot from 2017-06-02 17-00-08

jamesjsewell commented 7 years ago

I get the same thing as well. When I test it out by using the actual front end form, it works and returns a jwt.

ketanbhardwaj commented 7 years ago

@jamesjsewell When I use the correct login and password then I too get the correct response with JWT and all but with invalid login credentials "Unauthorized" is coming. Is the same happening with you??

jamesjsewell commented 7 years ago

yeah me too. It's because the login route is actually returning a jwt that goes into the user's cookies. The jwt is read by looking in the cookies for a cookie named 'token'. This token can then be used when requesting info from routes. That's why you won't receive anything from postman

imalik8088 commented 7 years ago

This behavior is also documented in the tests https://github.com/joshuaslate/mern-starter/blob/master/server/tests/loginAndAuthTest.js#L108

just run npm run test in the server folder.

ketanbhardwaj commented 7 years ago

@iNeedCode I get the below result when running test There is no "Unauthorized" response in the test. I even tried by creating HTML form but still same issue.

AUTH REGISTRATION POST /api/auth/register 422 3.636 ms - 44 ✓ should FAIL [422] to create a user without parameters POST /api/auth/register 422 0.721 ms - 38 ✓ should FAIL [422] to create a user with incomplete parameters (96ms) (node:14302) DeprecationWarning: Mongoose: mpromise (mongoose's default promise library) is deprecated, plug in your own promise library instead: http://mongoosejs.com/docs/promises.html POST /api/auth/register 201 196.020 ms - 435 ✓ should CREATE [201] a valid user (202ms) POST /api/auth/register 422 7.169 ms - 49 ✓ should FAIL [422] to create a user with occupied email

AUTH LOGIN POST /api/auth/register 201 456.061 ms - 435 POST /api/auth/login 400 1.157 ms - - ✓ should FAIL [400] to login without parameters POST /api/auth/login 400 0.467 ms - - ✓ should FAIL [400] to login with bad parameters POST /api/auth/login 401 1.780 ms - - ✓ should FAIL [401] to login with invalid credentials POST /api/auth/login 200 12.617 ms - 435 ✓ should LOGIN [200] with valid credential

imalik8088 commented 7 years ago

@ketanbhardwaj What is your requirement or what would like to expect (http code, body) from wrong credentials?

In my opinion 401 is fine because you didn't want to tell the user that either this user is existing or not an invalid password.

ketanbhardwaj commented 7 years ago

@iNeedCode So below is the code that executes for login: Now when compare password is called with invalid credentials then
{ error: 'Your login details could not be verified. Please try again.' } json should be outputted but instead Unauthorized is showing.

const localLogin = new LocalStrategy(localOptions, (email, password, done) => { User.findOne({ email }, (err, user) => { if (err) { return done(err); } if (!user) { return done(null, false, { error: 'Your login details could not be verified. Please try again.' }); }

user.comparePassword(password, (err, isMatch) => {
  if (err) { return done(err); }
  **if (!isMatch) { return done(null, false, { error: 'Your login details could not be verified. Please try again.' }); }**

  return done(null, user);
});

}); });

joshuaslate commented 7 years ago

Hi everyone,

Sorry, I have been busy. Take a look at the custom callback section of the Passport docs: http://passportjs.org/docs

This can be implemented, I just didn't do it here (yet). I'd be open to a PR if anyone had the time.

Thanks,

Josh

ketanbhardwaj commented 7 years ago

@joshuaslate Still not able to overcome that "Unauthorized" response. I tried from the client side as well.

screenshot from 2017-06-08 12-53-00

eldyvoon commented 7 years ago

I got this issue as well, worked in front end but doesn't work in postman, I even passed the Test in the header, still got unauthorized. Need Help.

capozzic1 commented 7 years ago

I have the same issue.

joshuaslate commented 7 years ago

Hey, I was able to solve this in my new starter, which uses Koa instead of Express. I would imagine the implementation would be similar, if you care to take a look: https://github.com/joshuaslate/mkrn-starter

marcusmolchany commented 6 years ago

Had the same issue, got this to work using the custom callbacks passport docs.

router.post('/login', (req, res) => {
  passport.authenticate('local', (err, user, info) => {
    if (err) {
      return res.status(500).send();
    }
    if (!user && info) {
      return res.status(422).send(info);
    }

    // do something here and send back a res.json()
  })(req, res);
});
mirsahib commented 5 years ago

had the same issue tried everything above https://github.com/mirsahib/Authentication

DaraSingh1998 commented 4 years ago

This recently happened with one of my junior and what he did wrong was: He wrote passport.authenticate("teacher", { successRedirect: "/teacher/", failurRedirect: "/", failurFlash: true, })(req, res, next);

instead of passport.authenticate("teacher", { successRedirect: "/teacher/", failureRedirect: "/", failureFlash: true, })(req, res, next);

There is a missing "e" in failureRedirect and failureFlash and so when a wrong credential is entered the callback doesn't know where to go so it returns "UNAUTHORIZED" with a code 401. There might be some similar errors in yours also.

Anirudh24-me commented 4 years ago

I also had the same issue, but while registering. I solved it by replacing

This function(username, password, done) { User.findOne({ username: username }, function(err, user) { if (err) { return done(err); } if (!user) { return done(null, false, { message: 'Incorrect username.' }); } if (!user.validPassword(password)) { return done(null, false, { message: 'Incorrect password.' }); } return done(null, user); }); }

With this

User.authenticate()

abderrezek commented 3 years ago

try this return done({ error: 'Your login details could not be verified. Please try again.' }, false );

abutalhasheikh33 commented 3 years ago

I also had the same issue, but while registering. I solved it by replacing

This function(username, password, done) { User.findOne({ username: username }, function(err, user) { if (err) { return done(err); } if (!user) { return done(null, false, { message: 'Incorrect username.' }); } if (!user.validPassword(password)) { return done(null, false, { message: 'Incorrect password.' }); } return done(null, user); }); }

With this

User.authenticate()

I getting unauthorized although i am using User.authenticate() const express = require("express") const app = express() const bodyParser = require('body-parser') app.set('view engine', "ejs") app.use(bodyParser.urlencoded({ extended: true })) app.use(express.static(__dirname + "/public")) const mongoose = require('mongoose') const url = "mongodb://localhost/axre" mongoose.connect(url, { useNewUrlParser: true, useUnifiedTopology: true, useCreateIndex: true }); const studentRoutes = require('./routes/index') const passport = require('passport') const LocalStrategy = require('passport-local') let Student = require('./models/student') const Teacher=require('./models/teacher') //passport configuration

app.use(require("express-session")({ secret: "Final year project", resave: false, saveUninitialized: false }));

app.use(passport.initialize()) app.use(passport.session())

passport.use("local",new LocalStrategy({usernameField:"emailid"},Student.authenticate())) passport.use("teacher-signup",new LocalStrategy({usernameField:"emailid"},Teacher.authenticate())) passport.use(Student.createStrategy()) passport.use(Teacher.createStrategy())

passport.serializeUser((user,done)=>{

let key={
    type:user.type,
    id:user.id
}

    done(null,key)

}) passport.deserializeUser((key,done)=>{ let Model=(key.type === "Student")?"Student":"Teacher" Model.findOne({_id:key.id},(err,user)=>{ if(err){ console.log(err) } else{

        done(null,user)
    }
})

})

app.use(require('connect-flash')()); app.use(function (req, res, next) { res.locals.student = req.user res.locals.messages = require('express-messages')(req, res); next(); });

app.use(studentRoutes)

app.listen(3000, () => { console.log("listening") })

fResult commented 2 years ago

login with passport local strategy's request body must be only username and password (can't use email)

Body Request

{
  "username": "email@example.com",
  "password": "your_password"
}

Response from API

image

My Auth Service

import { Injectable } from "@nestjs/common";
import { UsersService } from "../../users/services/users.service";

@Injectable()
export class AuthService {
  constructor(
    // @Inject(forwardRef(UsersService))
    private usersService: UsersService,
  ) {}

  async validateUser(email: string, password: string) {
    const user = await this.usersService.findByEmail(email);

    if (user?.password === password) {
      return user;
    }

    // throw new UnauthorizedException("Email or Password is incorrect");
    return null;
  }
}

My Auth Controller

import { ClassSerializerInterceptor, Controller, Post, Request, UseGuards, UseInterceptors } from "@nestjs/common";
import { User } from "../../../users/entities/user.entity";
import { LocalAuthGuard } from "./../../guards/local-auth.guard";

@UseInterceptors(ClassSerializerInterceptor)
@Controller("auth")
export class AuthController {
  @UseGuards(LocalAuthGuard)
  @Post("login")
  async login(@Request() req: { user: User }) {
    return req.user;
  }
}

My Local Strategy file

import { AuthService } from "../service/auth.service";
import { Injectable, UnauthorizedException } from "@nestjs/common";
import { PassportStrategy } from "@nestjs/passport";
import { Strategy /*as Local*/ } from "passport-local";

@Injectable()
export class LocalStrategy extends PassportStrategy(Strategy) {
  constructor(private authService: AuthService) {
    super();
  }

  async validate(email: string, password: string): Promise<any> {
    const user = await this.authService.validateUser(email, password);

    if (!user) {
      throw new UnauthorizedException("Username or Password is invalid");
    }
    return user;
  }
}