feature/migrate-to-bcrypt

[jantaylor]

Context

• This is part of release-1.5 #148
• References Issue: #158

Tasks

• [x] Remove dependency bcrypt-nodejs in package.json
• [x] Add dependency bcrypt in package.json
• [x] Migrate file app/data/user-dao.js to bcrypt
  • Validated that the password worked with the insecure password plain text and bcrypt hash
• [x] Validate the installation with the local test
  • Runs and finishes immediately with "no files to check".
• [x] Add and submit the changes in package-lock.json
• [X] Add the primary dependency list to the readme.md
  • There is not a dependency list in readme.md
• [x] Check that the npm tasks are working as expected
  • test:ci fails the first test due to the before() which runs cy.dbReset(). If I comment it out, it passes, and then the next test suite fails it's before() which does the cy.dbReset().
• [x] Update the readme.md with the extra relevant info (if needed)
  • No extra info needed however I added a section for using mongo as a docker container

Additional changes:

• Updated the repository in package.json
• Added a section for using mongo as a docker container
• Had to also change app/routes/session.js to async/await for bcrypt or plain text check

I ran npm run precommit and it returned a long list of formatting fixes and changed 36 files. I did not commit those changes as it would disrupt this PR and feature.

[PeterWunderlich]

LGTM.

[binarymist]

@jantaylor Are there no changes to be applied to docker-compose? You could also add a check list item that purpleteam doesn't find any more or fewer bugs? NodeGoat is one of the main SUTs we use to hone purpleteam

[jantaylor]

Are there no changes to be applied to docker-compose?

@binarymist Not to the docker-compose. Now that you mentioned that, I think there's a change required for the Dockerfile as alpine images of node usually don't have the required software installed for bcrypt, usually python. https://github.com/kelektiv/node.bcrypt.js#dependencies

I'll try building it and see if it works otherwise, it'd need to have python installed or move to node:12.

You could also add a check list item that purpleteam doesn't find any more or fewer bugs? NodeGoat is one of the main SUTs we use to hone purpleteam

I'm not sure I'm understanding the request. Did you want me to add a known issues list?
It looks like purpleteam is a security application that can test your apps. If my understanding is correct then are you are using NodeGoat for training & validations?

[binarymist]

I'm not sure I'm understanding the request. Did you want me to add a known issues list? It looks like purpleteam is a security application that can test your apps. If my understanding is correct then are you are using NodeGoat for training & validations?

Purpleteam was born out of this very file which is of course now redundant. We've been using NodeGoat since as a SUT. I was thinking you could just add a task to the existing task list in this PR to just run purpleteam over NodeGoat once this change is ready? For me, this just means updating my NodeGoat fork and redeploying it using purpleteam-iac-sut which was also created to deploy NodeGoat on AWS for free using Terraform, then running purpleteam over it.

When I see your ready with this PR I can do the honors.

[lirantal]

@jantaylor happy to land the changes but want to run a check by you incase you had anything else you wanted to follow-up on.

[jantaylor]

@lirantal It looks like most of the changes are with the package-lock.json. For the the other two files, we need to make sure that app/routes/session.js calls await on userDAO.validateLogin. Feel free to land the changes.

[lirantal]

Seems that it does: https://github.com/OWASP/NodeGoat/pull/237/files#diff-c08aab9e521b9749dbc6a23ffb3163a41871c1d89162017d8c9bb702dfc29d2eR56

If you think it's ready as-is, I'll go ahead and land it.

[jantaylor]

@lirantal it looks like the conflicts need to be resoled. Want me to resolve them so we can get this PR merged?

[lirantal]

@jantaylor yes, thank you 🙏🏽