From 59ca22ce99784d35e1f0a65de5e9af20c93f9a05 Mon Sep 17 00:00:00 2001 From: Liran Funaro Date: Sun, 21 Jun 2026 18:57:38 +0300 Subject: [PATCH 1/2] [dependency] Replace unmaintained library govaluate with expr Signed-off-by: Liran Funaro --- common/policydsl/policyparser.go | 419 +++++++++----------------- common/policydsl/policyparser_test.go | 143 ++++++--- go.mod | 2 +- go.sum | 4 +- tools/configtxgen/encoder_test.go | 5 +- 5 files changed, 249 insertions(+), 324 deletions(-) diff --git a/common/policydsl/policyparser.go b/common/policydsl/policyparser.go index f06e28e8d9..d7c7c192d4 100644 --- a/common/policydsl/policyparser.go +++ b/common/policydsl/policyparser.go @@ -1,5 +1,5 @@ /* -Copyright IBM Corp. 2017 All Rights Reserved. +Copyright IBM Corp. All Rights Reserved. SPDX-License-Identifier: Apache-2.0 */ @@ -13,341 +13,204 @@ import ( "strconv" "strings" - "github.com/Knetic/govaluate" + "github.com/expr-lang/expr" cb "github.com/hyperledger/fabric-protos-go-apiv2/common" mb "github.com/hyperledger/fabric-protos-go-apiv2/msp" "google.golang.org/protobuf/proto" ) -// Gate values +type parser struct { + principals []*mb.MSPPrincipal + principalsIDMap map[string]int32 +} + +// Gate values. const ( GateAnd = "And" GateOr = "Or" GateOutOf = "OutOf" ) -// Role values for principals -const ( - RoleAdmin = "admin" - RoleMember = "member" - RoleClient = "client" - RolePeer = "peer" - RoleOrderer = "orderer" -) - -var ( - regex = regexp.MustCompile( - fmt.Sprintf("^([[:alnum:].-]+)([.])(%s|%s|%s|%s|%s)$", - RoleAdmin, RoleMember, RoleClient, RolePeer, RoleOrderer), - ) - regexErr = regexp.MustCompile("^No parameter '([^']+)' found[.]$") -) - -// a stub function - it returns the same string as it's passed. -// This will be evaluated by second/third passes to convert to a proto policy -func outof(args ...interface{}) (interface{}, error) { - toret := "outof(" - - if len(args) < 2 { - return nil, fmt.Errorf("expected at least two arguments to NOutOf. Given %d", len(args)) - } - - arg0 := args[0] - // govaluate treats all numbers as float64 only. But and/or may pass int/string. Allowing int/string for flexibility of caller - if n, ok := arg0.(float64); ok { - toret += strconv.Itoa(int(n)) - } else if n, ok := arg0.(int); ok { - toret += strconv.Itoa(n) - } else if n, ok := arg0.(string); ok { - toret += n - } else { - return nil, fmt.Errorf("unexpected type %s", reflect.TypeOf(arg0)) - } - - for _, arg := range args[1:] { - toret += ", " - - switch t := arg.(type) { - case string: - if regex.MatchString(t) { - toret += "'" + t + "'" - } else { - toret += t - } - default: - return nil, fmt.Errorf("unexpected type %s", reflect.TypeOf(arg)) - } - } - - return toret + ")", nil -} - -func and(args ...interface{}) (interface{}, error) { - args = append([]interface{}{len(args)}, args...) - return outof(args...) -} - -func or(args ...interface{}) (interface{}, error) { - args = append([]interface{}{1}, args...) - return outof(args...) -} - -func firstPass(args ...interface{}) (interface{}, error) { - toret := "outof(ID" - for _, arg := range args { - toret += ", " - - switch t := arg.(type) { - case string: - if regex.MatchString(t) { - toret += "'" + t + "'" - } else { - toret += t - } - case float32: - case float64: - toret += strconv.Itoa(int(t)) - default: - return nil, fmt.Errorf("unexpected type %s", reflect.TypeOf(arg)) - } - } - - return toret + ")", nil -} - -func secondPass(args ...interface{}) (interface{}, error) { - /* general sanity check, we expect at least 3 args */ - if len(args) < 3 { - return nil, fmt.Errorf("at least 3 arguments expected, got %d", len(args)) - } - - /* get the first argument, we expect it to be the context */ - var ctx *context - switch v := args[0].(type) { - case *context: - ctx = v - default: - return nil, fmt.Errorf("unrecognized type, expected the context, got %s", reflect.TypeOf(args[0])) - } - - /* get the second argument, we expect an integer telling us - how many of the remaining we expect to have*/ - var t int - switch arg := args[1].(type) { - case float64: - t = int(arg) - default: - return nil, fmt.Errorf("unrecognized type, expected a number, got %s", reflect.TypeOf(args[1])) - } - - /* get the n in the t out of n */ - n := len(args) - 2 - - /* sanity check - t should be positive, permit equal to n+1, but disallow over n+1 */ - if t < 0 || t > n+1 { - return nil, fmt.Errorf("invalid t-out-of-n predicate, t %d, n %d", t, n) - } - - policies := make([]*cb.SignaturePolicy, 0) - - /* handle the rest of the arguments */ - for _, principal := range args[2:] { - switch t := principal.(type) { - /* if it's a string, we expect it to be formed as - . , where MSP_ID is the MSP identifier - and ROLE is either a member, an admin, a client, a peer or an orderer*/ - case string: - /* split the string */ - subm := regex.FindAllStringSubmatch(t, -1) - if subm == nil || len(subm) != 1 || len(subm[0]) != 4 { - return nil, fmt.Errorf("error parsing principal %s", t) - } - - /* get the right role */ - var r mb.MSPRole_MSPRoleType - - switch subm[0][3] { - case RoleMember: - r = mb.MSPRole_MEMBER - case RoleAdmin: - r = mb.MSPRole_ADMIN - case RoleClient: - r = mb.MSPRole_CLIENT - case RolePeer: - r = mb.MSPRole_PEER - case RoleOrderer: - r = mb.MSPRole_ORDERER - default: - return nil, fmt.Errorf("error parsing role %s", t) - } - - /* build the principal we've been told */ - mspRole, err := proto.Marshal(&mb.MSPRole{MspIdentifier: subm[0][1], Role: r}) - if err != nil { - return nil, fmt.Errorf("error marshalling msp role: %s", err) - } - - p := &mb.MSPPrincipal{ - PrincipalClassification: mb.MSPPrincipal_ROLE, - Principal: mspRole, - } - ctx.principals = append(ctx.principals, p) - - /* create a SignaturePolicy that requires a signature from - the principal we've just built*/ - dapolicy := SignedBy(int32(ctx.IDNum)) - policies = append(policies, dapolicy) - - /* increment the identity counter. Note that this is - suboptimal as we are not reusing identities. We - can deduplicate them easily and make this puppy - smaller. For now it's fine though */ - // TODO: deduplicate principals - ctx.IDNum++ - - /* if we've already got a policy we're good, just append it */ - case *cb.SignaturePolicy: - policies = append(policies, t) - - default: - return nil, fmt.Errorf("unrecognized type, expected a principal or a policy, got %s", reflect.TypeOf(principal)) - } - } - - return NOutOf(int32(t), policies), nil -} - -type context struct { - IDNum int - principals []*mb.MSPPrincipal -} - -func newContext() *context { - return &context{IDNum: 0, principals: make([]*mb.MSPPrincipal, 0)} -} +var roleRegex = regexp.MustCompile("^([[:alnum:].-]+)[.](admin|member|client|peer|orderer)$") // FromString takes a string representation of the policy, // parses it and returns a SignaturePolicyEnvelope that // implements that policy. The supported language is as follows: // // GATE(P[, P]) +// OutOf(N, P[, P]) // // where: -// - GATE is either "and" or "or" -// - P is either a principal or another nested call to GATE +// - GATE is either "And" (also "AND" or "and") or "Or" (also "OR" or "or") +// - OutOf also supports "OUTOF" or "outof" +// - P is either a principal or another nested call to GATE or OutOf +// - N is a positive number up to the number of arguments // // A principal is defined as: // -// # ORG.ROLE +// ORG.ROLE // // where: // - ORG is a string (representing the MSP identifier) -// - ROLE takes the value of any of the RoleXXX constants representing -// the required role +// - ROLE takes the value of admin, member, client, peer, or orderer func FromString(policy string) (*cb.SignaturePolicyEnvelope, error) { - // first we translate the and/or business into outof gates - intermediate, err := govaluate.NewEvaluableExpressionWithFunctions( - policy, map[string]govaluate.ExpressionFunction{ - GateAnd: and, - strings.ToLower(GateAnd): and, - strings.ToUpper(GateAnd): and, - GateOr: or, - strings.ToLower(GateOr): or, - strings.ToUpper(GateOr): or, - GateOutOf: outof, - strings.ToLower(GateOutOf): outof, - strings.ToUpper(GateOutOf): outof, - }, + p := parser{ + principalsIDMap: make(map[string]int32), + } + program, err := expr.Compile( + policy, + expr.Function(GateAnd, p.and), + expr.Function(strings.ToLower(GateAnd), p.and), + expr.Function(strings.ToUpper(GateAnd), p.and), + expr.Function(GateOr, p.or), + expr.Function(strings.ToLower(GateOr), p.or), + expr.Function(strings.ToUpper(GateOr), p.or), + expr.Function(GateOutOf, p.nOutOf), + expr.Function(strings.ToLower(GateOutOf), p.nOutOf), + expr.Function(strings.ToUpper(GateOutOf), p.nOutOf), ) if err != nil { return nil, err } - - intermediateRes, err := intermediate.Evaluate(map[string]interface{}{}) + res, err := expr.Run(program, nil) if err != nil { - // attempt to produce a meaningful error - if regexErr.MatchString(err.Error()) { - sm := regexErr.FindStringSubmatch(err.Error()) - if len(sm) == 2 { - return nil, fmt.Errorf("unrecognized token '%s' in policy string", sm[1]) - } - } - return nil, err } - - resStr, ok := intermediateRes.(string) + rule, ok := res.(*cb.SignaturePolicy) if !ok { return nil, fmt.Errorf("invalid policy string '%s'", policy) } - // we still need two passes. The first pass just adds an extra - // argument ID to each of the outof calls. This is - // required because govaluate has no means of giving context - // to user-implemented functions other than via arguments. - // We need this argument because we need a global place where - // we put the identities that the policy requires - exp, err := govaluate.NewEvaluableExpressionWithFunctions( - resStr, - map[string]govaluate.ExpressionFunction{"outof": firstPass}, - ) - if err != nil { - return nil, err + return &cb.SignaturePolicyEnvelope{ + Identities: p.principals, + Version: 0, + Rule: rule, + }, nil +} + +func (p *parser) nOutOf(args ...any) (any, error) { + if len(args) < 2 { + return nil, fmt.Errorf("expected at least two arguments to NOutOf. Given %d", len(args)) } - res, err := exp.Evaluate(map[string]interface{}{}) - if err != nil { - // attempt to produce a meaningful error - if regexErr.MatchString(err.Error()) { - sm := regexErr.FindStringSubmatch(err.Error()) - if len(sm) == 2 { - return nil, fmt.Errorf("unrecognized token '%s' in policy string", sm[1]) - } + // How many of the policies we expect to accept. + var t int + switch arg := args[0].(type) { + case int: + t = arg + case float64: + t = int(arg) + case string: + var err error + t, err = strconv.Atoi(arg) + if err != nil { + return nil, fmt.Errorf("unrecognized type, expected a number, got %s", reflect.TypeOf(args[0])) } - - return nil, err + default: + return nil, fmt.Errorf("unrecognized type, expected a number, got %s", reflect.TypeOf(args[0])) } - resStr, ok = res.(string) - if !ok { - return nil, fmt.Errorf("invalid policy string '%s'", policy) + // Sanity check - t should be positive, permit equal to n+1, but disallow over n+1. + n := len(args[1:]) + if t < 0 || t > n+1 { + return nil, fmt.Errorf("invalid t-out-of-n predicate, t %d, n %d", t, n) } - ctx := newContext() - parameters := make(map[string]interface{}, 1) - parameters["ID"] = ctx + policies, err := p.parsePolicies(args[1:]) + if err != nil { + return nil, err + } + return NOutOf(int32(t), policies), nil //nolint:gosec // strconv.Atoi result conversion to int16/32. +} - exp, err = govaluate.NewEvaluableExpressionWithFunctions( - resStr, - map[string]govaluate.ExpressionFunction{"outof": secondPass}, - ) +func (p *parser) and(args ...any) (any, error) { + policies, err := p.parsePolicies(args) if err != nil { return nil, err } + return NOutOf(int32(len(policies)), policies), nil //nolint:gosec // int -> int32. +} - res, err = exp.Evaluate(parameters) +func (p *parser) or(args ...any) (any, error) { + policies, err := p.parsePolicies(args) if err != nil { - // attempt to produce a meaningful error - if regexErr.MatchString(err.Error()) { - sm := regexErr.FindStringSubmatch(err.Error()) - if len(sm) == 2 { - return nil, fmt.Errorf("unrecognized token '%s' in policy string", sm[1]) + return nil, err + } + return NOutOf(1, policies), nil +} + +func (p *parser) parsePolicies(policyArgs []any) ([]*cb.SignaturePolicy, error) { + if len(policyArgs) < 1 { + return nil, fmt.Errorf("at least one policy arguments expected, got %d", len(policyArgs)) + } + + policies := make([]*cb.SignaturePolicy, len(policyArgs)) + for i, arg := range policyArgs { + switch principal := arg.(type) { + // If it's a string, we expect it to be formed as ., + // where MSP_ID is the MSP identifier and ROLE is either a member, + // an admin, a client, a peer or an orderer. + case string: + policy, err := p.parsePolicy(principal) + if err != nil { + return nil, err } + policies[i] = policy + + // If we've already got a policy we're good, just append it. + case *cb.SignaturePolicy: + policies[i] = principal + + default: + valType := reflect.TypeOf(principal) + return nil, fmt.Errorf("unrecognized type, expected a principal or a policy, got %s", valType) } + } + return policies, nil +} - return nil, err +func (p *parser) parsePolicy(val string) (*cb.SignaturePolicy, error) { + subMatch := roleRegex.FindStringSubmatch(val) + if subMatch == nil { + return nil, fmt.Errorf("unrecognized token '%s' in policy string", val) } + mspID := subMatch[1] + roleName := strings.ToUpper(subMatch[2]) + key := mspID + "." + roleName - rule, ok := res.(*cb.SignaturePolicy) + principalID, ok := p.principalsIDMap[key] if !ok { - return nil, fmt.Errorf("invalid policy string '%s'", policy) + principal, err := newPrinciple(mspID, roleName) + if err != nil { + return nil, err + } + principalID = int32(len(p.principals)) //nolint:gosec // int -> int32. + p.principals = append(p.principals, principal) + p.principalsIDMap[key] = principalID } - p := &cb.SignaturePolicyEnvelope{ - Identities: ctx.principals, - Version: 0, - Rule: rule, + // Create a SignaturePolicy that requires a signature from the principal we've just built. + return SignedBy(principalID), nil +} + +func newPrinciple(mspID, roleName string) (*mb.MSPPrincipal, error) { + // get the MSP role. + role, ok := mb.MSPRole_MSPRoleType_value[roleName] + if !ok { + return nil, fmt.Errorf("error parsing role %s", roleName) + } + + // Build the principal we've been told. + mspRole, err := proto.Marshal(&mb.MSPRole{ + MspIdentifier: mspID, + Role: mb.MSPRole_MSPRoleType(role), + }) + if err != nil { + return nil, fmt.Errorf("error marshalling msp role: %w", err) } - return p, nil + return &mb.MSPPrincipal{ + PrincipalClassification: mb.MSPPrincipal_ROLE, + Principal: mspRole, + }, nil } diff --git a/common/policydsl/policyparser_test.go b/common/policydsl/policyparser_test.go index 131b45b823..5ea2efbfd5 100644 --- a/common/policydsl/policyparser_test.go +++ b/common/policydsl/policyparser_test.go @@ -1,5 +1,5 @@ /* -Copyright IBM Corp. 2017 All Rights Reserved. +Copyright IBM Corp. All Rights Reserved. SPDX-License-Identifier: Apache-2.0 */ @@ -12,9 +12,9 @@ import ( "github.com/hyperledger/fabric-protos-go-apiv2/common" "github.com/hyperledger/fabric-protos-go-apiv2/msp" "github.com/stretchr/testify/require" - "google.golang.org/protobuf/proto" "github.com/hyperledger/fabric-x-common/protoutil" + "github.com/hyperledger/fabric-x-common/utils/test" ) func TestOutOf1(t *testing.T) { @@ -39,7 +39,7 @@ func TestOutOf1(t *testing.T) { Identities: principals, } - require.Equal(t, p1, p2) + test.RequireProtoEqual(t, p1, p2) } func TestOutOf2(t *testing.T) { @@ -64,7 +64,7 @@ func TestOutOf2(t *testing.T) { Identities: principals, } - require.Equal(t, p1, p2) + test.RequireProtoEqual(t, p1, p2) } func TestAnd(t *testing.T) { @@ -89,7 +89,7 @@ func TestAnd(t *testing.T) { Identities: principals, } - require.Equal(t, p1, p2) + test.RequireProtoEqual(t, p1, p2) } func TestAndClientPeerOrderer(t *testing.T) { @@ -114,7 +114,7 @@ func TestAndClientPeerOrderer(t *testing.T) { Identities: principals, } - require.True(t, proto.Equal(p1, p2)) + test.RequireProtoEqual(t, p1, p2) } func TestOr(t *testing.T) { @@ -139,7 +139,7 @@ func TestOr(t *testing.T) { Identities: principals, } - require.Equal(t, p1, p2) + test.RequireProtoEqual(t, p1, p2) } func TestComplex1(t *testing.T) { @@ -169,7 +169,7 @@ func TestComplex1(t *testing.T) { Identities: principals, } - require.Equal(t, p1, p2) + test.RequireProtoEqual(t, p1, p2) } func TestComplex2(t *testing.T) { @@ -204,7 +204,7 @@ func TestComplex2(t *testing.T) { Identities: principals, } - require.Equal(t, p1, p2) + test.RequireProtoEqual(t, p1, p2) } func TestMSPIDWIthSpecialChars(t *testing.T) { @@ -215,17 +215,26 @@ func TestMSPIDWIthSpecialChars(t *testing.T) { principals = append(principals, &msp.MSPPrincipal{ PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "MSP"}), + Principal: protoutil.MarshalOrPanic(&msp.MSPRole{ + Role: msp.MSPRole_MEMBER, + MspIdentifier: "MSP", + }), }) principals = append(principals, &msp.MSPPrincipal{ PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "MSP.WITH.DOTS"}), + Principal: protoutil.MarshalOrPanic(&msp.MSPRole{ + Role: msp.MSPRole_MEMBER, + MspIdentifier: "MSP.WITH.DOTS", + }), }) principals = append(principals, &msp.MSPPrincipal{ PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "MSP-WITH-DASHES"}), + Principal: protoutil.MarshalOrPanic(&msp.MSPRole{ + Role: msp.MSPRole_MEMBER, + MspIdentifier: "MSP-WITH-DASHES", + }), }) p2 := &common.SignaturePolicyEnvelope{ @@ -234,18 +243,18 @@ func TestMSPIDWIthSpecialChars(t *testing.T) { Identities: principals, } - require.Equal(t, p1, p2) + test.RequireProtoEqual(t, p1, p2) } func TestBadStringsNoPanic(t *testing.T) { - _, err := FromString("OR('A.member', Bmember)") // error after 1st Evaluate() - require.EqualError(t, err, "unrecognized token 'Bmember' in policy string") + _, err := FromString("OR('A.member', Bmember)") + require.ErrorContains(t, err, "cannot fetch Bmember") - _, err = FromString("OR('A.member', 'Bmember')") // error after 2nd Evalute() - require.EqualError(t, err, "unrecognized token 'Bmember' in policy string") + _, err = FromString("OR('A.member', 'Bmember')") + require.ErrorContains(t, err, "unrecognized token 'Bmember' in policy string") - _, err = FromString(`OR('A.member', '\'Bmember\'')`) // error after 3rd Evalute() - require.EqualError(t, err, "unrecognized token 'Bmember' in policy string") + _, err = FromString(`OR('A.member', '\'Bmember\'')`) + require.ErrorContains(t, err, "unrecognized token ''Bmember'' in policy string") } func TestNodeOUs(t *testing.T) { @@ -280,7 +289,7 @@ func TestNodeOUs(t *testing.T) { Identities: principals, } - require.Equal(t, p1, p2) + test.RequireProtoEqual(t, p1, p2) } func TestOutOfNumIsString(t *testing.T) { @@ -305,45 +314,53 @@ func TestOutOfNumIsString(t *testing.T) { Identities: principals, } - require.Equal(t, p1, p2) + test.RequireProtoEqual(t, p1, p2) } func TestOutOfErrorCase(t *testing.T) { - p1, err1 := FromString("") // 1st NewEvaluableExpressionWithFunctions() returns an error + p1, err1 := FromString("") require.Nil(t, p1) - require.EqualError(t, err1, "Unexpected end of expression") + require.ErrorContains(t, err1, "unexpected token EOF") p2, err2 := FromString("OutOf(1)") // outof() if len(args)<2 require.Nil(t, p2) - require.EqualError(t, err2, "expected at least two arguments to NOutOf. Given 1") + require.ErrorContains(t, err2, "expected at least two arguments to NOutOf. Given 1") - p3, err3 := FromString("OutOf(true, 'A.member')") // outof() }else{. 1st arg is non of float, int, string + p2a, err2a := FromString("And()") // and() if len(args)<1 + require.Nil(t, p2a) + require.ErrorContains(t, err2a, "at least one policy arguments expected, got 0") + + p2b, err2b := FromString("Or()") // or() if len(args)<1 + require.Nil(t, p2b) + require.ErrorContains(t, err2b, "at least one policy arguments expected, got 0") + + p3, err3 := FromString("OutOf(true, 'A.member')") // outof() 1st arg is non of float, int, string require.Nil(t, p3) - require.EqualError(t, err3, "unexpected type bool") + require.ErrorContains(t, err3, "unrecognized type, expected a number, got bool") p4, err4 := FromString("OutOf(1, 2)") // oufof() switch default. 2nd arg is not string. require.Nil(t, p4) - require.EqualError(t, err4, "unexpected type float64") + require.ErrorContains(t, err4, "unrecognized type, expected a principal or a policy, got int") - p5, err5 := FromString("OutOf(1, 'true')") // firstPass() switch default + p5, err5 := FromString("OutOf(1, 'true')") // switch default require.Nil(t, p5) - require.EqualError(t, err5, "unexpected type bool") + require.ErrorContains(t, err5, "unrecognized token 'true' in policy string") - p6, err6 := FromString(`OutOf('\'\\\'A\\\'\'', 'B.member')`) // secondPass() switch args[1].(type) default + p6, err6 := FromString(`OutOf('\'\\\'A\\\'\'', 'B.member')`) // switch default require.Nil(t, p6) - require.EqualError(t, err6, "unrecognized type, expected a number, got string") + require.ErrorContains(t, err6, "unrecognized type, expected a number, got string") - p7, err7 := FromString(`OutOf(1, '\'1\'')`) // secondPass() switch args[1].(type) default + p7, err7 := FromString(`OutOf(1, '\'1\'')`) // switch default require.Nil(t, p7) - require.EqualError(t, err7, "unrecognized type, expected a principal or a policy, got float64") + require.ErrorContains(t, err7, "unrecognized token ''1'' in policy string") - p8, err8 := FromString(`''`) // 2nd NewEvaluateExpressionWithFunction() returns an error + p8, err8 := FromString(`''`) require.Nil(t, p8) - require.EqualError(t, err8, "Unexpected end of expression") + require.ErrorContains(t, err8, "invalid policy string") - p9, err9 := FromString(`'\'\''`) // 3rd NewEvaluateExpressionWithFunction() returns an error + p9, err9 := FromString(`'\'\''`) require.Nil(t, p9) - require.EqualError(t, err9, "Unexpected end of expression") + require.ErrorContains(t, err9, "invalid policy string") } func TestBadStringBeforeFAB11404_ThisCanDeleteAfterFAB11404HasMerged(t *testing.T) { @@ -368,7 +385,7 @@ func TestSecondPassBoundaryCheck(t *testing.T) { // Prohibit t<0 p0, err0 := FromString("OutOf(-1, 'A.member', 'B.member')") require.Nil(t, p0) - require.EqualError(t, err0, "invalid t-out-of-n predicate, t -1, n 2") + require.ErrorContains(t, err0, "invalid t-out-of-n predicate, t -1, n 2") // Permit t==0 : always satisfied policy // There is no clear usecase of t=0, but somebody may already use it, so we don't treat as an error. @@ -388,7 +405,7 @@ func TestSecondPassBoundaryCheck(t *testing.T) { Rule: NOutOf(0, []*common.SignaturePolicy{SignedBy(0), SignedBy(1)}), Identities: principals, } - require.Equal(t, expected1, p1) + test.RequireProtoEqual(t, expected1, p1) // Check upper boundary // Permit t==n+1 : never satisfied policy @@ -400,10 +417,54 @@ func TestSecondPassBoundaryCheck(t *testing.T) { Rule: NOutOf(3, []*common.SignaturePolicy{SignedBy(0), SignedBy(1)}), Identities: principals, } - require.Equal(t, expected2, p2) + test.RequireProtoEqual(t, expected2, p2) // Prohibit t>n + 1 p3, err3 := FromString("OutOf(4, 'A.member', 'B.member')") require.Nil(t, p3) - require.EqualError(t, err3, "invalid t-out-of-n predicate, t 4, n 2") + require.ErrorContains(t, err3, "invalid t-out-of-n predicate, t 4, n 2") +} + +func TestPrincipalDeduplication(t *testing.T) { + t.Parallel() + + // Test that duplicate principals are deduplicated + // AND('A.member', OR('B.member', 'A.member')) should only have 2 identities, not 3 + p1, err := FromString("AND('A.member', OR('B.member', 'A.member'))") + require.NoError(t, err) + + // Verify we only have 2 principals (A.member and B.member), not 3 + require.Len(t, p1.Identities, 2, "expected 2 unique principals after deduplication") + + principals := []*msp.MSPPrincipal{ + { // B.member appears first during parsing (inside the nested OR), so it gets index 0 + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic(&msp.MSPRole{ + Role: msp.MSPRole_MEMBER, MspIdentifier: "B", + }), + }, + { // A.member appears second, so it gets index 1 + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic(&msp.MSPRole{ + Role: msp.MSPRole_MEMBER, MspIdentifier: "A", + }), + }, + } + + // The expected structure: + // - B.member is at index 0 + // - A.member is at index 1 + // - The rule is AND(SignedBy(1), OR(SignedBy(0), SignedBy(1))) + // Note: A.member appears twice in the rule but only once in identities + p2 := &common.SignaturePolicyEnvelope{ + Version: 0, + Rule: NOutOf(2, []*common.SignaturePolicy{ + SignedBy(1), NOutOf(1, []*common.SignaturePolicy{ + SignedBy(0), SignedBy(1), + }), + }), + Identities: principals, + } + + test.RequireProtoEqual(t, p1, p2) } diff --git a/go.mod b/go.mod index c1670cd234..1dde198126 100644 --- a/go.mod +++ b/go.mod @@ -8,9 +8,9 @@ go 1.26.3 require ( github.com/IBM/idemix v0.1.1 - github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible github.com/alecthomas/kingpin/v2 v2.4.0 github.com/cockroachdb/errors v1.12.0 + github.com/expr-lang/expr v1.17.8 github.com/go-viper/mapstructure/v2 v2.5.0 github.com/gorilla/handlers v1.5.1 github.com/gorilla/mux v1.8.0 diff --git a/go.sum b/go.sum index eaf2dc2f82..ee24a1a250 100644 --- a/go.sum +++ b/go.sum @@ -11,8 +11,6 @@ github.com/IBM/idemix v0.1.1 h1:vU0sx/yZHw5peHdfLhQZ7A4X+H+U4UfZY0vyooBmFlY= github.com/IBM/idemix v0.1.1/go.mod h1:ZtSPMmSzhK5MGDtd+Yl8IRscc8vze1HGhTXLnsJzOus= github.com/IBM/mathlib v0.2.0 h1:/6KUsbFm04UV9aBV7oN6+jGMfckxmh9+HcLVartRhMQ= github.com/IBM/mathlib v0.2.0/go.mod h1:5KCSHHlg4Ec6w5gets9IVIa2H7qe0Cl47v87CCS9BEQ= -github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible h1:1G1pk05UrOh0NlF1oeaaix1x8XzrfjIDK47TY0Zehcw= -github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible/go.mod h1:r7JcOSlj0wfOMncg0iLm8Leh48TZaKVeNIfJntJ2wa0= github.com/Masterminds/semver/v3 v3.5.0 h1:kQceYJfbupGfZOKZQg0kou0DgAKhzDg2NZPAwZ/2OOE= github.com/Masterminds/semver/v3 v3.5.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= github.com/alecthomas/kingpin/v2 v2.4.0 h1:f48lwail6p8zpO1bC4TxtqACaGqHYA22qkHjHpqDjYY= @@ -50,6 +48,8 @@ github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymF github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= +github.com/expr-lang/expr v1.17.8 h1:W1loDTT+0PQf5YteHSTpju2qfUfNoBt4yw9+wOEU9VM= +github.com/expr-lang/expr v1.17.8/go.mod h1:8/vRC7+7HBzESEqt5kKpYXxrxkr31SaO8r40VO/1IT4= github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM= github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU= github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= diff --git a/tools/configtxgen/encoder_test.go b/tools/configtxgen/encoder_test.go index 6071fc82ba..a017cb2a3e 100644 --- a/tools/configtxgen/encoder_test.go +++ b/tools/configtxgen/encoder_test.go @@ -202,8 +202,9 @@ var _ = ginkgo.Describe("Encoder", func() { ginkgo.It("wraps and returns the error", func() { err := addPolicies(cg, policies, "Readers") - gomega.Expect(err).To(gomega.MatchError("invalid signature policy rule 'garbage': " + - "unrecognized token 'garbage' in policy string")) + gomega.Expect(err).To(gomega.MatchError(gomega.ContainSubstring( + "invalid signature policy rule 'garbage'", + ))) }) }) From 1b3f1a77465954823ca4cbc941bab857d99fb80b Mon Sep 17 00:00:00 2001 From: Liran Funaro Date: Thu, 2 Jul 2026 14:06:57 +0300 Subject: [PATCH 2/2] Address PR comments Signed-off-by: Liran Funaro --- common/policydsl/policyparser.go | 63 ++-- common/policydsl/policyparser_test.go | 512 ++++++++++++++------------ 2 files changed, 319 insertions(+), 256 deletions(-) diff --git a/common/policydsl/policyparser.go b/common/policydsl/policyparser.go index d7c7c192d4..a880f4be03 100644 --- a/common/policydsl/policyparser.go +++ b/common/policydsl/policyparser.go @@ -10,10 +10,10 @@ import ( "fmt" "reflect" "regexp" - "strconv" "strings" "github.com/expr-lang/expr" + "github.com/expr-lang/expr/ast" cb "github.com/hyperledger/fabric-protos-go-apiv2/common" mb "github.com/hyperledger/fabric-protos-go-apiv2/msp" "google.golang.org/protobuf/proto" @@ -26,23 +26,35 @@ type parser struct { // Gate values. const ( - GateAnd = "And" - GateOr = "Or" - GateOutOf = "OutOf" + GateAnd = "and" + GateOr = "or" + GateOutOf = "outof" ) -var roleRegex = regexp.MustCompile("^([[:alnum:].-]+)[.](admin|member|client|peer|orderer)$") +// Pattern matches a role case-insensitively: +// +// ^ - Assert start of string +// (?i) - Enable case-insensitivity +// ( - Group 1 start: Identifier +// [[:alnum:].-]+ - Alphanumeric, dot, or hyphen (1 or more) +// ) - Group 1 end +// [.] - Match a literal dot +// ( - Group 2 start: Role +// admin|member|client|... - Specific system role options +// ) - Group 2 end +// $ - Assert end of string +var roleRegex = regexp.MustCompile("^(?i)([[:alnum:].-]+)[.](admin|member|client|peer|orderer)$") // FromString takes a string representation of the policy, // parses it and returns a SignaturePolicyEnvelope that // implements that policy. The supported language is as follows: // -// GATE(P[, P]) -// OutOf(N, P[, P]) +// GATE(P[, P]) +// OutOf(N, P[, P]) // // where: -// - GATE is either "And" (also "AND" or "and") or "Or" (also "OR" or "or") -// - OutOf also supports "OUTOF" or "outof" +// - GATE is either "And" or "Or" (case-insensitive) +// - OutOf is case-insensitive // - P is either a principal or another nested call to GATE or OutOf // - N is a positive number up to the number of arguments // @@ -59,15 +71,10 @@ func FromString(policy string) (*cb.SignaturePolicyEnvelope, error) { } program, err := expr.Compile( policy, + expr.Patch(&p), expr.Function(GateAnd, p.and), - expr.Function(strings.ToLower(GateAnd), p.and), - expr.Function(strings.ToUpper(GateAnd), p.and), expr.Function(GateOr, p.or), - expr.Function(strings.ToLower(GateOr), p.or), - expr.Function(strings.ToUpper(GateOr), p.or), expr.Function(GateOutOf, p.nOutOf), - expr.Function(strings.ToLower(GateOutOf), p.nOutOf), - expr.Function(strings.ToUpper(GateOutOf), p.nOutOf), ) if err != nil { return nil, err @@ -94,19 +101,8 @@ func (p *parser) nOutOf(args ...any) (any, error) { } // How many of the policies we expect to accept. - var t int - switch arg := args[0].(type) { - case int: - t = arg - case float64: - t = int(arg) - case string: - var err error - t, err = strconv.Atoi(arg) - if err != nil { - return nil, fmt.Errorf("unrecognized type, expected a number, got %s", reflect.TypeOf(args[0])) - } - default: + t, ok := args[0].(int) + if !ok { return nil, fmt.Errorf("unrecognized type, expected a number, got %s", reflect.TypeOf(args[0])) } @@ -214,3 +210,14 @@ func newPrinciple(mspID, roleName string) (*mb.MSPPrincipal, error) { Principal: mspRole, }, nil } + +// Visit implements the expr.Visitor interface. +// It is used to traverse the AST and enforce lowercase naming conventions. +func (*parser) Visit(node *ast.Node) { + if callNode, isCallNode := (*node).(*ast.CallNode); isCallNode { + if identifierNode, isIDNode := callNode.Callee.(*ast.IdentifierNode); isIDNode { + // Enforce lowercase naming convention in the AST + identifierNode.Value = strings.ToLower(identifierNode.Value) + } + } +} diff --git a/common/policydsl/policyparser_test.go b/common/policydsl/policyparser_test.go index 5ea2efbfd5..f340f3c665 100644 --- a/common/policydsl/policyparser_test.go +++ b/common/policydsl/policyparser_test.go @@ -18,235 +18,294 @@ import ( ) func TestOutOf1(t *testing.T) { + t.Parallel() p1, err := FromString("OutOf(1, 'A.member', 'B.member')") require.NoError(t, err) - principals := make([]*msp.MSPPrincipal, 0) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}), - }) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}), - }) - p2 := &common.SignaturePolicyEnvelope{ - Version: 0, - Rule: NOutOf(1, []*common.SignaturePolicy{SignedBy(0), SignedBy(1)}), - Identities: principals, + Version: 0, + Rule: NOutOf(1, []*common.SignaturePolicy{SignedBy(0), SignedBy(1)}), + Identities: []*msp.MSPPrincipal{ + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}, + ), + }, + }, } test.RequireProtoEqual(t, p1, p2) } func TestOutOf2(t *testing.T) { + t.Parallel() p1, err := FromString("OutOf(2, 'A.member', 'B.member')") require.NoError(t, err) - principals := make([]*msp.MSPPrincipal, 0) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}), - }) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}), - }) - p2 := &common.SignaturePolicyEnvelope{ - Version: 0, - Rule: NOutOf(2, []*common.SignaturePolicy{SignedBy(0), SignedBy(1)}), - Identities: principals, + Version: 0, + Rule: NOutOf(2, []*common.SignaturePolicy{SignedBy(0), SignedBy(1)}), + Identities: []*msp.MSPPrincipal{ + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}, + ), + }, + }, } test.RequireProtoEqual(t, p1, p2) } func TestAnd(t *testing.T) { + t.Parallel() p1, err := FromString("AND('A.member', 'B.member')") require.NoError(t, err) - principals := make([]*msp.MSPPrincipal, 0) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}), - }) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}), - }) - p2 := &common.SignaturePolicyEnvelope{ - Version: 0, - Rule: And(SignedBy(0), SignedBy(1)), - Identities: principals, + Version: 0, + Rule: And(SignedBy(0), SignedBy(1)), + Identities: []*msp.MSPPrincipal{ + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}, + ), + }, + }, } test.RequireProtoEqual(t, p1, p2) } func TestAndClientPeerOrderer(t *testing.T) { + t.Parallel() p1, err := FromString("AND('A.client', 'B.peer')") require.NoError(t, err) - principals := make([]*msp.MSPPrincipal, 0) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_CLIENT, MspIdentifier: "A"}), - }) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_PEER, MspIdentifier: "B"}), - }) - p2 := &common.SignaturePolicyEnvelope{ - Version: 0, - Rule: And(SignedBy(0), SignedBy(1)), - Identities: principals, + Version: 0, + Rule: And(SignedBy(0), SignedBy(1)), + Identities: []*msp.MSPPrincipal{ + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_CLIENT, MspIdentifier: "A"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_PEER, MspIdentifier: "B"}, + ), + }, + }, } test.RequireProtoEqual(t, p1, p2) } func TestOr(t *testing.T) { + t.Parallel() p1, err := FromString("OR('A.member', 'B.member')") require.NoError(t, err) - principals := make([]*msp.MSPPrincipal, 0) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}), - }) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}), - }) - p2 := &common.SignaturePolicyEnvelope{ - Version: 0, - Rule: Or(SignedBy(0), SignedBy(1)), - Identities: principals, + Version: 0, + Rule: Or(SignedBy(0), SignedBy(1)), + Identities: []*msp.MSPPrincipal{ + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}, + ), + }, + }, } test.RequireProtoEqual(t, p1, p2) } func TestComplex1(t *testing.T) { + t.Parallel() p1, err := FromString("OR('A.member', AND('B.member', 'C.member'))") require.NoError(t, err) - principals := make([]*msp.MSPPrincipal, 0) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}), - }) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "C"}), - }) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}), - }) - p2 := &common.SignaturePolicyEnvelope{ - Version: 0, - Rule: Or(SignedBy(2), And(SignedBy(0), SignedBy(1))), - Identities: principals, + Version: 0, + Rule: Or(SignedBy(2), And(SignedBy(0), SignedBy(1))), + Identities: []*msp.MSPPrincipal{ + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "C"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}, + ), + }, + }, } test.RequireProtoEqual(t, p1, p2) } func TestComplex2(t *testing.T) { + t.Parallel() p1, err := FromString("OR(AND('A.member', 'B.member'), OR('C.admin', 'D.member'))") require.NoError(t, err) - principals := make([]*msp.MSPPrincipal, 0) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}), - }) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}), - }) + p2 := &common.SignaturePolicyEnvelope{ + Version: 0, + Rule: Or(And(SignedBy(0), SignedBy(1)), Or(SignedBy(2), SignedBy(3))), + Identities: []*msp.MSPPrincipal{ + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_ADMIN, MspIdentifier: "C"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "D"}, + ), + }, + }, + } - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_ADMIN, MspIdentifier: "C"}), - }) + test.RequireProtoEqual(t, p1, p2) +} - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "D"}), - }) +func TestCaseInsensitive(t *testing.T) { + t.Parallel() + p1, err := FromString( + "OuToF(1, AnD('A.mEmBer', 'B.cliENT'), Or('C.adMIn', 'D.orDerEr'))", + ) + require.NoError(t, err) p2 := &common.SignaturePolicyEnvelope{ - Version: 0, - Rule: Or(And(SignedBy(0), SignedBy(1)), Or(SignedBy(2), SignedBy(3))), - Identities: principals, + Version: 0, + Rule: NOutOf(1, []*common.SignaturePolicy{ + And(SignedBy(0), SignedBy(1)), + Or(SignedBy(2), SignedBy(3)), + }), + Identities: []*msp.MSPPrincipal{ + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_CLIENT, MspIdentifier: "B"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_ADMIN, MspIdentifier: "C"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_ORDERER, MspIdentifier: "D"}, + ), + }, + }, } test.RequireProtoEqual(t, p1, p2) } func TestMSPIDWIthSpecialChars(t *testing.T) { + t.Parallel() p1, err := FromString("OR('MSP.member', 'MSP.WITH.DOTS.member', 'MSP-WITH-DASHES.member')") require.NoError(t, err) - principals := make([]*msp.MSPPrincipal, 0) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{ - Role: msp.MSPRole_MEMBER, - MspIdentifier: "MSP", - }), - }) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{ - Role: msp.MSPRole_MEMBER, - MspIdentifier: "MSP.WITH.DOTS", - }), - }) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{ - Role: msp.MSPRole_MEMBER, - MspIdentifier: "MSP-WITH-DASHES", - }), - }) - p2 := &common.SignaturePolicyEnvelope{ - Version: 0, - Rule: NOutOf(1, []*common.SignaturePolicy{SignedBy(0), SignedBy(1), SignedBy(2)}), - Identities: principals, + Version: 0, + Rule: NOutOf(1, []*common.SignaturePolicy{SignedBy(0), SignedBy(1), SignedBy(2)}), + Identities: []*msp.MSPPrincipal{ + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic(&msp.MSPRole{ + Role: msp.MSPRole_MEMBER, + MspIdentifier: "MSP", + }), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic(&msp.MSPRole{ + Role: msp.MSPRole_MEMBER, + MspIdentifier: "MSP.WITH.DOTS", + }), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic(&msp.MSPRole{ + Role: msp.MSPRole_MEMBER, + MspIdentifier: "MSP-WITH-DASHES", + }), + }, + }, } test.RequireProtoEqual(t, p1, p2) } func TestBadStringsNoPanic(t *testing.T) { + t.Parallel() _, err := FromString("OR('A.member', Bmember)") require.ErrorContains(t, err, "cannot fetch Bmember") @@ -258,66 +317,46 @@ func TestBadStringsNoPanic(t *testing.T) { } func TestNodeOUs(t *testing.T) { + t.Parallel() p1, err := FromString("OR('A.peer', 'B.admin', 'C.orderer', 'D.client')") require.NoError(t, err) - principals := make([]*msp.MSPPrincipal, 0) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_PEER, MspIdentifier: "A"}), - }) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_ADMIN, MspIdentifier: "B"}), - }) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_ORDERER, MspIdentifier: "C"}), - }) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_CLIENT, MspIdentifier: "D"}), - }) - p2 := &common.SignaturePolicyEnvelope{ - Version: 0, - Rule: NOutOf(1, []*common.SignaturePolicy{SignedBy(0), SignedBy(1), SignedBy(2), SignedBy(3)}), - Identities: principals, - } - - test.RequireProtoEqual(t, p1, p2) -} - -func TestOutOfNumIsString(t *testing.T) { - p1, err := FromString("OutOf('1', 'A.member', 'B.member')") - require.NoError(t, err) - - principals := make([]*msp.MSPPrincipal, 0) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}), - }) - - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}), - }) - - p2 := &common.SignaturePolicyEnvelope{ - Version: 0, - Rule: NOutOf(1, []*common.SignaturePolicy{SignedBy(0), SignedBy(1)}), - Identities: principals, + Version: 0, + Rule: NOutOf(1, []*common.SignaturePolicy{SignedBy(0), SignedBy(1), SignedBy(2), SignedBy(3)}), + Identities: []*msp.MSPPrincipal{ + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_PEER, MspIdentifier: "A"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_ADMIN, MspIdentifier: "B"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_ORDERER, MspIdentifier: "C"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_CLIENT, MspIdentifier: "D"}, + ), + }, + }, } test.RequireProtoEqual(t, p1, p2) } func TestOutOfErrorCase(t *testing.T) { + t.Parallel() p1, err1 := FromString("") require.Nil(t, p1) require.ErrorContains(t, err1, "unexpected token EOF") @@ -364,6 +403,7 @@ func TestOutOfErrorCase(t *testing.T) { } func TestBadStringBeforeFAB11404_ThisCanDeleteAfterFAB11404HasMerged(t *testing.T) { + t.Parallel() s1 := "1" // ineger in string p1, err1 := FromString(s1) require.Nil(t, p1) @@ -380,7 +420,8 @@ func TestBadStringBeforeFAB11404_ThisCanDeleteAfterFAB11404HasMerged(t *testing. require.EqualError(t, err3, `invalid policy string ''\'1\'''`) } -func TestSecondPassBoundaryCheck(t *testing.T) { +func TestSecondBoundaryCheck(t *testing.T) { + t.Parallel() // Check lower boundary // Prohibit t<0 p0, err0 := FromString("OutOf(-1, 'A.member', 'B.member')") @@ -391,19 +432,23 @@ func TestSecondPassBoundaryCheck(t *testing.T) { // There is no clear usecase of t=0, but somebody may already use it, so we don't treat as an error. p1, err1 := FromString("OutOf(0, 'A.member', 'B.member')") require.NoError(t, err1) - principals := make([]*msp.MSPPrincipal, 0) - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}), - }) - principals = append(principals, &msp.MSPPrincipal{ - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}), - }) expected1 := &common.SignaturePolicyEnvelope{ - Version: 0, - Rule: NOutOf(0, []*common.SignaturePolicy{SignedBy(0), SignedBy(1)}), - Identities: principals, + Version: 0, + Rule: NOutOf(0, []*common.SignaturePolicy{SignedBy(0), SignedBy(1)}), + Identities: []*msp.MSPPrincipal{ + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}, + ), + }, + }, } test.RequireProtoEqual(t, expected1, p1) @@ -413,9 +458,22 @@ func TestSecondPassBoundaryCheck(t *testing.T) { p2, err2 := FromString("OutOf(3, 'A.member', 'B.member')") require.NoError(t, err2) expected2 := &common.SignaturePolicyEnvelope{ - Version: 0, - Rule: NOutOf(3, []*common.SignaturePolicy{SignedBy(0), SignedBy(1)}), - Identities: principals, + Version: 0, + Rule: NOutOf(3, []*common.SignaturePolicy{SignedBy(0), SignedBy(1)}), + Identities: []*msp.MSPPrincipal{ + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "A"}, + ), + }, + { + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic( + &msp.MSPRole{Role: msp.MSPRole_MEMBER, MspIdentifier: "B"}, + ), + }, + }, } test.RequireProtoEqual(t, expected2, p2) @@ -436,21 +494,6 @@ func TestPrincipalDeduplication(t *testing.T) { // Verify we only have 2 principals (A.member and B.member), not 3 require.Len(t, p1.Identities, 2, "expected 2 unique principals after deduplication") - principals := []*msp.MSPPrincipal{ - { // B.member appears first during parsing (inside the nested OR), so it gets index 0 - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{ - Role: msp.MSPRole_MEMBER, MspIdentifier: "B", - }), - }, - { // A.member appears second, so it gets index 1 - PrincipalClassification: msp.MSPPrincipal_ROLE, - Principal: protoutil.MarshalOrPanic(&msp.MSPRole{ - Role: msp.MSPRole_MEMBER, MspIdentifier: "A", - }), - }, - } - // The expected structure: // - B.member is at index 0 // - A.member is at index 1 @@ -463,7 +506,20 @@ func TestPrincipalDeduplication(t *testing.T) { SignedBy(0), SignedBy(1), }), }), - Identities: principals, + Identities: []*msp.MSPPrincipal{ + { // B.member appears first during parsing (inside the nested OR), so it gets index 0 + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic(&msp.MSPRole{ + Role: msp.MSPRole_MEMBER, MspIdentifier: "B", + }), + }, + { // A.member appears second, so it gets index 1 + PrincipalClassification: msp.MSPPrincipal_ROLE, + Principal: protoutil.MarshalOrPanic(&msp.MSPRole{ + Role: msp.MSPRole_MEMBER, MspIdentifier: "A", + }), + }, + }, } test.RequireProtoEqual(t, p1, p2)