From 7f9b4df63106869876ce20ec2317f6fea981719f Mon Sep 17 00:00:00 2001 From: Vipin Rathor Date: Thu, 12 Apr 2018 03:23:04 -0700 Subject: [PATCH 1/7] Add support to force username case conversion --- conf/zeppelin-site.xml.template | 6 ++++++ .../org/apache/zeppelin/conf/ZeppelinConfiguration.java | 5 +++++ .../main/java/org/apache/zeppelin/utils/SecurityUtils.java | 6 ++++++ 3 files changed, 17 insertions(+) diff --git a/conf/zeppelin-site.xml.template b/conf/zeppelin-site.xml.template index b12a072c148..2eaf41bf9e8 100755 --- a/conf/zeppelin-site.xml.template +++ b/conf/zeppelin-site.xml.template @@ -410,6 +410,12 @@ Anonymous user allowed by default + + zeppelin.username.force.lowercase + false + Force convert username case to lower case, useful for Active Directory. Default is not to change case + + zeppelin.notebook.default.owner.username diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java index 2960cd03faa..061e230be14 100644 --- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java +++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java @@ -517,6 +517,10 @@ public boolean isAnonymousAllowed() { return getBoolean(ConfVars.ZEPPELIN_ANONYMOUS_ALLOWED); } + public boolean isUsernameForceLowerCase() { + return getBoolean(ConfVars.ZEPPELIN_USERNAME_FORCE_LOWERCASE); + } + public boolean isNotebookPublic() { return getBoolean(ConfVars.ZEPPELIN_NOTEBOOK_PUBLIC); } @@ -767,6 +771,7 @@ public enum ConfVars { // i.e. http://localhost:8080 ZEPPELIN_ALLOWED_ORIGINS("zeppelin.server.allowed.origins", "*"), ZEPPELIN_ANONYMOUS_ALLOWED("zeppelin.anonymous.allowed", true), + ZEPPELIN_USERNAME_FORCE_LOWERCASE("zeppelin.username.force.lowercase", false), ZEPPELIN_CREDENTIALS_PERSIST("zeppelin.credentials.persist", true), ZEPPELIN_CREDENTIALS_ENCRYPT_KEY("zeppelin.credentials.encryptKey", null), ZEPPELIN_WEBSOCKET_MAX_TEXT_MESSAGE_SIZE("zeppelin.websocket.max.text.message.size", "1024000"), diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java b/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java index f9f5f228ff0..27271bccb8f 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java @@ -44,6 +44,7 @@ import org.apache.zeppelin.conf.ZeppelinConfiguration; import org.apache.zeppelin.realm.ActiveDirectoryGroupRealm; import org.apache.zeppelin.realm.LdapRealm; +import org.apache.zeppelin.server.ZeppelinServer; /** * Tools for securing Zeppelin. @@ -91,6 +92,11 @@ public static String getPrincipal() { String principal; if (subject.isAuthenticated()) { principal = extractPrincipal(subject); + if (ZeppelinServer.notebook.getConf().isUsernameForceLowerCase()) { + log.debug("Converting principal name " + principal + + " to lower case:" + principal.toLowerCase()); + principal = principal.toLowerCase(); + } } else { principal = ANONYMOUS; } From 83fb686e4c4ee42a067d423aa4e434d36eff436a Mon Sep 17 00:00:00 2001 From: Vipin Rathor Date: Mon, 16 Apr 2018 22:51:55 -0700 Subject: [PATCH 2/7] Incorporating PR review suggestion to zeppelin-site.xml-template --- conf/zeppelin-site.xml.template | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conf/zeppelin-site.xml.template b/conf/zeppelin-site.xml.template index 2eaf41bf9e8..e665a9b3c10 100755 --- a/conf/zeppelin-site.xml.template +++ b/conf/zeppelin-site.xml.template @@ -413,7 +413,7 @@ zeppelin.username.force.lowercase false - Force convert username case to lower case, useful for Active Directory. Default is not to change case + Force convert username case to lower case, useful for Active Directory/LDAP. Default is not to change case From b2722999e00fd5ce622732378247d78e8e53488f Mon Sep 17 00:00:00 2001 From: Vipin Rathor Date: Tue, 17 Apr 2018 13:59:31 -0500 Subject: [PATCH 3/7] Fixing Travis CI build failure due to indentation --- .../src/main/java/org/apache/zeppelin/utils/SecurityUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java b/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java index 27271bccb8f..f5329f31683 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java @@ -94,7 +94,7 @@ public static String getPrincipal() { principal = extractPrincipal(subject); if (ZeppelinServer.notebook.getConf().isUsernameForceLowerCase()) { log.debug("Converting principal name " + principal - + " to lower case:" + principal.toLowerCase()); + + " to lower case:" + principal.toLowerCase()); principal = principal.toLowerCase(); } } else { From f83ce9f9d24c8a243e8faa7bd9cccdfec0073765 Mon Sep 17 00:00:00 2001 From: Vipin Rathor Date: Wed, 18 Apr 2018 16:59:17 -0500 Subject: [PATCH 4/7] Adding new test testUsernameForceLowerCase and fixing canGetPrincipalName --- .../zeppelin/security/SecurityUtilsTest.java | 58 ++++++++++++++++--- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/security/SecurityUtilsTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/security/SecurityUtilsTest.java index ac6f1688002..0c535b968eb 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/security/SecurityUtilsTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/security/SecurityUtilsTest.java @@ -21,23 +21,26 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.when; +import java.io.IOException; +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.net.InetAddress; +import java.net.URISyntaxException; +import java.net.UnknownHostException; import org.apache.commons.configuration.ConfigurationException; +import org.apache.zeppelin.conf.ZeppelinConfiguration; +import org.apache.zeppelin.notebook.Notebook; +import org.apache.zeppelin.server.ZeppelinServer; +import org.apache.zeppelin.utils.SecurityUtils; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.Mockito; import org.powermock.api.mockito.PowerMockito; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; - -import java.net.InetAddress; -import java.net.URISyntaxException; -import java.net.UnknownHostException; - import sun.security.acl.PrincipalImpl; -import org.apache.zeppelin.conf.ZeppelinConfiguration; -import org.apache.zeppelin.utils.SecurityUtils; - @RunWith(PowerMockRunner.class) @PrepareForTest(org.apache.shiro.SecurityUtils.class) public class SecurityUtilsTest { @@ -113,12 +116,49 @@ public void notAURIOrigin() @Test public void canGetPrincipalName() { String expectedName = "java.security.Principal.getName()"; + setupPrincipalName(expectedName); + assertEquals(expectedName, SecurityUtils.getPrincipal()); + } + + @Test + public void testUsernameForceLowerCase() throws IOException, InterruptedException { + String expectedName = "java.security.Principal.getName()"; + System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_USERNAME_FORCE_LOWERCASE + .getVarName(), String.valueOf(true)); + setupPrincipalName(expectedName); + assertEquals(expectedName.toLowerCase(), SecurityUtils.getPrincipal()); + + } + + private void setupPrincipalName(String expectedName) { SecurityUtils.setIsEnabled(true); PowerMockito.mockStatic(org.apache.shiro.SecurityUtils.class); when(org.apache.shiro.SecurityUtils.getSubject()).thenReturn(subject); when(subject.isAuthenticated()).thenReturn(true); when(subject.getPrincipal()).thenReturn(new PrincipalImpl(expectedName)); - assertEquals(expectedName, SecurityUtils.getPrincipal()); + Notebook notebook = Mockito.mock(Notebook.class); + try { + setFinalStatic(ZeppelinServer.class.getDeclaredField("notebook"), notebook); + when(ZeppelinServer.notebook.getConf()) + .thenReturn(new ZeppelinConfiguration(this.getClass().getResource("/zeppelin-site.xml"))); + } catch (NoSuchFieldException e) { + e.printStackTrace(); + } catch (IllegalAccessException e) { + e.printStackTrace(); + } catch (ConfigurationException e) { + e.printStackTrace(); + } + } + + private void setFinalStatic(Field field, Object newValue) + throws NoSuchFieldException, IllegalAccessException { + field.setAccessible(true); + Field modifiersField = Field.class.getDeclaredField("modifiers"); + modifiersField.setAccessible(true); + modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL); + field.set(null, newValue); } + + } From 549f84d96029fdbf1a7999955d797531639092ec Mon Sep 17 00:00:00 2001 From: Vipin Rathor Date: Wed, 18 Apr 2018 20:39:14 -0500 Subject: [PATCH 5/7] Convert username for Notebook Authorization as well --- zeppelin-zengine/pom.xml | 5 ++++ .../notebook/NotebookAuthorization.java | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/zeppelin-zengine/pom.xml b/zeppelin-zengine/pom.xml index d37f6eab88d..93d92936a90 100644 --- a/zeppelin-zengine/pom.xml +++ b/zeppelin-zengine/pom.xml @@ -56,6 +56,11 @@ zeppelin-interpreter ${project.version} + + ${project.groupId} + zeppelin-server + ${project.version} + org.slf4j diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java index f73b49e3a09..16b9d76bea7 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java @@ -35,6 +35,7 @@ import org.apache.commons.lang.StringUtils; import org.apache.zeppelin.conf.ZeppelinConfiguration; import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars; +import org.apache.zeppelin.server.ZeppelinServer; import org.apache.zeppelin.storage.ConfigStorage; import org.apache.zeppelin.user.AuthenticationInfo; import org.slf4j.Logger; @@ -205,6 +206,21 @@ public void setWriters(String noteId, Set entities) { saveToFile(); } + /* + * If case conversion is enforced, then change entity names to lower case + */ + private Set checkCaseAndConvert(Set entities) { + if (ZeppelinServer.notebook.getConf().isUsernameForceLowerCase()) { + Set set2 = new HashSet(); + for (String name : entities) { + set2.add(name.toLowerCase()); + } + return set2; + } else { + return entities; + } + } + public Set getOwners(String noteId) { Map> noteAuthInfo = authInfo.get(noteId); Set entities = null; @@ -214,6 +230,8 @@ public Set getOwners(String noteId) { entities = noteAuthInfo.get("owners"); if (entities == null) { entities = new HashSet<>(); + } else { + entities = checkCaseAndConvert(entities); } } return entities; @@ -228,6 +246,8 @@ public Set getReaders(String noteId) { entities = noteAuthInfo.get("readers"); if (entities == null) { entities = new HashSet<>(); + } else { + entities = checkCaseAndConvert(entities); } } return entities; @@ -242,6 +262,8 @@ public Set getRunners(String noteId) { entities = noteAuthInfo.get("runners"); if (entities == null) { entities = new HashSet<>(); + } else { + entities = checkCaseAndConvert(entities); } } return entities; @@ -256,6 +278,8 @@ public Set getWriters(String noteId) { entities = noteAuthInfo.get("writers"); if (entities == null) { entities = new HashSet<>(); + } else { + entities = checkCaseAndConvert(entities); } } return entities; From c112098034da33c5997638d3967014890af0c029 Mon Sep 17 00:00:00 2001 From: Vipin Rathor Date: Wed, 18 Apr 2018 21:10:13 -0500 Subject: [PATCH 6/7] Remove maven cyclic refernce and use conf object directly --- zeppelin-zengine/pom.xml | 5 ----- .../org/apache/zeppelin/notebook/NotebookAuthorization.java | 3 +-- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/zeppelin-zengine/pom.xml b/zeppelin-zengine/pom.xml index 93d92936a90..d37f6eab88d 100644 --- a/zeppelin-zengine/pom.xml +++ b/zeppelin-zengine/pom.xml @@ -56,11 +56,6 @@ zeppelin-interpreter ${project.version} - - ${project.groupId} - zeppelin-server - ${project.version} - org.slf4j diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java index 16b9d76bea7..137af651aa9 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java @@ -35,7 +35,6 @@ import org.apache.commons.lang.StringUtils; import org.apache.zeppelin.conf.ZeppelinConfiguration; import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars; -import org.apache.zeppelin.server.ZeppelinServer; import org.apache.zeppelin.storage.ConfigStorage; import org.apache.zeppelin.user.AuthenticationInfo; import org.slf4j.Logger; @@ -210,7 +209,7 @@ public void setWriters(String noteId, Set entities) { * If case conversion is enforced, then change entity names to lower case */ private Set checkCaseAndConvert(Set entities) { - if (ZeppelinServer.notebook.getConf().isUsernameForceLowerCase()) { + if (conf.isUsernameForceLowerCase()) { Set set2 = new HashSet(); for (String name : entities) { set2.add(name.toLowerCase()); From 886acb9142bdcc3b3183d6200c1c3eab52f35282 Mon Sep 17 00:00:00 2001 From: Vipin Rathor Date: Fri, 20 Apr 2018 03:57:31 -0500 Subject: [PATCH 7/7] Add lowercase username support for interpreter permission --- .../apache/zeppelin/interpreter/InterpreterOption.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java index e8a92255fc9..0c01d97ecf7 100644 --- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java +++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java @@ -19,6 +19,7 @@ import java.util.ArrayList; import java.util.List; +import org.apache.zeppelin.conf.ZeppelinConfiguration; /** * @@ -27,6 +28,7 @@ public class InterpreterOption { public static final transient String SHARED = "shared"; public static final transient String SCOPED = "scoped"; public static final transient String ISOLATED = "isolated"; + private static ZeppelinConfiguration conf = ZeppelinConfiguration.create(); // always set it as true, keep this field just for backward compatibility boolean remote = true; @@ -66,6 +68,13 @@ public void setUserPermission(boolean setPermission) { } public List getOwners() { + if (null != owners && conf.isUsernameForceLowerCase()) { + List lowerCaseUsers = new ArrayList(); + for (String owner : owners) { + lowerCaseUsers.add(owner.toLowerCase()); + } + return lowerCaseUsers; + } return owners; }