mkhorasani / Streamlit-Authenticator

A secure authentication module to validate user credentials in a Streamlit application.
Apache License 2.0
1.38k stars 229 forks source link

Adding id field to credentials breaks authentication on page reload #108

Closed eytan-ohana closed 5 months ago

eytan-ohana commented 5 months ago

Hi @mkhorasani,

I just wanted to start off with this is an excellent extension to streamlit!

In my use case, the users credentials (email and hashed password) are stored in a mongo database which is reloaded every time the user refreshes the page.

It looks like in the new release, v0.2.4, with the addition of the id key in the credentials, https://github.com/mkhorasani/Streamlit-Authenticator/blob/a498615e0b7c8705268395f3e489e87f673a66c1/streamlit_authenticator/authenticate.py#L51

whenever the user reloads the page the id key is reset (because the session state is cleared) meaning now when the authenticator tries to search for the user based on the cookie/token here

https://github.com/mkhorasani/Streamlit-Authenticator/blob/a498615e0b7c8705268395f3e489e87f673a66c1/streamlit_authenticator/authenticate.py#L127

then it never actually finds the user because id has already been changed.

I hope that provides enough context :)

mkhorasani commented 5 months ago

Hi @eytan-ohana, thank you for reaching out. The ID field is not saved in the session state, it is actually saved in the config file just like the name, email, and hashed password. Please make sure you are saving new fields to your remote database to ensure that it does not break down.

mkhorasani commented 5 months ago

And just for your information the ID is not related to Streamlit's session_id, it is a randomly generated the first time the field is populated.

eytan-ohana commented 5 months ago

HI @mkhorasani thanks for the quick response. What I mean to say is every time the session state is refreshed the id is regenerated so without saving the id to my external db thats where the reauthentication fails.

Looks like youre right ill go ahead and save the generated ids to the db too. It looks like the only way i can do that is through accessing the authenticator objects credentials attribute after the object is created, which doesn't seem too clean to me but should still work fine just the same.

thanks for your support!

mkhorasani commented 5 months ago

No worries. It just needs to be saved once for each user, so I hope it won't be too much of a hassle.

Cheers.

On Tue, Jan 23, 2024 at 4:37 PM Eytan Ohana @.***> wrote:

HI @mkhorasani https://github.com/mkhorasani thanks for the quick response. What I mean to say is every time the session state is refreshed the id is regenerated so without saving the id to my external db thats where the reauthentication fails.

Looks like youre right ill go ahead and save the generated ids to the db too. It looks like the only way i can do that is through accessing the authenticator objects credentials attribute after the object is created, which doesn't seem too clean to me but should still work fine just the same.

thanks for your support!

— Reply to this email directly, view it on GitHub https://github.com/mkhorasani/Streamlit-Authenticator/issues/108#issuecomment-1906074224, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJIVK624EJU5P52VS4SGQUTYP64LLAVCNFSM6AAAAABCF2QWSOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBWGA3TIMRSGQ . You are receiving this because you were mentioned.Message ID: @.***>

mkhorasani commented 5 months ago

Hi @eytan-ohana, I have decided to axe Streamlit-Authenticator v0.2.5 and replace it with v0.3.1. The 'ID' field is now removed and you will no longer need to worry about it. Many thanks.

eytan-ohana commented 5 months ago

Thats awesome news @mkhorasani thanks for letting me know!

Can I ask what prompted that decision?

mkhorasani commented 5 months ago

I believe it will not provide any added value and it led to an influx of issues being raised by developers who were updating to the latest version without updating their code.

eytan-ohana commented 5 months ago

Makes sense Thank you!