# Implementations and analysis of my python login system (~80 lines)

I wrote this login system in the evening and i would like to know what should I fix / change, if should I use classes and, about readability, if this code is OK. I'm a begginer in coding so made this small project only for exercising, but good to know wether if i'm things doing right or wrong.

import os.path

credentials = []
database_path = "C:\\Users\\filip\\PycharmProjects\\database"
database = os.listdir(database_path)

def initialize():
if choose_option == 1:
elif choose_option == 2:
elif choose_option == 0:
exit('Exiting...')
else:
print("You should enter '0', '1', or '2'.")
initialize()

else:
print('Username too short (min 8 characters)\n\n')
else:

else:
print('Password does not match with confirmation\n\n')
else:
print('Password too short (min 8 characters)\n\n')

credentials_file = os.path.join(database_path, f'{credentials[0]}.txt')
file = open(credentials_file, 'w')
file.write(credentials[1])
file.close()
print('Account created successfully\n\n')
initialize()

database = os.listdir(database_path) #Refreshes the database, so if you create an account you can login without having to run the program again
for file in database:
file = open(f"C:\\Users\\filip\\PycharmProjects\\database\\{file}", 'r')
print('WELCOME!')
else:
else:

if __name__ == '__main__':
initialize()


Also, if you remember, please tell me in the comments simple projects that you did when you were learning, so I can do it too. ps: I know this login system is not secure at all but maybe later I could do some encryptation on those credencials...

• I made a program that could score a Scrabble game, very fun indeed. Aug 11, 2020 at 0:13
• did it have an real interface? Aug 11, 2020 at 0:20
• Did you test this? Does it work?
– Mast
Aug 11, 2020 at 6:52
• Minesweeper is a great starter Aug 11, 2020 at 8:40
• @filip augusto yes it was a CLI interface Aug 12, 2020 at 7:44

When entering passwords, you may want to opt for using the getpass library. It allows you (if the console supports it) to hide passwords as they're being typed. You'd use getpass.getpass just as you are using input to ask for the password. The only difference is, the password won't show in the console as it's typed.

You're using recursion in get_password in order to ask the user again. This isn't a good idea. If the user fails too many times, your program can crash. Just use a loop:

def get_password():
while True:
return
else:
print('Password does not match with confirmation\n\n')
else:
print('Password too short (min 8 characters)\n\n')


I'm looping until correct input is entered, then I'm just returning from within the infinite loop.

And the same goes for the rest of your functions. Recursion is great when used properly, but it carries more problems than most other looping methods. Use it in cases where it's appropriate (like when the problem itself is recursive), not just for general looping.

add_credentials would be safer if it used a with block to close the file for you:

def add_credentials_to_database():
credentials_file = os.path.join(database_path, f'{credentials[0]}.txt')
with open(credentials_file, 'w') as file:
file.write(credentials[1])
print('Account created successfully\n\n')
initialize()


Now, even if an error were to happen half-way through the function, there's a greater chance that the file would be closed. Making a practice of using with also prevents you from forgetting to add a call to file.close().

At the top you have:

credentials = []
database_path = "C:\\Users\\filip\\PycharmProjects\\database"
database = os.listdir(database_path)


A few things to note:

• credentials shouldn't be a global here. Global mutable states (which credentials is) tend to complicate code and make it harder to understand. In this set up, I would probably make credentials a local variable of initialize, have get_password and the like to return the data instead of adding it to the list directly, and manually pass that data to add_credentials_to_database.

• database_path is a module-level constant, and as such should be in upper-case like DATABASE_PATH according to PEP8.

• Arguably database is also (effectively) constant, so it should also be in upper-case.

In initialize, I would get rid of the int conversion of choose_option. All the call to int is doing is allowing for a ValueError to be raised if the user enters in a dumb menu option like "a". Just use the raw string value and compare against it:

choose_option = input('0. Exit\n1. Login\n2. Sign up\n\nOption: ')
if choose_option == "1":
. . .


If you actually needed it as an integer to do math on it or something, it would be different. It's simply an "indicator value" though, so the type of choose_option doesn't really matter, and keeping it as a string has less problems.

And my favorite projects that I still do whenever I learn a new language are:

• A naïve, bruteforce prime number finder (and then maybe a sieve). Fairly basic.
• Conway's Game of Life. Harder, and has the potential to turn into a large project.
• A Mandelbrot Set image generator. Even harder, and usually becomes a larger project.
• Thank you so much this was very usefull, tomorrow i'll applicate the changes you suggested!! Then I will post it Aug 11, 2020 at 2:04
• @filipaugusto No problem. And just note, code in questions can't be altered once posted. If you'd like a review of the new code, it needs to be as a separate question. Aug 11, 2020 at 2:20
• Ok, but could you do a really quick look at it? would be nice Aug 11, 2020 at 10:14
• @filipaugusto If you post it as a separate question, I should have time to review it later. Aug 11, 2020 at 13:39

As you mentioned, as-is the system is not at all secure. However, it is easy to hash passwords properly, and this is a great opportunity to learn the concept.

The idea of password hashing is to add a random salt, which makes it harder to use precomputed tables for breaking the password, and a one-way function which can only be "encrypted", but not "decrypted". The output of the one-way function is stored. When the user inputs a new password, the one-way function is applied again and the output is compared to the stored value. There is no way to decrypt the password without first knowing it.

One of the best current password hashing schemes is bcrypt. For Python, this is provided by the bcrypt module, which has to be installed. On Linux distributions, it is usually available in package python3-bcrypt. On Windows and Mac, you can install it with pip install bcrypt.

There is a good tutorial on bcrypt usage here. When applied to your code, you'd generate the salt and compute hash when user first provides the password:

            credentials.append(password)


becomes:

            password_utf8 = password.encode('utf-8') # Convert to UTF-8 for hashing
salt = bcrypt.gensalt() # Generate random salt
hash = bcrypt.hashpw(password_utf8, salt) # Apply one-way function
credentials.append(hash) # Store the hash


And when verifying, use bcrypt.checkpw():

if enter_password == file.read():


becomes:

if bcrypt.checkpw(enter_password, file.read()):


If you then look into the file, you'll see a string like $2b$12\$viuQE9cX.ZCoqZewzRqJz.Ae76yYRH.4fkOKe9CJPl3ssm6n50AiC, which is the hashed password.

• That just added a whole new level of security, didn't know that was that easy. Also, about storing credentials, do you know a more efficient and safe way of doing it? Because seems that creating a .txt file than writing the password on it is not good. When I was first writing the project, I just improvised a way to store, and that was what came to my mind. Aug 13, 2020 at 10:51
• @filipaugusto For scalability of storing more information (multiple users, usernames, access rights etc.), one can consider a range of formats from JSON to a sqlite database. But the security aspect will remain the same - whoever can read the file, can read the stored data, and there is no simple way to avoid that. In real systems the solution is to restrict access to the file with file permissions and system-level user accounts.
– jpa
Aug 13, 2020 at 12:37
• Ok I got it, so i'll keep it the same Aug 14, 2020 at 10:37

You loop over the database when it isn't necessary:

    if f'{enter_login}.txt' in database:
for file in database:
file = open(f"C:\\Users\\filip\\PycharmProjects\\database\\{file}", 'r')
print('WELCOME!')
else:
else:


can become:

    if f'{enter_login}.txt' in database:
print('WELCOME!')
else:
else:


The way you had it would probably only get slow if you have hundreds or thousands of users, but it's good to be aware of when you're doing unnecessary work.

I also recommend using pathlib instead of manually manipulating strings that represent file paths.

Here's what that would look like: (incorporating Carcigenicate's answer)

from pathlib import Path
from getpass import getpass

credentials = []
DATABASE_PATH = Path("C:\\Users\\filip\\PycharmProjects\\database")

def initialize():
while True:
if choose_option == '1':
return
elif choose_option == '2':

elif choose_option == '0':
exit('Exiting...')
return
else:
print("You should enter '0', '1', or '2'.")

while True:
else:
print('Username too short (min 8 characters)\n\n')

while True:
else:
print('Password does not match with confirmation\n\n')
else:
print('Password too short (min 8 characters)\n\n')

print('Account created successfully\n\n')

while True:
print('WELCOME!')
return
else:

• As you mentioned path, if I were to make this .py into a .exe, how would I store their credentials if I don't know their path, like, In the code above the program goes through \Users\filip, what if I don't know their user? Aug 13, 2020 at 10:57