[ZEPPELIN-1718] Prevent anonymous user to set note permission

zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java

@@ -93,7 +93,9 @@ public NotebookRestApi(Notebook notebook, NotebookServer notebookServer, SearchS
   @GET
   @Path("{noteId}/permissions")
   @ZeppelinApi
-  public Response getNotePermissions(@PathParam("noteId") String noteId) {
+  public Response getNotePermissions(@PathParam("noteId") String noteId) throws IOException {
+
+    checkIfUserIsAnon(blockNotAuthenticatedUserError());
     checkIfUserCanRead(noteId,
         "Insufficient privileges you cannot get the list of permissions for this note");
     HashMap<String, Set<String>> permissionsMap = new HashMap<>();
@@ -111,12 +113,27 @@ private String ownerPermissionError(Set<String> current, Set<String> allowed) th
         "User belongs to: " + current.toString();
   }
 
+  private String blockNotAuthenticatedUserError() throws IOException {
+    LOG.info("Anonymous user cannot set any permissions for this note.");
+    return  "Only authenticated user can set the permission.";
+  }
+
   /**
    * Set of utils method to check if current user can perform action to the note.
    * Since we only have security on notebook level, from now we keep this logic in this class.
    * In the future we might want to generalize this for the rest of the api enmdpoints.
    */
-
+
+  /**
+   * Check if the current user is not authenticated(anonymous user) or not
+   */
+  private void checkIfUserIsAnon(String errorMsg) {
+    boolean isAuthenticated = SecurityUtils.isAuthenticated();
+    if (!isAuthenticated) {
+      throw new ForbiddenException(errorMsg);
+    }
+  }
+
   /**
    * Check if the current user own the given note.
    */
@@ -178,7 +195,8 @@ public Response putNotePermissions(@PathParam("noteId") String noteId, String re
     HashSet<String> userAndRoles = new HashSet<>();
     userAndRoles.add(principal);
     userAndRoles.addAll(roles);
-
+
+    checkIfUserIsAnon(blockNotAuthenticatedUserError());
     checkIfUserIsOwner(noteId,
         ownerPermissionError(userAndRoles, notebookAuthorization.getOwners(noteId)));
 

zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java

@@ -69,70 +69,6 @@ public void setUp() {
     anonymous = new AuthenticationInfo("anonymous");
   }
 
-  @Test
-  public void testPermissions() throws IOException {
-    Note note1 = ZeppelinServer.notebook.createNote(anonymous);
-    // Set only readers
-    String jsonRequest = "{\"readers\":[\"admin-team\"],\"owners\":[]," +
-            "\"writers\":[]}";
-    PutMethod put = httpPut("/notebook/" + note1.getId() + "/permissions/", jsonRequest);
-    LOG.info("testPermissions response\n" + put.getResponseBodyAsString());
-    assertThat("test update method:", put, isAllowed());
-    put.releaseConnection();
-
-
-    GetMethod get = httpGet("/notebook/" + note1.getId() + "/permissions/");
-    assertThat(get, isAllowed());
-    Map<String, Object> resp = gson.fromJson(get.getResponseBodyAsString(), new TypeToken<Map<String, Object>>() {
-    }.getType());
-    Map<String, Set<String>> authInfo = (Map<String, Set<String>>) resp.get("body");
-
-    // Check that both owners and writers is set to the princpal if empty
-    assertEquals(authInfo.get("readers"), Lists.newArrayList("admin-team"));
-    assertEquals(authInfo.get("owners"), Lists.newArrayList("anonymous"));
-    assertEquals(authInfo.get("writers"), Lists.newArrayList("anonymous"));
-    get.releaseConnection();
-
-
-    Note note2 = ZeppelinServer.notebook.createNote(anonymous);
-    // Set only writers
-    jsonRequest = "{\"readers\":[],\"owners\":[]," +
-        "\"writers\":[\"admin-team\"]}";
-    put = httpPut("/notebook/" + note2.getId() + "/permissions/", jsonRequest);
-    assertThat("test update method:", put, isAllowed());
-    put.releaseConnection();
-
-    get = httpGet("/notebook/" + note2.getId() + "/permissions/");
-    assertThat(get, isAllowed());
-    resp = gson.fromJson(get.getResponseBodyAsString(), new TypeToken<Map<String, Object>>() {
-    }.getType());
-    authInfo = (Map<String, Set<String>>) resp.get("body");
-    // Check that owners is set to the princpal if empty
-    assertEquals(authInfo.get("owners"), Lists.newArrayList("anonymous"));
-    assertEquals(authInfo.get("writers"), Lists.newArrayList("admin-team"));
-    get.releaseConnection();
-
-
-    // Test clear permissions
-    jsonRequest = "{\"readers\":[],\"owners\":[],\"writers\":[]}";
-    put = httpPut("/notebook/" + note2.getId() + "/permissions/", jsonRequest);
-    put.releaseConnection();
-    get = httpGet("/notebook/" + note2.getId() + "/permissions/");
-    assertThat(get, isAllowed());
-    resp = gson.fromJson(get.getResponseBodyAsString(), new TypeToken<Map<String, Object>>() {
-    }.getType());
-    authInfo = (Map<String, Set<String>>) resp.get("body");
-
-    assertEquals(authInfo.get("readers"), Lists.newArrayList());
-    assertEquals(authInfo.get("writers"), Lists.newArrayList());
-    assertEquals(authInfo.get("owners"), Lists.newArrayList());
-    get.releaseConnection();
-    //cleanup
-    ZeppelinServer.notebook.removeNote(note1.getId(), anonymous);
-    ZeppelinServer.notebook.removeNote(note2.getId(), anonymous);
-
-  }
-
   @Test
   public void testGetNoteParagraphJobStatus() throws IOException {
     Note note1 = ZeppelinServer.notebook.createNote(anonymous);
@@ -273,5 +209,3 @@ public void testClearAllParagraphOutput() throws IOException {
     ZeppelinServer.notebook.removeNote(note.getId(), anonymous);
   }
 }
-
-

zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookSecurityRestApiTest.java

@@ -17,11 +17,11 @@
 package org.apache.zeppelin.rest;
 
 import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertThat;
+import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Map;
 
 import org.apache.commons.httpclient.HttpMethodBase;
@@ -56,6 +56,7 @@ public static void destroy() throws Exception {
 
   @Before
   public void setUp() {}
+
 
   @Test
   public void testThatUserCanCreateAndRemoveNote() throws IOException {
@@ -108,7 +109,25 @@ public void testThatWriterCannotRemoveNote() throws IOException {
     Note deletedNote = ZeppelinServer.notebook.getNote(noteId);
     assertNull("Deleted note should be null", deletedNote);
   }
-
+
+  @Test
+  public void testThatUserCanSearchNote() throws IOException {
+    String noteId1 = createNoteForUser("test1", "admin", "password1");
+    createParagraphForUser(noteId1, "admin", "password1", "title1", "ThisIsToTestSearchMethodWithPermissions 1");
+
+    String noteId2 = createNoteForUser("test2", "user1", "password2");
+    createParagraphForUser(noteId1, "admin", "password1", "title2", "ThisIsToTestSearchMethodWithPermissions 2");
+
+    //set permission for each note
+    setPermissionForNote(noteId1, "admin", "password1");
+    setPermissionForNote(noteId1, "user1", "password2");
+
+    searchNoteBasedOnPermission("ThisIsToTestSearchMethodWithPermissions", "admin", "password1");
+
+    deleteNoteForUser(noteId1, "admin", "password1");
+    deleteNoteForUser(noteId2, "user1", "password2");
+  }
+
   private void userTryRemoveNote(String noteId, String user, String pwd, Matcher<? super HttpMethodBase> m) throws IOException {
     DeleteMethod delete = httpDelete(("/notebook/" + noteId), user, pwd);
     assertThat(delete, m);
@@ -153,4 +172,47 @@ private void deleteNoteForUser(String noteId, String user, String pwd) throws IO
       assertNull("Deleted note should be null", deletedNote);
     }
   }
+
+  private void createParagraphForUser(String noteId, String user, String pwd, String title, String text) throws IOException {
+    String payload = "{\"title\": \"" + title + "\",\"text\": \"" + text + "\"}";
+    PostMethod post = httpPost(("/notebook/" + noteId + "/paragraph"), payload, user, pwd);
+    post.releaseConnection();
+  }
+
+  private void setPermissionForNote(String noteId, String user, String pwd) throws IOException {
+    String payload = "{\"owners\":[\"" + user + "\"],\"readers\":[\"" + user + "\"],\"writers\":[\"" + user + "\"]}";
+    PutMethod put = httpPut(("/notebook/" + noteId + "/permissions"), payload, user, pwd);
+    put.releaseConnection();
+  }
+
+
+  private void searchNoteBasedOnPermission(String searchText, String user, String pwd) throws IOException{
+    GetMethod searchNote = httpGet(("/notebook/search?q=" + searchText), user, pwd);
+    Map<String, Object> respSearchResult = gson.fromJson(searchNote.getResponseBodyAsString(),
+      new TypeToken<Map<String, Object>>() {
+      }.getType());
+    ArrayList searchBody = (ArrayList) respSearchResult.get("body");
+    assertEquals("At-least one search results is there", true, searchBody.size() >= 1);
+
+    for (int i = 0; i < searchBody.size(); i++) {
+      Map<String, String> searchResult = (Map<String, String>) searchBody.get(i);
+      String userId = searchResult.get("id").split("/", 2)[0];
+
+      GetMethod getPermission = httpGet(("/notebook/" + userId + "/permissions"), user, pwd);
+      Map<String, Object> resp = gson.fromJson(getPermission.getResponseBodyAsString(),
+        new TypeToken<Map<String, Object>>() {
+        }.getType());
+      Map<String, ArrayList> permissions = (Map<String, ArrayList>) resp.get("body");
+      ArrayList owners = permissions.get("owners");
+      ArrayList readers = permissions.get("readers");
+      ArrayList writers = permissions.get("writers");
+
+      if (owners.size() != 0 && readers.size() != 0 && writers.size() != 0) {
+        assertEquals("User has permissions  ", true, (owners.contains(user) || readers.contains(user) ||
+          writers.contains(user)));
+      }
+      getPermission.releaseConnection();
+    }
+    searchNote.releaseConnection();
+  }
 }

zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java

@@ -705,70 +705,6 @@ public void testDeleteParagraph() throws IOException {
     ZeppelinServer.notebook.removeNote(note.getId(), anonymous);
   }
 
-  @Test
-  public void testSearch() throws IOException {
-    Map<String, String> body;
-
-    GetMethod getSecurityTicket = httpGet("/security/ticket");
-    getSecurityTicket.addRequestHeader("Origin", "http://localhost");
-    Map<String, Object> respSecurityTicket = gson.fromJson(getSecurityTicket.getResponseBodyAsString(),
-        new TypeToken<Map<String, Object>>() {
-        }.getType());
-    body = (Map<String, String>) respSecurityTicket.get("body");
-    String username = body.get("principal");
-    getSecurityTicket.releaseConnection();
-
-    Note note1 = ZeppelinServer.notebook.createNote(anonymous);
-    String jsonRequest = "{\"title\": \"title1\", \"text\": \"ThisIsToTestSearchMethodWithPermissions 1\"}";
-    PostMethod postNoteText = httpPost("/notebook/" + note1.getId() + "/paragraph", jsonRequest);
-    postNoteText.releaseConnection();
-
-    Note note2 = ZeppelinServer.notebook.createNote(anonymous);
-    jsonRequest = "{\"title\": \"title1\", \"text\": \"ThisIsToTestSearchMethodWithPermissions 2\"}";
-    postNoteText = httpPost("/notebook/" + note2.getId() + "/paragraph", jsonRequest);
-    postNoteText.releaseConnection();
-
-    String jsonPermissions = "{\"owners\":[\"" + username + "\"],\"readers\":[\"" + username + "\"],\"writers\":[\"" + username + "\"]}";
-    PutMethod putPermission = httpPut("/notebook/" + note1.getId() + "/permissions", jsonPermissions);
-    putPermission.releaseConnection();
-
-    jsonPermissions = "{\"owners\":[\"admin\"],\"readers\":[\"admin\"],\"writers\":[\"admin\"]}";
-    putPermission = httpPut("/notebook/" + note2.getId() + "/permissions", jsonPermissions);
-    putPermission.releaseConnection();
-
-    GetMethod searchNote = httpGet("/notebook/search?q='ThisIsToTestSearchMethodWithPermissions'");
-    searchNote.addRequestHeader("Origin", "http://localhost");
-    Map<String, Object> respSearchResult = gson.fromJson(searchNote.getResponseBodyAsString(),
-        new TypeToken<Map<String, Object>>() {
-        }.getType());
-    ArrayList searchBody = (ArrayList) respSearchResult.get("body");
-
-    assertEquals("At-least one search results is there", true, searchBody.size() >= 1);
-
-    for (int i = 0; i < searchBody.size(); i++) {
-      Map<String, String> searchResult = (Map<String, String>) searchBody.get(i);
-      String userId = searchResult.get("id").split("/", 2)[0];
-      GetMethod getPermission = httpGet("/notebook/" + userId + "/permissions");
-      getPermission.addRequestHeader("Origin", "http://localhost");
-      Map<String, Object> resp = gson.fromJson(getPermission.getResponseBodyAsString(),
-          new TypeToken<Map<String, Object>>() {
-          }.getType());
-      Map<String, ArrayList> permissions = (Map<String, ArrayList>) resp.get("body");
-      ArrayList owners = permissions.get("owners");
-      ArrayList readers = permissions.get("readers");
-      ArrayList writers = permissions.get("writers");
-
-      if (owners.size() != 0 && readers.size() != 0 && writers.size() != 0) {
-        assertEquals("User has permissions  ", true, (owners.contains(username) || readers.contains(username) ||
-            writers.contains(username)));
-      }
-      getPermission.releaseConnection();
-    }
-    searchNote.releaseConnection();
-    ZeppelinServer.notebook.removeNote(note1.getId(), anonymous);
-    ZeppelinServer.notebook.removeNote(note2.getId(), anonymous);
-  }
-
   @Test
   public void testTitleSearch() throws IOException {
     Note note = ZeppelinServer.notebook.createNote(anonymous);
@@ -796,4 +732,3 @@ public void testTitleSearch() throws IOException {
   }
 
 }
-

zeppelin-web/src/app/notebook/notebook.controller.js

@@ -91,6 +91,34 @@
       return value;
     };
 
+    $scope.blockAnonUsers = function() {
+      var principal = $rootScope.ticket.principal;
+      if (principal) {
+        $scope.isAnonymous = principal === 'anonymous' ? true : false;
+        if ($scope.isAnonymous) {
+          var zeppelinVersion = $rootScope.zeppelinVersion;
+          var url = 'https://zeppelin.apache.org/docs/' + zeppelinVersion + '/security/notebook_authorization.html';
+          var content = 'Only authenticated user can set the permission.' +
+            '<a data-toggle="tooltip" data-placement="top" title="Learn more" target="_blank" href=' + url + '>' +
+            '<i class="icon-question" />' +
+            '</a>';
+          BootstrapDialog.show({
+            closable: false,
+            closeByBackdrop: false,
+            closeByKeyboard: false,
+            title: 'No permission',
+            message: content,
+            buttons: [{
+              label: 'Close',
+              action: function(dialog) {
+                dialog.close();
+              }
+            }]
+          });
+        }
+      }
+    };
+
     /** Init the new controller */
     var initNotebook = function() {
       noteVarShareService.clear();
@@ -741,6 +769,7 @@
     };
 
     $scope.togglePermissions = function() {
+      $scope.blockAnonUsers();
       if ($scope.showPermissions) {
         $scope.closePermissions();
         angular.element('#selectOwners').select2({});

zeppelin-web/src/app/notebook/notebook.html

@@ -65,7 +65,7 @@ <h5>Interpreter binding</h5>
   </div>
 
   <!-- permissions -->
-  <div ng-if="showPermissions" class="permissions">
+  <div ng-if="showPermissions && ticket.principal && !isAnonymous" class="permissions">
     <div>
       <h4>Note Permissions (Only note owners can change)</h4>
     </div>