Skip to content

Michael Munsey HW submission#8

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

Michael Munsey HW submission#8
munseym wants to merge 1 commit into
pce-uw-jscript400:masterfrom
munseym:master

Conversation

@munseym

@munseym munseym commented Jul 30, 2019

Copy link
Copy Markdown

No description provided.

@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.

This is really excellent work, Michael. I tried to give you a few helpful comments for the next time, but let me know if you're looking for any additional feedback.

Comment thread api/models/user.js

const schema = mongoose.Schema({
username: String,
password: String

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'll also want to include a boolean here of admin.

Comment thread api/routes/auth.js
}finally{
next()
}
})

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 great! Just be careful where to put this. It's likely that not all routes will need auth. For example, your signup and login routes below will still attempt to run this code, even though it's not needed.

Additionally, this could be put in app.js.

Comment thread api/routes/books.js
const error = new Error(`Unauthorized.`)
error.status = 401
next(error)
}

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 small bit of code could then like go to a route-level middleware function like isLoggedIn.

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