unitaryfund / metriq-client

Fork of papers-with-code client
https://metriq.info
Apache License 2.0
3 stars 10 forks source link

Bug: Won't get an error with a weak password when making an account #22

Closed KarenRezkalla2 closed 1 year ago

KarenRezkalla2 commented 2 years ago

When I went to go create an account on metriq I put in a weak password (one with no numbers so just lower and uppercase letters) and the create account button stayed shaded (so it's not blue enough for me to press it to be allowed to create an account). I reloaded my page multiple times and didn't know what the problem was until I typed different passwords and it worked.

I would suggest an error message saying that the password created is not strong and a list of the requirements for a strong password on the account creating page of metriq.

QuantumAli commented 1 year ago

I am new here, but this seemed easy enough! as for the actual implementation, I do not know the backend side of it


def check_password_strength(password):
    failed_criteria = []
    common_words = ["password123.", "12345678", "qwerty", "qwerty123."] #this list can be expanded
    if len(password) < 8:
        failed_criteria.append("Minimum length requirement")
    if password.isalpha():
        failed_criteria.append("Contains only alphabetic characters")
    if password.isdigit():
        failed_criteria.append("Contains only numeric characters")
    if password.islower():
        failed_criteria.append("Contains only lowercase characters")
    if password.isupper():
        failed_criteria.append("Contains only uppercase characters")
    if not any(char.isalnum() for char in password):
        failed_criteria.append("Does not contain any special characters")
    if password.lower() in common_words:
        failed_criteria.append("Utilizes a common password")

    return failed_criteria

instead of append, it can be all changed to return True with the last line as return False and it still can be checked the same, but it does not tell you what you are missing

How it is checked:

user_password = input("Enter Your password: ")
# same variable outside of scope to make it easier to read
failed_criteria=check_password_strength(user_password)
if not failed_criteria:
    print('Strong Password')
else:
    for criterion in failed_criteria:
        print('-' + criterion)
QuantumAli commented 1 year ago

I just realized that since this is on a website it may need to be done on Javascript rather than python

vprusso commented 1 year ago

Thanks, @QuantumAli !

Unless I am mistaken, I believe it's possible that the initial OP meant to post this in this metriq-app project (as opposed to the metriq-client one).

As you rightly presumed, the checking for password viability should be done in he metriq-app project, and indeed, from what I can tell, seems to be doing so as expected.

Indeed, these checks do seem to already be covered in Javascript here.

@WrathfulSpatula is it possible that this issue was raised in error, or, perhaps there's some other check that I'm not aware of in this case that might have triggered the writing of this issue? From what I recall, this validation was done very early on in the project, so I'm surprised this issue was written up in the first place (but again, I could be missing the proper context).

WrathfulSpatula commented 1 year ago

Karen didn't understand the reasoning behind our password standards, which are actually conformant to the new NIST standards. NIST suggests that mixed letters and numbers, uppercase and lowercase letters, and special characters achieve relatively little for security while making a password difficult to remember and enter. NIST now suggests, drop all criteria except that a password is long: we require at least 12 characters. (NIST also suggests allowing users the option to see the password that they're entering in plain text or not at their option, which we also offer in the front end.) We do give an error, if the password is less than 12 characters.

(Sorry I was conked out with a 101 F fever for three days and didn't get here sooner, but...) There's no need to change the standard. If we did, we'd require 14 or 16 characters. This is more password total entropy, anyway!

vprusso commented 1 year ago

Gotcha. Okay, well, first of all, hope your fever has gone down. Second, with all that being said, I'm going to close out this issue. Reopen if I misunderstood something here though.