-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[ENH] CredentialManager: Store passwords in System Keyring Services #1641
Conversation
Current coverage is 88.72% (diff: 100%)@@ master #1641 diff @@
==========================================
Files 78 78
Lines 8148 8148
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
Hits 7229 7229
Misses 919 919
Partials 0 0
|
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.
Sure, there is a test. But a short how-to-use example in a module docstring would also be nice.
Short example added. |
keyring.set_password(self.service_name, self.username, value) | ||
|
||
def delete_password(self): | ||
keyring.delete_password(self.service_name, self.username) |
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.
Can these methods be replaced with __setattr__
, __getattr__
, and __delattr__
so the supported interface becomes more like a general secure key-value store?
|
||
class CredentialManager: | ||
def __init__(self, username): | ||
""" |
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.
Class docstring goes outside __init__
.
username: username used for storing a matching password. | ||
|
||
Examples: | ||
>>> cm = CredentialManager('Foo') |
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.
cm = CredentialManager('Some Widget')
|
||
@property | ||
def key(self): | ||
return keyring.get_password(self.service_name, self.username) |
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.
Should getter return None by default?
|
||
|
||
class CredentialManager: | ||
def __init__(self, username): |
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.
Service name (namespace), not username (key).
b2c0129
to
7fa17a2
Compare
@kernc please check again. |
Credential Manager is used for safely storing passwords in system's keyring services.
Credential Manager is used for safely storing passwords in system's keyring services.