Skip to content

Tim Willis HW5#17

Open
t1mwillis wants to merge 4 commits into
pce-uw-jscript400:masterfrom
t1mwillis:master
Open

Tim Willis HW5#17
t1mwillis wants to merge 4 commits into
pce-uw-jscript400:masterfrom
t1mwillis:master

Conversation

@t1mwillis

Copy link
Copy Markdown

Finishing up

@bwreid bwreid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done!

Comment thread api/routes/auth.js Outdated
if (!username) throw new Error(`Username required to login`)

const guest = await User.findOne({username})
const isValid = bcrypt.compare(password, guest.password)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might need an await?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does 😄

Comment thread api/routes/auth.js Outdated
//make sure request body is OK - 400
const { permissions } = req.body
const { id } = req.params
if ( permissions !== (true || false)) throw new Error (`There was a problem with your request body`)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work like you want because of the way JS evaluates code. In this case, the first thing that's evaluated will be true || false which will evaluate to true. Then, you'll be left with the statement permissions !== true.

Comment thread api/routes/auth.js Outdated
res.status(status).json({status})

} catch (e) {
if (e.message == 'There was a problem with your request body') {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this is the best way to capture the messages, although I see where you're going! Instead, maybe throw a "key" that you can interpret. For example:

if (!updatedUser) throw new Error('user_does_not_exist')
// ...
if (e.message == 'user_does_not_exist') { /* ... */ }

This might be a bit easier in that if you want to change the message, you can do so more easily.

Comment thread api/routes/books.js
}
//try/catch
try {
const payload = jsonwebtoken.verify(token, SECRET_KEY)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what you want to use for whether or not the person is authorized. Just having a token is not enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants