Repository: zeppelin Updated Branches: refs/heads/branch-0.6 c6a1e198e -> 70524348f
[Zeppelin 1042] Extra space is present as part of username in search box ### What is this PR for? Sometimes extra space is present as part of username in search box while trying to setup Zeppelin permissions ### What type of PR is it? [Bug Fix] ### Todos * [x] - trim string and then add user * [x] - implement searching in Active Directory * [x] - improve order by search result * [x] - implement debounce to reduce server request ### What is the Jira issue? * [ZEPPELIN-1042](https://issues.apache.org/jira/browse/ZEPPELIN-1042) ### How should this be tested? Zeppelin is configured with LDAP authentication. Here is the scenario 1. Login as 'user1' user and create a notebook ('Untitled Note 1') 2. Try to change the permission of 'Untitled Note 1'. Start typing in owners box -> user1 3. The search box appears with a saved name ' user1' (There is an extra space in front of user1') 4. Then click on the search box item and save the permissions. The permissions that get saved have all got an extra space before the username 'user1' though it is not intended to have that space. 5. Later while trying to change the permissions of notebook as 'user1' user, it will disallow because it recognizes ' user1' (user1 with extra space) as owner instead of plain 'user1' ### Questions: * Does the licenses files need update? * Is there breaking changes for older versions? * Does this needs documentation? Author: Prabhjyot Singh <[email protected]> Closes #1086 from prabhjyotsingh/ZEPPELIN-1042 and squashes the following commits: 0de7a3d [Prabhjyot Singh] rename variable and CI fixes 4d61f7d [Prabhjyot Singh] Merge remote-tracking branch 'origin/master' into ZEPPELIN-1042 a3d5b5c [Prabhjyot Singh] trim and then add user 57de67f [Prabhjyot Singh] implement debounce d5e5d96 [Prabhjyot Singh] update search preference 179ea73 [Prabhjyot Singh] enable search for Active Directory (cherry picked from commit 6f434c5614e03d7c2ca9a6921c58c5d843b3dab0) Signed-off-by: Mina Lee <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/70524348 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/70524348 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/70524348 Branch: refs/heads/branch-0.6 Commit: 70524348f7a44300926d638a147a85c2f2207797 Parents: c6a1e19 Author: Prabhjyot Singh <[email protected]> Authored: Sat Jun 25 19:53:49 2016 +0530 Committer: Mina Lee <[email protected]> Committed: Mon Jul 18 14:58:39 2016 +0900 ---------------------------------------------------------------------- .../org/apache/zeppelin/rest/GetUserList.java | 26 +++++++--- .../apache/zeppelin/rest/SecurityRestApi.java | 20 +++++++- .../server/ActiveDirectoryGroupRealm.java | 54 ++++++++++++++++++++ .../src/app/notebook/notebook.controller.js | 37 ++++++++++---- zeppelin-web/src/app/notebook/notebook.html | 6 +-- 5 files changed, 121 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/70524348/zeppelin-server/src/main/java/org/apache/zeppelin/rest/GetUserList.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/GetUserList.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/GetUserList.java index c603fe9..b322561 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/GetUserList.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/GetUserList.java @@ -17,12 +17,14 @@ package org.apache.zeppelin.rest; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.reflect.FieldUtils; import org.apache.shiro.realm.jdbc.JdbcRealm; import org.apache.shiro.realm.ldap.JndiLdapContextFactory; import org.apache.shiro.realm.ldap.JndiLdapRealm; import org.apache.shiro.realm.text.IniRealm; import org.apache.shiro.util.JdbcUtils; +import org.apache.zeppelin.server.ActiveDirectoryGroupRealm; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -58,7 +60,7 @@ public class GetUserList { Iterator it = getIniUser.entrySet().iterator(); while (it.hasNext()) { Map.Entry pair = (Map.Entry) it.next(); - userList.add(pair.getKey().toString()); + userList.add(pair.getKey().toString().trim()); } return userList; } @@ -66,7 +68,7 @@ public class GetUserList { /** * function to extract users from LDAP */ - public List<String> getUserList(JndiLdapRealm r) { + public List<String> getUserList(JndiLdapRealm r, String searchText) { List<String> userList = new ArrayList<>(); String userDnTemplate = r.getUserDnTemplate(); String userDn[] = userDnTemplate.split(",", 2); @@ -79,12 +81,13 @@ public class GetUserList { constraints.setSearchScope(SearchControls.SUBTREE_SCOPE); String[] attrIDs = {userDnPrefix}; constraints.setReturningAttributes(attrIDs); - NamingEnumeration result = ctx.search(userDnSuffix, "(objectclass=*)", constraints); + NamingEnumeration result = ctx.search(userDnSuffix, "(" + userDnPrefix + "=*" + searchText + + "*)", constraints); while (result.hasMore()) { Attributes attrs = ((SearchResult) result.next()).getAttributes(); if (attrs.get(userDnPrefix) != null) { String currentUser = attrs.get(userDnPrefix).toString(); - userList.add(currentUser.split(":")[1]); + userList.add(currentUser.split(":")[1].trim()); } } } catch (Exception e) { @@ -93,6 +96,17 @@ public class GetUserList { return userList; } + public List<String> getUserList(ActiveDirectoryGroupRealm r, String searchText) { + List<String> userList = new ArrayList<>(); + try { + LdapContext ctx = r.getLdapContextFactory().getSystemLdapContext(); + userList = r.searchForUserName(searchText, ctx); + } catch (Exception e) { + LOG.error("Error retrieving User list from ActiveDirectory Realm", e); + } + return userList; + } + /** * function to extract users from JDBCs */ @@ -123,7 +137,7 @@ public class GetUserList { username = retval[0]; } - if (username.equals("") || tablename.equals("")){ + if (StringUtils.isBlank(username) || StringUtils.isBlank(tablename)) { return userlist; } @@ -139,7 +153,7 @@ public class GetUserList { ps = con.prepareStatement(userquery); rs = ps.executeQuery(); while (rs.next()) { - userlist.add(rs.getString(1)); + userlist.add(rs.getString(1).trim()); } } catch (Exception e) { LOG.error("Error retrieving User list from JDBC Realm", e); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/70524348/zeppelin-server/src/main/java/org/apache/zeppelin/rest/SecurityRestApi.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/SecurityRestApi.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/SecurityRestApi.java index 11c8f96..b8bfc9f 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/SecurityRestApi.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/SecurityRestApi.java @@ -20,10 +20,12 @@ package org.apache.zeppelin.rest; import org.apache.shiro.realm.Realm; import org.apache.shiro.realm.jdbc.JdbcRealm; +import org.apache.shiro.realm.ldap.AbstractLdapRealm; import org.apache.shiro.realm.ldap.JndiLdapRealm; import org.apache.shiro.realm.text.IniRealm; import org.apache.zeppelin.annotation.ZeppelinApi; import org.apache.zeppelin.conf.ZeppelinConfiguration; +import org.apache.zeppelin.server.ActiveDirectoryGroupRealm; import org.apache.zeppelin.server.JsonResponse; import org.apache.zeppelin.ticket.TicketContainer; import org.apache.zeppelin.utils.SecurityUtils; @@ -93,7 +95,7 @@ public class SecurityRestApi { */ @GET @Path("userlist/{searchText}") - public Response getUserList(@PathParam("searchText") String searchText) { + public Response getUserList(@PathParam("searchText") final String searchText) { List<String> usersList = new ArrayList<>(); try { @@ -105,7 +107,10 @@ public class SecurityRestApi { if (name.equals("iniRealm")) { usersList.addAll(getUserListObj.getUserList((IniRealm) realm)); } else if (name.equals("ldapRealm")) { - usersList.addAll(getUserListObj.getUserList((JndiLdapRealm) realm)); + usersList.addAll(getUserListObj.getUserList((JndiLdapRealm) realm, searchText)); + } else if (name.equals("activeDirectoryRealm")) { + usersList.addAll(getUserListObj.getUserList((ActiveDirectoryGroupRealm) realm, + searchText)); } else if (name.equals("jdbcRealm")) { usersList.addAll(getUserListObj.getUserList((JdbcRealm) realm)); } @@ -116,6 +121,17 @@ public class SecurityRestApi { } List<String> autoSuggestList = new ArrayList<>(); Collections.sort(usersList); + Collections.sort(usersList, new Comparator<String>() { + @Override + public int compare(String o1, String o2) { + if (o1.matches(searchText + "(.*)") && o2.matches(searchText + "(.*)")) { + return 0; + } else if (o1.matches(searchText + "(.*)")) { + return -1; + } + return 0; + } + }); int maxLength = 0; for (int i = 0; i < usersList.size(); i++) { String userLowerCase = usersList.get(i).toLowerCase(); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/70524348/zeppelin-server/src/main/java/org/apache/zeppelin/server/ActiveDirectoryGroupRealm.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/server/ActiveDirectoryGroupRealm.java b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ActiveDirectoryGroupRealm.java index fc3ccc8..cc868d7 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/server/ActiveDirectoryGroupRealm.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ActiveDirectoryGroupRealm.java @@ -24,6 +24,7 @@ import org.apache.shiro.authz.AuthorizationInfo; import org.apache.shiro.authz.SimpleAuthorizationInfo; import org.apache.shiro.realm.Realm; import org.apache.shiro.realm.ldap.AbstractLdapRealm; +import org.apache.shiro.realm.ldap.DefaultLdapContextFactory; import org.apache.shiro.realm.ldap.LdapContextFactory; import org.apache.shiro.realm.ldap.LdapUtils; import org.apache.shiro.subject.PrincipalCollection; @@ -77,6 +78,25 @@ public class ActiveDirectoryGroupRealm extends AbstractLdapRealm { | M E T H O D S | ============================================*/ + LdapContextFactory ldapContextFactory; + + public LdapContextFactory getLdapContextFactory() { + if (this.ldapContextFactory == null) { + if (log.isDebugEnabled()) { + log.debug("No LdapContextFactory specified - creating a default instance."); + } + + DefaultLdapContextFactory defaultFactory = new DefaultLdapContextFactory(); + defaultFactory.setPrincipalSuffix(this.principalSuffix); + defaultFactory.setSearchBase(this.searchBase); + defaultFactory.setUrl(this.url); + defaultFactory.setSystemUsername(this.systemUsername); + defaultFactory.setSystemPassword(this.systemPassword); + this.ldapContextFactory = defaultFactory; + } + + return this.ldapContextFactory; + } /** * Builds an {@link AuthenticationInfo} object by querying the active directory LDAP context for @@ -159,6 +179,40 @@ public class ActiveDirectoryGroupRealm extends AbstractLdapRealm { return new SimpleAuthorizationInfo(roleNames); } + public List<String> searchForUserName(String containString, LdapContext ldapContext) throws + NamingException { + List<String> userNameList = new ArrayList<>(); + + SearchControls searchCtls = new SearchControls(); + searchCtls.setSearchScope(SearchControls.SUBTREE_SCOPE); + + String searchFilter = "(&(objectClass=*)(userPrincipalName=*" + containString + "*))"; + Object[] searchArguments = new Object[]{containString}; + + NamingEnumeration answer = ldapContext.search(searchBase, searchFilter, searchArguments, + searchCtls); + + while (answer.hasMoreElements()) { + SearchResult sr = (SearchResult) answer.next(); + + if (log.isDebugEnabled()) { + log.debug("Retrieving userprincipalname names for user [" + sr.getName() + "]"); + } + + Attributes attrs = sr.getAttributes(); + if (attrs != null) { + NamingEnumeration ae = attrs.getAll(); + while (ae.hasMore()) { + Attribute attr = (Attribute) ae.next(); + if (attr.getID().toLowerCase().equals("cn")) { + userNameList.addAll(LdapUtils.getAllAttributeValues(attr)); + } + } + } + } + return userNameList; + } + private Set<String> getRoleNamesForUser(String username, LdapContext ldapContext) throws NamingException { Set<String> roleNames = new LinkedHashSet<>(); http://git-wip-us.apache.org/repos/asf/zeppelin/blob/70524348/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 8187ad0..9c7a93a 100644 --- a/zeppelin-web/src/app/notebook/notebook.controller.js +++ b/zeppelin-web/src/app/notebook/notebook.controller.js @@ -824,15 +824,16 @@ angular.module('zeppelinWebApp').controller('NotebookCtrl', function convertToArray(role) { - if (role === 'owners') { + if (!$scope.permissions) { + return; + } else if (role === 'owners' && typeof $scope.permissions.owners === 'string') { searchText = $scope.permissions.owners.split(','); - } - else if (role === 'readers') { + } else if (role === 'readers' && typeof $scope.permissions.readers === 'string') { searchText = $scope.permissions.readers.split(','); - } - else if (role === 'writers') { + } else if (role === 'writers' && typeof $scope.permissions.writers === 'string') { searchText = $scope.permissions.writers.split(','); } + for (var i = 0; i < searchText.length; i++) { searchText[i] = searchText[i].trim(); } @@ -885,6 +886,22 @@ angular.module('zeppelinWebApp').controller('NotebookCtrl', updatePreviousList(); }; + $scope.$watch('permissions.owners', _.debounce(function(readers) { + $scope.$apply(function() { + $scope.search('owners'); + }); + }, 350)); + $scope.$watch('permissions.readers', _.debounce(function(readers) { + $scope.$apply(function() { + $scope.search('readers'); + }); + }, 350)); + $scope.$watch('permissions.writers', _.debounce(function(readers) { + $scope.$apply(function() { + $scope.search('writers'); + }); + }, 350)); + // function to find suggestion list on change $scope.search = function(role) { convertToArray(role); @@ -893,12 +910,10 @@ angular.module('zeppelinWebApp').controller('NotebookCtrl', $scope.selectIndex = -1; $scope.suggestions = []; selectedUser = searchText[selectedUserIndex]; - if(selectedUser !== ''){ - getSuggestions(selectedUser); - } - else - { - $scope.suggestions = []; + if (selectedUser !== '') { + getSuggestions(selectedUser); + } else { + $scope.suggestions = []; } }; http://git-wip-us.apache.org/repos/asf/zeppelin/blob/70524348/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 de8cbdf..9b60dd7 100644 --- a/zeppelin-web/src/app/notebook/notebook.html +++ b/zeppelin-web/src/app/notebook/notebook.html @@ -72,7 +72,7 @@ limitations under the License. data-ng-model="permissions"> <p><span class="owners">Owners </span><input ng-model="permissions.owners" placeholder="search for users" - class="input" ng-change="search('owners')" + class="input" ng-keydown="checkKeyDown($event,'owners')" ng-keyup="checkKeyUp($event)"> Owners can change permissions,read and write the note.</p> @@ -87,7 +87,7 @@ limitations under the License. </div> <p><span class="readers">Readers </span><input ng-model="permissions.readers" placeholder="search for users" - class="input" ng-change="search('readers')" + class="input" ng-keydown="checkKeyDown($event,'readers')" ng-keyup="checkKeyUp($event)"> Readers can only read the note.</p> <div ng-if="role === 'readers'" class="userlist"> @@ -101,7 +101,7 @@ limitations under the License. </div> <p><span class="writers">Writers </span><input ng-model="permissions.writers" placeholder="search for users" - class="input" ng-change="search('writers')" + class="input" ng-keydown="checkKeyDown($event,'writers')" ng-keyup="checkKeyUp($event)"> Writers can read and write the note.</p> <div ng-if="role === 'writers'" class="userlist">
