Fix keyring - name collision bug #16
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a Quest ID is the same as the system username the Quest password concatenated with the username was treated as the default user. The mitigation for this was to prefix the current username with "_config_" and treat that as the username to store the user comma separated list. To make things more clear the variable name 'current' was changed to 'key_to_users_csv'. This fix was manually tested. It does not seem like this project has unit tests although a harness exists.
|
Sorry about the late handle on this. Reviewing it now. |
| @@ -104,8 +106,8 @@ def remove_user(username): | |||
| raise NoUserException("Error: expected a username, given None.") | |||
| if not user_exists(username): | |||
| raise NoUserException("Error: %s is not a valid user."% username) | |||
| current = getpass.getuser() | |||
| users = keyring.get_password(PROG, current) | |||
| key_to_users_csv = getpass.getuser() | |||
There was a problem hiding this comment.
key_to_users_csv = CONFIG_PREFIX + getpass.getuser()
|
Might be good to just make a helper function instead of add the prefix in every instance, makes it easier to modify if anything. def getuser():
return CONFIG_PREFIX + getpass.getuser()or something similar |
|
Read |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See
97380dafor details on fix for #15.I also changed the package to use the newest available keyring package. The currently available one works fine but you may want to freeze it there instead of using >=. I will leave that up to you.