Simple password generator with a GUI

Question

I made a simple password-generating app with a GUI written in Python and the tkinter library.

Here's how the GUI looks like:

GUI of the app

The passwords are strings composed of random characters. The user can choose the length of the password, and the characters that are allowed to be in it.

Here's the source code:

import string
import tkinter as tk
from tkinter import ttk
import random
from tkinter.constants import DISABLED, E, END, NORMAL, NW, VERTICAL
class GUI(tk.Frame):
 def __init__(self, master):
 super().__init__(master)
 self.pack()
 self.widget_vars()
 self.create_widgets()
 self.style()
 def generate_password(self):
 passw = Password(self.length.get(), self.lower.get(), self.upper.get(),
 self.digits.get(), self.punct.get())
 # You can only insert to Text if the state is NORMAL
 self.password_text.config(state=NORMAL)
 self.password_text.delete("1.0", END) # Clears out password_text
 self.password_text.insert(END, passw.password)
 self.password_text.config(state=DISABLED)
 def widget_vars(self):
 self.length = tk.IntVar(self, value=16)
 self.lower = tk.BooleanVar(self, value=True)
 self.upper = tk.BooleanVar(self, value=True)
 self.digits = tk.BooleanVar(self, value=True)
 self.punct = tk.BooleanVar(self, value=True)
 def create_widgets(self):
 # Define widgets
 self.lower_checkbtn = ttk.Checkbutton(self, variable=self.lower)
 self.lower_label = ttk.Label(self, text="string.ascii_lower")
 self.upper_checkbtn = ttk.Checkbutton(self, variable=self.upper)
 self.upper_label = ttk.Label(self, text="string.ascii_upper")
 self.digits_checkbtn = ttk.Checkbutton(self, variable=self.digits)
 self.digits_label = ttk.Label(self, text="string.digits")
 self.punct_checkbtn = ttk.Checkbutton(self, variable=self.punct)
 self.punct_label = ttk.Label(self, text="string.punctuation")
 self.length_spinbox = ttk.Spinbox(self, from_=1, to=128, width=3,
 textvariable=self.length)
 self.length_label = ttk.Label(self, text="Password length")
 self.separator = ttk.Separator(self, orient=VERTICAL)
 self.generate_btn = ttk.Button(self, text="Generate password",
 command=self.generate_password)
 self.password_text = tk.Text(self, height=4, width=32, state=DISABLED)
 # Place widgets on the screen
 self.length_label.grid(column=0, row=0, rowspan=4, sticky=E)
 self.length_spinbox.grid(column=1, row=0, rowspan=4, padx=4, pady=2)
 self.lower_label.grid(column=3, row=0, sticky=E, padx=4)
 self.lower_checkbtn.grid(column=4, row=0, padx=4, pady=2)
 self.upper_label.grid(column=3, row=1, sticky=E, padx=4)
 self.upper_checkbtn.grid(column=4, row=1, padx=4, pady=2)
 self.digits_label.grid(column=3, row=2, sticky=E, padx=4)
 self.digits_checkbtn.grid(column=4, row=2, padx=4, pady=2)
 self.punct_label.grid(column=3, row=3, sticky=E, padx=4)
 self.punct_checkbtn.grid(column=4, row=3, padx=4, pady=2)
 self.separator.grid(column=2, row=0, rowspan=4, ipady=45)
 self.generate_btn.grid(columnspan=5, row=4, padx=4, pady=2)
 self.password_text.grid(columnspan=5, row=6, padx=4, pady=2)
 self.grid(padx=10, pady=10)
 def style(self):
 self.style = ttk.Style(self)
 self.style.theme_use("clam")
class Password:
 def __init__(self, length: int,
 allow_lowercase: bool,
 allow_uppercase: bool,
 allow_digits: bool,
 allow_punctuation: bool) -> None:
 self.length = length
 self.allow_lowercase = allow_lowercase
 self.allow_uppercase = allow_uppercase
 self.allow_digits = allow_digits
 self.allow_punctuation = allow_punctuation
 self.allowed_chars = self.gen_allowed_chars()
 self.password = self.gen_password()
 def gen_allowed_chars(self) -> str:
 # I use a string, because random.choice doesn't work with sets:
 chars = ''
 if self.allow_lowercase:
 chars += string.ascii_lowercase
 if self.allow_uppercase:
 chars += string.ascii_uppercase
 if self.allow_digits:
 chars += string.digits
 if self.allow_punctuation:
 chars += string.punctuation
 return chars
 def gen_password(self) -> str:
 password = ''
 for _ in range(self.length):
 password += random.choice(self.allowed_chars)
 return password
if __name__ == '__main__':
 root = tk.Tk()
 root.title("Password Generator")
 app = GUI(root)
 app.mainloop()

Can this code be improved in any way? Does this code follow common best practices? I'd appreciate some advice, especially on the GUI class (I'm new to tkinter).

Thank You in advance.

Answer 1

• Don't use random for passwords, use secrets
• "has-a-root" is a cleaner pattern here, I think, than "is-a-root"; in other words, don't inherit - instantiate
• Cut down the repetition in your options by generalizing to a collection of strings, each expected to be an attribute name on the string module. Represent this name consistently between the UI and the module lookup logic.
• Type-hint your method signatures.
• Prefer ''.join() over successive concatenation
• Try to avoid assigning new class members outside of __init__.
• Where possible, reduce the number of references you keep on your GUI class. Almost none of your controls actually need to have references kept.
• Do not call mainloop on your frame; call it on your root
• Name your variables
• Sort your grid declarations according to column and row
• Your Password is not a very useful representation of a class. Whether or not it is kept as-is, it should be made immutable. Also, distinguish between a password and a password generator. A password generator knowing all of its generator parameters but having no actual password state would be more useful. After such a representation is implemented, you could change your TK logic to trace on all of your options variables, and only upon such a change trace, re-initialize your generator. Repeat clicks on 'Generate' will reuse the same generator instance.
• Don't call things master. In this case "parent" is more appropriate.

Suggested

import secrets
import string
import tkinter as tk
from dataclasses import dataclass, field
from tkinter import ttk
from tkinter.constants import DISABLED, E, END, NORMAL, VERTICAL
from typing import Iterable, Collection, Protocol, Literal
TraceMode = Literal[
 'r', # read
 'w', # write
 'u', # undefine
 'a', # array
]
class TkTrace(Protocol):
 def __call__(self, name: str, index: str, mode: TraceMode): ...
class OptControl:
 NAMES = ('ascii_lowercase', 'ascii_uppercase', 'digits', 'punctuation')
 def __init__(self, parent: tk.Widget, name: str, trace: TkTrace) -> None:
 self.name = name
 self.var = tk.BooleanVar(parent, name=name, value=True)
 self.var.trace(mode='w', callback=trace)
 self.label = ttk.Label(parent, text=name)
 self.check = ttk.Checkbutton(parent, variable=self.var)
 @classmethod
 def make_all(cls, parent: tk.Widget, trace: TkTrace) -> Iterable['OptControl']:
 for name in cls.NAMES:
 yield cls(parent, name, trace)
class GUI:
 def __init__(self, parent: tk.Tk):
 self.root = tk.Frame(parent)
 self.root.pack()
 self.length = tk.IntVar(self.root, value=16)
 self.length.trace('w', self.opt_changed)
 self.opts = tuple(OptControl.make_all(self.root, self.opt_changed))
 self.password_text = self.create_widgets()
 self.style()
 self.opt_changed()
 @property
 def selected_opts(self) -> Iterable[str]:
 for opt in self.opts:
 if opt.var.get():
 yield opt.name
 def generate_password(self) -> None:
 # You can only insert to Text if the state is NORMAL
 self.password_text.config(state=NORMAL)
 self.password_text.delete('1.0', END) # Clears out password_text
 self.password_text.insert(END, self.generator.gen_password())
 self.password_text.config(state=DISABLED)
 def opt_changed(self, *args) -> None:
 self.generator = PasswordGenerator(
 length=self.length.get(),
 opts=tuple(self.selected_opts),
 )
 def create_widgets(self) -> tk.Text:
 length_label = ttk.Label(self.root, text='Password length')
 length_label.grid(column=0, row=0, rowspan=4, sticky=E)
 generate_btn = ttk.Button(
 self.root, text='Generate password', command=self.generate_password)
 generate_btn.grid(column=0, row=4, columnspan=5, padx=4, pady=2)
 password_text = tk.Text(self.root, height=4, width=32, state=DISABLED)
 password_text.grid(column=0, row=6, columnspan=5, padx=4, pady=2)
 length_spinbox = ttk.Spinbox(
 self.root, from_=1, to=128, width=3, textvariable=self.length)
 length_spinbox.grid(column=1, row=0, rowspan=4, padx=4, pady=2)
 separator = ttk.Separator(self.root, orient=VERTICAL)
 separator.grid(column=2, row=0, rowspan=4, ipady=45)
 for row, opt in enumerate(self.opts):
 opt.label.grid(column=3, row=row, sticky=E, padx=4)
 opt.check.grid(column=4, row=row, padx=4, pady=2)
 self.root.grid(padx=10, pady=10)
 return password_text
 def style(self) -> None:
 style = ttk.Style(self.root)
 style.theme_use('clam')
@dataclass(frozen=True)
class PasswordGenerator:
 length: int
 opts: Collection[str]
 allowed_chars: str = field(init=False)
 def __post_init__(self):
 super().__setattr__('allowed_chars', ''.join(self._gen_allowed_chars()))
 def gen_password(self) -> str:
 return ''.join(self._gen_password_chars())
 def _gen_allowed_chars(self) -> Iterable[str]:
 for opt in self.opts:
 yield getattr(string, opt)
 def _gen_password_chars(self) -> Iterable[str]:
 for _ in range(self.length):
 yield secrets.choice(self.allowed_chars)
if __name__ == '__main__':
 root = tk.Tk()
 root.title('Password Generator')
 GUI(root)
 root.mainloop()

Answer 2

I don't think Password needs to be a class. __init__ is calling gen_password, so this really could be boiled down to a single function call:

def gen_password(length: int, lowercase: bool, uppercase: bool, digits: bool, punctuation: bool) -> str:
 char_groups = {
 'lowercase': string.ascii_lowercase,
 'uppercase': string.ascii_uppercase,
 'digits': string.digits,
 'punctuation': string.punctuation
 }
 # gets the corresponding keys for any True arguments in the
 # function call
 groups = (grp for grp, arg in zip(
 ('lowercase', 'uppercase', 'digits', 'punctuation'), 
 (lowercase, uppercase, digits, punctuation)
 ) if arg)
 # your set of allowed chars gets generated here
 allowed_chars = ''.join((char_groups[group] for group in groups))
 # instead of the for loop, just sample the chars
 return ''.join(random.sample(allowed_chars, k=length))

Which changes your generate_password function to look like:

 def generate_password(self):
 # I'd use keywords here for clarity
 passw = gen_password(
 length=self.length.get(), 
 lowercase=self.lower.get(), 
 uppercase=self.upper.get(),
 digits=self.digits.get(), 
 punctuation=self.punct.get()
 )
 # You can only insert to Text if the state is NORMAL
 self.password_text.config(state=NORMAL)
 self.password_text.delete("1.0", END) # Clears out password_text
 self.password_text.insert(END, passw)
 self.password_text.config(state=DISABLED)