raghav-reglobe commented on PR #70371:
URL: https://github.com/apache/doris/pull/70371#issuecomment-6739894975
Thanks — and thanks to @morningman for the e2e suite.
On the "hold the role" model: I read the concern as the role being a moving
target — an empty role granted today can be
filled tomorrow, and every holder inherits what it gains. This PR
deliberately keeps the privilege-superset rule rather
than a membership rule, for three reasons; I am open to changing it if you
still prefer membership.
1. It is the rule Doris already applies to the direct path. `GRANT <priv> ON
<obj> TO <user>` requires `grant_priv` plus
that privilege on that object
(`GrantTablePrivilegeCommand.checkTablePrivileges` rule 4, and the resource /
workload-group equivalents). A role grant is now held to the same bound,
object by object, so the two paths cannot be
played against each other.
2. The later-filling case is bounded by the same rule at the moment it
happens. Whoever runs `GRANT <priv> ON <obj> TO
ROLE r` must hold `grant_priv` plus that privilege on that object, and
`SHOW GRANTS` lists the role's holders, so the
account that fills the role is the one answerable for who inherits it —
exactly as it is for a role filled before
the grant. A membership requirement on the earlier grantor would not stop
this: the later grantor is a different
account acting under its own privileges.
3. Membership conflates consuming a role with administering it. Doris has no
`WITH ADMIN OPTION`, so "must hold the
role" would force a security-administration account — `grant_priv` plus
the data privileges it may hand out, but
itself not a data consumer — to become a member of every role it
administers, and would make `SHOW GRANTS` on such an
account list every role in the cluster. The privilege-superset rule lets
that account grant exactly the roles whose
privileges it already holds and nothing more, with `admin_priv` exempt as
you suggest.
If the project prefers the membership model anyway, the natural shape is
"hold the role, or hold every privilege it
carries" (either suffices; `admin_priv` exempt) — it keeps the
security-admin pattern working and adds the membership
path for accounts that have the role but not its privileges globally. I can
push that as a follow-up commit with the
tests adjusted, if you want it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
