Repository: zeppelin Updated Branches: refs/heads/master 908b2a74f -> 9e9ea3aea
[ZEPPELIN-1483] Zeppelin home page list notebooks doesn't show notebook with group permission ### What is this PR for? Zeppelin home page list notebooks doesn't show notebook with group permission ### What type of PR is it? [Bug Fix] ### Todos * [x] - consume userAndRole instead of AuthenticationInfo ### What is the Jira issue? * [ZEPPELIN-1483](https://issues.apache.org/jira/browse/ZEPPELIN-1483) ### How should this be tested? In current scenario only those notebook lists that have direct user permission, those with group does not list up, but if user have link to those notebook, it can still be accessed. IMO the notebook with group permission should also be listed in the home screen. ### Screenshots (if appropriate)  ### Questions: * Does the licenses files need update? n/a * Is there breaking changes for older versions? n/a * Does this needs documentation? n/a Author: Prabhjyot Singh <[email protected]> Author: Prabhjyot Singh <[email protected]> Closes #1454 from prabhjyotsingh/ZEPPELIN-1483 and squashes the following commits: 2484833 [Prabhjyot Singh] Merge remote-tracking branch 'origin/master' into ZEPPELIN-1483 c8d810e [Prabhjyot Singh] organise imports d3261c4 [Prabhjyot Singh] consume userAndRole instead of AuthenticationInfo Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/9e9ea3ae Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/9e9ea3ae Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/9e9ea3ae Branch: refs/heads/master Commit: 9e9ea3aea0a4e4ffaa87e0783fc85fb18cdc9887 Parents: 908b2a7 Author: Prabhjyot Singh <[email protected]> Authored: Thu Oct 20 12:15:29 2016 +0530 Committer: Prabhjyot Singh <[email protected]> Committed: Thu Oct 20 14:25:06 2016 +0530 ---------------------------------------------------------------------- .../apache/zeppelin/rest/NotebookRestApi.java | 12 +++---- .../apache/zeppelin/socket/NotebookServer.java | 36 ++++++++++---------- .../zeppelin/integration/AuthenticationIT.java | 2 +- .../zeppelin/rest/ZeppelinRestApiTest.java | 6 ++-- .../org/apache/zeppelin/notebook/Notebook.java | 6 ++-- .../apache/zeppelin/notebook/NotebookTest.java | 20 +++++------ 6 files changed, 42 insertions(+), 40 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/9e9ea3ae/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java index d9af812..d581404 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java @@ -39,7 +39,6 @@ import com.google.common.reflect.TypeToken; import com.google.gson.Gson; import org.apache.commons.lang3.StringUtils; import org.apache.zeppelin.interpreter.InterpreterResult; -import org.apache.zeppelin.scheduler.Job; import org.apache.zeppelin.utils.InterpreterBindingUtils; import org.quartz.CronExpression; import org.slf4j.Logger; @@ -162,7 +161,7 @@ public class NotebookRestApi { AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal()); note.persist(subject); notebookServer.broadcastNote(note); - notebookServer.broadcastNoteList(subject); + notebookServer.broadcastNoteList(subject, userAndRoles); return new JsonResponse<>(Status.OK).build(); } @@ -199,7 +198,8 @@ public class NotebookRestApi { @ZeppelinApi public Response getNotebookList() throws IOException { AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal()); - List<Map<String, String>> notesInfo = notebookServer.generateNotebooksInfo(false, subject); + List<Map<String, String>> notesInfo = notebookServer.generateNotebooksInfo(false, subject, + SecurityUtils.getRoles()); return new JsonResponse<>(Status.OK, "", notesInfo).build(); } @@ -278,7 +278,7 @@ public class NotebookRestApi { note.setName(noteName); note.persist(subject); notebookServer.broadcastNote(note); - notebookServer.broadcastNoteList(subject); + notebookServer.broadcastNoteList(subject, SecurityUtils.getRoles()); return new JsonResponse<>(Status.CREATED, "", note.getId()).build(); } @@ -302,7 +302,7 @@ public class NotebookRestApi { } } - notebookServer.broadcastNoteList(subject); + notebookServer.broadcastNoteList(subject, SecurityUtils.getRoles()); return new JsonResponse<>(Status.OK, "").build(); } @@ -327,7 +327,7 @@ public class NotebookRestApi { AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal()); Note newNote = notebook.cloneNote(notebookId, newNoteName, subject); notebookServer.broadcastNote(newNote); - notebookServer.broadcastNoteList(subject); + notebookServer.broadcastNoteList(subject, SecurityUtils.getRoles()); return new JsonResponse<>(Status.CREATED, "", newNote.getId()).build(); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/9e9ea3ae/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java ---------------------------------------------------------------------- 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 969bdf9..2f9faba 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 @@ -20,7 +20,6 @@ import com.google.common.base.Strings; import com.google.gson.Gson; import com.google.gson.GsonBuilder; import com.google.gson.reflect.TypeToken; - import org.apache.commons.lang.StringUtils; import org.apache.commons.vfs2.FileSystemException; import org.apache.zeppelin.conf.ZeppelinConfiguration; @@ -172,10 +171,10 @@ public class NotebookServer extends WebSocketServlet implements /** Lets be elegant here */ switch (messagereceived.op) { case LIST_NOTES: - unicastNoteList(conn, subject); + unicastNoteList(conn, subject, userAndRoles); break; case RELOAD_NOTES_FROM_REPO: - broadcastReloadedNoteList(subject); + broadcastReloadedNoteList(subject, userAndRoles); break; case GET_HOME_NOTE: sendHomeNote(conn, userAndRoles, notebook, messagereceived); @@ -491,7 +490,7 @@ public class NotebookServer extends WebSocketServlet implements } public List<Map<String, String>> generateNotebooksInfo(boolean needsReload, - AuthenticationInfo subject) { + AuthenticationInfo subject, HashSet<String> userAndRoles) { Notebook notebook = notebook(); @@ -508,7 +507,7 @@ public class NotebookServer extends WebSocketServlet implements } } - List<Note> notes = notebook.getAllNotes(subject); + List<Note> notes = notebook.getAllNotes(userAndRoles); List<Map<String, String>> notesInfo = new LinkedList<>(); for (Note note : notes) { Map<String, String> info = new HashMap<>(); @@ -535,34 +534,35 @@ public class NotebookServer extends WebSocketServlet implements .put("interpreterBindings", settingList)); } - public void broadcastNoteList(AuthenticationInfo subject) { + public void broadcastNoteList(AuthenticationInfo subject, HashSet userAndRoles) { if (subject == null) { subject = new AuthenticationInfo(StringUtils.EMPTY); } //send first to requesting user - List<Map<String, String>> notesInfo = generateNotebooksInfo(false, subject); + List<Map<String, String>> notesInfo = generateNotebooksInfo(false, subject, userAndRoles); multicastToUser(subject.getUser(), new Message(OP.NOTES_INFO).put("notes", notesInfo)); //to others afterwards for (String user: userConnectedSockets.keySet()) { if (subject.getUser() == user) { continue; } - notesInfo = generateNotebooksInfo(false, new AuthenticationInfo(user)); + notesInfo = generateNotebooksInfo(false, new AuthenticationInfo(user), userAndRoles); multicastToUser(user, new Message(OP.NOTES_INFO).put("notes", notesInfo)); } } - public void unicastNoteList(NotebookSocket conn, AuthenticationInfo subject) { - List<Map<String, String>> notesInfo = generateNotebooksInfo(false, subject); + public void unicastNoteList(NotebookSocket conn, AuthenticationInfo subject, + HashSet<String> userAndRoles) { + List<Map<String, String>> notesInfo = generateNotebooksInfo(false, subject, userAndRoles); unicast(new Message(OP.NOTES_INFO).put("notes", notesInfo), conn); } - public void broadcastReloadedNoteList(AuthenticationInfo subject) { + public void broadcastReloadedNoteList(AuthenticationInfo subject, HashSet userAndRoles) { if (subject == null) { subject = new AuthenticationInfo(StringUtils.EMPTY); } //reload and reply first to requesting user - List<Map<String, String>> notesInfo = generateNotebooksInfo(true, subject); + List<Map<String, String>> notesInfo = generateNotebooksInfo(true, subject, userAndRoles); multicastToUser(subject.getUser(), new Message(OP.NOTES_INFO).put("notes", notesInfo)); //to others afterwards for (String user: userConnectedSockets.keySet()) { @@ -570,7 +570,7 @@ public class NotebookServer extends WebSocketServlet implements continue; } //reloaded already above; parameter - false - notesInfo = generateNotebooksInfo(false, new AuthenticationInfo(user)); + notesInfo = generateNotebooksInfo(false, new AuthenticationInfo(user), userAndRoles); multicastToUser(user, new Message(OP.NOTES_INFO).put("notes", notesInfo)); } } @@ -678,7 +678,7 @@ public class NotebookServer extends WebSocketServlet implements AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal); note.persist(subject); broadcastNote(note); - broadcastNoteList(subject); + broadcastNoteList(subject, userAndRoles); } } @@ -713,7 +713,7 @@ public class NotebookServer extends WebSocketServlet implements note.persist(subject); addConnectionToNote(note.getId(), (NotebookSocket) conn); conn.send(serializeMessage(new Message(OP.NEW_NOTE).put("note", note))); - broadcastNoteList(subject); + broadcastNoteList(subject, userAndRoles); } private void removeNote(NotebookSocket conn, HashSet<String> userAndRoles, @@ -735,7 +735,7 @@ public class NotebookServer extends WebSocketServlet implements AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal); notebook.removeNote(noteId, subject); removeNote(noteId); - broadcastNoteList(subject); + broadcastNoteList(subject, userAndRoles); } private void updateParagraph(NotebookSocket conn, HashSet<String> userAndRoles, @@ -777,7 +777,7 @@ public class NotebookServer extends WebSocketServlet implements AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal); addConnectionToNote(newNote.getId(), (NotebookSocket) conn); conn.send(serializeMessage(new Message(OP.NEW_NOTE).put("note", newNote))); - broadcastNoteList(subject); + broadcastNoteList(subject, userAndRoles); } protected Note importNote(NotebookSocket conn, HashSet<String> userAndRoles, @@ -796,7 +796,7 @@ public class NotebookServer extends WebSocketServlet implements note = notebook.importNote(noteJson, noteName, subject); note.persist(subject); broadcastNote(note); - broadcastNoteList(subject); + broadcastNoteList(subject, userAndRoles); } return note; } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/9e9ea3ae/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java index 6001429..e1a6f9e 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java @@ -202,7 +202,7 @@ public class AuthenticationIT extends AbstractZeppelinIT { try { WebElement element = pollingWait(By.xpath("//*[@id='notebook-names']//a[contains(@href, '" + noteId + "')]"), MAX_BROWSER_TIMEOUT_SEC); - collector.checkThat("Check is user has permission to view this notebook link", false, + collector.checkThat("Check is user has permission to view this notebook link", true, CoreMatchers.equalTo(element.isDisplayed())); } catch (Exception e) { //This should have failed, nothing to worry. http://git-wip-us.apache.org/repos/asf/zeppelin/blob/9e9ea3ae/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java index c2606f8..3b92c14 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java @@ -19,9 +19,11 @@ package org.apache.zeppelin.rest; import java.io.IOException; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; +import com.google.common.collect.Sets; import org.apache.commons.httpclient.methods.DeleteMethod; import org.apache.commons.httpclient.methods.GetMethod; import org.apache.commons.httpclient.methods.PostMethod; @@ -352,8 +354,8 @@ public class ZeppelinRestApiTest extends AbstractTestRestApi { }.getType()); List<Map<String, String>> body = (List<Map<String, String>>) resp.get("body"); //TODO(khalid): anonymous or specific user notes? - AuthenticationInfo subject = new AuthenticationInfo("anonymous"); - assertEquals("List notebooks are equal", ZeppelinServer.notebook.getAllNotes(subject).size(), body.size()); + HashSet<String> anonymous = Sets.newHashSet("anonymous"); + assertEquals("List notebooks are equal", ZeppelinServer.notebook.getAllNotes(anonymous).size(), body.size()); get.releaseConnection(); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/9e9ea3ae/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java index d996488..9ad6d4c 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java @@ -540,10 +540,10 @@ public class Notebook implements NoteEventListener { } } - public List<Note> getAllNotes(AuthenticationInfo subject) { + public List<Note> getAllNotes(HashSet<String> userAndRoles) { final Set<String> entities = Sets.newHashSet(); - if (subject != null) { - entities.add(subject.getUser()); + if (userAndRoles != null) { + entities.addAll(userAndRoles); } synchronized (notes) { http://git-wip-us.apache.org/repos/asf/zeppelin/blob/9e9ea3ae/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java ---------------------------------------------------------------------- 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 d0af2c9..87e2415 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 @@ -885,20 +885,20 @@ public class NotebookTest implements JobListenerFactory{ public void testGetAllNotes() throws Exception { Note note1 = notebook.createNote(anonymous); Note note2 = notebook.createNote(anonymous); - assertEquals(2, notebook.getAllNotes(anonymous).size()); + assertEquals(2, notebook.getAllNotes(Sets.newHashSet("anonymous")).size()); notebook.getNotebookAuthorization().setOwners(note1.getId(), Sets.newHashSet("user1")); notebook.getNotebookAuthorization().setWriters(note1.getId(), Sets.newHashSet("user1")); notebook.getNotebookAuthorization().setReaders(note1.getId(), Sets.newHashSet("user1")); - assertEquals(1, notebook.getAllNotes(anonymous).size()); - assertEquals(2, notebook.getAllNotes(new AuthenticationInfo("user1")).size()); + assertEquals(1, notebook.getAllNotes(Sets.newHashSet("anonymous")).size()); + assertEquals(2, notebook.getAllNotes(Sets.newHashSet("user1")).size()); notebook.getNotebookAuthorization().setOwners(note2.getId(), Sets.newHashSet("user2")); notebook.getNotebookAuthorization().setWriters(note2.getId(), Sets.newHashSet("user2")); notebook.getNotebookAuthorization().setReaders(note2.getId(), Sets.newHashSet("user2")); - assertEquals(0, notebook.getAllNotes(anonymous).size()); - assertEquals(1, notebook.getAllNotes(new AuthenticationInfo("user1")).size()); - assertEquals(1, notebook.getAllNotes(new AuthenticationInfo("user2")).size()); + assertEquals(0, notebook.getAllNotes(Sets.newHashSet("anonymous")).size()); + assertEquals(1, notebook.getAllNotes(Sets.newHashSet("user1")).size()); + assertEquals(1, notebook.getAllNotes(Sets.newHashSet("user2")).size()); notebook.removeNote(note1.getId(), anonymous); notebook.removeNote(note2.getId(), anonymous); } @@ -906,15 +906,15 @@ public class NotebookTest implements JobListenerFactory{ @Test public void testGetAllNotesWithDifferentPermissions() throws IOException { - AuthenticationInfo user1 = new AuthenticationInfo("user1"); - AuthenticationInfo user2 = new AuthenticationInfo("user2"); + HashSet<String> user1 = Sets.newHashSet("user1"); + HashSet<String> user2 = Sets.newHashSet("user1"); List<Note> notes1 = notebook.getAllNotes(user1); List<Note> notes2 = notebook.getAllNotes(user2); assertEquals(notes1.size(), 0); assertEquals(notes2.size(), 0); //creates note and sets user1 owner - Note note = notebook.createNote(user1); + Note note = notebook.createNote(new AuthenticationInfo("user1")); // note is public since readers and writers empty notes1 = notebook.getAllNotes(user1); @@ -933,7 +933,7 @@ public class NotebookTest implements JobListenerFactory{ notes1 = notebook.getAllNotes(user1); notes2 = notebook.getAllNotes(user2); assertEquals(notes1.size(), 1); - assertEquals(notes2.size(), 0); + assertEquals(notes2.size(), 1); } private void delete(File file){
