zddr commented on code in PR #21968: URL: https://github.com/apache/doris/pull/21968#discussion_r1271468935
########## fe/fe-core/src/main/java/org/apache/doris/ldap/LdapManager.java: ########## @@ -200,24 +207,22 @@ private LdapUserInfo getUserInfoFromCache(String fullName) { * Step2: get roles by ldap groups; * Step3: merge the roles; */ - private Role getLdapGroupsPrivs(String userName, String clusterName) throws DdlException { + private Set<Role> getLdapGroupsRoles(String userName, String clusterName) 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); - List<String> rolesNames = Lists.newArrayList(); + Set<Role> roles = Sets.newHashSet(); for (String group : ldapGroups) { String qualifiedRole = ClusterNamespace.getFullName(clusterName, group); if (Env.getCurrentEnv().getAuth().doesRoleExist(qualifiedRole)) { - rolesNames.add(qualifiedRole); + roles.add(Env.getCurrentEnv().getAuth().getRoleByName(qualifiedRole)); } } - LOG.debug("get user:{} ldap groups:{} and doris roles:{}", userName, ldapGroups, rolesNames); + LOG.debug("get user:{} ldap groups:{} and doris roles:{}", userName, ldapGroups, roles); Role ldapGroupsPrivs = new Role(LDAP_GROUPS_PRIVS_NAME); Review Comment: if create a default role in RoleManager,user will can grant priv to this role,that will change auth of all ldap user.so I change the name if LDAP_GROUPS_PRIVS_NAME to ldapDefaultRole only. -- 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: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org