Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restructure database and user authentication. Setup password hashing with salt rounds. #27

Merged
merged 13 commits into from Nov 21, 2019

Conversation

@tcharlan
Copy link
Collaborator

tcharlan commented Nov 19, 2019

No description provided.

@tcharlan tcharlan requested a review from Derpthemeus Nov 19, 2019
app/api/users.js Outdated Show resolved Hide resolved
app/api/users.js Outdated Show resolved Hide resolved
app/api/users.js Outdated Show resolved Hide resolved
app/api/users.js Outdated Show resolved Hide resolved
app/api/users.js Outdated Show resolved Hide resolved
app/api/users.js Outdated Show resolved Hide resolved
app/api/users.js Outdated Show resolved Hide resolved
if (err) {
res.status(400).json({ error: "Failed to authenticate." });
} else if (valid) {
const token = jwt.encode({ username: user.username }, SECRET);

This comment has been minimized.

Copy link
@Derpthemeus

Derpthemeus Nov 21, 2019

Collaborator

SECRET is undefined


router.post("/user", (req, res) => {
bcrypt.genSalt(saltRounds, (err, salt) => {
bcrypt.hash(req.body.password, salt, (err, hash) => {

This comment has been minimized.

Copy link
@Derpthemeus

Derpthemeus Nov 21, 2019

Collaborator

This is erroring. It looks like bcrypt.hash requires you to specify all 4 parameters (but the progress parameter can be undefined)

status: req.body.status
});

newUser.save((err) => {

This comment has been minimized.

Copy link
@Derpthemeus

Derpthemeus Nov 21, 2019

Collaborator

This isn't currently checking that usernames are unique. Was that check unintentionally omitted, or do we just plan on adding it at a later date?

Copy link
Collaborator

Derpthemeus left a comment

Approving as per @rrossetti 's request

@tcharlan tcharlan merged commit 38dd5dd into dev Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.