Improve rpcauth.py by using argparse and getpass modules #14756

pull promag wants to merge 1 commits into bitcoin:master from promag:2018-11-improve-rpcauth.py changing 2 files +28 −22

promag commented at 3:15 PM on November 19, 2018: member

This PR improves argument handling in rpcauth.py script by using argparse module. Specifying - as password makes it prompt securely with getpass module which prevents leaking passwords to bash history.
promag commented at 3:15 PM on November 19, 2018: member

Based on #14742.
practicalswift commented at 3:28 PM on November 19, 2018: contributor

Concept ACK

Nice usability improvement!
promag renamed this:
~~Improve by using argparse and getpass modules~~
Improve rpcauth.py by using argparse and getpass modules
on Nov 19, 2018
dongcarl commented at 3:57 PM on November 19, 2018: member

utACK
fanquake added the label Scripts and tools on Nov 19, 2018
promag force-pushed on Nov 21, 2018
promag commented at 9:44 AM on November 21, 2018: member

Rebased.
promag force-pushed on Nov 21, 2018
laanwj commented at 1:24 PM on November 21, 2018: member

You're changing a ton of unrelated things in this PR, making it hard to review.

This is not a very complicated script so I'll utACK it, but please don't do this next time.
promag commented at 1:33 PM on November 21, 2018: member

@laanwj indeed it escalated, the original intent was to add getpass module.
jnewbery commented at 9:07 PM on November 21, 2018: member
Conditional tested ACK ffba16cab0f1549d8b5b18bd254fb52ab44b2876.

This changes the interface to rpcauth.py: previously rpcauth.py <username> <pw> would work. Now it errors with:
```
/rpcauth.py satoshi p4ssw0rd
usage: rpcauth.py [-h] [-p PASSWORD] username
rpcauth.py: error: unrecognized arguments: p4ssw0rd
```
Is that ok? If this script is only used manually and occasionally, then it's fine, but if it's called by other scripts it could be annoying.

Note that we don't have a standalone test for rpcauth, and the coverage provided by rpc_users.py doesn't test providing a password, so didn't catch this interface change.
promag commented at 9:17 PM on November 21, 2018: member

@jnewbery I guess I can make it compatible.
promag commented at 9:42 PM on November 21, 2018: member

@jnewbery updated to maintain the same interface. (I'll squash and update OP after feedback)
jnewbery commented at 10:01 PM on November 21, 2018: member

tACK b8b321776e43b0fbf7a3e131e3af827eb892768b. Much better, thanks!
rpcauth: Improve by using argparse and getpass modules d6cde007db
promag force-pushed on Nov 21, 2018
promag commented at 10:32 PM on November 21, 2018: member

Thanks @jnewbery, squashed and updated OP.
jnewbery commented at 10:56 PM on November 21, 2018: member

ACK d6cde007db9d3e6ee93bd98a9bbfdce9bfa9b15b. Verified the same as b8b3217
laanwj merged this on Nov 22, 2018
laanwj closed this on Nov 22, 2018
laanwj referenced this in commit 708cbb172d on Nov 22, 2018
PastaPastaPasta referenced this in commit 2932a58bae on Jul 19, 2020
PastaPastaPasta referenced this in commit 5b38df433f on Jul 22, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels