4
\$\begingroup\$

This is a problem from "Automate the boring stuff".

Write a function that uses regular expressions to make sure the password string it is passed is strong. A strong password is defined as one that is at least eight characters long, contains both uppercase and lowercase characters, and has at least one digit. You may need to test the string against multiple regex patterns to validate its strength.

How to improve this code?

#! /usr/bin/python3

import re

def uppercase_check(password):
    if re.search('[A-Z]', password): #atleast one uppercase character
        return True
    return False

def lowercase_check(password):
    if re.search('[a-z]', password): #atleast one lowercase character
        return True
    return False

def digit_check(password):
    if re.search('[0-9]', password): #atleast one digit
        return True
    return False


def user_input_password_check():
    password = input("Enter password : ")
        #atleast 8 character long
    if len(password) >= 8 and uppercase_check(password) and lowercase_check(password) and digit_check(password):
        print("Strong Password")
    else:
        print("Weak Password")

user_input_password_check()
\$\endgroup\$
5
  • 3
    \$\begingroup\$ Please give some feedback to the authors of "Automate the boring stuff" that their task is misguided. Hopefully they will remove it then. \$\endgroup\$ Commented Jul 6, 2019 at 15:05
  • \$\begingroup\$ obXKCD \$\endgroup\$
    – Barmar
    Commented Jul 6, 2019 at 17:09
  • 4
    \$\begingroup\$ Aside: Obligatory Security.SE question about above XKCD. Also an article about how NIST changed their recommendations on the matter as well. In brief, this mindset about password "strength" is rapidly being outdated. Your question is fine, though, since you're given a problem and asking for a review of the solution. Just realize that this is a toy problem and not something that you should be implementing in production code. (Note that password security is more often done wrong than right, as well.) \$\endgroup\$
    – jpmc26
    Commented Jul 7, 2019 at 2:44
  • \$\begingroup\$ @jpmc26 Welp, it's better to have such a check than no check at all. Otherwise 2-character passwords are allowed too. \$\endgroup\$
    – Mast
    Commented Jul 7, 2019 at 18:49
  • \$\begingroup\$ @Mast Have a length check. That's the minimum. If you're going to do more, filter out common passwords (like at least the 10000 most common) and maybe some patterns or keywords that make the password easier to guess. Put visible recommendations for random passwords and 2-factor auth in your app. The kinds of requirements here actually reduce the password space and encourage poor password choice. \$\endgroup\$
    – jpmc26
    Commented Jul 8, 2019 at 5:33

4 Answers 4

9
\$\begingroup\$

I'm submitting this as a different answer because it goes in a different direction from my previous one, eliminating the bool cast as well as individual functions. You can simply define a tuple of regular expressions and apply all.

rexes = ('[A-Z]', '[a-z]', '[0-9]')
# ...
if len(password) >= 8 and all(re.search(r, password) for r in rexes)):
    print('Strong password')
\$\endgroup\$
2
  • 2
    \$\begingroup\$ While it doesn't use regexps just thought it worth pointing out an alternative using unicodedata.category. You categorise each letter and then check each of the types you want is present (lower case, upper case and digit), something along the lines of: if len(password) >= 8 and {'Ll', 'Lu', 'Nd'}.issubset(unicodedata.category(ch) for ch in password)... \$\endgroup\$ Commented Jul 6, 2019 at 17:34
  • \$\begingroup\$ @JonClements True, and that approach is probably more friendly to i18n \$\endgroup\$
    – Reinderien
    Commented Jul 6, 2019 at 23:09
7
\$\begingroup\$

First, please note that this security measure has been thoroughly debunked (although the reasons why are complex and psychological). Any code resulting from this exercise is therefore by the standards of the security community following an anti-pattern, and should not be used no matter how polished.

That said:

  • Rather than the

    if something:
        return True
    return False
    

    a more readable pattern is simply return something.

  • This code would be more scriptable if it either took passwords as arguments (using argparse) or read one password per line from standard input.
  • Another way to make this more scriptable is to use the really common

    if __name__ == "__main__":
        sys.exit(main())
    

    pattern.

\$\endgroup\$
4
\$\begingroup\$

Firstly: what @l0b0 said. This is fundamentally misguided, and you're better off doing an actual entropy measurement.

As for your check functions, you can rewrite them as:

def uppercase_check(password: str) -> bool:
   return bool(re.search('[A-Z]', password))

Note that it uses proper type-hinting, and doesn't need an if.

\$\endgroup\$
2
\$\begingroup\$
  • You preform multiple function calls which are all very related. You can cram each of the regex checks into one if statement.
  • You should use a main guard to ensure that the code is only running if this file is the main one, so that any programs importing this file don't have any issues.
  • You should also use docstrings. These allow potential documentation to display information about what the methods(s) are supposed to accomplish.
  • Using condensed if statements can clean up the messiness of having if: ... else: ... when there is just one statement being executed in each block.

Here is the updated code:

#! /usr/bin/python3

import re

def check(password):
    """ 
    Ensures password has at least one uppercase, one lowercase, and one digit
    """
    return False if not re.search('[A-Z]', password) or not re.search('[a-z]', password) or not re.search('[0-9]', password) else True

if __name__ == '__main__':
    password = input("Enter password: ")
    print("Strong password") if check(password) else print("Weak Password")
\$\endgroup\$
3
  • 6
    \$\begingroup\$ return False if not isn't the best way to go about this. Instead, cast to bool. \$\endgroup\$
    – Reinderien
    Commented Jul 6, 2019 at 13:51
  • 2
    \$\begingroup\$ You could also refrain from not using less fewer negations in your code. A much more natural way to express the original task is return has_digits and has_upper and has_lower. \$\endgroup\$ Commented Jul 6, 2019 at 15:02
  • \$\begingroup\$ Use some vertical whitespace in that very long boolean condition. Might even make sense to switch to an any call. \$\endgroup\$
    – jpmc26
    Commented Jul 7, 2019 at 2:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.