2
\$\begingroup\$

This is a finished login system where 2 players have to enter their username and password to proceed onto the game. The usernames and passwords are stored in a text file in this formatusername: (username) password: (password). If one of the players enter their details in wrong it will loop it 3 times and if it is sill not correct they have to restart the code to try again.

import sys

counter = 0 
check_failed = True
while check_failed:
    print("Could player 1 enter their username and password")
    username1=input("Please enter your username ")
    password=input("Please enter your password ")
    with open("accountfile.txt","r") as username_finder:
        for line in username_finder:
            if ("username: " + username1 + " password: " + password) == line.strip():  
                print("you are logged in")
                check_failed = False
                check_failed = True
                while check_failed:
                    print("Could player 2 enter their username and password")
                    username2=input("Please enter your username ")
                    password=input("Please enter your password ")
                    with open("accountfile.txt","r") as username_finder:
                        for line in username_finder:
                            if ("username: " + username2 + " password: " + password) == line.strip():
                                print("you are logged in")
                                check_failed = False

                        else:
                            print("Sorry, this username or password does not exist please try again")
                            counter = counter + 1
                            if counter == 3:
                                print("----------------------------------------------------")
                                print("You have been locked out please restart to try again")
                                sys.exit()
        else:
            print("Sorry, this username or password does not exist please try again")
            counter = counter + 1
            if counter == 3:
                print("----------------------------------------------------")
                print("You have been locked out please restart to try again")
                sys.exit()

As I said this is a finished Login system and I just want suggestions on how I can improve this Login system.

\$\endgroup\$
2
\$\begingroup\$

One guiding principle in software development is Don't Repeat Yourself (DRY). Your code for player 1 and 2 are almost identical, so you should extract them into a login function:

import crypt

class AuthenticationError(Exception):
    pass

def encrypt(word, salt):
    # use some one-way hashing function here
    return crypt.crypt(word, salt)

def login(users, max_tries=3):
    for i in range(max_tries):
        username = input("Please enter your username ")
        password = input("Please enter your password ")
        if username in users and encrypt(password, username) == users[username]:
            print("You are logged in as", username)
            return username
        else:
            print(f"Login failed. You have {max_tries - i - 1} tries left.")
    raise AuthenticationError(f"You have been locked out please restart to try again.")

This uses a hashing function, like @ChadG suggested in their answer, with a salt to prevent rainbow table attacks.

By raising an Exception (here a custom one, to give it a nice name), the execution of the program is automatically halted, so no need to explicitly do sys.exit.

This code assumes that you have read the users file before into a dictionary (only need to do it once!), something like this:

import re

def get_users(file_name):
    line_fmt = re.compile(r'username: (.*) password: (.*)')
    with open(file_name) as f:
        return dict(line_fmt.match(line.strip()).groups() for line in f)

Or, this slightly expanded version which ensures that the entries not conforming to the format are skipped (including any trailing newline):

def get_users(file_name):
    line_fmt = re.compile(r'username: (.*) password: (.*)')
    users = {}
    with open(file_name) as f:
        for line in f:
            try:
                name, password = line_fmt.match(line.strip()).groups()
                users[name] = password
            except AttributeError:
                print("Malformed line:", line)
        return users

Use it like this:

if __name__ == "__main__":
    registered_users = get_users("accountfile.txt")

    print("Could player 1 enter their username and password")
    player1 = login(registered_users)

    print("Could player 2 enter their username and password")
    player2 = login(registered_users)

    # start dice game

This uses a if __name__ == "__main__": guard to allow importing from this script from another script.

\$\endgroup\$
  • 1
    \$\begingroup\$ the only thing is that this is project that I'm doing at school and their python don't have the module crypt and I can't install it but other than that everthing else you provided was very helpful! \$\endgroup\$ – colkat406 Dec 5 '18 at 18:11
  • \$\begingroup\$ @colkat406: That's why I put it into its own function, so you can easily swap out the implementation. \$\endgroup\$ – Graipher Dec 5 '18 at 18:13
  • \$\begingroup\$ how would use the code you wrote in my code because I'm trying really hard to do it but just not working for me \$\endgroup\$ – colkat406 Dec 5 '18 at 18:24
  • \$\begingroup\$ @colkat406: Which part are you having trouble with? What does "not working" mean? It should work if you paste it all into one script and run it. Unless I made a mistake somewhere... \$\endgroup\$ – Graipher Dec 5 '18 at 18:25
  • \$\begingroup\$ No its not you its me and I'm having trouble puting your code into mine \$\endgroup\$ – colkat406 Dec 5 '18 at 18:27
3
\$\begingroup\$

You don't need to set check_failed to False before you set it to True; you can remove that line. And passwords should never be stored in plain text: you should store the hashed value, then hash what the user enters and compare it to what you have stored.

\$\endgroup\$
  • \$\begingroup\$ Thanks for suggesting this also when ever player 1 and player 2 enter their details correctly it print "you are logged in" but it also prints "Sorry, this username or password does not exist please try again" twice for some reason afterwards. do you think you can help? \$\endgroup\$ – colkat406 Dec 5 '18 at 17:00
  • \$\begingroup\$ That can be printed in two places, change one of them slightly so you know which case is printing it. That should help narrow down the problem. \$\endgroup\$ – Chad G Dec 5 '18 at 17:05
  • \$\begingroup\$ What do you mean by change one of them slightly? \$\endgroup\$ – colkat406 Dec 5 '18 at 17:07
  • \$\begingroup\$ You have two places in you code with this line print("Sorry, this username or password does not exist please try again") You dont know which one of them is actually printing that line. So change one to print("SORRY, this username or password does not exist please try again"). This way you know which one is causing your problem, and you can then trace why its happening. \$\endgroup\$ – Chad G Dec 5 '18 at 17:10
  • 1
    \$\begingroup\$ You are also opening the file twice which isnt needed. You should probably open the file, parse the user names and passwords into a dict and then start asking the users for their information. \$\endgroup\$ – Chad G Dec 5 '18 at 17:25
1
\$\begingroup\$

The code you have written for player two is just the same for player one as they both perform the same end result(log in a player). So instead I would recommend you create a function in which logs in a player and then just use that function twice instead of having to type out the whole code twice. This makes it much simpler and easier.

Also as a side note you could store the users in a dictionary:

users = {
    'Joe': 'juk725',
    'Mat': 'axr3',
    'Th3_j0k3r': 'bl4z3',
    'ag4r-j3lly': 'Micr0b3'
}

--> and then create a pickle file(on a separate program) where these users are stored:

import pickle

users = {
    'Joe': 'juk725',
    'Mat': 'axr3',
    'Th3_j0k3r': 'bl4z3',
    'ag4r-j3lly': 'Micr0b3'
}

with open('users.pickle', 'wb') as afile:
    # Pickle the 'data' dictionary using the highest protocol available.
    pickle.dump(users, f, pickle.HIGHEST_PROTOCOL)

--> then create a definition of users in you login system:

with open('users.pickle', 'rb') as a_file:
    users = pickle.load(a_file)

This addition will allow you to give new users access to signup / register as well as providing an easy way to store users.

[ ocr gcse comp science right ;) ]

login_or_signup = input('do you want to login or signup? ')

# sign up (creating new account)
if login_or_signup.lower() == "signup":

    create_username = input("enter a new username: ")
    create_password = input("enter a new password (Your password cannot be the same as your username !!!!!!!): ")

    if create_password in users:
        create_password = input("password taken re-enter: ")

    users[create_username] = create_password
\$\endgroup\$
  • \$\begingroup\$ do i put the users in a seperate python promgram \$\endgroup\$ – colkat406 Dec 16 '18 at 18:25
  • \$\begingroup\$ yep so open a new program and type the second block of code in \$\endgroup\$ – Krishna Dec 16 '18 at 18:26
  • \$\begingroup\$ its saying that in the second block of code that ' f ' is not defined \$\endgroup\$ – colkat406 Dec 16 '18 at 18:36
  • \$\begingroup\$ and the first one at the beginning of your login system and the last at the end of your login system \$\endgroup\$ – Krishna Dec 16 '18 at 18:36
  • 1
    \$\begingroup\$ check the answer above at the very bottom \$\endgroup\$ – Krishna Dec 19 '18 at 10:26

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.