[Pki-devel] [PATCH] 0027..0029 support external authorization LDAP server
Fraser Tweedale
ftweedal at redhat.com
Fri Mar 13 07:00:26 UTC 2015
Endi, thanks for reviewing.
New patch attached; additional comments inline.
On Thu, Mar 12, 2015 at 11:35:18PM +0700, Endi Sukma Dewata wrote:
> Sorry, revising my comments based on the latest patches.
>
> On 3/12/2015 10:50 PM, Endi Sukma Dewata wrote:
> >>>1. The TOKEN_GROUPS probably should be a List<String> to simplify the
> >>>creation and the usage of the list of groups.
> >>>
> >>We are constrained by the IAuthToken interface, which has only the
> >>String[] method.
> >
> >In AuthToken the attributes are actually stored in a Hashtable<String,
> >Object>, and there's already a generic get() that returns Object, so we
> >probably can just add a set(String name, Object value). If the list of
> >groups is only kept in memory I don't see a reason to concatenate them
> >into a single string.
>
> Scratch the last sentence, but I think the List<String> is still better
> because you won't need to convert into array of Strings.
>
I have left it as String[] for now. If we want List<String> we can
add additional methods to the interface rather than casting to/from
Object all over the place.
> >3. The listGroups() is always called with "*" filter so it will pull all
> >groups in the database, then the code will execute a search operation
> >for each group to check if the user is a member. I think this is highly
> >inefficient. The listGroups() should have been called with the right
> >filter to get the right list of groups with a single search operation.
>
> Never mind. The new patches already addressed this issue. The comments below
> are still valid.
>
> >4. When constructing a DN the attribute values should be escaped with
> >LDAPUtil.escapeRDNValue() to prevent problems with DN special characters.
> >
> >5. When constructing LDAP filter the attribute values should be escaped
> >with LDAPUtil.escapeFilter() to prevent problems with LDAP filter
> >special characters.
> >
I have escaped end-user-supplied values, but not config values.
> >6. If there's an exception the method should rethrow the exception so
> >the client will see the problem.
> >
No more squashing of exceptions.
> >7. The array of groups can be added into a list with a single operation:
> >
> > groups.addAll(Arrays.asList(groupsArray));
> >
Superseded by item 9.
> >8. The Utils.stripQuotes() right now is called multiple times for the
> >same value. It can be simplified as follows:
> >
> > matched = groups.contains(Utils.stripQuotes(value));
> >
Done.
>
> One more thing:
>
> 9. I see the "gid" attribute being set in TokenAuthentication. Probably it
> should be changed to IAuthToken.GID so we know where it's set and used.
> Alternatively we can replace it with IAuthToken.GROUPS that contains just a
> single group, and remove IAuthToken.GID.
>
Good suggestion; implemented.
> --
> Endi S. Dewata
-------------- next part --------------
>From 14ecc408abbbcafa226ca26885b3a01881a7765a Mon Sep 17 00:00:00 2001
From: Christina Fu <cfu at redhat.com>
Date: Tue, 10 Mar 2015 02:06:02 -0400
Subject: [PATCH] Store groups on AuthToken and update group evaluator
Update the UidPwdDirAuthentication plugin to retrieve all the user's
groups from a directory and store them on the AuthToken.
Also update the group evaluator to match against all the groups
stored in the AuthToken. The "gid" and "groups" are merged into a
single collection, if the ACL operation is "=" the collection is
checked under disjunction, and if the operation is "!=", then
conjunction.
---
.../certsrv/authentication/IAuthToken.java | 2 +
.../cms/authentication/DirBasedAuthentication.java | 36 ++++++-
.../cms/authentication/TokenAuthentication.java | 9 +-
.../authentication/UidPwdDirAuthentication.java | 104 ++++++++++++++++++---
.../cms/evaluators/GroupAccessEvaluator.java | 21 ++---
5 files changed, 139 insertions(+), 33 deletions(-)
diff --git a/base/common/src/com/netscape/certsrv/authentication/IAuthToken.java b/base/common/src/com/netscape/certsrv/authentication/IAuthToken.java
index 3c03cc1f5e85a92237067fde98e20d9f13a0b947..a71432446edcf6b5d838f1115df16b26acd01dce 100644
--- a/base/common/src/com/netscape/certsrv/authentication/IAuthToken.java
+++ b/base/common/src/com/netscape/certsrv/authentication/IAuthToken.java
@@ -38,6 +38,8 @@ public interface IAuthToken {
* Constant for userid.
*/
public static final String USER_ID = "userid";
+ public static final String UID = "uid";
+ public static final String GROUPS = "groups";
/**
* Sets an attribute value within this AttrSet.
diff --git a/base/server/cms/src/com/netscape/cms/authentication/DirBasedAuthentication.java b/base/server/cms/src/com/netscape/cms/authentication/DirBasedAuthentication.java
index f2d09df9e410ac817e2a90d4f82bbd4c488a8ab2..78aa399b41263c3da1b050f1729eb48157d730e4 100644
--- a/base/server/cms/src/com/netscape/cms/authentication/DirBasedAuthentication.java
+++ b/base/server/cms/src/com/netscape/cms/authentication/DirBasedAuthentication.java
@@ -74,6 +74,13 @@ public abstract class DirBasedAuthentication
/* configuration parameter keys */
protected static final String PROP_LDAP = "ldap";
protected static final String PROP_BASEDN = "basedn";
+ protected static final String PROP_GROUPS_ENABLE = "groupsEnable";
+ protected static final String PROP_GROUPS_BASEDN = "groupsBasedn";
+ protected static final String PROP_GROUPS = "groups";
+ protected static final String PROP_GROUP_OBJECT_CLASS = "groupObjectClass";
+ protected static final String PROP_GROUP_USERID_NAME = "groupUseridName";
+ protected static final String PROP_USERID_NAME = "useridName";
+ protected static final String PROP_SEARCH_GROUP_USER_BY_USERDN = "searchGroupUserByUserdn";
protected static final String PROP_DNPATTERN = "dnpattern";
protected static final String PROP_LDAPSTRINGATTRS = "ldapStringAttributes";
protected static final String PROP_LDAPBYTEATTRS = "ldapByteAttributes";
@@ -94,6 +101,14 @@ public abstract class DirBasedAuthentication
/* ldap base dn */
protected String mBaseDN = null;
+ protected boolean mGroupsEnable = false;
+ protected String mGroups = null; // e.g. "ou=Groups"
+ protected String mGroupsBaseDN = null; // in case it's different from mBaseDN
+ protected String mGroupObjectClass = null;
+ protected String mUserIDName = null; // e.g. "uid"
+ protected String mGroupUserIDName = null; // in case it's different from mUserIDName
+ /* whether to search for member=<userDN> or member=<mGroupUserIdName>=<uid> */
+ protected boolean mSearchGroupUserByUserdn = true;
/* factory of anonymous ldap connections */
protected ILdapConnFactory mConnFactory = null;
@@ -243,10 +258,25 @@ public abstract class DirBasedAuthentication
/* initialize ldap server configuration */
mLdapConfig = mConfig.getSubStore(PROP_LDAP);
- if (needBaseDN)
+ if (needBaseDN) {
mBaseDN = mLdapConfig.getString(PROP_BASEDN);
- if (needBaseDN && ((mBaseDN == null) || (mBaseDN.length() == 0) || (mBaseDN.trim().equals(""))))
- throw new EPropertyNotFound(CMS.getUserMessage("CMS_BASE_GET_PROPERTY_FAILED", "basedn"));
+ if (mBaseDN == null || mBaseDN.trim().equals(""))
+ throw new EPropertyNotFound(CMS.getUserMessage("CMS_BASE_GET_PROPERTY_FAILED", "basedn"));
+ mGroupsEnable = mLdapConfig.getBoolean(PROP_GROUPS_ENABLE, false);
+ CMS.debug("DirBasedAuthentication: mGroupsEnable=" + (mGroupsEnable ? "true" : "false"));
+ mGroupsBaseDN = mLdapConfig.getString(PROP_GROUPS_BASEDN, mBaseDN);
+ CMS.debug("DirBasedAuthentication: mGroupsBaseDN="+ mGroupsBaseDN);
+ mGroups= mLdapConfig.getString(PROP_GROUPS, "ou=groups");
+ CMS.debug("DirBasedAuthentication: mGroups="+ mGroups);
+ mGroupObjectClass = mLdapConfig.getString(PROP_GROUP_OBJECT_CLASS, "groupofuniquenames");
+ CMS.debug("DirBasedAuthentication: mGroupObjectClass="+ mGroupObjectClass);
+ mUserIDName = mLdapConfig.getString(PROP_USERID_NAME, "uid");
+ CMS.debug("DirBasedAuthentication: mUserIDName="+ mUserIDName);
+ mSearchGroupUserByUserdn = mLdapConfig.getBoolean(PROP_SEARCH_GROUP_USER_BY_USERDN, true);
+ CMS.debug("DirBasedAuthentication: mSearchGroupUserByUserdn="+ mSearchGroupUserByUserdn);
+ mGroupUserIDName = mLdapConfig.getString(PROP_GROUP_USERID_NAME, "cn");
+ CMS.debug("DirBasedAuthentication: mGroupUserIDName="+ mGroupUserIDName);
+ }
mConnFactory = CMS.getLdapAnonConnFactory();
mConnFactory.init(mLdapConfig);
diff --git a/base/server/cms/src/com/netscape/cms/authentication/TokenAuthentication.java b/base/server/cms/src/com/netscape/cms/authentication/TokenAuthentication.java
index 99abad7eb6ccd676f1d4f09d569f2f83c13adfbc..5eeddecb38c5a780fcab22a54dd9a13bdf88537c 100644
--- a/base/server/cms/src/com/netscape/cms/authentication/TokenAuthentication.java
+++ b/base/server/cms/src/com/netscape/cms/authentication/TokenAuthentication.java
@@ -54,10 +54,6 @@ import com.netscape.cmsutil.xml.XMLObject;
public class TokenAuthentication implements IAuthManager,
IProfileAuthenticator {
- /* result auth token attributes */
- public static final String TOKEN_UID = "uid";
- public static final String TOKEN_GID = "gid";
-
/* required credentials */
public static final String CRED_SESSION_ID = IAuthManager.CRED_SESSION_ID;
protected String[] mRequiredCreds = { CRED_SESSION_ID };
@@ -191,9 +187,10 @@ public class TokenAuthentication implements IAuthManager,
String uid = parser.getValue("uid");
String gid = parser.getValue("gid");
+ String[] groups = {gid};
- authToken.set(TOKEN_UID, uid);
- authToken.set(TOKEN_GID, gid);
+ authToken.set(IAuthToken.UID, uid);
+ authToken.set(IAuthToken.GROUPS, groups);
if (context != null) {
CMS.debug("SessionContext.USER_ID " + uid + " SessionContext.GROUP_ID " + gid);
diff --git a/base/server/cms/src/com/netscape/cms/authentication/UidPwdDirAuthentication.java b/base/server/cms/src/com/netscape/cms/authentication/UidPwdDirAuthentication.java
index c85644d59eb4ab0354517140c623f3e36eb5d93c..b750c4f713cf3e0ce9c15ae8549c10bdd8c2abd1 100644
--- a/base/server/cms/src/com/netscape/cms/authentication/UidPwdDirAuthentication.java
+++ b/base/server/cms/src/com/netscape/cms/authentication/UidPwdDirAuthentication.java
@@ -18,10 +18,12 @@
package com.netscape.cms.authentication;
// ldap java sdk
+import java.util.ArrayList;
import java.util.Enumeration;
import java.util.Locale;
import java.util.Vector;
+import netscape.ldap.LDAPAttribute;
import netscape.ldap.LDAPConnection;
import netscape.ldap.LDAPEntry;
import netscape.ldap.LDAPException;
@@ -45,6 +47,8 @@ import com.netscape.certsrv.profile.IProfileAuthenticator;
import com.netscape.certsrv.property.Descriptor;
import com.netscape.certsrv.property.IDescriptor;
import com.netscape.certsrv.request.IRequest;
+import com.netscape.certsrv.usrgrp.EUsrGrpException;
+import com.netscape.cmsutil.ldap.LDAPUtil;
/**
* uid/pwd directory based authentication manager
@@ -59,7 +63,6 @@ public class UidPwdDirAuthentication extends DirBasedAuthentication
public static final String CRED_UID = "uid";
public static final String CRED_PWD = "pwd";
protected static String[] mRequiredCreds = { CRED_UID, CRED_PWD };
- public static final String USERID = "userid";
/* Holds configuration parameters accepted by this implementation.
* This list is passed to the configuration console so configuration
@@ -89,10 +92,58 @@ public class UidPwdDirAuthentication extends DirBasedAuthentication
};
/**
- * Default constructor, initialization must follow.
+ * Retrieves group base dn.
*/
- public UidPwdDirAuthentication() {
- super();
+ private String getGroupBaseDN() {
+ return mGroups + "," + mGroupsBaseDN;
+ }
+
+ /**
+ * List groups of which user is a member.
+ */
+ private ArrayList<String> listGroups(LDAPConnection ldapconn, String uid, String userdn)
+ throws EUsrGrpException, LDAPException {
+ String method = "UidPwdDirAuthentication: listGroups: ";
+ CMS.debug(method + " begins");
+ String[] attrs = {};
+
+ String k = null;
+ if (mGroupObjectClass.equalsIgnoreCase("groupOfUniqueNames"))
+ k = "uniquemember";
+ else if (mGroupObjectClass.equalsIgnoreCase("groupOfNames"))
+ k = "member";
+ else {
+ CMS.debug("UidPwdDirAuthentication: isMemberOfLdapGroup: unrecognized mGroupObjectClass: " + mGroupObjectClass);
+ return null;
+ }
+
+ String filter = null;
+ if (mSearchGroupUserByUserdn)
+ filter = k + "=" + userdn;
+ else
+ filter = k + "=" + mGroupUserIDName + "=" + LDAPUtil.escapeFilter(uid);
+
+ CMS.debug(method + "searching " + getGroupBaseDN() + " for (&(objectclass=" + mGroupObjectClass + ")(" + filter + "))");
+ LDAPSearchResults res = ldapconn.search(
+ getGroupBaseDN(),
+ LDAPv2.SCOPE_SUB,
+ "(&(objectclass=" + mGroupObjectClass + ")(" + filter + "))",
+ attrs, true /* attrsOnly */ );
+
+ CMS.debug(method + " ends");
+ return buildGroups(res);
+ }
+
+ private ArrayList<String> buildGroups(LDAPSearchResults res) {
+ ArrayList<String> v = new ArrayList<>();
+
+ while (res.hasMoreElements()) {
+ LDAPEntry entry = (LDAPEntry) res.nextElement();
+ String groupDN = entry.getDN();
+ CMS.debug("UidPwdDirAuthentication: Authenticate: Found group membership: " + groupDN);
+ v.add(groupDN);
+ }
+ return v;
}
/**
@@ -131,18 +182,29 @@ public class UidPwdDirAuthentication extends DirBasedAuthentication
throw new EInvalidCredentials(CMS.getUserMessage("CMS_AUTHENTICATION_INVALID_CREDENTIAL"));
}
+ /*
+ * first try and see if the directory server supports "memberOf"
+ * if so, use it, if not, then pull all groups to check
+ */
+ String emptyAttrs[] = {};
+ String groupAttrs[] = {"memberOf"};
+
// get user dn.
- CMS.debug("Authenticating: Searching for UID=" + uid +
- " base DN=" + mBaseDN);
- LDAPSearchResults res = conn.search(mBaseDN,
- LDAPv2.SCOPE_SUB, "(uid=" + uid + ")", null, false);
+ CMS.debug("UidPwdDirAuthentication: Authenticating: Searching for " +
+ mUserIDName + "=" + uid + " base DN=" + mBaseDN);
+ LDAPSearchResults res = conn.search(
+ mBaseDN,
+ LDAPv2.SCOPE_SUB,
+ "(" + mUserIDName + "=" + LDAPUtil.escapeFilter(uid) + ")",
+ (mGroupsEnable ? groupAttrs : emptyAttrs),
+ false);
+ LDAPEntry entry = null;
if (res.hasMoreElements()) {
- //LDAPEntry entry = (LDAPEntry)res.nextElement();
- LDAPEntry entry = res.next();
+ entry = res.next();
userdn = entry.getDN();
- CMS.debug("Authenticating: Found User DN=" + userdn);
+ CMS.debug("UidPwdDirAuthentication: Authenticating: Found User DN=" + userdn);
} else {
log(ILogger.LL_SECURITY, CMS.getLogMessage("CMS_AUTH_USER_NOT_EXIST", uid));
throw new EInvalidCredentials(CMS.getUserMessage("CMS_AUTHENTICATION_INVALID_CREDENTIAL"));
@@ -150,9 +212,25 @@ public class UidPwdDirAuthentication extends DirBasedAuthentication
// bind as user dn and pwd - authenticates user with pwd.
conn.authenticate(userdn, pwd);
+
+ LDAPAttribute attribute = entry.getAttribute("memberOf");
+ if ( attribute != null ) {
+ CMS.debug("UidPwdDirAuthentication: Authenticate: Found memberOf attribute");
+ String[] groups = attribute.getStringValueArray();
+ token.set(IAuthToken.GROUPS, groups);
+ } else if (mGroupsEnable) {
+ CMS.debug("UidPwdDirAuthentication: Authenticate: memberOf attribute not found.");
+ ArrayList<String> groups = null;
+ groups = listGroups(conn, uid, userdn);
+ if (groups != null) {
+ String[] groupsArray = new String[groups.size()];
+ token.set(IAuthToken.GROUPS, groups.toArray(groupsArray));
+ }
+ }
+
// set uid in the token.
- token.set(CRED_UID, uid);
- token.set(USERID, uid);
+ token.set(IAuthToken.UID, uid);
+ token.set(IAuthToken.USER_ID, uid);
return userdn;
} catch (ELdapException e) {
diff --git a/base/server/cms/src/com/netscape/cms/evaluators/GroupAccessEvaluator.java b/base/server/cms/src/com/netscape/cms/evaluators/GroupAccessEvaluator.java
index e8a32d752be327bf8cbb4c45d5717f84900fbc4a..1f3717851f2193ab1ed9cfc497d604994f998225 100644
--- a/base/server/cms/src/com/netscape/cms/evaluators/GroupAccessEvaluator.java
+++ b/base/server/cms/src/com/netscape/cms/evaluators/GroupAccessEvaluator.java
@@ -17,6 +17,7 @@
// --- END COPYRIGHT BLOCK ---
package com.netscape.cms.evaluators;
+import java.util.Arrays;
import com.netscape.certsrv.apps.CMS;
import com.netscape.certsrv.authentication.IAuthToken;
import com.netscape.certsrv.base.EBaseException;
@@ -101,9 +102,9 @@ public class GroupAccessEvaluator implements IAccessEvaluator {
// should define "uid" at a common place
String uid = null;
- uid = authToken.getInString("userid");
+ uid = authToken.getInString(IAuthToken.USER_ID);
if (uid == null) {
- uid = authToken.getInString("uid");
+ uid = authToken.getInString(IAuthToken.UID);
if (uid == null) {
CMS.debug("GroupAccessEvaluator: evaluate: uid null");
log(ILogger.LL_FAILURE, CMS.getLogMessage("EVALUTOR_UID_NULL"));
@@ -112,15 +113,13 @@ public class GroupAccessEvaluator implements IAccessEvaluator {
}
CMS.debug("GroupAccessEvaluator: evaluate: uid=" + uid + " value=" + value);
- String groupname = authToken.getInString("gid");
-
- if (groupname != null) {
- CMS.debug("GroupAccessEvaluator: evaluate: authToken gid=" + groupname);
- if (op.equals("=")) {
- return groupname.equals(Utils.stripQuotes(value));
- } else if (op.equals("!=")) {
- return !groupname.equals(Utils.stripQuotes(value));
- }
+ String[] groups = authToken.getInStringArray(IAuthToken.GROUPS);
+ if (groups != null) {
+ boolean matched = Arrays.asList(groups).contains(Utils.stripQuotes(value));
+ if (op.equals("="))
+ return matched;
+ else if (op.equals("!="))
+ return !matched;
} else {
CMS.debug("GroupAccessEvaluator: evaluate: no gid in authToken");
IUser id = null;
--
2.1.0
More information about the Pki-devel
mailing list