oso icon indicating copy to clipboard operation
oso copied to clipboard

sqlalchemy-oso cannot handle or clauses involving joins

Open snstanton opened this issue 2 years ago • 1 comments

Environment

oso 0.26.1 sqlalchemy-oso 0.26.1 Python 3.10

Setup

Given these models:

class User(db.Model):
  id = Column(BigInteger, primary_key=True)

class Group(db.Model):
  id = Column(BigInteger, primary_key=True)
  type = Column(String)
  members = relationship("User", secondary="Membership", viewonly=True)

class Membership(db.Model):
  user_id=Column(ForeignKey(User.id), primary_key=True)
  group_id=Column(ForeignKey(Group.id), primary_key=True)

And a policy like:

has_role(user: User, "member", group: Group) if user in group.members;
allow(user: User, "read", group: Group) if group.type = "PUBLIC" or has_role(user, "member", group);

Problem

For a query like:

Group.query.all()

The generated query is incorrectly using an inner join instead of a left join:

SELECT ...
FROM group, membership AS membership_1
WHERE group.type = %s OR group.id = membership_1.group_id AND %s = membership_1.user_id

This results in the query skipping public groups that the user is not a member of.

Partial workaround

I have rewritten my model to work around this partially (poor scalability) by changing the policy to look like:

has_role(user: User, "member", group: Group) if group.id in user.get_group_ids();

and adding the following method to the User model:

def get_group_ids(self):
        return [i.group_id for i in Member.query.filter_b(user_id=self.id)]

This results in a query like:

SELECT ...
FROM group
WHERE group.type = %s OR group.id = %s OR group.id = %s ...

This works, but has scalability issues if the group list is too large.

snstanton avatar Jul 01 '22 23:07 snstanton

I'd recommend adding the relationships and configure it for eager loading. It works well with those situations for me, except in cases where you have three or four layers of nested joins. It doesn't modify the classes though to eagerly load policy data if you aren't already doing it

kkirsche avatar Jul 19 '22 22:07 kkirsche