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

[FABC-905] Add passfile to server config file template #210

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stephyee
Copy link
Contributor

Type of change

  • Bug fix

Description

Add server config option passfile. This config will take precedence over password and will not be stored in the generated yaml.

Related issues

FABC-905

@stephyee stephyee force-pushed the fabc-905 branch 7 times, most recently from c9fe885 to 7b34d63 Compare January 12, 2021 22:08
@stephyee stephyee marked this pull request as ready for review January 12, 2021 22:30
@stephyee stephyee requested review from a team as code owners January 12, 2021 22:30
cmd/fabric-ca-server/config.go Show resolved Hide resolved
cmd/fabric-ca-server/main_test.go Outdated Show resolved Hide resolved
lib/ca_test.go Outdated Show resolved Hide resolved
lib/ca_test.go Outdated Show resolved Hide resolved
@@ -154,6 +154,8 @@ func (s *ServerCmd) registerFlags() {
pflags.StringVarP(&s.homeDirectory, "home", "H", "", fmt.Sprintf("Server's home directory (default \"%s\")", filepath.Dir(cfg)))
util.FlagString(s.myViper, pflags, "boot", "b", "",
"The user:pass for bootstrap admin which is required to build default config file")
util.FlagString(s.myViper, pflags, "bootfile", "f", "",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usage question here: when I try to specify a username and passfile, it complains because I haven't specified a password after a colon separator:
fabric-ca-server init -b admin -f pass.file
Error: Failed to create default configuration file: The value 'admin' on the command line is missing a colon separator

While it isn't too difficult to add the : after admin, it feels like we shouldn't require that when -f is used. Any reason that shouldn't be the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed, this was a design choice (trying to change as little as possible). The init and start commands still require both -b admin:pass with the -f pass.file flag specified. The pass from -b user:pass will be used as a backup when a password cannot be read from pass.file when starting server.

Another option as you suggested is to allow only -b admin when -f pass.file is provided then

  1. Allow the server to error on startup if the pass.file cannot be read.
  2. Error when creating the default config file if the pass.file doesn't exist, cannot be read, etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sykesm Any thoughts / opinions here?

Adds support for bootstrapping server using a password file. The
password file will take precedence over password specified in config or
flag.

Signed-off-by: Tiffany Harris <tiffany.harris@ibm.com>
Base automatically changed from master to main March 13, 2021 17:41
@ryjones
Copy link
Member

ryjones commented Jun 16, 2021

@hyperledger/fabric-core-maintainers @hyperledger/fabric-ca-maintainers I attempted a rebase to resolve the conflicts, but I don't know the code well enough to make a choice. Could someone please rebase this for merging?

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.

3 participants