Repository: zeppelin Updated Branches: refs/heads/master a5ca2e518 -> 1b589da59
[ZEPPELIN-2654] add roles to permissions of interpreter ### What is this PR for? Added the ability to set roles (groups) which is available to use an interpreter (Ñow it is possible to set permissions to users only). The same solution as permissions of notes. ### What type of PR is it? Improvement ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-2654 ### How should this be tested? 1. Create note with Interpreter (for ex. spark) 2. Set permissions to Interpreter (spark) for `role1` 3. Log in by user with role `role1` and open note (from item 1) 4. Run paragraph ``` println("test")``` **Result == test** 6. Logout 7. Log in by user without role `role1` and open note (form item 1) 8. Run paragraph ``` println("test")``` **Result == "...no permissions..."** ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: Tinkoff DWH <tinkoff....@gmail.com> Closes #2413 from tinkoff-dwh/ZEPPELIN-2654 and squashes the following commits: f4ed56d [Tinkoff DWH] [ZEPPELIN-2654] add roles to permissions of interpreter Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/1b589da5 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/1b589da5 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/1b589da5 Branch: refs/heads/master Commit: 1b589da5944f6106f292b718cf99c61c879887c5 Parents: a5ca2e5 Author: Tinkoff DWH <tinkoff....@gmail.com> Authored: Thu Jun 15 16:25:42 2017 +0500 Committer: Lee moon soo <m...@apache.org> Committed: Thu Jun 22 13:29:09 2017 -0700 ---------------------------------------------------------------------- .../zeppelin/interpreter/InterpreterOption.java | 10 ++--- .../zeppelin/user/AuthenticationInfo.java | 33 +++++++++++++++- .../zeppelin/interpreter/InterpreterTest.java | 2 +- .../apache/zeppelin/socket/NotebookServer.java | 5 ++- .../interpreter-create/interpreter-create.html | 6 +-- .../app/interpreter/interpreter.controller.js | 41 ++++++++++++++------ .../src/app/interpreter/interpreter.html | 6 +-- .../interpreter/InterpreterSetting.java | 25 +++++++++++- .../interpreter/InterpreterSettingManager.java | 10 ++++- .../org/apache/zeppelin/notebook/Paragraph.java | 20 ++++------ 10 files changed, 115 insertions(+), 43 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1b589da5/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java ---------------------------------------------------------------------- diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java index 1e9c1cc..37a0d99 100644 --- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java +++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/interpreter/InterpreterOption.java @@ -37,7 +37,7 @@ public class InterpreterOption { boolean isExistingProcess; boolean setPermission; - List<String> users; + List<String> owners; boolean isUserImpersonate; public boolean isExistingProcess() { @@ -64,8 +64,8 @@ public class InterpreterOption { this.setPermission = setPermission; } - public List<String> getUsers() { - return users; + public List<String> getOwners() { + return owners; } public boolean isUserImpersonate() { @@ -106,8 +106,8 @@ public class InterpreterOption { option.perUser = other.perUser; option.isExistingProcess = other.isExistingProcess; option.setPermission = other.setPermission; - option.users = (null == other.users) ? - new ArrayList<String>() : new ArrayList<>(other.users); + option.owners = (null == other.owners) ? + new ArrayList<String>() : new ArrayList<>(other.owners); return option; } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1b589da5/zeppelin-interpreter/src/main/java/org/apache/zeppelin/user/AuthenticationInfo.java ---------------------------------------------------------------------- diff --git a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/user/AuthenticationInfo.java b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/user/AuthenticationInfo.java index 11d1562..766418a 100644 --- a/zeppelin-interpreter/src/main/java/org/apache/zeppelin/user/AuthenticationInfo.java +++ b/zeppelin-interpreter/src/main/java/org/apache/zeppelin/user/AuthenticationInfo.java @@ -18,6 +18,10 @@ package org.apache.zeppelin.user; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -28,9 +32,10 @@ import org.slf4j.LoggerFactory; public class AuthenticationInfo { private static final Logger LOG = LoggerFactory.getLogger(AuthenticationInfo.class); String user; + List<String> roles; String ticket; UserCredentials userCredentials; - public static final AuthenticationInfo ANONYMOUS = new AuthenticationInfo("anonymous", + public static final AuthenticationInfo ANONYMOUS = new AuthenticationInfo("anonymous", null, "anonymous"); public AuthenticationInfo() {} @@ -44,9 +49,13 @@ public class AuthenticationInfo { * @param user * @param ticket */ - public AuthenticationInfo(String user, String ticket) { + public AuthenticationInfo(String user, String roles, String ticket) { this.user = user; this.ticket = ticket; + if (StringUtils.isNotBlank(roles) && roles.length() > 2) { + String[] r = roles.substring(1, roles.length() - 1).split(","); + this.roles = Arrays.asList(r); + } } public String getUser() { @@ -57,6 +66,26 @@ public class AuthenticationInfo { this.user = user; } + public List<String> getRoles() { + return roles; + } + + public void setRoles(List<String> roles) { + this.roles = roles; + } + + public List<String> getUsersAndRoles() { + List<String> usersAndRoles = new ArrayList<>(); + if (roles != null) { + usersAndRoles.addAll(roles); + } + if (user != null) { + usersAndRoles.add(user); + } + + return usersAndRoles; + } + public String getTicket() { return ticket; } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1b589da5/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/InterpreterTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/InterpreterTest.java b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/InterpreterTest.java index 4141e95..305268c 100644 --- a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/InterpreterTest.java +++ b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/interpreter/InterpreterTest.java @@ -63,7 +63,7 @@ public class InterpreterTest { null, paragraphTitle, paragraphText, - new AuthenticationInfo("testUser", "testTicket"), + new AuthenticationInfo("testUser", null, "testTicket"), null, null, null, http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1b589da5/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java b/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java index 5588be0..93b2566 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 @@ -226,7 +226,8 @@ public class NotebookServer extends WebSocketServlet addUserConnection(messagereceived.principal, conn); } AuthenticationInfo subject = - new AuthenticationInfo(messagereceived.principal, messagereceived.ticket); + new AuthenticationInfo(messagereceived.principal, messagereceived.roles, + messagereceived.ticket); /** Lets be elegant here */ switch (messagereceived.op) { @@ -1807,7 +1808,7 @@ public class NotebookServer extends WebSocketServlet p.setText(text); p.setTitle(title); AuthenticationInfo subject = - new AuthenticationInfo(fromMessage.principal, fromMessage.ticket); + new AuthenticationInfo(fromMessage.principal, fromMessage.roles, fromMessage.ticket); p.setAuthenticationInfo(subject); p.settings.setParams(params); p.setConfig(config); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1b589da5/zeppelin-web/src/app/interpreter/interpreter-create/interpreter-create.html ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/app/interpreter/interpreter-create/interpreter-create.html b/zeppelin-web/src/app/interpreter/interpreter-create/interpreter-create.html index 7e62d8f..58e3558 100644 --- a/zeppelin-web/src/app/interpreter/interpreter-create/interpreter-create.html +++ b/zeppelin-web/src/app/interpreter/interpreter-create/interpreter-create.html @@ -252,13 +252,13 @@ limitations under the License. <div ng-show="newInterpreterSetting.option.setPermission" class="permissionsForm"> <div> <p> - Enter comma separated users in the fields. <br /> + Enter comma separated users and groups in the fields. <br /> Empty field (*) implies anyone can run this interpreter. </p> <div> <span class="owners">Owners </span> - <select id="newInterpreterUsers" class="form-control" multiple="multiple"> - <option ng-repeat="user in newInterpreterSetting.option.users" selected="selected">{{user}}</option> + <select id="newInterpreterOwners" class="form-control" multiple="multiple"> + <option ng-repeat="owner in newInterpreterSetting.option.owners" selected="selected">{{owner}}</option> </select> </div> </div> http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1b589da5/zeppelin-web/src/app/interpreter/interpreter.controller.js ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/app/interpreter/interpreter.controller.js b/zeppelin-web/src/app/interpreter/interpreter.controller.js index 6afc077..c9a840b 100644 --- a/zeppelin-web/src/app/interpreter/interpreter.controller.js +++ b/zeppelin-web/src/app/interpreter/interpreter.controller.js @@ -38,10 +38,10 @@ function InterpreterCtrl ($rootScope, $scope, $http, baseUrlSrv, ngToast, $timeo let getSelectJson = function () { let selectJson = { - tags: false, + tags: true, + minimumInputLength: 3, multiple: true, tokenSeparators: [',', ' '], - minimumInputLength: 2, ajax: { url: function (params) { if (!params.term) { @@ -51,17 +51,36 @@ function InterpreterCtrl ($rootScope, $scope, $http, baseUrlSrv, ngToast, $timeo }, delay: 250, processResults: function (data, params) { - let users = [] + let results = [] + if (data.body.users.length !== 0) { - for (let i = 0; i < data.body.users.length; i++) { + let users = [] + for (let len = 0; len < data.body.users.length; len++) { users.push({ - 'id': data.body.users[i], - 'text': data.body.users[i] + 'id': data.body.users[len], + 'text': data.body.users[len] + }) + } + results.push({ + 'text': 'Users :', + 'children': users + }) + } + if (data.body.roles.length !== 0) { + let roles = [] + for (let len = 0; len < data.body.roles.length; len++) { + roles.push({ + 'id': data.body.roles[len], + 'text': data.body.roles[len] }) } + results.push({ + 'text': 'Roles :', + 'children': roles + }) } return { - results: users, + results: results, pagination: { more: false } @@ -74,7 +93,7 @@ function InterpreterCtrl ($rootScope, $scope, $http, baseUrlSrv, ngToast, $timeo } $scope.togglePermissions = function (intpName) { - angular.element('#' + intpName + 'Users').select2(getSelectJson()) + angular.element('#' + intpName + 'Owners').select2(getSelectJson()) if ($scope.showInterpreterAuth) { $scope.closePermissions() } else { @@ -84,7 +103,7 @@ function InterpreterCtrl ($rootScope, $scope, $http, baseUrlSrv, ngToast, $timeo $scope.$on('ngRenderFinished', function (event, data) { for (let setting = 0; setting < $scope.interpreterSettings.length; setting++) { - angular.element('#' + $scope.interpreterSettings[setting].name + 'Users').select2(getSelectJson()) + angular.element('#' + $scope.interpreterSettings[setting].name + 'Owners').select2(getSelectJson()) } }) @@ -340,7 +359,7 @@ function InterpreterCtrl ($rootScope, $scope, $http, baseUrlSrv, ngToast, $timeo // remote always true for now setting.option.remote = true } - setting.option.users = angular.element('#' + setting.name + 'Users').val() + setting.option.owners = angular.element('#' + setting.name + 'Owners').val() let request = { option: angular.copy(setting.option), @@ -478,7 +497,7 @@ function InterpreterCtrl ($rootScope, $scope, $http, baseUrlSrv, ngToast, $timeo if (newSetting.option.setPermission === undefined) { newSetting.option.setPermission = false } - newSetting.option.users = angular.element('#newInterpreterUsers').val() + newSetting.option.owners = angular.element('#newInterpreterOwners').val() let request = angular.copy($scope.newInterpreterSetting) http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1b589da5/zeppelin-web/src/app/interpreter/interpreter.html ---------------------------------------------------------------------- diff --git a/zeppelin-web/src/app/interpreter/interpreter.html b/zeppelin-web/src/app/interpreter/interpreter.html index a9c932e..d58cb3a 100644 --- a/zeppelin-web/src/app/interpreter/interpreter.html +++ b/zeppelin-web/src/app/interpreter/interpreter.html @@ -372,14 +372,14 @@ limitations under the License. <div ng-show="setting.option.setPermission" class="permissionsForm"> <div> <p> - Enter comma separated users in the fields. <br /> + Enter comma separated users and groups in the fields. <br /> Empty field (*) implies anyone can run this interpreter. </p> <div> <span class="owners">Owners </span> - <select id="{{setting.name}}Users" class="form-control" multiple="multiple" ng-disabled="!valueform.$visible"> - <option ng-repeat="user in setting.option.users" selected="selected">{{user}}</option> + <select id="{{setting.name}}Owners" class="form-control" multiple="multiple" ng-disabled="!valueform.$visible"> + <option ng-repeat="owner in setting.option.owners" selected="selected">{{owner}}</option> </select> </div> </div> http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1b589da5/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java index 2efba48..7155112 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java @@ -28,11 +28,14 @@ import java.util.Properties; import java.util.Set; import java.util.concurrent.locks.ReentrantReadWriteLock; -import com.google.gson.annotations.SerializedName; +import org.apache.zeppelin.dep.Dependency; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.apache.zeppelin.dep.Dependency; +import com.google.gson.JsonArray; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.annotations.SerializedName; import static org.apache.zeppelin.notebook.utility.IdHashes.generateId; @@ -397,4 +400,22 @@ public class InterpreterSetting { public void clearNoteIdAndParaMap() { runtimeInfosToBeCleared = null; } + + // For backward compatibility of interpreter.json format after ZEPPELIN-2654 + public void convertPermissionsFromUsersToOwners(JsonObject jsonObject) { + if (jsonObject != null) { + JsonObject option = jsonObject.getAsJsonObject("option"); + if (option != null) { + JsonArray users = option.getAsJsonArray("users"); + if (users != null) { + if (this.option.getOwners() == null) { + this.option.owners = new LinkedList<>(); + } + for (JsonElement user : users) { + this.option.getOwners().add(user.getAsString()); + } + } + } + } + } } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1b589da5/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java index 5034e33..8270aba 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java @@ -25,6 +25,8 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import com.google.gson.Gson; import com.google.gson.GsonBuilder; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; import com.google.gson.internal.StringMap; import com.google.gson.reflect.TypeToken; import java.io.BufferedReader; @@ -162,8 +164,9 @@ public class InterpreterSettingManager { InterpreterInfoSaving infoSaving; try (BufferedReader json = Files.newBufferedReader(interpreterBindingPath, StandardCharsets.UTF_8)) { - infoSaving = gson.fromJson(json, InterpreterInfoSaving.class); - + JsonParser jsonParser = new JsonParser(); + JsonObject jsonObject = jsonParser.parse(json).getAsJsonObject(); + infoSaving = gson.fromJson(jsonObject.toString(), InterpreterInfoSaving.class); for (String k : infoSaving.interpreterSettings.keySet()) { InterpreterSetting setting = infoSaving.interpreterSettings.get(k); List<InterpreterInfo> infos = setting.getInterpreterInfos(); @@ -182,6 +185,9 @@ public class InterpreterSettingManager { // previously created setting should turn this feature on here. setting.getOption().setRemote(true); + setting.convertPermissionsFromUsersToOwners( + jsonObject.getAsJsonObject("interpreterSettings").getAsJsonObject(setting.getId())); + // Update transient information from InterpreterSettingRef InterpreterSetting interpreterSettingObject = interpreterSettingsRef.get(setting.getGroup()); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/1b589da5/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java index 78a0e8d..9bb5d21 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Paragraph.java @@ -342,17 +342,13 @@ public class Paragraph extends Job implements Serializable, Cloneable { return null; } - private boolean hasPermission(String user, List<String> intpUsers) { - if (1 > intpUsers.size()) { + private boolean hasPermission(List<String> userAndRoles, List<String> intpUsersAndRoles) { + if (1 > intpUsersAndRoles.size()) { return true; } - - for (String u : intpUsers) { - if (user.trim().equals(u.trim())) { - return true; - } - } - return false; + Set<String> intersection = new HashSet<>(intpUsersAndRoles); + intersection.retainAll(userAndRoles); + return (intpUsersAndRoles.isEmpty() || (intersection.size() > 0)); } public boolean isBlankParagraph() { @@ -441,12 +437,12 @@ public class Paragraph extends Job implements Serializable, Cloneable { } private boolean interpreterHasUser(InterpreterSetting intp) { - return intp.getOption().permissionIsSet() && intp.getOption().getUsers() != null; + return intp.getOption().permissionIsSet() && intp.getOption().getOwners() != null; } private boolean isUserAuthorizedToAccessInterpreter(InterpreterOption intpOpt) { - return intpOpt.permissionIsSet() && hasPermission(authenticationInfo.getUser(), - intpOpt.getUsers()); + return intpOpt.permissionIsSet() && hasPermission(authenticationInfo.getUsersAndRoles(), + intpOpt.getOwners()); } private InterpreterSetting getInterpreterSettingById(String id) {
