0

I need to make a simple password checker, it needs to check if the string has at least one lowercase letter, one uppercase letter and one number.

here's what i have

passwd = raw_input("enter your password: ")
upper = 0
lower = 0 
number = 0
while len(passwd) < 7 and upper <= 0 and lower <= 0 and number <= 0:
    print 'start'
    for x in passwd:
        if x.isupper()==True:
            print 'upper'
            upper+=1
        elif x.islower()==True:
            print 'lower'
            lower+=1
        elif x.isalpha()==False:
            print 'num'
            number+=1
    print ('password not complex enough')
    upper = 0
    lower = 0
    number = 0
    passwd = raw_input('please enter password again ')

those random looking prints are for debugging so i can see where it's reaching.

and what looks like is happening is that it only checks the first condition, the length condition and then leaves the loop.

here's a sample of what is happening

enter your password: pA1
start
lower
upper
num
password not complex enough
please enter password again password

why is it only checking the length condition? what did i do wrong?

Muffinator
  • 123
  • 2
  • 11
  • _“why is it only checking the length condition?”_ - what makes you think it was only doing that? You get `lower`, `upper` and `num` in your output - where do you think that comes from, if not from those conditions? You just get `password not complex enough` because you output that _every_ time, completely independent of what your actual condition checks resulted in. – CBroe Aug 25 '17 at 09:28
  • 1
    change `elif` to `if` – Stack Aug 25 '17 at 09:29
  • Your condition for `while` loop is `True` only in the first loop, because as soon as upper, lower or number gets `< 0` it will evaluate to `False`, because you're using `and`. – user3053452 Aug 25 '17 at 09:31
  • @CBroe I don't think it's only doing that, what i meant is that it only uses that condition to leave the loop, if the string is longer than 7 then it leaves the loop without checking if there are uppercase or lowercase letters – Muffinator Aug 25 '17 at 09:35
  • Well yeah, of course, because you made `len(passwd) < 7` one of the conditions that decides whether it even enters the loop body ... – CBroe Aug 25 '17 at 10:13

4 Answers4

8

You can simplify the whole logic by utilising unicodedata.category to collect what category of a character is and then check to see if all expected are present, eg:

import unicodedata

is_valid = (
    len(password) >= 7 and 
    {'Ll', 'Lu', 'Nd'}.issubset(unicodedata.category(ch) for ch in password)
)
Jon Clements
  • 124,071
  • 31
  • 219
  • 256
  • Very nice. Though I'd prefer `{'Ll', 'Lu', 'Nd'} <= set(map(unicodedata.category, password))` or `{'Ll', 'Lu', 'Nd'}.issubset(map(unicodedata.category, password))`. – Stefan Pochmann Aug 25 '17 at 09:43
  • @Stefan going for the `.issubset` to allow it to take any iterable and avoid the explicit creation of a `set` and the gen-comp instead of `map` to keep it functionally the same in Py 2 and Py3... (avoiding an unnecessary `list` built in Py 2)... But yeah... they'll all work... – Jon Clements Aug 25 '17 at 09:48
  • Hmm, ok, although a list as long as a password is probably not that terribly wasteful :-) – Stefan Pochmann Aug 25 '17 at 09:57
  • @Stefan a very fair point :) – Jon Clements Aug 25 '17 at 09:57
  • I wanted to do it the simplest and most readable way possible, this seemed like the right thing for me. I was thinking about using Regex to make it simpler but your way is fantastic – Muffinator Aug 25 '17 at 09:58
  • Surprisingly, this is *slower* than [pure sets solution](https://stackoverflow.com/a/45878517/244297) (just did a check using `%timeit` in ipython). – Eugene Yarmash Aug 25 '17 at 20:10
1

You have some problems with your checking logic. First of all you re-set your control variables before the while condition could check them. Furthermore you used and to test if all conditions are true, while or is what you should be using. If you want to keep the basic structure of your code, the best way to go is to use only True in your while statement and then break the loop once all conditions are fulfilled. Of course, as shown in the other answers, there are much more compact ways to check that all conditions are fulfilled.

passwd = raw_input("enter your password: ")
upper = 0
lower = 0 
number = 0
while True:
    for x in passwd:
        if x.isupper()==True:
            print 'upper'
            upper+=1
        elif x.islower()==True:
            print 'lower'
            lower+=1
        elif x.isalpha()==False:
            print 'num'
            number+=1
    if len(passwd) < 7 or upper <= 0 or lower <= 0 or number <= 0:
        print ('password not complex enough')
        passwd = raw_input('please enter password again ')
        upper = 0
        lower = 0
        number = 0
    else:
        break



print 'final password:', passwd

Output:

enter your password: pA
lower
upper
password not complex enough
please enter password again pA1ghlfx78
lower
upper
num
lower
lower
lower
lower
lower
num
num
final password: pA1ghlfx78
Thomas Kühn
  • 8,046
  • 3
  • 33
  • 52
  • 1
    Thanks! I knew there were better ways and more efficient ways to do it, the one i was thinking of was with Regex but wanted to keep it as simple to read as possible and i think this is the best way to do it – Muffinator Aug 25 '17 at 09:41
1

If a password must consist of ASCII letters and numbers only, then the most efficient approach would be to use sets:

from string import ascii_lowercase, ascii_uppercase, digits

def is_valid(password):
    if len(password) < 7:
        return False
    password_set = set(password)
    return not any(password_set.isdisjoint(x)
                   for x in (ascii_lowercase, ascii_uppercase, digits))

If unicode characters are allowed, you could use any():

 def is_valid(password):
     return (len(password) >= 7 and
             any(c.islower() for c in password) and
             any(c.isupper() for c in password) and
             any(c.isdigit() for c in password))
Eugene Yarmash
  • 119,667
  • 33
  • 277
  • 336
-1

May this is helpful for you.I use sum(1 for c in the string if c.islower()) to find the total number of lowercase. and same way to find the total number of uppercase and digit.len(string) to find length of string.

string=input()
if ((sum(1 for c in string if c.islower())>=1) and (sum(1 for c in string if 
c.isupper())>=1) and (sum(1 for c in string if c.isdigit())>=1) and (len(string)>=7)):
    print("Password is complex")
else:
    print ("Password not complex enough")
Rohit-Pandey
  • 1,684
  • 14
  • 22