Make AmMemberOf to use only the mspIDs in collection policy

Signed-off-by: senthil <cendhu@gmail.com>
hyperledger · May 31, 2023 · 494b8b8 · 494b8b8
1 parent 7143cd2
commit 494b8b8
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 31 deletions.
diff --git a/core/common/privdata/membershipinfo.go b/core/common/privdata/membershipinfo.go
@@ -35,33 +35,20 @@ func NewMembershipInfoProvider(mspID string, selfSignedData protoutil.SignedData
 }
 
 // AmMemberOf checks whether the current peer is a member of the given collection config.
-// If getPolicy returns an error, it will drop the error and return false - same as a RejectAll policy.
-// It is used when a chaincode is upgraded to see if the peer's org has become eligible after	a collection
-// change.
+// It is used when a chaincode is upgraded to see if the peer's org has become eligible after a collection change.
 func (m *MembershipProvider) AmMemberOf(channelName string, collectionPolicyConfig *peer.CollectionPolicyConfig) (bool, error) {
 	deserializer := m.IdentityDeserializerFactory(channelName)
 
 	// Do a simple check to see if the mspid matches any principal identities in the SignaturePolicy - FAB-17059
-	if collectionPolicyConfig.GetSignaturePolicy() != nil {
-		memberOrgs := getMemberOrgs(collectionPolicyConfig.GetSignaturePolicy().GetIdentities(), deserializer)
-
-		if _, ok := memberOrgs[m.mspID]; ok {
-			return true, nil
-		}
-	}
-
-	// Fall back to default access policy evaluation otherwise
-	accessPolicy, err := getPolicy(collectionPolicyConfig, deserializer)
-	if err != nil {
-		// drop the error and return false - same as reject all policy
-		logger.Errorf("Reject all due to error getting policy: %s", err)
-		return false, nil
-	}
-	if err := accessPolicy.EvaluateSignedData([]*protoutil.SignedData{&m.selfSignedData}); err != nil {
+	if collectionPolicyConfig.GetSignaturePolicy() == nil {
+		logger.Warningf("collection membership policy is nil")
 		return false, nil
 	}
 
-	return true, nil
+	memberOrgs := getMemberOrgs(collectionPolicyConfig.GetSignaturePolicy().GetIdentities(), deserializer)
+
+	_, ok := memberOrgs[m.mspID]
+	return ok, nil
 }
 
 func (m *MembershipProvider) MyImplicitCollectionName() string {

diff --git a/core/common/privdata/membershipinfo_test.go b/core/common/privdata/membershipinfo_test.go
@@ -51,17 +51,6 @@ func TestMembershipInfoProvider(t *testing.T) {
 	res, err = membershipProvider.AmMemberOf("test1", getBadAccessPolicy([]string{"signer0"}, 1))
 	require.False(t, res)
 	require.Nil(t, err)
-
-	// verify membership provider with empty mspID and fall back to default access policy evaluation returns true
-	membershipProvider = NewMembershipInfoProvider("", peerSelfSignedData, identityDeserializer)
-	res, err = membershipProvider.AmMemberOf("test1", getAccessPolicy([]string{"peer0", "peer1"}))
-	require.True(t, res)
-	require.Nil(t, err)
-
-	// verify membership provider with empty mspID and fall back to default access policy evaluation returns false
-	res, err = membershipProvider.AmMemberOf("test1", getAccessPolicy([]string{"peer2", "peer3"}))
-	require.False(t, res)
-	require.Nil(t, err)
 }
 
 func TestMyImplicitCollectionName(t *testing.T) {

diff --git a/integration/ledger/reset_rollback_test.go b/integration/ledger/reset_rollback_test.go
@@ -193,6 +193,53 @@ var _ = Describe("rollback, reset, pause, resume, and unjoin peer node commands"
 		helper.assertPresentInCollectionM("marblesp", "marble2", peer)
 	})
 
+	It("change collection members and rebuild databases", func() {
+		By("Checking ledger height on each peer")
+		for _, peer := range helper.peers {
+			Expect(helper.getLedgerHeight(peer)).Should(Equal(14))
+		}
+
+		org1peer0 := setup.network.Peer("Org1", "peer0")
+		org2peer0 := setup.network.Peer("Org2", "peer0")
+		org3peer0 := setup.network.Peer("Org3", "peer0")
+
+		By("verifying marble1 to marble5 exist in collectionMarbles & collectionMarblePrivateDetails in the members")
+		for i := 1; i <= 5; i++ {
+			helper.assertPresentInCollectionM("marblesp", fmt.Sprintf("marble%d", i), org1peer0)
+			helper.assertPresentInCollectionM("marblesp", fmt.Sprintf("marble%d", i), org2peer0)
+			helper.assertPresentInCollectionMPD("marblesp", fmt.Sprintf("marble%d", i), org2peer0)
+			helper.assertPresentInCollectionMPD("marblesp", fmt.Sprintf("marble%d", i), org3peer0)
+		}
+
+		// Add org1 to collectionMarblesPrivateDetails
+		By("Updating the chaincode definition to version 2.0")
+		updatedChaincode := nwo.Chaincode{
+			Name:              "marblesp",
+			Version:           "2.0",
+			Path:              components.Build("github.com/hyperledger/fabric/integration/chaincode/marbles_private/cmd"),
+			Lang:              "binary",
+			PackageFile:       filepath.Join(setup.testDir, "marbles-pvtdata.tar.gz"),
+			Label:             "marbles-private-20",
+			SignaturePolicy:   `OR ('Org1MSP.member','Org2MSP.member', 'Org3MSP.member')`,
+			CollectionsConfig: filepath.Join("testdata", "collection_configs", "collections_config3.json"),
+			Sequence:          "2",
+		}
+
+		helper.deployChaincode(updatedChaincode)
+
+		// statedb rebuild test
+		By("Stopping peers and deleting the statedb folder on peer Org1.peer0")
+		org1peer0 = setup.network.Peer("Org1", "peer0")
+		setup.stopPeers()
+		dbPath := filepath.Join(setup.network.PeerLedgerDir(org1peer0), "stateLeveldb")
+		Expect(os.RemoveAll(dbPath)).NotTo(HaveOccurred())
+		Expect(dbPath).NotTo(BeADirectory())
+		By("Restarting the peer Org1.peer0")
+		setup.startPeers()
+		Expect(dbPath).To(BeADirectory())
+		helper.assertPresentInCollectionM("marblesp", "marble2", org1peer0)
+	})
+
 	// This test exercises peer node unjoin on the following peers:
 	// Org1.peer0 - unjoin
 	// Org2.peer0 -
@@ -527,6 +574,20 @@ func (th *testHelper) assertPresentInCollectionM(chaincodeName, marbleName strin
 	}
 }
 
+// assertAbsentInCollectionM asserts that the private data for given marble is present in collection
+// 'readMarble' at the given peers
+func (th *testHelper) assertAbsentInCollectionM(chaincodeName, marbleName string, peerList ...*nwo.Peer) {
+	command := commands.ChaincodeQuery{
+		ChannelID: th.channelID,
+		Name:      chaincodeName,
+		Ctor:      fmt.Sprintf(`{"Args":["readMarble","%s"]}`, marbleName),
+	}
+	expectedMsg := fmt.Sprintf(`{"docType":"marble","name":"%s"`, marbleName)
+	for _, peer := range peerList {
+		th.queryChaincode(peer, command, expectedMsg, false)
+	}
+}
+
 // assertPresentInCollectionMPD asserts that the private data for given marble is present
 // in collection 'readMarblePrivateDetails' at the given peers
 func (th *testHelper) assertPresentInCollectionMPD(chaincodeName, marbleName string, peerList ...*nwo.Peer) {

diff --git a/integration/ledger/testdata/collection_configs/collections_config3.json b/integration/ledger/testdata/collection_configs/collections_config3.json
@@ -0,0 +1,18 @@
+[
+ 	{
+	 	"name": "collectionMarbles",
+	 	"policy": "OR('Org1MSP.member', 'Org2MSP.member')",
+	 	"requiredPeerCount": 1,
+	 	"maxPeerCount": 2,
+	 	"blockToLive":1000000,
+		"memberOnlyRead": false
+	},
+ 	{
+	 	"name": "collectionMarblePrivateDetails",
+	 	"policy": "OR('Org1MSP.member', 'Org2MSP.member', 'Org3MSP.member')",
+	 	"requiredPeerCount": 1,
+	 	"maxPeerCount": 2,
+	 	"blockToLive":1000000,
+		"memberOnlyRead": false
+ 	}
+]