[FAB-7893] Prevent unforced delete of sub-affiliations

This patch adds a couple of unit tests and change code on removal of
an affiliation with sub-affiliations to check that this only happens
when the force flag is specified.

Before the bug fix the new tests fail:

$ go test -v -run=TestAffiliationCmd github.com/hyperledger/fabric-ca/cmd/
fabric-ca-client
...
Successfully added affiliation: org4.dept1.team
Successfully removed affiliation: &{AffiliationInfo:{Name:org3 Affiliation
s:[] Identities:[]} CAName:}
Successfully modified affiliation: &{AffiliationInfo:{Name:org3 Affiliatio
ns:[] Identities:[]} CAName:}
Successfully removed affiliation: &{AffiliationInfo:{Name:org4 Affiliation
s:[{Name:org4.dept1 Affiliations:[{Name:org4.dept1.team Affiliations:[] Id
entities:[]}] Identities:[]}] Identities:[]} CAName:}
2018/01/30 11:31:21 [ERROR] Server has stopped serving: accept tcp [::]:70
90: use of closed network connection
--- FAIL: TestAffiliationCmd (4.13s)
        Error Trace:    main_test.go:858
	Error:      	An error is expected but got nil.
	Messages:   	Should have failed, no force argument provided and
 affiliation being deleted had sub-affiliations
FAIL
exit status 1
FAIL	github.com/hyperledger/fabric-ca/cmd/fabric-ca-client	4.147s

After the bug fix the new tests pass:

$ go test -v -run=TestAffiliationCmd github.com/hyperledger/fabric-ca/cmd/
fabric-ca-client
...
Successfully added affiliation: org4.dept1.team
Successfully removed affiliation: &{AffiliationInfo:{Name:org3 Affiliation
s:[] Identities:[]} CAName:}
Successfully modified affiliation: &{AffiliationInfo:{Name:org3 Affiliatio
ns:[] Identities:[]} CAName:}
Error: Response from server: Error Code: 20 - Authorization failure

Successfully removed affiliation: &{AffiliationInfo:{Name:org4 Affiliation
s:[{Name:org4.dept1 Affiliations:[{Name:org4.dept1.team Affiliations:[] Id
entities:[]}] Identities:[]}] Identities:[]} CAName:}
2018/01/30 11:33:06 [ERROR] Server has stopped serving: accept tcp [::]:70
90: use of closed network connection
--- PASS: TestAffiliationCmd (4.12s)
PASS
ok  	github.com/hyperledger/fabric-ca/cmd/fabric-ca-client	4.129s

Patch-set #2: Improve error messages.

Change-Id: Ib5a7acb68a42f251970cce6735173ea296ef5d54
Signed-off-by: Arnaud J Le Hors <[email protected]>

cmd/fabric-ca-client/main_test.go

@@ -853,6 +853,16 @@ func TestAffiliationCmd(t *testing.T) {
 	_, err = registry.GetAffiliation("org3")
 	assert.NoError(t, err, "Failed to rename 'org1' to 'org3' successfully")
 
+	err = RunMain([]string{
+		cmdName, "affiliation", "remove", "org4"})
+	assert.Error(t, err, "Should have failed, no force argument provided and affiliation being deleted had sub-affiliations")
+
+	// if previous test failed, don't bother with the next one
+	if err != nil {
+		err = RunMain([]string{
+			cmdName, "affiliation", "remove", "org4", "--force"})
+		assert.NoError(t, err, "Failed to remove affiliation with force argument")
+	}
 }
 
 // Verify the certificate has attribute 'name' with a value of 'val'

lib/dbaccessor.go

@@ -378,43 +378,17 @@ func (d *Accessor) deleteAffiliationTx(tx *sqlx.Tx, args ...interface{}) (interf
 	}
 	idNamesStr := strings.Join(idNames, ",")
 
+	// First check that all settings are correct
 	if len(ids) > 0 {
 		if !isRegistar {
 			return nil, newAuthErr(ErrUpdateConfigRemoveAff, "Removing affiliation affects identities, but caller is not a registrar")
 		}
-
-		// Force enabled, delete any associated identities and certificates
-		if force {
-			log.Debugf("IDs '%s' to removed based on affiliation '%s' removal", idNamesStr, name)
-
-			if !identityRemoval {
-				return nil, newAuthErr(ErrUpdateConfigRemoveAff, "Identity removal is not allowed on server")
-			}
-
-			// Delete all the identities in one database request
-			query := "DELETE FROM users WHERE (id IN (?))"
-			inQuery, args, err := sqlx.In(query, idNames)
-			if err != nil {
-				return nil, newHTTPErr(500, ErrRemoveAffDB, "Failed to construct query '%s': %s", query, err)
-			}
-			_, err = tx.Exec(tx.Rebind(inQuery), args...)
-			if err != nil {
-				return nil, newHTTPErr(500, ErrRemoveAffDB, "Failed to execute query '%s' for multiple identity removal: %s", query, err)
-			}
-
-			// Revoke all the certificates associated with the removed identities above with reason of "affiliationchange" (3)
-			query = "UPDATE certificates SET status='revoked', revoked_at=CURRENT_TIMESTAMP, reason = ? WHERE (id IN (?) AND status != 'revoked')"
-			inQuery, args, err = sqlx.In(query, ocsp.AffiliationChanged, idNames)
-			if err != nil {
-				return nil, newHTTPErr(500, ErrRemoveAffDB, "Failed to construct query '%s': %s", query, err)
-			}
-			_, err = tx.Exec(tx.Rebind(inQuery), args...)
-			if err != nil {
-				return nil, newHTTPErr(500, ErrRemoveAffDB, "Failed to execute query '%s' for multiple certificate removal: %s", query, err)
-			}
-		} else {
+		if !identityRemoval {
+			return nil, newAuthErr(ErrUpdateConfigRemoveAff, "Identity removal is not allowed on server")
+		}
+		if !force {
 			// If force option is not specified, only delete affiliation if there are no identities that have that affiliation
-			return nil, newAuthErr(ErrUpdateConfigRemoveAff, "The request to remove affiliation '%s' has the following identities associated: %s. Need to use 'force' to remove identities and affiliation", name, idNamesStr)
+			return nil, newAuthErr(ErrUpdateConfigRemoveAff, "Cannot delete affiliation '%s'. The affiliation has the following identities associated: %s. Need to use 'force' to remove identities and affiliation", name, idNamesStr)
 		}
 	}
 
@@ -429,20 +403,59 @@ func (d *Accessor) deleteAffiliationTx(tx *sqlx.Tx, args ...interface{}) (interf
 	if err != nil {
 		return nil, newHTTPErr(500, ErrRemoveAffDB, "Failed to select sub-affiliations of '%s': %s", allAffs, err)
 	}
+
+	if len(allAffs) > 0 {
+		if !force {
+			// If force option is not specified, only delete affiliation if there are no sub-affiliations
+			return nil, newAuthErr(ErrUpdateConfigRemoveAff, "Cannot delete affiliation '%s'. The affiliation has the following sub-affiliations: %s. Need to use 'force' to remove affiliation and sub-affiliations", name, allAffs)
+		}
+	}
 	allAffs = append(allAffs, aff)
+
+	// Now proceed with deletion
+
+	// delete any associated identities and certificates
+	if len(ids) > 0 {
+		log.Debugf("IDs '%s' to be removed based on affiliation '%s' removal", idNamesStr, name)
+
+		// Delete all the identities in one database request
+		query := "DELETE FROM users WHERE (id IN (?))"
+		inQuery, args, err := sqlx.In(query, idNames)
+		if err != nil {
+			return nil, newHTTPErr(500, ErrRemoveAffDB, "Failed to construct query '%s': %s", query, err)
+		}
+		_, err = tx.Exec(tx.Rebind(inQuery), args...)
+		if err != nil {
+			return nil, newHTTPErr(500, ErrRemoveAffDB, "Failed to execute query '%s' for multiple identity removal: %s", query, err)
+		}
+
+		// Revoke all the certificates associated with the removed identities above with reason of "affiliationchange" (3)
+		query = "UPDATE certificates SET status='revoked', revoked_at=CURRENT_TIMESTAMP, reason = ? WHERE (id IN (?) AND status != 'revoked')"
+		inQuery, args, err = sqlx.In(query, ocsp.AffiliationChanged, idNames)
+		if err != nil {
+			return nil, newHTTPErr(500, ErrRemoveAffDB, "Failed to construct query '%s': %s", query, err)
+		}
+		_, err = tx.Exec(tx.Rebind(inQuery), args...)
+		if err != nil {
+			return nil, newHTTPErr(500, ErrRemoveAffDB, "Failed to execute query '%s' for multiple certificate removal: %s", query, err)
+		}
+	}
+
 	log.Debug("All affiliations to be removed: ", allAffs)
 
 	// Delete the requested affiliation
 	_, err = tx.Exec(tx.Rebind(deleteAffiliation), name)
 	if err != nil {
 		return nil, newHTTPErr(500, ErrRemoveAffDB, "Failed to delete affiliation '%s': %s", name, err)
 	}
-	// Delete all the sub-affiliations
-	_, err = tx.Exec(tx.Rebind("DELETE FROM affiliations where (name LIKE ?)"), subAffName)
-	if err != nil {
-		return nil, newHTTPErr(500, ErrRemoveAffDB, "Failed to delete affiliations: %s", err)
-	}
 
+	if len(allAffs) > 1 {
+		// Delete all the sub-affiliations
+		_, err = tx.Exec(tx.Rebind("DELETE FROM affiliations where (name LIKE ?)"), subAffName)
+		if err != nil {
+			return nil, newHTTPErr(500, ErrRemoveAffDB, "Failed to delete affiliations: %s", err)
+		}
+	}
 	// Return the identities and affiliations that were removed
 	result := d.getResult(ids, allAffs)