Strong Password Detection

Question

I did the following Excercise from Automate the boring stuff with Python Chapter 7:

Write a function that uses regular exppressions 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 a least one digit. You may need to test the string against multiple regex patterns to validate its strengh.

My Solution:

I wrote four functions which check the individual aspects of the required password detection. Then these four functions are used to write the strong-password function which validates strings against all the requirements.

To test this i also checked out the unittest module.

Please let me know if theres sth to do better.

Is this a good way to test? Are these good test cases? Are there any bad practices/ little issues in the coding / testing?

Heres the code:

password.py

import re

def valid_length(string):
    """checks if length is > 8 to be a strong password"""
    lenght_regex = re.compile(r'.{8,}')
    if not lenght_regex.search(string):
        return False
    return True

def has_upper(string):
    """Check if string contains one upper letter or more"""
    upper_regex = re.compile(r'.*[A-Z]+.*')
    if not upper_regex.search(string):
        return False
    return True

def has_lower(string):
    """Check if string contains one lower letter or more"""
    lower_regex = re.compile(r'.*[a-z]+.*')
    if not lower_regex.search(string):
        return False
    return True

def has_digit(string):
    """Check if one or more signs is a digit"""
    digit_regex = re.compile(r'.*\d+.*')
    if not digit_regex.search(string):
        return False
    return True

def strong_password(password):
    """
    Validate if passed password is considered "strong",
    Password is considered strong if:
    - is eight characters or longer
    - contains uppercase and lowercase characters
    - has one digit or more
    """
    if not valid_length(password):
        return False
    if not has_upper(password):
        return False
    if not has_lower(password):
        return False
    if not has_digit(password):
        return False
    return True

password_unit_test.py

import unittest
import password as p

class TestIsStrongPassword(unittest.TestCase):
    """Test of strong password detection function."""

    def test_valid_length(self):
        """Test that only a string length of > 8 is accecpted"""
        self.assertEqual(p.valid_length('abcd'), False)
        self.assertEqual(p.valid_length('abcdefg'), False)
        self.assertEqual(p.valid_length('abcdefgh'), True)
        self.assertEqual(p.valid_length('abcdefghi'), True)

    def test_has_upper(self):
        """Test that only strings containing uppercase are accepted"""
        self.assertEqual(p.has_upper('abcd'), False)
        self.assertEqual(p.has_upper('aBcd'), True)
        self.assertEqual(p.has_upper('aBCd'), True)
        self.assertEqual(p.has_upper('Abcd'), True)
        self.assertEqual(p.has_upper('abcD'), True)
        self.assertEqual(p.has_upper('ABCD'), True)

    def test_has_lower(self):
        """Test that only strings containing lowercase are accepted"""
        self.assertEqual(p.has_lower('abcd'), True)
        self.assertEqual(p.has_lower('aBcd'), True)
        self.assertEqual(p.has_lower('aBCd'), True)
        self.assertEqual(p.has_lower('Abcd'), True)
        self.assertEqual(p.has_lower('abcD'), True)
        self.assertEqual(p.has_lower('ABCD'), False)

    def test_has_digit(self):
        """Test that only strings containing lowercase are accepted"""
        self.assertEqual(p.has_digit('abcd'), False)
        self.assertEqual(p.has_digit('a1cd'), True)
        self.assertEqual(p.has_digit('a12d'), True)
        self.assertEqual(p.has_digit('1bcd'), True)
        self.assertEqual(p.has_digit('abc1'), True)
        self.assertEqual(p.has_digit('1234'), True)

    def test_strong_password(self):
        """
        Test strong password function. Passed strings have to pass 
        all tests in valid_length, uppper, lower and digit functions.
        """

        # Test from single functions should all fail 
        # (not met all criteria)
        self.assertEqual(False, p.strong_password('abcd'))
        self.assertEqual(False, p.strong_password('abcdefg'))
        self.assertEqual(False, p.strong_password('abcdefgh'))
        self.assertEqual(False, p.strong_password('abcdefghi'))

        self.assertEqual(False, p.strong_password('abcd'))
        self.assertEqual(False, p.strong_password('aBcd'))
        self.assertEqual(False, p.strong_password('aBCd'))
        self.assertEqual(False, p.strong_password('Abcd'))
        self.assertEqual(False, p.strong_password('abcD'))
        self.assertEqual(False, p.strong_password('ABCD'))

        self.assertEqual(False, p.strong_password('abcd'))
        self.assertEqual(False, p.strong_password('a1cd'))
        self.assertEqual(False, p.strong_password('a12d'))
        self.assertEqual(False, p.strong_password('1bcd'))
        self.assertEqual(False, p.strong_password('abc1'))
        self.assertEqual(False, p.strong_password('1234'))

        # Combinations which met more than one cirteria
        self.assertEqual(False, p.strong_password('12345678'))
        self.assertEqual(False, p.strong_password('Abcdefgh'))
        self.assertEqual(False, p.strong_password('A12345678'))
        self.assertEqual(False, p.strong_password('Abcdfg1'))
        self.assertEqual(True, p.strong_password('A12345678b'))
        self.assertEqual(True, p.strong_password('Abcdefg1'))
        self.assertEqual(True, p.strong_password('123456aB'))
        self.assertEqual(True, p.strong_password('aB345678'))

if __name__ == '__main__':
    unittest.main()

Answer 1

Good job on the easily understandable code.

Good

• Good functions, with clear names!
• Modular approach
• Unittests
• Docstrings

Improvements

• Regex with lots of backtracking can produce some major performance loss

  Consider that this re.search(r'.*[A-Z]+.*', string)

  is equal to re.search(r'[A-Z]+', string)

  or even re.search(r'[A-Z]', string) as Toby correctly suggested.

  Since we only care if one character is in the given string.

• Return directly

  Instead of doing

  if exdpression:
      return True
  return False
  

  Return directly with return expression

• Your compile has no performance gain, because with every new string it will compile again. Instead you could compile only onc,e and store it as a constant.

• Use the all keyword to check if all expressions evaluates to truthy.

• Instead of assertEqual(expression, function)

  Do the more direct assertFalse or assertTrue

Revised code

import re
import unittest

PASSWORD_CHECKS = [
    re.compile(r'[A-Z]'),
    re.compile(r'.{8,}'),
    re.compile(r'[a-z]'),
    re.compile(r'[0-9]'),
]

def strong_password(password):
    """
    Validate if passed password is considered "strong",
    Password is considered strong if:
      - is eight characters or longer
      - contains uppercase and lowercase characters
      - has one digit or more
    """
    return all(check.search(password) for check in PASSWORD_CHECKS)

class TestIsStrongPassword(unittest.TestCase):
    """Test of strong password detection function."""
    def test_strong_password(self):
        """
        Test strong password function. Passed strings have to pass 
        all tests in valid_length, uppper, lower and digit functions.
        """

        # Test from single functions should all fail 
        # (not met all criteria)
        self.assertFalse(strong_password('abcd'))
        self.assertFalse(strong_password('abcdefg'))
        self.assertFalse(strong_password('abcdefgh'))
        self.assertFalse(strong_password('abcdefghi'))

        self.assertFalse(strong_password('abcd'))
        self.assertFalse(strong_password('aBcd'))
        self.assertFalse(strong_password('aBCd'))
        self.assertFalse(strong_password('Abcd'))
        self.assertFalse(strong_password('abcD'))
        self.assertFalse(strong_password('ABCD'))

        self.assertFalse(strong_password('abcd'))
        self.assertFalse(strong_password('a1cd'))
        self.assertFalse(strong_password('a12d'))
        self.assertFalse(strong_password('1bcd'))
        self.assertFalse(strong_password('abc1'))
        self.assertFalse(strong_password('1234'))

        # Combinations which met more than one cirteria
        self.assertFalse(strong_password('12345678'))
        self.assertFalse(strong_password('Abcdefgh'))
        self.assertFalse(strong_password('A12345678'))
        self.assertFalse(strong_password('Abcdfg1'))
        self.assertTrue(strong_password('A12345678b'))
        self.assertTrue(strong_password('Abcdefg1'))
        self.assertTrue(strong_password('123456aB'))
        self.assertTrue(strong_password('aB345678'))

if __name__ == '__main__':
    unittest.main()

Notes

What is a strong password? Obligatory xkcd