From 6e85730343399c174090d2cf329bc3407d12e142 Mon Sep 17 00:00:00 2001 From: Prasad Wagle Date: Sun, 31 Jan 2016 22:30:51 -0800 Subject: [PATCH 01/14] Notebook Authorization --- .../apache/zeppelin/rest/NotebookRestApi.java | 44 +++++++++ .../org/apache/zeppelin/socket/Message.java | 1 + .../zeppelin/socket/NotebookServer.java | 95 +++++++++++++++++++ .../src/app/notebook/notebook-actionBar.html | 5 + .../src/app/notebook/notebook.controller.js | 72 ++++++++++++++ zeppelin-web/src/app/notebook/notebook.html | 23 +++++ .../websocketEvents.factory.js | 4 +- .../org/apache/zeppelin/notebook/Note.java | 48 ++++++++++ 8 files changed, 291 insertions(+), 1 deletion(-) 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 486e5b122c7..19de0790d84 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 @@ -18,6 +18,8 @@ package org.apache.zeppelin.rest; import java.io.IOException; +import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -75,6 +77,48 @@ public NotebookRestApi(Notebook notebook, NotebookServer notebookServer, SearchS this.notebookIndex = search; } + /** + * list note owners + */ + @GET + @Path("{noteId}/permissions") + public Response getNotePermissions(@PathParam("noteId") String noteId) { + Note note = notebook.getNote(noteId); + HashMap permissionsMap = new HashMap(); + permissionsMap.put("owners", note.getOwners()); + permissionsMap.put("readers", note.getReaders()); + permissionsMap.put("writers", note.getWriters()); + return new JsonResponse<>(Status.OK, "", permissionsMap).build(); + } + + /** + * Set note owners + */ + @PUT + @Path("{noteId}/permissions") + public Response putNotePermissions(@PathParam("noteId") String noteId, String req) + throws IOException { + HashMap permMap = gson.fromJson(req, + new TypeToken>(){}.getType()); + Note note = notebook.getNote(noteId); + LOG.debug("Set permissions {} {} {}", permMap.get("owners"), + permMap.get("readers"), + permMap.get("writers")); + // TODO(prasadwagle) Authenticate and check authorization +// if (!note.isOwner(userAndGroups())) { +// return new JsonResponse<>(Status.FORBIDDEN, ownerPermissionError(userAndGroups, +// note)).build(); +// } + note.setOwners(permMap.get("owners")); + note.setReaders(permMap.get("readers")); + note.setWriters(permMap.get("writers")); + LOG.debug("After set permissions {} {} {}", note.getOwners(), note.getReaders(), + note.getWriters()); + note.persist(); + notebookServer.broadcastNote(note); + return new JsonResponse<>(Status.OK).build(); + } + /** * bind a setting to note * @throws IOException diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/Message.java b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/Message.java index 2a8e06d7687..4f6bf842527 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/Message.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/Message.java @@ -96,6 +96,7 @@ public static enum OP { PARAGRAPH_APPEND_OUTPUT, // [s-c] append output PARAGRAPH_UPDATE_OUTPUT, // [s-c] update (replace) output PING, + AUTH_INFO, ANGULAR_OBJECT_UPDATE, // [s-c] add/update angular object ANGULAR_OBJECT_REMOVE, // [s-c] add angular object del 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 9a4a378aa99..80c8e86a51d 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 @@ -358,8 +358,32 @@ public void broadcastReloadedNoteList() { broadcastAll(new Message(OP.NOTES_INFO).put("notes", notesInfo)); } + void readPermissionEror(NotebookSocket conn, String principal, Note note) throws IOException { + LOG.info("Cannot read. Connection readers {}. Allowed readers {}", + principal, note.getReaders()); + conn.send(serializeMessage(new Message(OP.AUTH_INFO).put("info", + "Insufficient privileges to read note.\n" + + "Allowed readers: " + note.getReaders().toString() + "\n" + + "User belongs to: " + principal))); + } + + void writePermissionError(NotebookSocket conn, String principal, Note note) throws IOException { + LOG.info("Cannot write. Connection writers {}. Allowed writers {}", + principal, note.getWriters()); + conn.send(serializeMessage(new Message(OP.AUTH_INFO).put("info", + "Insufficient privileges to write note.\n" + + "Allowed writers: " + note.getWriters().toString() + "\n" + + "User belongs to: " + principal))); + } + private void sendNote(NotebookSocket conn, Notebook notebook, Message fromMessage) throws IOException { + + LOG.info("New operation from {} : {} : {} : {} : {}", conn.getRequest().getRemoteAddr(), + conn.getRequest().getRemotePort(), + fromMessage.principal, fromMessage.op, fromMessage.get("id") + ); + String noteId = (String) fromMessage.get("id"); if (noteId == null) { return; @@ -367,6 +391,14 @@ private void sendNote(NotebookSocket conn, Notebook notebook, Note note = notebook.getNote(noteId); if (note != null) { + HashSet usersAndGroups = new HashSet(); + usersAndGroups.add(fromMessage.principal); + // TODO(prasadwagle) add groups to usersAndGroups + if (!note.isReader(usersAndGroups)) { + readPermissionEror(conn, fromMessage.principal, note); + broadcastNoteList(); + return; + } addConnectionToNote(note.id(), conn); conn.send(serializeMessage(new Message(OP.NOTE).put("note", note))); sendAllAngularObjects(note, conn); @@ -476,6 +508,15 @@ private void updateParagraph(NotebookSocket conn, Notebook notebook, Map config = (Map) fromMessage .get("config"); final Note note = notebook.getNote(getOpenNoteId(conn)); + + HashSet usersAndGroups = new HashSet(); + usersAndGroups.add(fromMessage.principal); + // TODO(prasadwagle) add groups to usersAndGroups + if (!note.isWriter(usersAndGroups)) { + writePermissionError(conn, fromMessage.principal, note); + return; + } + Paragraph p = note.getParagraph(paragraphId); p.settings.setParams(params); p.setConfig(config); @@ -517,6 +558,15 @@ private void removeParagraph(NotebookSocket conn, Notebook notebook, } final Note note = notebook.getNote(getOpenNoteId(conn)); + + HashSet usersAndGroups = new HashSet(); + usersAndGroups.add(fromMessage.principal); + // TODO(prasadwagle) add groups to usersAndGroups + if (!note.isWriter(usersAndGroups)) { + writePermissionError(conn, fromMessage.principal, note); + return; + } + /** We dont want to remove the last paragraph */ if (!note.isLastParagraph(paragraphId)) { note.removeParagraph(paragraphId); @@ -533,6 +583,15 @@ private void clearParagraphOutput(NotebookSocket conn, Notebook notebook, } final Note note = notebook.getNote(getOpenNoteId(conn)); + + HashSet usersAndGroups = new HashSet(); + usersAndGroups.add(fromMessage.principal); + // TODO(prasadwagle) add groups to usersAndGroups + if (!note.isWriter(usersAndGroups)) { + writePermissionError(conn, fromMessage.principal, note); + return; + } + note.clearParagraphOutput(paragraphId); broadcastNote(note); } @@ -654,6 +713,15 @@ private void moveParagraph(NotebookSocket conn, Notebook notebook, final int newIndex = (int) Double.parseDouble(fromMessage.get("index") .toString()); final Note note = notebook.getNote(getOpenNoteId(conn)); + + HashSet usersAndGroups = new HashSet(); + usersAndGroups.add(fromMessage.principal); + // TODO(prasadwagle) add groups to usersAndGroups + if (!note.isWriter(usersAndGroups)) { + writePermissionError(conn, fromMessage.principal, note); + return; + } + note.moveParagraph(paragraphId, newIndex); note.persist(); broadcastNote(note); @@ -664,6 +732,15 @@ private void insertParagraph(NotebookSocket conn, Notebook notebook, final int index = (int) Double.parseDouble(fromMessage.get("index") .toString()); final Note note = notebook.getNote(getOpenNoteId(conn)); + + HashSet usersAndGroups = new HashSet(); + usersAndGroups.add(fromMessage.principal); + // TODO(prasadwagle) add groups to usersAndGroups + if (!note.isWriter(usersAndGroups)) { + writePermissionError(conn, fromMessage.principal, note); + return; + } + note.insertParagraph(index); note.persist(); broadcastNote(note); @@ -677,6 +754,15 @@ private void cancelParagraph(NotebookSocket conn, Notebook notebook, } final Note note = notebook.getNote(getOpenNoteId(conn)); + + HashSet usersAndGroups = new HashSet(); + usersAndGroups.add(fromMessage.principal); + // TODO(prasadwagle) add groups to usersAndGroups + if (!note.isWriter(usersAndGroups)) { + writePermissionError(conn, fromMessage.principal, note); + return; + } + Paragraph p = note.getParagraph(paragraphId); p.abort(); } @@ -689,6 +775,15 @@ private void runParagraph(NotebookSocket conn, Notebook notebook, } final Note note = notebook.getNote(getOpenNoteId(conn)); + + HashSet usersAndGroups = new HashSet(); + usersAndGroups.add(fromMessage.principal); + // TODO(prasadwagle) add groups to usersAndGroups + if (!note.isWriter(usersAndGroups)) { + writePermissionError(conn, fromMessage.principal, note); + return; + } + Paragraph p = note.getParagraph(paragraphId); String text = (String) fromMessage.get("paragraph"); p.setText(text); diff --git a/zeppelin-web/src/app/notebook/notebook-actionBar.html b/zeppelin-web/src/app/notebook/notebook-actionBar.html index 0340e28d689..0b4a1be5d30 100644 --- a/zeppelin-web/src/app/notebook/notebook-actionBar.html +++ b/zeppelin-web/src/app/notebook/notebook-actionBar.html @@ -129,6 +129,11 @@

tooltip-placement="bottom" tooltip="Interpreter binding"> + + + + + +
diff --git a/zeppelin-web/src/components/websocketEvents/websocketEvents.factory.js b/zeppelin-web/src/components/websocketEvents/websocketEvents.factory.js index 800d450d50d..39d785cb985 100644 --- a/zeppelin-web/src/components/websocketEvents/websocketEvents.factory.js +++ b/zeppelin-web/src/components/websocketEvents/websocketEvents.factory.js @@ -52,12 +52,14 @@ angular.module('zeppelinWebApp').factory('websocketEvents', function($rootScope, $location.path('notebook/' + data.note.id); } else if (op === 'NOTES_INFO') { $rootScope.$broadcast('setNoteMenu', data.notes); + } else if (op === 'AUTH_INFO') { + alert(data.info.toString()); } else if (op === 'PARAGRAPH') { $rootScope.$broadcast('updateParagraph', data); } else if (op === 'PARAGRAPH_APPEND_OUTPUT') { $rootScope.$broadcast('appendParagraphOutput', data); } else if (op === 'PARAGRAPH_UPDATE_OUTPUT') { - $rootScope.$broadcast('updateParagraphOutput', data); + $rootScope.$broadcast('updateParagraphOutput', data); } else if (op === 'PROGRESS') { $rootScope.$broadcast('updateProgress', data); } else if (op === 'COMPLETION_LIST') { diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java index 27e2f77cee2..abab2ae8d27 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Note.java @@ -58,6 +58,9 @@ public class Note implements Serializable, JobListener { private String name = ""; private String id; + private HashSet owners = new HashSet(); + private HashSet readers = new HashSet(); + private HashSet writers = new HashSet(); @SuppressWarnings("rawtypes") Map> angularObjects = new HashMap<>(); @@ -114,6 +117,51 @@ public void setName(String name) { this.name = name; } + public HashSet getOwners() { + return (new HashSet(owners)); + } + + public void setOwners(HashSet owners) { + this.owners = new HashSet(owners); + } + + public HashSet getReaders() { + return (new HashSet(readers)); + } + + public void setReaders(HashSet readers) { + this.readers = new HashSet(readers); + } + + public HashSet getWriters() { + return (new HashSet(writers)); + } + + public void setWriters(HashSet writers) { + this.writers = new HashSet(writers); + } + + public boolean isOwner(HashSet entities) { + return isMember(entities, this.owners); + } + + public boolean isWriter(HashSet entities) { + return isMember(entities, this.writers) || isMember(entities, this.owners); + } + + public boolean isReader(HashSet entities) { + return isMember(entities, this.readers) || + isMember(entities, this.owners) || + isMember(entities, this.writers); + } + + // return true if b is empty or if (a intersection b) is non-empty + private boolean isMember(HashSet a, HashSet b) { + Set intersection = new HashSet(b); + intersection.retainAll(a); + return (b.isEmpty() || (intersection.size() > 0)); + } + public NoteInterpreterLoader getNoteReplLoader() { return replLoader; } From a8d0ecbb8f973a10a60e17f0b27b2c63c6623e78 Mon Sep 17 00:00:00 2001 From: Prasad Wagle Date: Thu, 4 Feb 2016 17:57:19 -0800 Subject: [PATCH 02/14] Add security documentation --- docs/security/authentication.md | 31 ++++++++++ docs/security/interpreter_authorization.md | 28 +++++++++ docs/security/notebook_authorization.md | 66 ++++++++++++++++++++++ docs/security/overview.md | 29 ++++++++++ 4 files changed, 154 insertions(+) create mode 100644 docs/security/authentication.md create mode 100644 docs/security/interpreter_authorization.md create mode 100644 docs/security/notebook_authorization.md create mode 100644 docs/security/overview.md diff --git a/docs/security/authentication.md b/docs/security/authentication.md new file mode 100644 index 00000000000..b71d28ad2a4 --- /dev/null +++ b/docs/security/authentication.md @@ -0,0 +1,31 @@ +--- +layout: page +title: "Authentication" +description: "Authentication" +group: security +--- + +# Zeppelin Authentication + +Authentication is company-specific. + +One option is to use [Basic Access Authentication](https://en.wikipedia.org/wiki/Basic_access_authentication) + +Anoteher is to have an authentication server that can verify user credentials in an LDAP server. +If an incoming request to the Zeppelin server does not have a cookie with user information encrypted with the authentication server public key, the user +is redirected to the authentication server. Once the user is verified, the authentication server redirects the browser to a specific +URL in the Zeppelin server which sets the authentication cookie in the browser. +The end result is that all requests to the Zeppelin +web server have the authentication cookie which contains user and groups information. diff --git a/docs/security/interpreter_authorization.md b/docs/security/interpreter_authorization.md new file mode 100644 index 00000000000..371ba4a35d2 --- /dev/null +++ b/docs/security/interpreter_authorization.md @@ -0,0 +1,28 @@ +--- +layout: page +title: "Notebook Authorization" +description: "Notebook Authorization" +group: security +--- + +# Zeppelin Interpreter Authorization + +The Interpreter authorization problem is more complex. Different interpreters require different strategies. + +For the Hive interpreter, we need to maintain per-user connection pools. +The interpreter method takes the user string as parameter and executes the jdbc call using a connection in the user's connection pool. + +In case of Presto, we don't need password if the Presto DB server runs backend code using HDFS authorization for the user. +For databases like Vertica and Mysql we would have to store password information for users. diff --git a/docs/security/notebook_authorization.md b/docs/security/notebook_authorization.md new file mode 100644 index 00000000000..6bbaf45218f --- /dev/null +++ b/docs/security/notebook_authorization.md @@ -0,0 +1,66 @@ +--- +layout: page +title: "Notebook Authorization" +description: "Notebook Authorization" +group: security +--- + +# Zeppelin Notebook Authorization + +## Overview +There are different aspects to Zeppelin security: + +* Authentication: is the user who they say they are? +* Notebook authorization: does the user have permissions to read or write to a note? +* Interpreter authorization: does the user have permissions to perform interpreter operations e.g. access data source objects? + + +## Authentication + +Authentication is company-specific. One option is to have an authentication server that can verify user credentials in an LDAP server. +If an incoming request to the Zeppelin server does not have a cookie with user information encrypted with the authentication server public key, the user +is redirected to the authentication server. Once the user is verified, the authentication server redirects the browser to a specific +URL in the Zeppelin server which sets the authentication cookie in the browser. +The end result is that all requests to the Zeppelin +web server have the authentication cookie which contains user and groups information. + + +## Notebook Authorization + +We assume that there is an authentication component that associates a user string and a set of group strings with every NotebookSocket. + +Each note has the following: +* set of owner entities (users or groups) +* set of reader entities (users or groups) +* set of writer entities (users or groups) + +If a set is empty, it means that any user can perform that operation. + +The NotebookServer classifies every Note operation into three categories: read, write, manage. +Before executing a Note operation, it checks if the user and the groups associated with the NotebookSocket have permissions. For example, before executing an read +operation, it checks if the user and the groups have at least one entity that belongs to the reader entities. + +To initialize and modify note permissions, we provide UI like "Interpreter binding". The user inputs comma separated entities for owners, readers and writers. +We execute a rest api call with this information. In the backend we get the user information for the connection and allow the operation if the user and groups +associated with the current user have at least one entity that belongs to owner entities for the note. + +## Interpreter Authorization +The Interpreter authorization problem is more complex. Different interpreters require different strategies. + +For the Hive interpreter, we need to maintain per-user connection pools. +The interpreter method takes the user string as parameter and executes the jdbc call using a connection in the user's connection pool. + +In case of Presto, we don't need password if the Presto DB server runs backend code using HDFS authorization for the user. +For databases like Vertica and Mysql we would have to store password information for users. diff --git a/docs/security/overview.md b/docs/security/overview.md new file mode 100644 index 00000000000..7c2631144e3 --- /dev/null +++ b/docs/security/overview.md @@ -0,0 +1,29 @@ +--- +layout: page +title: "Security Overview" +description: "Security Overview" +group: security +--- + +{% include JB/setup %} + +# Security Overview + +There are three aspects to Zeppelin security: + +* Authentication: is the user who they say they are? [More](overview.html) +* Notebook authorization: does the user have permissions to read or write to a note? [More](notebook_authorization.html) +* Interpreter authorization: does the user have permissions to perform interpreter operations e.g. access data source objects? [More](interpreter_authorization.html) + From 6b9e27447ddc2b7f58be9bad16cf527521405ce1 Mon Sep 17 00:00:00 2001 From: Prasad Wagle Date: Thu, 4 Feb 2016 18:01:53 -0800 Subject: [PATCH 03/14] Add unit test for note permissions --- .../zeppelin/notebook/NotebookTest.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) 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 506b682b293..ba9175d69be 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 @@ -27,10 +27,12 @@ import java.io.File; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.HashSet; import org.apache.commons.io.FileUtils; import org.apache.zeppelin.conf.ZeppelinConfiguration; @@ -362,6 +364,31 @@ public void testAngularObjectRemovalOnInterpreterRestart() throws InterruptedExc notebook.removeNote(note.id()); } + @Test + public void testPermissions() throws IOException { + // create a note and a paragraph + Note note = notebook.createNote(); + // empty owners, readers and writers means note is public + assertEquals(note.isOwner(new HashSet(Arrays.asList("user2"))), true); + assertEquals(note.isReader(new HashSet(Arrays.asList("user2"))), true); + assertEquals(note.isWriter(new HashSet(Arrays.asList("user2"))), true); + + note.setOwners(new HashSet(Arrays.asList("user1"))); + note.setReaders(new HashSet(Arrays.asList("user1", "user2"))); + note.setWriters(new HashSet(Arrays.asList("user1"))); + + assertEquals(note.isOwner(new HashSet(Arrays.asList("user2"))), false); + assertEquals(note.isOwner(new HashSet(Arrays.asList("user1"))), true); + + assertEquals(note.isReader(new HashSet(Arrays.asList("user3"))), false); + assertEquals(note.isReader(new HashSet(Arrays.asList("user2"))), true); + + assertEquals(note.isWriter(new HashSet(Arrays.asList("user2"))), false); + assertEquals(note.isWriter(new HashSet(Arrays.asList("user1"))), true); + + notebook.removeNote(note.id()); + } + @Test public void testAbortParagraphStatusOnInterpreterRestart() throws InterruptedException, IOException { From 06c5e07c23cdc5f2e95dd8368faf5780f0f0665a Mon Sep 17 00:00:00 2001 From: Prasad Wagle Date: Thu, 4 Feb 2016 18:02:47 -0800 Subject: [PATCH 04/14] Update navigation.html for security docs --- docs/_includes/themes/zeppelin/_navigation.html | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/_includes/themes/zeppelin/_navigation.html b/docs/_includes/themes/zeppelin/_navigation.html index d0581b14549..00c6938e457 100644 --- a/docs/_includes/themes/zeppelin/_navigation.html +++ b/docs/_includes/themes/zeppelin/_navigation.html @@ -55,6 +55,15 @@
  • Tajo
    • +
  • + Security + +
  • Display System
      From fbbd04b39e8b0a629c2b6284016c4853c7f6425a Mon Sep 17 00:00:00 2001 From: Prasad Wagle Date: Thu, 4 Feb 2016 18:04:38 -0800 Subject: [PATCH 05/14] Make insufficient privileges error message easier to read --- .../java/org/apache/zeppelin/socket/NotebookServer.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 80c8e86a51d..90c4bb3bd30 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 @@ -362,8 +362,8 @@ void readPermissionEror(NotebookSocket conn, String principal, Note note) throw LOG.info("Cannot read. Connection readers {}. Allowed readers {}", principal, note.getReaders()); conn.send(serializeMessage(new Message(OP.AUTH_INFO).put("info", - "Insufficient privileges to read note.\n" + - "Allowed readers: " + note.getReaders().toString() + "\n" + + "Insufficient privileges to read note.\n\n" + + "Allowed readers: " + note.getReaders().toString() + "\n\n" + "User belongs to: " + principal))); } @@ -371,8 +371,8 @@ void writePermissionError(NotebookSocket conn, String principal, Note note) thr LOG.info("Cannot write. Connection writers {}. Allowed writers {}", principal, note.getWriters()); conn.send(serializeMessage(new Message(OP.AUTH_INFO).put("info", - "Insufficient privileges to write note.\n" + - "Allowed writers: " + note.getWriters().toString() + "\n" + + "Insufficient privileges to write note.\n\n" + + "Allowed writers: " + note.getWriters().toString() + "\n\n" + "User belongs to: " + principal))); } From 3a5e5c08daae705c5a48a63d4f3b9660a3b7ef17 Mon Sep 17 00:00:00 2001 From: Prasad Wagle Date: Thu, 4 Feb 2016 18:05:18 -0800 Subject: [PATCH 06/14] Check if user has permissions to modify note permissions --- .../apache/zeppelin/rest/NotebookRestApi.java | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) 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 19de0790d84..d9fc7a18bfa 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 @@ -48,6 +48,7 @@ import org.apache.zeppelin.search.SearchService; import org.apache.zeppelin.server.JsonResponse; import org.apache.zeppelin.socket.NotebookServer; +import org.apache.zeppelin.utils.SecurityUtils; import org.quartz.CronExpression; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -91,6 +92,14 @@ public Response getNotePermissions(@PathParam("noteId") String noteId) { return new JsonResponse<>(Status.OK, "", permissionsMap).build(); } + String ownerPermissionError(HashSet usersAndGroups, Note note) throws IOException { + LOG.info("Cannot change permissions. Connection owners {}. Allowed owners {}", + usersAndGroups, note.getOwners()); + return "Insufficient privileges to change permissions.\n\n" + + "Allowed owners: " + note.getOwners().toString() + "\n\n" + + "User belongs to: " + usersAndGroups.toString(); + } + /** * Set note owners */ @@ -101,14 +110,21 @@ public Response putNotePermissions(@PathParam("noteId") String noteId, String re HashMap permMap = gson.fromJson(req, new TypeToken>(){}.getType()); Note note = notebook.getNote(noteId); - LOG.debug("Set permissions {} {} {}", permMap.get("owners"), + String principal = SecurityUtils.getPrincipal(); + LOG.info("Set permissions {} {} {} {} {}", + noteId, + principal, + permMap.get("owners"), permMap.get("readers"), - permMap.get("writers")); - // TODO(prasadwagle) Authenticate and check authorization -// if (!note.isOwner(userAndGroups())) { -// return new JsonResponse<>(Status.FORBIDDEN, ownerPermissionError(userAndGroups, -// note)).build(); -// } + permMap.get("writers") + ); + HashSet usersAndGroups = new HashSet(); + usersAndGroups.add(principal); + // TODO(prasadwagle) add groups to usersAndGroups + if (!note.isOwner(usersAndGroups)) { + return new JsonResponse<>(Status.FORBIDDEN, ownerPermissionError(usersAndGroups, + note)).build(); + } note.setOwners(permMap.get("owners")); note.setReaders(permMap.get("readers")); note.setWriters(permMap.get("writers")); From 25543155b2e1b7065493352688836ba1d0d7c9d1 Mon Sep 17 00:00:00 2001 From: Prasad Wagle Date: Fri, 5 Feb 2016 16:26:53 -0800 Subject: [PATCH 07/14] Use user and roles for checking note permissions --- .../apache/zeppelin/rest/NotebookRestApi.java | 23 ++- .../apache/zeppelin/rest/SecurityRestApi.java | 8 + .../org/apache/zeppelin/socket/Message.java | 1 + .../zeppelin/socket/NotebookServer.java | 183 +++++++++--------- .../apache/zeppelin/utils/SecurityUtils.java | 23 +++ .../zeppelin/socket/NotebookServerTest.java | 2 +- .../websocketEvents.factory.js | 3 +- 7 files changed, 136 insertions(+), 107 deletions(-) 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 d9fc7a18bfa..be46f13b0a2 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 @@ -92,12 +92,13 @@ public Response getNotePermissions(@PathParam("noteId") String noteId) { return new JsonResponse<>(Status.OK, "", permissionsMap).build(); } - String ownerPermissionError(HashSet usersAndGroups, Note note) throws IOException { + String ownerPermissionError(HashSet current, + HashSet allowed) throws IOException { LOG.info("Cannot change permissions. Connection owners {}. Allowed owners {}", - usersAndGroups, note.getOwners()); + current.toString(), allowed.toString()); return "Insufficient privileges to change permissions.\n\n" + - "Allowed owners: " + note.getOwners().toString() + "\n\n" + - "User belongs to: " + usersAndGroups.toString(); + "Allowed owners: " + allowed.toString() + "\n\n" + + "User belongs to: " + current.toString(); } /** @@ -111,6 +112,7 @@ public Response putNotePermissions(@PathParam("noteId") String noteId, String re new TypeToken>(){}.getType()); Note note = notebook.getNote(noteId); String principal = SecurityUtils.getPrincipal(); + HashSet roles = SecurityUtils.getRoles(); LOG.info("Set permissions {} {} {} {} {}", noteId, principal, @@ -118,12 +120,13 @@ public Response putNotePermissions(@PathParam("noteId") String noteId, String re permMap.get("readers"), permMap.get("writers") ); - HashSet usersAndGroups = new HashSet(); - usersAndGroups.add(principal); - // TODO(prasadwagle) add groups to usersAndGroups - if (!note.isOwner(usersAndGroups)) { - return new JsonResponse<>(Status.FORBIDDEN, ownerPermissionError(usersAndGroups, - note)).build(); + + HashSet userAndRoles = new HashSet(); + userAndRoles.add(principal); + userAndRoles.addAll(roles); + if (!note.isOwner(userAndRoles)) { + return new JsonResponse<>(Status.FORBIDDEN, ownerPermissionError(userAndRoles, + note.getOwners())).build(); } note.setOwners(permMap.get("owners")); note.setReaders(permMap.get("readers")); diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/SecurityRestApi.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/SecurityRestApi.java index d6f3dec05d4..905eb2bdd9a 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/SecurityRestApi.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/SecurityRestApi.java @@ -21,12 +21,15 @@ import org.apache.zeppelin.server.JsonResponse; import org.apache.zeppelin.ticket.TicketContainer; import org.apache.zeppelin.utils.SecurityUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.ws.rs.GET; import javax.ws.rs.Path; import javax.ws.rs.Produces; import javax.ws.rs.core.Response; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; /** @@ -36,6 +39,8 @@ @Path("/security") @Produces("application/json") public class SecurityRestApi { + private static final Logger LOG = LoggerFactory.getLogger(SecurityRestApi.class); + /** * Required by Swagger. */ @@ -56,6 +61,7 @@ public SecurityRestApi() { public Response ticket() { ZeppelinConfiguration conf = ZeppelinConfiguration.create(); String principal = SecurityUtils.getPrincipal(); + HashSet roles = SecurityUtils.getRoles(); JsonResponse response; // ticket set to anonymous for anonymous user. Simplify testing. String ticket; @@ -66,9 +72,11 @@ public Response ticket() { Map data = new HashMap<>(); data.put("principal", principal); + data.put("roles", roles.toString()); data.put("ticket", ticket); response = new JsonResponse(Response.Status.OK, "", data); + LOG.warn(response.toString()); return response.build(); } } diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/Message.java b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/Message.java index 4f6bf842527..a141daff92c 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/Message.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/Message.java @@ -112,6 +112,7 @@ public static enum OP { public Map data = new HashMap(); public String ticket = "anonymous"; public String principal = "anonymous"; + public String roles = ""; public Message(OP op) { this.op = op; 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 90c4bb3bd30..2b4e6af009b 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 @@ -24,6 +24,7 @@ import javax.servlet.http.HttpServletRequest; +import com.google.gson.reflect.TypeToken; import org.apache.zeppelin.conf.ZeppelinConfiguration; import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars; import org.apache.zeppelin.display.AngularObject; @@ -99,6 +100,7 @@ public void onMessage(NotebookSocket conn, String msg) { LOG.debug("RECEIVE << " + messagereceived.op); LOG.debug("RECEIVE PRINCIPAL << " + messagereceived.principal); LOG.debug("RECEIVE TICKET << " + messagereceived.ticket); + LOG.debug("RECEIVE ROLES << " + messagereceived.roles); String ticket = TicketContainer.instance.getTicket(messagereceived.principal); if (ticket != null && !ticket.equals(messagereceived.ticket)) throw new Exception("Invalid ticket " + messagereceived.ticket + " != " + ticket); @@ -110,6 +112,12 @@ public void onMessage(NotebookSocket conn, String msg) { throw new Exception("Anonymous access not allowed "); } + HashSet userAndRoles = new HashSet(); + userAndRoles.add(messagereceived.principal); + HashSet roles = gson.fromJson(messagereceived.roles, + new TypeToken>(){}.getType()); + userAndRoles.addAll(roles); + /** Lets be elegant here */ switch (messagereceived.op) { case LIST_NOTES: @@ -119,57 +127,57 @@ public void onMessage(NotebookSocket conn, String msg) { broadcastReloadedNoteList(); break; case GET_HOME_NOTE: - sendHomeNote(conn, notebook); + sendHomeNote(conn, userAndRoles, notebook); break; case GET_NOTE: - sendNote(conn, notebook, messagereceived); + sendNote(conn, userAndRoles, notebook, messagereceived); break; case NEW_NOTE: - createNote(conn, notebook, messagereceived); + createNote(conn, userAndRoles, notebook, messagereceived); break; case DEL_NOTE: - removeNote(conn, notebook, messagereceived); + removeNote(conn, userAndRoles, notebook, messagereceived); break; case CLONE_NOTE: - cloneNote(conn, notebook, messagereceived); + cloneNote(conn, userAndRoles, notebook, messagereceived); break; case IMPORT_NOTE: - importNote(conn, notebook, messagereceived); + importNote(conn, userAndRoles, notebook, messagereceived); break; case COMMIT_PARAGRAPH: - updateParagraph(conn, notebook, messagereceived); + updateParagraph(conn, userAndRoles, notebook, messagereceived); break; case RUN_PARAGRAPH: - runParagraph(conn, notebook, messagereceived); + runParagraph(conn, userAndRoles, notebook, messagereceived); break; case CANCEL_PARAGRAPH: - cancelParagraph(conn, notebook, messagereceived); + cancelParagraph(conn, userAndRoles, notebook, messagereceived); break; case MOVE_PARAGRAPH: - moveParagraph(conn, notebook, messagereceived); + moveParagraph(conn, userAndRoles, notebook, messagereceived); break; case INSERT_PARAGRAPH: - insertParagraph(conn, notebook, messagereceived); + insertParagraph(conn, userAndRoles, notebook, messagereceived); break; case PARAGRAPH_REMOVE: - removeParagraph(conn, notebook, messagereceived); + removeParagraph(conn, userAndRoles, notebook, messagereceived); break; case PARAGRAPH_CLEAR_OUTPUT: - clearParagraphOutput(conn, notebook, messagereceived); + clearParagraphOutput(conn, userAndRoles, notebook, messagereceived); break; case NOTE_UPDATE: - updateNote(conn, notebook, messagereceived); + updateNote(conn, userAndRoles, notebook, messagereceived); break; case COMPLETION: - completion(conn, notebook, messagereceived); + completion(conn, userAndRoles, notebook, messagereceived); break; case PING: break; //do nothing case ANGULAR_OBJECT_UPDATED: - angularObjectUpdated(conn, notebook, messagereceived); + angularObjectUpdated(conn, userAndRoles, notebook, messagereceived); break; case LIST_CONFIGURATIONS: - sendAllConfigurations(conn, notebook); + sendAllConfigurations(conn, userAndRoles, notebook); break; default: broadcastNoteList(); @@ -358,25 +366,17 @@ public void broadcastReloadedNoteList() { broadcastAll(new Message(OP.NOTES_INFO).put("notes", notesInfo)); } - void readPermissionEror(NotebookSocket conn, String principal, Note note) throws IOException { - LOG.info("Cannot read. Connection readers {}. Allowed readers {}", - principal, note.getReaders()); - conn.send(serializeMessage(new Message(OP.AUTH_INFO).put("info", - "Insufficient privileges to read note.\n\n" + - "Allowed readers: " + note.getReaders().toString() + "\n\n" + - "User belongs to: " + principal))); - } - - void writePermissionError(NotebookSocket conn, String principal, Note note) throws IOException { - LOG.info("Cannot write. Connection writers {}. Allowed writers {}", - principal, note.getWriters()); + void permissionError(NotebookSocket conn, String op, HashSet current, + HashSet allowed) throws IOException { + LOG.info("Cannot {}. Connection readers {}. Allowed readers {}", + op, current, allowed); conn.send(serializeMessage(new Message(OP.AUTH_INFO).put("info", - "Insufficient privileges to write note.\n\n" + - "Allowed writers: " + note.getWriters().toString() + "\n\n" + - "User belongs to: " + principal))); + "Insufficient privileges to " + op + " note.\n\n" + + "Allowed users or roles: " + allowed.toString() + "\n\n" + + "User belongs to: " + current.toString()))); } - private void sendNote(NotebookSocket conn, Notebook notebook, + private void sendNote(NotebookSocket conn, HashSet userAndRoles, Notebook notebook, Message fromMessage) throws IOException { LOG.info("New operation from {} : {} : {} : {} : {}", conn.getRequest().getRemoteAddr(), @@ -391,11 +391,8 @@ private void sendNote(NotebookSocket conn, Notebook notebook, Note note = notebook.getNote(noteId); if (note != null) { - HashSet usersAndGroups = new HashSet(); - usersAndGroups.add(fromMessage.principal); - // TODO(prasadwagle) add groups to usersAndGroups - if (!note.isReader(usersAndGroups)) { - readPermissionEror(conn, fromMessage.principal, note); + if (!note.isReader(userAndRoles)) { + permissionError(conn, "read", userAndRoles, note.getReaders()); broadcastNoteList(); return; } @@ -405,7 +402,8 @@ private void sendNote(NotebookSocket conn, Notebook notebook, } } - private void sendHomeNote(NotebookSocket conn, Notebook notebook) throws IOException { + private void sendHomeNote(NotebookSocket conn, HashSet userAndRoles, + Notebook notebook) throws IOException { String noteId = notebook.getConf().getString(ConfVars.ZEPPELIN_NOTEBOOK_HOMESCREEN); Note note = null; @@ -414,6 +412,11 @@ private void sendHomeNote(NotebookSocket conn, Notebook notebook) throws IOExcep } if (note != null) { + if (!note.isReader(userAndRoles)) { + permissionError(conn, "read", userAndRoles, note.getReaders()); + broadcastNoteList(); + return; + } addConnectionToNote(note.id(), conn); conn.send(serializeMessage(new Message(OP.NOTE).put("note", note))); sendAllAngularObjects(note, conn); @@ -423,7 +426,8 @@ private void sendHomeNote(NotebookSocket conn, Notebook notebook) throws IOExcep } } - private void updateNote(WebSocket conn, Notebook notebook, Message fromMessage) + private void updateNote(WebSocket conn, HashSet userAndRoles, + Notebook notebook, Message fromMessage) throws SchedulerException, IOException { String noteId = (String) fromMessage.get("id"); String name = (String) fromMessage.get("name"); @@ -465,7 +469,8 @@ private boolean isCronUpdated(Map configA, return cronUpdated; } - private void createNote(NotebookSocket conn, Notebook notebook, Message message) + private void createNote(NotebookSocket conn, HashSet userAndRoles, + Notebook notebook, Message message) throws IOException { Note note = notebook.createNote(); note.addParagraph(); // it's an empty note. so add one paragraph @@ -483,7 +488,8 @@ private void createNote(NotebookSocket conn, Notebook notebook, Message message) broadcastNoteList(); } - private void removeNote(WebSocket conn, Notebook notebook, Message fromMessage) + private void removeNote(NotebookSocket conn, HashSet userAndRoles, + Notebook notebook, Message fromMessage) throws IOException { String noteId = (String) fromMessage.get("id"); if (noteId == null) { @@ -491,13 +497,19 @@ private void removeNote(WebSocket conn, Notebook notebook, Message fromMessage) } Note note = notebook.getNote(noteId); + + if (!note.isOwner(userAndRoles)) { + permissionError(conn, "remove", userAndRoles, note.getOwners()); + return; + } + notebook.removeNote(noteId); removeNote(noteId); broadcastNoteList(); } - private void updateParagraph(NotebookSocket conn, Notebook notebook, - Message fromMessage) throws IOException { + private void updateParagraph(NotebookSocket conn, HashSet userAndRoles, + Notebook notebook, Message fromMessage) throws IOException { String paragraphId = (String) fromMessage.get("id"); if (paragraphId == null) { return; @@ -509,11 +521,8 @@ private void updateParagraph(NotebookSocket conn, Notebook notebook, .get("config"); final Note note = notebook.getNote(getOpenNoteId(conn)); - HashSet usersAndGroups = new HashSet(); - usersAndGroups.add(fromMessage.principal); - // TODO(prasadwagle) add groups to usersAndGroups - if (!note.isWriter(usersAndGroups)) { - writePermissionError(conn, fromMessage.principal, note); + if (!note.isWriter(userAndRoles)) { + permissionError(conn, "write", userAndRoles, note.getWriters()); return; } @@ -526,7 +535,8 @@ private void updateParagraph(NotebookSocket conn, Notebook notebook, broadcast(note.id(), new Message(OP.PARAGRAPH).put("paragraph", p)); } - private void cloneNote(NotebookSocket conn, Notebook notebook, Message fromMessage) + private void cloneNote(NotebookSocket conn, HashSet userAndRoles, + Notebook notebook, Message fromMessage) throws IOException, CloneNotSupportedException { String noteId = getOpenNoteId(conn); String name = (String) fromMessage.get("name"); @@ -536,7 +546,8 @@ private void cloneNote(NotebookSocket conn, Notebook notebook, Message fromMessa broadcastNoteList(); } - protected Note importNote(NotebookSocket conn, Notebook notebook, Message fromMessage) + protected Note importNote(NotebookSocket conn, HashSet userAndRoles, + Notebook notebook, Message fromMessage) throws IOException { Note note = null; if (fromMessage != null) { @@ -550,8 +561,8 @@ protected Note importNote(NotebookSocket conn, Notebook notebook, Message fromMe return note; } - private void removeParagraph(NotebookSocket conn, Notebook notebook, - Message fromMessage) throws IOException { + private void removeParagraph(NotebookSocket conn, HashSet userAndRoles, + Notebook notebook, Message fromMessage) throws IOException { final String paragraphId = (String) fromMessage.get("id"); if (paragraphId == null) { return; @@ -559,11 +570,8 @@ private void removeParagraph(NotebookSocket conn, Notebook notebook, final Note note = notebook.getNote(getOpenNoteId(conn)); - HashSet usersAndGroups = new HashSet(); - usersAndGroups.add(fromMessage.principal); - // TODO(prasadwagle) add groups to usersAndGroups - if (!note.isWriter(usersAndGroups)) { - writePermissionError(conn, fromMessage.principal, note); + if (!note.isWriter(userAndRoles)) { + permissionError(conn, "write", userAndRoles, note.getWriters()); return; } @@ -575,8 +583,8 @@ private void removeParagraph(NotebookSocket conn, Notebook notebook, } } - private void clearParagraphOutput(NotebookSocket conn, Notebook notebook, - Message fromMessage) throws IOException { + private void clearParagraphOutput(NotebookSocket conn, HashSet userAndRoles, + Notebook notebook, Message fromMessage) throws IOException { final String paragraphId = (String) fromMessage.get("id"); if (paragraphId == null) { return; @@ -584,11 +592,8 @@ private void clearParagraphOutput(NotebookSocket conn, Notebook notebook, final Note note = notebook.getNote(getOpenNoteId(conn)); - HashSet usersAndGroups = new HashSet(); - usersAndGroups.add(fromMessage.principal); - // TODO(prasadwagle) add groups to usersAndGroups - if (!note.isWriter(usersAndGroups)) { - writePermissionError(conn, fromMessage.principal, note); + if (!note.isWriter(userAndRoles)) { + permissionError(conn, "write", userAndRoles, note.getWriters()); return; } @@ -596,7 +601,7 @@ private void clearParagraphOutput(NotebookSocket conn, Notebook notebook, broadcastNote(note); } - private void completion(NotebookSocket conn, Notebook notebook, + private void completion(NotebookSocket conn, HashSet userAndRoles, Notebook notebook, Message fromMessage) throws IOException { String paragraphId = (String) fromMessage.get("id"); String buffer = (String) fromMessage.get("buf"); @@ -620,8 +625,8 @@ private void completion(NotebookSocket conn, Notebook notebook, * @param notebook the notebook. * @param fromMessage the message. */ - private void angularObjectUpdated(NotebookSocket conn, Notebook notebook, - Message fromMessage) { + private void angularObjectUpdated(NotebookSocket conn, HashSet userAndRoles, + Notebook notebook, Message fromMessage) { String noteId = (String) fromMessage.get("noteId"); String paragraphId = (String) fromMessage.get("paragraphId"); String interpreterGroupId = (String) fromMessage.get("interpreterGroupId"); @@ -703,7 +708,7 @@ private void angularObjectUpdated(NotebookSocket conn, Notebook notebook, } } - private void moveParagraph(NotebookSocket conn, Notebook notebook, + private void moveParagraph(NotebookSocket conn, HashSet userAndRoles, Notebook notebook, Message fromMessage) throws IOException { final String paragraphId = (String) fromMessage.get("id"); if (paragraphId == null) { @@ -714,11 +719,8 @@ private void moveParagraph(NotebookSocket conn, Notebook notebook, .toString()); final Note note = notebook.getNote(getOpenNoteId(conn)); - HashSet usersAndGroups = new HashSet(); - usersAndGroups.add(fromMessage.principal); - // TODO(prasadwagle) add groups to usersAndGroups - if (!note.isWriter(usersAndGroups)) { - writePermissionError(conn, fromMessage.principal, note); + if (!note.isWriter(userAndRoles)) { + permissionError(conn, "write", userAndRoles, note.getWriters()); return; } @@ -727,17 +729,14 @@ private void moveParagraph(NotebookSocket conn, Notebook notebook, broadcastNote(note); } - private void insertParagraph(NotebookSocket conn, Notebook notebook, - Message fromMessage) throws IOException { + private void insertParagraph(NotebookSocket conn, HashSet userAndRoles, + Notebook notebook, Message fromMessage) throws IOException { final int index = (int) Double.parseDouble(fromMessage.get("index") .toString()); final Note note = notebook.getNote(getOpenNoteId(conn)); - HashSet usersAndGroups = new HashSet(); - usersAndGroups.add(fromMessage.principal); - // TODO(prasadwagle) add groups to usersAndGroups - if (!note.isWriter(usersAndGroups)) { - writePermissionError(conn, fromMessage.principal, note); + if (!note.isWriter(userAndRoles)) { + permissionError(conn, "write", userAndRoles, note.getWriters()); return; } @@ -746,7 +745,7 @@ private void insertParagraph(NotebookSocket conn, Notebook notebook, broadcastNote(note); } - private void cancelParagraph(NotebookSocket conn, Notebook notebook, + private void cancelParagraph(NotebookSocket conn, HashSet userAndRoles, Notebook notebook, Message fromMessage) throws IOException { final String paragraphId = (String) fromMessage.get("id"); if (paragraphId == null) { @@ -755,11 +754,8 @@ private void cancelParagraph(NotebookSocket conn, Notebook notebook, final Note note = notebook.getNote(getOpenNoteId(conn)); - HashSet usersAndGroups = new HashSet(); - usersAndGroups.add(fromMessage.principal); - // TODO(prasadwagle) add groups to usersAndGroups - if (!note.isWriter(usersAndGroups)) { - writePermissionError(conn, fromMessage.principal, note); + if (!note.isWriter(userAndRoles)) { + permissionError(conn, "write", userAndRoles, note.getWriters()); return; } @@ -767,7 +763,7 @@ private void cancelParagraph(NotebookSocket conn, Notebook notebook, p.abort(); } - private void runParagraph(NotebookSocket conn, Notebook notebook, + private void runParagraph(NotebookSocket conn, HashSet userAndRoles, Notebook notebook, Message fromMessage) throws IOException { final String paragraphId = (String) fromMessage.get("id"); if (paragraphId == null) { @@ -776,11 +772,8 @@ private void runParagraph(NotebookSocket conn, Notebook notebook, final Note note = notebook.getNote(getOpenNoteId(conn)); - HashSet usersAndGroups = new HashSet(); - usersAndGroups.add(fromMessage.principal); - // TODO(prasadwagle) add groups to usersAndGroups - if (!note.isWriter(usersAndGroups)) { - writePermissionError(conn, fromMessage.principal, note); + if (!note.isWriter(userAndRoles)) { + permissionError(conn, "write", userAndRoles, note.getWriters()); return; } @@ -815,8 +808,8 @@ private void runParagraph(NotebookSocket conn, Notebook notebook, } } - private void sendAllConfigurations(NotebookSocket conn, Notebook notebook) - throws IOException { + private void sendAllConfigurations(NotebookSocket conn, HashSet userAndRoles, + Notebook notebook) throws IOException { ZeppelinConfiguration conf = notebook.getConf(); Map configurations = conf.dumpConfigurations(conf, diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java b/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java index 1d06e3a5ebf..e7e39f223a6 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java @@ -23,6 +23,8 @@ import java.net.URI; import java.net.URISyntaxException; import java.net.UnknownHostException; +import java.util.Arrays; +import java.util.HashSet; /** * Tools for securing Zeppelin @@ -52,6 +54,7 @@ public static Boolean isValidOrigin(String sourceHost, ZeppelinConfiguration con */ public static String getPrincipal() { Subject subject = org.apache.shiro.SecurityUtils.getSubject(); + String principal; if (subject.isAuthenticated()) { principal = subject.getPrincipal().toString(); @@ -61,4 +64,24 @@ public static String getPrincipal() { } return principal; } + + /** + * Return the roles associated with the authenticated user if any otherwise returns empty set + * TODO(prasadwagle) Find correct way to get user roles (see SHIRO-492) + * @return shiro roles + */ + public static HashSet getRoles() { + Subject subject = org.apache.shiro.SecurityUtils.getSubject(); + HashSet roles = new HashSet<>(); + + if (subject.isAuthenticated()) { + for (String role : Arrays.asList("role1", "role2", "role3")) { + if (subject.hasRole(role)) { + roles.add(role); + } + } + } + return roles; + } + } diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java index 7d8f3cf710f..774f30ac108 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/socket/NotebookServerTest.java @@ -140,7 +140,7 @@ public void testImportNotebook() throws IOException { Message messageReceived = notebookServer.deserializeMessage(msg); Note note = null; try { - note = notebookServer.importNote(null, notebook, messageReceived); + note = notebookServer.importNote(null, null, notebook, messageReceived); } catch (NullPointerException e) { //broadcastNoteList(); failed nothing to worry. LOG.error("Exception in NotebookServerTest while testImportNotebook, failed nothing to " + diff --git a/zeppelin-web/src/components/websocketEvents/websocketEvents.factory.js b/zeppelin-web/src/components/websocketEvents/websocketEvents.factory.js index 39d785cb985..f40c47f3079 100644 --- a/zeppelin-web/src/components/websocketEvents/websocketEvents.factory.js +++ b/zeppelin-web/src/components/websocketEvents/websocketEvents.factory.js @@ -30,7 +30,8 @@ angular.module('zeppelinWebApp').factory('websocketEvents', function($rootScope, websocketCalls.sendNewEvent = function(data) { data.principal = $rootScope.ticket.principal; data.ticket = $rootScope.ticket.ticket; - console.log('Send >> %o, %o, %o, %o', data.op, data.principal, data.ticket, data); + data.roles = $rootScope.ticket.roles; + console.log('Send >> %o, %o, %o, %o, %o', data.op, data.principal, data.ticket, data.roles, data); websocketCalls.ws.send(JSON.stringify(data)); }; From 6c89dbe93d5b3e0c2d86ff892271a25fc1809e9d Mon Sep 17 00:00:00 2001 From: Prasad Wagle Date: Wed, 10 Feb 2016 13:44:31 -0800 Subject: [PATCH 08/14] Implement Moon's suggestions on note permissions background and wildcard placeholder --- zeppelin-web/src/app/notebook/notebook.css | 22 +++++++++++++++++++++ zeppelin-web/src/app/notebook/notebook.html | 11 ++++++----- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/zeppelin-web/src/app/notebook/notebook.css b/zeppelin-web/src/app/notebook/notebook.css index b41bdb4d181..b1285932ba4 100644 --- a/zeppelin-web/src/app/notebook/notebook.css +++ b/zeppelin-web/src/app/notebook/notebook.css @@ -132,6 +132,28 @@ overflow: hidden !important; } +/* + Note Permissions Panel +*/ + +.permissions { + background: white; + padding: 10px 15px 15px 15px; + margin-left: -10px; + margin-right: -10px; + font-family: 'Roboto', sans-serif; + box-shadow: 0 2px 4px rgba(0, 0, 0, 0.15); + border-bottom: 1px solid #E5E5E5; +} + +.permissions .permissionsForm { + list-style-type: none; + background: #EFEFEF; + padding: 10px 10px 10px 10px; + box-shadow: 0 1px 1px rgba(0, 0, 0, 0.15); + border: 1px solid #E5E5E5; +} + /* Note Setting Panel */ diff --git a/zeppelin-web/src/app/notebook/notebook.html b/zeppelin-web/src/app/notebook/notebook.html index 046f07af625..8c114dd4fd7 100644 --- a/zeppelin-web/src/app/notebook/notebook.html +++ b/zeppelin-web/src/app/notebook/notebook.html @@ -22,13 +22,14 @@

      Note Permissions (Only note owners can change)

      - Enter comma separated users and groups in the fields. Empty set means anyone can do the operation. + Enter comma separated users and groups in the fields.
      + Empty field (*) implies anyone can do the operation.

      -
      -

      Owners : Owners can change permissions, read and write the note.

      -

      Readers : Readers can only read the note.

      -

      Writers : Writers can read and write the note.

      +

      Owners : Owners can change permissions, read and write the note.

      +

      Readers : Readers can only read the note.

      +

      Writers : Writers can read and write the note.


      From 1ac076e1a87e6ee11644cf77d78f59d444e82180 Mon Sep 17 00:00:00 2001 From: Prasad Wagle Date: Wed, 10 Feb 2016 20:54:10 -0800 Subject: [PATCH 09/14] Fixed typo in _navigation.html and updated interpreter_authorization.md --- docs/_includes/themes/zeppelin/_navigation.html | 2 +- docs/security/interpreter_authorization.md | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/docs/_includes/themes/zeppelin/_navigation.html b/docs/_includes/themes/zeppelin/_navigation.html index 00c6938e457..a7da085055d 100644 --- a/docs/_includes/themes/zeppelin/_navigation.html +++ b/docs/_includes/themes/zeppelin/_navigation.html @@ -58,7 +58,7 @@
    • Security
        -
      • Overview
        • +
      • Overview
      • Authentication
      • Notebook Authorization
      • Interpreter Authorization
        • diff --git a/docs/security/interpreter_authorization.md b/docs/security/interpreter_authorization.md index 371ba4a35d2..dc4934b1ec6 100644 --- a/docs/security/interpreter_authorization.md +++ b/docs/security/interpreter_authorization.md @@ -17,12 +17,18 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> -# Zeppelin Interpreter Authorization +# Zeppelin Interpreter and Data Source Authorization -The Interpreter authorization problem is more complex. Different interpreters require different strategies. +## Interpreter Authorization + +Interpreter authorization involves permissions like creating an interpreter and execution queries using it. + +## Data Source Authorization + +Data source authorization involves authenticating to the data source like a Mysql database and letting it determine user permissions. For the Hive interpreter, we need to maintain per-user connection pools. -The interpreter method takes the user string as parameter and executes the jdbc call using a connection in the user's connection pool. +The interpret method takes the user string as parameter and executes the jdbc call using a connection in the user's connection pool. In case of Presto, we don't need password if the Presto DB server runs backend code using HDFS authorization for the user. -For databases like Vertica and Mysql we would have to store password information for users. +For databases like Vertica and Mysql we have to store password information for users. From 28ea69763cf2954d8fae84568a974672c65b18a8 Mon Sep 17 00:00:00 2001 From: Prasad Wagle Date: Mon, 15 Feb 2016 14:56:18 -0800 Subject: [PATCH 10/14] Fixed issues with security documentation reported by @AhyoungRyu --- conf/shiro.ini | 9 +++--- .../themes/zeppelin/_navigation.html | 15 ++++----- docs/security/authentication.md | 2 +- docs/security/interpreter_authorization.md | 2 +- docs/security/notebook_authorization.md | 31 +------------------ docs/security/overview.md | 5 ++- 6 files changed, 16 insertions(+), 48 deletions(-) diff --git a/conf/shiro.ini b/conf/shiro.ini index 1cc3cf809ac..f03b5281489 100644 --- a/conf/shiro.ini +++ b/conf/shiro.ini @@ -19,8 +19,9 @@ # 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 -user1 = password2 -user2 = password3 +user1 = password2, role1, role2 +user2 = password3, role3 +user3 = password4, role2 # Sample LDAP configuration, for user Authentication, currently tested for single Realm [main] @@ -33,6 +34,6 @@ user2 = password3 # anon means the access is anonymous. # authcBasic means Basic Auth Security # To enfore security, comment the line below and uncomment the next one -/** = anon -#/** = authcBasic +#/** = anon +/** = authcBasic diff --git a/docs/_includes/themes/zeppelin/_navigation.html b/docs/_includes/themes/zeppelin/_navigation.html index a7da085055d..d1b0bbbbad6 100644 --- a/docs/_includes/themes/zeppelin/_navigation.html +++ b/docs/_includes/themes/zeppelin/_navigation.html @@ -55,15 +55,6 @@
      • Tajo
      • -
    • - Security - -
    • Display System
        @@ -88,6 +79,12 @@
      • Notebook API
      • Configuration API
        • + +
      • Security Overview
        • +
      • Authentication
        • +
      • Notebook Authorization
        • +
      • Interpreter Authorization
        • +
      • Writing Zeppelin Interpreter
      • How to contribute (code)
        • diff --git a/docs/security/authentication.md b/docs/security/authentication.md index b71d28ad2a4..4483e4244cc 100644 --- a/docs/security/authentication.md +++ b/docs/security/authentication.md @@ -17,7 +17,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> -# Zeppelin Authentication +# Authentication Authentication is company-specific. diff --git a/docs/security/interpreter_authorization.md b/docs/security/interpreter_authorization.md index dc4934b1ec6..d1c792d113f 100644 --- a/docs/security/interpreter_authorization.md +++ b/docs/security/interpreter_authorization.md @@ -17,7 +17,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> -# Zeppelin Interpreter and Data Source Authorization +# Interpreter and Data Source Authorization ## Interpreter Authorization diff --git a/docs/security/notebook_authorization.md b/docs/security/notebook_authorization.md index 6bbaf45218f..b9521b14577 100644 --- a/docs/security/notebook_authorization.md +++ b/docs/security/notebook_authorization.md @@ -17,27 +17,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> -# Zeppelin Notebook Authorization - -## Overview -There are different aspects to Zeppelin security: - -* Authentication: is the user who they say they are? -* Notebook authorization: does the user have permissions to read or write to a note? -* Interpreter authorization: does the user have permissions to perform interpreter operations e.g. access data source objects? - - -## Authentication - -Authentication is company-specific. One option is to have an authentication server that can verify user credentials in an LDAP server. -If an incoming request to the Zeppelin server does not have a cookie with user information encrypted with the authentication server public key, the user -is redirected to the authentication server. Once the user is verified, the authentication server redirects the browser to a specific -URL in the Zeppelin server which sets the authentication cookie in the browser. -The end result is that all requests to the Zeppelin -web server have the authentication cookie which contains user and groups information. - - -## Notebook Authorization +# Notebook Authorization We assume that there is an authentication component that associates a user string and a set of group strings with every NotebookSocket. @@ -55,12 +35,3 @@ operation, it checks if the user and the groups have at least one entity that be To initialize and modify note permissions, we provide UI like "Interpreter binding". The user inputs comma separated entities for owners, readers and writers. We execute a rest api call with this information. In the backend we get the user information for the connection and allow the operation if the user and groups associated with the current user have at least one entity that belongs to owner entities for the note. - -## Interpreter Authorization -The Interpreter authorization problem is more complex. Different interpreters require different strategies. - -For the Hive interpreter, we need to maintain per-user connection pools. -The interpreter method takes the user string as parameter and executes the jdbc call using a connection in the user's connection pool. - -In case of Presto, we don't need password if the Presto DB server runs backend code using HDFS authorization for the user. -For databases like Vertica and Mysql we would have to store password information for users. diff --git a/docs/security/overview.md b/docs/security/overview.md index 7c2631144e3..e76410d13cc 100644 --- a/docs/security/overview.md +++ b/docs/security/overview.md @@ -23,7 +23,6 @@ limitations under the License. There are three aspects to Zeppelin security: -* Authentication: is the user who they say they are? [More](overview.html) +* Authentication: is the user who they say they are? [More](authentication.html) * Notebook authorization: does the user have permissions to read or write to a note? [More](notebook_authorization.html) -* Interpreter authorization: does the user have permissions to perform interpreter operations e.g. access data source objects? [More](interpreter_authorization.html) - +* Interpreter and data source authorization: does the user have permissions to perform interpreter operations or access data source objects? [More](interpreter_authorization.html) From 733530f0fe35e8d0ada2f44653e04060dfdf87d9 Mon Sep 17 00:00:00 2001 From: Prasad Wagle Date: Thu, 25 Feb 2016 13:00:06 -0800 Subject: [PATCH 11/14] Minor doc fix --- docs/security/authentication.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/security/authentication.md b/docs/security/authentication.md index 4483e4244cc..081d41915c4 100644 --- a/docs/security/authentication.md +++ b/docs/security/authentication.md @@ -23,7 +23,7 @@ Authentication is company-specific. One option is to use [Basic Access Authentication](https://en.wikipedia.org/wiki/Basic_access_authentication) -Anoteher is to have an authentication server that can verify user credentials in an LDAP server. +Another option is to have an authentication server that can verify user credentials in an LDAP server. If an incoming request to the Zeppelin server does not have a cookie with user information encrypted with the authentication server public key, the user is redirected to the authentication server. Once the user is verified, the authentication server redirects the browser to a specific URL in the Zeppelin server which sets the authentication cookie in the browser. From 52f491468818634dd6403cfee69f7f3653b2a512 Mon Sep 17 00:00:00 2001 From: Prasad Wagle Date: Thu, 25 Feb 2016 15:22:51 -0800 Subject: [PATCH 12/14] Check whether roles is non-empty before adding to userAndRoles --- .../org/apache/zeppelin/socket/NotebookServer.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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 2b4e6af009b..c53faa841d5 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 @@ -114,9 +114,13 @@ public void onMessage(NotebookSocket conn, String msg) { HashSet userAndRoles = new HashSet(); userAndRoles.add(messagereceived.principal); - HashSet roles = gson.fromJson(messagereceived.roles, - new TypeToken>(){}.getType()); - userAndRoles.addAll(roles); + if (!messagereceived.roles.equals("")) { + HashSet roles = gson.fromJson(messagereceived.roles, + new TypeToken>(){}.getType()); + if (roles != null) { + userAndRoles.addAll(roles); + } + } /** Lets be elegant here */ switch (messagereceived.op) { From 24e8de4a8540da37039b4472e8ab5739e0a320c8 Mon Sep 17 00:00:00 2001 From: Prasad Wagle Date: Thu, 25 Feb 2016 16:01:13 -0800 Subject: [PATCH 13/14] Remove duplicate imports --- .../java/org/apache/zeppelin/socket/NotebookServer.java | 8 -------- 1 file changed, 8 deletions(-) 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 81230e3af7f..3b7da732171 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 @@ -16,14 +16,6 @@ */ package org.apache.zeppelin.socket; -import java.io.IOException; -import java.net.URISyntaxException; -import java.net.UnknownHostException; -import java.util.*; -import java.util.concurrent.ConcurrentLinkedQueue; - -import javax.servlet.http.HttpServletRequest; - import com.google.common.base.Strings; import com.google.gson.Gson; import com.google.gson.reflect.TypeToken; From e7cffd821dfad92012b6449fccb7a7d64300dc7c Mon Sep 17 00:00:00 2001 From: Prasad Wagle Date: Thu, 25 Feb 2016 16:54:47 -0800 Subject: [PATCH 14/14] Restore anon default in shiro.ini --- conf/shiro.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/conf/shiro.ini b/conf/shiro.ini index f03b5281489..8d803d009b5 100644 --- a/conf/shiro.ini +++ b/conf/shiro.ini @@ -34,6 +34,6 @@ user3 = password4, role2 # anon means the access is anonymous. # authcBasic means Basic Auth Security # To enfore security, comment the line below and uncomment the next one -#/** = anon -/** = authcBasic +/** = anon +#/** = authcBasic