You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by ah...@apache.org on 2016/12/28 03:14:50 UTC
zeppelin git commit: [ZEPPELIN-1718] Prevent anonymous user to set
note permission
Repository: zeppelin
Updated Branches:
refs/heads/master 3e274e740 -> 14a38b790
[ZEPPELIN-1718] Prevent anonymous user to set note permission
### What is this PR for?
Currently anonymous user can open the notebook permission page and type sth in `Owner`/ `Reader` / `Writer` and then even can save it. However, in fact, it doesn't work actually.
e.g. An anonymous user can type `admin` / `user1` to the note permission setting fields.
It doesn't make sense. At least we should disallow the non-authenticated users(a.k.a anonymous users) by deactivating those permission related features(will handle interpreter owner setting in another PR). So what I did in this PR is
- Block opening notebook permission setting page \w notebook rest api & add related docs link: https://zeppelin.apache.org/docs/0.7.0-SNAPSHOT/security/notebook_authorization.html
### TODO
- [x] Fix test case
### What type of PR is it?
Bug Fix | Improvement
### What is the Jira issue?
[ZEPPELIN-1718](https://issues.apache.org/jira/browse/ZEPPELIN-1718)
### How should this be tested?
1. Don't activate shiro authentication
2. Open any notes
3. Click lock icon in the top of the note -> warning dialog will be shown \w "Only authenticated user can set the permission." sentence
### Screenshots (if appropriate)
- Doesn't show note permission setting page & show warning dialog if anonymous user tries to click lock icon
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no
Author: AhyoungRyu <fb...@hanmail.net>
Closes #1694 from AhyoungRyu/prevent-anon-user and squashes the following commits:
72ecf6a [AhyoungRyu] Fix test cases based on anon user's limitation
f48f991 [AhyoungRyu] Add anon user checking condition
9e83ff5 [AhyoungRyu] Fix minor check-style
e970e25 [AhyoungRyu] Block anon user note permission setting \w notebook rest api
Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/14a38b79
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/14a38b79
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/14a38b79
Branch: refs/heads/master
Commit: 14a38b79059ef6b80e832ca4070ee02becee1b98
Parents: 3e274e7
Author: AhyoungRyu <fb...@hanmail.net>
Authored: Wed Dec 21 12:29:43 2016 +0900
Committer: ahyoungryu <ah...@apache.org>
Committed: Wed Dec 28 12:14:41 2016 +0900
----------------------------------------------------------------------
.../apache/zeppelin/rest/NotebookRestApi.java | 24 ++++++-
.../zeppelin/rest/NotebookRestApiTest.java | 66 ------------------
.../rest/NotebookSecurityRestApiTest.java | 70 ++++++++++++++++++--
.../zeppelin/rest/ZeppelinRestApiTest.java | 65 ------------------
.../src/app/notebook/notebook.controller.js | 29 ++++++++
zeppelin-web/src/app/notebook/notebook.html | 2 +-
6 files changed, 117 insertions(+), 139 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/14a38b79/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 2b9ba11..d197d6f 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
@@ -93,7 +93,9 @@ public class NotebookRestApi {
@GET
@Path("{noteId}/permissions")
@ZeppelinApi
- public Response getNotePermissions(@PathParam("noteId") String noteId) {
+ public Response getNotePermissions(@PathParam("noteId") String noteId) throws IOException {
+
+ checkIfUserIsAnon(blockNotAuthenticatedUserError());
checkIfUserCanRead(noteId,
"Insufficient privileges you cannot get the list of permissions for this note");
HashMap<String, Set<String>> permissionsMap = new HashMap<>();
@@ -111,12 +113,27 @@ public class NotebookRestApi {
"User belongs to: " + current.toString();
}
+ private String blockNotAuthenticatedUserError() throws IOException {
+ LOG.info("Anonymous user cannot set any permissions for this note.");
+ return "Only authenticated user can set the permission.";
+ }
+
/**
* Set of utils method to check if current user can perform action to the note.
* Since we only have security on notebook level, from now we keep this logic in this class.
* In the future we might want to generalize this for the rest of the api enmdpoints.
*/
-
+
+ /**
+ * Check if the current user is not authenticated(anonymous user) or not
+ */
+ private void checkIfUserIsAnon(String errorMsg) {
+ boolean isAuthenticated = SecurityUtils.isAuthenticated();
+ if (!isAuthenticated) {
+ throw new ForbiddenException(errorMsg);
+ }
+ }
+
/**
* Check if the current user own the given note.
*/
@@ -178,7 +195,8 @@ public class NotebookRestApi {
HashSet<String> userAndRoles = new HashSet<>();
userAndRoles.add(principal);
userAndRoles.addAll(roles);
-
+
+ checkIfUserIsAnon(blockNotAuthenticatedUserError());
checkIfUserIsOwner(noteId,
ownerPermissionError(userAndRoles, notebookAuthorization.getOwners(noteId)));
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/14a38b79/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java
index 1923be8..166a19c 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookRestApiTest.java
@@ -70,70 +70,6 @@ public class NotebookRestApiTest extends AbstractTestRestApi {
}
@Test
- public void testPermissions() throws IOException {
- Note note1 = ZeppelinServer.notebook.createNote(anonymous);
- // Set only readers
- String jsonRequest = "{\"readers\":[\"admin-team\"],\"owners\":[]," +
- "\"writers\":[]}";
- PutMethod put = httpPut("/notebook/" + note1.getId() + "/permissions/", jsonRequest);
- LOG.info("testPermissions response\n" + put.getResponseBodyAsString());
- assertThat("test update method:", put, isAllowed());
- put.releaseConnection();
-
-
- GetMethod get = httpGet("/notebook/" + note1.getId() + "/permissions/");
- assertThat(get, isAllowed());
- Map<String, Object> resp = gson.fromJson(get.getResponseBodyAsString(), new TypeToken<Map<String, Object>>() {
- }.getType());
- Map<String, Set<String>> authInfo = (Map<String, Set<String>>) resp.get("body");
-
- // Check that both owners and writers is set to the princpal if empty
- assertEquals(authInfo.get("readers"), Lists.newArrayList("admin-team"));
- assertEquals(authInfo.get("owners"), Lists.newArrayList("anonymous"));
- assertEquals(authInfo.get("writers"), Lists.newArrayList("anonymous"));
- get.releaseConnection();
-
-
- Note note2 = ZeppelinServer.notebook.createNote(anonymous);
- // Set only writers
- jsonRequest = "{\"readers\":[],\"owners\":[]," +
- "\"writers\":[\"admin-team\"]}";
- put = httpPut("/notebook/" + note2.getId() + "/permissions/", jsonRequest);
- assertThat("test update method:", put, isAllowed());
- put.releaseConnection();
-
- get = httpGet("/notebook/" + note2.getId() + "/permissions/");
- assertThat(get, isAllowed());
- resp = gson.fromJson(get.getResponseBodyAsString(), new TypeToken<Map<String, Object>>() {
- }.getType());
- authInfo = (Map<String, Set<String>>) resp.get("body");
- // Check that owners is set to the princpal if empty
- assertEquals(authInfo.get("owners"), Lists.newArrayList("anonymous"));
- assertEquals(authInfo.get("writers"), Lists.newArrayList("admin-team"));
- get.releaseConnection();
-
-
- // Test clear permissions
- jsonRequest = "{\"readers\":[],\"owners\":[],\"writers\":[]}";
- put = httpPut("/notebook/" + note2.getId() + "/permissions/", jsonRequest);
- put.releaseConnection();
- get = httpGet("/notebook/" + note2.getId() + "/permissions/");
- assertThat(get, isAllowed());
- resp = gson.fromJson(get.getResponseBodyAsString(), new TypeToken<Map<String, Object>>() {
- }.getType());
- authInfo = (Map<String, Set<String>>) resp.get("body");
-
- assertEquals(authInfo.get("readers"), Lists.newArrayList());
- assertEquals(authInfo.get("writers"), Lists.newArrayList());
- assertEquals(authInfo.get("owners"), Lists.newArrayList());
- get.releaseConnection();
- //cleanup
- ZeppelinServer.notebook.removeNote(note1.getId(), anonymous);
- ZeppelinServer.notebook.removeNote(note2.getId(), anonymous);
-
- }
-
- @Test
public void testGetNoteParagraphJobStatus() throws IOException {
Note note1 = ZeppelinServer.notebook.createNote(anonymous);
note1.addParagraph();
@@ -273,5 +209,3 @@ public class NotebookRestApiTest extends AbstractTestRestApi {
ZeppelinServer.notebook.removeNote(note.getId(), anonymous);
}
}
-
-
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/14a38b79/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookSecurityRestApiTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookSecurityRestApiTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookSecurityRestApiTest.java
index 0c714fd..0f4a440 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookSecurityRestApiTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookSecurityRestApiTest.java
@@ -17,11 +17,11 @@
package org.apache.zeppelin.rest;
import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertThat;
+import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
import java.io.IOException;
+import java.util.ArrayList;
import java.util.Map;
import org.apache.commons.httpclient.HttpMethodBase;
@@ -56,6 +56,7 @@ public class NotebookSecurityRestApiTest extends AbstractTestRestApi {
@Before
public void setUp() {}
+
@Test
public void testThatUserCanCreateAndRemoveNote() throws IOException {
@@ -108,7 +109,25 @@ public class NotebookSecurityRestApiTest extends AbstractTestRestApi {
Note deletedNote = ZeppelinServer.notebook.getNote(noteId);
assertNull("Deleted note should be null", deletedNote);
}
-
+
+ @Test
+ public void testThatUserCanSearchNote() throws IOException {
+ String noteId1 = createNoteForUser("test1", "admin", "password1");
+ createParagraphForUser(noteId1, "admin", "password1", "title1", "ThisIsToTestSearchMethodWithPermissions 1");
+
+ String noteId2 = createNoteForUser("test2", "user1", "password2");
+ createParagraphForUser(noteId1, "admin", "password1", "title2", "ThisIsToTestSearchMethodWithPermissions 2");
+
+ //set permission for each note
+ setPermissionForNote(noteId1, "admin", "password1");
+ setPermissionForNote(noteId1, "user1", "password2");
+
+ searchNoteBasedOnPermission("ThisIsToTestSearchMethodWithPermissions", "admin", "password1");
+
+ deleteNoteForUser(noteId1, "admin", "password1");
+ deleteNoteForUser(noteId2, "user1", "password2");
+ }
+
private void userTryRemoveNote(String noteId, String user, String pwd, Matcher<? super HttpMethodBase> m) throws IOException {
DeleteMethod delete = httpDelete(("/notebook/" + noteId), user, pwd);
assertThat(delete, m);
@@ -153,4 +172,47 @@ public class NotebookSecurityRestApiTest extends AbstractTestRestApi {
assertNull("Deleted note should be null", deletedNote);
}
}
+
+ private void createParagraphForUser(String noteId, String user, String pwd, String title, String text) throws IOException {
+ String payload = "{\"title\": \"" + title + "\",\"text\": \"" + text + "\"}";
+ PostMethod post = httpPost(("/notebook/" + noteId + "/paragraph"), payload, user, pwd);
+ post.releaseConnection();
+ }
+
+ private void setPermissionForNote(String noteId, String user, String pwd) throws IOException {
+ String payload = "{\"owners\":[\"" + user + "\"],\"readers\":[\"" + user + "\"],\"writers\":[\"" + user + "\"]}";
+ PutMethod put = httpPut(("/notebook/" + noteId + "/permissions"), payload, user, pwd);
+ put.releaseConnection();
+ }
+
+
+ private void searchNoteBasedOnPermission(String searchText, String user, String pwd) throws IOException{
+ GetMethod searchNote = httpGet(("/notebook/search?q=" + searchText), user, pwd);
+ Map<String, Object> respSearchResult = gson.fromJson(searchNote.getResponseBodyAsString(),
+ new TypeToken<Map<String, Object>>() {
+ }.getType());
+ ArrayList searchBody = (ArrayList) respSearchResult.get("body");
+ assertEquals("At-least one search results is there", true, searchBody.size() >= 1);
+
+ for (int i = 0; i < searchBody.size(); i++) {
+ Map<String, String> searchResult = (Map<String, String>) searchBody.get(i);
+ String userId = searchResult.get("id").split("/", 2)[0];
+
+ GetMethod getPermission = httpGet(("/notebook/" + userId + "/permissions"), user, pwd);
+ Map<String, Object> resp = gson.fromJson(getPermission.getResponseBodyAsString(),
+ new TypeToken<Map<String, Object>>() {
+ }.getType());
+ Map<String, ArrayList> permissions = (Map<String, ArrayList>) resp.get("body");
+ ArrayList owners = permissions.get("owners");
+ ArrayList readers = permissions.get("readers");
+ ArrayList writers = permissions.get("writers");
+
+ if (owners.size() != 0 && readers.size() != 0 && writers.size() != 0) {
+ assertEquals("User has permissions ", true, (owners.contains(user) || readers.contains(user) ||
+ writers.contains(user)));
+ }
+ getPermission.releaseConnection();
+ }
+ searchNote.releaseConnection();
+ }
}
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/14a38b79/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java
index 692b4da..8fa02f4 100644
--- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java
+++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java
@@ -706,70 +706,6 @@ public class ZeppelinRestApiTest extends AbstractTestRestApi {
}
@Test
- public void testSearch() throws IOException {
- Map<String, String> body;
-
- GetMethod getSecurityTicket = httpGet("/security/ticket");
- getSecurityTicket.addRequestHeader("Origin", "http://localhost");
- Map<String, Object> respSecurityTicket = gson.fromJson(getSecurityTicket.getResponseBodyAsString(),
- new TypeToken<Map<String, Object>>() {
- }.getType());
- body = (Map<String, String>) respSecurityTicket.get("body");
- String username = body.get("principal");
- getSecurityTicket.releaseConnection();
-
- Note note1 = ZeppelinServer.notebook.createNote(anonymous);
- String jsonRequest = "{\"title\": \"title1\", \"text\": \"ThisIsToTestSearchMethodWithPermissions 1\"}";
- PostMethod postNoteText = httpPost("/notebook/" + note1.getId() + "/paragraph", jsonRequest);
- postNoteText.releaseConnection();
-
- Note note2 = ZeppelinServer.notebook.createNote(anonymous);
- jsonRequest = "{\"title\": \"title1\", \"text\": \"ThisIsToTestSearchMethodWithPermissions 2\"}";
- postNoteText = httpPost("/notebook/" + note2.getId() + "/paragraph", jsonRequest);
- postNoteText.releaseConnection();
-
- String jsonPermissions = "{\"owners\":[\"" + username + "\"],\"readers\":[\"" + username + "\"],\"writers\":[\"" + username + "\"]}";
- PutMethod putPermission = httpPut("/notebook/" + note1.getId() + "/permissions", jsonPermissions);
- putPermission.releaseConnection();
-
- jsonPermissions = "{\"owners\":[\"admin\"],\"readers\":[\"admin\"],\"writers\":[\"admin\"]}";
- putPermission = httpPut("/notebook/" + note2.getId() + "/permissions", jsonPermissions);
- putPermission.releaseConnection();
-
- GetMethod searchNote = httpGet("/notebook/search?q='ThisIsToTestSearchMethodWithPermissions'");
- searchNote.addRequestHeader("Origin", "http://localhost");
- Map<String, Object> respSearchResult = gson.fromJson(searchNote.getResponseBodyAsString(),
- new TypeToken<Map<String, Object>>() {
- }.getType());
- ArrayList searchBody = (ArrayList) respSearchResult.get("body");
-
- assertEquals("At-least one search results is there", true, searchBody.size() >= 1);
-
- for (int i = 0; i < searchBody.size(); i++) {
- Map<String, String> searchResult = (Map<String, String>) searchBody.get(i);
- String userId = searchResult.get("id").split("/", 2)[0];
- GetMethod getPermission = httpGet("/notebook/" + userId + "/permissions");
- getPermission.addRequestHeader("Origin", "http://localhost");
- Map<String, Object> resp = gson.fromJson(getPermission.getResponseBodyAsString(),
- new TypeToken<Map<String, Object>>() {
- }.getType());
- Map<String, ArrayList> permissions = (Map<String, ArrayList>) resp.get("body");
- ArrayList owners = permissions.get("owners");
- ArrayList readers = permissions.get("readers");
- ArrayList writers = permissions.get("writers");
-
- if (owners.size() != 0 && readers.size() != 0 && writers.size() != 0) {
- assertEquals("User has permissions ", true, (owners.contains(username) || readers.contains(username) ||
- writers.contains(username)));
- }
- getPermission.releaseConnection();
- }
- searchNote.releaseConnection();
- ZeppelinServer.notebook.removeNote(note1.getId(), anonymous);
- ZeppelinServer.notebook.removeNote(note2.getId(), anonymous);
- }
-
- @Test
public void testTitleSearch() throws IOException {
Note note = ZeppelinServer.notebook.createNote(anonymous);
String jsonRequest = "{\"title\": \"testTitleSearchOfParagraph\", \"text\": \"ThisIsToTestSearchMethodWithTitle \"}";
@@ -796,4 +732,3 @@ public class ZeppelinRestApiTest extends AbstractTestRestApi {
}
}
-
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/14a38b79/zeppelin-web/src/app/notebook/notebook.controller.js
----------------------------------------------------------------------
diff --git a/zeppelin-web/src/app/notebook/notebook.controller.js b/zeppelin-web/src/app/notebook/notebook.controller.js
index cbed858..bb95b8d 100644
--- a/zeppelin-web/src/app/notebook/notebook.controller.js
+++ b/zeppelin-web/src/app/notebook/notebook.controller.js
@@ -91,6 +91,34 @@
return value;
};
+ $scope.blockAnonUsers = function() {
+ var principal = $rootScope.ticket.principal;
+ if (principal) {
+ $scope.isAnonymous = principal === 'anonymous' ? true : false;
+ if ($scope.isAnonymous) {
+ var zeppelinVersion = $rootScope.zeppelinVersion;
+ var url = 'https://zeppelin.apache.org/docs/' + zeppelinVersion + '/security/notebook_authorization.html';
+ var content = 'Only authenticated user can set the permission.' +
+ '<a data-toggle="tooltip" data-placement="top" title="Learn more" target="_blank" href=' + url + '>' +
+ '<i class="icon-question" />' +
+ '</a>';
+ BootstrapDialog.show({
+ closable: false,
+ closeByBackdrop: false,
+ closeByKeyboard: false,
+ title: 'No permission',
+ message: content,
+ buttons: [{
+ label: 'Close',
+ action: function(dialog) {
+ dialog.close();
+ }
+ }]
+ });
+ }
+ }
+ };
+
/** Init the new controller */
var initNotebook = function() {
noteVarShareService.clear();
@@ -744,6 +772,7 @@
};
$scope.togglePermissions = function() {
+ $scope.blockAnonUsers();
if ($scope.showPermissions) {
$scope.closePermissions();
angular.element('#selectOwners').select2({});
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/14a38b79/zeppelin-web/src/app/notebook/notebook.html
----------------------------------------------------------------------
diff --git a/zeppelin-web/src/app/notebook/notebook.html b/zeppelin-web/src/app/notebook/notebook.html
index 78a556b..002e950 100644
--- a/zeppelin-web/src/app/notebook/notebook.html
+++ b/zeppelin-web/src/app/notebook/notebook.html
@@ -65,7 +65,7 @@ limitations under the License.
</div>
<!-- permissions -->
- <div ng-if="showPermissions" class="permissions">
+ <div ng-if="showPermissions && ticket.principal && !isAnonymous" class="permissions">
<div>
<h4>Note Permissions (Only note owners can change)</h4>
</div>