Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add optional regex to enforce password complexity #1

Merged
merged 1 commit into from
Jul 4, 2014

Conversation

dsamojlenko
Copy link
Contributor

As discussed, with a minor change: instead of adding an additional parameter for the regex, I made the third parameter generic (pass_req) and run password validation based on type. This keeps things backwards-compatible without cluttering up the method call.

  • if pass_req is a string, assume it's a regex and test the password against that;
  • if above fails with a TypeError, pass_req is an integer so we test for password length only;

Also as discussed, I set the default password requirement to minimum length 6.

lirazsiri added a commit that referenced this pull request Jul 4, 2014
Add optional regex to enforce password complexity
@lirazsiri lirazsiri merged commit 1364d30 into turnkeylinux:master Jul 4, 2014
@lirazsiri
Copy link
Member

I like that you got this down to one optional extra argument. I would make the type check explicit rather than relying on an exception from re.match though as that would make the logic a bit easier to understand.

So if it's a number, treat it like a password length, if it's a string, treat it like a regular expression.

if isinstance(pass_req, int):
  print "pass_req is a number"
else:
  print "pass_req is a string"

@dsamojlenko dsamojlenko deleted the patch-1 branch July 15, 2014 19:24
@dsamojlenko
Copy link
Contributor Author

I originally was approaching the condition that way, but came across some discussions about why you shouldn't check for type in python. Being new to the language, maybe these discussions were just purists or whatever, but the arguments seemed compelling.. for example, http://stackoverflow.com/questions/3501382/checking-whether-a-variable-is-an-integer-or-not

I will defer to your judgement and could submit another pull request - what do you think?

@lirazsiri
Copy link
Member

Don't worry. We can use isinstance safely. Not only that but it's the
right thing to do.

I understand why you would be apprehensive about doing that
especially given that the answer on stackoverflow is subtle. It suggests
you don't hardwire type in order to allow subclasses of that type to
still work.

It assumes you realize there's a difference between checking whether a
variable is a certain type and checking whether it that type or any
subclass of that type - which is what isinstance does.

In any case, in this use case all of these general rules of thumb are
kind of frivolous anyway because we don't need to preserve the
flexibility to accept any argument type we just want to know whether
someone is passing a number or a string as an option into our function
and do that in a more readable way than catching an exception.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants