Skip to content

J.Sorge w5 Exercise - Complete#26

Open
Jiansorge wants to merge 1 commit into
pce-uw-jscript400:masterfrom
Jiansorge:master
Open

J.Sorge w5 Exercise - Complete#26
Jiansorge wants to merge 1 commit into
pce-uw-jscript400:masterfrom
Jiansorge:master

Conversation

@Jiansorge
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown
Collaborator

@bwreid bwreid left a comment

Choose a reason for hiding this comment

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

Overall this looks great. Just a couple comments.

Comment thread api/routes/auth.js

const payload = { id: user._id }
const options = { expiresIn: '1 day' }
const token = jwt.sign(payload, SECRET_KEY, options)
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.

Because you're performing this operation multiple times, this is a great thing to put into a function.

Comment thread api/routes/auth.js
}

Object.assign(userToUpdate, admin)
await user.save()
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 shouldn't work, as there's no user variable.

Comment thread api/routes/books.js
try {
const token = req.headers.authorization.split('Bearer ')[1]
// A valid JWT token is not provided (status 401)
if (!token) {
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.

You don't want to just check whether or not there is a token, you want to check whether or not the token is valid. In this case, you should use payload from below.

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