seawinde commented on code in PR #63411:
URL: https://github.com/apache/doris/pull/63411#discussion_r3309741107
##########
fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapManager.java:
##########
@@ -256,12 +257,8 @@ private Set<Role> getLdapGroupsRoles(String userName)
throws DdlException {
// get user ldap group. the ldap group name should be the same as the
doris role name
List<String> ldapGroups = ldapClient.getGroups(userName);
Set<Role> roles = Sets.newHashSet();
- for (String group : ldapGroups) {
- String qualifiedRole = group;
- if (Env.getCurrentEnv().getAuth().doesRoleExist(qualifiedRole)) {
-
roles.add(Env.getCurrentEnv().getAuth().getRoleByName(qualifiedRole));
- }
- }
+ addExistingRoles(roles, ldapGroups, false);
+ addExistingRoles(roles, Arrays.asList(LdapConfig.ldap_default_roles),
true);
Review Comment:
This adds ldap_default_roles unconditionally for every LDAP-authenticated
user. If the intended behavior is to provide fallback roles only when the LDAP
user has no group-derived Doris roles, this should be guarded, for example
by `if (roles.isEmpty())`. Otherwise users who already have LDAP group
roles will also receive these default roles, which may unintentionally broaden
privileges.
--
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]
