Skip to content
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

Refactor askpass to return the password only #70

Open
covert-encryption opened this issue Jan 4, 2022 · 3 comments
Open

Refactor askpass to return the password only #70

covert-encryption opened this issue Jan 4, 2022 · 3 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@covert-encryption
Copy link
Owner

covert-encryption commented Jan 4, 2022

Currently it returns a tuple (password, valid visible), which is not meaningful for non-Covert passwords. Should refactor this utility function not to return validity but rather handle that via a separate function call if needed.

@covert-encryption covert-encryption added enhancement New feature or request help wanted Extra attention is needed labels Jan 4, 2022
@covert-encryption
Copy link
Owner Author

Actually the flag from that function is visibility (not validity), indicating whether a random-generated password was made (and also whether the user had password input visible otherwise), allowing the password to be displayed in console afterwards. Looks like this cannot be easily avoided but will need greater refactoring.

One possibility is implementing a holder class for passphrases, similar to pubkey.Key class, where extra functionality can be more easily be implemented.

@covert-encryption
Copy link
Owner Author

Passwords definitely should be refactored into a class type, where additional information such as the string shown in UI may be more easily included, and which could then be further developed to do the password hashing in a background thread internally (this benefits GUI and avoids the current code in CLI for handling that - it is better to share the same code with both).

Then askpass could just return that type and avoid much confusion.

@covert-encryption
Copy link
Owner Author

This issue is of high priority.

Adding a class Password used in all passphrase handling is needed, avoiding the confusion of current implementation which sometimes uses bytes and sometimes str, and often a tuple to include visibility information. All could use a class type instead to carry all the necessary information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant