Please Critique My Code [Beginner Python]

[Sypherz]

I'm about 1-2 weeks into learning Python. I'm learning by making random projects and learning from Youtube/StackOverflow/etc. as I progress. I spent a chunk of my day doing a rudimentary login system that allows you to create an account and simply returns True if you successfully log in one of two ways.

Keep in mind I'm a beginner to programming, so any advice on redundancy/bad habits/#commentyourdamncode/etc. is especially helpful. Thanks!!

PLEASE suggest good practice projects! I will do my best and post them when they're ready.


main.py

import os
import sys
import sql
from time import sleep #stay tuned for login attempt limit DLC only $9.99
from sql import add, printDB, searchDB

loggedOn = False

def newAccount():
    newUser = input("Please enter your desired username.")
    newPassword = input("Please enter your desired password.")
    auth = valid(newPassword) #condense this&next line to "if valid(newPassword)"
    if auth == True:
        print("Accepted! You are now logged on as", newUser, ".")
        add(newUser, newPassword)
        loggedOn = True
    else:
        print("Password choice not accepted.")
        newAccount()

def valid(password__): #this can be expanded later
    if len(password__) < 6:
        return False
    else:
        return True

def login():
    user = input("Please enter your username. ")
    password = input("Please enter your password. ")
    if searchDB(user, password):
        print("Congratulations", user, "you have been logged on!")
        loggedOn = True
    else:
        print("User or Pass invalid, please try again.")
        login()

def home():
    auth = False
    i = input("Press 1 to login, or 2 to make a new account.")
    if i == '1':
        login()
    elif i == '2':
        newAccount()
    else:
        print("Please choose an appropriate answer")
        home()

print("Welcome to generic website!")
home()

sql.py

import sqlite3
import sys

def printDB(): #for reference
    conn = sqlite3.connect('files.db')
    theCurser = conn.cursor()

    result = conn.execute("SELECT ID, Username, Password FROM Accounts")

    for row in result:
        print("Account Number: ", row[0])
        print("Username: ", row[1])
        print("Password: ", row[2])
    conn.close()

def create():
    try:
        conn.execute("CREATE TABLE Accounts(ID INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, Username TEXT, Password TEXT);")

        conn.commit()
        print("Table Created")

    except sqlite3.OperationalError:
        print("")

def add(un, pw):
    conn = sqlite3.connect('files.db')
    theCurser = conn.cursor()
    varU = (un,) #I think I can take these out but I'm scared to.
    varP = (pw,)
    conn.execute("INSERT INTO Accounts(Username, Password) VALUES (?, ?)", (un, pw))
    conn.commit()
    conn.close()


def searchDB(usn, psw):
    conn = sqlite3.connect('files.db')
    cursor = conn.cursor()
    h = cursor.execute("SELECT Username, Password FROM Accounts")
    j = cursor.fetchall()
    for row in j:
        if row[0] == usn and row[1] == psw:
            conn.close()
            return True
    conn.close()
    return False

 

[kmjt]

I've never programmed in python but this looks a bit weird:

if auth == True:

In most languages, auth itself would hold either true or false (assuming it holds a boolean) so there is no need to compare it to true again. Most likely what you are doing is actually:

if True == True

Which is obviously true. To simplify you can probably just do:

if auth:

I've never looked into python though so you might want to double check this.

Syntax for an If statement using a boolean



Quickly looking you can fix logic places like this:

def valid(password__): #this can be expanded later
if len(password__) < 6:
return False
else:
return True

you should be able to just write something like

def valid(password__): #this can be expanded later
return len(password__) >= 6

len(password__) >= 6 itself is a boolean expression so you should be able to just return it. See how if put a little more thought into your functions (or whatever they are called in python) you can shorten your code? Once again... I don't know python this is just logic fix so double check the syntax.


I noticed this here:

result = conn.execute("SELECT ID, Username, Password FROM Accounts")

I see that the table you created is using all 3 of these columns (id, username, password). Did you know in SQL you can simply do SELECT * FROM Accounts to get all of the columns of the table? Not a big deal in your case but just in case you didn't know. So if you have a table with 100 columns, you wouldn't need to write the 100 column names in your SELECT you can just use *

Also keep in mind you should always encrypt any passwords before storing them into a database. Obviously you are just practicing but keep this in mind if you ever build a live application. Most log in tutorials on the web skimp over the security aspect but you always need to encrypt passwords and any other sensitive data. That way if your database ever gets hacked the hacker would only have the encrypted passwords which won't be that useful. If you store unencrypted passwords, the hacker would obviously get those.

If you have a Lynda account this is a decent tutorial:
Programming Foundations: Web Security

And here is a short video on youtube with a pretty good overview of encryption:

Looking back at your code, it looks like you also have sql injection vulnerability. You might also want to look into this because it is one of the most dangerous security holes out there:

 

[Sypherz]

@kmjt Thanks man! I put a comment on saying "#condense this&next line to "if valid(newPassword)" for your first point so I think we're on the same page.

I didn't know that return len(password__) >= 6 could be a boolean, that's super helpful.

The SQL stuff confuses the heck out of me but I'll focus on security if I do something involving it soon, just for the practice.

 

[kmjt]

I highly recommend subscribing to Lynda if you aren't already (I think they have a free trial you can take advantage of before you commit to paying). The subscription is like $25 monthly if you like it. For example here is a python course (there are many python courses on Lynda, this is just one):

Welcome

With your subscription, you have access to all of the courses you could imagine. So when you want to learn about databases (using SQL), they have many courses on that too. Lynda I believe is run by LinkedIn if you ever heard of that website.

 

[Sypherz]

I've come so far :')