-
Notifications
You must be signed in to change notification settings - Fork 403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhancements to Secret Management #364
Conversation
…dio into max/use_keyring_storage
I see. So the warnings are from the main branch? |
Yes, they arise when calling |
Created an issue for tracking: #372 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
(you might consider adding type annotation for what the following two functions return though: _get_secrets_handler, _get_usersettings_path)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this security related upgrade, @maxjeblick
I still ran into issues when just upgrading to the PR branch, but using a clean environment everything was working fine. I think we can live with that.
lgtm
Sorry for the late reply @pascal-pfeiffer, I ran into some new timeout issues after I tried to configure keyring on the remote machine. Compared to the reviewed PR, I added the following:
Please feel free to re-review the changes. |
This pull request adds different backend handlers for storing passwords with
keyring
being the default.On systems where keyring fails, it is possible to select a local .env backend, or not to save the tokens at all.
Purpose
The purpose of these changes is to provide the application with greater flexibility and security when it comes to managing and storing sensitive information.
Backward compatibility
A migration process is implemented that converts the old pickle format.
On systems where migration to keyring fails, the user needs to manually reenter missing tokens (this is mentioned in the logs).
Fixes #363