You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by mo...@apache.org on 2016/11/14 19:05:10 UTC
zeppelin git commit: [ZEPPELIN-1647] Save roles and use for
broadcasting note list per user
Repository: zeppelin
Updated Branches:
refs/heads/master 44d359d54 -> 6069a083a
[ZEPPELIN-1647] Save roles and use for broadcasting note list per user
### What is this PR for?
So far roles have been accessible only from SecurityUtils for Rest api or from websocket message field. However sometimes it's required to access roles in websocket server even without receiving message, say for broadcasting note list per user. More details in issue.
### What type of PR is it?
Bug Fix | Improvement
### Todos
* [x] - add roles in NotebookAuthorization
* [x] - assign roles on login
* [x] - use roles on broadcast
* [x] - test
### What is the Jira issue?
[ZEPPELIN-1647](https://issues.apache.org/jira/browse/ZEPPELIN-1647)
### How should this be tested?
login as user1, and user2 at same time and each user should have own workbench (based on notebook permissions)
### Screenshots (if appropriate)
TBD
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no
Author: Khalid Huseynov <kh...@gmail.com>
Closes #1619 from khalidhuseynov/fix/apply-correct-roles-broadcast and squashes the following commits:
490c4d0 [Khalid Huseynov] add roles functionality test
260adbb [Khalid Huseynov] remove unused import
d7df363 [Khalid Huseynov] bugfix: pass userAndRoles instead of roles
85be8b7 [Khalid Huseynov] change arg HashSet -> Set
1baa549 [Khalid Huseynov] set roles on login
83dd249 [Khalid Huseynov] use correct roles in broadcast
4bd7169 [Khalid Huseynov] add roles in NotebookAuthorization
Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/6069a083
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/6069a083
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/6069a083
Branch: refs/heads/master
Commit: 6069a083afe0bb66b664deac2d332984498b933b
Parents: 44d359d
Author: Khalid Huseynov <kh...@gmail.com>
Authored: Thu Nov 10 20:13:31 2016 +0900
Committer: Lee moon soo <mo...@apache.org>
Committed: Mon Nov 14 11:05:06 2016 -0800
----------------------------------------------------------------------
.../org/apache/zeppelin/rest/LoginRestApi.java | 4 ++
.../apache/zeppelin/rest/NotebookRestApi.java | 4 +-
.../apache/zeppelin/socket/NotebookServer.java | 27 ++++++++-----
.../org/apache/zeppelin/notebook/Notebook.java | 2 +-
.../notebook/NotebookAuthorization.java | 25 +++++++++++-
.../NotebookAuthorizationInfoSaving.java | 1 -
.../apache/zeppelin/notebook/NotebookTest.java | 42 ++++++++++++++++++++
7 files changed, 90 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/6069a083/zeppelin-server/src/main/java/org/apache/zeppelin/rest/LoginRestApi.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/LoginRestApi.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/LoginRestApi.java
index 0a23922..94e4c25 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/LoginRestApi.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/LoginRestApi.java
@@ -19,6 +19,7 @@ package org.apache.zeppelin.rest;
import org.apache.shiro.authc.*;
import org.apache.shiro.subject.Subject;
import org.apache.zeppelin.annotation.ZeppelinApi;
+import org.apache.zeppelin.notebook.NotebookAuthorization;
import org.apache.zeppelin.server.JsonResponse;
import org.apache.zeppelin.ticket.TicketContainer;
import org.apache.zeppelin.utils.SecurityUtils;
@@ -89,6 +90,9 @@ public class LoginRestApi {
response = new JsonResponse(Response.Status.OK, "", data);
//if no exception, that's it, we're done!
+
+ //set roles for user in NotebookAuthorization module
+ NotebookAuthorization.getInstance().setRoles(principal, roles);
} catch (UnknownAccountException uae) {
//username wasn't in the system, show them an error message?
LOG.error("Exception in login: ", uae);
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/6069a083/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 1f03b64..2b9ba11 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
@@ -258,8 +258,10 @@ public class NotebookRestApi {
@ZeppelinApi
public Response getNoteList() throws IOException {
AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal());
+ HashSet<String> userAndRoles = SecurityUtils.getRoles();
+ userAndRoles.add(subject.getUser());
List<Map<String, String>> notesInfo = notebookServer.generateNotesInfo(false, subject,
- SecurityUtils.getRoles());
+ userAndRoles);
return new JsonResponse<>(Status.OK, "", notesInfo).build();
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/6069a083/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 8a84587..b34a853 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
@@ -426,9 +426,10 @@ public class NotebookServer extends WebSocketServlet implements
private void multicastToUser(String user, Message m) {
if (!userConnectedSockets.containsKey(user)) {
- LOG.warn("Broadcasting to user that is not in connections map");
+ LOG.warn("Multicasting to user {} that is not in connections map", user);
return;
}
+
for (NotebookSocket conn: userConnectedSockets.get(user)) {
try {
conn.send(serializeMessage(m));
@@ -506,7 +507,7 @@ public class NotebookServer extends WebSocketServlet implements
}
public List<Map<String, String>> generateNotesInfo(boolean needsReload,
- AuthenticationInfo subject, HashSet<String> userAndRoles) {
+ AuthenticationInfo subject, Set<String> userAndRoles) {
Notebook notebook = notebook();
@@ -558,13 +559,7 @@ public class NotebookServer extends WebSocketServlet implements
List<Map<String, String>> notesInfo = generateNotesInfo(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 = generateNotesInfo(false, new AuthenticationInfo(user), userAndRoles);
- multicastToUser(user, new Message(OP.NOTES_INFO).put("notes", notesInfo));
- }
+ broadcastNoteListExcept(notesInfo, subject);
}
public void unicastNoteList(NotebookSocket conn, AuthenticationInfo subject,
@@ -577,20 +572,30 @@ public class NotebookServer extends WebSocketServlet implements
if (subject == null) {
subject = new AuthenticationInfo(StringUtils.EMPTY);
}
+
//reload and reply first to requesting user
List<Map<String, String>> notesInfo = generateNotesInfo(true, subject, userAndRoles);
multicastToUser(subject.getUser(), new Message(OP.NOTES_INFO).put("notes", notesInfo));
//to others afterwards
+ broadcastNoteListExcept(notesInfo, subject);
+ }
+
+ private void broadcastNoteListExcept(List<Map<String, String>> notesInfo,
+ AuthenticationInfo subject) {
+ Set<String> userAndRoles;
+ NotebookAuthorization authInfo = NotebookAuthorization.getInstance();
for (String user: userConnectedSockets.keySet()) {
- if (subject.getUser() == user) {
+ if (subject.getUser().equals(user)) {
continue;
}
//reloaded already above; parameter - false
+ userAndRoles = authInfo.getRoles(user);
+ userAndRoles.add(user);
notesInfo = generateNotesInfo(false, new AuthenticationInfo(user), userAndRoles);
multicastToUser(user, new Message(OP.NOTES_INFO).put("notes", notesInfo));
}
}
-
+
void permissionError(NotebookSocket conn, String op,
String userName,
Set<String> userAndRoles,
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/6069a083/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 6f0f793..cee3d68 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
@@ -542,7 +542,7 @@ public class Notebook implements NoteEventListener {
}
}
- public List<Note> getAllNotes(HashSet<String> userAndRoles) {
+ public List<Note> getAllNotes(Set<String> userAndRoles) {
final Set<String> entities = Sets.newHashSet();
if (userAndRoles != null) {
entities.addAll(userAndRoles);
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/6069a083/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java
index d835c89..e486e7c 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorization.java
@@ -24,6 +24,7 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.OutputStreamWriter;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
@@ -32,6 +33,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
+import org.apache.commons.lang.StringUtils;
import org.apache.zeppelin.conf.ZeppelinConfiguration;
import org.apache.zeppelin.user.AuthenticationInfo;
import org.slf4j.Logger;
@@ -53,6 +55,10 @@ public class NotebookAuthorization {
* { "note1": { "owners": ["u1"], "readers": ["u1", "u2"], "writers": ["u1"] }, "note2": ... } }
*/
private static Map<String, Map<String, Set<String>>> authInfo = new HashMap<>();
+ /*
+ * contains roles for each user
+ */
+ private static Map<String, Set<String>> userRoles = new HashMap<>();
private static ZeppelinConfiguration conf;
private static Gson gson;
private static String filePath;
@@ -108,7 +114,24 @@ public class NotebookAuthorization {
NotebookAuthorizationInfoSaving.class);
authInfo = info.authInfo;
}
-
+
+ public void setRoles(String user, Set<String> roles) {
+ if (StringUtils.isBlank(user)) {
+ LOG.warn("Setting roles for empty user");
+ return;
+ }
+ roles = validateUser(roles);
+ userRoles.put(user, roles);
+ }
+
+ public Set<String> getRoles(String user) {
+ Set<String> roles = Sets.newHashSet();
+ if (userRoles.containsKey(user)) {
+ roles.addAll(userRoles.get(user));
+ }
+ return roles;
+ }
+
private void saveToFile() {
String jsonString;
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/6069a083/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorizationInfoSaving.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorizationInfoSaving.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorizationInfoSaving.java
index 2a0668d..38cd0b6 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorizationInfoSaving.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/NotebookAuthorizationInfoSaving.java
@@ -17,7 +17,6 @@
package org.apache.zeppelin.notebook;
-import java.util.HashMap;
import java.util.Map;
import java.util.Set;
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/6069a083/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 3807bd0..ae81281 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
@@ -675,6 +675,48 @@ public class NotebookTest implements JobListenerFactory{
}
@Test
+ public void testAuthorizationRoles() throws IOException {
+ String user1 = "user1";
+ String user2 = "user2";
+ Set<String> roles = Sets.newHashSet("admin");
+ // set admin roles for both user1 and user2
+ notebookAuthorization.setRoles(user1, roles);
+ notebookAuthorization.setRoles(user2, roles);
+
+ Note note = notebook.createNote(new AuthenticationInfo(user1));
+
+ // check that user1 is owner, reader and writer
+ assertEquals(notebookAuthorization.isOwner(note.getId(),
+ Sets.newHashSet(user1)), true);
+ assertEquals(notebookAuthorization.isReader(note.getId(),
+ Sets.newHashSet(user1)), true);
+ assertEquals(notebookAuthorization.isWriter(note.getId(),
+ Sets.newHashSet(user1)), true);
+
+ // since user1 and user2 both have admin role, user2 will be reader and writer as well
+ assertEquals(notebookAuthorization.isOwner(note.getId(),
+ Sets.newHashSet(user2)), false);
+ assertEquals(notebookAuthorization.isReader(note.getId(),
+ Sets.newHashSet(user2)), true);
+ assertEquals(notebookAuthorization.isWriter(note.getId(),
+ Sets.newHashSet(user2)), true);
+
+ // check that user1 has note listed in his workbench
+ Set<String> user1AndRoles = notebookAuthorization.getRoles(user1);
+ user1AndRoles.add(user1);
+ List<Note> user1Notes = notebook.getAllNotes(user1AndRoles);
+ assertEquals(user1Notes.size(), 1);
+ assertEquals(user1Notes.get(0).getId(), note.getId());
+
+ // check that user2 has note listed in his workbech because of admin role
+ Set<String> user2AndRoles = notebookAuthorization.getRoles(user2);
+ user2AndRoles.add(user2);
+ List<Note> user2Notes = notebook.getAllNotes(user2AndRoles);
+ assertEquals(user2Notes.size(), 1);
+ assertEquals(user2Notes.get(0).getId(), note.getId());
+ }
+
+ @Test
public void testAbortParagraphStatusOnInterpreterRestart() throws InterruptedException,
IOException {
Note note = notebook.createNote(anonymous);