Expand API behaves weirdly when no object is specified

zepatrik commented 3 years ago

Describe the bug

Noticed by @radekg in https://github.com/ory/keto/issues/598#issuecomment-842677636:

curl --silent 'http://localhost:4466/expand?namespace=default-namespace&subject=Fry&relation=member&max-depth=2' | jq '.'

returns

{
  "type": "union",
  "subject": "default-namespace:#member",
  "children": [
    {
      "type": "leaf",
      "subject": "Bender"
    },
    {
      "type": "leaf",
      "subject": "Fry"
    },
    {
      "type": "leaf",
      "subject": "Hermes"
    },
    {
      "type": "leaf",
      "subject": "default-namespace:it-director#member"
    },
    {
      "type": "leaf",
      "subject": "default-namespace:it-director#member"
    },
    {
      "type": "leaf",
      "subject": "default-namespace:dev-director#member"
    },
    {
      "type": "leaf",
      "subject": "default-namespace:it-director#member"
    }
  ]
}

this is completely not what the logic suggests the result should be.

Reproducing the bug

Steps to reproduce the behavior:

Setup from https://gruchalski.com/posts/2021-05-15-rbac-with-ory-keto/ and above query.

aravindarc commented 3 years ago

@zepatrik I am a little confused about this bug,

Expand api is for getting to know which subject has given relation over given object not the other way around (which object satisfies the given relation for given subject), why is there a subject included in the request, is this a mistake

What is the intended behaviour:

If there is no object should the intended behaviour be an error saying missing parameter

Or should the (children in) result contain all objects which have a member relation with children of their own and type as union: like this

An object

{
"type": "union",
"subject": "default-namespace:#member",
"children": [
{
  "type": "union",
  "subject": "default-namespace:production-creator:#member",
  "children": [...]
},
{
  "type": "union",
  "subject": "default-namespace:production-viewer#member",
  "children": [...]
},
{
  "type": "union",
  "subject": "default-namespace:it-director#member",
  "children": [...]
},
...
]
}

or a list

[
  {
    "type": "union",
    "subject": "default-namespace:production-creator:#member",
    "children": [...]
  },
  {
    "type": "union",
    "subject": "default-namespace:production-viewer#member",
    "children": [...]
  },
  {
    "type": "union",
    "subject": "default-namespace:it-director#member",
    "children": [...]
  },
  ...
]

zepatrik commented 3 years ago

If there is no object should the intended behaviour be an error saying missing parameter

This, or rather it should use the empty string as the object. Also, you are right that specifying a subject is not very useful on the expand API, so it should also result in a 400 Bad Request.

aravindarc commented 3 years ago

Firstly, the api documentation states that namespace, object and relation are required parameters.

Even if object can be omitted, I think the current response makes perfect sense, it is like querying with a ✱ regex

Response with all parameters mentioned: namespace=default-namespace&object=dev-director&relation=member

{
"type": "union",
"subject": "default-namespace:dev-director#member",
"children": [
    {
        "type": "leaf",
        "subject": "Bender"
    },
    {
        "type": "leaf",
        "subject": "Fry"
    }
]
}

Response with relation parameter omitted: namespace=default-namespace&object=dev-director default-namespace:dev-director#✱ will match only one tuple default-namespace:dev-director#{member}

{
"type": "union",
"subject": "default-namespace:dev-director#",
"children": [
    {
        "type": "leaf",
        "subject": "Bender"
    },
    {
        "type": "leaf",
        "subject": "Fry"
    }
]
}

Response with object parameter omitted: namespace=default-namespace&relation=member default-namespace:✱#member will match many tuples default-namespace:{dev-director, it-director, production-creator, production-viewer, ...}#member
```
{
"type": "union",
"subject": "default-namespace:#member",
"children": [
    {
        "type": "leaf",
        "subject": "Bender"
    },
    {
        "type": "leaf",
        "subject": "Fry"
    },
    ...
}
```
Similarly if we omit both object and relation parameters, the api responds in a similar fashion.

The children are the union of all subjects of all matching tuples

zepatrik commented 3 years ago

Well, your cases are valid, but I think they are better covered by the List API. Expand is really about expanding a subject set (namespace-relation-object). I would like for probable performance issues with those wildcards further down the road to not support that in expand. List already can query for partial tuples, so all your cases can be expressed there.

aravindarc commented 3 years ago

Yeah, I totally agree, then we have to enforce the required variables(namespace, object and relation). The api should return 400 if any one of the required variables are missing right?

zepatrik commented 3 years ago

Yes exactly.

kmherrmann commented 1 year ago

Closed due to inactivity. Please re-open if still relevant

ory / keto