From a9267ece7183f6f7b78f856797639d10337eb980 Mon Sep 17 00:00:00 2001 From: senthil Date: Tue, 30 May 2023 23:29:19 +0530 Subject: [PATCH] Make AmMemberOf to use only the mspIDs in collection policy Signed-off-by: senthil Signed-off-by: denyeart --- core/common/privdata/membershipinfo.go | 27 +++----- core/common/privdata/membershipinfo_test.go | 10 --- integration/ledger/reset_rollback_test.go | 62 +++++++++++++++++++ .../collections_config3.json | 18 ++++++ 4 files changed, 87 insertions(+), 30 deletions(-) create mode 100644 integration/ledger/testdata/collection_configs/collections_config3.json diff --git a/core/common/privdata/membershipinfo.go b/core/common/privdata/membershipinfo.go index 6f5aa294ae2..fad77b71541 100644 --- a/core/common/privdata/membershipinfo.go +++ b/core/common/privdata/membershipinfo.go @@ -28,31 +28,18 @@ 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 } diff --git a/core/common/privdata/membershipinfo_test.go b/core/common/privdata/membershipinfo_test.go index 6f6a3f57ffa..3d1f6197d1a 100644 --- a/core/common/privdata/membershipinfo_test.go +++ b/core/common/privdata/membershipinfo_test.go @@ -51,16 +51,6 @@ func TestMembershipInfoProvider(t *testing.T) { assert.False(t, res) assert.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"})) - assert.True(t, res) - assert.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"})) - assert.False(t, res) - assert.Nil(t, err) } func getAccessPolicy(signers []string) *peer.CollectionPolicyConfig { diff --git a/integration/ledger/reset_rollback_test.go b/integration/ledger/reset_rollback_test.go index 0f79d045561..34708767d10 100644 --- a/integration/ledger/reset_rollback_test.go +++ b/integration/ledger/reset_rollback_test.go @@ -192,6 +192,54 @@ var _ = Describe("rollback, reset, pause and resume peer node commands", func() Expect(dbPath).To(BeADirectory()) 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) + }) + }) type setup struct { @@ -458,6 +506,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 new file mode 100644 index 00000000000..0a86f3df761 --- /dev/null +++ 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 + } +]