[zeppelin] branch master updated: [ZEPPELIN-5061] fix note list broadcast

[zj...@apache.org]

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 605d704  [ZEPPELIN-5061] fix note list broadcast
605d704 is described below

commit 605d7044d824cb863b5ec1f517781ff81562386f
Author: David Golightly <da...@leapyear.io>
AuthorDate: Mon Sep 28 09:28:46 2020 -0700

    [ZEPPELIN-5061] fix note list broadcast
    
    ### What is this PR for?
    As described in the linked issue, there has been a bug where, when a user creates or updates a note, that user's note list is published to all connected users, instead of their own note lists. This has the security problem that it leaks information about a user's note list to other users who may not have permission to see it. This PR fixes that bug by querying each connected user's note list and publishing that instead.
    
    ### What type of PR is it?
    Bug Fix
    
    ### What is the Jira issue?
    https://issues.apache.org/jira/browse/ZEPPELIN-5061
    
    ### How should this be tested?
    Manual testing:
    * Configure a list of users using a `shiro.ini` file. Example:
    ```
    [users]
    # List of users with their password allowed to access Zeppelin.
    # To use a different strategy (LDAP / Database / ...) check the shiro doc at http://shiro.apache.org/configuration.html#Configuration-INISections
    admin = password1, admin
    user1 = password2, role1
    user2 = password3, role1
    ```
    * Turn off public notebooks in `zeppelin-site.xml` by setting the property `zeppelin.notebook.public` to `false`
    * Boot Zeppelin server on this PR
    * Log in as user1
    * In a separate browser, log in as user2
    * As user1, create a note
    Previous result: user1's note appears on user2's dashboard. Refreshing user2's browser will cause the note to disappear from their dashboard.
    Fixed expectations:
    - user1's note does not appear on user2's dashboard.
    - the display of any notes created by user1 is not affected when user2 creates a note
    - when `zeppelin.notebook.public` is set to `true`, user1's note appears as expected on user1's dashboard
    
    Author: David Golightly <da...@leapyear.io>
    
    Closes #3926 from david-golightly-leapyear/dg/notebook-sharing-fix and squashes the following commits:
    
    175285e9c [David Golightly] address feedback
    4f0023143 [David Golightly] get variable outside of loop
    99306186c [David Golightly] remove unused import
    e9926e6ca [David Golightly] fix note list broadcast
    
    (cherry picked from commit 596c287b744989451e09e7747218f3cd99e6b140)
    Signed-off-by: Jeff Zhang <zj...@apache.org>
---
 .../apache/zeppelin/socket/ConnectionManager.java  | 12 +++++++
 .../org/apache/zeppelin/socket/NotebookServer.java | 39 ++++++++++------------
 2 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/ConnectionManager.java b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/ConnectionManager.java
index fa99501..e7f99aa 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/ConnectionManager.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/ConnectionManager.java
@@ -360,6 +360,18 @@ public class ConnectionManager {
     }
   }
 
+  public interface UserIterator {
+    public void handleUser(String user, Set<String> userAndRoles);
+  }
+
+  public void forAllUsers(UserIterator iterator) {
+    for (String user : userSocketMap.keySet()) {
+      Set<String> userAndRoles = authorizationService.getRoles(user);
+      userAndRoles.add(user);
+      iterator.handleUser(user, userAndRoles);
+    }
+  }
+
   public void broadcastNoteListExcept(List<NoteInfo> notesInfo,
                                       AuthenticationInfo subject) {
     Set<String> userAndRoles;
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 45e1abe..2c7ece1 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
@@ -81,6 +81,7 @@ import org.apache.zeppelin.service.JobManagerService;
 import org.apache.zeppelin.service.NotebookService;
 import org.apache.zeppelin.service.ServiceContext;
 import org.apache.zeppelin.service.SimpleServiceCallback;
+import org.apache.zeppelin.socket.ConnectionManager.UserIterator;
 import org.apache.zeppelin.ticket.TicketContainer;
 import org.apache.zeppelin.types.InterpreterSettingsList;
 import org.apache.zeppelin.user.AuthenticationInfo;
@@ -636,17 +637,22 @@ public class NotebookServer extends WebSocketServlet
   }
 
   public void inlineBroadcastNoteList(AuthenticationInfo subject, Set<String> userAndRoles) {
-    if (subject == null) {
-      subject = new AuthenticationInfo(StringUtils.EMPTY);
-    }
-    //send first to requesting user
+    broadcastNoteListUpdate();
+  }
+
+  public void broadcastNoteListUpdate() {
     AuthorizationService authorizationService = getNotebookAuthorizationService();
-    List<NoteInfo> notesInfo = getNotebook().getNotesInfo(
-        noteId -> authorizationService.isReader(noteId, userAndRoles));
-    Message message = new Message(OP.NOTES_INFO).put("notes", notesInfo);
-    getConnectionManager().multicastToUser(subject.getUser(), message);
-    //to others afterwards
-    getConnectionManager().broadcastNoteListExcept(notesInfo, subject);
+
+    getConnectionManager().forAllUsers(new UserIterator() {
+      @Override
+      public void handleUser(String user, Set<String> userAndRoles) {
+        List<NoteInfo> notesInfo = getNotebook().getNotesInfo(
+            noteId -> authorizationService.isReader(noteId, userAndRoles));
+
+        getConnectionManager().multicastToUser(user,
+          new Message(OP.NOTES_INFO).put("notes", notesInfo));
+      }
+    });
   }
 
   public void broadcastNoteList(AuthenticationInfo subject, Set<String> userAndRoles) {
@@ -772,18 +778,7 @@ public class NotebookServer extends WebSocketServlet
 
   public void broadcastReloadedNoteList(NotebookSocket conn, ServiceContext context)
       throws IOException {
-    getNotebookService().listNotesInfo(true, context,
-        new WebSocketServiceCallback<List<NoteInfo>>(conn) {
-          @Override
-          public void onSuccess(List<NoteInfo> notesInfo,
-                                ServiceContext context) throws IOException {
-            super.onSuccess(notesInfo, context);
-            getConnectionManager().multicastToUser(context.getAutheInfo().getUser(),
-                new Message(OP.NOTES_INFO).put("notes", notesInfo));
-            //to others afterwards
-            getConnectionManager().broadcastNoteListExcept(notesInfo, context.getAutheInfo());
-          }
-        });
+    broadcastNoteListUpdate();
   }
 
   void permissionError(NotebookSocket conn, String op, String userName, Set<String> userAndRoles,