[FAB-3415] Improve LDAP usage and error handling

1) Do not try to load users into the DB when LDAP is enabled.
2) Don't require the -b option when LDAP is enabled.

Specifically, changes are as follows:
a) cmd/fabric-ca-server/config.go
   Added some comments (per Yang's request).
   Require the '--boot' option only if LDAP is disabled.
b) cmd/fabric-ca-server/main.go
   Change server usage message to denote that "--boot" is required only if ldap.enabled is false.
c) cmd/fabric-ca-server/main_test.go
   Added test case
d) doc/source/users-guide.rst
   Updated doc to indicate that "-b" (or "--boot") is required only if ldap.enabled is false.
e) lib/ca.go
   Load the user's table into the DB only if LDAP is disabled.
f) lib/client_test.go
   Remove test directories that weren't being cleaned up.

Change-Id: Ic673d628e43b057381ea3af29d64d60cd9834847
Signed-off-by: Keith Smith <bksmith@us.ibm.com>

cmd/fabric-ca-server/config.go

@@ -134,9 +134,12 @@ ca:
 #     These attributes are useful for making access control decisions in
 #     chaincode.
 #  There are two main configuration options:
-#  1) The fabric-ca-server is the registry
+#  1) The fabric-ca-server is the registry.
+#     This is true if "ldap.enabled" in the ldap section below is false.
 #  2) An LDAP server is the registry, in which case the fabric-ca-server
 #     calls the LDAP server to perform these tasks.
+#     This is true if "ldap.enabled" in the ldap section below is true,
+#     which means this "registry" section is ignored.
 #############################################################################
 registry:
   # Maximum number of times a password/secret can be reused for enrollment
@@ -186,6 +189,7 @@ db:
 #############################################################################
 ldap:
    # Enables or disables the LDAP client (default: false)
+   # If this is set to true, the "registry" section is ignored.
    enabled: false
    # The URL of the LDAP server
    url: ldap://<adminDN>:<adminPassword>@<host>:<port>/<base>
@@ -411,26 +415,37 @@ func configInit() (err error) {
 }
 
 func createDefaultConfigFile() error {
-	// Create a default config, but only if they provided an administrative
-	// user ID and password
-	up := viper.GetString("boot")
-	if up == "" {
-		return errors.New("The '-b user:pass' option is required")
-	}
-	ups := strings.Split(up, ":")
-	if len(ups) < 2 {
-		return fmt.Errorf("The value '%s' on the command line is missing a colon separator", up)
-	}
-	if len(ups) > 2 {
-		ups = []string{ups[0], strings.Join(ups[1:], ":")}
-	}
-	user := ups[0]
-	pass := ups[1]
-	if len(user) >= 1024 {
-		return fmt.Errorf("The identity name must be less than 1024 characters: '%s'", user)
-	}
-	if len(pass) == 0 {
-		return errors.New("An empty password in the '-b user:pass' option is not permitted")
+	var user, pass string
+	// If LDAP is enabled, authentication of enrollment requests are performed
+	// by using LDAP authentication; therefore, no bootstrap username and password
+	// are required.
+	ldapEnabled := viper.GetBool("ldap.enabled")
+	if !ldapEnabled {
+		// When LDAP is disabled, the fabric-ca-server functions as its own
+		// identity registry; therefore, we require that the default configuration
+		// file have a bootstrap username and password that is used to enroll a
+		// bootstrap administrator.  Other identities can be dynamically registered.
+		// Create the default config, but only if they provided this bootstrap
+		// username and password.
+		up := viper.GetString("boot")
+		if up == "" {
+			return errors.New("The '-b user:pass' option is required")
+		}
+		ups := strings.Split(up, ":")
+		if len(ups) < 2 {
+			return fmt.Errorf("The value '%s' on the command line is missing a colon separator", up)
+		}
+		if len(ups) > 2 {
+			ups = []string{ups[0], strings.Join(ups[1:], ":")}
+		}
+		user = ups[0]
+		pass = ups[1]
+		if len(user) >= 1024 {
+			return fmt.Errorf("The identity name must be less than 1024 characters: '%s'", user)
+		}
+		if len(pass) == 0 {
+			return errors.New("An empty password in the '-b user:pass' option is not permitted")
+		}
 	}
 
 	var myhost string

cmd/fabric-ca-server/main.go

@@ -57,7 +57,7 @@ func init() {
 	pflags := rootCmd.PersistentFlags()
 	pflags.StringVarP(&cfgFileName, "config", "c", cfg, "Configuration file")
 	util.FlagString(pflags, "boot", "b", "",
-		"The user:pass for bootstrap admin which is required to build default config file")
+		"The user:pass for bootstrap admin; it is required to build default config file when ldap.enabled is false")
 
 	// Register flags for all tagged and exported fields in the config
 	serverCfg = &lib.ServerConfig{}

cmd/fabric-ca-server/main_test.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"path"
 	"path/filepath"
 	"regexp"
 	"testing"
@@ -29,8 +30,9 @@ import (
 )
 
 const (
-	initYaml  = "i.yaml"
-	startYaml = "s.yaml"
+	initYaml    = "i.yaml"
+	startYaml   = "s.yaml"
+	ldapTestDir = "ldapTestDir"
 )
 
 var (
@@ -130,6 +132,24 @@ func TestErrors(t *testing.T) {
 	viper.SetDefault("ca.name", "acme.com")
 }
 
+func TestLDAP(t *testing.T) {
+	os.RemoveAll(ldapTestDir)
+	defer os.RemoveAll(ldapTestDir)
+	// Test with "-b" option
+	err := RunMain([]string{cmdName, "init", "-c", path.Join(ldapTestDir, "config.yaml"),
+		"-b", "a:b", "--ldap.enabled", "--ldap.url", "ldap://CN=admin@localhost:389/dc=example,dc=com"})
+	if err != nil {
+		t.Errorf("Failed to init server with LDAP enabled and -b: %s", err)
+	}
+	// Try without "-b" option
+	os.RemoveAll(ldapTestDir)
+	err = RunMain([]string{cmdName, "init", "-c", path.Join(ldapTestDir, "config.yaml"),
+		"--ldap.enabled", "--ldap.url", "ldap://CN=admin@localhost:389/dc=example,dc=com"})
+	if err != nil {
+		t.Errorf("Failed to init server with LDAP enabled and no -b: %s", err)
+	}
+}
+
 func TestValid(t *testing.T) {
 	os.Unsetenv(homeEnvVar)
 	blockingStart = false
@@ -269,6 +289,7 @@ func TestClean(t *testing.T) {
 	os.RemoveAll("../../testdata/msp")
 	os.Remove("../../testdata/fabric-ca-server.db")
 	os.Remove("../../testdata/ca-cert.pem")
+	os.RemoveAll(ldapTestDir)
 }
 
 func cleanUpMultiCAFiles() {

docs/source/users-guide.rst

@@ -127,7 +127,10 @@ The following starts the `fabric-ca-server` with default settings.
     # fabric-ca-server start -b admin:adminpw
 
 The `-b` option provides the enrollment ID and secret for a bootstrap
-administrator.  A default configuration file named `fabric-ca-server-config.yaml`
+administrator; this is required if LDAP is not enabled with the "ldap.enabled"
+setting.
+
+A default configuration file named `fabric-ca-server-config.yaml`
 is created in the local directory which can be customized.
 
 Start Server via Docker
@@ -745,9 +748,11 @@ Initialize the Fabric CA server as follows:
 
     # fabric-ca-server init -b admin:adminpw
 
-The ``-b`` (bootstrap identity) option is required for initialization. At
-least one bootstrap identity is required to start the Fabric CA server. The
-server configuration file contains a Certificate Signing Request (CSR)
+The ``-b`` (bootstrap identity) option is required for initialization when
+LDAP is disabled. At least one bootstrap identity is required to start the
+Fabric CA server; this identity is the server administrator.
+
+The server configuration file contains a Certificate Signing Request (CSR)
 section that can be configured. The following is a sample CSR.
 
 If you are going to connect to the Fabric CA server remotely over TLS,

lib/ca.go

@@ -465,9 +465,13 @@ func (ca *CA) initDB() error {
 
 	// If the DB doesn't exist, bootstrap it
 	if !exists {
-		err = ca.loadUsersTable()
-		if err != nil {
-			return err
+		// Since users come from LDAP when enabled,
+		// load them from the config file only when LDAP is disabled
+		if !ca.Config.LDAP.Enabled {
+			err = ca.loadUsersTable()
+			if err != nil {
+				return err
+			}
 		}
 		err = ca.loadAffiliationsTable()
 		if err != nil {
@@ -488,6 +492,11 @@ func (ca *CA) initUserRegistry() error {
 		// Use LDAP for the user registry
 		ca.registry, err = ldap.NewClient(ldapCfg, ca.server.csp)
 		log.Debugf("Initialized LDAP identity registry; err=%s", err)
+		if err == nil {
+			log.Info("Successfully initialized LDAP client")
+		} else {
+			log.Warningf("Failed to initialize LDAP client; err=%s", err)
+		}
 		return err
 	}
 

lib/client_test.go

@@ -382,4 +382,6 @@ func TestLast(t *testing.T) {
 	// Cleanup
 	os.RemoveAll("../testdata/msp")
 	os.RemoveAll(serversDir)
+	os.RemoveAll("multica")
+	os.RemoveAll("rootDir")
 }