🚀 rbac: Support group reference in superUsers config

[Sancretor]

Workspace

rbac

🔖 Feature description

Hello dear community,

Like explained below, I'd like to use group reference in the superUsers field of the RBAC plugin, so I don't have to list all users one by one.

As I'm pretty motivated to open a PR for this feature, I just wanted to know if there is a reason for not supporting this, so I'm not working on a PR that will be rejected right away.

Thanks for your feedback 🙏

🎤 Context

While testing the RBAC plugin (which is great, thanks community !), I realized the following configuration is failing :

permission:
  enabled: true
  rbac:
    admin:
      superUsers:
        - name: group:default/my-group # <--- this is the important part

It seems RBAC plugin does not support group reference in the superUsers field, unlike the users one.
The documentation states it works for users but do not provide any clue regarding superUsers.

✌️ Possible Implementation

No response

👀 Have you spent some time to check if this feature request has been raised before?

• I checked and didn't find similar issue

🏢 Have you read the Code of Conduct?

• I have read the Code of Conduct

Are you willing to submit PR?

Yes I am willing to submit a PR!

[PatAKnight]

Hey @Sancretor,

To be honest, we initially stuck to users for superUsers because it was the most straightforward path at the time. Currently, we just check the user object passed into the handle method against the list here. If there's a match, we bypass the standard checks and return ALLOW.

Expanding this to groups is a great idea, though it adds a layer of complexity. We'll need to resolve a user's group memberships, specifically, verifying if the user object already contains the necessary group claims or if we'd need to fetch them and ensuring the comparisons remain efficient to avoid performance hits.

For context on how we currently handle groups:

• Conditional policies: These utilize the group claims directly from the user object (which doesn't account for transient memberships).
• Basic permissions: These do account for transient group membership.

Depending on the approach, a note in the README.md might be sufficient regarding identity resolution for transient groups.

It depends on what works best, and the performance hits might not even be a major issue if the superUser list remains reasonably sized, but it's a good edge case to keep in mind.

We'd love to see a PR if you're up for it!

[Sancretor]

Hi @PatAKnight,

Thanks for your detailed answer !

At first look it seemed the function already has access to the user direct memberships. That would mean we could mimic the Conditional policies' behavior quite easily, without any additional data requests.

Regarding transient memberships, whatever superUsers size, this would hurt every single user on each of its requests by calling the database or a service. Or like you said, this would need to change identity resolution at sign-in, so all transient memberships are resolved once.

Well, maybe a good first approach could be to check for direct memberships only, and add a note in the README.md, as this won't impact performances and slightly improve the current behavior.

Alright, I'll start working on a PR soon.

[PatAKnight]

Well, maybe a good first approach could be to check for direct memberships only, and add a note in the README.md, as this won't impact performances and slightly improve the current behavior.

Yep, we can always just wait to see if anyone presents a use case for needing transient memberships. We haven't gotten one for the conditional policies, yet. So, this should be fine as a starting point.