diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/realm/RoleMappingLdapRealm.java b/zeppelin-server/src/main/java/org/apache/zeppelin/realm/RoleMappingLdapRealm.java index dfee10062fa..212a80c6790 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/realm/RoleMappingLdapRealm.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/realm/RoleMappingLdapRealm.java @@ -33,11 +33,15 @@ /** * RoleMappingLdapRealm is a simple LDAP realm that returns configured role for specific users. - * Multiple roles(quoted string, comma separated) can be specified for a user. + * Multiple roles (comma separated) can be specified for a user. * Example configurations: * ldapRealm = org.apache.zeppelin.realm.RoleMappingLdapRealm * ldapRealm.rolesByUsername = "user1":"admin","user2":"role1,role2","user3":"role3" - * ldapRealm.defaultRole = "role1,role2" + * ldapRealm.defaultRole = role1,role2 + * + *

Quotes are required around a {@code rolesByUsername} value that contains a comma, because + * the map entries themselves are comma separated. Quotes are not needed for {@code defaultRole} + * and are stripped if present. */ public class RoleMappingLdapRealm extends DefaultLdapRealm { @@ -79,9 +83,21 @@ public Set getRoleNamesForUser(String username, LdapContext ldapContext, String roleString = rolesByUsername.getOrDefault(username, defaultRole); Set roleNames = Arrays.stream(roleString.split(ROLE_NAMES_DELIMITER)) - .map(String::trim).collect(Collectors.toSet()); + .map(RoleMappingLdapRealm::stripQuotes).collect(Collectors.toSet()); LOGGER.debug("For user {}, RoleMappingLdapRealm returns {} as the role", username, roleNames); return roleNames; } + + // Shiro's Ini parser does not strip quotes for String properties such as defaultRole. + private static String stripQuotes(String role) { + String stripped = role.trim(); + if (stripped.startsWith("\"")) { + stripped = stripped.substring(1); + } + if (stripped.endsWith("\"")) { + stripped = stripped.substring(0, stripped.length() - 1); + } + return stripped.trim(); + } } diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/realm/RoleMappingLdapRealmTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/realm/RoleMappingLdapRealmTest.java index 07cea7adebd..00375bb0840 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/realm/RoleMappingLdapRealmTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/realm/RoleMappingLdapRealmTest.java @@ -18,10 +18,15 @@ */ package org.apache.zeppelin.realm; +import org.apache.shiro.config.Ini; +import org.apache.shiro.config.ogdl.ReflectionBuilder; import org.junit.jupiter.api.Test; import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Set; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; public class RoleMappingLdapRealmTest { @@ -59,4 +64,51 @@ public void testGetRolesByUsernameWithMultipleRoles() { assertTrue(realm.getRoleNamesForUser("user3", null, null).contains("guest1")); assertTrue(realm.getRoleNamesForUser("user3", null, null).contains("guest2")); } + + @Test + public void testDefaultRoleWithQuotes() { + RoleMappingLdapRealm realm = new RoleMappingLdapRealm(); + realm.setDefaultRole("\"guest1,guest2\""); + + Set roleNames = realm.getRoleNamesForUser("user3", null, null); + + assertEquals(Set.of("guest1", "guest2"), roleNames); + } + + @Test + public void testDefaultRoleWithIndividuallyQuotedRoles() { + RoleMappingLdapRealm realm = new RoleMappingLdapRealm(); + realm.setDefaultRole("\"guest1\",\"guest2\""); + + Set roleNames = realm.getRoleNamesForUser("user3", null, null); + + assertEquals(Set.of("guest1", "guest2"), roleNames); + } + + @Test + public void testRolesByUsernameWithQuotedRoles() { + RoleMappingLdapRealm realm = new RoleMappingLdapRealm(); + Map rolesByUsername = new LinkedHashMap<>(); + rolesByUsername.put("user1", "\"role1,role2\""); + realm.setRolesByUsername(rolesByUsername); + + Set roleNames = realm.getRoleNamesForUser("user1", null, null); + + assertEquals(Set.of("role1", "role2"), roleNames); + } + + @Test + public void testQuotedRolesParsedFromIni() { + Ini ini = new Ini(); + ini.load("ldapRealm = org.apache.zeppelin.realm.RoleMappingLdapRealm\n" + + "ldapRealm.defaultRole = \"role1,role2\"\n" + + "ldapRealm.rolesByUsername = \"user1\":\"admin\",\"user2\":\"role1,role2\"\n"); + Map objects = new ReflectionBuilder().buildObjects( + ini.getSection(Ini.DEFAULT_SECTION_NAME)); + RoleMappingLdapRealm realm = (RoleMappingLdapRealm) objects.get("ldapRealm"); + + assertEquals(Set.of("admin"), realm.getRoleNamesForUser("user1", null, null)); + assertEquals(Set.of("role1", "role2"), realm.getRoleNamesForUser("user2", null, null)); + assertEquals(Set.of("role1", "role2"), realm.getRoleNamesForUser("user3", null, null)); + } }