Skip to main content
added 381 characters in body
Source Link
Daniel
  • 4.6k
  • 2
  • 18
  • 40

Comments

Comments

Your code is horriblehard to read through, not so much because of how or what, but rather because of the excessive use of comments. Comments should addadd something to code, not say exactly whatrepeat the code says, or saytell the reader what the code already strongly implies.

Use hashes

Use hashes

Just hashing is very weak, but this is rather easy to implement. Also note that md5 is prone to hash collision attacks and should not be used anymore. If your platform supports it, use SHA3-256 Merely hashing is very weak, but this is rather easy to implement. Also note that md5 is prone to hash collision attacks and should not be used anymore. If your platform supports it, use SHA(3)-256.

The problem with this application is that if anybody gets to access the code, they can read the username/password and password in plaintext. You would be better off first hashing the passwords, then using the hashlib modulehashlib to check if the input matches. I've written an improved version of your code below, which shows youto show what I mean.:

from hashlib import md5
from getpass import getpass
import sys

print("Hello! Welcome to FaceSnap!") 

attempts = 0
check_username = '5945261a168e06a5b763cc5f4908b6b2'"5945261a168e06a5b763cc5f4908b6b2"
check_password = '48d6215903dff56238e52e8891380c8f'"48d6215903dff56238e52e8891380c8f"
# These hashes have been generated earlier on.
# This is not how you would go about storing usernames and passwords,
# but for the sake of simplicity, we'll do it like this.

while True: 
    print("\n")
    username = input("Username: ")
    password = getpass("Password: ")
    # Getpass will not echo input to the screen, so your password remains 
    # invisible
    print("\n")
    
    if attempts == 3:
        printsys.exit("Too many failed attempts.")
        exit()

    if md5(username.encode("utf-8")).hexdigest() == check_username:
        if md5(password.encode("utf-8")).hexdigest() == check_password:
            print("Username and password entered correctly.")
            # Username and password match - do something here
        else:
            print("Password entered incorrectly.")
            attempts += 1
    else:
        print("Username entered incorrectly.")
        attempts += 1

Use Scrypt

In reality, you wouldn't hash passwords to store them in a database, but use a dedicated key derivation function, such as Scrypt. The first cryptography library for Python that comes to mind is PyCrypto, but cryptography makes it significantly harder to shoot yourself in the foot.

Comments

Your code is horrible to read through, not so much because of how or what, but rather because of the excessive use of comments. Comments should add something to code, not say exactly what the code says, or say what the code already strongly implies.

Use hashes

Just hashing is very weak, but this is rather easy to implement. Also note that md5 is prone to hash collision attacks and should not be used anymore. If your platform supports it, use SHA3-256. The problem with this application is that if anybody gets to access the code, they can read the username/password in plaintext. You would be better off first hashing the passwords, then using the hashlib module to check if the input matches. I've written an improved version of your code below, which shows you what I mean.

from hashlib import md5
from getpass import getpass

print("Hello! Welcome to FaceSnap!") 

attempts = 0
check_username = '5945261a168e06a5b763cc5f4908b6b2'
check_password = '48d6215903dff56238e52e8891380c8f'
# These hashes have been generated earlier on.
# This is not how you would go about storing usernames and passwords,
# but for the sake of simplicity, we'll do it like this.

while True: 
    print("\n")
    username = input("Username: ")
    password = getpass("Password: ")
    # Getpass will not echo input to the screen, so your password remains 
    # invisible
    print("\n")
    
    if attempts == 3:
        print("Too many attempts.")
        exit()

    if md5(username.encode("utf-8")).hexdigest() == check_username:
        if md5(password.encode("utf-8")).hexdigest() == check_password:
            print("Username and password entered correctly.")
            # Username and password match - do something here
        else:
            print("Password entered incorrectly.")
            attempts += 1
    else:
        print("Username entered incorrectly.")
        attempts += 1

Comments

Your code is hard to read through, because of the excessive use of comments. Comments should add something to code, not repeat the code, or tell the reader what the code already strongly implies.

Use hashes

Merely hashing is very weak, but this is rather easy to implement. Also note that md5 is prone to hash collision attacks and should not be used anymore. If your platform supports it, use SHA(3)-256.

The problem with this application is that if anybody gets to access the code, they can read the username and password in plaintext. You would be better off first hashing the passwords, then using hashlib to check if the input matches. I've written an improved version of your code below, to show what I mean:

from hashlib import md5
from getpass import getpass
import sys

print("Hello! Welcome to FaceSnap!") 

attempts = 0
check_username = "5945261a168e06a5b763cc5f4908b6b2"
check_password = "48d6215903dff56238e52e8891380c8f"
# These hashes have been generated earlier on.
# This is not how you would go about storing usernames and passwords,
# but for the sake of simplicity, we'll do it like this.

while True: 
    username = input("Username: ")
    password = getpass("Password: ")
    # Getpass will not echo input to the screen, so your password remains 
    # invisible
    print()
    
    if attempts == 3:
        sys.exit("Too many failed attempts.")

    if md5(username.encode().hexdigest() == check_username:
        if md5(password.encode().hexdigest() == check_password:
            print("Username and password entered correctly.")
            # Username and password match - do something here
        else:
            print("Password entered incorrectly.")
            attempts += 1
    else:
        print("Username entered incorrectly.")
        attempts += 1

Use Scrypt

In reality, you wouldn't hash passwords to store them in a database, but use a dedicated key derivation function, such as Scrypt. The first cryptography library for Python that comes to mind is PyCrypto, but cryptography makes it significantly harder to shoot yourself in the foot.

added 45 characters in body
Source Link
Daniel
  • 4.6k
  • 2
  • 18
  • 40

Comments

Your code is horrible to read through, not so much because of how or what, but rather because of the excessive use of comments. Comments should add something to code, not say exactly what the code says, or say what the code already strongly implies.

Use hashes

Just hashing is very weak, but this is rather easy to implement. Also note that md5 is prone to hash collision attacks and should not be used anymore. If your platform supports it, use SHA3-256. The problem with this application is that if anybody gets to access the code, they can read the username/password in plaintext. You would be better off first hashing the passwords, then using the hashlib module to check if the input matches. I've written an improved version of your code below, which shows you what I mean.

from hashlib import md5
from getpass import getpass

print("Hello! Welcome to FaceSnap!") 

attempts = 0
check_username = '5945261a168e06a5b763cc5f4908b6b2'
check_password = '48d6215903dff56238e52e8891380c8f'
# These hashes have been generated earlier on.
# This is not how you would go about storing usernames and passwords,
# but for the sake of simplicity, we'll do it like this.

while True: 
    print("\n")
    username = input("Username: ")
    password = getpass("Password: ")
    # Getpass will not echo input to the screen, so your password remains 
    # invisible
    print("\n")
    
    if attempts == 3:
        print("Too many attempts.")
        exit()

    if md5(username.encode("utf-8")).hexdigest() == check_username:
        if md5(password.encode("utf-8")).hexdigest() == check_password:
            passprint("Username and password entered correctly.")
            # Username and password match - do something here
        else:
            print("Password entered incorrectly.")
            attempts += 1
    else:
        print("Username entered incorrectly.")
        attempts += 1

Comments

Your code is horrible to read through, not so much because of how or what, but rather because of the excessive use of comments. Comments should add something to code, not say exactly what the code says, or say what the code already strongly implies.

Use hashes

Just hashing is very weak, but this is rather easy to implement. Also note that md5 is prone to hash collision attacks and should not be used anymore. If your platform supports it, use SHA3-256. The problem with this application is that if anybody gets to access the code, they can read the username/password in plaintext. You would be better off first hashing the passwords, then using the hashlib module to check if the input matches. I've written an improved version of your code below, which shows you what I mean.

from hashlib import md5
from getpass import getpass

print("Hello! Welcome to FaceSnap!") 

attempts = 0
check_username = '5945261a168e06a5b763cc5f4908b6b2'
check_password = '48d6215903dff56238e52e8891380c8f'
# These hashes have been generated earlier on.
# This is not how you would go about storing usernames and passwords,
# but for the sake of simplicity, we'll do it like this.

while True: 
    username = input("Username: ")
    password = getpass("Password: ")
    
    if attempts == 3:
        print("Too many attempts.")
        exit()

    if md5(username.encode("utf-8")).hexdigest() == check_username:
        if md5(password.encode("utf-8")).hexdigest() == check_password:
            pass
            # Username and password match - do something here
        else:
            print("Password entered incorrectly.")
            attempts += 1
    else:
        print("Username entered incorrectly.")
        attempts += 1

Comments

Your code is horrible to read through, not so much because of how or what, but rather because of the excessive use of comments. Comments should add something to code, not say exactly what the code says, or say what the code already strongly implies.

Use hashes

Just hashing is very weak, but this is rather easy to implement. Also note that md5 is prone to hash collision attacks and should not be used anymore. If your platform supports it, use SHA3-256. The problem with this application is that if anybody gets to access the code, they can read the username/password in plaintext. You would be better off first hashing the passwords, then using the hashlib module to check if the input matches. I've written an improved version of your code below, which shows you what I mean.

from hashlib import md5
from getpass import getpass

print("Hello! Welcome to FaceSnap!") 

attempts = 0
check_username = '5945261a168e06a5b763cc5f4908b6b2'
check_password = '48d6215903dff56238e52e8891380c8f'
# These hashes have been generated earlier on.
# This is not how you would go about storing usernames and passwords,
# but for the sake of simplicity, we'll do it like this.

while True: 
    print("\n")
    username = input("Username: ")
    password = getpass("Password: ")
    # Getpass will not echo input to the screen, so your password remains 
    # invisible
    print("\n")
    
    if attempts == 3:
        print("Too many attempts.")
        exit()

    if md5(username.encode("utf-8")).hexdigest() == check_username:
        if md5(password.encode("utf-8")).hexdigest() == check_password:
            print("Username and password entered correctly.")
            # Username and password match - do something here
        else:
            print("Password entered incorrectly.")
            attempts += 1
    else:
        print("Username entered incorrectly.")
        attempts += 1
Source Link
Daniel
  • 4.6k
  • 2
  • 18
  • 40

Comments

Your code is horrible to read through, not so much because of how or what, but rather because of the excessive use of comments. Comments should add something to code, not say exactly what the code says, or say what the code already strongly implies.

Use hashes

Just hashing is very weak, but this is rather easy to implement. Also note that md5 is prone to hash collision attacks and should not be used anymore. If your platform supports it, use SHA3-256. The problem with this application is that if anybody gets to access the code, they can read the username/password in plaintext. You would be better off first hashing the passwords, then using the hashlib module to check if the input matches. I've written an improved version of your code below, which shows you what I mean.

from hashlib import md5
from getpass import getpass

print("Hello! Welcome to FaceSnap!") 

attempts = 0
check_username = '5945261a168e06a5b763cc5f4908b6b2'
check_password = '48d6215903dff56238e52e8891380c8f'
# These hashes have been generated earlier on.
# This is not how you would go about storing usernames and passwords,
# but for the sake of simplicity, we'll do it like this.

while True: 
    username = input("Username: ")
    password = getpass("Password: ")
    
    if attempts == 3:
        print("Too many attempts.")
        exit()

    if md5(username.encode("utf-8")).hexdigest() == check_username:
        if md5(password.encode("utf-8")).hexdigest() == check_password:
            pass
            # Username and password match - do something here
        else:
            print("Password entered incorrectly.")
            attempts += 1
    else:
        print("Username entered incorrectly.")
        attempts += 1