This is an automated email from the ASF dual-hosted git repository. zjffdu pushed a commit to branch branch-0.9 in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/branch-0.9 by this push: new 596c287 [ZEPPELIN-5061] fix note list broadcast 596c287 is described below commit 596c287b744989451e09e7747218f3cd99e6b140 Author: David Golightly <david.goligh...@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 <david.goligh...@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 --- .../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,