curriculum icon indicating copy to clipboard operation
curriculum copied to clipboard

Express 105: Forms and Deployment: Password vulnerability risk

Open EmilyErickson opened this issue 1 year ago • 5 comments

Checks

Describe your suggestion

I just finished the MDN library tutorial. There is a warning in the MDN instructions to hide database usernames and passwords, but there isn't a clear explanation on how to do that. I was looking through other contributions and several of the most recent contributions have their database credentials in the app.js file.

There is also a warning in the Odin assignment for this lesson, but I think it would be better if there was some sort of step by step walkthrough explaining how to hide sensitive information. There is a link in one of the MDN notes to dotenv and I ended up using that but I had to do quite a bit of troubleshooting. I think having some basic instructions would be very beneficial.

I would be happy to help work on this problem, but I am not sure of the best way to go about it since I don't have much experience. Thanks!

Path

Node / JS

Lesson Url

https://www.theodinproject.com/lessons/nodejs-express-105-forms-and-deployment

(Optional) Discord Name

No response

(Optional) Additional Comments

No response

EmilyErickson avatar Feb 21 '24 19:02 EmilyErickson

Good shout for sure.

Just to provide some additional context, I think one of the big issues here is that the learner will generally be committing/pushing as per usual while following the MDN tutorial. However, the MDN tutorial seems to not expect a learner to do so, as the instructions for setting up a repo are not until the deployment stage at the very end. Hence it makes sense why they have no issues with someone hardcoding their connection string in plain sight, but of course this would be an issue for anyone pushing to Github while doing this for obvious reasons. The tutorial's intent seems to be to hardcode then replace with process.env.XXXXX before pushing then deploying, with the real connection string going in Glitch/Railway's env variable settings (so the real connection string never gets published).

Environment variables and .env are not introduced in the course until Security Configuration, which is far beyond the first point a learner could hardcode and push their connection string (which I believe is 102 - CRUD and MVC).

While this will of course be something we can easily handle in the Node revamp, it seems like a tricky situation to resolve without bringing much of the security lesson forward, or telling the learner to not push to a repo whilst coding along, or perhaps a little more simply add an big warning to push to a private repo instead if the learner wishes to push to their own repo? Thoughts from maintainers?

MaoShizhong avatar Feb 22 '24 00:02 MaoShizhong

It might not be the best solution, but we could add an instruction telling users to just import their credentials from a gitignored file. What do yall think of that?

Some change here does sound good so I'll go ahead and assign you @EmilyErickson, thanks for putting this issue up and thanks @MaoShizhong for jumping in with your thoughts!

wise-king-sullyman avatar Feb 27 '24 01:02 wise-king-sullyman

Does it make sense to address it in an earlier lesson? I found this very simple overview of how to use dotenv - [https://javascript.plainenglish.io/how-to-use-dotenv-and-why-you-need-it-in-all-your-projects-a7319e16124a]

Would it work to add an assignment to the Express 102: CRUD and MVC lesson that would have students read that article and then put their credentials in an .env file instead of following the MDN instructions?

EmilyErickson avatar Feb 27 '24 02:02 EmilyErickson

As someone who just completed the MDN Express tutorial (without committing my MongoDB auth string to my repo) and Security Configuration lesson, I just wish I had read through the Security Configuration lesson just before completing part 3 of the MDN Express tutorial (Experss 102 in TOP). Using a .env file felt pretty easy, I just didn't know it was a thing until part 7, so something like "by the way, when MDN says 'paste your MongoDB URI in app.js', don't do that! Do this instead" could have gone a long way.

tylerprehl avatar Feb 27 '24 04:02 tylerprehl

This issue is stale because it has had no activity for the last 30 days.

github-actions[bot] avatar Mar 29 '24 01:03 github-actions[bot]

Closing this as the merge of milestone 1 of the Node revamp addresses this (https://github.com/TheOdinProject/theodinproject/pull/4582#event-13561502963)

MaoShizhong avatar Jul 18 '24 18:07 MaoShizhong