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 d9af812f164..d5814046183 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.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 Response putNotePermissions(@PathParam("noteId") String noteId, String re 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 Response bind(@PathParam("noteId") String noteId) { @ZeppelinApi public Response getNotebookList() throws IOException { AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal()); - List> notesInfo = notebookServer.generateNotebooksInfo(false, subject); + List> notesInfo = notebookServer.generateNotebooksInfo(false, subject, + SecurityUtils.getRoles()); return new JsonResponse<>(Status.OK, "", notesInfo).build(); } @@ -278,7 +278,7 @@ public Response createNote(String message) throws IOException { 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 Response deleteNote(@PathParam("notebookId") String notebookId) throws IO } } - notebookServer.broadcastNoteList(subject); + notebookServer.broadcastNoteList(subject, SecurityUtils.getRoles()); return new JsonResponse<>(Status.OK, "").build(); } @@ -327,7 +327,7 @@ public Response cloneNote(@PathParam("notebookId") String notebookId, String mes 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(); } 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 969bdf9510d..2f9faba42db 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.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 void onMessage(NotebookSocket conn, String msg) { /** 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 void getInterpreterBindings(NotebookSocket conn, Message fromMessage) } public List> generateNotebooksInfo(boolean needsReload, - AuthenticationInfo subject) { + AuthenticationInfo subject, HashSet userAndRoles) { Notebook notebook = notebook(); @@ -508,7 +507,7 @@ public List> generateNotebooksInfo(boolean needsReload, } } - List notes = notebook.getAllNotes(subject); + List notes = notebook.getAllNotes(userAndRoles); List> notesInfo = new LinkedList<>(); for (Note note : notes) { Map info = new HashMap<>(); @@ -535,34 +534,35 @@ public void broadcastInterpreterBindings(String noteId, .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> notesInfo = generateNotebooksInfo(false, subject); + List> 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> notesInfo = generateNotebooksInfo(false, subject); + public void unicastNoteList(NotebookSocket conn, AuthenticationInfo subject, + HashSet userAndRoles) { + List> 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> notesInfo = generateNotebooksInfo(true, subject); + List> 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 void broadcastReloadedNoteList(AuthenticationInfo subject) { 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 @@ private void updateNote(NotebookSocket conn, HashSet userAndRoles, AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal); note.persist(subject); broadcastNote(note); - broadcastNoteList(subject); + broadcastNoteList(subject, userAndRoles); } } @@ -713,7 +713,7 @@ private void createNote(NotebookSocket conn, HashSet userAndRoles, 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 userAndRoles, @@ -735,7 +735,7 @@ private void removeNote(NotebookSocket conn, HashSet userAndRoles, AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal); notebook.removeNote(noteId, subject); removeNote(noteId); - broadcastNoteList(subject); + broadcastNoteList(subject, userAndRoles); } private void updateParagraph(NotebookSocket conn, HashSet userAndRoles, @@ -777,7 +777,7 @@ private void cloneNote(NotebookSocket conn, HashSet userAndRoles, 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 userAndRoles, @@ -796,7 +796,7 @@ protected Note importNote(NotebookSocket conn, HashSet userAndRoles, note = notebook.importNote(noteJson, noteName, subject); note.persist(subject); broadcastNote(note); - broadcastNoteList(subject); + broadcastNoteList(subject, userAndRoles); } return note; } 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 60014298566..e1a6f9efb01 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 void testGroupPermission() throws Exception { 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. 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 c2606f82494..3b92c14550c 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 @@ 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 void testListNotebooks() throws IOException { }.getType()); List> body = (List>) 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 anonymous = Sets.newHashSet("anonymous"); + assertEquals("List notebooks are equal", ZeppelinServer.notebook.getAllNotes(anonymous).size(), body.size()); get.releaseConnection(); } 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 d996488cb61..9ad6d4c7bb6 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 int compare(Note note1, Note note2) { } } - public List getAllNotes(AuthenticationInfo subject) { + public List getAllNotes(HashSet userAndRoles) { final Set entities = Sets.newHashSet(); - if (subject != null) { - entities.add(subject.getUser()); + if (userAndRoles != null) { + entities.addAll(userAndRoles); } synchronized (notes) { 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 d0af2c90abe..87e241526f6 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 void testNormalizeNoteName() throws IOException { 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 void testGetAllNotes() throws Exception { @Test public void testGetAllNotesWithDifferentPermissions() throws IOException { - AuthenticationInfo user1 = new AuthenticationInfo("user1"); - AuthenticationInfo user2 = new AuthenticationInfo("user2"); + HashSet user1 = Sets.newHashSet("user1"); + HashSet user2 = Sets.newHashSet("user1"); List notes1 = notebook.getAllNotes(user1); List 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 void testGetAllNotesWithDifferentPermissions() throws IOException { 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){