Password checker in Python

Question

Using Python 2.7.12 I wrote a simple little script, psk_validate.py, that prompts the user for a potential password and checks if it has upper and lower-case characters, numbers, special characters, and that it has a length of at least 8 characters.

From what I understand one could use the regex library to write this much more efficiently, however, I have yet to learn about regex.

The program seems to work just fine, and with a program this small I think that not using regex is also just fine.

I'd like any and all feedback about this program. In particular, I'd like to know if a program written this simply could be used in real-world applications. I'd also like to know if there are any logical errors and/or bugs in the program.

from sys import exit
def check_upper(input):
 uppers = 0 
 upper_list = "A B C D E F G H I J K L M N O P Q R S T U V W X Y Z".split()
 for char in input:
 if char in upper_list:
 uppers += 1
 if uppers > 0:
 return True
 else:
 return False
def check_lower(input):
 lowers = 0
 lower_list = "a b c d e f g h i j k l m n o p q r s t u v w x y z".split()
 for char in input:
 if char in lower_list:
 lowers += 1
 if lowers > 0:
 return True
 else:
 return False
def check_number(input):
 numbers = 0
 number_list = "1 2 3 4 5 6 7 8 9 0".split()
 for char in input:
 if char in number_list:
 numbers += 1
 if numbers > 0:
 return True
 else:
 return False
def check_special(input):
 specials = 0
 special_list = "! @ $ % ^ & * ( ) _ - + = { } [ ] | \ , . > < / ? ~ ` \" ' : ;".split()
 for char in input:
 if char in special_list:
 specials += 1
 if specials > 0:
 return True
 else:
 return False
def check_len(input):
 if len(input) >= 8:
 return True
 else:
 return False
def validate_password(input):
 check_dict = {
 'upper': check_upper(input),
 'lower': check_lower(input),
 'number': check_number(input),
 'special': check_special(input),
 'len' : check_len(input)
 }
 if check_upper(input) & check_lower(input) & check_number(input) & check_special(input) & check_len(input):
 return True
 else:
 print "Invalid password! Review below and change your password accordingly!"
 print
 if check_dict['upper'] == False:
 print "Password needs at least one upper-case character."
 if check_dict['lower'] == False:
 print "Password needs at least one lower-case character."
 if check_dict['number'] == False:
 print "Password needs at least one number."
 if check_dict['special'] == False:
 print "Password needs at least one special character."
 if check_dict['len'] == False:
 print "Password needs to be at least 8 characters in length." 
 print 
while True:
 password = raw_input("Enter desired password: ")
 print 
 if validate_password(password):
 print "Password meets all requirements and may be used."
 print 
 print "Exiting program..."
 print
 exit(0)

• WoJ: Related: blogs.dropbox.com/tech/2012/04/…

Answer 1

Concept

Obligatory XKCD comic, before I begin:

XKCD Password Strength

Enforcing password strength by requiring human-unfriendly characters is no longer considered good practice. Nevertheless, I'll review the code as you have written it.

"Obvious" simplifications

• Any code with the pattern if bool_expr: return True; else: return False should be written simply as return bool_expr.

• Strings are directly iterable; there is no need to convert them into a list first, using .split(). In other words, the code would work the same if you just wrote:

  upper_list = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
  

• Better yet, you could just use string.ascii_uppercase.

• The uppers += 1 counting loop could be written more expressively using the sum() built-in function. Actually, in this case, since you only care whether uppers > 0, you could just use the any() function.

With those changes, your check_upper() function becomes a one-liner:

def contains_upper(s):
 return any(c in ascii_uppercase for c in s)

I've renamed check_upper() to contains_upper() to make it clear that the function returns True or False. Also, avoid using variable names, like input, that coincide with names of built-in functions: it could cause trouble if you ever want to use input().

Code duplication

Most of your check_something() functions are identical. You should generalize, instead of duplicating the code.

from string import ascii_uppercase, ascii_lowercase, digits
def contains(required_chars, s):
 return any(c in required_chars for c in s)
def contains_upper(s):
 return contains(ascii_uppercase, s)
def contains_lower(s):
 return contains(ascii_lowercase, s)
def contains_digit(s):
 return contains(digits, s)
def contains_special(s):
 return contains(r"""!@$%^&*()_-+={}[]|,円.></?~`"':;""", s)
def long_enough(s):
 return len(s) >= 8

Note that I've used a raw long string to help deal with the need for backslashes in the punctuation string.

validate_password()

The check_dict isn't doing anything for you. You'd be no worse off with five boolean variables. You are also calling each validation function twice.

The & (binary bitwise AND) operator is not quite appropriate here. The and (boolean AND) operator would be more appropriate. Even though the results appear identical, the execution differs: the logical and allows short-circuit evaluation.

Personally, I'd write it this way, gathering up a list of all of the failure messages:

def validate_password(password):
 VALIDATIONS = (
 (contains_upper, 'Password needs at least one upper-case character.'),
 (contains_lower, 'Password needs at least one lower-case character.'),
 (contains_digit, 'Password needs at least one number.'),
 (contains_special, 'Password needs at least one special character.'),
 (long_enough, 'Password needs to be at least 8 characters in length.'),
 )
 failures = [
 msg for validator, msg in VALIDATIONS if not validator(password)
 ]
 if not failures:
 return True
 else:
 print("Invalid password! Review below and change your password accordingly!\n")
 for msg in failures:
 print(msg)
 print('')
 return False

If the function returns True in one place, then it would be good practice to return False instead of None in the other branch, for consistency.

Free-floating code

It is customary to put if __name__ == '__main__': around the statements in the module that are not inside a function. That way, you could incorporate the functions into another program by doing import psk_validate without actually running this program.

Calling sys.exit(0) is rarely desirable or necessary, if you structure the code properly. Here, all you needed was a break.

if __name__ == '__main__':
 while True:
 password = raw_input("Enter desired password: ")
 print()
 if validate_password(password):
 print("Password meets all requirements and may be used.\n")
 print("Exiting program...\n")
 break

• jpmc26: I think it's also obligatory to link the relevant Security.SE question, too. ;) +1
• Olivier Grégoire: As a Python-enthusiast (not a pro), I had a hard time reading the msg for validator, msg in VALIDATIONS...line. As I parsed in my head as a list of two elements, one of which is msg for validator and the other one is msg in VALIDATIONS... (and it made no sense). Wouldn't it be more readable to just put parentheses around validator, msg, making the line msg for (validator, msg) in VALIDATIONS...?
• Noah Cristino: This, XKCD Password generation challenge is also relevant: codegolf.stackexchange.com/questions/122756/…
• Peilonrayz: @OlivierGrégoire I personally haven't come across for (a, b) in ..., however if I did come across it then I'd go 'huh odd', which would break my train of thought.
• jpmc26: @OlivierGrégoire It's an example of unpacking. It's the same thing as x, y = [1,2]. The loop version is very common with dict iteration as well: for k, v in d.items(). The assignment is not typically enclosed in parentheses. You might also be thrown by the fact it's a list comprehension, too. If you have trouble reading it, I'd actually recommend a newline after the msg, to separate the element evaluation from the loop iteration. Now that you know the pattern, though, you'll start recognizing it more easily.

Answer 2

• You can make check_upper, check_lower, etc all use one function, and so you want to make a function such as check_contains(input, letters). Which can further be improved by:

  1. Returning early by using return True if char in letters is true.
  2. You can make this a comprehension.
  3. You can use any to achieve the same as (1) when using (2).

  And so I'd use:

  def check_contains(input, letters):
   return any(char in letters for char in input)
  

• I'd personally make validate_password(input) only return true or false, however to keep with what you're doing, I'll keep it so that it prints.

• Remove sys.exit, it's not intended to be used in programs. Instead use break to break out of the while loop.
• I'd use getpass, rather than raw_input to get the users password. This is as it should turn echo off, and so won't display the users password, so others can shoulder serf for their password.
• Rather than manually writing out the strings, you can use strings.

And so I'd change your code to:

from getpass import getpass
import string
def check_contains(input, letters):
 return any(char in letters for char in input)
def validate_password(input):
 valid = True
 if not check_contains(input, string.ascii_uppercase):
 valid = False
 print "Password needs at least one upper-case character."
 if not check_contains(input, string.ascii_lowercase):
 valid = False
 print "Password needs at least one lower-case character."
 if not check_contains(input, string.digits):
 valid = False
 print "Password needs at least one number."
 if not check_contains(input, string.punctuation + '#'):
 valid = False
 print "Password needs at least one special character."
 if len(input) < 8:
 valid = False
 print "Password needs to be at least 8 characters in length."
 return valid
while True:
 password = getpass("Enter desired password: ")
 if validate_password(password):
 print "Valid password"
 break

If you were to make your program follow SRP, then validate_password shouldn't print. And so you may want to use the below instead. If printing your messages is super important for you, then that should be a separate function than validating if the password is correct.

from getpass import getpass
import string
def check_contains(input, letters):
 return any(char in letters for char in input)
def validate_password(input):
 return all([
 check_contains(input, string.ascii_uppercase),
 check_contains(input, string.ascii_lowercase),
 check_contains(input, string.digits),
 check_contains(input, string.punctuation + '#'),
 len(input) >= 8
 ])
while True:
 password = getpass("Enter desired password: ")
 if validate_password(password):
 print "Valid password"
 break
 else:
 print "invalid password"

• grundic: You can replace hardcoded letters with string.ascii_lowercase, string.ascii_uppercase and numbers with string.digits as stated in the documentation
• Peilonrayz: @grundic I edited to add that, I don't like having to remember it's 'lowercase', rather than 'lower' so I normally skip using it, ;P
• grundic: Sure ;) And don't forget to import string :p
• Peilonrayz: @grundic thanks, I legitimately forgot to add them, )
• Graipher: If many passwords need to be validated, you probably want to define global constants for the character classes as sets.