Skip to content

Commit

Permalink
Fix Node OU error message
Browse files Browse the repository at this point in the history
If signing identity didn't match any expected Node OU roles,
the error message indicated that it matched multiple roles. This confused users.
Now there will be a separate error message for no OU roles versus multiple OU roles.

Signed-off-by: David Enyeart <enyeart@us.ibm.com>
  • Loading branch information
denyeart authored and ale-linux committed Oct 12, 2020
1 parent ab7104a commit 8930c8c
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 20 deletions.
16 changes: 13 additions & 3 deletions msp/mspimplvalidate.go
Expand Up @@ -209,7 +209,12 @@ func (msp *bccspmsp) validateIdentityOUsV11(id *identity) error {
break
}
}
if counter != 1 {

// the identity should have exactly one OU role, return an error if the counter is not 1.
if counter == 0 {
return errors.Errorf("the identity does not have an OU that resolves to client or peer. OUs: %s, MSP: [%s]", OUIDs(id.GetOrganizationalUnits()), msp.name)
}
if counter > 1 {
return errors.Errorf("the identity must be a client or a peer identity to be valid, not a combination of them. OUs: %s, MSP: [%s]", OUIDs(id.GetOrganizationalUnits()), msp.name)
}

Expand Down Expand Up @@ -263,8 +268,13 @@ func (msp *bccspmsp) validateIdentityOUsV142(id *identity) error {
break
}
}
if counter != 1 {
return errors.Errorf("the identity must be a client, a peer, an orderer or an admin identity to be valid, not a combination of them. OUs: %s, MSP: [%s]", OUIDs(id.GetOrganizationalUnits()), msp.name)

// the identity should have exactly one OU role, return an error if the counter is not 1.
if counter == 0 {
return errors.Errorf("the identity does not have an OU that resolves to client, peer, orderer, or admin role. OUs: %s, MSP: [%s]", OUIDs(id.GetOrganizationalUnits()), msp.name)
}
if counter > 1 {
return errors.Errorf("the identity must have a client, a peer, an orderer, or an admin OU role to be valid, not a combination of them. OUs: %s, MSP: [%s]", OUIDs(id.GetOrganizationalUnits()), msp.name)
}

return nil
Expand Down
52 changes: 35 additions & 17 deletions msp/nodeous_test.go
Expand Up @@ -41,28 +41,46 @@ func TestInvalidAdminNodeOU(t *testing.T) {
}

func TestInvalidSigningIdentityNodeOU(t *testing.T) {
// testdata/nodeous2:
// the configuration enables NodeOUs but the signing identity does not carry
// any valid NodeOUS. Therefore signing identity validation should fail
thisMSP := getLocalMSPWithVersion(t, "testdata/nodeous2", MSPv1_1)
assert.True(t, thisMSP.(*bccspmsp).ouEnforcement)
t.Run("signing_identity_validation_fails_with_MSPv1_4_3", func(t *testing.T) {
// testdata/nodeous2:
// the configuration enables NodeOUs but the signing identity does not carry
// any valid NodeOUS. Therefore signing identity validation should fail
thisMSP := getLocalMSPWithVersion(t, "testdata/nodeous2", MSPv1_4_3)
assert.True(t, thisMSP.(*bccspmsp).ouEnforcement)

id, err := thisMSP.GetDefaultSigningIdentity()
assert.NoError(t, err)

id, err := thisMSP.GetDefaultSigningIdentity()
assert.NoError(t, err)
err = id.Validate()
assert.EqualError(t, err, "could not validate identity's OUs: the identity does not have an OU that resolves to client, peer, orderer, or admin role. OUs: [], MSP: [SampleOrg]")
})

err = id.Validate()
assert.Error(t, err)
t.Run("signing_identity_validation_fails_with_MSPv1_1", func(t *testing.T) {
// testdata/nodeous2:
// the configuration enables NodeOUs but the signing identity does not carry
// any valid NodeOUS. Therefore signing identity validation should fail
thisMSP := getLocalMSPWithVersion(t, "testdata/nodeous2", MSPv1_1)
assert.True(t, thisMSP.(*bccspmsp).ouEnforcement)

// MSPv1_0 should not fail
thisMSP, err = getLocalMSPWithVersionAndError(t, "testdata/nodeous1", MSPv1_0)
assert.False(t, thisMSP.(*bccspmsp).ouEnforcement)
assert.NoError(t, err)
id, err := thisMSP.GetDefaultSigningIdentity()
assert.NoError(t, err)

id, err = thisMSP.GetDefaultSigningIdentity()
assert.NoError(t, err)
err = id.Validate()
assert.EqualError(t, err, "could not validate identity's OUs: the identity does not have an OU that resolves to client or peer. OUs: [], MSP: [SampleOrg]")
})

err = id.Validate()
assert.NoError(t, err)
t.Run("signing_identity_validation_succeeds_with_MSPv1_0", func(t *testing.T) {
// MSPv1_0 should not fail, node OUs not yet implemented in 1_0
thisMSP, err := getLocalMSPWithVersionAndError(t, "testdata/nodeous1", MSPv1_0)
assert.False(t, thisMSP.(*bccspmsp).ouEnforcement)
assert.NoError(t, err)

id, err := thisMSP.GetDefaultSigningIdentity()
assert.NoError(t, err)

err = id.Validate()
assert.NoError(t, err)
})
}

func TestValidMSPWithNodeOU(t *testing.T) {
Expand Down

0 comments on commit 8930c8c

Please sign in to comment.