HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Simple encryption based on MD5

Submitted by: @import:stackexchange-codereview··
0
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 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 pep8 :

E127 continuation line over-indented for visual indent
E126 continuation line over-indented for hanging indent
E501 line too long


So 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 used


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 :

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


You 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 = password


Similarly, you can rewrite :

if username is not None:
        self.username = username if case_sensitive else username.lower()
    else:
        self.username = None


by 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 long
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 (<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 used
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 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.