Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get all roles from resource #1427

Open
jneo8 opened this issue Dec 2, 2021 · 13 comments · May be fixed by #1675
Open

Get all roles from resource #1427

jneo8 opened this issue Dec 2, 2021 · 13 comments · May be fixed by #1675

Comments

@jneo8
Copy link

jneo8 commented Dec 2, 2021

Is there any function like authorized_actions but list all roles instead of actions?

authorized_roles(actor, resource)

@samscott89
Copy link
Member

Hey @jneo8! That's a great idea, we can definitely consider adding that.

In the meantime, it's actually not too bad to implement that yourself. Here's how authorized_actions works:

results = self.query_rule("allow", actor, Variable("action"), resource)

So it would be something like:

from oso import Variable

role_results = oso.query_rule("has_role", actor, Variable("role"), resource)
roles = set()
for result in role_results:
    role = result.get("bindings").get("role")
    roles.add(role)

This wouldn't handle the "wildcard" case (the user is allowed to have any role). But that's not commonly needed.

@jneo8
Copy link
Author

jneo8 commented Dec 3, 2021

@samscott89 Thanks for your reply.

But I get this error msg when I try to implement

polar.exceptions.UnexpectedPolarTypeError: Received Expression from Polar VM.
The Expression type is only supported when using django-oso or sqlalchemy-oso's data filtering features.
Did you perform an operation over an unbound variable in your policy?

To silence this error and receive an Expression result, pass accept_expression=True to Oso.query.

Get help with Oso from our engineers: https://help.osohq.com/error/UnexpectedPolarTypeError

Because I'm not using either django-oso or sqlalchemy-oso, I add accept_expression=True to my code and the return value type sometimes become polar.expression.Expression, but what I want is str type
Here is my workaround:

class Roles(enum.Enum):
    manager = "manager"
    member = "member"
    ...
 
def get_actor_roles(oso: Oso, actor, resource) -> Set[Union[Roles]]:
    roles = set()
    for result in oso.query_rule(
        "has_role",
        actor,
        Variable("role"),
        resource,
        accept_expression=True,
    ):
        role = result.get("bindings").get("role")
        if isinstance(role, str):    # Make sure is return type is string 
            if role in Roles._value2member_map_: 
                roles.add(Roles(role))
    return roles

@samscott89
Copy link
Member

Hey @jneo8. Glad you have a workaround, but to make sure it's correct would you mind sharing the policy that caused this?

@jneo8
Copy link
Author

jneo8 commented Jan 27, 2022

@samscott89 thanks for you reply.
I can only share basic policy from our project.
We are using async SQLAlchemy && Ltree(PostgreSQL) together

allow (actor, action, resource) if
    has_permission(actor, action, resource);


resource Org {
    permissions = [
        "read",
        "edit",
        "invite",
    ];
    roles = [
        # Role can inherit
        "superuser",
        "agent",
        "manager",

        "member"
    ];

    relations = {
        parent: Org,
    };

    # superuser
    "agent" if "superuser";

    # agent
    "manager" if "agent";

    # manager
    "member" if "manager";
    "invite" if "manager";

    # member
    "read" if "member";

}

has_relation(parent_org: Org, "parent", child_org: Org) if
    parent_org = child_org.parent;


actor User {
}

# Superuser rule
has_role(user: User, "superuser", _org: Org) if
    user.is_superuser = true;

has_role(user: User, role_name: String, org: Org) if
    org_role in user.org_roles and
    org_role.org.id = org.id and
    org_role.role.value = role_name;
OrgIDSeq = Sequence("org_id_seq")

class Org(Base):
    """Org."""

    __tablename__ = "org"

    id: Any = Column(Integer, OrgIDSeq, primary_key=True)
    name = Column(String, nullable=False)
    path = Column(LtreeType, nullable=False)
    is_root = Column(Boolean, default=False)

    types = Column(
        ARRAY(
            ChoiceType(OrgType, impl=String()),
            as_tuple=True,
        ),
    )

    # relationship
    parent = relationship(
        "Org",
        primaryjoin=remote(path) == foreign(func.subpath(path, 0, -1)),
        backref="children",
        viewonly=True,
        lazy="immediate",
    )
    org_roles = relationship(
        "OrgRole",
        back_populates="org",
    )

    __table_args__ = (Index("ix_org_path", path, postgresql_using="gist"),)

UserOrgRole = Table(
    "user_org_role",
    Base.metadata,
    Column("user_id", Integer, ForeignKey("user.id"), primary_key=True),
    Column(
        "org_role_id", Integer, ForeignKey("org_role.id"), primary_key=True
    ),
)


class User(Base):

    __tablename__ = "user"

    id = Column(Integer, primary_key=True)
    name = Column(String)
    email = Column(EmailType)
    password = Column(String(length=128), nullable=False)
    is_active = Column(Boolean, default=False, nullable=False)
    is_verified = Column(Boolean, default=False, nullable=False)
    is_superuser = Column(Boolean, default=False, nullable=False)
    org_roles = relationship(
        "OrgRole",
        secondary=UserOrgRole,
        back_populates="users",
        lazy="immediate",
    )



class OrgRole(Base):

    __tablename__ = "org_role"

    id = Column(Integer, primary_key=True)
    org_id = Column(Integer, ForeignKey("org.id"))
    org = relationship("Org", back_populates="org_roles", lazy="immediate")
    role = Column(ChoiceType(Roles, impl=String()))

    users = relationship(
        "User",
        secondary=UserOrgRole,
        back_populates="org_roles",
    )

@samscott89
Copy link
Member

Thanks for sharing @jneo8! That generally looks good to me.

One thing is that you might want to change the has_role check operate over the org not the org.id. E.g.

has_role(user: User, role_name: String, org: Org) if
    org_role in user.org_roles and
    org_role.org = org and # << change here
    org_role.role.value = role_name;

@kkirsche
Copy link
Contributor

The example provided worked for me with my policy. @samscott89 let me know if you'd like me to make a PR for the this functionality.

@kkirsche
Copy link
Contributor

kkirsche commented Feb 2, 2023

Just following up on this to see if this would be of interest as a pull request as I hadn’t heard back in the past couple of months.

@kkirsche
Copy link
Contributor

Never heard back, so just submitted #1675 in hopes of getting some feedback for or against

@gj
Copy link
Member

gj commented Mar 4, 2023

Hey @kkirsche, I'm sorry we didn't get back to you sooner on this one. I'll talk to @samscott89 about it this week. Personally, I'm leaning (slightly) against shipping it mostly b/c I think the code you wrote in #1675 shows that it isn't too burdensome for users to implement this functionality even without a dedicated API, and I'm a bit wary of extending language library APIs in a piecemeal fashion (e.g., we ship this and then someone wonders why it's only in Python). Also I think to get #1675 shipshape I'd probably want to add tests and maybe see if there's a logical place to mention the new API in the docs (maybe where we talk about authorized_actions since I imagine this would be used similarly). I'm open to alternate opinions / discussing further. As always, your contributions are greatly appreciated!

@kkirsche
Copy link
Contributor

kkirsche commented Mar 4, 2023

I think the problem I have with that reasoning is I had to dig through your GitHub issues to figure out how to do it. For a less experienced or interested user, I expect they'd believe this simply isn't possible to do.

I only figured it out due to dumb luck not because it was easy

@kkirsche
Copy link
Contributor

kkirsche commented Mar 4, 2023

I'd rather ask for those items, file backlog issues to add to other languages, etc than expect users to understand how to use a low level API like query rule, which personally I don't understand well at all

@gj
Copy link
Member

gj commented Mar 19, 2023

Totally fair points. I talked to @samscott89 about it, and unfortunately right now we don't have the bandwidth to add tests & docs and get this ready to ship. Would you be up for doing that @kkirsche?

@kkirsche
Copy link
Contributor

kkirsche commented Jul 6, 2023

Ah I missed this. Yeah, this is actually going to be in #1572 in example code I'm sharing there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants