You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by pr...@apache.org on 2016/06/28 16:10:22 UTC
zeppelin git commit: [Zeppelin 1042] Extra space is present as part
of username in search box
Repository: zeppelin
Updated Branches:
refs/heads/master c0cee7caf -> 6f434c561
[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 <pr...@gmail.com>
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
Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/6f434c56
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/6f434c56
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/6f434c56
Branch: refs/heads/master
Commit: 6f434c5614e03d7c2ca9a6921c58c5d843b3dab0
Parents: c0cee7c
Author: Prabhjyot Singh <pr...@gmail.com>
Authored: Sat Jun 25 19:53:49 2016 +0530
Committer: Prabhjyot Singh <pr...@gmail.com>
Committed: Tue Jun 28 21:40:15 2016 +0530
----------------------------------------------------------------------
.../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/6f434c56/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/6f434c56/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/6f434c56/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/6f434c56/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 0b93ae9..0c32361 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) {
angular.element('.userlist').show();
@@ -894,12 +911,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/6f434c56/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">