Python code review | Tkinter gui application

[Sr999]

Hey, i am learning python for like 12-14days and i have finished my first complete gui project using tkinter because i am self learner i have no place to get feedback about my code in aspect more than "the code is working or not working" so i would like to ask from you guys "the experts" to review my code and give me any notes to improve my code please try to mention in your review these aspects:
Code readability
Code efficiency
and any other notes you have for me
last thing i did learned about OOP but i didnt found any use of it in this code tell me otherwise if im wrong and also i could split each frame window(add_page,main_page,display_page) functions into different files but i felt it is a small code and its not so important to do so so again if im wrong lemme know

this is my code
https://github.com/SeanR11/PasswordManager

[rob101]

The #1 and most significant change you could make is to not have the passwords stored in the clear, rather store said as a sha256 hash value, which could be reversed when a user opens the app and enters a "master password". That aside, the code (from a very quick look) seems well written and your UI looks nice.

To add: maybe what I have in mind would not in fact work, but rather some encryption of the stored passwords would be the way to go. I'll have a think about it.

[menator01]

You should not do wild card imports eg from tkinter import *. This will cause problems down the road.
Other than that, nicely done.

[Sr999]

The #1 and most significant change you could make is to not have the passwords stored in the clear, rather store said as a sha256 hash value, which could be reversed when a user opens the app and enters a "master password". That aside, the code (from a very quick look) seems well written and your UI looks nice.

To add: maybe what I have in mind would not in fact work, but rather some encryption of the stored passwords would be the way to go. I'll have a think about it.

Hey Rob thanks for your answer it took me a while to come back cause of work, anyhow i did added admin verfication function that runs before the display in this function i have generated one time sha256 password and created a popup box that let the user insert admin password after each guess the input is hashed with sha256 and attempt to match the hashed password i have set to default admin password

tab is accessable to the user, in the passwordmanager.csv file i added that the password is saved as encrypted password
i didnt use encryption before so it took me a whle read about it and i found that sha256 cannot be reversed (tell me if im wrong) so i used Cryptography lib to generate one time key then i use the same key over and over to encrypt the password i do understand that using same key may cause security problem but i am begginer so i have to learn other ways to change key now and thenbut hold the key value without giving it access to everyone

if i can ask you to please go back to the git link i have updated the code there if ucan review the new segments i have added and maybe more tips to improve will come to your mind once you will rebrief the code
again thanks for your time :)

[Sr999]

You should not do wild card imports eg from tkinter import *. This will cause problems down the road.
Other than that, nicely done.

Hey menator01 in my learning path i understood from few sources that in case i am using one or two classes from a file i should directly import
such as: from tkinter import Tk
but when i use more than a few classes from the same file i should use import * as it easier to use
now i tried to change the import tkinter line and this is what i got
from tkinter import Label, Tk, Entry, Canvas, Frame, Button, PhotoImage, W, E, CENTER, NO, END
now tell me if im wrong but it looks little bit messy and i do not want everytime i call a label or button to use the full path as i would have use if the import was
import tkinter and then each time i use tkiner.Tk() or tkinter.Button()

tell me which way is more correctly to use as i said i am here to learn and accept your reviews
again thanks for your time you can also re-enter my git i have updated the code changed all imports that had * to specific classes execpt import tkinter

[menator01]

I was intrigued by your project and started to take my own approach from your project. This will give you an example of how I was taught or advised how to do imports. I've only done the cover part. Buttons don't work yet.

import tkinter as tk
import pandas as pd 
from datetime import date 
from random import sample
import math 
import os 

# Create path to executing directory
path = os.path.realpath(os.path.dirname(__file__))

class Data:
    '''
        Class will handle all data manipulations
    '''
    def __init__(self):
        pass 


class Window:
    ''' Class will handle all views '''
    def __init__(self, parent):
        parent.columnconfigure(0, weight=1)
        parent.rowconfigure(0, weight=1)
        parent.minsize(800,600)
        parent.resizable(False, False)

        container = tk.Frame(parent)
        container.grid(column=0, row=0, sticky='news')

        container.columnconfigure(0, weight=3)

        logo = tk.PhotoImage(file=f'{path}/resources/PasswordManager/logo.png')
        logo.backup = logo
        header = tk.Label(container)
        header['image'] = logo
        header.grid(column=0, row=0, padx=5, pady=0, sticky='new')

        btn_frame = tk.Frame(container)
        btn_frame.grid(column=0, row=1, sticky='new', padx=800/3, pady=5)
        btn_frame.grid_columnconfigure(0, weight=3, uniform='btn')
        btn_frame.grid_columnconfigure(1, weight=3, uniform='btn')

        self.display_btn = tk.Button(btn_frame, text='Display', cursor='hand2')
        self.display_btn['bg'] = 'red'
        self.display_btn['fg'] = 'white'
        self.display_btn.grid(column=0, row=0, sticky='new', padx=2, pady=2)
        self.display_btn.bind('<Enter>', lambda event: self.hover(self.display_btn))

        self.add_btn = tk.Button(btn_frame, text='Add', cursor='hand2')
        self.add_btn['bg'] = 'red'
        self.add_btn['fg'] = 'white'
        self.add_btn.grid(column=1, row=0, sticky='new', padx=4, pady=4)
        self.add_btn.bind('<Enter>', lambda event: self.hover(self.add_btn))

    def hover(self, btn):
        btn['activebackground'] = 'white'
        btn['activeforeground'] = 'red'



class Controller:
    def __init__(self, data, window):
        ''' Class will handle communications between Data and Window classes '''
        self.data = data 
        self.window = window


        # Button Commands
        self.window.display_btn['command'] = self.display
        self.window.add_btn['command'] = self.add 


    def display(self):
        print('display here')

    def add(self):
        print('Add data')


if __name__ == '__main__':
    root = tk.Tk()
    controller = Controller(Data(), Window(root))
    root.mainloop()

[rob101]

You're not wrong: hashing is a one way function.

What I had in mind would be over complicated for an app such as this.

I am not an expert in the use of encryption and many people who do put themselves in that category have made fundamental errors which have completely compromised the user data; the LastPass Data Breach possibly being the most high profile one in recent times.

I know your app is not being offered "as a service", but it's still worth keeping in mind just how difficult it is to get this kind of thing right and how easy it is to get it wrong.

I will again have a look at your app and feedback if I see anything that I think you should re-visit.

[Sr999]

I was intrigued by your project and started to take my own approach from your project. This will give you an example of how I was taught or advised how to do imports. I've only done the cover part. Buttons don't work yet.

import tkinter as tk
import pandas as pd 
from datetime import date 
from random import sample
import math 
import os 

# Create path to executing directory
path = os.path.realpath(os.path.dirname(__file__))

class Data:
    '''
        Class will handle all data manipulations
    '''
    def __init__(self):
        pass 


class Window:
    ''' Class will handle all views '''
    def __init__(self, parent):
        parent.columnconfigure(0, weight=1)
        parent.rowconfigure(0, weight=1)
        parent.minsize(800,600)
        parent.resizable(False, False)

        container = tk.Frame(parent)
        container.grid(column=0, row=0, sticky='news')

        container.columnconfigure(0, weight=3)

        logo = tk.PhotoImage(file=f'{path}/resources/PasswordManager/logo.png')
        logo.backup = logo
        header = tk.Label(container)
        header['image'] = logo
        header.grid(column=0, row=0, padx=5, pady=0, sticky='new')

        btn_frame = tk.Frame(container)
        btn_frame.grid(column=0, row=1, sticky='new', padx=800/3, pady=5)
        btn_frame.grid_columnconfigure(0, weight=3, uniform='btn')
        btn_frame.grid_columnconfigure(1, weight=3, uniform='btn')

        self.display_btn = tk.Button(btn_frame, text='Display', cursor='hand2')
        self.display_btn['bg'] = 'red'
        self.display_btn['fg'] = 'white'
        self.display_btn.grid(column=0, row=0, sticky='new', padx=2, pady=2)
        self.display_btn.bind('<Enter>', lambda event: self.hover(self.display_btn))

        self.add_btn = tk.Button(btn_frame, text='Add', cursor='hand2')
        self.add_btn['bg'] = 'red'
        self.add_btn['fg'] = 'white'
        self.add_btn.grid(column=1, row=0, sticky='new', padx=4, pady=4)
        self.add_btn.bind('<Enter>', lambda event: self.hover(self.add_btn))

    def hover(self, btn):
        btn['activebackground'] = 'white'
        btn['activeforeground'] = 'red'



class Controller:
    def __init__(self, data, window):
        ''' Class will handle communications between Data and Window classes '''
        self.data = data 
        self.window = window


        # Button Commands
        self.window.display_btn['command'] = self.display
        self.window.add_btn['command'] = self.add 


    def display(self):
        print('display here')

    def add(self):
        print('Add data')


if __name__ == '__main__':
    root = tk.Tk()
    controller = Controller(Data(), Window(root))
    root.mainloop()

Hey again, your code actually look much better and professional and I see there is good use of OOP here and I didnt even think about using it , I am waiting to see the full code you will right as I see I have a lot to learn

[Sr999]

You're not wrong: hashing is a one way function.

What I had in mind would be over complicated for an app such as this.

I am not an expert in the use of encryption and many people who do put themselves in that category have made fundamental errors which have completely compromised the user data; the LastPass Data Breach possibly being the most high profile one in recent times.

I know your app is not being offered "as a service", but it's still worth keeping in mind just how difficult it is to get this kind of thing right and how easy it is to get it wrong.

I will again have a look at your app and feedback if I see anything that I think you should re-visit.

Same I had in mind because it’s not offered as service and I am in a learning journey I wanted to try to add this addition to the program
so I used simple hash-encrypt methods I found online after lil dig

Again appreciate your time and I am
Waiting for your review
Thanks again

[rob101]

Again appreciate your time and I am
Waiting for your review
Thanks again

You are very welcome and while I don't want to come across as an "ass" here, it was very easy to circumvent your 'admin password', so without knowing what that password is, I can see that the fake google login password is 123123.

I'm not saying that what you've created here is not good in its own right, it's just I don't think it should be used (as it is) for any serious password management, but by putting this up on GitHub, you are kind of suggesting that it could be used in such a way: sorry to be so negative about it.

Possibly (but I don't know, because I've never tried to do this) the way this could work, is to have the .csv file stored in an encrypted .zip archive (or the like of). I do know that one can create such an archive at will, but I've no clear idea how you could have your app access and use that, but it's an idea.