-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: allow keychain interactions without a password #1548
Conversation
go-ipfs allows you to list keys, create keys, etc without using a keychain password. This PR brings js-ipfs in line with that though will need a few other PRs merging first, if we think this is a good idea.
@@ -19,6 +19,7 @@ describe('key', () => runOnAndOff.off((thing) => { | |||
this.timeout(40 * 1000) | |||
|
|||
return ipfs(`${pass} key gen ${name} --type rsa --size 2048`) | |||
.then(() => ipfs(`${pass} key list -l`)) |
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.
This change changes the assertion as now it's testing on the output of key list
rather that the name is included in the output when running key gen
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.
js-ipfs
used to do:
$ jsipfs key gen foobar
generated Qmb1Wj3YR4PGgscvXJ3JgaiX6GCQFXLwP6Qo7c5kNR6PQK foobar
but to bring it inline with go-ipfs
it now outputs:
$ jsipfs key gen foobar
Qmb1Wj3YR4PGgscvXJ3JgaiX6GCQFXLwP6Qo7c5kNR6PQK
so the name is no longer in the output and the test would fail. In order to verify that we have generated a key with the passed name, I changed the test to examine the output of jsipfs key list
after generating the key which includes the name.
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.
@diasdavid & @Stebalien could we get a steer on whether this is somehing js-ipfs should be dropping to or if go-ipfs should start requiring keychain interactions without a password?
@@ -5,7 +5,7 @@ const expect = require('chai').expect | |||
const runOnAndOff = require('../utils/on-and-off') | |||
const hat = require('hat') | |||
|
|||
describe('key', () => runOnAndOff.off((thing) => { | |||
describe.only('key', () => runOnAndOff.off((thing) => { |
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.
Reminder to remove this before merge ;)
So, go-ipfs has never supported encrypted keys but I believe it has been on the "todo" list for a while (I can see some remnants in the code). However, IMO, it makes sense to support keys without a password (e.g., for system daemons). Note: the password should not be passed as a command-line flag. Ideally, on repo init, IPFS would ask for a password (or allow a Passing a password on the command-line pretty much defeats the point:
|
Hi! js-ipfs master just got a whole new set of automated tests with #2528, #2440 and also running some of the test suites from our early testers (hi5 to @achingbrain for setting it all up!). Would you mind rebasing the master branch on this PR to ensure it runs all the latest tests? Thank you! |
This was fixed! |
go-ipfs allows you to list keys, create keys, etc without using a keychain password. This PR brings js-ipfs in line with that though will need a few other PRs merging first, if we think this is a good idea.