[ IMPR ] hashed passwords

[Winston-Hsiao]

Notable changes/additions:

1. created_at and updated_at fields to user (set on User creation)
2. email_verified_at field for when email verification is completed (has to be set in email verification api route, should be set automatically by default for standard auth methods)
3. OAuth providers (Google and Github) normalized with standard User model. Signing up with Google and Github now creates a new User, leaving hashed_password empty and instead setting the linked provider on the User object itself
4. Password strength check/validation

New npm packages:

1. zxcvbn password strength

New pip packages:

1. bcrypt: bcrypt pip page recommends argon2id or scrypt as alternatives to bcrypt itself
2. email-validator required for EmailStr pydantic type (validate user input emails)

[Winston-Hsiao]

@codekansas My local environment isn't properly set up yet to test this unfortunately, will need to take some time to set up local dynamo db instance, etc. But this should be the general extent to what we need added.

1. store hashed_password on user model
2. hash password input by user and store on account creation
3. on sign in use verify_password to check if input password is correct.

[codekansas]

looks good - only thing is, not sure what we will do for the password if the user logs in with google / github oauth instead. open to suggestions

[Winston-Hsiao]

looks good - only thing is, not sure what we will do for the password if the user logs in with google / github oauth instead. open to suggestions

Approaches I've seen in the past is having a nested OAuth account type related to the User model that can be defined or undefined (in the case that someone signs up using google/github oauth, we still make a User object like with standard sign up. We would also have to make hashed_password optional for the User model in that case. So all Users are lumped in that one type/collection which is good, and we can distinguish login/auth method based on if they have a linked account or a hashed_password set.

We'll have to adjust the various models we have now to get User data more normalized and then support the account type variants, rather than having them all be separate.

[chennisden]

Relatedly, we should run some checks, both client-side and server-side, that the password the user inputs is actually secure (e.g. we ought to reject 123456).

I am no fan of "must have an uppercase character/number/etc" requirements and think zxcvbn is a much better solution.

[Winston-Hsiao]

Relatedly, we should run some checks, both client-side and server-side, that the password the user inputs is actually secure (e.g. we ought to reject 123456).

I am no fan of "must have an uppercase character/number/etc" requirements and think zxcvbn is a much better solution.

agreed on both client and server side validation for password strength.

also agreed, I've implemented "must have an uppercase character/number/etc" myself in my own project and worry sometimes it's too much/annoying for new users, zxcvbn looks like a great solution, great suggestion.

[codekansas]

mostly good, can you address the comments and then we can merge

[codekansas]

is it advisable to keep these fields here? what are we going to use them for? in theory we could look this up from OAuth if we need to (although it would be kind of gross), i'm just not sure what we would use these IDs for once we have already retrieved the email. apologies if you'd addressed this already in the doc

[Winston-Hsiao]

good point,

only case I could see now is if someone wanted to unlink an OAuth provider. We have their user data already loaded (they're a signed in user and want to set a password and unlink the provider without deleting/resetting their account and all their listings/artifacts/data), we can then backtrace and search the OAuthKey based on their linked/associated OAuthProvider (that we have stored on their user object) otherwise we'd have to somehow persist the OAuth token past sign up and in cookies or something. Then we'd do the CRUD to delete the key and set their provider to empty string, also for consistency in the user model if a hashed_password value is not set it is assumed they have a populated github_id or google_id so having the 2 string fields exist could be useful for some validation checks and in terms of db storage space it's pretty light and not much of a concern.

[Winston-Hsiao]

Leaving in for now, resolved everything else and need approval for merge.

But can revisit this in the future if it really seems redundant/unnecessary we can remove them to keep the user model more lean.

[codekansas]

looks great :)