[#60] chain: Support numeric conditions #60
|
@ -10,6 +10,7 @@ import (
|
|||
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/schema/common"
|
||||
"github.com/nspcc-dev/neo-go/pkg/encoding/fixedn"
|
||||
)
|
||||
|
||||
const (
|
||||
|
@ -233,8 +234,7 @@ func getConditionTypeAndConverter(op string) (chain.ConditionType, convertFuncti
|
|||
return 0, nil, fmt.Errorf("unsupported condition operator: '%s'", op)
|
||||
}
|
||||
case strings.HasPrefix(op, "Numeric"):
|
||||
// TODO
|
||||
return 0, nil, fmt.Errorf("currently nummeric conditions unsupported: '%s'", op)
|
||||
return numericConditionTypeAndConverter(op)
|
||||
case strings.HasPrefix(op, "Date"):
|
||||
switch op {
|
||||
case CondDateEquals:
|
||||
|
@ -269,12 +269,39 @@ func getConditionTypeAndConverter(op string) (chain.ConditionType, convertFuncti
|
|||
}
|
||||
}
|
||||
|
||||
func numericConditionTypeAndConverter(op string) (chain.ConditionType, convertFunction, error) {
|
||||
switch op {
|
||||
case CondNumericEquals:
|
||||
return chain.CondNumericEquals, numericConvertFunction, nil
|
||||
case CondNumericNotEquals:
|
||||
return chain.CondNumericNotEquals, numericConvertFunction, nil
|
||||
case CondNumericLessThan:
|
||||
return chain.CondNumericLessThan, numericConvertFunction, nil
|
||||
case CondNumericLessThanEquals:
|
||||
return chain.CondNumericLessThanEquals, numericConvertFunction, nil
|
||||
case CondNumericGreaterThan:
|
||||
return chain.CondNumericGreaterThan, numericConvertFunction, nil
|
||||
case CondNumericGreaterThanEquals:
|
||||
return chain.CondNumericGreaterThanEquals, numericConvertFunction, nil
|
||||
default:
|
||||
return 0, nil, fmt.Errorf("unsupported condition operator: '%s'", op)
|
||||
}
|
||||
}
|
||||
|
||||
type convertFunction func(string) (string, error)
|
||||
|
||||
func noConvertFunction(val string) (string, error) {
|
||||
return val, nil
|
||||
}
|
||||
|
||||
func numericConvertFunction(val string) (string, error) {
|
||||
|
||||
if _, err := fixedn.Fixed8FromString(val); err == nil {
|
||||
return val, nil
|
||||
}
|
||||
|
||||
return "", fmt.Errorf("invalid numeric value: '%s'", val)
|
||||
}
|
||||
|
||||
func dateConvertFunction(val string) (string, error) {
|
||||
if _, err := strconv.ParseInt(val, 10, 64); err == nil {
|
||||
return val, nil
|
||||
|
|
|
@ -385,6 +385,12 @@ func TestConvertToChainCondition(t *testing.T) {
|
|||
CondArnLike: {condKeyAWSPrincipalARN: {principal}},
|
||||
CondArnNotEquals: {"key18": {"val18"}},
|
||||
CondArnNotLike: {"key19": {"val19"}},
|
||||
CondNumericEquals: {"key20": {"-20"}},
|
||||
dstepanov-yadro
commented
negative, zero, hex, binary values tests should be added. negative, zero, hex, binary values tests should be added.
also values with spaces, with '+' sign.
dkirillov
commented
AWS supports only integer and decimals. So here (in AWS supports only integer and decimals. So here (in `converter_test.go`) I wouldn't add hex/binary values.
If this is required we can support such values in native condition types and add tests there.
dstepanov-yadro
commented
Tests should not be only positive. I was expecting to see a negative binary and hex test. Also we need to support decimals) Tests should not be only positive. I was expecting to see a negative binary and hex test. Also we need to support decimals)
|
||||
CondNumericNotEquals: {"key21": {"+21"}},
|
||||
CondNumericLessThan: {"key22": {"0"}},
|
||||
CondNumericLessThanEquals: {"key23": {"23.23"}},
|
||||
CondNumericGreaterThan: {"key24": {"-24.24"}},
|
||||
CondNumericGreaterThanEquals: {"key25": {"+25.25"}},
|
||||
}
|
||||
|
||||
expectedCondition := []GroupedConditions{
|
||||
|
@ -549,11 +555,76 @@ func TestConvertToChainCondition(t *testing.T) {
|
|||
Value: "val19",
|
||||
}},
|
||||
},
|
||||
{
|
||||
Conditions: []chain.Condition{{
|
||||
Op: chain.CondNumericEquals,
|
||||
Object: chain.ObjectRequest,
|
||||
Key: "key20",
|
||||
Value: "-20",
|
||||
}},
|
||||
},
|
||||
{
|
||||
Conditions: []chain.Condition{{
|
||||
Op: chain.CondNumericNotEquals,
|
||||
Object: chain.ObjectRequest,
|
||||
Key: "key21",
|
||||
Value: "+21",
|
||||
}},
|
||||
},
|
||||
{
|
||||
Conditions: []chain.Condition{{
|
||||
Op: chain.CondNumericLessThan,
|
||||
Object: chain.ObjectRequest,
|
||||
Key: "key22",
|
||||
Value: "0",
|
||||
}},
|
||||
},
|
||||
{
|
||||
Conditions: []chain.Condition{{
|
||||
Op: chain.CondNumericLessThanEquals,
|
||||
Object: chain.ObjectRequest,
|
||||
Key: "key23",
|
||||
Value: "23.23",
|
||||
}},
|
||||
},
|
||||
{
|
||||
Conditions: []chain.Condition{{
|
||||
Op: chain.CondNumericGreaterThan,
|
||||
Object: chain.ObjectRequest,
|
||||
Key: "key24",
|
||||
Value: "-24.24",
|
||||
}},
|
||||
},
|
||||
{
|
||||
Conditions: []chain.Condition{{
|
||||
Op: chain.CondNumericGreaterThanEquals,
|
||||
Object: chain.ObjectRequest,
|
||||
Key: "key25",
|
||||
Value: "+25.25",
|
||||
}},
|
||||
},
|
||||
}
|
||||
|
||||
actualCondition, err := convertToChainCondition(conditions)
|
||||
require.NoError(t, err)
|
||||
require.ElementsMatch(t, expectedCondition, actualCondition)
|
||||
|
||||
invalidConditions := []Condition{
|
||||
{"key1": {"invalid"}},
|
||||
{"key2": {"1 2"}},
|
||||
{"key3": {"0x12f"}},
|
||||
{"key4": {"0b1010"}},
|
||||
{"key5": {"+Inf"}},
|
||||
{"key6": {"-Inf"}},
|
||||
{"key7": {"inf"}},
|
||||
{"key8": {"NaN"}},
|
||||
{"key9": {"nan"}},
|
||||
}
|
||||
|
||||
for _, cond := range invalidConditions {
|
||||
_, err = convertToChainCondition(Conditions{CondNumericEquals: cond})
|
||||
require.Error(t, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestParsePrincipalARN(t *testing.T) {
|
||||
|
|
|
@ -6,6 +6,7 @@ import (
|
|||
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/resource"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/util"
|
||||
"github.com/nspcc-dev/neo-go/pkg/encoding/fixedn"
|
||||
"golang.org/x/exp/slices"
|
||||
)
|
||||
|
||||
|
@ -186,6 +187,38 @@ func (c *Condition) Match(req resource.Request) bool {
|
|||
return val >= c.Value
|
||||
case CondSliceContains:
|
||||
return slices.Contains(strings.Split(val, condSliceContainsDelimiter), c.Value)
|
||||
case CondNumericEquals, CondNumericNotEquals, CondNumericLessThan, CondNumericLessThanEquals, CondNumericGreaterThan,
|
||||
CondNumericGreaterThanEquals:
|
||||
return c.matchNumeric(val)
|
||||
}
|
||||
}
|
||||
|
||||
func (c *Condition) matchNumeric(val string) bool {
|
||||
valDecimal, err := fixedn.Fixed8FromString(val)
|
||||
dkirillov
commented
Probably we should try parse float if parsing integer failed because spec says Probably we should try parse float if parsing integer failed because [spec](https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_condition_operators.html#Conditions_Numeric) says `Numeric condition operators let you construct Condition elements that restrict access based on comparing a key to an integer or decimal value.`
mbiryukova
commented
Can we parse only float in this case? Can we parse only float in this case?
dkirillov
commented
No. In general we cannot do this because equals for float can have some error:
Or we have to write custom equal function and not use No. In general we cannot do this because equals for float can have some error:
```golang
tmp := 0.1
tmp2 := 0.2
fmt.Println(tmp+tmp2 == 0.3) // false
```
Or we have to write custom equal function and not use `==`
fyrchik
commented
What is the length of a decimal? What is the length of a decimal?
neo-go has a fixed decimal package https://github.com/nspcc-dev/neo-go/blob/master/pkg/encoding/fixedn/decimal.go (and we already depend on neo-go)
fyrchik
commented
Do we have any compatibility tests here? The description at the link you have provided is vague. Do we have any compatibility tests here? The description at the link you have provided is vague.
dkirillov
commented
I couldn't find more precise description of this conditions. So we decided to use I couldn't find more precise description of this conditions. So we decided to use `1e-8` precision. We can use decimal from neo-go I think
|
||||
if err != nil {
|
||||
return c.Op == CondNumericNotEquals
|
||||
}
|
||||
|
||||
condVal, err := fixedn.Fixed8FromString(c.Value)
|
||||
if err != nil {
|
||||
return c.Op == CondNumericNotEquals
|
||||
}
|
||||
|
||||
switch c.Op {
|
||||
default:
|
||||
panic(fmt.Sprintf("unimplemented: %d", c.Op))
|
||||
case CondNumericEquals:
|
||||
return valDecimal.Equal(condVal)
|
||||
case CondNumericNotEquals:
|
||||
return !valDecimal.Equal(condVal)
|
||||
case CondNumericLessThan:
|
||||
return valDecimal.LessThan(condVal)
|
||||
case CondNumericLessThanEquals:
|
||||
return valDecimal.LessThan(condVal) || valDecimal.Equal(condVal)
|
||||
case CondNumericGreaterThan:
|
||||
aarifullin marked this conversation as resolved
Outdated
aarifullin
commented
I suppose the correct way is to compare them like this
I suppose the correct way is to compare them like this
```go
valFloat < condVal || equalFloat(valFloat, condVal)
```
|
||||
return valDecimal.GreaterThan(condVal)
|
||||
case CondNumericGreaterThanEquals:
|
||||
return valDecimal.GreaterThan(condVal) || valDecimal.Equal(condVal)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -6,6 +6,7 @@ import (
|
|||
"git.frostfs.info/TrueCloudLab/policy-engine/pkg/resource/testutil"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/schema/common"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/schema/native"
|
||||
"git.frostfs.info/TrueCloudLab/policy-engine/schema/s3"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
|
@ -149,3 +150,138 @@ func TestCondSliceContainsMatch(t *testing.T) {
|
|||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestNumericConditionsMatch(t *testing.T) {
|
||||
propKey := s3.PropertyKeyMaxKeys
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
conditions []Condition
|
||||
value string
|
||||
status Status
|
||||
}{
|
||||
{
|
||||
name: "value from interval",
|
||||
conditions: []Condition{
|
||||
{
|
||||
Op: CondNumericLessThan,
|
||||
Object: ObjectRequest,
|
||||
Key: propKey,
|
||||
Value: "100",
|
||||
},
|
||||
{
|
||||
Op: CondNumericGreaterThan,
|
||||
Object: ObjectRequest,
|
||||
Key: propKey,
|
||||
Value: "80",
|
||||
},
|
||||
{
|
||||
Op: CondNumericNotEquals,
|
||||
Object: ObjectRequest,
|
||||
Key: propKey,
|
||||
Value: "91",
|
||||
},
|
||||
},
|
||||
value: "90",
|
||||
status: Allow,
|
||||
},
|
||||
{
|
||||
name: "border value",
|
||||
conditions: []Condition{
|
||||
{
|
||||
Op: CondNumericEquals,
|
||||
Object: ObjectRequest,
|
||||
Key: propKey,
|
||||
Value: "50",
|
||||
},
|
||||
{
|
||||
Op: CondNumericLessThanEquals,
|
||||
Object: ObjectRequest,
|
||||
Key: propKey,
|
||||
Value: "50",
|
||||
},
|
||||
{
|
||||
Op: CondNumericGreaterThanEquals,
|
||||
Object: ObjectRequest,
|
||||
Key: propKey,
|
||||
Value: "50",
|
||||
},
|
||||
},
|
||||
value: "50",
|
||||
status: Allow,
|
||||
},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
resource := testutil.NewResource(native.ResourceFormatRootContainers, nil)
|
||||
request := testutil.NewRequest(native.MethodPutObject, resource, map[string]string{propKey: tc.value})
|
||||
|
||||
ch := Chain{Rules: []Rule{{
|
||||
Status: Allow,
|
||||
Actions: Actions{Names: []string{native.MethodPutObject}},
|
||||
Resources: Resources{Names: []string{native.ResourceFormatRootContainers}},
|
||||
Condition: tc.conditions,
|
||||
}}}
|
||||
st, _ := ch.Match(request)
|
||||
require.Equal(t, tc.status.String(), st.String())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestInvalidNumericValues(t *testing.T) {
|
||||
propKey := s3.PropertyKeyMaxKeys
|
||||
propValues := []string{"", "invalid"}
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
conditionType ConditionType
|
||||
match bool
|
||||
}{
|
||||
{
|
||||
name: "NumericEquals condition",
|
||||
conditionType: CondNumericEquals,
|
||||
match: false,
|
||||
},
|
||||
{
|
||||
name: "NumericNotEquals condition",
|
||||
conditionType: CondNumericNotEquals,
|
||||
match: true,
|
||||
},
|
||||
{
|
||||
name: "NumericLessThan condition",
|
||||
conditionType: CondNumericLessThan,
|
||||
match: false,
|
||||
},
|
||||
{
|
||||
name: "NumericLessThanEquals condition",
|
||||
conditionType: CondNumericLessThanEquals,
|
||||
match: false,
|
||||
},
|
||||
{
|
||||
name: "NumericGreaterThan condition",
|
||||
conditionType: CondNumericGreaterThan,
|
||||
match: false,
|
||||
},
|
||||
{
|
||||
name: "NumericGreaterThanEquals condition",
|
||||
conditionType: CondNumericGreaterThanEquals,
|
||||
match: false,
|
||||
},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
resource := testutil.NewResource(native.ResourceFormatRootContainers, nil)
|
||||
condition := Condition{
|
||||
Op: tc.conditionType,
|
||||
Object: ObjectRequest,
|
||||
Key: propKey,
|
||||
Value: "50",
|
||||
}
|
||||
|
||||
for _, propValue := range propValues {
|
||||
request := testutil.NewRequest(native.MethodPutObject, resource, map[string]string{propKey: propValue})
|
||||
|
||||
match := condition.Match(request)
|
||||
require.Equal(t, tc.match, match)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
@ -6,6 +6,7 @@ const (
|
|||
PropertyKeyDelimiter = "s3:delimiter"
|
||||
PropertyKeyPrefix = "s3:prefix"
|
||||
PropertyKeyVersionID = "s3:VersionId"
|
||||
PropertyKeyMaxKeys = "s3:max-keys"
|
||||
|
||||
ResourceFormatS3All = "arn:aws:s3:::*"
|
||||
ResourceFormatS3Bucket = "arn:aws:s3:::%s"
|
||||
|
|
+Inf
,inf
,NaN
,nan
are valid float strings, see https://pkg.go.dev/strconv#example-ParseFloatDoes IAM supports it?
No. But now we use Fixed8 and it doesn't support it.
Probably we should include this
+Inf, inf, NaN, nan
in test cases