ory / keto

Open Source (Go) implementation of "Zanzibar: Google's Consistent, Global Authorization System". Ships gRPC, REST APIs, newSQL, and an easy and granular permission language. Supports ACL, RBAC, and other access models.
https://www.ory.sh/?utm_source=github&utm_medium=banner&utm_campaign=keto
Apache License 2.0
4.78k stars 347 forks source link

User incorrectly granted access due to an invalid relationship #1452

Open viktorgt opened 11 months ago

viktorgt commented 11 months ago

Preflight checklist

Ory Network Project

https://practical-nash-7o50l8l3hg.projects.oryapis.com

Describe the bug

Using the provided OPL:

class User implements Namespace { }

class Shop implements Namespace {
  related: {
    viewers: User[]
  }

  permits = {
    view: (ctx: Context): boolean =>
      this.related.viewers.includes(ctx.subject)
  }
}

class Organization implements Namespace {
  related: {
    viewers: User[]
    parents: Organization[]
  }

  permits = {
    view: (ctx: Context): boolean =>
      this.related.viewers.includes(ctx.subject) ||
      this.related.parents.traverse((p) => p.permits.view(ctx))
  }
}

With the provided relations:

[
  {
    "namespace": "Shop",
    "object": "shop1",
    "relation": "viewers",
    "subject_id": "john"
  },
  {
    "namespace": "Organization",
    "object": "orga1",
    "relation": "parents",
    "subject_set": {
      "namespace": "Shop",
      "object": "shop1",
      "relation": ""
    }
  }
]

The user "john" is permitted to view the Organization "orga1". However, based on the provided OPL, this is incorrect since:

Expected Behavior:

The check should return false as the user "john".

Actual Behavior:

The check returns true, incorrectly granting "john" the permission to view the Organization "orga1".

Reproducing the bug

  1. Configure OPL and create relations.
  2. Check if the user "john" has permission to view the Organization "orga1".

Relevant log output

No response

Relevant configuration

No response

Version

ory network

On which operating system are you observing this issue?

None

In which environment are you deploying?

Ory Network

Additional Context

Question: Is it allowed to create invalid relations that are against the OPL?

cmmoran commented 8 months ago

Ah. You have hit the heart of the challenge that the ory:keto creators faced when they decided to create the OPL. The OPL parser is not a compiler. It's a loose transpiler at best. To be completely unfair, calling it a converter would not be totally wrong either.

The point is: while the OPL looks like typescript, it is most definitely NOT true typescript. It makes sense though. What OPL really does is it gives developers the ability to define keto's configuration using a language that we're familiar with. While our IDE's give us nice typechecking, when the permission model is built by keto, the namespace models are not fully typechecked and there are ways (as you have discovered) to "fool" the typechecking that is being done.

As a case-in-point, during a traverse, the parser determines the types for the relation being traversed (which in your example is: parents). And it will discover that type to be Organization. A check is performed to ensure that parents in-fact exists on the current namespace (Organization) and within the traversal the relation view is checked to ensure that it exists in All of the relevant types (see above) associated with the parents relation. That's it. No check is made to ensure that the thing is, in-fact of type X or Y. From keto's perspective of a subject, it's irrelevant.

I say that but you hit on a weakness of OPL as it stands today. Right now the types associated with related fields is, in the words of Hector Barbossa (youtube link): "more whatcha call guidelines than actual rules". What that means is this: the type only matters when it comes down to either 1. traversing that relation or 2. resolving a permits on that relation. Keto's parser will check that the type associated with that relation (for example parents) has the associated relation to be traversed (for example: viewers) or the associated permits (for example: view). In this case, unfortunately (and rightly confusingly) Shop does, in fact have a relation (a dynamic one under permits) called view. But so does Organization. What this means is that, when rules are checked, if a relation is created that associates a Shop where only an Organization is expected, it will work not as you intended but it will work as it was designed because both namespaces have the relation named view, so the typechecks will pass.