Skip to content

Add auto-generate password option to get_password dialog#71

Open
marcos-mendez wants to merge 1 commit intoturnkeylinux:masterfrom
popsolutions:master
Open

Add auto-generate password option to get_password dialog#71
marcos-mendez wants to merge 1 commit intoturnkeylinux:masterfrom
popsolutions:master

Conversation

@marcos-mendez
Copy link
Copy Markdown
Member

  • Add generate_password() using secrets module (CSPRNG)
  • get_password() now offers 'Generate' (recommended) or 'Manual' menu
  • Generated passwords shown in centered reverse-video box with colors
  • Confirmation step ensures user saved the password before proceeding
  • Fully backward compatible: offer_generate=True by default
  • Pass offer_generate=False for original behavior
  • Imports: added secrets, string (stdlib only)

- Add generate_password() using secrets module (CSPRNG)
- get_password() now offers 'Generate' (recommended) or 'Manual' menu
- Generated passwords shown in centered reverse-video box with colors
- Confirmation step ensures user saved the password before proceeding
- Fully backward compatible: offer_generate=True by default
- Pass offer_generate=False for original behavior
- Imports: added secrets, string (stdlib only)
@JedMeister
Copy link
Copy Markdown
Member

Thanks again @marcos-mendez - LGTM. Although I'm not sure why you removed a number of the existing docstrings? Plus I also noticed that there were some mistakes in the typing (that I did previously). So I've opened a new PR based on this one (i.e. this commit with one more on top).

Leave this PR open and it will be auto marked as merged once the new one I opened is merged.

Also as a somewhat related FYI; looking over this code (both yours and mine) prompted me to open an issue relating to code style and linting. The detail I noted in the issue has been rattling around in my head for some time but I hadn't actually documented it anywhere. So now I have! 😁

@JedMeister
Copy link
Copy Markdown
Member

And actually... I just had a quick offline chat about this with @OnGle. He raised some additional considerations that hadn't occurred to me. Rather than risk misrepresenting him, I'll let him have a closer look and comment himself. Although he's knocked off so won't get to that until next week.

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.

2 participants