TrueCloudLab/policy-engine

Fork 11

generated from TrueCloudLab/basic

Support group conditions #33

New issue

Closed

opened 2023-12-14 12:15:58 +00:00 by dkirillov · 5 comments

dkirillov commented

2023-12-14 12:15:58 +00:00

Member

In IAM service it's required to be able set rules that match request if its owner belongs to specific group in frostfsid (note that subject/user can belong to several groups).

We have several options as I can see:

Introduce new condition type (e.g. CondStringContains or more group specific name). And add the following case here:

	case CondStringContains:
		return strings.Contains(val, c.Value)

so we can form rule condition:

{
  "Op":"CondStringContains",
  "Object":"ObjectRequest",
  "Key":"groupIDs",
  "Value":"1"
}

and some request property groupIDs: 1,2,3.

Introduce new condition type ( CondContains ), change Request so that return several values. Then we can use the following case:

	case CondContains:
		return slices.Contains(val, c.Value)

so we can form rule condition:

{
  "Op":"CondContains",
  "Object":"ObjectRequest",
  "Key":"groupIDs",
  "Value":"1"
}

and some request property groupIDs: [1,2,3].

Use CondStringLike condition but support more complex wildcard using in glob. Then we can have rule condition:

{
  "Op":"CondStringLike",
  "Object":"ObjectRequest",
  "Key":"groupIDs",
  "Value":"*1*"
}

and form request property as groupIDs: 1,2,3.

Probably we can have some other options

In IAM service it's required to be able set rules that match request if its owner belongs to specific group in [frostfsid](https://git.frostfs.info/TrueCloudLab/frostfs-contract/src/commit/4dcb575caa29aff5b33d3c2e294fdb32e3bafb4c/frostfsid/frostfsid_contract.go) (note that subject/user can belong to several groups). We have several options as I can see: * Introduce new [condition type](https://git.frostfs.info/TrueCloudLab/policy-engine/src/commit/1d07331f5df5a143602062fcb4fdd74ff295d373/pkg/chain/chain.go#L81) (e.g. `CondStringContains` or more group specific name). And add the following case [here](https://git.frostfs.info/TrueCloudLab/policy-engine/src/commit/1d07331f5df5a143602062fcb4fdd74ff295d373/pkg/chain/chain.go#L158): ```golang case CondStringContains: return strings.Contains(val, c.Value) ``` so we can form rule condition: ```json { "Op":"CondStringContains", "Object":"ObjectRequest", "Key":"groupIDs", "Value":"1" } ``` and some request property `groupIDs: 1,2,3`. * Introduce new condition type ( `CondContains` ), change [Request](https://git.frostfs.info/TrueCloudLab/policy-engine/src/commit/1d07331f5df5a143602062fcb4fdd74ff295d373/pkg/resource/resource.go#L10) so that return several values. Then we can use the following case: ```golang case CondContains: return slices.Contains(val, c.Value) ``` so we can form rule condition: ```json { "Op":"CondContains", "Object":"ObjectRequest", "Key":"groupIDs", "Value":"1" } ``` and some request property `groupIDs: [1,2,3]`. * Use `CondStringLike` condition but support more complex wildcard using in [glob](https://git.frostfs.info/TrueCloudLab/policy-engine/src/commit/1d07331f5df5a143602062fcb4fdd74ff295d373/util/glob.go#L12). Then we can have rule condition: ```json { "Op":"CondStringLike", "Object":"ObjectRequest", "Key":"groupIDs", "Value":"*1*" } ``` and form request property as `groupIDs: 1,2,3`. Probably we can have some other options

alexvanin commented

2023-12-14 14:55:29 +00:00

Owner

Let's resolve it before the end of the year.

First option is cheap and straightforward, ~~but requires to define property delimiter to split single string to multiple values, which may be tricky. I think comma is okay if we support character escaping (now or later).~~

Edit 1: First option is more about finding sub-string in a value string which is also fair solution.

Second option gives us more unintuitive cases, e.g. operation is CondStringEquals and len(val)==2 for some reason. Should we compare both values in val or only the first? I think such condition is worse than having hardcoded delimiter in a string.

/cc @fyrchik @dstepanov-yadro @aarifullin

Let's resolve it before the end of the year. First option is cheap and straightforward, ~~but requires to define property delimiter to split single string to multiple values, which may be tricky. I think comma is okay if we support character escaping (now or later).~~ Edit 1: First option is more about finding sub-string in a value string which is also fair solution. Second option gives us more unintuitive cases, e.g. operation is `CondStringEquals` and `len(val)==2` for some reason. Should we compare both values in `val` or only the first? I think such condition is worse than having hardcoded delimiter in a string. /cc @fyrchik @dstepanov-yadro @aarifullin

fyrchik commented

2023-12-18 07:58:19 +00:00

Owner

I like the first and the last options.
The first looks like the cheapest option (and can be reused for other things)
Regarging the last one: don't we need to eventually implement it anyway because that's is what aws support (and thus what IAM converter needs)?

Don't like the second one: returning any or multiple values seems an overkill.

I like the first and the last options. The first looks like the cheapest option (and can be reused for other things) Regarging the last one: don't we need to eventually implement it anyway because that's is what aws support (and thus what IAM converter needs)? Don't like the second one: returning `any` or multiple values seems an overkill.

dkirillov commented

2023-12-18 08:25:14 +00:00

Author

Member

Regarding the first and last option: as @alexvanin noticed using strings.Contains or regexp *1* will match groupIDs: 11,12 that seems incorrect.
So probably we should transform the first option:

name condition CondSlicesContains
use switch case

	case CondSlicesContains:
		return slices.Contains(strings.Split(val, ","), c.Value)

Regarding the first and last option: as @alexvanin noticed using `strings.Contains` or regexp `*1*` will match `groupIDs: 11,12` that seems incorrect. So probably we should transform the first option: * name condition `CondSlicesContains` * use switch case ```golang case CondSlicesContains: return slices.Contains(strings.Split(val, ","), c.Value) ```

fyrchik commented

2023-12-18 08:54:48 +00:00

Owner

The exact implementation is up to debate, we could even use \x00 as a separator -- this way strings with , are supported too.
And for regexp it is possible to use (^|,)...($|,)

The exact implementation is up to debate, we could even use `\x00` as a separator -- this way strings with `,` are supported too. And for regexp it is possible to use `(^|,)...($|,)`

alexvanin commented

2023-12-18 10:32:57 +00:00

Owner

Let's go for \x00 separator and CondSliceContains condition, unless there are strong opinions against it.

Let's go for `\x00` separator and `CondSliceContains` condition, unless there are strong opinions against it.

👍 1