patternpythonMinor
Simple encryption based on MD5
Viewed 0 times
encryptionsimplemd5based
Problem
I made a Python 3(.4.2) program that only adds classes and is meant to be imported. It allows the creation of files that store usernames and hashed passwords. It uses salting as well. It also can easily check if any given string is the original password (through the
``
def generate_salt(length=64):
"""Returns a salt of length length.
This is from all lowercase printable not-whitespace characters.
"""
return "".join((random.choice(_POSSIBLE_CHARS) for n in range(length)))
class User:
"""Stores the username/password for one user
and the logic to write it to a file.
"""
def __init__(self, password=None, username=None, file=None,
case_sensitive=None, salt=None):
self.salt = generate_salt() if salt is None else salt
if password is not None and not isinstance(password,
hashlib._hashlib.HASH):
try:
password = (password + self.salt).encode()
except AttributeError:
password = password + self.salt.encode()
self.password = hashlib.md5(password)
del password # Just in case
self.password_digest = str(self.password.digest())[2:-1]
elif isinstance(password, hashlib._hashlib.HASH):
self.password = password
self.password_digest = str(self.password.digest())[2:-1]
else:
self.password = None
self.password_digest = None
if username is not None:
self.username = username if case_sensitive else username.lower()
else:
self.username = None
self._previous_username = ""
self._
User.check() attribute of the User class.)``
"""A simple encryption module"""
__version__ = "1.0.0"
__all__ = ["generate_salt", "User", "Database"]
import hashlib
import random
_POSSIBLE_CHARS = ("0123456789abcdefghijklmnopqrstuvwxyz"
"!\"#$%&'()*+,-./:;?@[\\]^_{|}~")def generate_salt(length=64):
"""Returns a salt of length length.
This is from all lowercase printable not-whitespace characters.
"""
return "".join((random.choice(_POSSIBLE_CHARS) for n in range(length)))
class User:
"""Stores the username/password for one user
and the logic to write it to a file.
"""
def __init__(self, password=None, username=None, file=None,
case_sensitive=None, salt=None):
self.salt = generate_salt() if salt is None else salt
if password is not None and not isinstance(password,
hashlib._hashlib.HASH):
try:
password = (password + self.salt).encode()
except AttributeError:
password = password + self.salt.encode()
self.password = hashlib.md5(password)
del password # Just in case
self.password_digest = str(self.password.digest())[2:-1]
elif isinstance(password, hashlib._hashlib.HASH):
self.password = password
self.password_digest = str(self.password.digest())[2:-1]
else:
self.password = None
self.password_digest = None
if username is not None:
self.username = username if case_sensitive else username.lower()
else:
self.username = None
self._previous_username = ""
self._
Solution
Style
Your code looks good and is properly documented. Using various tools to check it, here is what we have.
With
So nothing very worrying.
With
Errors
Warnings
And
Now, there is quite a lot of code and I have a limited times so I'll just point out other things that could easily be improved.
Basic logic
By reorganising the code in :
You can make it much clearer. You could handle the
Don't repeat yourself
In the code above, a part of the logic is duplicated.
Similarly, you can rewrite :
by inversing conditions and replacing
Then, this is nothing but
```
self.username = username i
Your code looks good and is properly documented. Using various tools to check it, here is what we have.
With
pep8 :E127 continuation line over-indented for visual indent
E126 continuation line over-indented for hanging indent
E501 line too longSo nothing very worrying.
With
pylint :C:120, 0: Line too long (104/80) (line-too-long)
C:155, 0: Line too long (93/80) (line-too-long)
C:181, 0: Line too long (84/80) (line-too-long)
C: 1, 0: Missing module docstring (missing-docstring)
F: 5, 0: Unable to import 'hashlib' (invalid syntax (, line 6)) (import-error)
R: 19, 0: Too many instance attributes (9/7) (too-many-instance-attributes)
R: 23, 4: Too many arguments (6/5) (too-many-arguments)
W: 27,51: Access to a protected member _hashlib of a client class (protected-access)
W: 35,34: Access to a protected member _hashlib of a client class (protected-access)
C: 48,36: Invalid variable name "f" (invalid-name)
C: 61,27: Invalid variable name "f" (invalid-name)
E: 70,27: Undefined variable 'a' (undefined-variable)
C: 75,36: Invalid variable name "f" (invalid-name)
E: 92,23: Using variable 'password' before assignment (used-before-assignment)
C:112,32: Invalid variable name "f" (invalid-name)
C:121,12: Invalid variable name "n" (invalid-name)
C:127,12: Invalid variable name "n" (invalid-name)
C:130,37: Invalid variable name "f" (invalid-name)
W: 82,12: Attribute 'previous_username' defined outside __init__ (attribute-defined-outside-init)
C:150,27: Invalid variable name "f" (invalid-name)
C:154,12: Invalid variable name "n" (invalid-name)
C:186,32: Invalid variable name "f" (invalid-name)
C:189,12: Invalid variable name "n" (invalid-name)
W:192,27: Using possibly undefined loop variable 'n' (undefined-loop-variable)
W:192,39: Using possibly undefined loop variable 'n' (undefined-loop-variable)
C:193,37: Invalid variable name "f" (invalid-name)
C:200,37: Invalid variable name "f" (invalid-name)
W:200,37: Unused variable 'f' (unused-variable)Errors
Undefined variable are definitly something you should look at because there would lead to severe errors.Warnings
Unused variable are a good hint that something is probably not quite what you wanted to do.And
pyflakes leads to pretty similar results :crypto.py:35: undefined name 'password'
crypto.py:36: undefined name 'password'
crypto.py:70: undefined name 'a'
crypto.py:92: undefined name 'password'
crypto.py:92: local variable 'password' is assigned to but never used
crypto.py:200: local variable 'f' is assigned to but never usedNow, there is quite a lot of code and I have a limited times so I'll just point out other things that could easily be improved.
Basic logic
By reorganising the code in :
if password is not None and not isinstance(password,
hashlib._hashlib.HASH):
try:
password = (password + self.salt).encode()
except AttributeError:
password = password + self.salt.encode()
self.password = hashlib.md5(password)
del password # Just in case
self.password_digest = str(self.password.digest())[2:-1]
elif isinstance(password, hashlib._hashlib.HASH):
self.password = password
self.password_digest = str(self.password.digest())[2:-1]
else:
self.password = None
self.password_digest = NoneYou can make it much clearer. You could handle the
None case first.if password is None:
self.password = None
self.password_digest = None
elif isinstance(password, hashlib._hashlib.HASH):
self.password = password
self.password_digest = str(self.password.digest())[2:-1]
else:
try:
password = (password + self.salt).encode()
except AttributeError:
password = password + self.salt.encode()
self.password = hashlib.md5(password)
del password # Just in case
self.password_digest = str(self.password.digest())[2:-1]Don't repeat yourself
In the code above, a part of the logic is duplicated.
if password is None:
self.password_digest = None
else:
if not isinstance(password, hashlib._hashlib.HASH):
try:
password = (password + self.salt).encode()
except AttributeError:
password = password + self.salt.encode()
password = hashlib.md5(password)
self.password_digest = str(password.digest())[2:-1]
self.password = passwordSimilarly, you can rewrite :
if username is not None:
self.username = username if case_sensitive else username.lower()
else:
self.username = Noneby inversing conditions and replacing
None by username when it is the same:if username is None:
self.username = None
else:
self.username = username if case_sensitive else username.lower()Then, this is nothing but
```
self.username = username i
Code Snippets
E127 continuation line over-indented for visual indent
E126 continuation line over-indented for hanging indent
E501 line too longC:120, 0: Line too long (104/80) (line-too-long)
C:155, 0: Line too long (93/80) (line-too-long)
C:181, 0: Line too long (84/80) (line-too-long)
C: 1, 0: Missing module docstring (missing-docstring)
F: 5, 0: Unable to import 'hashlib' (invalid syntax (<string>, line 6)) (import-error)
R: 19, 0: Too many instance attributes (9/7) (too-many-instance-attributes)
R: 23, 4: Too many arguments (6/5) (too-many-arguments)
W: 27,51: Access to a protected member _hashlib of a client class (protected-access)
W: 35,34: Access to a protected member _hashlib of a client class (protected-access)
C: 48,36: Invalid variable name "f" (invalid-name)
C: 61,27: Invalid variable name "f" (invalid-name)
E: 70,27: Undefined variable 'a' (undefined-variable)
C: 75,36: Invalid variable name "f" (invalid-name)
E: 92,23: Using variable 'password' before assignment (used-before-assignment)
C:112,32: Invalid variable name "f" (invalid-name)
C:121,12: Invalid variable name "n" (invalid-name)
C:127,12: Invalid variable name "n" (invalid-name)
C:130,37: Invalid variable name "f" (invalid-name)
W: 82,12: Attribute 'previous_username' defined outside __init__ (attribute-defined-outside-init)
C:150,27: Invalid variable name "f" (invalid-name)
C:154,12: Invalid variable name "n" (invalid-name)
C:186,32: Invalid variable name "f" (invalid-name)
C:189,12: Invalid variable name "n" (invalid-name)
W:192,27: Using possibly undefined loop variable 'n' (undefined-loop-variable)
W:192,39: Using possibly undefined loop variable 'n' (undefined-loop-variable)
C:193,37: Invalid variable name "f" (invalid-name)
C:200,37: Invalid variable name "f" (invalid-name)
W:200,37: Unused variable 'f' (unused-variable)crypto.py:35: undefined name 'password'
crypto.py:36: undefined name 'password'
crypto.py:70: undefined name 'a'
crypto.py:92: undefined name 'password'
crypto.py:92: local variable 'password' is assigned to but never used
crypto.py:200: local variable 'f' is assigned to but never usedif password is not None and not isinstance(password,
hashlib._hashlib.HASH):
try:
password = (password + self.salt).encode()
except AttributeError:
password = password + self.salt.encode()
self.password = hashlib.md5(password)
del password # Just in case
self.password_digest = str(self.password.digest())[2:-1]
elif isinstance(password, hashlib._hashlib.HASH):
self.password = password
self.password_digest = str(self.password.digest())[2:-1]
else:
self.password = None
self.password_digest = Noneif password is None:
self.password = None
self.password_digest = None
elif isinstance(password, hashlib._hashlib.HASH):
self.password = password
self.password_digest = str(self.password.digest())[2:-1]
else:
try:
password = (password + self.salt).encode()
except AttributeError:
password = password + self.salt.encode()
self.password = hashlib.md5(password)
del password # Just in case
self.password_digest = str(self.password.digest())[2:-1]Context
StackExchange Code Review Q#78037, answer score: 4
Revisions (0)
No revisions yet.