github-actions[bot] commented on code in PR #67444:
URL: https://github.com/apache/doris/pull/67444#discussion_r3913082084
##########
gensrc/thrift/FrontendService.thrift:
##########
@@ -1623,6 +1623,7 @@ struct TShowProcessListResult {
}
struct TShowUserRequest {
+ 1: optional Types.TUserIdentity current_user_ident // to filter rows by
the requesting user's privileges
Review Comment:
[P1] Preserve session-mapped roles across this callback. Authentication
integrations keep their granted roles only in the originating ConnectContext,
and Auth.getRolesByUserWithLdap consults that set only on the session thread.
The BE callback reaches a Thrift worker with just this identity, so an OIDC/JIT
user whose mapped role has global ADMIN_PRIV or GRANT_PRIV is evaluated as a
normal user and cannot see all rows. Please carry the immutable
authenticated-role set (or a trusted precomputed authorization decision)
through the plan/request and cover a mapped-role administrator.
##########
fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java:
##########
@@ -2098,16 +2100,29 @@ public String getDefaultCloudCluster(String user) {
// ====== END CLOUD ======
// for mysql.user table
- public List<List<String>> getAllUserInfo() {
+ public List<List<String>> getAllUserInfo(UserIdentity currentUser) {
+ // Only role administrators (ADMIN_PRIV or GRANT_PRIV) may see every
account. A
+ // non-privileged user may only see their own account, so that
mysql.user does not
+ // leak the cluster's account list and privilege topology to arbitrary
users.
+ boolean canSeeAll = currentUser != null
+ &&
Env.getCurrentEnv().getAccessManager().checkGlobalPriv(currentUser,
PrivPredicate.GRANT);
List<List<String>> userInfos = Lists.newArrayList();
readLock();
try {
Map<String, List<User>> nameToUsers = userManager.getNameToUsers();
for (List<User> users : nameToUsers.values()) {
for (User user : users) {
if (!user.isSetByDomainResolver()) {
- List<String> userInfo =
Lists.newArrayList(Collections.nCopies(32, ""));
UserIdentity userIdent = user.getUserIdentity();
+ // Match by user name only (host intentionally
ignored): the account name is
+ // the identity subject, so a caller sees every
same-named entry regardless of
+ // its host pattern. Password material is masked
below, so nothing sensitive
+ // leaks even across host variants of the same name.
+ if (!canSeeAll && (currentUser == null ||
!userIdent.getQualifiedUser()
Review Comment:
[P1] Scope the self case to the exact authenticated account. Doris stores
multiple User entries for one name when their hosts differ, and
ConnectContext.currentUserIdentity is the exact user@host entry that matched
login. With a restricted u@hostA and a separate privileged u@hostB, this
username-only comparison admits both rows and exposes host, global privilege
flags, and password-policy state from the other account. Please compare the
complete UserIdentity here and add a same-name/different-host regression.
##########
fe/fe-core/src/test/java/org/apache/doris/service/FrontendServiceImplTest.java:
##########
@@ -583,11 +583,44 @@ public void fetchSchemaTableData() throws Exception {
}
@Test
- public void testShowUser() {
+ public void testShowUser() throws Exception {
Review Comment:
[P1] Remove the now-unused TShowUserResult import. This rewritten test no
longer declares a TShowUserResult, and the CheckStyle job on this exact head
fails at line 69 with UnusedImports. Please remove that import so the required
FE style gate passes.
--
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]
