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

Securely save user credentials #8

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

mkpro118
Copy link

@mkpro118 mkpro118 commented Jul 3, 2024

Rationale

The current implementation of lazycodr's saved credentials has API keys and PATs stored as plaintext in json files. This PR aims to securely store these credentials using a password+random-salt based encryption strategy.

What's Changed

  • User credentials are encrypted before saving to disk.
  • Operations that require credentials now have a password prompt. Credentials cannot be decrypted without the correct password.
  • Updated the lazycodr config credentials command to include a prompt for passwords
  • Added the following subcommands to lazycodr config
    • update-password: To update the user's passwords.
    • delete-credentials: To delete all existing credentials. Note that this still requires a password. (Actually this could effectively be accomplished by a simple rm <credentials_file>, if one knows what the credentials_file is)
  • Prompts updated as necessary.
  • Minor CLI change, the progress bar for the PR generator is paused for password prompts.

What has not changed

  • All operation are unaffected, and continue as normal.
  • The credentials are still stored are JSON files at the default location (but the content values are encrypted)
  • The existing functions that use the @use_credentials decorator expect a dict instance and access credentials using the [] syntax. The CredentialManager implements the corresponding __getitem__ method, so the existing implementation is still syntactically compatible.

Security

  • The CredentialManager makes every effort to never store raw passwords or credentials. It actually intentionally loses its references to raw passwords and credentials and operates on hashes and encrypted credentials.
  • It never stores or caches unencrypted/decrypted credentials.
  • Encryption keys are generated securely using the system's hardware configuration, so they are machine specific. This disallows decryption of the credentials on machines different from the ones they were encrypted on.
  • Encryption keys are generated securely using the python standard libraries secrets module.
  • Password hashes are generated using the python standard libraries hashlib module.

Known limitations

  • Users can potentially lose work if passwords are incorrect. For example, if one is creating a PR with a long/complicated template, entering the password incorrectly will cause the program to abort, which will lose all the edits made. This may be mitigated if Store the PR template between calls #4 is implemented.
  • Slight overhead due to the decryption performed on every credential access. (I do not believe that it is noticeable, but I have not confirmed this with any benchmarks)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant