zeppelin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From prabhjyotsi...@apache.org
Subject zeppelin git commit: ZEPPELIN-3312 Add option to convert username to lower case
Date Mon, 23 Apr 2018 03:41:09 GMT
Repository: zeppelin
Updated Branches:
  refs/heads/branch-0.8 bd5b99ca3 -> 5ddee2445


ZEPPELIN-3312 Add option to convert username to lower case

This PR introduces a new configuration property to convert username to lower case. This is
useful when the users (from external sources like AD/LDAP) are coming in with mixed-case names
and Hadoop services (like Hive) can't authorize them correctly because Hadoop services recognize
users only in lower case (like Linux).

Adding a new config option "zeppelin.username.force.lowercase" to handle such scenarios.

Behavior without this PR:
Access is denied to CaMel case user while running a Hive paragraph

Behavior with this PR:
User is allowed to run query when proposed configuration set to true.
By default, keeping zeppelin.username.force.lowercase=false to retain the current behavior.

[Bug Fix]

* [ ] - Unit test

* https://issues.apache.org/jira/browse/ZEPPELIN-3312

* Travis CI should pass
* Manual steps to test:
1. Configure Zeppelin with Active Directory authentication
2. Login to Zeppelin as a CaMel case user
3. Try to run a simple JDBC note with a Hive query (like a select * query). This would fail
with "user [CaMel] does not have proper privileges to [USE] operation" error message.
4. Now set zeppelin.username.force.lowercase=true in custom zeppelin-site.xml configuration.
5. Once again, login as CaMel case user. This time the same Hive query would run as expected.
Because the username is now passed in lower case.
6. Also notice that after successful login, the login username (in the top-right corner) will
be in lower case too.

* Login as CaMel case user:
<img width="596" alt="screen shot 2018-04-12 at 3 33 01 am" src="https://user-images.githubusercontent.com/15668387/38672744-faf00f8e-3e03-11e8-86b2-cc5981d380d2.png">
* Notice the converted username post login:
<img width="806" alt="screen shot 2018-04-12 at 3 33 43 am" src="https://user-images.githubusercontent.com/15668387/38672777-108c7b66-3e04-11e8-8c97-467b4b73fe3d.png">

* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Vipin Rathor <v.rathor@gmail.com>

Closes #2923 from VipinRathor/ZEPPELIN-3312 and squashes the following commits:

886acb914 [Vipin Rathor] Add lowercase username support for interpreter permission
c11209803 [Vipin Rathor] Remove maven cyclic refernce and use conf object directly
549f84d96 [Vipin Rathor] Convert username for Notebook Authorization as well
f83ce9f9d [Vipin Rathor] Adding new test testUsernameForceLowerCase and fixing canGetPrincipalName
b2722999e [Vipin Rathor] Fixing Travis CI build failure due to indentation
83fb686e4 [Vipin Rathor] Incorporating PR review suggestion to zeppelin-site.xml-template
7f9b4df63 [Vipin Rathor] Add support to force username case conversion

Change-Id: Id13a5eca9718063cb629454a9d3d2fc6f3cfb663
(cherry picked from commit b89c9ad23b297942f72434baa4e4de0a435324e4)
Signed-off-by: Prabhjyot Singh <prabhjyotsingh@gmail.com>

# Conflicts:
#	zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java
#	zeppelin-server/src/test/java/org/apache/zeppelin/security/SecurityUtilsTest.java


Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/5ddee244
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/5ddee244
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/5ddee244

Branch: refs/heads/branch-0.8
Commit: 5ddee2445a6118ce2a8465ffa07f8a0dd9d31a53
Parents: bd5b99c
Author: Vipin Rathor <v.rathor@gmail.com>
Authored: Fri Apr 20 03:57:31 2018 -0500
Committer: Prabhjyot Singh <prabhjyotsingh@gmail.com>
Committed: Mon Apr 23 09:10:51 2018 +0530

----------------------------------------------------------------------
 conf/zeppelin-site.xml.template                 |  6 +++
 .../zeppelin/conf/ZeppelinConfiguration.java    |  5 ++
 .../zeppelin/interpreter/InterpreterOption.java |  9 ++++
 .../apache/zeppelin/utils/SecurityUtils.java    |  6 +++
 .../zeppelin/security/SecurityUtilsTest.java    | 52 ++++++++++++++++++--
 .../notebook/NotebookAuthorization.java         | 23 +++++++++
 6 files changed, 96 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5ddee244/conf/zeppelin-site.xml.template
----------------------------------------------------------------------
diff --git a/conf/zeppelin-site.xml.template b/conf/zeppelin-site.xml.template
index b12a072..e665a9b 100755
--- a/conf/zeppelin-site.xml.template
+++ b/conf/zeppelin-site.xml.template
@@ -411,6 +411,12 @@
 </property>
 
 <property>
+  <name>zeppelin.username.force.lowercase</name>
+  <value>false</value>
+  <description>Force convert username case to lower case, useful for Active Directory/LDAP.
Default is not to change case</description>
+</property>
+
+<property>
   <name>zeppelin.notebook.default.owner.username</name>
   <value></value>
   <description>Set owner role by default</description>

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5ddee244/zeppelin-interpreter/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
----------------------------------------------------------------------
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 5b367b7..b655c3d 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
@@ -509,6 +509,10 @@ public class ZeppelinConfiguration extends XMLConfiguration {
     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);
   }
@@ -758,6 +762,7 @@ public class ZeppelinConfiguration extends XMLConfiguration {
     // 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"),

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5ddee244/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java
----------------------------------------------------------------------
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 e8a9225..0c01d97 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 @@ package org.apache.zeppelin.interpreter;
 
 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 class InterpreterOption {
   }
 
   public List<String> getOwners() {
+    if (null != owners && conf.isUsernameForceLowerCase()) {
+      List<String> lowerCaseUsers = new ArrayList<String>();
+      for (String owner : owners) {
+        lowerCaseUsers.add(owner.toLowerCase());
+      }
+      return lowerCaseUsers;
+    }
     return owners;
   }
 

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5ddee244/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java
----------------------------------------------------------------------
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 50f167b..b7ce42b 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
@@ -41,6 +41,7 @@ import org.apache.shiro.web.mgt.DefaultWebSecurityManager;
 import org.apache.zeppelin.conf.ZeppelinConfiguration;
 import org.apache.zeppelin.realm.ActiveDirectoryGroupRealm;
 import org.apache.zeppelin.realm.LdapRealm;
+import org.apache.zeppelin.server.ZeppelinServer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -93,6 +94,11 @@ public class SecurityUtils {
     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;
     }

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5ddee244/zeppelin-server/src/test/java/org/apache/zeppelin/security/SecurityUtilsTest.java
----------------------------------------------------------------------
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 1527816..5bb4118 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
@@ -19,21 +19,26 @@ package org.apache.zeppelin.security;
 import static org.junit.Assert.*;
 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 sun.security.acl.PrincipalImpl;
 
-import java.net.URISyntaxException;
-import java.net.UnknownHostException;
-import java.net.InetAddress;
-
 @RunWith(PowerMockRunner.class)
 @PrepareForTest(org.apache.shiro.SecurityUtils.class)
 public class SecurityUtilsTest {
@@ -104,12 +109,49 @@ public class SecurityUtilsTest {
   @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);
   }
+
+
 }

http://git-wip-us.apache.org/repos/asf/zeppelin/blob/5ddee244/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java
----------------------------------------------------------------------
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 f73b49e..137af65 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
@@ -205,6 +205,21 @@ public class NotebookAuthorization {
     saveToFile();
   }
 
+  /*
+  * If case conversion is enforced, then change entity names to lower case
+  */
+  private Set<String> checkCaseAndConvert(Set<String> entities) {
+    if (conf.isUsernameForceLowerCase()) {
+      Set<String> set2 = new HashSet<String>();
+      for (String name : entities) {
+        set2.add(name.toLowerCase());
+      }
+      return set2;
+    } else {
+      return entities;
+    }
+  }
+
   public Set<String> getOwners(String noteId) {
     Map<String, Set<String>> noteAuthInfo = authInfo.get(noteId);
     Set<String> entities = null;
@@ -214,6 +229,8 @@ public class NotebookAuthorization {
       entities = noteAuthInfo.get("owners");
       if (entities == null) {
         entities = new HashSet<>();
+      } else {
+        entities = checkCaseAndConvert(entities);
       }
     }
     return entities;
@@ -228,6 +245,8 @@ public class NotebookAuthorization {
       entities = noteAuthInfo.get("readers");
       if (entities == null) {
         entities = new HashSet<>();
+      } else {
+        entities = checkCaseAndConvert(entities);
       }
     }
     return entities;
@@ -242,6 +261,8 @@ public class NotebookAuthorization {
       entities = noteAuthInfo.get("runners");
       if (entities == null) {
         entities = new HashSet<>();
+      } else {
+        entities = checkCaseAndConvert(entities);
       }
     }
     return entities;
@@ -256,6 +277,8 @@ public class NotebookAuthorization {
       entities = noteAuthInfo.get("writers");
       if (entities == null) {
         entities = new HashSet<>();
+      } else {
+        entities = checkCaseAndConvert(entities);
       }
     }
     return entities;


Mime
View raw message