[FAB-10063] Fix Access Denied error in Local Discovery

Changed the local discovery provider to use the MSP ID as a cache key so that one local service is created per MSP. Change-Id: Ic71e96188d2e63930c992d9ebc99809d58dcd0b8 Signed-off-by: Bob Stasyszyn <Bob.Stasyszyn@securekey.com>
hyperledger · May 15, 2018 · 89d1ed6 · 89d1ed6
1 parent dd2868a
commit 89d1ed6
Show file tree

Hide file tree

Showing 11 changed files with 61 additions and 19 deletions.
diff --git a/pkg/client/common/discovery/dynamicdiscovery/localservice.go b/pkg/client/common/discovery/dynamicdiscovery/localservice.go
@@ -18,19 +18,23 @@ import (
 // Fabric's Discovery service for the peers that are in the local MSP.
 type LocalService struct {
 	*service
+	mspID string
 }
 
 // newLocalService creates a Local Discovery Service to query the list of member peers in the local MSP.
-func newLocalService(options options) *LocalService {
+func newLocalService(mspID string, options options) *LocalService {
 	logger.Debugf("Creating new dynamic discovery service with cache refresh interval %s", options.refreshInterval)
 
-	s := &LocalService{}
+	s := &LocalService{mspID: mspID}
 	s.service = newService(s.queryPeers, options)
 	return s
 }
 
 // Initialize initializes the service with local context
 func (s *LocalService) Initialize(ctx contextAPI.Local) error {
+	if ctx.Identifier().MSPID != s.mspID {
+		return errors.Errorf("expecting context for MSP [%s] but got [%s]", s.mspID, ctx.Identifier().MSPID)
+	}
 	return s.service.Initialize(ctx)
 }
 

diff --git a/pkg/client/common/discovery/dynamicdiscovery/localservice_test.go b/pkg/client/common/discovery/dynamicdiscovery/localservice_test.go
@@ -49,15 +49,24 @@ func TestLocalDiscoveryService(t *testing.T) {
 		return discClient, nil
 	}
 
+	// Test initialize with invalid MSP ID
 	service := newLocalService(
+		mspID2,
+		options{},
+	)
+	err := service.Initialize(localCtx)
+	assert.Error(t, err)
+
+	service = newLocalService(
+		mspID1,
 		options{
 			refreshInterval: 500 * time.Millisecond,
 			responseTimeout: 2 * time.Second,
 		},
 	)
 	defer service.Close()
 
-	err := service.Initialize(localCtx)
+	err = service.Initialize(localCtx)
 	assert.NoError(t, err)
 	// Initialize again should produce no error
 	err = service.Initialize(localCtx)

diff --git a/pkg/client/common/discovery/dynamicdiscovery/provider.go b/pkg/client/common/discovery/dynamicdiscovery/provider.go
@@ -63,8 +63,8 @@ func New(config fab.EndpointConfig, opts ...Opt) *Provider {
 
 	return &Provider{
 		cache: lazycache.New("Dynamic_Discovery_Service_Cache", func(key lazycache.Key) (interface{}, error) {
-			if key.String() == "" {
-				return newLocalService(options), nil
+			if mk, ok := key.(*mspKey); ok {
+				return newLocalService(mk.mspID, options), nil
 			}
 			return newChannelService(options), nil
 		}),
@@ -81,8 +81,8 @@ func (p *Provider) CreateDiscoveryService(channelID string) (fab.DiscoveryServic
 }
 
 // CreateLocalDiscoveryService returns a local discovery service
-func (p *Provider) CreateLocalDiscoveryService() (fab.DiscoveryService, error) {
-	ref, err := p.cache.Get(lazycache.NewStringKey(""))
+func (p *Provider) CreateLocalDiscoveryService(mspID string) (fab.DiscoveryService, error) {
+	ref, err := p.cache.Get(newMSPKey(mspID))
 	if err != nil {
 		return nil, errors.WithMessage(err, "failed to get local discovery service from cache")
 	}
@@ -93,3 +93,15 @@ func (p *Provider) CreateLocalDiscoveryService() (fab.DiscoveryService, error) {
 func (p *Provider) Close() {
 	p.cache.Close()
 }
+
+type mspKey struct {
+	mspID string
+}
+
+func newMSPKey(mspID string) *mspKey {
+	return &mspKey{mspID: mspID}
+}
+
+func (k *mspKey) String() string {
+	return k.mspID
+}
diff --git a/pkg/client/common/discovery/dynamicdiscovery/provider_test.go b/pkg/client/common/discovery/dynamicdiscovery/provider_test.go
@@ -17,7 +17,8 @@ import (
 )
 
 const (
-	ch = "orgchannel"
+	ch  = "orgchannel"
+	ch2 = "channel2"
 
 	mspID1 = "Org1MSP"
 	mspID2 = "Org2MSP"
@@ -44,20 +45,36 @@ func TestDiscoveryProvider(t *testing.T) {
 	p := New(config, WithRefreshInterval(30*time.Second), WithResponseTimeout(10*time.Second))
 	defer p.Close()
 
-	service, err := p.CreateDiscoveryService(ch)
+	service1, err := p.CreateDiscoveryService(ch)
 	assert.NoError(t, err)
 
 	chCtx := mocks.NewMockChannelContext(ctx, ch)
 
-	err = service.(*channelService).Initialize(chCtx)
+	err = service1.(*channelService).Initialize(chCtx)
 	assert.NoError(t, err)
 
-	localService, err := p.CreateLocalDiscoveryService()
+	service2, err := p.CreateDiscoveryService(ch)
+	assert.NoError(t, err)
+	assert.Equal(t, service1, service2)
+
+	service2, err = p.CreateDiscoveryService(ch2)
+	assert.NoError(t, err)
+	assert.NotEqual(t, service1, service2)
+
+	localService1, err := p.CreateLocalDiscoveryService(mspID1)
 	assert.NoError(t, err)
 
 	localCtx := mocks.NewMockLocalContext(ctx, nil)
-	err = localService.(*LocalService).Initialize(localCtx)
+	err = localService1.(*LocalService).Initialize(localCtx)
+	assert.NoError(t, err)
+
+	localService2, err := p.CreateLocalDiscoveryService(mspID1)
+	assert.NoError(t, err)
+	assert.Equal(t, localService1, localService2)
+
+	localService2, err = p.CreateLocalDiscoveryService(mspID2)
 	assert.NoError(t, err)
+	assert.NotEqual(t, localService1, localService2)
 }
 
 type config struct {

diff --git a/pkg/client/common/discovery/staticdiscovery/provider.go b/pkg/client/common/discovery/staticdiscovery/provider.go
@@ -64,7 +64,7 @@ func (dp *DiscoveryProvider) CreateDiscoveryService(channelID string) (fab.Disco
 }
 
 // CreateLocalDiscoveryService return a local discovery service
-func (dp *DiscoveryProvider) CreateLocalDiscoveryService() (fab.DiscoveryService, error) {
+func (dp *DiscoveryProvider) CreateLocalDiscoveryService(mspID string) (fab.DiscoveryService, error) {
 	peers := []fab.Peer{}
 
 	netPeers, err := dp.config.NetworkPeers()

diff --git a/pkg/client/common/discovery/staticdiscovery/staticdiscovery_test.go b/pkg/client/common/discovery/staticdiscovery/staticdiscovery_test.go
@@ -83,7 +83,7 @@ func TestStaticLocalDiscovery(t *testing.T) {
 	clientCtx := mocks.NewMockContext(mockmsp.NewMockSigningIdentity("user1", "Org1MSP"))
 	discoveryProvider.Initialize(clientCtx)
 
-	discoveryService, err := discoveryProvider.CreateLocalDiscoveryService()
+	discoveryService, err := discoveryProvider.CreateLocalDiscoveryService(clientCtx.Identifier().MSPID)
 	assert.NoError(t, err)
 
 	localCtx := mocks.NewMockLocalContext(clientCtx, discoveryProvider)

diff --git a/pkg/client/common/mocks/mockdiscovery.go b/pkg/client/common/mocks/mockdiscovery.go
@@ -43,7 +43,7 @@ func (dp *MockStaticDiscoveryProvider) CreateDiscoveryService(channelID string)
 }
 
 // CreateLocalDiscoveryService return discovery service for specific channel
-func (dp *MockStaticDiscoveryProvider) CreateLocalDiscoveryService() (fab.DiscoveryService, error) {
+func (dp *MockStaticDiscoveryProvider) CreateLocalDiscoveryService(mspID string) (fab.DiscoveryService, error) {
 	return &MockStaticDiscoveryService{Error: dp.Error, Peers: dp.Peers}, nil
 }
 

diff --git a/pkg/common/providers/fab/provider.go b/pkg/common/providers/fab/provider.go
@@ -67,7 +67,7 @@ type DiscoveryService interface {
 
 // LocalDiscoveryProvider is used to discover peers in the local MSP
 type LocalDiscoveryProvider interface {
-	CreateLocalDiscoveryService() (DiscoveryService, error)
+	CreateLocalDiscoveryService(mspID string) (DiscoveryService, error)
 }
 
 // TargetFilter allows for filtering target peers

diff --git a/pkg/context/context.go b/pkg/context/context.go
@@ -253,7 +253,7 @@ func NewLocal(clientProvider context.ClientProvider) (*Local, error) {
 		return nil, errors.WithMessage(err, "failed to get client context to create local context")
 	}
 
-	discoveryService, err := client.LocalDiscoveryProvider().CreateLocalDiscoveryService()
+	discoveryService, err := client.LocalDiscoveryProvider().CreateLocalDiscoveryService(client.Identifier().MSPID)
 	if err != nil {
 		return nil, errors.WithMessage(err, "failed to create local discovery service")
 	}

diff --git a/pkg/fab/mocks/mockdiscovery.go b/pkg/fab/mocks/mockdiscovery.go
@@ -48,7 +48,7 @@ func (dp *MockStaticDiscoveryProvider) CreateDiscoveryService(channelID string)
 }
 
 // CreateLocalDiscoveryService return local discovery service
-func (dp *MockStaticDiscoveryProvider) CreateLocalDiscoveryService() (fab.DiscoveryService, error) {
+func (dp *MockStaticDiscoveryProvider) CreateLocalDiscoveryService(mspID string) (fab.DiscoveryService, error) {
 
 	if dp.customDiscoveryService != nil {
 		return dp.customDiscoveryService, nil

diff --git a/pkg/fab/mocks/mocklocal.go b/pkg/fab/mocks/mocklocal.go
@@ -28,7 +28,7 @@ func NewMockLocalContext(client *MockContext, discoveryProvider fab.LocalDiscove
 	var localDiscovery fab.DiscoveryService
 	if discoveryProvider != nil {
 		var err error
-		localDiscovery, err = discoveryProvider.CreateLocalDiscoveryService()
+		localDiscovery, err = discoveryProvider.CreateLocalDiscoveryService(client.Identifier().MSPID)
 		if err != nil {
 			panic(fmt.Sprintf("error creating local discovery service: %s", err))
 		}