Conversation
bwreid
left a comment
bwreid
left a comment
There was a problem hiding this comment.
Overall, nice job! Error handling looks great, just a bit of work needed to some of the routes.
| if (!user) throw new Error('Username could not be found.') | ||
|
|
||
| const validPassword = await bcrypt.compare(password, user.password) | ||
| if (!validPassword) throw new Error('Password is incorrect.') |
There was a problem hiding this comment.
You'll want to give the same errors in either case. Otherwise, it's easier for someone to "guess" at a valid username.
| const user = await User.findOne({ _id: validToken.id }) | ||
|
|
||
| //Check for valid token | ||
| if (!tokenValidate) { res.status(401).json({ status, response: 'Token is not valid.' })} |
There was a problem hiding this comment.
This should probably be validToken instead, right?
| if (!user.admin == true) { res.status(400).json({ status, response: 'User is not an Admin.' }) | ||
|
|
||
| //Try and see if user can be found, | ||
| if (!user == null) { |
There was a problem hiding this comment.
This statement is a bit odd. I think what you want here is if (!user).
| //Try and see if user can be found, | ||
| if (!user == null) { | ||
| user.admin = req.body.admin | ||
| const user = await User.save |
There was a problem hiding this comment.
This should be:
await user.save()|
|
||
| return payload | ||
| } | ||
|
|
There was a problem hiding this comment.
You could just export this function so you don't need to copy it everywhere!
| const response = await Book.find().select('-__v') | ||
| } else { | ||
| status = 401 | ||
| } |
There was a problem hiding this comment.
I'd be surprised if this worked? For this route, it's OK to let anyone request this information. But, it also looks like response won't be defined in the case the user is not an admin.
| validToken = tokenValidate(req); | ||
| const user = await User.findOne({ _id: validToken.id }) | ||
|
|
||
| if (!user.admin == true) { |
There was a problem hiding this comment.
I think this is the opposite of what you want. We only want the user to be able to create a book if they are an admin. So, I think we just need if (user.admin) here.
2 participants
No description provided.