Account creation and log-in system

Question

I've been learning Python for about 1 - 1 1/2 weeks now and just finished my first small project to help solidify what I've learnt. It's a very basic (bad) login / create account system that I run in PowerShell but just wanted someone feedback and thoughts as to how I'm getting on in regards to how long I've been learning.

import os.path


def app(user_name):
    print(user_name)

def create():

    exists = os.path.exists("C:/Users/Tom/Desktop/test/login_project/account_info.txt")

    if not exists:
        f = open("account_info.txt", "w")
        f.close()

    creating_username = True
    creating_password = True

    while creating_username:
        user_name = input("\nEnter a username: ")

        f = open("account_info.txt", "r")
        file_contents = f.read()

        if f"'{user_name}'" in file_contents:
            print("\nUsername already in use.")
        else:
            creating_username = False

    email = input("Enter an e-mail address: ")

    while creating_password:

        password = input("Enter a password: ")
        password_confirm = input("Confirm password: ")

        if password == password_confirm:

            info_list = [user_name, password, email]

            f = open("account_info.txt", "a+")
            f.write(f"{info_list}\r\n")
            f.close()

            print("\nAccount Created.\n")

            creating_password = False

            choosing()

        else:
            print("\nPasswords did not match.\n")

def login():

    exists = os.path.exists("C:/Users/Tom/Desktop/test/login_project/account_info.txt")
    entering_info = True

    while entering_info:

        user_name = input("\nEnter username: ")
        password = input("Enter password: ")

        if not exists:
            entering_info = False
            print("\nError: [0] account(s) found in database\n")
            choosing()

        else:
            f = open("account_info.txt", "r")
            file_contents = f.read()

            f = open("account_info.txt", "r")
            list_ = [char for char in f.readlines() if char != "\n"]

            counter_one = 1
            counter_two = 1

            for i in list_:
                if f"'{user_name}'" in i:
                    break
                counter_one += 1

            for y in list_:
                if f"'{password}'" in y:
                    break
                counter_two += 1

            f.close()

            if f"'{user_name}'" in file_contents:
                if f"'{password}'" in file_contents and counter_one == counter_two:
                    print("Login Successful")
                    entering_info = False
                    app(user_name)
                else:
                    print("\nPassword incorrect.")
            else:
                print("\nUsername incorrect.")

def choosing():

    main_choosing = True
    main_choice = ""

    print("Please continue to login or create an account.")
    print("(Type 'login' to login, or 'create' to create an account).")
    print("('exit' to close program)\n")

    while main_choosing:

        main_choice = input("- ")

        if main_choice == "login" or main_choice == "create" or main_choice == "exit":

            if main_choice == "login":
                main_choosing = False
                login()
            elif main_choice == "create":
                main_choosing = False
                create()
            else:
                print("\nClosing...\nClosed.")
                break

        else:
            print("\nEnter a valid command.\n")


print("\nWelcome.\n")
choosing()

Answer 1

Welcome to code review, and good job for your first program I suppose.

Style

• Docstrings: Python documentation strings (or docstrings) provide a convenient way of associating documentation with Python modules, functions, classes, and methods. As you can see, even for a relatively simple function, documenting using comments quickly makes it unpleasant and difficult to read. So, to solve this, the docstring was introduced. You should include docstrings to the functions you created indicating what they return and some type hints if necessary.

  def app(user_name): 
      """Print user name."""
  
  def create():
      """Create new account."""
  

• Descriptive variable names: The name app(user_name) can be changed to print_username() since it's a good practice to create
  descriptive variable names which improve readability for you and for other people using your code. Same goes for create() that can be
  create_account()

• PEP0008 The official Python style guide https://www.python.org/dev/peps/pep-0008/ I suggest you check this, will help you write more pythonic code.

• Blank lines: Your functions contain many blank lines and according to PEP0008 you should use blank lines sparingly: Surround top-level function and class definitions with two blank lines. Method definitions inside a class are surrounded by a single blank line. Extra blank lines may be used (sparingly) to separate groups of related functions. Blank lines may be omitted between a bunch of related one-liners (e.g. a set of dummy implementations).

Code

The code is a bit long and can be shortened. Storing all usernames and passwords and everything else in the same file I think is a bad idea and is very unreliable because all things would be mixed up and you don't know which is what, I suggest you create a .txt file for each user and make the file name same as the user name so the program checks the folder and if the user is not found, it prompts the user accordingly. And still this is not the best way to do it however it is better than mixing all things up.

exists = os.path.exists("C:/Users/Tom/Desktop/test/login_project/account_info.txt")

this line should be cleaned up or replaced by input() (If i'm using your code it's very obvious that the path will not exist and the condition if not exists: will always evaluate to True.

info_list = [user_name, password, email]

The variable user_name is referenced before assignment, you should define your variables at the top of the function body to avoid such problems.

unused variable(line 103) you should clean such things up upon completion.

main_choice = ""

Bug: This appears after creating a new account and trying to login. - Error: [0] account(s) found in database

These are unecessary variables:

creating_username = True
creating_password = True

followed by while creating_username: and while_creating_password:

which can be written:

while True:
    # block

and same goes to entering_info = True and main_choosing = True in other functions.

else:
    creating_username = False

can be replaced with break in the following way:

else:
    break

instead of manually closing the file you just opened, you may use with & as context managers that automatically carry the opening and closing activities for you in the following way and the file will be closed automatically.

f = open("account_info.txt", "a+")
f.write(f"{info_list}\r\n")
f.close()

can be written:

with open('account_info.txt', 'a+') as data:
    # do things

line 69: the file was opened twice (I think you forgot to clean this up)

f = open("account_info.txt", "r")

file_contents = f.read()

f = open("account_info.txt", "r")
list_ = [char for char in f.readlines() if char != "\n"]

Descriptive variable names: I think this is the second time I mention this: a code is designed to be interpreted by machines and to be read and understood by human beings, try as much as you can to avoid ambiguous variable/method/function names.

for i in list_:
for y in list_:
counter_one = 1
counter_two = 1

what is i? what is y? what is counter_one what does it count? what is counter_two?

print("\nWelcome.\n")
choosing()

if __name__ == '__main__': guard to be used at the end of your script, allows your module to be imported by other modules without running the whole script.

This should be placed at the end of your script like the following:

if __name__ == '__main__':
    print('\nWelcome.\n')
    choosing()

I tried to simplify your program and here's what I came up with (And still this is not the proper way to do it in the real world, it's just for the sake of the example) and you might modify it if you want it to keep running and keep asking for input or whatever structure you want, you might consider this as an assignment:

import os


def create_account(path):
    """Create and save username to a Usernames folder."""
    if 'Usernames' not in os.listdir(path):
        os.mkdir('Usernames')
    os.chdir('Usernames')
    new_username = input('Enter new user name: ')
    if new_username + '.txt' in os.listdir(path + 'Usernames'):
        raise ValueError(f'Username {new_username} already exists.')
    new_password = input('Enter new password: ')
    confirm_pass = input('Confirm password: ')
    while new_password != confirm_pass:
        print('Passwords do not match!')
        new_password = input('Enter new password: ')
        confirm_pass = input('Confirm password: ')
    with open(new_username + '.txt', 'w') as new_user:
        new_user.write(new_username + ' ')
        new_user.write(new_password)
    print(f'Account created successfully.')


def login(path):
    """Login using user name and password."""
    if 'Usernames' in os.listdir(path):
        os.chdir('Usernames')
        username = input('Enter Username or q to quit: ')
        while username + '.txt' not in os.listdir(path + 'Usernames') and username != 'q':
            print(f'Username {username} does not exist.')
            username = input('Enter Username or q to quit: ')
        if username == 'q':
            exit(0)
        password = input('Enter password: ')
        if username + '.txt' in os.listdir(path + 'Usernames'):
            user, valid_pass = open(username + '.txt').read().rstrip().split()
            while password != valid_pass:
                print('Invalid password.')
                password = input('Enter password: ')
            print('Login successful.')
    else:
        print('Database not found!')
        exit(1)


def start_login(path):
    """Run login interactively."""
    print('Welcome to my program.')
    user_mode = input('Enter L for login or C for creating a new account or Q to exit.').lower()
    while user_mode not in 'lcq':
        print('Invalid command')
        user_mode = input('Enter L for login or C for creating a new account or Q to exit.').lower()
    if user_mode == 'l':
        login(path)
    if user_mode == 'c':
        create_account(path)
    if user_mode == 'q':
        print('Thank you for using my program')
        exit(0)


if __name__ == '__main__':
    database_path = input('Enter database path: ').rstrip()
    while not os.path.exists(database_path):
        print('Invalid path')
        database_path = input('Enter database path: ').rstrip()
    start_login(database_path)

Answer 2

Your app function is just the same as the print function. So there is no need for it to exist at the moment.

When working with files you should always use the with keyword to ensure the file is properly closed (even in the case of exceptions).

The pathlib module has a Path object which makes handling file paths a lot easier.

You are doing a lot of unnecessary stuff. To get rid of this I would change your file format to more accurately describe the data. You have usernames and each username has a password and an email. That sounds like a dictionary to me. An easy way to save a dictionary to a file is to do so in JSON format. There is a json module in the standard-library, which is very easy to use.

import json
from pathlib import Path

def get_users(file_name):
    with open(file_name) as f:
        try:
            return json.load(f)
        except json.JSONDecodeError:
            return {}

def add_user(file_name):
    users = get_users(file_name) if Path(file_name).exists() else {}

    while True:
        username = input("\nEnter a username: ")
        if username not in users:
            break
        print("\nUsername already in use.")

    email = input("Enter an e-mail address: ")

    while True:
        password = input("Enter a password: ")
        password_confirm = input("Confirm password: ")
        if password == password_confirm:
            break
        print("\nPasswords did not match.\n")

    users[username] = email, password

    with open(file_name, "w") as f:
        json.dump(users, f)

This could be improved further by using the getpass module. This way the password is not visible on screen while it is being typed. For actual security you would want to save the password not in plaintext, but this is left as an exercise.

The actual login code becomes a lot easier with this as well, because you don't need to see if the username string is contained in the file content (and the same for the password and then match the positions...), you just have a dictionary:

def login(file_name):
    users = get_users(file_name)
    if not users:
        print("\nError: 0 account(s) found in database\n")
        return

    user_name = input("\nEnter username: ")
    if user_name not in users:
        print("\nUsername incorrect.")
        return
    password = input("Enter password: ")
    if password != users[username][1]:
        print("\nPassword incorrect.")
        return
    print("Login Successful")
    return user_name, users[user_name]

The main code is mostly unchanged. I would put everything into an infinite while True loop to avoid hitting the stack limit at some point (a problem recursive code has in Python). I also added a if __name__ == "__main__": guard to allow importing from this script without running the code. The actual file path is only needed at this point, everything else has it as a parameter.

DB_PATH = Path("C:/Users/Tom/Desktop/test/login_project/account_info.txt")

def main(file_name=DB_PATH):
    print("\nWelcome.\n")
    commands = {"create", "login", "exit"}

    while True:
        print("Please continue to login or create an account.")
        print("(Type 'login' to login, or 'create' to create an account).")
        print("('exit' to close program)\n")
        user_input = input("- ")
        if user_input not in commands:
            print("\nEnter a valid command.\n")
            continue
        if user_input == "create":
            add_user(file_name)
        elif user_input == "login":
            user = login(file_name)
            if user:
                print(user[0])
        else:
            print("\nClosing...\nClosed.")
            return

if __name__ == "__main__":
    main()

If you wanted to take this further, I would probably make the user database a class that automatically saves to disk on modifications and reads when needed. Something like this.

Answer 3

I see two race conditions.

1. When you write the file, two processes may write it at the same time and one entry will be lost.
2. Between reading the user list and adding a new user, i.e. in the time when the user chooses a password, another process could add the same user, so the user is added twice even when you checked that the user is not in the list when reading the file before.

Answer 4

The stuff with counter_one and counter_two is rather oblique, and it seems that it would break if someone were to choose the same password as someone else (or username, but you do have safeguards against that). If I understand what you're trying to do correctly, you could just do

account_match = any([(f"'{user_name}'" in line) and (f"'{password}'" in line) for line in f.readlines()])

I think there should be more safeguards against infinite loops. For instance:

def create(max_attempts = default_max_attempts,
           max_time = default_max_time):
    exists = os.path.exists("C:/Users/Tom/Desktop/test/login_project/account_info.txt")
    if not exists:
        f = open("account_info.txt", "w")
        f.close()
    start_time = time.time()
    while attempts < max_attempts:
        if time.time()-start_time() > max_time:
            print("Max time exceeded. Exiting account creation.")
            return "Max time exceeded."  
        user_name = input("\nEnter a username: ")
        with  open("account_info.txt", "r") as f:
            file_contents = f.read()
            if f"'{user_name}'" in file_contents:
                user_choice = input("\nUsername already in use. Press X to exit, any other key to continue.")
                if user_choice.upper() == 'X':
                    confirmation = input("Exit account creation? Y/N")
                    if confirmation.upper() == "Y":
                        print("Exiting account creation.")                        
                        return "User exited account creation."
                attempts +=1
                continue
            break
    if attempts == max_attempts:
        return "Max attempts reached."          
    while attempts < max_attempts:
        if time.time()-start_time() > max_time:
            print("Max time exceeded. Exiting account creation.")
            return "Max time exceeded."
        password = input("Enter a password: ")
        password_confirm = input("Confirm password: ")
        if password == password_confirm:
            info_list = [user_name, password, email]
            with open("account_info.txt", "a+") as f:
                f.write(f"{info_list}\r\n")
                f.close()
            print("\nAccount Created.\n")
            choosing()
            break
        else:
            user_choice = input("\nPasswords did not match. Press X to exit, any other key to continue.")
            if user_choice.upper() == 'X':
                confirmation = input("Exit account creation? Y/N")
                if confirmation.upper() == "Y":
                    print("Exiting account creation.")                        
                    return "User exited account creation."
            attempts +=1
            continue
    if attempts == max_attempts:
        return "Max attempts reached."