[FAB-5726] 5. Dynamic Cfg - identities: Modify

This change set implements the functionality to modify
an identity.

Change-Id: I0434802aaceef397a77cf1117af4a12500376646
Signed-off-by: Saad Karim <[email protected]>

cmd/fabric-ca-client/identity.go

@@ -230,7 +230,7 @@ func (c *ClientCmd) runModifyIdentity(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	fmt.Printf("Successfully modified identity: %+v\n", resp)
+	fmt.Printf("Successfully modified identity: %+v\n", resp.IdentityInfo)
 
 	return nil
 }

cmd/fabric-ca-client/main_test.go

@@ -552,11 +552,11 @@ func TestIdentityCmd(t *testing.T) {
 
 	// Add user using JSON
 	err = RunMain([]string{
-		cmdName, "identity", "add", "testuser1", "--json", `{"secret": "user1pw", "type": "user", "affiliation": "org1", "max_enrollments": 1, "attrs": [{"name:": "hf.Revoker", "value": "true"}]}`})
+		cmdName, "identity", "add", "-d", "testuser1", "--json", `{"secret": "user1pw", "type": "user", "affiliation": "org1", "max_enrollments": 1, "attrs": [{"name": "hf.Revoker", "value": "false"},{"name": "hf.IntermediateCA", "value": "false"}]}`})
 	assert.NoError(t, err, "Failed to add user 'testuser1'")
 
 	err = RunMain([]string{
-		cmdName, "identity", "add", "testuser1", "--json", `{"secret": "user1pw", "type": "user", "affiliation": "org1", "max_enrollments": 1, "attrs": [{"name:": "hf.Revoker", "value": "true"}]}`})
+		cmdName, "identity", "add", "testuser1", "--json", `{"secret": "user1pw", "type": "user", "affiliation": "org1", "max_enrollments": 1, "attrs": [{"name:": "hf.Revoker", "value": "false"}]}`})
 	assert.Error(t, err, "Should have failed to add same user twice")
 
 	// Add user using flags
@@ -573,22 +573,53 @@ func TestIdentityCmd(t *testing.T) {
 	err = RunMain([]string{cmdName, "enroll", "-u", enrollURL})
 	util.FatalError(t, err, "Failed to enroll user")
 
-	server.CA.Config.Cfg.Identities.AllowRemove = true
-
 	registry := server.CA.DBAccessor()
-	_, err = registry.GetUser("testuser1", nil)
+	user, err := registry.GetUser("testuser1", nil)
 	assert.NoError(t, err, "Failed to get user 'testuser1'")
 
+	_, err = user.GetAttribute("hf.IntermediateCA")
+	assert.NoError(t, err, "Failed to get attribute")
+
 	_, err = registry.GetUser("testuser2", nil)
 	assert.NoError(t, err, "Failed to get user 'testuser2'")
 
+	// Modify value for hf.Revoker, add hf.Registrar.Roles, and delete hf.IntermediateCA attribute
+	err = RunMain([]string{
+		cmdName, "identity", "modify", "testuser1", "--type", "peer", "--affiliation", ".", "--attrs", "hf.Revoker=true,hf.Registrar.Roles=peer,hf.IntermediateCA="})
+	assert.NoError(t, err, "Failed to modify user 'testuser1'")
+
+	user, err = registry.GetUser("testuser1", nil)
+	assert.NoError(t, err, "Failed to get user 'testuser1'")
+
+	if user.GetType() != "peer" {
+		t.Error("Failed to correctly modify user 'testuser1'")
+	}
+	affPath := lib.GetUserAffiliation(user)
+	if affPath != "" {
+		t.Error("Failed to correctly modify user 'testuser1'")
+	}
+	attrs, err := user.GetAttributes(nil)
+	assert.NoError(t, err, "Failed to get user attributes")
+	attrMap := getAttrsMap(attrs)
+
+	val := attrMap["hf.Revoker"]
+	assert.Equal(t, "true", val.Value, "Failed to correctly modify attributes for user 'testuser1'")
+
+	val = attrMap["hf.Registrar.Roles"]
+	assert.Equal(t, "peer", val.Value, "Failed to correctly modify attributes for user 'testuser1'")
+
+	_, found := attrMap["hf.IntermediateCA"]
+	assert.False(t, found, "Failed to delete attribute 'hf.IntermediateCA'")
+
 	err = RunMain([]string{
 		cmdName, "identity", "remove", "testuser1"})
-	assert.NoError(t, err, "Failed to remove user")
+	assert.Error(t, err, "Should have failed, identity removal not allowed on server")
+
+	server.CA.Config.Cfg.Identities.AllowRemove = true
 
 	err = RunMain([]string{
-		cmdName, "identity", "modify", "testuser", "--type", "peer"})
-	assert.Error(t, err, "Should have failed, not yet implemented")
+		cmdName, "identity", "remove", "testuser1"})
+	assert.NoError(t, err, "Failed to remove user")
 }
 
 // Verify the certificate has attribute 'name' with a value of 'val'
@@ -2104,3 +2135,15 @@ func startServer(home string, port int, parentURL string, t *testing.T) *lib.Ser
 	}
 	return srv
 }
+
+func getAttrsMap(attrs []api.Attribute) map[string]api.Attribute {
+	attrMap := make(map[string]api.Attribute)
+	for _, attr := range attrs {
+		attrMap[attr.Name] = api.Attribute{
+			Name:  attr.Name,
+			Value: attr.Value,
+			ECert: attr.ECert,
+		}
+	}
+	return attrMap
+}

docs/source/servercli.rst

@@ -18,10 +18,11 @@ Fabric-CA Server's CLI
       -b, --boot string                               The user:pass for bootstrap admin which is required to build default config file
           --ca.certfile string                        PEM-encoded CA certificate file (default "ca-cert.pem")
           --ca.chainfile string                       PEM-encoded CA chain file (default "ca-chain.pem")
-          --ca.keyfile string                         PEM-encoded CA key file (default "ca-key.pem")
+          --ca.keyfile string                         PEM-encoded CA key file
       -n, --ca.name string                            Certificate Authority name
           --cacount int                               Number of non-default CA instances
           --cafiles stringSlice                       A list of comma-separated CA configuration files
+          --cfg.identities.allowremove                Enables removal of identities dynamically
           --crl.expiry duration                       Expiration for the CRL generated by the gencrl request (default 24h0m0s)
           --crlsizelimit int                          Size limit of an acceptable CRL in bytes (default 512000)
           --csr.cn string                             The common name field of the certificate signing request to a parent fabric-ca-server
@@ -50,7 +51,7 @@ Fabric-CA Server's CLI
           --ldap.userfilter string                    The LDAP user filter to use when searching for users (default "(uid=%s)")
       -p, --port int                                  Listening port of fabric-ca-server (default 7054)
           --registry.maxenrollments int               Maximum number of enrollments; valid if LDAP not enabled (default -1)
-          --tls.certfile string                       PEM-encoded TLS certificate file for server's listening port (default "tls-cert.pem")
+          --tls.certfile string                       PEM-encoded TLS certificate file for server's listening port
           --tls.clientauth.certfiles stringSlice      A list of comma-separated PEM-encoded trusted certificate files (e.g. root1.pem,root2.pem)
           --tls.clientauth.type string                Policy the server will follow for TLS Client Authentication. (default "noclientcert")
           --tls.enabled                               Enable TLS on the listening port

docs/source/users-guide.rst

@@ -1482,13 +1482,13 @@ The first method is via the `--json` flag where you describe the identity in a J
 
 .. code:: bash
 
-    fabric-ca-client identity add user1 --json '{"secret": "user1pw", "type": "user", "affiliation": "org1", "max_enrollments": 1, "attrs": [{"name:": "hf.Revoker", "value": "true"}]}'
+    fabric-ca-client identity add user1 --json '{"secret": "user1pw", "type": "user", "affiliation": "org1", "max_enrollments": 1, "attrs": [{"name": "hf.Revoker", "value": "true"}]}'
 
 The following adds a user with root affiliation. Note that an affiliation name of "." means the root affiliation.
 
 .. code:: bash
 
-    fabric-ca-client identity add user1 --json '{"secret": "user1pw", "type": "user", "affiliation": ".", "max_enrollments": 1, "attrs": [{"name:": "hf.Revoker", "value": "true"}]}'
+    fabric-ca-client identity add user1 --json '{"secret": "user1pw", "type": "user", "affiliation": ".", "max_enrollments": 1, "attrs": [{"name": "hf.Revoker", "value": "true"}]}'
 
 The second method for adding an identity is to use direct flags. See the example below for adding 'user1'.
 
@@ -1524,9 +1524,11 @@ is not modified will retain its original value.
 
 NOTE: A maxenrollments value of "-2" specifies that the CA's max enrollment setting is to be used.
 
+The command below make multiple modification to an identity using the --json flag.
+
 .. code:: bash
 
-    fabric-ca-client identity modify user1 --json '{"secret": "newPassword", "affiliation": "."}'
+    fabric-ca-client identity modify user1 --json '{"secret": "newPassword", "affiliation": ".", "attrs": [{"name": "hf.Regisrar.Roles", "value": "peer,client"},{"name": "hf.Revoker", "value": "true"}]}'
 
 The commands below make modifcations using direct flags. The following updates the enrollment secret (or password) for identity 'user1' to 'newsecret'.
 

lib/ca.go

@@ -842,7 +842,7 @@ func (ca *CA) addIdentity(id *CAConfigIdentity, errIfFound bool) error {
 		MaxEnrollments: id.MaxEnrollments,
 		Level:          ca.levels.Identity,
 	}
-	err = ca.registry.InsertUser(rec)
+	err = ca.registry.InsertUser(&rec)
 	if err != nil {
 		return errors.WithMessage(err, fmt.Sprintf("Failed to insert identity '%s'", id.Name))
 	}

lib/caconfig.go

@@ -112,7 +112,7 @@ type cfgOptions struct {
 
 // identitiesOptions are options that are related to identities
 type identitiesOptions struct {
-	AllowRemove bool `help:"Enables removing of identities dynamically"`
+	AllowRemove bool `help:"Enables removal of identities dynamically"`
 }
 
 // CAInfo is the CA information on a fabric-ca-server

lib/dasqlite_test.go

@@ -93,7 +93,12 @@ func Truncate(db *sqlx.DB) {
 func TestEmptyAccessor(t *testing.T) {
 	a := &Accessor{}
 	ui := spi.UserInfo{}
-	err := a.InsertUser(ui)
+	err := a.InsertUser(nil)
+	if err == nil {
+		t.Error("Passing in nil should have resulted in an error")
+	}
+
+	err = a.InsertUser(&ui)
 	if err == nil {
 		t.Error("Empty Accessor InsertUser should have failed")
 	}
@@ -254,7 +259,7 @@ func testInsertAndGetUser(ta TestAccessor, t *testing.T) {
 		},
 	}
 
-	err := ta.Accessor.InsertUser(insert)
+	err := ta.Accessor.InsertUser(&insert)
 	if err != nil {
 		t.Errorf("Error occured during insert query of ID: %s, error: %s", insert.Name, err)
 	}
@@ -329,7 +334,7 @@ func testDeleteUser(ta TestAccessor, t *testing.T) {
 		Attributes: []api.Attribute{},
 	}
 
-	err := ta.Accessor.InsertUser(insert)
+	err := ta.Accessor.InsertUser(&insert)
 	if err != nil {
 		t.Errorf("Error occured during insert query of id: %s, error: %s", insert.Name, err)
 	}
@@ -357,14 +362,19 @@ func testUpdateUser(ta TestAccessor, t *testing.T) {
 		MaxEnrollments: 1,
 	}
 
-	err := ta.Accessor.InsertUser(insert)
+	err := ta.Accessor.InsertUser(&insert)
 	if err != nil {
 		t.Errorf("Error occured during insert query of ID: %s, error: %s", insert.Name, err)
 	}
 
 	insert.Pass = "654321"
 
-	ta.Accessor.UpdateUser(insert)
+	err = ta.Accessor.UpdateUser(nil, true)
+	if err == nil {
+		t.Error("Passing in nil should have resulted in an error")
+	}
+
+	err = ta.Accessor.UpdateUser(&insert, true)
 	if err != nil {
 		t.Errorf("Error occured during update query of ID: %s, error: %s", insert.Name, err)
 	}

lib/dbaccessor.go

@@ -48,7 +48,7 @@ DELETE FROM users
 
 	updateUser = `
 UPDATE users
-	SET token = :token, type = :type, affiliation = :affiliation, attributes = :attributes, state = :state, level = :level
+	SET token = :token, type = :type, affiliation = :affiliation, attributes = :attributes, state = :state, max_enrollments = :max_enrollments, level = :level
 	WHERE (id = :id);`
 
 	getUser = `
@@ -112,7 +112,10 @@ func (d *Accessor) SetDB(db *sqlx.DB) {
 }
 
 // InsertUser inserts user into database
-func (d *Accessor) InsertUser(user spi.UserInfo) error {
+func (d *Accessor) InsertUser(user *spi.UserInfo) error {
+	if user == nil {
+		return errors.New("User is not defined")
+	}
 	log.Debugf("DB: Add identity %s", user.Name)
 
 	err := d.checkDB()
@@ -190,7 +193,11 @@ func deleteUserTx(tx *sqlx.Tx, args ...interface{}) error {
 }
 
 // UpdateUser updates user in database
-func (d *Accessor) UpdateUser(user spi.UserInfo) error {
+func (d *Accessor) UpdateUser(user *spi.UserInfo, updatePass bool) error {
+	if user == nil {
+		return errors.New("User is not defined")
+	}
+
 	log.Debugf("DB: Update identity %s", user.Name)
 	err := d.checkDB()
 	if err != nil {
@@ -204,9 +211,11 @@ func (d *Accessor) UpdateUser(user spi.UserInfo) error {
 
 	// Hash the password before storing it
 	pwd := []byte(user.Pass)
-	pwd, err = bcrypt.GenerateFromPassword(pwd, bcrypt.DefaultCost)
-	if err != nil {
-		return errors.Wrap(err, "Failed to hash password")
+	if updatePass {
+		pwd, err = bcrypt.GenerateFromPassword(pwd, bcrypt.DefaultCost)
+		if err != nil {
+			return errors.Wrap(err, "Failed to hash password")
+		}
 	}
 
 	// Store the updated user entry
@@ -665,35 +674,10 @@ func (u *DBUser) Revoke() error {
 }
 
 // ModifyAttributes adds a new attribute, modifies existing attribute, or delete attribute
-func (u *DBUser) ModifyAttributes(attrs []api.Attribute) error {
-	log.Debugf("Modify Attributes: %+v", attrs)
-	userAttrs, _ := u.GetAttributes(nil)
-	var attr api.Attribute
-	for _, attr = range attrs {
-		log.Debugf("Attribute request: %+v", attr)
-		found := false
-		for i := range userAttrs {
-			if userAttrs[i].Name == attr.Name {
-				if attr.Value == "" {
-					log.Debugf("Deleting attribute: %+v", userAttrs[i])
-					if i == len(userAttrs)-1 {
-						userAttrs = userAttrs[:len(userAttrs)-1]
-					} else {
-						userAttrs = append(userAttrs[:i], userAttrs[i+1:]...)
-					}
-				} else {
-					log.Debugf("Updating existing attribute from '%+v' to '%+v'", userAttrs[i], attr)
-					userAttrs[i].Value = attr.Value
-				}
-				found = true
-				break
-			}
-		}
-		if !found && attr.Value != "" {
-			log.Debugf("Adding '%+v' as new attribute", attr)
-			userAttrs = append(userAttrs, attr)
-		}
-	}
+func (u *DBUser) ModifyAttributes(newAttrs []api.Attribute) error {
+	log.Debugf("Modify Attributes: %+v", newAttrs)
+	currentAttrs, _ := u.GetAttributes(nil)
+	userAttrs := getNewAttributes(currentAttrs, newAttrs)
 
 	attrBytes, err := json.Marshal(userAttrs)
 	if err != nil {

lib/identity.go

@@ -276,7 +276,7 @@ func (i *Identity) ModifyIdentity(req *api.ModifyIdentityRequest) (*api.Identity
 		return nil, err
 	}
 
-	log.Debug("Successfully modified identity")
+	log.Debugf("Successfully modified identity '%s'", result.ID)
 	return result, nil
 }
 

lib/ldap/client.go

@@ -218,12 +218,12 @@ func (lc *Client) GetUser(username string, attrNames []string) (spi.User, error)
 }
 
 // InsertUser inserts a user
-func (lc *Client) InsertUser(user spi.UserInfo) error {
+func (lc *Client) InsertUser(user *spi.UserInfo) error {
 	return errNotSupported
 }
 
 // UpdateUser updates a user
-func (lc *Client) UpdateUser(user spi.UserInfo) error {
+func (lc *Client) UpdateUser(user *spi.UserInfo, updatePass bool) error {
 	return errNotSupported
 }
 

lib/servererror.go

@@ -145,6 +145,8 @@ const (
 	ErrRemoveIdentity = 56
 	// Failed to get boolean query parameter
 	ErrGettingBoolQueryParm = 57
+	// Failed to modify identity
+	ErrModifyingIdentity = 58
 )
 
 // Construct a new HTTP error.