GauriSP10 / streamlit_login_auth_ui

It lets you connect your streamlit application to a pre-built and secure Login/ Sign-Up page.
MIT License
202 stars 62 forks source link

Updates on username, delete account, pytest and others #10

Closed fsmosca closed 11 months ago

fsmosca commented 1 year ago

Changes

Delete account feature

To try this branch, you can install it with:

pip install git+https://github.com/fsmosca/streamlit_login_auth_ui.git@username-is-not-case-sensitive

If that does not work, clone the repo, cd to package and install with setup.

git clone -b username-is-not-case-sensitive https://github.com/fsmosca/streamlit_login_auth_ui.git
cd streamlit_login_auth_ui
pip install .
darrida commented 1 year ago

I think this looks good. My only strong suggestion would be related to the password length amounts:

Current standards tend to specify a minimum length of 8 (or 12), and a maximum length of at least 64 characters.

If it's using argon2 for hashing, then the actual hash length that is stored is consistent, regardless of the password length. Personally, when a tool limits my password length to something as short as 16 characters I start to wonder if they are actually storing my password securely (since correct hashing renders the length of my own password not very meaningful -- so, why would they be limiting it?). This is storing the password in a secure format.

(I'm not a contributor yet -- just an interested developer - but have been working on a PR for this that adds options for some custom handlers (enabling developers to use different auth, storage, and cookie mechanisms) and browser based tests for the front end functionality of this library).

darrida commented 1 year ago

I'd also be interested to discuss where the name of the auth json file lives. In my own work (that is not yet a PR) I added the ability to pass in a custom handler for alternate storage methods. One has the option to pass in their own parameters to their handlers when initializing the login class.

For the default storage method that currently exists, I abstracted it to a handler that is a default option when the login class is initialized.

My changes may be too significant to be a PR -- though I did retain all existing functionality, and also created a _legacy.py module to allow all current examples and uses of the library to continue to function too. I've not submitted my PR yet, but @GauriSP10 may find it too much for a PR (mine is far too much for a single PR, but I loss track of how much I was adding). If it does end up being too much I can spin it into its own library -- but it would be nice to have a single auth library like this for streamline.

Essentially, in the Streamlit community forums there were so many questions about different forms of functionality I thought it might be helpful to move in the direction of offering a modular (user storage, auth-type, what cookies are stored/verified, password reset message method, etc) approach to non-core functionality (login, managed sessions, etc).

fsmosca commented 1 year ago

I think this looks good. My only strong suggestion would be related to the password length amounts:

Current standards tend to specify a minimum length of 8 (or 12), and a maximum length of at least 64 characters.

Password validation is in my todo list. Currently in the main branch, a single char is accepted. I will add in this branch, a min 8 and max 64, and no whitespace.

(I'm not a contributor yet -- just an interested developer - but have been working on a PR for this that adds options for some custom handlers (enabling developers to use different auth, storage, and cookie mechanisms) and browser based tests for the front end functionality of this library).

That sounds interesting.

What I plan is to add a deta database support to save user registrations info. The developer will provide the project key and deta database name.

fsmosca commented 1 year ago

I'd also be interested to discuss where the name of the auth json file lives.

Locally there should not be a problem. In the cloud there can be a lot of options. When deployed in streamlit cloud (free), saving the file there is problematic. Currently the main and this branch saves users auth file in the streamlit server and this is not good. As what I said in the other post, for this branch I plan to add a deta database support for saving users auth file.

darrida commented 1 year ago

Password validation is in my todo list. Currently in the main branch, a single char is accepted. I will add in this branch, a min 8 and max 64, and no whitespace.

Sounds great.

fsmosca commented 1 year ago

Password validation is in my todo list. Currently in the main branch, a single char is accepted. I will add in this branch, a min 8 and max 64, and no whitespace.

Sounds great.

It is added.

darrida commented 1 year ago

Locally there should not be a problem. In the cloud there can be a lot of options. When deployed in streamlit cloud (free), saving the file there is problematic. Currently the main and this branch saves users auth file in the streamlit server and this is not good. As what I said in the other post, for this branch I plan to add a deta database support for saving users auth file.

I mainly meant in terms of if the name of the file/flag for using it should live in the main login class constructor in general -- or if it should live in it's own user storage handler (see further below).

The concept below is very much my own concept that might be outside of or much broader than the scope @GauriSP10 has in mind for this library -- but related to your database storage need, here is what I'm looking at.

If I contribute my custom handlers concept, here is an example of custom storage created without the need to modify the core library (i.e., wouldn't have to be wait for a contribution to be considered/accepted, and update to be pushed).

https://github.com/darrida/streamlit_login_auth_ui/blob/migrate-cookies-manager/samples/dbmodel_storage.py

This is an example situation I wanted to use SQLModel (https://sqlmodel.tiangolo.com) to store user accounts/hashed passwords. I used the UserAuth and UserStorage Protocol classes as models to create my own. At the bottom of that example dbmodel_storage.py module you can see that the custom auth and storage handlers are passed into the main login class when it's initialized, and then everything works just like it would with the default storage (currently the standard local json file).

For this, the library's default auth and storage code have also been moved to handlers (_.handlers.py modeled after the same handler protocols (protocols.py). They are just set as the default arguments for the relevant parameters when no custom auth or user storage is passed in.

Hypothetically this same approach would also work with the deta database solution you want to use (and most any solution anyone else would want to use).

What if I wait for the @GauriSP10 to accept your PR -- after which I'll update my fork from that to incorporate your changes listed above. Then I'll submit my own that includes the custom handlers approach to see if @GauriSP10 if interested. If they aren't, then I'll hold onto my fork and publish my own library (even though my preference would be for there to be a single main library for this -- that, and it's @GauriSP10 that figured out the central pieces of this, so would prefer to add to their work rather than just copy, attribute, and do my own thing).

Anything else I add regarding this I'll put into it's own GitHub issue, since this is certainly outside of the scope of the this PR -- it just was somewhat relevant to @fsmosca's direction in terms of their ideas.

fsmosca commented 1 year ago

I am not sure if @GauriSP10 will accept my PR. Accepted or not, I have this branch that I can play around.

I am not familiar with sqlmodel, I will try to learn from it. Your storage will save users info in the sql db. With deta cloud, this sql db can be saved in a deta drive. This looks fine. The one that I plan is to save the users info directly in deta bases. Both should work.

I like your users storage system. Is there any other way to store the sqlmodel db in cloud for free?

darrida commented 1 year ago

I am not sure if @GauriSP10 will accept my PR. Accepted or not, I have this branch that I can play around.

I am not familiar with sqlmodel, I will try to learn from it. Your storage will save users info in the sql db. With deta cloud, this sql db can be saved in a deta drive. This looks fine. The one that I plan is to save the users info directly in deta bases. Both should work.

I like your users storage system. Is there any other way to store the sqlmodel db in cloud for free?

SQLModel is a database ORM, like SQLAlchemy (it's build on top of it) -- so it will work with most/all of the popular relational databases (Postgres, Oracle, MySQL, MariaDB, SQLite, etc). So, hypothetically any place you can use one of those for free you could use DBModel. But, using the user storage protocol, you could also adjust your work on the cloud database you mentioned to conform to the protocol class/methods in the same way I've done with SQLModel. Again, this assumes it becomes a thing that can be used.

karmataco commented 1 year ago

Thank you for your branch! I'm still seeing the "No module named Argon2" error when deploying to Streamlit Cloud. Any suggestions?