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/19 13:55:43 UTC
zeppelin git commit: [ZEPPELIN-1657] Private/public mode for user
note creation/import
Repository: zeppelin
Updated Branches:
refs/heads/master 3eae4e071 -> c507c59b6
[ZEPPELIN-1657] Private/public mode for user note creation/import
### What is this PR for?
In multi-user environment when users create a notebook normally they would expect that notebook be listed only in their list/workbench. Currently Zeppelin creates/imports notes as public by default. There should be at least a way to pass through configuration to make this behaviour private be default. Should discuss whether it's public or private by default.
### What type of PR is it?
Improvement
### Todos
* [x] - set permissions on create/import
* [x] - test
* [x] - review, feedback, decide whether public/private by default
* [x] - pass through env. config
### What is the Jira issue?
[ZEPPELIN-1657](https://issues.apache.org/jira/browse/ZEPPELIN-1657)
### How should this be tested?
1. set `zeppelin.notebook.public` property as false in `zeppelin-site.xml`
2. login as user1 and create noteA, note should appear in your list of notes
3. logout and login as user2, shouldn't be able to see noteA
### Screenshots (if appropriate)
TBD
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? maybe
Author: Khalid Huseynov <kh...@gmail.com>
Closes #1625 from khalidhuseynov/feat/public-note-create and squashes the following commits:
010c262 [Khalid Huseynov] fix test: set state back
fc91b2f [Khalid Huseynov] add description to install config
f862ae1 [Khalid Huseynov] fix InterpreterFactory test
876b798 [Khalid Huseynov] fix vfs testSave npe
1a945e8 [Khalid Huseynov] add test
496b80c [Khalid Huseynov] read env var from conf file directly
28abffa [Khalid Huseynov] set permissions on note create and import
44297a4 [Khalid Huseynov] set new note permissions
9732409 [Khalid Huseynov] add variable to ZeppelinConfiguration
4062af0 [Khalid Huseynov] add var to env.sh
b7f28b3 [Khalid Huseynov] add property to site.xml
Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/c507c59b
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/c507c59b
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/c507c59b
Branch: refs/heads/master
Commit: c507c59b6a166a85a262673fe7f1d4a3020bd599
Parents: 3eae4e0
Author: Khalid Huseynov <kh...@gmail.com>
Authored: Thu Nov 17 14:53:10 2016 +0900
Committer: Lee moon soo <mo...@apache.org>
Committed: Sat Nov 19 05:55:40 2016 -0800
----------------------------------------------------------------------
conf/zeppelin-env.sh.template | 1 +
conf/zeppelin-site.xml.template | 7 ++-
docs/install/install.md | 10 +++-
.../zeppelin/conf/ZeppelinConfiguration.java | 6 ++
.../org/apache/zeppelin/notebook/Notebook.java | 7 +--
.../notebook/NotebookAuthorization.java | 26 ++++++++
.../conf/ZeppelinConfigurationTest.java | 10 ++++
.../interpreter/InterpreterFactoryTest.java | 5 +-
.../apache/zeppelin/notebook/NotebookTest.java | 63 ++++++++++++++++++++
.../notebook/repo/VFSNotebookRepoTest.java | 6 +-
.../src/test/resources/zeppelin-site.xml | 6 ++
11 files changed, 137 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c507c59b/conf/zeppelin-env.sh.template
----------------------------------------------------------------------
diff --git a/conf/zeppelin-env.sh.template b/conf/zeppelin-env.sh.template
index a35adba..50af004 100644
--- a/conf/zeppelin-env.sh.template
+++ b/conf/zeppelin-env.sh.template
@@ -38,6 +38,7 @@
# export ZEPPELIN_INTERPRETER_LOCALREPO # Local repository for interpreter's additional dependency loading
# export ZEPPELIN_NOTEBOOK_STORAGE # Refers to pluggable notebook storage class, can have two classes simultaneously with a sync between them (e.g. local and remote).
# export ZEPPELIN_NOTEBOOK_ONE_WAY_SYNC # If there are multiple notebook storages, should we treat the first one as the only source of truth?
+# export ZEPPELIN_NOTEBOOK_PUBLIC # Make notebook public by default when created, private otherwise
#### Spark interpreter configuration ####
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c507c59b/conf/zeppelin-site.xml.template
----------------------------------------------------------------------
diff --git a/conf/zeppelin-site.xml.template b/conf/zeppelin-site.xml.template
index 36f7e19..bde66b9 100755
--- a/conf/zeppelin-site.xml.template
+++ b/conf/zeppelin-site.xml.template
@@ -278,10 +278,15 @@
</property>
<property>
+ <name>zeppelin.notebook.public</name>
+ <value>true</value>
+ <description>Make notebook public by default when created, private otherwise</description>
+</property>
+
+<property>
<name>zeppelin.websocket.max.text.message.size</name>
<value>1024000</value>
<description>Size in characters of the maximum text message to be received by websocket. Defaults to 1024000</description>
</property>
</configuration>
-
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c507c59b/docs/install/install.md
----------------------------------------------------------------------
diff --git a/docs/install/install.md b/docs/install/install.md
index 4d89657..bccd129 100644
--- a/docs/install/install.md
+++ b/docs/install/install.md
@@ -87,7 +87,7 @@ Congratulations, you have successfully installed Apache Zeppelin! Here are few s
* For an in-depth overview, head to [Explore Apache Zeppelin UI](../quickstart/explorezeppelinui.html).
* And then, try run [tutorial](http://localhost:8080/#/notebook/2A94M5J1Z) notebook in your Zeppelin.
* And see how to change [configurations](#apache-zeppelin-configuration) like port number, etc.
-
+
#### Zeppelin with Apache Spark ...
* To know more about deep integration with [Apache Spark](http://spark.apache.org/), check [Spark Interpreter](../interpreter/spark.html).
@@ -307,6 +307,12 @@ You can configure Apache Zeppelin with either **environment variables** in `conf
<td>If there are multiple notebook storage locations, should we treat the first one as the only source of truth?</td>
</tr>
<tr>
+ <td>ZEPPELIN_NOTEBOOK_PUBLIC</td>
+ <td>zeppelin.notebook.public</td>
+ <td>true</td>
+ <td>Make notebook public (set only `owners`) by default when created/imported. If set to `false` will add `user` to `readers` and `writers` as well, making it private and invisible to other users unless permissions are granted.</td>
+ </tr>
+ <tr>
<td>ZEPPELIN_INTERPRETERS</td>
<td>zeppelin.interpreters</td>
<description></description>
@@ -377,4 +383,4 @@ exec bin/zeppelin-daemon.sh upstart
## Building from Source
-If you want to build from source instead of using binary package, follow the instructions [here](./build.html).
\ No newline at end of file
+If you want to build from source instead of using binary package, follow the instructions [here](./build.html).
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c507c59b/zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
index d99c529..368458b 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java
@@ -435,6 +435,10 @@ public class ZeppelinConfiguration extends XMLConfiguration {
return getBoolean(ConfVars.ZEPPELIN_ANONYMOUS_ALLOWED);
}
+ public boolean isNotebokPublic() {
+ return getBoolean(ConfVars.ZEPPELIN_NOTEBOOK_PUBLIC);
+ }
+
public String getConfDir() {
return getString(ConfVars.ZEPPELIN_CONF_DIR);
}
@@ -570,6 +574,8 @@ public class ZeppelinConfiguration extends XMLConfiguration {
ZEPPELIN_NOTEBOOK_AZURE_USER("zeppelin.notebook.azure.user", "user"),
ZEPPELIN_NOTEBOOK_STORAGE("zeppelin.notebook.storage", VFSNotebookRepo.class.getName()),
ZEPPELIN_NOTEBOOK_ONE_WAY_SYNC("zeppelin.notebook.one.way.sync", false),
+ // whether by default note is public or private
+ ZEPPELIN_NOTEBOOK_PUBLIC("zeppelin.notebook.public", true),
ZEPPELIN_INTERPRETER_REMOTE_RUNNER("zeppelin.interpreter.remoterunner",
System.getProperty("os.name")
.startsWith("Windows") ? "bin/interpreter.cmd" : "bin/interpreter.sh"),
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c507c59b/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 cee3d68..c093256 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
@@ -166,11 +166,7 @@ public class Notebook implements NoteEventListener {
bindInterpretersToNote(subject.getUser(), note.getId(), interpreterIds);
}
- if (subject != null && !"anonymous".equals(subject.getUser())) {
- Set<String> owners = new HashSet<>();
- owners.add(subject.getUser());
- notebookAuthorization.setOwners(note.getId(), owners);
- }
+ notebookAuthorization.setNewNotePermissions(note.getId(), subject);
noteSearchService.addIndexDoc(note);
note.persist(subject);
fireNoteCreateEvent(note);
@@ -225,6 +221,7 @@ public class Notebook implements NoteEventListener {
newNote.addCloneParagraph(p);
}
+ notebookAuthorization.setNewNotePermissions(newNote.getId(), subject);
newNote.persist(subject);
} catch (IOException e) {
logger.error(e.toString(), e);
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c507c59b/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 e486e7c..8fcecf4 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
@@ -156,6 +156,10 @@ public class NotebookAuthorization {
LOG.error("Error saving notebook authorization file: " + e.getMessage());
}
}
+
+ public boolean isPublic() {
+ return conf.isNotebokPublic();
+ }
private Set<String> validateUser(Set<String> users) {
Set<String> returnUser = new HashSet<>();
@@ -325,4 +329,26 @@ public class NotebookAuthorization {
}
}).toList();
}
+
+ public void setNewNotePermissions(String noteId, AuthenticationInfo subject) {
+ if (!AuthenticationInfo.isAnonymous(subject)) {
+ if (isPublic()) {
+ // add current user to owners - can be public
+ Set<String> owners = getOwners(noteId);
+ owners.add(subject.getUser());
+ setOwners(noteId, owners);
+ } else {
+ // add current user to owners, readers, writers - private note
+ Set<String> entities = getOwners(noteId);
+ entities.add(subject.getUser());
+ setOwners(noteId, entities);
+ entities = getReaders(noteId);
+ entities.add(subject.getUser());
+ setReaders(noteId, entities);
+ entities = getWriters(noteId);
+ entities.add(subject.getUser());
+ setWriters(noteId, entities);
+ }
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c507c59b/zeppelin-zengine/src/test/java/org/apache/zeppelin/conf/ZeppelinConfigurationTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/conf/ZeppelinConfigurationTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/conf/ZeppelinConfigurationTest.java
index f9d8ca3..3cc1022 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/conf/ZeppelinConfigurationTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/conf/ZeppelinConfigurationTest.java
@@ -24,6 +24,8 @@ import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars;
import org.junit.Before;
import org.junit.Test;
+import static org.junit.Assert.assertTrue;
+
import java.net.MalformedURLException;
import java.util.List;
@@ -87,4 +89,12 @@ public class ZeppelinConfigurationTest {
String notebookLocation = conf.getNotebookDir();
Assert.assertEquals("notebook", notebookLocation);
}
+
+ @Test
+ public void isNotebookPublicTest() throws ConfigurationException {
+
+ ZeppelinConfiguration conf = new ZeppelinConfiguration(this.getClass().getResource("/zeppelin-site.xml"));
+ boolean isIt = conf.isNotebokPublic();
+ assertTrue(isIt);
+ }
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c507c59b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java
index 9b3b3ef..c88779c 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/InterpreterFactoryTest.java
@@ -42,6 +42,7 @@ import org.apache.zeppelin.interpreter.remote.RemoteInterpreter;
import org.apache.zeppelin.notebook.JobListenerFactory;
import org.apache.zeppelin.notebook.Note;
import org.apache.zeppelin.notebook.Notebook;
+import org.apache.zeppelin.notebook.NotebookAuthorization;
import org.apache.zeppelin.notebook.repo.NotebookRepo;
import org.apache.zeppelin.notebook.repo.VFSNotebookRepo;
import org.apache.zeppelin.scheduler.SchedulerFactory;
@@ -66,6 +67,7 @@ public class InterpreterFactoryTest {
private NotebookRepo notebookRepo;
private DependencyResolver depResolver;
private SchedulerFactory schedulerFactory;
+ private NotebookAuthorization notebookAuthorization;
@Mock
private JobListenerFactory jobListenerFactory;
@@ -97,8 +99,9 @@ public class InterpreterFactoryTest {
SearchService search = mock(SearchService.class);
notebookRepo = new VFSNotebookRepo(conf);
+ notebookAuthorization = NotebookAuthorization.init(conf);
notebook = new Notebook(conf, notebookRepo, schedulerFactory, factory, jobListenerFactory, search,
- null, null);
+ notebookAuthorization, null);
}
@After
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c507c59b/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 ae81281..d5d6bca 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
@@ -1062,6 +1062,69 @@ public class NotebookTest implements JobListenerFactory{
assertEquals(notes2.size(), 1);
}
+ @Test
+ public void testPublicPrivateNewNote() throws IOException, SchedulerException {
+ HashSet<String> user1 = Sets.newHashSet("user1");
+ HashSet<String> user2 = Sets.newHashSet("user2");
+
+ // case of public note
+ assertTrue(conf.isNotebokPublic());
+ assertTrue(notebookAuthorization.isPublic());
+
+ List<Note> notes1 = notebook.getAllNotes(user1);
+ List<Note> notes2 = notebook.getAllNotes(user2);
+ assertEquals(notes1.size(), 0);
+ assertEquals(notes2.size(), 0);
+
+ // user1 creates note
+ Note notePublic = notebook.createNote(new AuthenticationInfo("user1"));
+
+ // both users have note
+ notes1 = notebook.getAllNotes(user1);
+ notes2 = notebook.getAllNotes(user2);
+ assertEquals(notes1.size(), 1);
+ assertEquals(notes2.size(), 1);
+ assertEquals(notes1.get(0).getId(), notePublic.getId());
+ assertEquals(notes2.get(0).getId(), notePublic.getId());
+
+ // user1 is only owner
+ assertEquals(notebookAuthorization.getOwners(notePublic.getId()).size(), 1);
+ assertEquals(notebookAuthorization.getReaders(notePublic.getId()).size(), 0);
+ assertEquals(notebookAuthorization.getWriters(notePublic.getId()).size(), 0);
+
+ // case of private note
+ System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_PUBLIC.getVarName(), "false");
+ ZeppelinConfiguration conf2 = ZeppelinConfiguration.create();
+ assertFalse(conf2.isNotebokPublic());
+ // notebook authorization reads from conf, so no need to re-initilize
+ assertFalse(notebookAuthorization.isPublic());
+
+ // check that still 1 note per user
+ notes1 = notebook.getAllNotes(user1);
+ notes2 = notebook.getAllNotes(user2);
+ assertEquals(notes1.size(), 1);
+ assertEquals(notes2.size(), 1);
+
+ // create private note
+ Note notePrivate = notebook.createNote(new AuthenticationInfo("user1"));
+
+ // only user1 have notePrivate right after creation
+ notes1 = notebook.getAllNotes(user1);
+ notes2 = notebook.getAllNotes(user2);
+ assertEquals(notes1.size(), 2);
+ assertEquals(notes2.size(), 1);
+ assertEquals(notes1.get(1).getId(), notePrivate.getId());
+
+ // user1 have all rights
+ assertEquals(notebookAuthorization.getOwners(notePrivate.getId()).size(), 1);
+ assertEquals(notebookAuthorization.getReaders(notePrivate.getId()).size(), 1);
+ assertEquals(notebookAuthorization.getWriters(notePrivate.getId()).size(), 1);
+
+ //set back public to true
+ System.setProperty(ConfVars.ZEPPELIN_NOTEBOOK_PUBLIC.getVarName(), "true");
+ ZeppelinConfiguration.create();
+ }
+
private void delete(File file){
if(file.isFile()) file.delete();
else if(file.isDirectory()){
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c507c59b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java
index f40d5f8..c8009e7 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/VFSNotebookRepoTest.java
@@ -35,6 +35,7 @@ import org.apache.zeppelin.interpreter.mock.MockInterpreter1;
import org.apache.zeppelin.notebook.JobListenerFactory;
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.notebook.ParagraphJobListener;
import org.apache.zeppelin.scheduler.SchedulerFactory;
@@ -56,6 +57,7 @@ public class VFSNotebookRepoTest implements JobListenerFactory {
private NotebookRepo notebookRepo;
private InterpreterFactory factory;
private DependencyResolver depResolver;
+ private NotebookAuthorization notebookAuthorization;
private File mainZepDir;
private File mainNotebookDir;
@@ -86,7 +88,9 @@ public class VFSNotebookRepoTest implements JobListenerFactory {
SearchService search = mock(SearchService.class);
notebookRepo = new VFSNotebookRepo(conf);
- notebook = new Notebook(conf, notebookRepo, schedulerFactory, factory, this, search, null, null);
+ notebookAuthorization = NotebookAuthorization.init(conf);
+ notebook = new Notebook(conf, notebookRepo, schedulerFactory, factory, this, search,
+ notebookAuthorization, null);
}
@After
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/c507c59b/zeppelin-zengine/src/test/resources/zeppelin-site.xml
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/resources/zeppelin-site.xml b/zeppelin-zengine/src/test/resources/zeppelin-site.xml
index f34540a..a8adc24 100644
--- a/zeppelin-zengine/src/test/resources/zeppelin-site.xml
+++ b/zeppelin-zengine/src/test/resources/zeppelin-site.xml
@@ -148,5 +148,11 @@
</property>
-->
+<property>
+ <name>zeppelin.notebook.public</name>
+ <value>true</value>
+ <description>Make notebook public by default when created, private otherwise</description>
+</property>
+
</configuration>