From 4a43b07c05026698fe268b4b45913a3b0f007f74 Mon Sep 17 00:00:00 2001 From: Anthony Corbacho Date: Fri, 28 Oct 2016 14:49:10 +0900 Subject: [PATCH 01/15] Add new method on ZeppelinConfiguration to get is zeppelin is running on anonimous mode or not --- .../org/apache/zeppelin/conf/ZeppelinConfiguration.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 88cc4ee4aa8..b972fff3843 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 @@ -344,7 +344,7 @@ public String getTrustStorePassword() { public String getNotebookDir() { return getString(ConfVars.ZEPPELIN_NOTEBOOK_DIR); } - + public String getUser() { return getString(ConfVars.ZEPPELIN_NOTEBOOK_S3_USER); } @@ -430,6 +430,10 @@ public String getRelativeDir(String path) { public boolean isWindowsPath(String path){ return path.matches("^[A-Za-z]:\\\\.*"); } + + public boolean isAnonymousAllowed() { + return getBoolean(ConfVars.ZEPPELIN_ANONYMOUS_ALLOWED); + } public String getConfDir() { return getString(ConfVars.ZEPPELIN_CONF_DIR); From da3415f3b3c12d72e0466a736ccaa0a3231c79ed Mon Sep 17 00:00:00 2001 From: Anthony Corbacho Date: Fri, 28 Oct 2016 16:22:13 +0900 Subject: [PATCH 02/15] Add new method to help to determinate if user is part of writer and/or owner for the given note --- .../notebook/NotebookAuthorization.java | 49 ++++++++++++++++--- 1 file changed, 42 insertions(+), 7 deletions(-) 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 c199c968f8f..14e240bdac0 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 @@ -17,18 +17,33 @@ package org.apache.zeppelin.notebook; -import com.google.common.base.Predicate; -import com.google.common.collect.FluentIterable; -import com.google.common.collect.Sets; -import com.google.gson.Gson; -import com.google.gson.GsonBuilder; +import java.io.BufferedReader; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.OutputStreamWriter; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.apache.commons.lang3.StringUtils; import org.apache.zeppelin.conf.ZeppelinConfiguration; import org.apache.zeppelin.user.AuthenticationInfo; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.*; -import java.util.*; +import com.google.common.base.Predicate; +import com.google.common.collect.FluentIterable; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; /** * Contains authorization information for notes @@ -238,6 +253,26 @@ private boolean isMember(Set a, Set b) { intersection.retainAll(a); return (b.isEmpty() || (intersection.size() > 0)); } + + public boolean isUserOwnerOrWriter(String user, String noteId) { + if (StringUtils.isBlank(user)) { + LOG.error("Cannot check user permission with blank login"); + return false; + } + Set users = ImmutableSet.of(user); + return isUserOwnerOrWriter(users, noteId); + } + + public boolean isUserOwnerOrWriter(Set users, String noteId) { + if (conf.isAnonymousAllowed()) { + LOG.debug("Zeppelin runs in anonymous mode"); + return true; + } + if (users == null) { + return false; + } + return isWriter(noteId, users); + } public void removeNote(String noteId) { authInfo.remove(noteId); From 0e4cc3c974cb52b8e6e039375c4748bc89665201 Mon Sep 17 00:00:00 2001 From: Anthony Corbacho Date: Fri, 28 Oct 2016 18:56:04 +0900 Subject: [PATCH 03/15] Add new method to check if user and roles are member of the note (at least owner, reader, writer) --- .../notebook/NotebookAuthorization.java | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) 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 14e240bdac0..8d95ffe8af7 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 @@ -32,7 +32,6 @@ import java.util.Map; import java.util.Set; -import org.apache.commons.lang3.StringUtils; import org.apache.zeppelin.conf.ZeppelinConfiguration; import org.apache.zeppelin.user.AuthenticationInfo; import org.slf4j.Logger; @@ -40,7 +39,6 @@ import com.google.common.base.Predicate; import com.google.common.collect.FluentIterable; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -253,25 +251,16 @@ private boolean isMember(Set a, Set b) { intersection.retainAll(a); return (b.isEmpty() || (intersection.size() > 0)); } - - public boolean isUserOwnerOrWriter(String user, String noteId) { - if (StringUtils.isBlank(user)) { - LOG.error("Cannot check user permission with blank login"); - return false; - } - Set users = ImmutableSet.of(user); - return isUserOwnerOrWriter(users, noteId); - } - - public boolean isUserOwnerOrWriter(Set users, String noteId) { + + public boolean isUserOwnerOrWriter(Set userAndRoles, String noteId) { if (conf.isAnonymousAllowed()) { LOG.debug("Zeppelin runs in anonymous mode"); return true; } - if (users == null) { + if (userAndRoles == null) { return false; } - return isWriter(noteId, users); + return isWriter(noteId, userAndRoles); } public void removeNote(String noteId) { From 21f9288ef7225f56c6a9d92d26209b7440d64c6a Mon Sep 17 00:00:00 2001 From: Anthony Corbacho Date: Fri, 28 Oct 2016 18:57:06 +0900 Subject: [PATCH 04/15] Replace check of aninonimous by method --- .../zeppelin/socket/NotebookServer.java | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 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 28a9ac36da0..4934265424a 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,10 +16,22 @@ */ package org.apache.zeppelin.socket; -import com.google.common.base.Strings; -import com.google.gson.Gson; -import com.google.gson.GsonBuilder; -import com.google.gson.reflect.TypeToken; +import java.io.IOException; +import java.net.URISyntaxException; +import java.net.UnknownHostException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Queue; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedQueue; + +import javax.servlet.http.HttpServletRequest; + import org.apache.commons.lang.StringUtils; import org.apache.commons.vfs2.FileSystemException; import org.apache.zeppelin.conf.ZeppelinConfiguration; @@ -36,7 +48,13 @@ import org.apache.zeppelin.interpreter.remote.RemoteAngularObjectRegistry; import org.apache.zeppelin.interpreter.remote.RemoteInterpreterProcessListener; import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion; -import org.apache.zeppelin.notebook.*; +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.NotebookEventListener; +import org.apache.zeppelin.notebook.Paragraph; +import org.apache.zeppelin.notebook.ParagraphJobListener; import org.apache.zeppelin.notebook.repo.NotebookRepo.Revision; import org.apache.zeppelin.notebook.socket.Message; import org.apache.zeppelin.notebook.socket.Message.OP; @@ -54,13 +72,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.servlet.http.HttpServletRequest; -import java.io.IOException; -import java.net.URISyntaxException; -import java.net.UnknownHostException; -import java.util.*; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentLinkedQueue; +import com.google.common.base.Strings; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.google.gson.reflect.TypeToken; /** * Zeppelin websocket service. @@ -147,8 +162,7 @@ public void onMessage(NotebookSocket conn, String msg) { } ZeppelinConfiguration conf = ZeppelinConfiguration.create(); - boolean allowAnonymous = conf. - getBoolean(ZeppelinConfiguration.ConfVars.ZEPPELIN_ANONYMOUS_ALLOWED); + boolean allowAnonymous = conf.isAnonymousAllowed(); if (!allowAnonymous && messagereceived.principal.equals("anonymous")) { throw new Exception("Anonymous access not allowed "); } From fe380abf6fc839f316a0eeaae43b3a1e44d58df9 Mon Sep 17 00:00:00 2001 From: Anthony Corbacho Date: Fri, 28 Oct 2016 20:19:32 +0900 Subject: [PATCH 05/15] Add webapp exception handler :) --- .../rest/exception/NotFoundException.java | 59 +++++++++++++++++++ .../rest/exception/UnauthorizedException.java | 50 ++++++++++++++++ .../apache/zeppelin/utils/ExceptionUtils.java | 36 +++++++++++ 3 files changed, 145 insertions(+) create mode 100644 zeppelin-server/src/main/java/org/apache/zeppelin/rest/exception/NotFoundException.java create mode 100644 zeppelin-server/src/main/java/org/apache/zeppelin/rest/exception/UnauthorizedException.java create mode 100644 zeppelin-server/src/main/java/org/apache/zeppelin/utils/ExceptionUtils.java diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/exception/NotFoundException.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/exception/NotFoundException.java new file mode 100644 index 00000000000..7f9c17d9e22 --- /dev/null +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/exception/NotFoundException.java @@ -0,0 +1,59 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * 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. + */ +package org.apache.zeppelin.rest.exception; +import static javax.ws.rs.core.Response.Status.NOT_FOUND; + +import javax.ws.rs.WebApplicationException; +import javax.ws.rs.core.Response; + +import org.apache.zeppelin.utils.ExceptionUtils; + +/** + * Not Found handler for WebApplicationException. + * + */ +public class NotFoundException extends WebApplicationException { + private static final long serialVersionUID = 2459398393216512293L; + + /** + * Create a HTTP 404 (Not Found) exception. + */ + public NotFoundException() { + super(ExceptionUtils.jsonResponse(NOT_FOUND)); + } + + /** + * Create a HTTP 404 (Not Found) exception. + * @param message the String that is the entity of the 404 response. + */ + public NotFoundException(String message) { + super(notFoundJson(message)); + } + + private static Response notFoundJson(String message) { + return ExceptionUtils.jsonResponseContent(NOT_FOUND, message); + } + + public NotFoundException(Throwable cause) { + super(cause, notFoundJson(cause.getMessage())); + } + + public NotFoundException(Throwable cause, String message) { + super(cause, notFoundJson(message)); + } + +} diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/exception/UnauthorizedException.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/exception/UnauthorizedException.java new file mode 100644 index 00000000000..7b968abaddc --- /dev/null +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/exception/UnauthorizedException.java @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * 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. + */ +package org.apache.zeppelin.rest.exception; + +import static javax.ws.rs.core.Response.Status.FORBIDDEN; + +import javax.ws.rs.WebApplicationException; +import javax.ws.rs.core.Response; + +import org.apache.zeppelin.utils.ExceptionUtils; + +/** + * UnauthorizedException handler for WebApplicationException. + * + */ +public class UnauthorizedException extends WebApplicationException { + private static final long serialVersionUID = 4394749068760407567L; + private static final String UNAUTHORIZED_MSG = "Authorization required"; + + public UnauthorizedException() { + super(unauthorizedJson(UNAUTHORIZED_MSG)); + } + + private static Response unauthorizedJson(String message) { + return ExceptionUtils.jsonResponseContent(FORBIDDEN, message); + } + + public UnauthorizedException(Throwable cause, String message) { + super(cause, unauthorizedJson(message)); + } + + public UnauthorizedException(String message) { + super(unauthorizedJson(message)); + } + +} diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/utils/ExceptionUtils.java b/zeppelin-server/src/main/java/org/apache/zeppelin/utils/ExceptionUtils.java new file mode 100644 index 00000000000..ce87e5e88ac --- /dev/null +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/utils/ExceptionUtils.java @@ -0,0 +1,36 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * 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. + */ +package org.apache.zeppelin.utils; + +import javax.ws.rs.core.Response.Status; + +import org.apache.zeppelin.server.JsonResponse; + +/** + * Utility method for exception in rest api. + * + */ +public class ExceptionUtils { + + public static javax.ws.rs.core.Response jsonResponse(Status status) { + return new JsonResponse<>(status).build(); + } + + public static javax.ws.rs.core.Response jsonResponseContent(Status status, String message) { + return new JsonResponse<>(status, message).build(); + } +} From 6030776cd867f7575010ee7fa99e0b0782d308dd Mon Sep 17 00:00:00 2001 From: Anthony Corbacho Date: Fri, 28 Oct 2016 20:19:54 +0900 Subject: [PATCH 06/15] Handle security check --- .../apache/zeppelin/rest/NotebookRestApi.java | 202 ++++++++++-------- 1 file changed, 110 insertions(+), 92 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 ae66330785d..e543a1c5aa5 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 @@ -23,6 +23,7 @@ import java.util.List; import java.util.Map; import java.util.Set; + import javax.ws.rs.DELETE; import javax.ws.rs.GET; import javax.ws.rs.POST; @@ -34,31 +35,33 @@ import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; -import com.google.common.collect.Sets; -import com.google.common.reflect.TypeToken; -import com.google.gson.Gson; import org.apache.commons.lang3.StringUtils; -import org.apache.zeppelin.interpreter.InterpreterResult; -import org.apache.zeppelin.utils.InterpreterBindingUtils; -import org.quartz.CronExpression; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.apache.zeppelin.annotation.ZeppelinApi; +import org.apache.zeppelin.interpreter.InterpreterResult; import org.apache.zeppelin.notebook.Note; import org.apache.zeppelin.notebook.Notebook; import org.apache.zeppelin.notebook.NotebookAuthorization; import org.apache.zeppelin.notebook.Paragraph; +import org.apache.zeppelin.rest.exception.UnauthorizedException; import org.apache.zeppelin.rest.message.CronRequest; -import org.apache.zeppelin.types.InterpreterSettingsList; import org.apache.zeppelin.rest.message.NewNoteRequest; import org.apache.zeppelin.rest.message.NewParagraphRequest; import org.apache.zeppelin.rest.message.RunParagraphWithParametersRequest; import org.apache.zeppelin.search.SearchService; import org.apache.zeppelin.server.JsonResponse; import org.apache.zeppelin.socket.NotebookServer; +import org.apache.zeppelin.types.InterpreterSettingsList; import org.apache.zeppelin.user.AuthenticationInfo; +import org.apache.zeppelin.utils.InterpreterBindingUtils; import org.apache.zeppelin.utils.SecurityUtils; +import org.openqa.selenium.NotFoundException; +import org.quartz.CronExpression; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.collect.Sets; +import com.google.common.reflect.TypeToken; +import com.google.gson.Gson; /** * Rest api endpoint for the notebook. @@ -90,6 +93,8 @@ public NotebookRestApi(Notebook notebook, NotebookServer notebookServer, SearchS @Path("{noteId}/permissions") @ZeppelinApi public Response getNotePermissions(@PathParam("noteId") String noteId) { + checkIfUserIsMember(noteId, + "Insufficient privileges you cannot get the list of permissions for this note"); HashMap> permissionsMap = new HashMap<>(); permissionsMap.put("owners", notebookAuthorization.getOwners(noteId)); permissionsMap.put("readers", notebookAuthorization.getReaders(noteId)); @@ -105,6 +110,41 @@ private String ownerPermissionError(Set current, Set allowed) th "User belongs to: " + current.toString(); } + /** + * 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. + */ + private void checkIfUserIsOwnerOrWriter(String noteId, String errorMsg) { + Set userAndRoles = Sets.newHashSet(); + userAndRoles.add(SecurityUtils.getPrincipal()); + userAndRoles.addAll(SecurityUtils.getRoles()); + if (!notebookAuthorization.isUserOwnerOrWriter(userAndRoles, noteId)) { + throw new UnauthorizedException(errorMsg); + } + } + + private void checkIfUserIsMember(String noteId, String errorMsg) { + Set userAndRoles = Sets.newHashSet(); + userAndRoles.add(SecurityUtils.getPrincipal()); + userAndRoles.addAll(SecurityUtils.getRoles()); + if (!notebookAuthorization.isUserOwnerOrWriter(userAndRoles, noteId)) { + throw new UnauthorizedException(errorMsg); + } + } + + private void checkIfNoteIsNotNull(Note note) { + if (note == null) { + throw new NotFoundException("note not found"); + } + } + + private void checkIfParagraphIsNotNull(Paragraph paragraph) { + if (paragraph == null) { + throw new NotFoundException("paragraph not found"); + } + } + /** * set note authorization information */ @@ -113,22 +153,21 @@ private String ownerPermissionError(Set current, Set allowed) th @ZeppelinApi public Response putNotePermissions(@PathParam("noteId") String noteId, String req) throws IOException { - HashMap> permMap = - gson.fromJson(req, new TypeToken>>() { - }.getType()); - Note note = notebook.getNote(noteId); String principal = SecurityUtils.getPrincipal(); HashSet roles = SecurityUtils.getRoles(); - LOG.info("Set permissions {} {} {} {} {}", noteId, principal, permMap.get("owners"), - permMap.get("readers"), permMap.get("writers")); - HashSet userAndRoles = new HashSet<>(); userAndRoles.add(principal); userAndRoles.addAll(roles); - if (!notebookAuthorization.isOwner(noteId, userAndRoles)) { - return new JsonResponse<>(Status.FORBIDDEN, - ownerPermissionError(userAndRoles, notebookAuthorization.getOwners(noteId))).build(); - } + + checkIfUserIsOwnerOrWriter(noteId, + ownerPermissionError(userAndRoles, notebookAuthorization.getOwners(noteId))); + + HashMap> permMap = + gson.fromJson(req, new TypeToken>>() {}.getType()); + Note note = notebook.getNote(noteId); + + LOG.info("Set permissions {} {} {} {} {}", noteId, principal, permMap.get("owners"), + permMap.get("readers"), permMap.get("writers")); HashSet readers = permMap.get("readers"); HashSet owners = permMap.get("owners"); @@ -170,6 +209,9 @@ public Response putNotePermissions(@PathParam("noteId") String noteId, String re @Path("interpreter/bind/{noteId}") @ZeppelinApi public Response bind(@PathParam("noteId") String noteId, String req) throws IOException { + checkIfUserIsOwnerOrWriter(noteId, + "Insufficient privileges you cannot bind any interpreters to this note"); + List settingIdList = gson.fromJson(req, new TypeToken>() { }.getType()); notebook.bindInterpretersToNote(SecurityUtils.getPrincipal(), noteId, settingIdList); @@ -204,10 +246,9 @@ public Response getNoteList() throws IOException { @ZeppelinApi public Response getNote(@PathParam("noteId") String noteId) throws IOException { Note note = notebook.getNote(noteId); - if (note == null) { - return new JsonResponse<>(Status.NOT_FOUND, "note not found.").build(); - } - + checkIfNoteIsNotNull(note); + checkIfUserIsMember(noteId, + "Insufficient privileges you cannot get this note"); return new JsonResponse<>(Status.OK, "", note).build(); } @@ -222,6 +263,8 @@ public Response getNote(@PathParam("noteId") String noteId) throws IOException { @Path("export/{noteId}") @ZeppelinApi public Response exportNote(@PathParam("noteId") String noteId) throws IOException { + checkIfUserIsMember(noteId, + "Insufficient privileges you cannot export this note"); String exportJson = notebook.exportNote(noteId); return new JsonResponse<>(Status.OK, "", exportJson).build(); } @@ -291,6 +334,7 @@ public Response createNote(String message) throws IOException { public Response deleteNote(@PathParam("noteId") String noteId) throws IOException { LOG.info("Delete note {} ", noteId); AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal()); + checkIfUserIsOwnerOrWriter(noteId, "Insufficient privileges you cannot remove this note"); if (!(noteId.isEmpty())) { Note note = notebook.getNote(noteId); if (note != null) { @@ -315,6 +359,7 @@ public Response deleteNote(@PathParam("noteId") String noteId) throws IOExceptio public Response cloneNote(@PathParam("noteId") String noteId, String message) throws IOException, CloneNotSupportedException, IllegalArgumentException { LOG.info("clone note by JSON {}", message); + checkIfUserIsOwnerOrWriter(noteId, "Insufficient privileges you cannot clone this note"); NewNoteRequest request = gson.fromJson(message, NewNoteRequest.class); String newNoteName = null; if (request != null) { @@ -342,9 +387,9 @@ public Response insertParagraph(@PathParam("noteId") String noteId, String messa LOG.info("insert paragraph {} {}", noteId, message); Note note = notebook.getNote(noteId); - if (note == null) { - return new JsonResponse<>(Status.NOT_FOUND, "note not found.").build(); - } + checkIfNoteIsNotNull(note); + checkIfUserIsOwnerOrWriter(noteId, + "Insufficient privileges you cannot add paragraph to this note"); NewParagraphRequest request = gson.fromJson(message, NewParagraphRequest.class); @@ -379,14 +424,10 @@ public Response getParagraph(@PathParam("noteId") String noteId, LOG.info("get paragraph {} {}", noteId, paragraphId); Note note = notebook.getNote(noteId); - if (note == null) { - return new JsonResponse(Status.NOT_FOUND, "note not found.").build(); - } - + checkIfNoteIsNotNull(note); + checkIfUserIsMember(noteId, "Insufficient privileges you cannot get this paragraph"); Paragraph p = note.getParagraph(paragraphId); - if (p == null) { - return new JsonResponse(Status.NOT_FOUND, "paragraph not found.").build(); - } + checkIfParagraphIsNotNull(p); return new JsonResponse<>(Status.OK, "", p).build(); } @@ -407,14 +448,11 @@ public Response moveParagraph(@PathParam("noteId") String noteId, LOG.info("move paragraph {} {} {}", noteId, paragraphId, newIndex); Note note = notebook.getNote(noteId); - if (note == null) { - return new JsonResponse(Status.NOT_FOUND, "note not found.").build(); - } + checkIfNoteIsNotNull(note); + checkIfUserIsOwnerOrWriter(noteId, "Insufficient privileges you cannot move paragraph"); Paragraph p = note.getParagraph(paragraphId); - if (p == null) { - return new JsonResponse(Status.NOT_FOUND, "paragraph not found.").build(); - } + checkIfParagraphIsNotNull(p); try { note.moveParagraph(paragraphId, Integer.parseInt(newIndex), true); @@ -444,14 +482,12 @@ public Response deleteParagraph(@PathParam("noteId") String noteId, LOG.info("delete paragraph {} {}", noteId, paragraphId); Note note = notebook.getNote(noteId); - if (note == null) { - return new JsonResponse(Status.NOT_FOUND, "note not found.").build(); - } + checkIfNoteIsNotNull(note); + checkIfUserIsOwnerOrWriter(noteId, + "Insufficient privileges you cannot remove paragraph from this note"); Paragraph p = note.getParagraph(paragraphId); - if (p == null) { - return new JsonResponse(Status.NOT_FOUND, "paragraph not found.").build(); - } + checkIfParagraphIsNotNull(p); AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal()); note.removeParagraph(SecurityUtils.getPrincipal(), paragraphId); @@ -475,9 +511,8 @@ public Response runNoteJobs(@PathParam("noteId") String noteId) throws IOException, IllegalArgumentException { LOG.info("run note jobs {} ", noteId); Note note = notebook.getNote(noteId); - if (note == null) { - return new JsonResponse<>(Status.NOT_FOUND, "note not found.").build(); - } + checkIfNoteIsNotNull(note); + checkIfUserIsOwnerOrWriter(noteId, "Insufficient privileges you cannot run job for this note"); try { note.runAll(); @@ -504,9 +539,9 @@ public Response stopNoteJobs(@PathParam("noteId") String noteId) throws IOException, IllegalArgumentException { LOG.info("stop note jobs {} ", noteId); Note note = notebook.getNote(noteId); - if (note == null) { - return new JsonResponse<>(Status.NOT_FOUND, "note not found.").build(); - } + checkIfNoteIsNotNull(note); + checkIfUserIsOwnerOrWriter(noteId, + "Insufficient privileges you cannot stop this job for this note"); for (Paragraph p : note.getParagraphs()) { if (!p.isTerminated()) { @@ -530,9 +565,8 @@ public Response getNoteJobStatus(@PathParam("noteId") String noteId) throws IOException, IllegalArgumentException { LOG.info("get note job status."); Note note = notebook.getNote(noteId); - if (note == null) { - return new JsonResponse<>(Status.NOT_FOUND, "note not found.").build(); - } + checkIfNoteIsNotNull(note); + checkIfUserIsMember(noteId, "Insufficient privileges you cannot get job status"); return new JsonResponse<>(Status.OK, null, note.generateParagraphsInfo()).build(); } @@ -553,14 +587,11 @@ public Response getNoteParagraphJobStatus(@PathParam("noteId") String noteId, throws IOException, IllegalArgumentException { LOG.info("get note paragraph job status."); Note note = notebook.getNote(noteId); - if (note == null) { - return new JsonResponse<>(Status.NOT_FOUND, "note not found.").build(); - } + checkIfNoteIsNotNull(note); + checkIfUserIsMember(noteId, "Insufficient privileges you cannot get job status"); Paragraph paragraph = note.getParagraph(paragraphId); - if (paragraph == null) { - return new JsonResponse<>(Status.NOT_FOUND, "paragraph not found.").build(); - } + checkIfParagraphIsNotNull(paragraph); return new JsonResponse<>(Status.OK, null, note.generateSingleParagraphInfo(paragraphId)). build(); @@ -583,14 +614,10 @@ public Response runParagraph(@PathParam("noteId") String noteId, LOG.info("run paragraph job asynchronously {} {} {}", noteId, paragraphId, message); Note note = notebook.getNote(noteId); - if (note == null) { - return new JsonResponse<>(Status.NOT_FOUND, "note not found.").build(); - } - + checkIfNoteIsNotNull(note); + checkIfUserIsOwnerOrWriter(noteId, "Insufficient privileges you cannot run job for this note"); Paragraph paragraph = note.getParagraph(paragraphId); - if (paragraph == null) { - return new JsonResponse<>(Status.NOT_FOUND, "paragraph not found.").build(); - } + checkIfParagraphIsNotNull(paragraph); // handle params if presented handleParagraphParams(message, note, paragraph); @@ -625,14 +652,10 @@ public Response runParagraphSynchronously(@PathParam("noteId") String noteId, LOG.info("run paragraph synchronously {} {} {}", noteId, paragraphId, message); Note note = notebook.getNote(noteId); - if (note == null) { - return new JsonResponse<>(Status.NOT_FOUND, "note not found.").build(); - } - + checkIfNoteIsNotNull(note); + checkIfUserIsOwnerOrWriter(noteId, "Insufficient privileges you cannot run paragraph"); Paragraph paragraph = note.getParagraph(paragraphId); - if (paragraph == null) { - return new JsonResponse<>(Status.NOT_FOUND, "paragraph not found.").build(); - } + checkIfParagraphIsNotNull(paragraph); // handle params if presented handleParagraphParams(message, note, paragraph); @@ -667,14 +690,10 @@ public Response stopParagraph(@PathParam("noteId") String noteId, @PathParam("paragraphId") String paragraphId) throws IOException, IllegalArgumentException { LOG.info("stop paragraph job {} ", noteId); Note note = notebook.getNote(noteId); - if (note == null) { - return new JsonResponse<>(Status.NOT_FOUND, "note not found.").build(); - } - + checkIfNoteIsNotNull(note); + checkIfUserIsOwnerOrWriter(noteId, "Insufficient privileges you cannot stop paragraph"); Paragraph p = note.getParagraph(paragraphId); - if (p == null) { - return new JsonResponse<>(Status.NOT_FOUND, "paragraph not found.").build(); - } + checkIfParagraphIsNotNull(p); p.abort(); return new JsonResponse<>(Status.OK).build(); } @@ -696,9 +715,9 @@ public Response registerCronJob(@PathParam("noteId") String noteId, String messa CronRequest request = gson.fromJson(message, CronRequest.class); Note note = notebook.getNote(noteId); - if (note == null) { - return new JsonResponse<>(Status.NOT_FOUND, "note not found.").build(); - } + checkIfNoteIsNotNull(note); + checkIfUserIsOwnerOrWriter(noteId, + "Insufficient privileges you cannot set a cron job for this note"); if (!CronExpression.isValidExpression(request.getCronString())) { return new JsonResponse<>(Status.BAD_REQUEST, "wrong cron expressions.").build(); @@ -727,9 +746,9 @@ public Response removeCronJob(@PathParam("noteId") String noteId) LOG.info("Remove cron job note {}", noteId); Note note = notebook.getNote(noteId); - if (note == null) { - return new JsonResponse<>(Status.NOT_FOUND, "note not found.").build(); - } + checkIfNoteIsNotNull(note); + checkIfUserIsOwnerOrWriter(noteId, + "Insufficient privileges you cannot remove this cron job from this note"); Map config = note.getConfig(); config.put("cron", null); @@ -754,9 +773,8 @@ public Response getCronJob(@PathParam("noteId") String noteId) LOG.info("Get cron job note {}", noteId); Note note = notebook.getNote(noteId); - if (note == null) { - return new JsonResponse<>(Status.NOT_FOUND, "note not found.").build(); - } + checkIfNoteIsNotNull(note); + checkIfUserIsMember(noteId, "Insufficient privileges you cannot get cron information"); return new JsonResponse<>(Status.OK, note.getConfig().get("cron")).build(); } From ed404a4e331e28f76cc2a28ebc3498d3343da34e Mon Sep 17 00:00:00 2001 From: Anthony Corbacho Date: Sat, 29 Oct 2016 14:58:04 +0900 Subject: [PATCH 07/15] Rename permission check note :: be more meaningful --- .../apache/zeppelin/rest/NotebookRestApi.java | 81 +++++++++++-------- .../notebook/NotebookAuthorization.java | 26 +++++- 2 files changed, 73 insertions(+), 34 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 e543a1c5aa5..7465ce3089c 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 @@ -42,6 +42,7 @@ import org.apache.zeppelin.notebook.Notebook; import org.apache.zeppelin.notebook.NotebookAuthorization; import org.apache.zeppelin.notebook.Paragraph; +import org.apache.zeppelin.rest.exception.NotFoundException; import org.apache.zeppelin.rest.exception.UnauthorizedException; import org.apache.zeppelin.rest.message.CronRequest; import org.apache.zeppelin.rest.message.NewNoteRequest; @@ -54,7 +55,6 @@ import org.apache.zeppelin.user.AuthenticationInfo; import org.apache.zeppelin.utils.InterpreterBindingUtils; import org.apache.zeppelin.utils.SecurityUtils; -import org.openqa.selenium.NotFoundException; import org.quartz.CronExpression; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -93,7 +93,7 @@ public NotebookRestApi(Notebook notebook, NotebookServer notebookServer, SearchS @Path("{noteId}/permissions") @ZeppelinApi public Response getNotePermissions(@PathParam("noteId") String noteId) { - checkIfUserIsMember(noteId, + checkIfUserCanRead(noteId, "Insufficient privileges you cannot get the list of permissions for this note"); HashMap> permissionsMap = new HashMap<>(); permissionsMap.put("owners", notebookAuthorization.getOwners(noteId)); @@ -115,20 +115,39 @@ private String ownerPermissionError(Set current, Set allowed) th * 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. */ - private void checkIfUserIsOwnerOrWriter(String noteId, String errorMsg) { + + /** + * Check if the current user own the given note. + */ + private void checkIfUserIsOwner(String noteId, String errorMsg) { + Set userAndRoles = Sets.newHashSet(); + userAndRoles.add(SecurityUtils.getPrincipal()); + userAndRoles.addAll(SecurityUtils.getRoles()); + if (!notebookAuthorization.hasReadAuthorization(userAndRoles, noteId)) { + throw new UnauthorizedException(errorMsg); + } + } + + /** + * Check if the current user is either Owner or Writer for the given note. + */ + private void checkIfUserCanWrite(String noteId, String errorMsg) { Set userAndRoles = Sets.newHashSet(); userAndRoles.add(SecurityUtils.getPrincipal()); userAndRoles.addAll(SecurityUtils.getRoles()); - if (!notebookAuthorization.isUserOwnerOrWriter(userAndRoles, noteId)) { + if (!notebookAuthorization.hasWriteAuthorization(userAndRoles, noteId)) { throw new UnauthorizedException(errorMsg); } } - private void checkIfUserIsMember(String noteId, String errorMsg) { + /** + * Check if the current user can access (at least he have to be reader) the given note. + */ + private void checkIfUserCanRead(String noteId, String errorMsg) { Set userAndRoles = Sets.newHashSet(); userAndRoles.add(SecurityUtils.getPrincipal()); userAndRoles.addAll(SecurityUtils.getRoles()); - if (!notebookAuthorization.isUserOwnerOrWriter(userAndRoles, noteId)) { + if (!notebookAuthorization.hasReadAuthorization(userAndRoles, noteId)) { throw new UnauthorizedException(errorMsg); } } @@ -159,7 +178,7 @@ public Response putNotePermissions(@PathParam("noteId") String noteId, String re userAndRoles.add(principal); userAndRoles.addAll(roles); - checkIfUserIsOwnerOrWriter(noteId, + checkIfUserCanWrite(noteId, ownerPermissionError(userAndRoles, notebookAuthorization.getOwners(noteId))); HashMap> permMap = @@ -172,7 +191,7 @@ public Response putNotePermissions(@PathParam("noteId") String noteId, String re HashSet readers = permMap.get("readers"); HashSet owners = permMap.get("owners"); HashSet writers = permMap.get("writers"); - // Set readers, if writers and owners is empty -> set to user requesting the change + // Set readers, if writers and owners if empty -> set to user requesting the change if (readers != null && !readers.isEmpty()) { if (writers.isEmpty()) { writers = Sets.newHashSet(SecurityUtils.getPrincipal()); @@ -209,7 +228,7 @@ public Response putNotePermissions(@PathParam("noteId") String noteId, String re @Path("interpreter/bind/{noteId}") @ZeppelinApi public Response bind(@PathParam("noteId") String noteId, String req) throws IOException { - checkIfUserIsOwnerOrWriter(noteId, + checkIfUserCanWrite(noteId, "Insufficient privileges you cannot bind any interpreters to this note"); List settingIdList = gson.fromJson(req, new TypeToken>() { @@ -225,6 +244,8 @@ public Response bind(@PathParam("noteId") String noteId, String req) throws IOEx @Path("interpreter/bind/{noteId}") @ZeppelinApi public Response bind(@PathParam("noteId") String noteId) { + checkIfUserCanRead(noteId, "Insufficient privileges you cannot get any interpreters settings"); + List settingList = InterpreterBindingUtils.getInterpreterBindings(notebook, noteId); notebookServer.broadcastInterpreterBindings(noteId, settingList); @@ -247,8 +268,8 @@ public Response getNoteList() throws IOException { public Response getNote(@PathParam("noteId") String noteId) throws IOException { Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsMember(noteId, - "Insufficient privileges you cannot get this note"); + checkIfUserCanRead(noteId, "Insufficient privileges you cannot get this note"); + return new JsonResponse<>(Status.OK, "", note).build(); } @@ -263,8 +284,7 @@ public Response getNote(@PathParam("noteId") String noteId) throws IOException { @Path("export/{noteId}") @ZeppelinApi public Response exportNote(@PathParam("noteId") String noteId) throws IOException { - checkIfUserIsMember(noteId, - "Insufficient privileges you cannot export this note"); + checkIfUserCanRead(noteId, "Insufficient privileges you cannot export this note"); String exportJson = notebook.exportNote(noteId); return new JsonResponse<>(Status.OK, "", exportJson).build(); } @@ -333,8 +353,8 @@ public Response createNote(String message) throws IOException { @ZeppelinApi public Response deleteNote(@PathParam("noteId") String noteId) throws IOException { LOG.info("Delete note {} ", noteId); + checkIfUserIsOwner(noteId, "Insufficient privileges you cannot delete this note"); AuthenticationInfo subject = new AuthenticationInfo(SecurityUtils.getPrincipal()); - checkIfUserIsOwnerOrWriter(noteId, "Insufficient privileges you cannot remove this note"); if (!(noteId.isEmpty())) { Note note = notebook.getNote(noteId); if (note != null) { @@ -359,7 +379,7 @@ public Response deleteNote(@PathParam("noteId") String noteId) throws IOExceptio public Response cloneNote(@PathParam("noteId") String noteId, String message) throws IOException, CloneNotSupportedException, IllegalArgumentException { LOG.info("clone note by JSON {}", message); - checkIfUserIsOwnerOrWriter(noteId, "Insufficient privileges you cannot clone this note"); + checkIfUserCanWrite(noteId, "Insufficient privileges you cannot clone this note"); NewNoteRequest request = gson.fromJson(message, NewNoteRequest.class); String newNoteName = null; if (request != null) { @@ -388,8 +408,7 @@ public Response insertParagraph(@PathParam("noteId") String noteId, String messa Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsOwnerOrWriter(noteId, - "Insufficient privileges you cannot add paragraph to this note"); + checkIfUserCanWrite(noteId, "Insufficient privileges you cannot add paragraph to this note"); NewParagraphRequest request = gson.fromJson(message, NewParagraphRequest.class); @@ -425,7 +444,7 @@ public Response getParagraph(@PathParam("noteId") String noteId, Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsMember(noteId, "Insufficient privileges you cannot get this paragraph"); + checkIfUserCanRead(noteId, "Insufficient privileges you cannot get this paragraph"); Paragraph p = note.getParagraph(paragraphId); checkIfParagraphIsNotNull(p); @@ -449,7 +468,7 @@ public Response moveParagraph(@PathParam("noteId") String noteId, Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsOwnerOrWriter(noteId, "Insufficient privileges you cannot move paragraph"); + checkIfUserCanWrite(noteId, "Insufficient privileges you cannot move paragraph"); Paragraph p = note.getParagraph(paragraphId); checkIfParagraphIsNotNull(p); @@ -483,7 +502,7 @@ public Response deleteParagraph(@PathParam("noteId") String noteId, Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsOwnerOrWriter(noteId, + checkIfUserIsOwner(noteId, "Insufficient privileges you cannot remove paragraph from this note"); Paragraph p = note.getParagraph(paragraphId); @@ -512,7 +531,7 @@ public Response runNoteJobs(@PathParam("noteId") String noteId) LOG.info("run note jobs {} ", noteId); Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsOwnerOrWriter(noteId, "Insufficient privileges you cannot run job for this note"); + checkIfUserCanWrite(noteId, "Insufficient privileges you cannot run job for this note"); try { note.runAll(); @@ -540,8 +559,7 @@ public Response stopNoteJobs(@PathParam("noteId") String noteId) LOG.info("stop note jobs {} ", noteId); Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsOwnerOrWriter(noteId, - "Insufficient privileges you cannot stop this job for this note"); + checkIfUserCanWrite(noteId, "Insufficient privileges you cannot stop this job for this note"); for (Paragraph p : note.getParagraphs()) { if (!p.isTerminated()) { @@ -566,7 +584,7 @@ public Response getNoteJobStatus(@PathParam("noteId") String noteId) LOG.info("get note job status."); Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsMember(noteId, "Insufficient privileges you cannot get job status"); + checkIfUserCanRead(noteId, "Insufficient privileges you cannot get job status"); return new JsonResponse<>(Status.OK, null, note.generateParagraphsInfo()).build(); } @@ -588,7 +606,7 @@ public Response getNoteParagraphJobStatus(@PathParam("noteId") String noteId, LOG.info("get note paragraph job status."); Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsMember(noteId, "Insufficient privileges you cannot get job status"); + checkIfUserCanRead(noteId, "Insufficient privileges you cannot get job status"); Paragraph paragraph = note.getParagraph(paragraphId); checkIfParagraphIsNotNull(paragraph); @@ -615,7 +633,7 @@ public Response runParagraph(@PathParam("noteId") String noteId, Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsOwnerOrWriter(noteId, "Insufficient privileges you cannot run job for this note"); + checkIfUserCanWrite(noteId, "Insufficient privileges you cannot run job for this note"); Paragraph paragraph = note.getParagraph(paragraphId); checkIfParagraphIsNotNull(paragraph); @@ -653,7 +671,7 @@ public Response runParagraphSynchronously(@PathParam("noteId") String noteId, Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsOwnerOrWriter(noteId, "Insufficient privileges you cannot run paragraph"); + checkIfUserCanWrite(noteId, "Insufficient privileges you cannot run paragraph"); Paragraph paragraph = note.getParagraph(paragraphId); checkIfParagraphIsNotNull(paragraph); @@ -691,7 +709,7 @@ public Response stopParagraph(@PathParam("noteId") String noteId, LOG.info("stop paragraph job {} ", noteId); Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsOwnerOrWriter(noteId, "Insufficient privileges you cannot stop paragraph"); + checkIfUserCanWrite(noteId, "Insufficient privileges you cannot stop paragraph"); Paragraph p = note.getParagraph(paragraphId); checkIfParagraphIsNotNull(p); p.abort(); @@ -716,8 +734,7 @@ public Response registerCronJob(@PathParam("noteId") String noteId, String messa Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsOwnerOrWriter(noteId, - "Insufficient privileges you cannot set a cron job for this note"); + checkIfUserCanWrite(noteId, "Insufficient privileges you cannot set a cron job for this note"); if (!CronExpression.isValidExpression(request.getCronString())) { return new JsonResponse<>(Status.BAD_REQUEST, "wrong cron expressions.").build(); @@ -747,7 +764,7 @@ public Response removeCronJob(@PathParam("noteId") String noteId) Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsOwnerOrWriter(noteId, + checkIfUserIsOwner(noteId, "Insufficient privileges you cannot remove this cron job from this note"); Map config = note.getConfig(); @@ -774,7 +791,7 @@ public Response getCronJob(@PathParam("noteId") String noteId) Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsMember(noteId, "Insufficient privileges you cannot get cron information"); + checkIfUserCanRead(noteId, "Insufficient privileges you cannot get cron information"); return new JsonResponse<>(Status.OK, note.getConfig().get("cron")).build(); } 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 8d95ffe8af7..42c2040e0f0 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 @@ -252,9 +252,20 @@ private boolean isMember(Set a, Set b) { return (b.isEmpty() || (intersection.size() > 0)); } - public boolean isUserOwnerOrWriter(Set userAndRoles, String noteId) { + public boolean isOwner(Set userAndRoles, String noteId) { if (conf.isAnonymousAllowed()) { - LOG.debug("Zeppelin runs in anonymous mode"); + LOG.debug("Zeppelin runs in anonymous mode, everybody is owner"); + return true; + } + if (userAndRoles == null) { + return false; + } + return isOwner(noteId, userAndRoles); + } + + public boolean hasWriteAuthorization(Set userAndRoles, String noteId) { + if (conf.isAnonymousAllowed()) { + LOG.debug("Zeppelin runs in anonymous mode, everybody is writer"); return true; } if (userAndRoles == null) { @@ -262,6 +273,17 @@ public boolean isUserOwnerOrWriter(Set userAndRoles, String noteId) { } return isWriter(noteId, userAndRoles); } + + public boolean hasReadAuthorization(Set userAndRoles, String noteId) { + if (conf.isAnonymousAllowed()) { + LOG.debug("Zeppelin runs in anonymous mode, everybody can read"); + return true; + } + if (userAndRoles == null) { + return false; + } + return isReader(noteId, userAndRoles); + } public void removeNote(String noteId) { authInfo.remove(noteId); From c8c42b26a0b7403a02f90cce62336c2eb9498873 Mon Sep 17 00:00:00 2001 From: Anthony Corbacho Date: Tue, 1 Nov 2016 12:33:20 +0900 Subject: [PATCH 08/15] Change cxf version from 2.7.7 to 2.7.8 to avoid method not found where throw WebAppException --- zeppelin-server/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zeppelin-server/pom.xml b/zeppelin-server/pom.xml index 862fc30ce35..ee53a37ac38 100644 --- a/zeppelin-server/pom.xml +++ b/zeppelin-server/pom.xml @@ -33,7 +33,7 @@ Zeppelin: Server - 2.7.7 + 2.7.8 4.3.6 2.6.0 From eacfa8eee2fde6d9fb1f3f8fc52b7b7967b6c665 Mon Sep 17 00:00:00 2001 From: Anthony Corbacho Date: Tue, 1 Nov 2016 17:24:38 +0900 Subject: [PATCH 09/15] Fix typo and bad copy paste for isOwner --- .../main/java/org/apache/zeppelin/rest/NotebookRestApi.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 7465ce3089c..e2df5104e99 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 @@ -123,7 +123,7 @@ private void checkIfUserIsOwner(String noteId, String errorMsg) { Set userAndRoles = Sets.newHashSet(); userAndRoles.add(SecurityUtils.getPrincipal()); userAndRoles.addAll(SecurityUtils.getRoles()); - if (!notebookAuthorization.hasReadAuthorization(userAndRoles, noteId)) { + if (!notebookAuthorization.isOwner(userAndRoles, noteId)) { throw new UnauthorizedException(errorMsg); } } @@ -191,7 +191,7 @@ public Response putNotePermissions(@PathParam("noteId") String noteId, String re HashSet readers = permMap.get("readers"); HashSet owners = permMap.get("owners"); HashSet writers = permMap.get("writers"); - // Set readers, if writers and owners if empty -> set to user requesting the change + // Set readers, if writers and owners is empty -> set to user requesting the change if (readers != null && !readers.isEmpty()) { if (writers.isEmpty()) { writers = Sets.newHashSet(SecurityUtils.getPrincipal()); From db0c39c4a14d7a97b5d7caec3d332056d0773920 Mon Sep 17 00:00:00 2001 From: Anthony Corbacho Date: Tue, 1 Nov 2016 17:30:17 +0900 Subject: [PATCH 10/15] Adress review and fix typos --- .../main/java/org/apache/zeppelin/rest/NotebookRestApi.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 e2df5104e99..5b27d0e6842 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 @@ -178,7 +178,7 @@ public Response putNotePermissions(@PathParam("noteId") String noteId, String re userAndRoles.add(principal); userAndRoles.addAll(roles); - checkIfUserCanWrite(noteId, + checkIfUserIsOwner(noteId, ownerPermissionError(userAndRoles, notebookAuthorization.getOwners(noteId))); HashMap> permMap = @@ -502,7 +502,7 @@ public Response deleteParagraph(@PathParam("noteId") String noteId, Note note = notebook.getNote(noteId); checkIfNoteIsNotNull(note); - checkIfUserIsOwner(noteId, + checkIfUserCanRead(noteId, "Insufficient privileges you cannot remove paragraph from this note"); Paragraph p = note.getParagraph(paragraphId); From b412266b4dd73fd005a403b77acdec0599e55b3a Mon Sep 17 00:00:00 2001 From: Anthony Corbacho Date: Tue, 1 Nov 2016 23:48:14 +0900 Subject: [PATCH 11/15] Refactored Abstract rest api test to also handle the case of tests with shiro (security), I also added some utility http method to do action with authenticated user --- .../zeppelin/rest/AbstractTestRestApi.java | 108 +++++++++++++++++- 1 file changed, 102 insertions(+), 6 deletions(-) diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java index 823b1dd1af4..deb034b6a71 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java @@ -26,18 +26,22 @@ import java.util.Properties; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.regex.Pattern; import org.apache.commons.exec.CommandLine; import org.apache.commons.exec.DefaultExecutor; import org.apache.commons.exec.PumpStreamHandler; import org.apache.commons.httpclient.HttpClient; import org.apache.commons.httpclient.HttpMethodBase; +import org.apache.commons.httpclient.cookie.CookiePolicy; import org.apache.commons.httpclient.methods.ByteArrayRequestEntity; import org.apache.commons.httpclient.methods.DeleteMethod; import org.apache.commons.httpclient.methods.GetMethod; import org.apache.commons.httpclient.methods.PostMethod; import org.apache.commons.httpclient.methods.PutMethod; import org.apache.commons.httpclient.methods.RequestEntity; +import org.apache.commons.io.FileUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.zeppelin.conf.ZeppelinConfiguration; import org.apache.zeppelin.interpreter.InterpreterSetting; import org.apache.zeppelin.server.ZeppelinServer; @@ -47,6 +51,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.gson.Gson; import com.google.gson.JsonElement; import com.google.gson.JsonParseException; import com.google.gson.JsonParser; @@ -60,6 +65,28 @@ public abstract class AbstractTestRestApi { protected static final boolean wasRunning = checkIfServerIsRunning(); static boolean pySpark = false; static boolean sparkR = false; + static Gson gson = new Gson(); + + private static File shiroIni = null; + private static String zeppelinShiro = + "[users]\n" + + "admin = password1, admin\n" + + "user1 = password2, role1, role2\n" + + "user2 = password3, role3\n" + + "user3 = password4, role2\n" + + "[main]\n" + + "sessionManager = org.apache.shiro.web.session.mgt.DefaultWebSessionManager\n" + + "securityManager.sessionManager = $sessionManager\n" + + "securityManager.sessionManager.globalSessionTimeout = 86400000\n" + + "shiro.loginUrl = /api/login\n" + + "[roles]\n" + + "role1 = *\n" + + "role2 = *\n" + + "role3 = *\n" + + "admin = *" + + "[urls]\n" + + "/api/version = anon\n" + + "/** = authc"; private String getUrl(String path) { String url; @@ -95,15 +122,26 @@ public void run() { } }; - protected static void startUp() throws Exception { + private static void start(boolean withAuth) throws Exception { if (!wasRunning) { System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_HOME.getVarName(), "../"); System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_WAR.getVarName(), "../zeppelin-web/dist"); LOG.info("Staring test Zeppelin up..."); + ZeppelinConfiguration conf = ZeppelinConfiguration.create(); + if (withAuth) { + // Set Anonymous session to false. + System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_ANONYMOUS_ALLOWED.getVarName(), "false"); + + // Create a shiro env test. + shiroIni = new File("../conf/shiro.ini"); + if (!shiroIni.exists()) { + shiroIni.createNewFile(); + } + FileUtils.writeStringToFile(shiroIni, zeppelinShiro); + } // exclude org.apache.zeppelin.rinterpreter.* for scala 2.11 test - ZeppelinConfiguration conf = ZeppelinConfiguration.create(); String interpreters = conf.getString(ZeppelinConfiguration.ConfVars.ZEPPELIN_INTERPRETERS); String interpretersCompatibleWithScala211Test = null; @@ -184,6 +222,14 @@ protected static void startUp() throws Exception { } } } + + protected static void startUpWithAutenticationEnabled() throws Exception { + start(true); + } + + protected static void startUp() throws Exception { + start(false); + } private static String getHostname() { try { @@ -244,7 +290,9 @@ protected static void shutDown() throws Exception { for (String setting : settingList) { ZeppelinServer.notebook.getInterpreterFactory().restart(setting); } - + if (shiroIni != null) { + FileUtils.deleteQuietly(shiroIni); + } LOG.info("Terminating test Zeppelin..."); ZeppelinServer.jettyWebServer.stop(); executor.shutdown(); @@ -286,49 +334,97 @@ protected static boolean checkIfServerIsRunning() { } protected static GetMethod httpGet(String path) throws IOException { + return httpGet(path, StringUtils.EMPTY, StringUtils.EMPTY); + } + + protected static GetMethod httpGet(String path, String user, String pwd) throws IOException { LOG.info("Connecting to {}", url + path); HttpClient httpClient = new HttpClient(); GetMethod getMethod = new GetMethod(url + path); getMethod.addRequestHeader("Origin", url); + if (userAndPasswordAreNotBlank(user, pwd)) { + getMethod.setRequestHeader("Cookie", "JSESSIONID="+ getCookie(user, pwd)); + } httpClient.executeMethod(getMethod); LOG.info("{} - {}", getMethod.getStatusCode(), getMethod.getStatusText()); return getMethod; } protected static DeleteMethod httpDelete(String path) throws IOException { + return httpDelete(path, StringUtils.EMPTY, StringUtils.EMPTY); + } + + protected static DeleteMethod httpDelete(String path, String user, String pwd) throws IOException { LOG.info("Connecting to {}", url + path); HttpClient httpClient = new HttpClient(); DeleteMethod deleteMethod = new DeleteMethod(url + path); deleteMethod.addRequestHeader("Origin", url); + if (userAndPasswordAreNotBlank(user, pwd)) { + deleteMethod.setRequestHeader("Cookie", "JSESSIONID="+ getCookie(user, pwd)); + } httpClient.executeMethod(deleteMethod); LOG.info("{} - {}", deleteMethod.getStatusCode(), deleteMethod.getStatusText()); return deleteMethod; } protected static PostMethod httpPost(String path, String body) throws IOException { + return httpPost(path, body, StringUtils.EMPTY, StringUtils.EMPTY); + } + + protected static PostMethod httpPost(String path, String request, String user, String pwd) + throws IOException { LOG.info("Connecting to {}", url + path); HttpClient httpClient = new HttpClient(); PostMethod postMethod = new PostMethod(url + path); - postMethod.addRequestHeader("Origin", url); - RequestEntity entity = new ByteArrayRequestEntity(body.getBytes("UTF-8")); - postMethod.setRequestEntity(entity); + postMethod.setRequestBody(request); + postMethod.getParams().setCookiePolicy(CookiePolicy.IGNORE_COOKIES); + if (userAndPasswordAreNotBlank(user, pwd)) { + postMethod.setRequestHeader("Cookie", "JSESSIONID="+ getCookie(user, pwd)); + } httpClient.executeMethod(postMethod); LOG.info("{} - {}", postMethod.getStatusCode(), postMethod.getStatusText()); return postMethod; } protected static PutMethod httpPut(String path, String body) throws IOException { + return httpPut(path, body, StringUtils.EMPTY, StringUtils.EMPTY); + } + + protected static PutMethod httpPut(String path, String body, String user, String pwd) throws IOException { LOG.info("Connecting to {}", url + path); HttpClient httpClient = new HttpClient(); PutMethod putMethod = new PutMethod(url + path); putMethod.addRequestHeader("Origin", url); RequestEntity entity = new ByteArrayRequestEntity(body.getBytes("UTF-8")); putMethod.setRequestEntity(entity); + if (userAndPasswordAreNotBlank(user, pwd)) { + putMethod.setRequestHeader("Cookie", "JSESSIONID="+ getCookie(user, pwd)); + } httpClient.executeMethod(putMethod); LOG.info("{} - {}", putMethod.getStatusCode(), putMethod.getStatusText()); return putMethod; } + private static String getCookie(String user, String password) throws IOException { + HttpClient httpClient = new HttpClient(); + PostMethod postMethod = new PostMethod(url + "/login"); + postMethod.addRequestHeader("Origin", url); + postMethod.setParameter("password", password); + postMethod.setParameter("userName", user); + httpClient.executeMethod(postMethod); + LOG.info("{} - {}", postMethod.getStatusCode(), postMethod.getStatusText()); + Pattern pattern = Pattern.compile("JSESSIONID=([a-zA-Z0-9-]*)"); + java.util.regex.Matcher matcher = pattern.matcher(postMethod.getResponseHeaders("Set-Cookie")[0].toString()); + return matcher.find()? matcher.group(1) : StringUtils.EMPTY; + } + + protected static boolean userAndPasswordAreNotBlank(String user, String pwd) { + if (StringUtils.isBlank(user) && StringUtils.isBlank(pwd)) { + return false; + } + return true; + } + protected Matcher responsesWith(final int expectedStatusCode) { return new TypeSafeMatcher() { WeakReference method; From decd1e9c6c32f761a806e2367eeaa94121931631 Mon Sep 17 00:00:00 2001 From: Anthony Corbacho Date: Tue, 1 Nov 2016 23:49:00 +0900 Subject: [PATCH 12/15] Simple implementation of notebook test with shiro (security) --- .../rest/NotebookSecurityRestApiTest.java | 156 ++++++++++++++++++ 1 file changed, 156 insertions(+) create mode 100644 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 new file mode 100644 index 00000000000..a27c19ec1c1 --- /dev/null +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/NotebookSecurityRestApiTest.java @@ -0,0 +1,156 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * 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. + */ +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 java.io.IOException; +import java.util.Map; + +import org.apache.commons.httpclient.HttpMethodBase; +import org.apache.commons.httpclient.methods.DeleteMethod; +import org.apache.commons.httpclient.methods.GetMethod; +import org.apache.commons.httpclient.methods.PostMethod; +import org.apache.commons.httpclient.methods.PutMethod; +import org.apache.zeppelin.notebook.Note; +import org.apache.zeppelin.server.ZeppelinServer; +import org.hamcrest.Matcher; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +import com.google.gson.Gson; +import com.google.gson.reflect.TypeToken; + +public class NotebookSecurityRestApiTest extends AbstractTestRestApi { + + Gson gson = new Gson(); + + @BeforeClass + public static void init() throws Exception { + AbstractTestRestApi.startUpWithAutenticationEnabled(); + } + + @AfterClass + public static void destroy() throws Exception { + AbstractTestRestApi.shutDown(); + } + + @Before + public void setUp() {} + + @Test + public void testThatUserCanCreateAndRemoveNote() throws IOException { + String noteId = createNoteForUser("test", "admin", "password1"); + assertNotNull(noteId); + String id = getNoteIdForUser(noteId, "admin", "password1"); + assertThat(id, is(noteId)); + deleteNoteForUser(noteId, "admin", "password1"); + } + + @Test + public void testThatOtherUserCanAccessNoteIfPermissionNotSet() throws IOException { + String noteId = createNoteForUser("test", "admin", "password1"); + + userTryGetNote(noteId, "user1", "password2", isAllowed()); + + deleteNoteForUser(noteId, "admin", "password1"); + } + + @Test + public void testThatOtherUserCannotAccessNoteIfPermissionSet() throws IOException { + String noteId = createNoteForUser("test", "admin", "password1"); + + //set permission + String payload = "{ \"owners\": [\"admin\"], \"readers\": [\"user2\"], \"writers\": [\"user2\"] }"; + PutMethod put = httpPut("/notebook/" + noteId + "/permissions", payload , "admin", "password1"); + assertThat("test set note premission method:", put, isAllowed()); + put.releaseConnection(); + + userTryGetNote(noteId, "user1", "password2", isForbiden()); + + userTryGetNote(noteId, "user2", "password3", isAllowed()); + + deleteNoteForUser(noteId, "admin", "password1"); + } + + @Test + public void testThatWriterCannotRemoveNote() throws IOException { + String noteId = createNoteForUser("test", "admin", "password1"); + + //set permission + String payload = "{ \"owners\": [\"admin\", \"user1\"], \"readers\": [\"user2\"], \"writers\": [\"user2\"] }"; + PutMethod put = httpPut("/notebook/" + noteId + "/permissions", payload , "admin", "password1"); + assertThat("test set note premission method:", put, isAllowed()); + put.releaseConnection(); + + userTryRemoveNote(noteId, "user2", "password3", isForbiden()); + userTryRemoveNote(noteId, "user1", "password2", isAllowed()); + + Note deletedNote = ZeppelinServer.notebook.getNote(noteId); + assertNull("Deleted note should be null", deletedNote); + } + + private void userTryRemoveNote(String noteId, String user, String pwd, Matcher m) throws IOException { + DeleteMethod delete = httpDelete(("/notebook/" + noteId), user, pwd); + assertThat(delete, m); + delete.releaseConnection(); + } + + private void userTryGetNote(String noteId, String user, String pwd, Matcher m) throws IOException { + GetMethod get = httpGet("/notebook/" + noteId, user, pwd); + assertThat(get, m); + get.releaseConnection(); + } + + private String getNoteIdForUser(String noteId, String user, String pwd) throws IOException { + GetMethod get = httpGet("/notebook/" + noteId, user, pwd); + assertThat("test note create method:", get, isAllowed()); + Map resp = gson.fromJson(get.getResponseBodyAsString(), new TypeToken>() { + }.getType()); + get.releaseConnection(); + return (String) ((Map)resp.get("body")).get("id"); + } + + private String createNoteForUser(String noteName, String user, String pwd) throws IOException { + String jsonRequest = "{\"name\":\"" + noteName + "\"}"; + PostMethod post = httpPost("/notebook/", jsonRequest, user, pwd); + assertThat("test note create method:", post, isCreated()); + Map resp = gson.fromJson(post.getResponseBodyAsString(), new TypeToken>() { + }.getType()); + post.releaseConnection(); + String newNoteId = (String) resp.get("body"); + Note newNote = ZeppelinServer.notebook.getNote(newNoteId); + assertNotNull("Can not find new note by id", newNote); + return newNoteId; + } + + private void deleteNoteForUser(String noteId, String user, String pwd) throws IOException { + DeleteMethod delete = httpDelete(("/notebook/" + noteId), user, pwd); + assertThat("Test delete method:", delete, isAllowed()); + delete.releaseConnection(); + // make sure note is deleted + if (!noteId.isEmpty()) { + Note deletedNote = ZeppelinServer.notebook.getNote(noteId); + assertNull("Deleted note should be null", deletedNote); + } + } +} From bab7e600780e19533b59c56e7c4b7e32e8841efc Mon Sep 17 00:00:00 2001 From: Anthony Corbacho Date: Tue, 1 Nov 2016 23:49:26 +0900 Subject: [PATCH 13/15] Rewording --- .../org/apache/zeppelin/notebook/NotebookAuthorization.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 42c2040e0f0..d835c890cbb 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 @@ -276,7 +276,7 @@ public boolean hasWriteAuthorization(Set userAndRoles, String noteId) { public boolean hasReadAuthorization(Set userAndRoles, String noteId) { if (conf.isAnonymousAllowed()) { - LOG.debug("Zeppelin runs in anonymous mode, everybody can read"); + LOG.debug("Zeppelin runs in anonymous mode, everybody is reader"); return true; } if (userAndRoles == null) { From 30815c1f003353f36060b68c8f5a8083b233d294 Mon Sep 17 00:00:00 2001 From: Anthony Corbacho Date: Wed, 2 Nov 2016 12:24:43 +0900 Subject: [PATCH 14/15] Fix typo --- .../test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java | 2 +- .../org/apache/zeppelin/rest/NotebookSecurityRestApiTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java index deb034b6a71..3c70b681715 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java @@ -223,7 +223,7 @@ private static void start(boolean withAuth) throws Exception { } } - protected static void startUpWithAutenticationEnabled() throws Exception { + protected static void startUpWithAuthenticationEnable() throws Exception { start(true); } 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 a27c19ec1c1..3c5978fd4d5 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 @@ -46,7 +46,7 @@ public class NotebookSecurityRestApiTest extends AbstractTestRestApi { @BeforeClass public static void init() throws Exception { - AbstractTestRestApi.startUpWithAutenticationEnabled(); + AbstractTestRestApi.startUpWithAuthenticationEnable(); } @AfterClass From 66159355f6db9b9c8226d58d9838bb0ab0fda0f5 Mon Sep 17 00:00:00 2001 From: Anthony Corbacho Date: Thu, 3 Nov 2016 11:59:07 +0900 Subject: [PATCH 15/15] Clean anonymous allowed property when shutting down zeppelin server --- .../java/org/apache/zeppelin/rest/AbstractTestRestApi.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java index 3c70b681715..6d103372987 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/AbstractTestRestApi.java @@ -66,6 +66,7 @@ public abstract class AbstractTestRestApi { static boolean pySpark = false; static boolean sparkR = false; static Gson gson = new Gson(); + static boolean isRunningWithAuth = false; private static File shiroIni = null; private static String zeppelinShiro = @@ -130,6 +131,7 @@ private static void start(boolean withAuth) throws Exception { ZeppelinConfiguration conf = ZeppelinConfiguration.create(); if (withAuth) { + isRunningWithAuth = true; // Set Anonymous session to false. System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_ANONYMOUS_ALLOWED.getVarName(), "false"); @@ -313,6 +315,11 @@ protected static void shutDown() throws Exception { LOG.info("Test Zeppelin terminated."); System.clearProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_INTERPRETERS.getVarName()); + if (isRunningWithAuth) { + isRunningWithAuth = false; + System + .clearProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_ANONYMOUS_ALLOWED.getVarName()); + } } }