Skip to content

[ZEPPELIN-1483] Zeppelin home page list notebooks doesn't show notebook with group permission #1454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
prabhjyotsingh wants to merge 3 commits into from
Closed

[ZEPPELIN-1483] Zeppelin home page list notebooks doesn't show notebook with group permission #1454

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d3261c4
consume userAndRole instead of AuthenticationInfo
prabhjyotsingh Sep 23, 2016
c8d810e
organise imports
prabhjyotsingh Sep 26, 2016
2484833
Merge remote-tracking branch 'origin/master' into ZEPPELIN-1483
Oct 20, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 17 additions & 29 deletions zeppelin-server/src/main/java/org/apache/zeppelin/rest/NotebookRestApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,49 +17,36 @@

package org.apache.zeppelin.rest;

import java.io.IOException;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import javax.ws.rs.DELETE;
import javax.ws.rs.GET;
import javax.ws.rs.POST;
import javax.ws.rs.PUT;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.QueryParam;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;

import com.google.common.collect.Sets;
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;
import org.slf4j.LoggerFactory;

import org.apache.zeppelin.annotation.ZeppelinApi;
import org.apache.zeppelin.interpreter.InterpreterResult;
import org.apache.zeppelin.notebook.Note;
import org.apache.zeppelin.notebook.Notebook;
import org.apache.zeppelin.notebook.NotebookAuthorization;
import org.apache.zeppelin.notebook.Paragraph;
import org.apache.zeppelin.rest.message.CronRequest;
import org.apache.zeppelin.types.InterpreterSettingsList;
import org.apache.zeppelin.rest.message.NewNotebookRequest;
import org.apache.zeppelin.rest.message.NewParagraphRequest;
import org.apache.zeppelin.rest.message.RunParagraphWithParametersRequest;
import org.apache.zeppelin.search.SearchService;
import org.apache.zeppelin.server.JsonResponse;
import org.apache.zeppelin.socket.NotebookServer;
import org.apache.zeppelin.types.InterpreterSettingsList;
import org.apache.zeppelin.user.AuthenticationInfo;
import org.apache.zeppelin.utils.InterpreterBindingUtils;
import org.apache.zeppelin.utils.SecurityUtils;
import org.quartz.CronExpression;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.ws.rs.*;
import javax.ws.rs.core.Response;

@corneadoug corneadoug Sep 26, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed if we include javax.ws.rs.*

import javax.ws.rs.core.Response.Status;
import java.io.IOException;
import java.util.*;

/**
* Rest api endpoint for the noteBook.
Expand Down Expand Up @@ -198,7 +185,8 @@ public Response bind(@PathParam("noteId") String noteId) {
@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();
}

Expand Down Expand Up @@ -277,7 +265,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();
}

Expand All @@ -301,7 +289,7 @@ public Response deleteNote(@PathParam("notebookId") String notebookId) throws IO
}
}

notebookServer.broadcastNoteList(subject);
notebookServer.broadcastNoteList(subject, SecurityUtils.getRoles());
return new JsonResponse<>(Status.OK, "").build();
}

Expand All @@ -326,7 +314,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();
}

Expand Down
34 changes: 18 additions & 16 deletions zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.zeppelin.conf.ZeppelinConfiguration;
import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars;
Expand Down Expand Up @@ -165,10 +164,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);
Expand Down Expand Up @@ -459,7 +458,7 @@ public void getInterpreterBindings(NotebookSocket conn, Message fromMessage)
}

public List<Map<String, String>> generateNotebooksInfo(boolean needsReload,
AuthenticationInfo subject) {
AuthenticationInfo subject, HashSet<String> userAndRoles) {

Notebook notebook = notebook();

Expand All @@ -475,7 +474,7 @@ public List<Map<String, String>> generateNotebooksInfo(boolean needsReload,
LOG.error("Fail to reload notes from repository", e);
}
}
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<>();
Expand All @@ -502,18 +501,21 @@ public void broadcastInterpreterBindings(String noteId,
.put("interpreterBindings", settingList));
}

public void broadcastNoteList(AuthenticationInfo subject) {
List<Map<String, String>> notesInfo = generateNotebooksInfo(false, subject);
public void broadcastNoteList(AuthenticationInfo subject,
HashSet<String> userAndRoles) {
List<Map<String, String>> notesInfo = generateNotebooksInfo(false, subject, userAndRoles);
broadcastAll(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) {
List<Map<String, String>> notesInfo = generateNotebooksInfo(true, subject);
public void broadcastReloadedNoteList(AuthenticationInfo subject,
HashSet<String> userAndRoles) {
List<Map<String, String>> notesInfo = generateNotebooksInfo(true, subject, userAndRoles);
broadcastAll(new Message(OP.NOTES_INFO).put("notes", notesInfo));
}

Expand Down Expand Up @@ -617,7 +619,7 @@ private void updateNote(NotebookSocket conn, HashSet<String> userAndRoles,
AuthenticationInfo subject = new AuthenticationInfo(fromMessage.principal);
note.persist(subject);
broadcastNote(note);
broadcastNoteList(subject);
broadcastNoteList(subject, userAndRoles);
}
}

Expand Down Expand Up @@ -652,7 +654,7 @@ private void createNote(NotebookSocket conn, HashSet<String> 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<String> userAndRoles,
Expand All @@ -674,7 +676,7 @@ private void removeNote(NotebookSocket conn, HashSet<String> userAndRoles,
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,
Expand Down Expand Up @@ -716,7 +718,7 @@ private void cloneNote(NotebookSocket conn, HashSet<String> 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<String> userAndRoles,
Expand All @@ -733,7 +735,7 @@ protected Note importNote(NotebookSocket conn, HashSet<String> userAndRoles,
note = notebook.importNote(noteJson, noteName, subject);
note.persist(subject);
broadcastNote(note);
broadcastNoteList(subject);
broadcastNoteList(subject, userAndRoles);
}
return note;
}
Expand Down
2 changes: 1 addition & 1 deletion zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
44 changes: 12 additions & 32 deletions zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,41 +17,12 @@

package org.apache.zeppelin.notebook;

import java.io.IOException;
import java.io.StringReader;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;

import com.google.common.base.Predicate;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.Sets;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.stream.JsonReader;
import org.apache.commons.codec.binary.StringUtils;
import org.quartz.CronScheduleBuilder;
import org.quartz.CronTrigger;
import org.quartz.JobBuilder;
import org.quartz.JobDetail;
import org.quartz.JobExecutionContext;
import org.quartz.JobExecutionException;
import org.quartz.JobKey;
import org.quartz.SchedulerException;
import org.quartz.TriggerBuilder;
import org.quartz.impl.StdSchedulerFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.apache.zeppelin.conf.ZeppelinConfiguration;
import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars;
import org.apache.zeppelin.display.AngularObject;
Expand All @@ -69,6 +40,15 @@
import org.apache.zeppelin.search.SearchService;
import org.apache.zeppelin.user.AuthenticationInfo;
import org.apache.zeppelin.user.Credentials;
import org.quartz.*;
import org.quartz.impl.StdSchedulerFactory;

@corneadoug corneadoug Sep 26, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed if we include org.quartz.*

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.io.StringReader;
import java.util.*;
import java.util.concurrent.TimeUnit;

@corneadoug corneadoug Sep 26, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed if we include java.util.*


/**
* Collection of Notes.
Expand Down Expand Up @@ -535,10 +515,10 @@ public int compare(Note note1, Note note2) {
}
}

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) {
Expand Down
12 changes: 6 additions & 6 deletions zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NotebookTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -857,20 +857,20 @@ public void testNormalizeNoteName() throws IOException {
public void testGetAllNotes() throws Exception {
Note note1 = notebook.createNote(null);
Note note2 = notebook.createNote(null);
assertEquals(2, notebook.getAllNotes(new AuthenticationInfo("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(new AuthenticationInfo("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(new AuthenticationInfo("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());
}

private void delete(File file){
Expand Down