-
Notifications
You must be signed in to change notification settings - Fork 3.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
commands: require wallet password for lnpay command #9238
Conversation
might be useful to also require password for e.g. channel closes and reverse swaps |
Previously it was only the actual commands that directly or indirectly verified the password. This adds a check that runs for any command requiring a password. related #9238
electrum/commands.py
Outdated
if wallet.has_password(): | ||
wallet.check_password(password) |
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.
Conceptually this should not be done here I think.
I moved this check to the @command
decorator in 37d090c.
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.
agreed
Well, yes, maybe. But also something to consider is what exactly is the threat-model, because at the end of the day we might as well password-protect every command :P
Anyway, I would say let's password-protect the "dangerous" list (1). Do you think list (2) should be password-protected as well? I am not sure. |
These are currently inconsistent across GUIs (PIN on android, if enabled. no auth on desktop). |
Okay, let's do that then. |
…channel_backup, reverse_swap, rebalance_channels
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.
Thanks!
Previously it was only the actual commands that directly or indirectly verified the password. This adds a check that runs for any command requiring a password. related #9238
ref #9236