xiangfu0 commented on code in PR #18975:
URL: https://github.com/apache/pinot/pull/18975#discussion_r4244880235
##########
pinot-core/src/main/java/org/apache/pinot/core/auth/BasicAuthPrincipal.java:
##########
@@ -62,6 +74,21 @@ public boolean hasTable(String tableName) {
return isTableIncluded(tableName) && isTableNotExcluded(tableName);
}
+ /// Returns whether this principal is scoped to every table, i.e. it carries
neither an allow-list nor an
+ /// exclude-list. Access control implementations use this to decide requests
that name no table, so the return value
+ /// must satisfy: `true` implies [#hasTable(String)] holds for every table
name. A subclass that narrows table scope
+ /// by any other means — in particular by overriding [#hasTable(String)] —
must override this method to match, or it
+ /// will report unrestricted scope while denying individual tables.
+ ///
+ /// Table lists are the signal rather than [ZkBasicAuthPrincipal]'s
`RoleType`. The static factory has no role, both
+ /// controller factories share this check, and an ADMIN that also carries an
allow-list is still scoped. Granting
+ /// cluster access by role would let that principal reach cluster state
outside the list. The bootstrapped admin
+ /// is bootstrapped with a null `UserConfig#getTables()`, which normalizes
to an empty set here, so
+ /// `RoleType.ADMIN` without a table list still passes.
+ public boolean hasUnrestrictedTableAccess() {
Review Comment:
(Carried over, optional.) `hasTable` and `hasUnrestrictedTableAccess` are
still non-`final`, so the invariant the Javadoc states — a subclass overriding
`hasTable` must also override this, or it reports unrestricted scope while
denying individual tables (a cluster-access escalation) — is documented but not
enforced. Correct today since the only subclass (`ZkBasicAuthPrincipal`)
doesn't override `hasTable`. Restoring `final` on both would make the compiler
enforce it rather than relying on the next subclass author reading the
contract. Not blocking.
--
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]
