Python Tkinter login GUI with database

Question

I created a simple login program that works how I wanted them to work. I separated the GUI and database codes into two different python files. I named the files as login.py and dbaseManip.py. The login.py consist of Tkinter syntax and all stuff related to GUI. On the other hand, dbaseManip.py holds the database related syntax such as the connection and the queries.

login.py

import tkinter as tk
from tkinter import messagebox
from tkinter import ttk
import dbaseManip as db
class loginForm(ttk.Frame):
 def __init__(self,parent,*arags,**kwargs):
 tk.Frame.__init__(self,parent)
 self.parent = parent
 self.parent.protocol("WM_DELETE_WINDOW",self.closing_window)
 window_width = 350
 window_height = 100
 screen_width = self.parent.winfo_screenwidth()
 screen_height = self.parent.winfo_screenheight()
 x_position = int((screen_width/2) - (window_width/2))
 y_position = int((screen_height/2) - (window_height/2))
 self.parent.geometry("{}x{}+{}+{}".format(window_width,window_height,x_position,y_position))
 self.widgeter()
 def widgeter(self):
 self.ent_username = ttk.Entry(self.parent)
 self.ent_username.insert(0,"username")
 self.ent_username.bind("<FocusIn>",self.foc_in_username)
 self.ent_username.bind("<FocusOut>",self.foc_out_username)
 self.ent_password = ttk.Entry(self.parent)
 self.ent_password.insert(0,"password")
 self.ent_password.bind("<FocusIn>",self.foc_in_password)
 self.ent_password.bind("<FocusOut>",self.foc_out_password)
 btn_login = ttk.Button(self.parent,text="Login",command=self.submit)
 btn_exit = ttk.Button(self.parent,text="Exit",command=self.closing_window)
 self.ent_username.grid(row=0,column=0,columnspan=3,sticky="nsew")
 self.ent_password.grid(row=1,column=0,columnspan=3,sticky="nsew")
 btn_login.grid(row=2,column=0,sticky="nsw")
 btn_exit.grid(row=2,column=1,sticky="nse")
 def foc_in_username(self, *args):
 if self.ent_username.get() == "username":
 self.ent_username.delete(0, tk.END)
 def foc_out_username(self,*args):
 if not self.ent_username.get():
 self.ent_username.insert(0,"username")
 def foc_in_password(self,*args):
 if self.ent_password.get() == "password":
 self.ent_password.delete(0,tk.END)
 self.ent_password.configure(show="*")
 def foc_out_password(self,*args):
 if not self.ent_password.get():
 self.ent_password.insert(0,"password")
 self.ent_password.configure(show="")
 def closing_window(self):
 answer = messagebox.askyesno("Exit","You want to exit the program?")
 if answer == True:
 self.parent.destroy()
 def submit(self):
 __verify = db.DatabaseManip(self.ent_username.get(),self.ent_password.get())
 __str_verify = str(__verify)
 if __str_verify == "correct":
 self.initialize_mainApplication()
 elif __str_verify is "incorrect":
 messagebox.showerror("Incorrect Password","You have entered incorrect password")
 # add deleter
 elif __str_verify == "notexist":
 messagebox.showerror("Username does not exist","The username you entered does not exist")
 def initialize_mainApplication(self):
 self.parent.destroy()
 self.parent = tk.Tk()
 self.mainApp = mainApplicaiton(self.parent)
 self.parent.mainloop()
class mainApplicaiton(tk.Frame):
 def __init__(self,parent):
 self.parent = parent
if __name__ == "__main__":
 root = tk.Tk()
 loginForm(root)
 root.mainloop()

dbaseManip.py

import mysql.connector
class DatabaseManip():
 def __init__(self,username,password):
 self.username = username
 self.password = password
 #
 # print(self.username)
 # print(self.password)
 self.dbaseInitializer()
 def dbaseInitializer(self):
 mydb = mysql.connector.connect(
 host="127.0.0.1",
 user="root",
 password="admin123",
 database="testdb"
 )
 self.myCursor = mydb.cursor()
 query_user_exists = "SELECT EXISTS(SELECT * FROM users WHERE username = %s)"
 self.myCursor.execute(query_user_exists,(self.username,))
 tuple_user_exists = self.myCursor.fetchone()
 total_user_exists = int(tuple_user_exists[0])
 if total_user_exists != 0:
 query_pass_validation = "SELECT password FROM users WHERE username = %s"
 self.myCursor.execute(query_pass_validation,(self.username,))
 __tuple_valid_pass = self.myCursor.fetchone()
 __password_validation = __tuple_valid_pass[0]
 if __password_validation == self.password:
 self.verdict = "correct"
 else:
 self.verdict = "incorrect"
 else:
 self.verdict = "notexist"
 def __str__(self):
 return self.verdict
 @property
 def username(self):
 return self.__username
 @username.setter
 def username(self,username):
 self.__username = username
 @property
 def password(self):
 return self.__password
 @password.setter
 def password(self,password):
 self.__password = password

This is how the system works:

1. The login form will pop up
2. After the user submitting their username and password, it will call the class DatabaseManip() that exists in dbaseManip.py.
3. The DatabaseManip Class will validate the user's account.
4. HERE COMES MY PROBLEM If the username does not exist in the database, it will assign a string "notexist" to a class variable called verdict. If the password is wrong, it will assign "incorrect". And lastly, if the validation is correct; it will assign "correct".
5. The verdict result (which is either "correct","incorrect" or "notexist") will be returned to the GUI class in login.py using the __str__ method.
6. In the loginForm class, a conditional statement will determine if the verdict from the databaseManip class is either "correct","incorrect" or "notexist". If it is "correct", a new window will pop-up. On the other hand, if it is either "incorrect" or "notexist", it will show an error message.

I think my way of coding this is extremely bad in terms of security and integrity. I'm not confident in the "correct", "incorrect", and "notexit" part. I would like to ask for advice to make this program more secure and not as wacky as the current state.

Answer 1

Firstly I'd suggest following PEP8 and/or using an auto-formatter like black, but if it's just you working on it that is a bit less of an issue than if other people were involved. Suffice to say, most code follows that and it usually gets flagged first thing (because it's so noticeable).

Reading the code, is that all there is, you are able to log in via a GUI? Then I'd say first thing would probably be not to hardcode the credentials to the database. So what/how's this securing anything? If the program has the credentials ... and the program is being run by the user who's authenticating ... what stops me from connecting directly to the database?

---

Apart from that fundamental issue looks mostly okay, no SQL injection since the execute will handle that.

__ as prefix for the variable names is unnecessary and confusing. I can't see why it's there at all.

Actually, SELECT password ... ... the password's stored in plaintext? Big no-no in anything used by other people. Do not store passwords in plaintext.

The properties at the end of DatabaseManip seem not very useful, in Python there's no need for them unless you want to control what/how values are stored in the object / how they're computed if they're not stored at all.

The __str__ method is there for what reason? That's very much confusing the responsibility of the class. It would somewhat make sense if it's being used interactively in a REPL, otherwise I can't see the reason for having it. Oh. Oh now I see it. Yeah, don't do that, that's bad style. How I'd expect things to go is something like this:

 def submit(self):
 result = db.DatabaseManip().check_credentials(self.ent_username,self.ent_password)
 if result == "correct":
 self.initialize_mainApplication()
 elif result == "incorrect":
 messagebox.showerror("Incorrect Password","You have entered incorrect password")
 # add deleter
 elif result == "notexist":
 messagebox.showerror("Username does not exist","The username you entered does not exist")
 else:
 messagebox.showerror("Unexpected validation result","Validation of credentials failed unexpectedly with result".format(result))

So here: Validating is an action, a method. The parameters of which aren't part of the helper object, but method parameters. The result is still a string (try making this an enumeration, that's more "correct" in a way as you really want to have a fixed set of possible results, not a string that can contain basically anything. Finally, the programmer error case is handled too (the last else).

Also note that ... is "foo" is likely incorrect, c.f. "foo" is "foobar"[0:3], is does a check for object identity, not equality! Not even numbers are same via is if they're not interned at the same time:

1000000 =わ=わ 1000000 # _always_ True
x = 1000000
y = 1000000
x is y # _probably_ False

Also if x == True can obviously be simplified to if x.

---

So it's not all bad, simply try and stay away from "stringly"-typed things, use the language features as they're intended and have a goal for what your security requirements are. I've not gone into more detail on the last bit simply because I still don't understand the intent here. Suffice to say like this it would only work if the user can't exit the UI and open a shell / other means of executing code on the machine (like in a kiosk configuration with limited access to peripherals and the machine itself). Or if the actual database access was moved to a separate external service that (again) can't be circumvented by the local user.