You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by zj...@apache.org on 2020/03/03 13:49:06 UTC

[zeppelin] branch master updated: [ZEPPELIN-4645]. Only list the interpreters that the user has permission in creating note and interpreter binding

This is an automated email from the ASF dual-hosted git repository.

zjffdu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git


The following commit(s) were added to refs/heads/master by this push:
     new 9822514  [ZEPPELIN-4645]. Only list the interpreters that the user has permission in creating note and interpreter binding
9822514 is described below

commit 982251418ff7d75a0c141b4c2960d95192d94b8d
Author: Jeff Zhang <zj...@apache.org>
AuthorDate: Wed Feb 26 17:33:07 2020 +0800

    [ZEPPELIN-4645]. Only list the interpreters that the user has permission in creating note and interpreter binding
    
    ### What is this PR for?
    
    It doesn't make sense to list the interpreters that user don't have permission. This PR just filter the interpreters that user don't have permission in the creating note page and interpreter binding section. But I would always add the default interpreter in the interpreter binding section without checking its permission, because it would make user confused if the default interpreter is lost after interpreter permission is changed.
    
    ### What type of PR is it?
    [ Improvement]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-4645
    
    ### How should this be tested?
    * CI pass and tested manually
    
    ### Screenshots (if appropriate)
    
    ### Questions:
    * Does the licenses files need update? No
    * Is there breaking changes for older versions? No
    * Does this needs documentation? No
    
    Author: Jeff Zhang <zj...@apache.org>
    
    Closes #3663 from zjffdu/ZEPPELIN-4645 and squashes the following commits:
    
    f920c8d15 [Jeff Zhang] [ZEPPELIN-4645]. Only list the interpreters that the user has permission in creating note and interpreter binding
---
 .../apache/zeppelin/service/NotebookService.java   |  4 ++-
 .../org/apache/zeppelin/socket/NotebookServer.java | 29 ++++++++++++++++------
 .../zeppelin/rest/InterpreterRestApiTest.java      |  4 +--
 .../apache/zeppelin/socket/NotebookServerTest.java |  3 ++-
 .../java/org/apache/zeppelin/notebook/Note.java    | 18 +++++++++-----
 .../helium/HeliumApplicationFactoryTest.java       |  4 ++-
 .../org/apache/zeppelin/notebook/NotebookTest.java | 18 +++++++-------
 7 files changed, 52 insertions(+), 28 deletions(-)

diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
index f7b3ab6..b6dda71 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/service/NotebookService.java
@@ -27,6 +27,7 @@ import com.google.gson.reflect.TypeToken;
 import java.io.IOException;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
+import java.util.ArrayList;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -1112,7 +1113,8 @@ public class NotebookService {
     // propagate change to (Remote) AngularObjectRegistry
     Note note = notebook.getNote(noteId);
     if (note != null) {
-      List<InterpreterSetting> settings = note.getBindedInterpreterSettings();
+      List<InterpreterSetting> settings =
+              note.getBindedInterpreterSettings(new ArrayList(context.getUserAndRoles()));
       for (InterpreterSetting setting : settings) {
         if (setting.getInterpreterGroup(user, note.getId()) == null) {
           continue;
diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
index d29ab4f..01c463d 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
@@ -271,7 +271,8 @@ public class NotebookServer extends WebSocketServlet
       ZeppelinConfiguration conf = ZeppelinConfiguration.create();
       boolean allowAnonymous = conf.isAnonymousAllowed();
       if (!allowAnonymous && messagereceived.principal.equals("anonymous")) {
-        throw new Exception("Anonymous access not allowed ");
+        LOG.warn("Anonymous access not allowed.");
+        return;
       }
 
       if (Message.isDisabledForRunningNotes(messagereceived.op)) {
@@ -429,7 +430,7 @@ public class NotebookServer extends WebSocketServlet
           getEditorSetting(conn, messagereceived);
           break;
         case GET_INTERPRETER_SETTINGS:
-          getInterpreterSettings(conn);
+          getInterpreterSettings(conn, messagereceived);
           break;
         case WATCHER:
           connectionManager.switchConnectionToWatcher(conn);
@@ -529,10 +530,12 @@ public class NotebookServer extends WebSocketServlet
 
   public void getInterpreterBindings(NotebookSocket conn, Message fromMessage) throws IOException {
     List<InterpreterSettingsList> settingList = new ArrayList<>();
+    ServiceContext context = getServiceContext(fromMessage);
     String noteId = (String) fromMessage.data.get("noteId");
     Note note = getNotebook().getNote(noteId);
     if (note != null) {
-      List<InterpreterSetting> bindedSettings = note.getBindedInterpreterSettings();
+      List<InterpreterSetting> bindedSettings =
+              note.getBindedInterpreterSettings(new ArrayList<>(context.getUserAndRoles()));
       for (InterpreterSetting setting : bindedSettings) {
         settingList.add(new InterpreterSettingsList(setting.getId(), setting.getName(),
                 setting.getInterpreterInfos(), true));
@@ -545,6 +548,7 @@ public class NotebookServer extends WebSocketServlet
   public void saveInterpreterBindings(NotebookSocket conn, Message fromMessage) throws IOException {
     List<InterpreterSettingsList> settingList = new ArrayList<>();
     String noteId = (String) fromMessage.data.get("noteId");
+    ServiceContext context = getServiceContext(fromMessage);
     Note note = getNotebook().getNote(noteId);
     if (note != null) {
       List<String> settingIdList =
@@ -555,7 +559,8 @@ public class NotebookServer extends WebSocketServlet
         getNotebook().saveNote(note,
                 new AuthenticationInfo(fromMessage.principal, fromMessage.roles, fromMessage.ticket));
       }
-      List<InterpreterSetting> bindedSettings = note.getBindedInterpreterSettings();
+      List<InterpreterSetting> bindedSettings =
+              note.getBindedInterpreterSettings(new ArrayList<>(context.getUserAndRoles()));
       for (InterpreterSetting setting : bindedSettings) {
         settingList.add(new InterpreterSettingsList(setting.getId(), setting.getName(),
                 setting.getInterpreterInfos(), true));
@@ -1973,7 +1978,8 @@ public class NotebookServer extends WebSocketServlet
         continue;
       }
 
-      List<InterpreterSetting> intpSettings = note.getBindedInterpreterSettings();
+      List<InterpreterSetting> intpSettings =
+              note.getBindedInterpreterSettings(new ArrayList<>(note.getOwners()));
       if (intpSettings.isEmpty()) {
         continue;
       }
@@ -2031,11 +2037,18 @@ public class NotebookServer extends WebSocketServlet
         });
   }
 
-  private void getInterpreterSettings(NotebookSocket conn)
+  private void getInterpreterSettings(NotebookSocket conn, Message message)
       throws IOException {
-    List<InterpreterSetting> availableSettings = getNotebook().getInterpreterSettingManager().get();
+    ServiceContext context = getServiceContext(message);
+    List<InterpreterSetting> allSettings = getNotebook().getInterpreterSettingManager().get();
+    List<InterpreterSetting> result = new ArrayList<>();
+    for (InterpreterSetting setting : allSettings) {
+      if (setting.isUserAuthorized(new ArrayList<>(context.getUserAndRoles()))) {
+        result.add(setting);
+      }
+    }
     conn.send(serializeMessage(
-        new Message(OP.INTERPRETER_SETTINGS).put("interpreterSettings", availableSettings)));
+        new Message(OP.INTERPRETER_SETTINGS).put("interpreterSettings", result)));
   }
 
   @Override
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java
index fa9f001..9819b87 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java
@@ -255,7 +255,7 @@ public class InterpreterRestApiTest extends AbstractTestRestApi {
       assertEquals(p.getReturn().message().get(0).getData(), getSimulatedMarkdownResult("markdown"));
 
       // when: restart interpreter
-      for (InterpreterSetting setting : note.getBindedInterpreterSettings()) {
+      for (InterpreterSetting setting : note.getBindedInterpreterSettings(new ArrayList<>())) {
         if (setting.getName().equals("md")) {
           // call restart interpreter API
           PutMethod put = httpPut("/interpreter/setting/restart/" + setting.getId(), "");
@@ -308,7 +308,7 @@ public class InterpreterRestApiTest extends AbstractTestRestApi {
 
       // when: get md interpreter
       InterpreterSetting mdIntpSetting = null;
-      for (InterpreterSetting setting : note.getBindedInterpreterSettings()) {
+      for (InterpreterSetting setting : note.getBindedInterpreterSettings(new ArrayList<>())) {
         if (setting.getName().equals("md")) {
           mdIntpSetting = setting;
           break;
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java
index 69df579..f035c6c 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java
@@ -36,6 +36,7 @@ import static org.mockito.Mockito.when;
 import java.io.IOException;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
@@ -273,7 +274,7 @@ public class NotebookServerTest extends AbstractTestRestApi {
 
     // get reference to interpreterGroup
     InterpreterGroup interpreterGroup = null;
-    List<InterpreterSetting> settings = note1.getBindedInterpreterSettings();
+    List<InterpreterSetting> settings = note1.getBindedInterpreterSettings(new ArrayList<>());
     for (InterpreterSetting setting : settings) {
       if (setting.getName().equals("angular")) {
         interpreterGroup = setting.getOrCreateInterpreterGroup("anonymous", "sharedProcess");
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java
index ec0caa2..0f902f6 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java
@@ -18,6 +18,7 @@
 package org.apache.zeppelin.notebook;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.Lists;
 import com.google.gson.ExclusionStrategy;
 import com.google.gson.FieldAttributes;
 import com.google.gson.Gson;
@@ -920,7 +921,7 @@ public class Note implements JsonSerializable {
   private void snapshotAngularObjectRegistry(String user) {
     angularObjects = new HashMap<>();
 
-    List<InterpreterSetting> settings = getBindedInterpreterSettings();
+    List<InterpreterSetting> settings = getBindedInterpreterSettings(Lists.newArrayList(user));
     if (settings == null || settings.size() == 0) {
       return;
     }
@@ -937,7 +938,7 @@ public class Note implements JsonSerializable {
   private void removeAllAngularObjectInParagraph(String user, String paragraphId) {
     angularObjects = new HashMap<>();
 
-    List<InterpreterSetting> settings = getBindedInterpreterSettings();
+    List<InterpreterSetting> settings = getBindedInterpreterSettings(Lists.newArrayList(user));
     if (settings == null || settings.size() == 0) {
       return;
     }
@@ -975,7 +976,7 @@ public class Note implements JsonSerializable {
     }
   }
 
-  public List<InterpreterSetting> getBindedInterpreterSettings() {
+  public List<InterpreterSetting> getBindedInterpreterSettings(List<String> userAndRoles) {
     // use LinkedHashSet because order matters, the first one represent the default interpreter setting.
     Set<InterpreterSetting> settings = new LinkedHashSet<>();
     // add the default interpreter group
@@ -987,7 +988,9 @@ public class Note implements JsonSerializable {
     // add the interpreter setting with the same group of default interpreter group
     for (InterpreterSetting intpSetting : interpreterSettingManager.get()) {
       if (intpSetting.getGroup().equals(defaultIntpSetting.getGroup())) {
-        settings.add(intpSetting);
+        if (intpSetting.isUserAuthorized(userAndRoles)) {
+          settings.add(intpSetting);
+        }
       }
     }
 
@@ -995,8 +998,11 @@ public class Note implements JsonSerializable {
     for (Paragraph p : getParagraphs()) {
       try {
         Interpreter intp = p.getBindedInterpreter();
-        settings.add((
-                (ManagedInterpreterGroup) intp.getInterpreterGroup()).getInterpreterSetting());
+        InterpreterSetting interpreterSetting = (
+                (ManagedInterpreterGroup) intp.getInterpreterGroup()).getInterpreterSetting();
+        if (interpreterSetting.isUserAuthorized(userAndRoles)) {
+          settings.add(interpreterSetting);
+        }
       } catch (InterpreterNotFoundException e) {
         // ignore this
       }
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java
index ac7bd5b..a39c83e 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/helium/HeliumApplicationFactoryTest.java
@@ -21,6 +21,8 @@ import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 
 import java.io.IOException;
+import java.util.ArrayList;
+
 import org.apache.zeppelin.interpreter.AbstractInterpreterTest;
 import org.apache.zeppelin.interpreter.Interpreter;
 import org.apache.zeppelin.interpreter.InterpreterException;
@@ -233,7 +235,7 @@ public class HeliumApplicationFactoryTest extends AbstractInterpreterTest {
 
     Note note1 = notebook.createNote("note1", anonymous);
     String mock1IntpSettingId = null;
-    for (InterpreterSetting setting : note1.getBindedInterpreterSettings()) {
+    for (InterpreterSetting setting : note1.getBindedInterpreterSettings(new ArrayList<>())) {
       if (setting.getName().equals("mock1")) {
         mock1IntpSettingId = setting.getId();
         break;
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
index b5f8866..94956ae 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
@@ -916,7 +916,7 @@ public class NotebookTest extends AbstractInterpreterTest implements ParagraphJo
     // create a note and a paragraph
     Note note = notebook.createNote("note1", anonymous);
 
-    AngularObjectRegistry registry = note.getBindedInterpreterSettings().get(0).getOrCreateInterpreterGroup(anonymous.getUser(), "sharedProcess")
+    AngularObjectRegistry registry = note.getBindedInterpreterSettings(new ArrayList<>()).get(0).getOrCreateInterpreterGroup(anonymous.getUser(), "sharedProcess")
         .getAngularObjectRegistry();
 
     Paragraph p1 = note.addNewParagraph(AuthenticationInfo.ANONYMOUS);
@@ -947,7 +947,7 @@ public class NotebookTest extends AbstractInterpreterTest implements ParagraphJo
     // create a note and a paragraph
     Note note = notebook.createNote("note1", anonymous);
 
-    AngularObjectRegistry registry = note.getBindedInterpreterSettings().get(0).getOrCreateInterpreterGroup(anonymous.getUser(), "sharedProcess")
+    AngularObjectRegistry registry = note.getBindedInterpreterSettings(new ArrayList<>()).get(0).getOrCreateInterpreterGroup(anonymous.getUser(), "sharedProcess")
         .getAngularObjectRegistry();
 
     Paragraph p1 = note.addNewParagraph(AuthenticationInfo.ANONYMOUS);
@@ -979,7 +979,7 @@ public class NotebookTest extends AbstractInterpreterTest implements ParagraphJo
     // create a note and a paragraph
     Note note = notebook.createNote("note1", anonymous);
 
-    AngularObjectRegistry registry = note.getBindedInterpreterSettings().get(0).getOrCreateInterpreterGroup(anonymous.getUser(), "sharedProcess")
+    AngularObjectRegistry registry = note.getBindedInterpreterSettings(new ArrayList<>()).get(0).getOrCreateInterpreterGroup(anonymous.getUser(), "sharedProcess")
         .getAngularObjectRegistry();
 
     // add local scope object
@@ -988,8 +988,8 @@ public class NotebookTest extends AbstractInterpreterTest implements ParagraphJo
     registry.add("o2", "object2", null, null);
 
     // restart interpreter
-    interpreterSettingManager.restart(note.getBindedInterpreterSettings().get(0).getId());
-    registry = note.getBindedInterpreterSettings().get(0)
+    interpreterSettingManager.restart(note.getBindedInterpreterSettings(new ArrayList<>()).get(0).getId());
+    registry = note.getBindedInterpreterSettings(new ArrayList<>()).get(0)
         .getOrCreateInterpreterGroup(anonymous.getUser(), "sharedProcess")
         .getAngularObjectRegistry();
 
@@ -1195,7 +1195,7 @@ public class NotebookTest extends AbstractInterpreterTest implements ParagraphJo
     p1.setAuthenticationInfo(anonymous);
 
     // restart interpreter with per user session enabled
-    for (InterpreterSetting setting : note1.getBindedInterpreterSettings()) {
+    for (InterpreterSetting setting : note1.getBindedInterpreterSettings(new ArrayList<>())) {
       setting.getOption().setPerNote(setting.getOption().SCOPED);
       notebook.getInterpreterSettingManager().restart(setting.getId());
     }
@@ -1243,7 +1243,7 @@ public class NotebookTest extends AbstractInterpreterTest implements ParagraphJo
 
 
     // restart interpreter with per note session enabled
-    for (InterpreterSetting setting : note1.getBindedInterpreterSettings()) {
+    for (InterpreterSetting setting : note1.getBindedInterpreterSettings(new ArrayList<>())) {
       setting.getOption().setPerNote(InterpreterOption.SCOPED);
       notebook.getInterpreterSettingManager().restart(setting.getId());
     }
@@ -1286,7 +1286,7 @@ public class NotebookTest extends AbstractInterpreterTest implements ParagraphJo
     assertEquals(p1.getReturn().message().get(0).getData(), p2.getReturn().message().get(0).getData());
 
     // restart interpreter with scoped mode enabled
-    for (InterpreterSetting setting : note1.getBindedInterpreterSettings()) {
+    for (InterpreterSetting setting : note1.getBindedInterpreterSettings(new ArrayList<>())) {
       setting.getOption().setPerNote(InterpreterOption.SCOPED);
       notebook.getInterpreterSettingManager().restart(setting.getId());
     }
@@ -1301,7 +1301,7 @@ public class NotebookTest extends AbstractInterpreterTest implements ParagraphJo
     assertNotEquals(p1.getReturn().message().get(0).getData(), p2.getReturn().message().get(0).getData());
 
     // restart interpreter with isolated mode enabled
-    for (InterpreterSetting setting : note1.getBindedInterpreterSettings()) {
+    for (InterpreterSetting setting : note1.getBindedInterpreterSettings(new ArrayList<>())) {
       setting.getOption().setPerNote(InterpreterOption.ISOLATED);
       setting.getInterpreterSettingManager().restart(setting.getId());
     }