From 952d13cd2b078dad97bdd38a71896eefe9de0d99 Mon Sep 17 00:00:00 2001 From: Airat Arifullin Date: Tue, 14 May 2024 12:23:26 +0300 Subject: [PATCH] [#1124] cli: Improve APE rule parsing * Make APE rule parser to read condition's kind in unambiguous using lexemes `ResourceCondition`, `RequestCondition` instead confusing `Object.Request`, `Object.Resource`. * Fix unit-tests. Signed-off-by: Airat Arifullin --- cmd/frostfs-cli/modules/control/add_rule.go | 2 +- cmd/frostfs-cli/modules/util/ape.go | 58 ++++----- cmd/frostfs-cli/modules/util/ape_test.go | 60 ++++----- go.mod | 2 +- go.sum | Bin 43495 -> 43495 bytes internal/ape/converter.go | 12 +- pkg/services/container/ape_test.go | 128 ++++++++++---------- pkg/services/object/ape/checker_test.go | 32 ++--- 8 files changed, 149 insertions(+), 145 deletions(-) diff --git a/cmd/frostfs-cli/modules/control/add_rule.go b/cmd/frostfs-cli/modules/control/add_rule.go index 45b36bfb1..a22d0525d 100644 --- a/cmd/frostfs-cli/modules/control/add_rule.go +++ b/cmd/frostfs-cli/modules/control/add_rule.go @@ -24,7 +24,7 @@ var addRuleCmd = &cobra.Command{ Long: "Add local APE rule to a node with following format:\n[:action_detail] [ ...] ", Example: `control add-rule --endpoint ... -w ... --address ... --chain-id ChainID --cid ... --rule "allow Object.Get *" --rule "deny Object.Get EbxzAdz5LB4uqxuz6crWKAumBNtZyK2rKsqQP7TdZvwr/*" ---rule "deny:QuotaLimitReached Object.Put Object.Resource:Department=HR *" +--rule "deny:QuotaLimitReached Object.Put ResourceCondition:Department=HR *" control add-rule --endpoint ... -w ... --address ... --chain-id ChainID --cid ... --path some_chain.json `, diff --git a/cmd/frostfs-cli/modules/util/ape.go b/cmd/frostfs-cli/modules/util/ape.go index d2dd0ced2..50253b366 100644 --- a/cmd/frostfs-cli/modules/util/ape.go +++ b/cmd/frostfs-cli/modules/util/ape.go @@ -21,7 +21,7 @@ var ( errUnknownAction = errors.New("action is not recognized") errUnknownBinaryOperator = errors.New("binary operator is not recognized") errUnknownCondObjectType = errors.New("condition object type is not recognized") - errMixedTypesInRule = errors.New("found mixed type of actions and conditions in rule") + errMixedTypesInRule = errors.New("found mixed type of actions in rule") errNoActionsInRule = errors.New("there are no actions in rule") errUnsupportedResourceFormat = errors.New("unsupported resource format") errFailedToParseAllAny = errors.New("any/all is not parsed") @@ -38,10 +38,10 @@ func PrintHumanReadableAPEChain(cmd *cobra.Command, chain *apechain.Chain) { cmd.Println("\tConditions:") for _, c := range rule.Condition { var ot string - switch c.Object { - case apechain.ObjectResource: + switch c.Kind { + case apechain.KindResource: ot = "Resource" - case apechain.ObjectRequest: + case apechain.KindRequest: ot = "Request" default: panic("unknown object type") @@ -100,9 +100,9 @@ func ParseAPEChain(chain *apechain.Chain, rules []string) error { // deny Object.Put * // deny:QuotaLimitReached Object.Put * // allow Object.Put * -// allow Object.Get Object.Resource:Department=HR Object.Request:Actor=ownerA * -// allow Object.Get any Object.Resource:Department=HR Object.Request:Actor=ownerA * -// allow Object.Get all Object.Resource:Department=HR Object.Request:Actor=ownerA * +// allow Object.Get ResourceCondition:Department=HR RequestCondition:Actor=ownerA * +// allow Object.Get any ResourceCondition:Department=HR RequestCondition:Actor=ownerA * +// allow Object.Get all ResourceCondition:Department=HR RequestCondition:Actor=ownerA * // allow Object.* * // allow Container.* * // @@ -138,7 +138,9 @@ func parseRuleLexemes(r *apechain.Rule, lexemes []string) error { return err } - var isObject *bool + var objectTargeted bool + var containerTargeted bool + for i, lexeme := range lexemes[1:] { anyExpr, anyErr := parseAnyAll(lexeme) if anyErr == nil { @@ -156,23 +158,30 @@ func parseRuleLexemes(r *apechain.Rule, lexemes []string) error { lexemes = lexemes[i+1:] break } - actionType = condition.Object == apechain.ObjectResource || condition.Object == apechain.ObjectRequest r.Condition = append(r.Condition, *condition) } else { + if actionType { + objectTargeted = true + } else { + containerTargeted = true + } + if objectTargeted && containerTargeted { + // Actually, APE chain allows to define rules for several resources, for example, if + // chain target is namespace, but the parser primitevly compiles verbs, + // conditions and resources in one rule. So, for the parser, one rule relates only to + // one resource type - object or container. + return errMixedTypesInRule + } + r.Actions.Names = append(r.Actions.Names, names...) } - if isObject == nil { - isObject = &actionType - } else if actionType != *isObject { - return errMixedTypesInRule - } } r.Actions.Names = unique(r.Actions.Names) if len(r.Actions.Names) == 0 { return fmt.Errorf("%w:%w", err, errNoActionsInRule) } for _, lexeme := range lexemes { - resource, errRes := parseResource(lexeme, *isObject) + resource, errRes := parseResource(lexeme, objectTargeted) if errRes != nil { return fmt.Errorf("%w:%w", err, errRes) } @@ -308,32 +317,27 @@ func parseResource(lexeme string, isObj bool) (string, error) { } const ( - ObjectResource = "object.resource" - ObjectRequest = "object.request" - - ContainerResource = "container.resource" - ContainerRequest = "container.request" + ResourceCondition = "resourcecondition" + RequestCondition = "requestcondition" ) -var typeToCondObject = map[string]apechain.ObjectType{ - ObjectResource: apechain.ObjectResource, - ObjectRequest: apechain.ObjectRequest, - ContainerResource: apechain.ContainerResource, - ContainerRequest: apechain.ContainerRequest, +var typeToCondKindType = map[string]apechain.ConditionKindType{ + ResourceCondition: apechain.KindResource, + RequestCondition: apechain.KindRequest, } func parseCondition(lexeme string) (*apechain.Condition, error) { typ, expression, found := strings.Cut(lexeme, ":") typ = strings.ToLower(typ) - objType, ok := typeToCondObject[typ] + condKindType, ok := typeToCondKindType[typ] if ok { if !found { return nil, fmt.Errorf("%w: %s", errInvalidConditionFormat, lexeme) } var cond apechain.Condition - cond.Object = objType + cond.Kind = condKindType lhs, rhs, binExpFound := strings.Cut(expression, "!=") if !binExpFound { diff --git a/cmd/frostfs-cli/modules/util/ape_test.go b/cmd/frostfs-cli/modules/util/ape_test.go index 3e4766098..d93210f41 100644 --- a/cmd/frostfs-cli/modules/util/ape_test.go +++ b/cmd/frostfs-cli/modules/util/ape_test.go @@ -109,46 +109,46 @@ func TestParseAPERule(t *testing.T) { }, { name: "Valid allow rule with conditions", - rule: "allow Object.Get Object.Resource:Department=HR Object.Request:Actor!=ownerA *", + rule: "allow Object.Get ResourceCondition:Department=HR RequestCondition:Actor!=ownerA *", expectRule: policyengine.Rule{ Status: policyengine.Allow, Actions: policyengine.Actions{Names: []string{nativeschema.MethodGetObject}}, Resources: policyengine.Resources{Names: []string{nativeschema.ResourceFormatAllObjects}}, Condition: []policyengine.Condition{ { - Op: policyengine.CondStringEquals, - Object: policyengine.ObjectResource, - Key: "Department", - Value: "HR", + Op: policyengine.CondStringEquals, + Kind: policyengine.KindResource, + Key: "Department", + Value: "HR", }, { - Op: policyengine.CondStringNotEquals, - Object: policyengine.ObjectRequest, - Key: "Actor", - Value: "ownerA", + Op: policyengine.CondStringNotEquals, + Kind: policyengine.KindRequest, + Key: "Actor", + Value: "ownerA", }, }, }, }, { name: "Valid rule for object with conditions with action detail", - rule: "deny:QuotaLimitReached Object.Get Object.Resource:Department=HR Object.Request:Actor!=ownerA *", + rule: "deny:QuotaLimitReached Object.Get ResourceCondition:Department=HR RequestCondition:Actor!=ownerA *", expectRule: policyengine.Rule{ Status: policyengine.QuotaLimitReached, Actions: policyengine.Actions{Names: []string{nativeschema.MethodGetObject}}, Resources: policyengine.Resources{Names: []string{nativeschema.ResourceFormatAllObjects}}, Condition: []policyengine.Condition{ { - Op: policyengine.CondStringEquals, - Object: policyengine.ObjectResource, - Key: "Department", - Value: "HR", + Op: policyengine.CondStringEquals, + Kind: policyengine.KindResource, + Key: "Department", + Value: "HR", }, { - Op: policyengine.CondStringNotEquals, - Object: policyengine.ObjectRequest, - Key: "Actor", - Value: "ownerA", + Op: policyengine.CondStringNotEquals, + Kind: policyengine.KindRequest, + Key: "Actor", + Value: "ownerA", }, }, }, @@ -170,12 +170,12 @@ func TestParseAPERule(t *testing.T) { }, { name: "Invalid rule with unknown condition binary operator", - rule: "deny Object.Put Object.Resource:Department
gk>3bMjIqrx&Q z4O2Z!v~!$7ybT@8!%MP^T%F3aBVCOGyu8cGO0y@+vnWjNWs?%aZrWxW_SK639ZVus diff --git a/internal/ape/converter.go b/internal/ape/converter.go index 2adc7645a..a0cc138d1 100644 --- a/internal/ape/converter.go +++ b/internal/ape/converter.go @@ -80,7 +80,7 @@ func appendTargetsOnly(source []apechain.Rule, st apechain.Status, act apechain. } for _, target := range targets { var roleCondition apechain.Condition - roleCondition.Object = apechain.ObjectRequest + roleCondition.Kind = apechain.KindRequest roleCondition.Key = nativeschema.PropertyKeyActorRole roleCondition.Value = target.Role().String() roleCondition.Op = apechain.CondStringEquals @@ -88,7 +88,7 @@ func appendTargetsOnly(source []apechain.Rule, st apechain.Status, act apechain. for _, binKey := range target.BinaryKeys() { var pubKeyCondition apechain.Condition - pubKeyCondition.Object = apechain.ObjectRequest + pubKeyCondition.Kind = apechain.KindRequest pubKeyCondition.Key = nativeschema.PropertyKeyActorPublicKey pubKeyCondition.Value = hex.EncodeToString(binKey) pubKeyCondition.Op = apechain.CondStringEquals @@ -112,7 +112,7 @@ func appendTargetsAndFilters(source []apechain.Rule, st apechain.Status, act ape Resources: res, } var roleCondition apechain.Condition - roleCondition.Object = apechain.ObjectRequest + roleCondition.Kind = apechain.KindRequest roleCondition.Key = nativeschema.PropertyKeyActorRole roleCondition.Value = target.Role().String() roleCondition.Op = apechain.CondStringEquals @@ -132,7 +132,7 @@ func appendTargetsAndFilters(source []apechain.Rule, st apechain.Status, act ape Resources: res, } var pubKeyCondition apechain.Condition - pubKeyCondition.Object = apechain.ObjectRequest + pubKeyCondition.Kind = apechain.KindRequest pubKeyCondition.Key = nativeschema.PropertyKeyActorPublicKey pubKeyCondition.Value = hex.EncodeToString(binKey) pubKeyCondition.Op = apechain.CondStringEquals @@ -155,10 +155,10 @@ func appendFilters(source []apechain.Condition, filters []eacl.Filter) ([]apecha var cond apechain.Condition var isObject bool if filter.From() == eacl.HeaderFromObject { - cond.Object = apechain.ObjectResource + cond.Kind = apechain.KindResource isObject = true } else if filter.From() == eacl.HeaderFromRequest { - cond.Object = apechain.ObjectRequest + cond.Kind = apechain.KindRequest } else { return nil, &ConvertEACLError{nested: fmt.Errorf("unknown filter from: %d", filter.From())} } diff --git a/pkg/services/container/ape_test.go b/pkg/services/container/ape_test.go index 94c3027c0..01a0fa4d2 100644 --- a/pkg/services/container/ape_test.go +++ b/pkg/services/container/ape_test.go @@ -228,10 +228,10 @@ func testDenyGetContainerForOthers(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: nativeschema.PropertyKeyActorRole, - Value: nativeschema.PropertyValueContainerRoleOthers, - Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: nativeschema.PropertyKeyActorRole, + Value: nativeschema.PropertyValueContainerRoleOthers, + Op: chain.CondStringEquals, }, }, }, @@ -328,10 +328,10 @@ func testDenyGetContainerByUserClaimTag(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: fmt.Sprintf(commonschema.PropertyKeyFormatFrostFSIDUserClaim, "tag-attr1"), - Value: "value100", - Op: chain.CondStringNotEquals, + Kind: chain.KindRequest, + Key: fmt.Sprintf(commonschema.PropertyKeyFormatFrostFSIDUserClaim, "tag-attr1"), + Value: "value100", + Op: chain.CondStringNotEquals, }, }, }, @@ -426,10 +426,10 @@ func testDenyGetContainerByGroupID(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: commonschema.PropertyKeyFrostFSIDGroupID, - Value: "19888", - Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: commonschema.PropertyKeyFrostFSIDGroupID, + Value: "19888", + Op: chain.CondStringEquals, }, }, }, @@ -500,10 +500,10 @@ func testDenySetContainerEACLForIR(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: nativeschema.PropertyKeyActorRole, - Value: nativeschema.PropertyValueContainerRoleIR, - Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: nativeschema.PropertyKeyActorRole, + Value: nativeschema.PropertyValueContainerRoleIR, + Op: chain.CondStringEquals, }, }, }, @@ -578,10 +578,10 @@ func testDenyGetContainerEACLForIRSessionToken(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: nativeschema.PropertyKeyActorRole, - Value: nativeschema.PropertyValueContainerRoleIR, - Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: nativeschema.PropertyKeyActorRole, + Value: nativeschema.PropertyValueContainerRoleIR, + Op: chain.CondStringEquals, }, }, }, @@ -657,10 +657,10 @@ func testDenyPutContainerForOthersSessionToken(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: nativeschema.PropertyKeyActorRole, - Value: nativeschema.PropertyValueContainerRoleOthers, - Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: nativeschema.PropertyKeyActorRole, + Value: nativeschema.PropertyValueContainerRoleOthers, + Op: chain.CondStringEquals, }, }, }, @@ -712,10 +712,10 @@ func testDenyPutContainerReadNamespaceFromFrostfsID(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: nativeschema.PropertyKeyActorRole, - Value: nativeschema.PropertyValueContainerRoleOthers, - Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: nativeschema.PropertyKeyActorRole, + Value: nativeschema.PropertyValueContainerRoleOthers, + Op: chain.CondStringEquals, }, }, }, @@ -796,10 +796,10 @@ func testDenyPutContainerInvalidNamespace(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: nativeschema.PropertyKeyActorRole, - Value: nativeschema.PropertyValueContainerRoleOthers, - Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: nativeschema.PropertyKeyActorRole, + Value: nativeschema.PropertyValueContainerRoleOthers, + Op: chain.CondStringEquals, }, }, }, @@ -879,10 +879,10 @@ func testDenyListContainersForPK(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: nativeschema.PropertyKeyActorPublicKey, - Value: hex.EncodeToString(pk.PublicKey().Bytes()), - Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: nativeschema.PropertyKeyActorPublicKey, + Value: hex.EncodeToString(pk.PublicKey().Bytes()), + Op: chain.CondStringEquals, }, }, }, @@ -993,10 +993,10 @@ func testDenyListContainersValidationNamespaceError(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: nativeschema.PropertyKeyActorPublicKey, - Value: actorPK.PublicKey().String(), - Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: nativeschema.PropertyKeyActorPublicKey, + Value: actorPK.PublicKey().String(), + Op: chain.CondStringEquals, }, }, }, @@ -1195,10 +1195,10 @@ func TestValidateContainerBoundedOperation(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: nativeschema.PropertyKeyActorRole, - Value: nativeschema.PropertyValueContainerRoleOthers, - Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: nativeschema.PropertyKeyActorRole, + Value: nativeschema.PropertyValueContainerRoleOthers, + Op: chain.CondStringEquals, }, }, }, @@ -1237,10 +1237,10 @@ func TestValidateContainerBoundedOperation(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: nativeschema.PropertyKeyActorRole, - Value: nativeschema.PropertyValueContainerRoleOthers, - Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: nativeschema.PropertyKeyActorRole, + Value: nativeschema.PropertyValueContainerRoleOthers, + Op: chain.CondStringEquals, }, }, }, @@ -1280,10 +1280,10 @@ func TestValidateContainerBoundedOperation(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: nativeschema.PropertyKeyActorRole, - Value: nativeschema.PropertyValueContainerRoleOthers, - Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: nativeschema.PropertyKeyActorRole, + Value: nativeschema.PropertyValueContainerRoleOthers, + Op: chain.CondStringEquals, }, }, }, @@ -1323,10 +1323,10 @@ func TestValidateContainerBoundedOperation(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: nativeschema.PropertyKeyActorRole, - Value: nativeschema.PropertyValueContainerRoleOthers, - Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: nativeschema.PropertyKeyActorRole, + Value: nativeschema.PropertyValueContainerRoleOthers, + Op: chain.CondStringEquals, }, }, }, @@ -1366,10 +1366,10 @@ func TestValidateContainerBoundedOperation(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: nativeschema.PropertyKeyActorRole, - Value: nativeschema.PropertyValueContainerRoleOthers, - Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: nativeschema.PropertyKeyActorRole, + Value: nativeschema.PropertyValueContainerRoleOthers, + Op: chain.CondStringEquals, }, }, }, @@ -1410,10 +1410,10 @@ func TestValidateContainerBoundedOperation(t *testing.T) { }, Condition: []chain.Condition{ { - Object: chain.ObjectRequest, - Key: nativeschema.PropertyKeyActorRole, - Value: nativeschema.PropertyValueContainerRoleOthers, - Op: chain.CondStringEquals, + Kind: chain.KindRequest, + Key: nativeschema.PropertyKeyActorRole, + Value: nativeschema.PropertyValueContainerRoleOthers, + Op: chain.CondStringEquals, }, }, }, diff --git a/pkg/services/object/ape/checker_test.go b/pkg/services/object/ape/checker_test.go index b5f064428..ddb336127 100644 --- a/pkg/services/object/ape/checker_test.go +++ b/pkg/services/object/ape/checker_test.go @@ -312,10 +312,10 @@ func TestAPECheck(t *testing.T) { Any: true, Condition: []chain.Condition{ { - Op: chain.CondStringLike, - Object: chain.ObjectResource, - Key: "attr1", - Value: "attribute*", + Op: chain.CondStringLike, + Kind: chain.KindResource, + Key: "attr1", + Value: "attribute*", }, }, }, @@ -351,10 +351,10 @@ func TestAPECheck(t *testing.T) { Any: true, Condition: []chain.Condition{ { - Op: chain.CondStringLike, - Object: chain.ObjectRequest, - Key: nativeschema.PropertyKeyActorPublicKey, - Value: senderKey, + Op: chain.CondStringLike, + Kind: chain.KindRequest, + Key: nativeschema.PropertyKeyActorPublicKey, + Value: senderKey, }, }, }, @@ -381,10 +381,10 @@ func TestAPECheck(t *testing.T) { Any: true, Condition: []chain.Condition{ { - Op: chain.CondStringEquals, - Object: chain.ObjectResource, - Key: nativeschema.PropertyKeyObjectPayloadLength, - Value: "1000", + Op: chain.CondStringEquals, + Kind: chain.KindResource, + Key: nativeschema.PropertyKeyObjectPayloadLength, + Value: "1000", }, }, }, @@ -503,10 +503,10 @@ func TestPutECChunk(t *testing.T) { Any: true, Condition: []chain.Condition{ { - Op: chain.CondStringEquals, - Object: chain.ObjectResource, - Key: "attr1", - Value: "value", + Op: chain.CondStringEquals, + Kind: chain.KindResource, + Key: "attr1", + Value: "value", }, }, },