5
\$\begingroup\$

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._pending_write = True
        if file is not None:
            with open(file, "a") as f:
                f.write("")  # To make sure it's a valid file
        self.file = file  # Can be None
        self.case_sensitive = False if case_sensitive is None else not \
                              not case_sensitive

    @classmethod
    def get_from_file(cls, file, username, case_sensitive=None):
        """Gets username and password from provided file. Returns None."""
        case_sensitive = False if case_sensitive is None else not not \
                         case_sensitive
        i = 0
        salt = False
        with open(file) as f:
            lines = "".join(f.readlines()).split("\n")
        for line in lines:
            i += 1
            if not not salt:
                password = hashlib.md5(line.encode())
                break
            if i:
                if line[:-64] == username:
                    salt = a[-64:]
        return cls(password, username, file, case_sensitive, salt)

    def set_file(self, new_file):
        """Sets the file. Returns None."""
        with open(new_file, "a") as f:
            f.write("")
        self.file = new_file

    def set_username(self, new_username):
        """Sets username. Returns None"""
        if not self._pending_write:
            self.previous_username = self.username
            self.username = new_username
        else:
            raise Exception("Pending write to file")

    def set_password(self, new_password):
        """Sets password. Returns None"""
        try:
            new_password = new_password.encode()
        except AttributeError:
            password = password + self.salt.encode()
        self.password = hashlib.md5(new_password)
        del new_password
        self.password_digest = str(self.password.digest())[2:-1]

    def set_case_sensitivity(self, new_case_sensitivity):
        """Sets case-sensitivity. Returns None"""
        self.case_sensitive = not not new_case_sensitivity

    def write_to_file(self):
        """Updates file with password.
        If username exsists in file, updates passwords.
        else, creates 2 new lines in the format:
        [username][salt]
        [password]
        """
        if None in (self.file, self.password, self.username):
            raise ValueError("Must have username, password and file")
        if self.username == "":
            raise ValueError('Username cannot be "" (blank)')
        with open(self.file) as f:
            f.seek(0, 0)
            lines = "".join(f.readlines()).split("\n")
        lines = ["", ""] if lines == [""] else lines
        passwords = lines[1::2]
        usernames = lines[::2]
        if not self.case_sensitive:
            usernames = [username.lower() for username in usernames]
            lines = [lines[line].lower() if not line % 2 else lines[line] for line in range(len(lines))]
        for n in range(len(usernames)):
            if usernames[n][:-64] == self._previous_username:
                usernames[n] = self.username + self.salt
                passwords[n] = self.password_digest
                break
        new_lines = []
        for n in range(len(usernames)):
            new_lines.append(usernames[n])
            new_lines.append(passwords[n])
        with open(self.file, "w") as f:
            f.write("\n".join(new_lines))

    def check(self, password):
        """Checks if a string is the original encoded password."""
        try:
            password = (password + self.salt).encode()
        except AttributeError:
            password = password + self.salt.encode()
        return str(hashlib.md5(password).digest())[2:-1] == self.password_digest


class Database:
    """A database is a collection of user classes.
    The union of these users is the file, a "database\""""
    def __init__(self, file, case_sensitive=None):
        self.case_sensitive = False if case_sensitive is None else not not \
                              case_sensitive
        self.file = file
        self.users = {}
        with open(file) as f:
            lines = "".join(f.readlines()).split("\n")
        if lines == [""]:
            return
        for n in range(0, len(lines), 2):
            self.users[lines[n][:-64]] = [hashlib.md5(lines[n + 1].encode()), lines[n][-64:]]
        for user in self.users:
            self.users[user] = User(self.users[user][0], user, file,
                                    case_sensitive, self.users[user][1])

    def get_user(self, username, default=None):
        """Returns a User() class assigned to a username.
        Returns default otherwise.
        """
        return self.users.get(username, default)

    def has_user(self, username):
        """Returns the exsistence of a username in the database."""
        return True if username in self.users else False

    def add_user(self, username, password, salt=None):
        """Adds a user, returns None.
        Parameter salt is only used if password given is a hash"""
        username = username.lower() if not self.case_sensitive else username
        if self.has_user(username):
            raise ValueError("User already exists.")
        self.users[username] = User(password, username, self.file,
                                    self.case_sensitive, salt)
        self.get_user(username).write_to_file()

    def remove_user(self, username):
        """Removes a user, their salt and their password from the dateabase file."""
        username = username.lower() if not self.case_sensitive else username
        user = self.get_user(username)
        if user is None:
            raise ValueError("User does not exsist.")
        with open(self.file) as f:

            lines = "".join(f.readlines()).split("\n")
        for n in range(0, len(lines), 2):
            if lines[n] == username:
                break
        new_lines = lines[:n] + lines[:n + 2]
        with open(self.file, "w") as f:
            f.seek(0, 0)
            f.write(new_lines)

    def clear(self):
        """Clears the database of users and blanks out its folder."""
        self.users = {}
        with open(self.file, "w") as f:
            return

    def check_password(self, username, password):
        """Returns True if the password is that of the username, else False"""
        username = username.lower() if not self.case_sensitive else username
        if self.has_user(username):
            return self.get_user(username).check(password)
        return False

I would like some feedback on how effective this code is, especially in the file I/O, as this is my first time working with files.

A test case of using the Database() class:

>>> db = Database("db.txt", True)
>>> db.clear()
>>> db.add_user("Reticality", "notMyPass")
>>> db.check_password("reticality", "123")
False
>>> db.check_password("reticality", "notMyPass")
False
>>> db.check_password("Reticality", "notMyPass")
True
>>> new_db = Database("new_db.txt")
>>> new_db.clear()
>>> db.add_user("retIcAlItY", "pass")
>>> db.check_password("Reticality", "pass")
True
>>> db.check_password("retIcAlItY", "pass")
True

The usernames / passwords are stored in a file, so you could restart it and not need to add the users every time.

\$\endgroup\$
4
\$\begingroup\$

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 (<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)

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 if username is None or case_sensitive else username.lower()

Counting - the pythonic way

You are trying to count iterations in :

    i = 0
    for line in lines:
        i += 1
        blabla

A builtin called enumerate exists just for this usage. You could write your code :

    for i, line in enumerate(lines, start=1):
        blabla

Pointless checks

In the same loop, you are checking if i. This condition will always be true as far as I can tell.

This leads me to my last point :

Try to write unit tests and check code coverage

\$\endgroup\$
  • \$\begingroup\$ Why the downvote? I'm ready to learn if I said something wrong. \$\endgroup\$ – SylvainD Oct 25 '15 at 20:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy