5
\$\begingroup\$

Background

I started learning Python about 3 months ago. I feel like I learned a lot, but I would like to receive some advice from the pros and some feedback about my code. So I wrote a small GUI app. I would like to know if it's efficient, readable, fits the PEP8 rules, and if you would do it in a different way.

Code Purpose

A Tkinter application that is simply a login form. You can choose to count the number of times you've logged in as a specific user.

App Screenshot

CSV File Example

Shaked, 123456, 0

Ellen, pass123, 0

Code

'''
A Python Exercise: Reading CSV File & Tkinter Implementation
Including the subjects: Grid Layout, Tk Variables and Binding
'''
import Tkinter as tk
import csv
CSV_FILE = "users.csv"
# Functions
def popup(title, msg):
 '''Open popup window with title and msg'''
 w = tk.Toplevel(root)
 w.title(title)
 w.minsize(200, 200)
 tk.Label(w, text=msg).pack()
 tk.Button(w, text="Close", command=w.destroy).pack(pady=10)
 w.bind("<Return>", lambda f: w.destroy())
def read_from_file():
 '''Read csv file and return a list like: [[username, password, count]]'''
 try:
 with open(CSV_FILE, 'rb') as f:
 users = []
 reader = csv.reader(f)
 for row in reader:
 row[2] = int(row[2]) # Make the count an integer so it can increase later
 users.append(row)
 return users
 except IOError:
 popup("Error", "File not found!")
def write_to_file(users):
 '''Get a list of all users and write it to the csv file'''
 with open(CSV_FILE, 'wb') as f:
 writer = csv.writer(f)
 for row in users:
 row[2] = str(row[2])
 writer.writerows(users)
class App(tk.Frame):
 def __init__(self, master=None):
 tk.Frame.__init__(self, master)
 self.master = master
 self.pack(anchor="w")
 # Labels
 tk.Label(self, text="Username:").grid(sticky="w")
 tk.Label(self, text="Password:").grid(row=1, sticky="e")
 # Entries
 self.n_string = tk.StringVar()
 self.p_string = tk.StringVar()
 name = tk.Entry(self, textvariable=self.n_string, width=30)
 passw = tk.Entry(self, textvariable=self.p_string, show='*', width=20)
 name.grid(row=0, column=1, sticky="we")
 passw.grid(row=1, column=1, sticky="we")
 # Check Button
 self.check_var = tk.BooleanVar()
 check = tk.Checkbutton(self, text="Count Login attempts", variable=self.check_var)
 check.grid(row=2, column=1, sticky="w")
 # Login Button
 login = tk.Button(self, text="Login", command=self.login)
 login.grid(row=2, column=1, sticky="e")
 # Binding
 self.master.bind("<Return>", self.login)
 def login(self, event=None):
 '''Login attempt'''
 users = read_from_file()
 if not users:
 return
 for row in users:
 if row[0] == self.n_string.get() and row[1] == self.p_string.get():
 msg = "Welcome %s!\n\nYour login count is %d" % (row[0], row[2])
 popup("Welcome", msg)
 if self.check_var.get(): # Check if "Count Login attempts" button is checked
 row[2] += 1 # add +1 login count
 write_to_file(users)
 break
 elif row[0] == self.n_string.get():
 print "Password incorrect."
 break
 else: # If there isn't a match of the username
 print "User name incorrect!"
# GUI settings
root = tk.Tk()
app = App(root)
root.title("Login Form")
root.minsize(200, 200)
# Initalize GUI
root.mainloop()
toolic
14.5k5 gold badges29 silver badges203 bronze badges
asked Oct 13, 2017 at 16:42
\$\endgroup\$
1
  • \$\begingroup\$ Weird.. It does work for me. Did you create a Windows Comma Seperated file? (when using mac). The script doesn't create a csv file by itself. \$\endgroup\$ Commented Oct 13, 2017 at 17:25

2 Answers 2

2
\$\begingroup\$

Documentation

While it is good that you added a docstring at the top of the code, it does not convey much useful information. It should describe the purpose of the code and include details about the CSV file. For example:

"""
Tkinter application for a login form.
It reads and writes a CSV file containing user names and passwords.
The CSV file is named "users.csv" and is in the current working directory.
The CSV file has this format...
"""

The PEP 8 style guide recommends adding docstrings for classes.

Portability

I realize this question was posted many years ago when Python version 2.x was prevalent, but now that it is deprecated, consider porting to 3.x. It may be as simple as changing these 2 print lines:

print "Password incorrect."
print "User name incorrect!"

to:

print("Password incorrect.")
print("User name incorrect!")

I get an error on the following line:

import Tkinter as tk

The error goes away when I change Tkinter to tkinter:

import tkinter as tk

Perhaps your version of Python or your operating system is more forgiving than mine.

Error diagnostics

It is good that you check for the file's existence and display an error message in the GUI:

popup("Error", "File not found!")

It would be helpful to the user to also show the file name:

popup("Error", f"File {CSV_FILE} not found!")
answered Apr 11 at 10:57
\$\endgroup\$
2
\$\begingroup\$

It's generally bad practice to store plaintext passwords. This is because if the (in this case) CSV is leaked, an attacker has knowledge of every username password combination. Instead you should store a cryptographic hash of the password using something like hashlib.

Then, when checking if the password is valid, you compare the hash of the input with what is stored in the CSV. If an attacker obtains the CSV they have the hash, but it is important to note that the hash of the hash != hash, so they can't immediately use the hashed password that is written in the CSV.

answered Apr 11 at 22:16
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.