diff --git a/common/configtx/update.go b/common/configtx/update.go index f74b59470cf..9a840b70ca5 100644 --- a/common/configtx/update.go +++ b/common/configtx/update.go @@ -18,6 +18,7 @@ package configtx import ( "fmt" + "strings" "github.com/hyperledger/fabric/common/policies" cb "github.com/hyperledger/fabric/protos/common" @@ -94,12 +95,15 @@ func (cm *configManager) authorizeUpdate(configUpdateEnv *cb.ConfigUpdateEnvelop // Get the modification policy for this config item if one was previously specified // or accept it if it is new, as the group policy will be evaluated for its inclusion - var policy policies.Policy if ok { - policy, _ = cm.PolicyManager().GetPolicy(oldValue.modPolicy()) + policy, ok := cm.policyForItem(oldValue) + if !ok { + return nil, fmt.Errorf("Unexpected missing policy %s for item %s", oldValue.modPolicy(), key) + } + // Ensure the policy is satisfied if err = policy.Evaluate(signedData); err != nil { - return nil, err + return nil, fmt.Errorf("Policy for %s not satisfied: %s", key, err) } } @@ -118,6 +122,19 @@ func (cm *configManager) authorizeUpdate(configUpdateEnv *cb.ConfigUpdateEnvelop return cm.computeUpdateResult(configMap), nil } +func (cm *configManager) policyForItem(item comparable) (policies.Policy, bool) { + if strings.HasPrefix(item.modPolicy(), PathSeparator) { + return cm.PolicyManager().GetPolicy(item.modPolicy()[1:]) + } + + // path is always at least of length 1 + manager, ok := cm.PolicyManager().Manager(item.path[1:]) + if !ok { + return nil, ok + } + return manager.GetPolicy(item.modPolicy()) +} + // computeUpdateResult takes a configMap generated by an update and produces a new configMap overlaying it onto the old config func (cm *configManager) computeUpdateResult(updatedConfig map[string]comparable) map[string]comparable { newConfigMap := make(map[string]comparable) diff --git a/common/configtx/update_test.go b/common/configtx/update_test.go new file mode 100644 index 00000000000..dc7f0ac5997 --- /dev/null +++ b/common/configtx/update_test.go @@ -0,0 +1,84 @@ +/* +Copyright IBM Corp. 2017 All Rights Reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package configtx + +import ( + "fmt" + "testing" + + mockconfigtx "github.com/hyperledger/fabric/common/mocks/configtx" + mockpolicies "github.com/hyperledger/fabric/common/mocks/policies" + cb "github.com/hyperledger/fabric/protos/common" + + "github.com/stretchr/testify/assert" +) + +func TestPolicyForItem(t *testing.T) { + // Policies are set to different error values to differentiate them in equal assertion + rootPolicy := &mockpolicies.Policy{Err: fmt.Errorf("rootPolicy")} + fooPolicy := &mockpolicies.Policy{Err: fmt.Errorf("fooPolicy")} + + cm := &configManager{ + Resources: &mockconfigtx.Resources{ + PolicyManagerVal: &mockpolicies.Manager{ + BasePathVal: "root", + Policy: rootPolicy, + SubManagersMap: map[string]*mockpolicies.Manager{ + "foo": &mockpolicies.Manager{ + Policy: fooPolicy, + BasePathVal: "foo", + }, + }, + }, + }, + } + + policy, ok := cm.policyForItem(comparable{ + path: []string{"root"}, + ConfigValue: &cb.ConfigValue{ + ModPolicy: "rootPolicy", + }, + }) + assert.True(t, ok) + assert.Equal(t, policy, rootPolicy, "Should have found relative policy off the root manager") + + policy, ok = cm.policyForItem(comparable{ + path: []string{"root", "wrong"}, + ConfigValue: &cb.ConfigValue{ + ModPolicy: "rootPolicy", + }, + }) + assert.False(t, ok, "Should not have found rootPolicy off a non-existant manager") + + policy, ok = cm.policyForItem(comparable{ + path: []string{"root", "foo"}, + ConfigValue: &cb.ConfigValue{ + ModPolicy: "foo", + }, + }) + assert.True(t, ok) + assert.Equal(t, policy, fooPolicy, "Should not have found relative foo policy the foo manager") + + policy, ok = cm.policyForItem(comparable{ + path: []string{"root", "foo"}, + ConfigValue: &cb.ConfigValue{ + ModPolicy: "/rootPolicy", + }, + }) + assert.True(t, ok) + assert.Equal(t, policy, rootPolicy, "Should not have found absolute root policy from the foo path position") +} diff --git a/common/mocks/policies/policies.go b/common/mocks/policies/policies.go index da0d7dcf910..c6d65e44886 100644 --- a/common/mocks/policies/policies.go +++ b/common/mocks/policies/policies.go @@ -43,11 +43,20 @@ type Manager struct { // PolicyMap is returned is used to look up Policies in PolicyMap map[string]*Policy + + // SubManagers is used for the return value of Manager and SubManagers + SubManagersMap map[string]*Manager } -// Managers panics +// Managers returns the values of SubManagers func (m *Manager) SubManagers() []policies.Manager { - panic("Unimplimented") + result := make([]policies.Manager, len(m.SubManagersMap)) + i := 0 + for _, manager := range m.SubManagersMap { + result[i] = manager + i++ + } + return result } // PolicyNames panics @@ -60,9 +69,13 @@ func (m *Manager) BasePath() string { return m.BasePathVal } -// Manager always returns itself +// Manager returns the Manager from SubManagers for the last component of the path func (m *Manager) Manager(path []string) (policies.Manager, bool) { - return m, true + if len(path) == 0 { + return m, true + } + manager, ok := m.SubManagersMap[path[len(path)-1]] + return manager, ok } // GetPolicy returns the value of Manager.Policy and whether it was nil or not