Skip to content

Commit 02bbd75

Browse files
committed
[improvement](fe) Support LDAP default role fallback mode
### What problem does this PR solve? Issue Number: N/A Related PR: #63411 Problem Summary: LDAP default roles should avoid broadening privileges for users that already have LDAP group-derived Doris roles by default. Add ldap_always_apply_default_roles so ldap_default_roles are fallback-only by default, while still allowing additive default roles when explicitly enabled. ### Release note Support configuring whether LDAP default roles are applied as fallback-only or always added. ### Check List (For Author) - Test: - Manual test: `git diff --cached --check` - Manual test: `env JAVA_HOME=/Library/Java/JavaVirtualMachines/jdk-17.jdk/Contents/Home /Users/zhanggen/.m2/wrapper/dists/apache-maven-3.9.5-bin/32db9c34/apache-maven-3.9.5/bin/mvn checkstyle:check -pl fe-common,fe-core` from `fe/` - Unit Test: Tried `env PATH=/private/tmp/doris-brew-shim:/opt/homebrew/bin:/usr/bin:/bin:/usr/sbin:/sbin FE_UT_PARALLEL=1 JAVA_HOME=/Library/Java/JavaVirtualMachines/jdk-17.jdk/Contents/Home CUSTOM_MVN=/Users/zhanggen/.m2/wrapper/dists/apache-maven-3.9.5-bin/32db9c34/apache-maven-3.9.5/bin/mvn ./run-fe-ut.sh --run 'org.apache.doris.mysql.authenticate.ldap.LdapManagerTest'`, but it failed before test execution because `thirdparty/installed/bin/protoc` is missing. - Behavior changed: Yes. By default, ldap_default_roles are applied only when no LDAP group-derived Doris role exists. Setting ldap_always_apply_default_roles=true keeps the additive behavior. - Does this need documentation: Yes. Updated ldap.conf template.
1 parent ecff045 commit 02bbd75

5 files changed

Lines changed: 55 additions & 5 deletions

File tree

conf/ldap.conf

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@
3131
# ldap_user_filter - User lookup filter, the placeholder {login} will be replaced by the user supplied login.
3232
# ldap_group_basedn - Search base for groups.
3333
# ldap_group_filter - Group lookup filter, the placeholder {login} will be replaced by the user supplied login. example : "(&(memberUid={login}))"
34-
# ldap_default_roles - Comma-separated Doris roles granted to every LDAP-authenticated user.
35-
# Online updates of ldap_default_roles refresh the LDAP user cache automatically.
34+
# ldap_default_roles - Comma-separated Doris roles granted when no LDAP group-derived Doris role exists.
35+
# ldap_always_apply_default_roles - Set true to grant ldap_default_roles to every LDAP-authenticated user.
36+
# Online updates of ldap_default_roles and ldap_always_apply_default_roles refresh the LDAP user cache automatically.
3637
## step2: Restart fe, and use root or admin account to log in to doris.
3738
## step3: Execute sql statement to set ldap admin password:
3839
# set ldap_admin_password = 'password';
@@ -44,6 +45,7 @@ ldap_user_basedn = ou=people,dc=domain,dc=com
4445
ldap_user_filter = (&(uid={login}))
4546
ldap_group_basedn = ou=group,dc=domain,dc=com
4647
# ldap_default_roles = ldap_default_role
48+
# ldap_always_apply_default_roles = true
4749

4850
# ldap_user_cache_timeout_s = 5 * 60;
4951

fe/fe-common/src/main/java/org/apache/doris/common/LdapConfig.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,12 @@ public class LdapConfig extends ConfigBase {
7878
@ConfigBase.ConfField(mutable = true)
7979
public static String[] ldap_default_roles = {};
8080

81+
/**
82+
* Whether ldap_default_roles should be applied even when LDAP group-derived Doris roles exist.
83+
*/
84+
@ConfigBase.ConfField(mutable = true)
85+
public static boolean ldap_always_apply_default_roles = false;
86+
8187
/**
8288
* The user LDAP information cache time.
8389
* After timeout, the user information will be retrieved from the LDAP service again.

fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6790,7 +6790,7 @@ public void replayDropGlobalFunction(FunctionSearchDesc functionSearchDesc) {
67906790
*/
67916791
public void setMutableConfigWithCallback(String key, String value) throws ConfigException {
67926792
ConfigBase.setMutableConfig(key, value);
6793-
if ("ldap_default_roles".equals(key)) {
6793+
if ("ldap_default_roles".equals(key) || "ldap_always_apply_default_roles".equals(key)) {
67946794
getAuth().getLdapManager().refresh(true, null);
67956795
}
67966796
if (configtoThreads.get(key) != null) {

fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapManager.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,9 @@ private Set<Role> getLdapGroupsRoles(String userName) throws DdlException {
258258
List<String> ldapGroups = ldapClient.getGroups(userName);
259259
Set<Role> roles = Sets.newHashSet();
260260
addExistingRoles(roles, ldapGroups, false);
261-
addExistingRoles(roles, Arrays.asList(LdapConfig.ldap_default_roles), true);
261+
if (LdapConfig.ldap_always_apply_default_roles || roles.isEmpty()) {
262+
addExistingRoles(roles, Arrays.asList(LdapConfig.ldap_default_roles), true);
263+
}
262264
if (LOG.isDebugEnabled()) {
263265
LOG.debug("get user:{} ldap groups:{} and doris roles:{}", userName, ldapGroups, roles);
264266
}

fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapManagerTest.java

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ public class LdapManagerTest {
4747
public void setUp() {
4848
Config.authentication_type = "ldap";
4949
LdapConfig.ldap_default_roles = new String[0];
50+
LdapConfig.ldap_always_apply_default_roles = false;
5051
}
5152

5253
private void mockClient(boolean userExist, boolean passwd) {
@@ -102,12 +103,51 @@ public void testCheckUserPasswd() {
102103
}
103104

104105
@Test
105-
public void testGetUserInfoWithLdapDefaultRoles() {
106+
public void testGetUserInfoWithLdapDefaultRolesFallback() {
106107
LdapManager ldapManager = new LdapManager();
107108
Deencapsulation.setField(ldapManager, "ldapClient", ldapClient);
108109
LdapConfig.ldap_default_roles = new String[] {LDAP_DEFAULT_ROLE, MISSING_LDAP_DEFAULT_ROLE};
109110
Role ldapGroupRole = new Role(LDAP_GROUP_ROLE);
110111
Role ldapDefaultRole = new Role(LDAP_DEFAULT_ROLE);
112+
mockClient(true, true, new ArrayList<>());
113+
try (MockedStatic<Env> envMockedStatic = Mockito.mockStatic(Env.class)) {
114+
mockAuth(envMockedStatic, ldapGroupRole, ldapDefaultRole);
115+
116+
LdapUserInfo ldapUserInfo = ldapManager.getUserInfo(USER1);
117+
Assert.assertNotNull(ldapUserInfo);
118+
Assert.assertFalse(ldapUserInfo.getRoles().contains(ldapGroupRole));
119+
Assert.assertTrue(ldapUserInfo.getRoles().contains(ldapDefaultRole));
120+
Assert.assertEquals(2, ldapUserInfo.getRoles().size());
121+
}
122+
}
123+
124+
@Test
125+
public void testGetUserInfoWithLdapDefaultRolesNotAppliedWhenLdapGroupRoleExists() {
126+
LdapManager ldapManager = new LdapManager();
127+
Deencapsulation.setField(ldapManager, "ldapClient", ldapClient);
128+
LdapConfig.ldap_default_roles = new String[] {LDAP_DEFAULT_ROLE, MISSING_LDAP_DEFAULT_ROLE};
129+
Role ldapGroupRole = new Role(LDAP_GROUP_ROLE);
130+
Role ldapDefaultRole = new Role(LDAP_DEFAULT_ROLE);
131+
mockClient(true, true, new ArrayList<>(Arrays.asList(LDAP_GROUP_ROLE)));
132+
try (MockedStatic<Env> envMockedStatic = Mockito.mockStatic(Env.class)) {
133+
mockAuth(envMockedStatic, ldapGroupRole, ldapDefaultRole);
134+
135+
LdapUserInfo ldapUserInfo = ldapManager.getUserInfo(USER1);
136+
Assert.assertNotNull(ldapUserInfo);
137+
Assert.assertTrue(ldapUserInfo.getRoles().contains(ldapGroupRole));
138+
Assert.assertFalse(ldapUserInfo.getRoles().contains(ldapDefaultRole));
139+
Assert.assertEquals(2, ldapUserInfo.getRoles().size());
140+
}
141+
}
142+
143+
@Test
144+
public void testGetUserInfoWithAlwaysApplyLdapDefaultRoles() {
145+
LdapManager ldapManager = new LdapManager();
146+
Deencapsulation.setField(ldapManager, "ldapClient", ldapClient);
147+
LdapConfig.ldap_default_roles = new String[] {LDAP_DEFAULT_ROLE, MISSING_LDAP_DEFAULT_ROLE};
148+
LdapConfig.ldap_always_apply_default_roles = true;
149+
Role ldapGroupRole = new Role(LDAP_GROUP_ROLE);
150+
Role ldapDefaultRole = new Role(LDAP_DEFAULT_ROLE);
111151
mockClient(true, true, new ArrayList<>(Arrays.asList(LDAP_GROUP_ROLE)));
112152
try (MockedStatic<Env> envMockedStatic = Mockito.mockStatic(Env.class)) {
113153
mockAuth(envMockedStatic, ldapGroupRole, ldapDefaultRole);

0 commit comments

Comments
 (0)