Feat/add password hashing with bcrypt (also fix create user function)

[Paulijuz]

lol this was not hard at all

I added the bcrypt package for password hashing. It's one of the older packages for password hashing, but it still remains one of the most popular ones. From what I can see it's considered safe and it's the one that the next js tutorial uses so I think it's good enough for our purposes.

Alternatively we could use argon2 or scrypt, but these packages have way fewer downloads. If someone has any objection tho it's not hard to switch (literally only three lines need to be changed.).

[JohanHjelsethStorstad]

The most important thing is the use of bcrypt.compare to avoid timing attacks.

[Paulijuz]

That's what's being used :+1:

[Paulijuz]

ERROR: failed to authorize: failed to fetch oauth token: unexpected status from POST request to https://auth.docker.io/token: 503 Service Unavailable

Hmmm

[theodorklauritzen]

Great work! I only have one question about peppering. After some research at Google, I found this Stackoverlofw post: https://stackoverflow.com/questions/16891729/best-practices-salting-peppering-passwords, where a user discourages the use of peppering. (We should be careful taking his advice since he is only a StackOverflow user.) I think pepper can be nice, but not how it is implemented now. If the pepper value gets exposed we cannot change the stored passwords without asking every user to update their password. A better solution here is to encrypt the password hash (without pepper and still using salt of course). Then if the encryption key is exposed but not the passwords we can decrypt and encrypt all the passwordhashes with a new key. I think all secrets should be easy to change at short notice, in case we are hacked. Here is a nice image I found for illustration:

It is worth noticing that Google google doesn't mention password peppering in their document about modern password security. https://cloud.google.com/solutions/modern-password-security-for-system-designers?ref=alphasec.io

[Paulijuz]

Totally agree @theodorklauritzen. I should have done more research. Although, I was a bit confused by what you meant by "I think pepper can be nice, but not how it is implemented now". The way I understand your argument we should not be using peppering at all. So I removed password peppering and replaced it with encryption.

There are a few things I'm unsure of and would like suggestions/feedback on:

• How should we handle seeding of password? Currently I have simply copy pasted the relevant functions into the seeder. When it was just a few lines it might have been fine, but now with encryption it's way too long to be maintainable. The obvious solution is to share the relevant files between the services, but sharing random files between src to prisma service seems messy.
• Where should the configuration for password hashing and encryption be? Currently only PASSWORD_SALT_ROUNDS and PASSWORD_ENCRYPTION_KEY are environment variables and the rest are at the top of password.ts. I don't feel we're realistically going to have the need to change the rest of the configuration, for example encryption encoding (hex or base64) or encryption algorithm, as part of deployment so they don't merit being environment variables. For non environment variable configuration we have ConfigVars.ts, but this would mean having a single file in the auth folder for all the different parts of auth.

[Paulijuz]

Thanks @theodorklauritzen. <3 I'll also wait @JohanHjelsethStorstad's review as this is quite critical functionality.

[JohanHjelsethStorstad]

I don think having the config in password.ts is a big deal

[JohanHjelsethStorstad]

We probably won't have a prismaservice in a few months. So I think the seeding will get cleaner then