[Pki-devel] [PATCH] 519 Fixed problems in group operations.
Endi Sukma Dewata
edewata at redhat.com
Wed Aug 27 14:38:01 UTC 2014
Previously modifying the description of an empty group failed
because the server tried to delete a uniqueMember attribute that
did not exist because the group was already empty. The servlets and
group subsystem has been fixed to retrieve the existing group data
first, perform the changes on it, then save it back to the database.
Also adding a new group will no longer require a description because
it's not required by the LDAP object class.
Ticket #818
--
Endi S. Dewata
-------------- next part --------------
From 2345efe854386ee156f3399c8adcb152fb2bf58a Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata" <edewata at redhat.com>
Date: Tue, 26 Aug 2014 18:49:56 -0400
Subject: [PATCH] Fixed problems in group operations.
Previously modifying the description of an empty group failed
because the server tried to delete a uniqueMember attribute that
did not exist because the group was already empty. The servlets and
group subsystem has been fixed to retrieve the existing group data
first, perform the changes on it, then save it back to the database.
Also adding a new group will no longer require a description because
it's not required by the LDAP object class.
Ticket #818
---
.../com/netscape/cmstools/group/GroupAddCLI.java | 3 +-
.../cms/servlet/admin/UsrGrpAdminServlet.java | 37 ++++++-----
.../org/dogtagpki/server/rest/GroupService.java | 15 +++--
.../src/com/netscape/cmscore/usrgrp/Group.java | 16 ++++-
.../com/netscape/cmscore/usrgrp/UGSubsystem.java | 75 +++++++++++++---------
5 files changed, 93 insertions(+), 53 deletions(-)
diff --git a/base/java-tools/src/com/netscape/cmstools/group/GroupAddCLI.java b/base/java-tools/src/com/netscape/cmstools/group/GroupAddCLI.java
index 6d824fe1ad034a595295c1467cc39047b5f63b18..8bb529aeff4a4e7912eb0f1ee27b525f6289c87c 100644
--- a/base/java-tools/src/com/netscape/cmstools/group/GroupAddCLI.java
+++ b/base/java-tools/src/com/netscape/cmstools/group/GroupAddCLI.java
@@ -42,13 +42,12 @@ public class GroupAddCLI extends CLI {
}
public void printHelp() {
- formatter.printHelp(getFullName() + " <Group ID> --description <Description> [OPTIONS...]", options);
+ formatter.printHelp(getFullName() + " <Group ID> [OPTIONS...]", options);
}
public void createOptions() {
Option option = new Option(null, "description", true, "Description");
option.setArgName("description");
- option.setRequired(true);
options.addOption(option);
}
diff --git a/base/server/cms/src/com/netscape/cms/servlet/admin/UsrGrpAdminServlet.java b/base/server/cms/src/com/netscape/cms/servlet/admin/UsrGrpAdminServlet.java
index 836369bc47ca9b4f1a5301326fa87862577d1016..cce1ce3f41748bd03ea07ac857e23265f8ff70f3 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/admin/UsrGrpAdminServlet.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/admin/UsrGrpAdminServlet.java
@@ -53,6 +53,7 @@ import com.netscape.certsrv.logging.ILogger;
import com.netscape.certsrv.password.IPasswordCheck;
import com.netscape.certsrv.usrgrp.EUsrGrpException;
import com.netscape.certsrv.usrgrp.IGroup;
+import com.netscape.certsrv.usrgrp.IGroupConstants;
import com.netscape.certsrv.usrgrp.IUGSubsystem;
import com.netscape.certsrv.usrgrp.IUser;
import com.netscape.cmsutil.util.Cert;
@@ -1673,17 +1674,15 @@ public class UsrGrpAdminServlet extends AdminServlet {
}
IGroup group = mMgr.createGroup(id);
- String members = super.getParameter(req,
- Constants.PR_GROUP_USER);
- String desc = super.getParameter(req,
- Constants.PR_GROUP_DESC);
- if (desc != null) {
- group.set("description", desc);
- } else {
- group.set("description", "");
+ // add description if specified
+ String description = super.getParameter(req, Constants.PR_GROUP_DESC);
+ if (description != null && !description.equals("")) {
+ group.set(IGroupConstants.ATTR_DESCRIPTION, description);
}
+ // add members if specified
+ String members = super.getParameter(req, Constants.PR_GROUP_USER);
if (members != null) {
StringTokenizer st = new StringTokenizer(members, ",");
@@ -1917,18 +1916,25 @@ public class UsrGrpAdminServlet extends AdminServlet {
return;
}
- IGroup group = mMgr.createGroup(id);
+ IGroup group = mMgr.getGroupFromName(id);
- String desc = super.getParameter(req,
- Constants.PR_GROUP_DESC);
-
- if (desc != null) {
- group.set("description", desc);
+ // update description if specified
+ String description = super.getParameter(req, Constants.PR_GROUP_DESC);
+ if (description != null) {
+ if (description.equals("")) {
+ group.delete(IGroupConstants.ATTR_DESCRIPTION);
+ } else {
+ group.set(IGroupConstants.ATTR_DESCRIPTION, description);
+ }
}
+ // update members if specified
String members = super.getParameter(req, Constants.PR_GROUP_USER);
-
if (members != null) {
+ // empty old member list
+ group.delete(IGroupConstants.ATTR_MEMBERS);
+
+ // read new member list
StringTokenizer st = new StringTokenizer(members, ",");
String groupName = group.getName();
@@ -1938,6 +1944,7 @@ public class UsrGrpAdminServlet extends AdminServlet {
multiRole = mConfig.getBoolean(MULTI_ROLE_ENABLE);
} catch (Exception eee) {
}
+
while (st.hasMoreTokens()) {
String memberName = st.nextToken();
if (multiRole) {
diff --git a/base/server/cms/src/org/dogtagpki/server/rest/GroupService.java b/base/server/cms/src/org/dogtagpki/server/rest/GroupService.java
index 2f0a1699651292262c88106df94b5dc70a7612a9..991a8b155813c1cf5ca4ee27c448b77c9e5c150e 100644
--- a/base/server/cms/src/org/dogtagpki/server/rest/GroupService.java
+++ b/base/server/cms/src/org/dogtagpki/server/rest/GroupService.java
@@ -48,6 +48,7 @@ import com.netscape.certsrv.group.GroupResource;
import com.netscape.certsrv.logging.IAuditor;
import com.netscape.certsrv.logging.ILogger;
import com.netscape.certsrv.usrgrp.IGroup;
+import com.netscape.certsrv.usrgrp.IGroupConstants;
import com.netscape.certsrv.usrgrp.IUGSubsystem;
import com.netscape.cms.servlet.admin.GroupMemberProcessor;
import com.netscape.cms.servlet.base.PKIService;
@@ -210,11 +211,10 @@ public class GroupService extends PKIService implements GroupResource {
IGroup group = userGroupManager.createGroup(groupID);
+ // add description if specified
String description = groupData.getDescription();
- if (description != null) {
- group.set("description", description);
- } else {
- group.set("description", "");
+ if (description != null && !description.equals("")) {
+ group.set(IGroupConstants.ATTR_DESCRIPTION, description);
}
// allow adding a group with no members
@@ -271,9 +271,14 @@ public class GroupService extends PKIService implements GroupResource {
throw new ResourceNotFoundException("Group " + groupID + " not found.");
}
+ // update description if specified
String description = groupData.getDescription();
if (description != null) {
- group.set("description", description);
+ if (description.equals("")) { // remove value if empty
+ group.delete(IGroupConstants.ATTR_DESCRIPTION);
+ } else { // otherwise replace value
+ group.set(IGroupConstants.ATTR_DESCRIPTION, description);
+ }
}
// allow adding a group with no members, except "Certificate
diff --git a/base/server/cmscore/src/com/netscape/cmscore/usrgrp/Group.java b/base/server/cmscore/src/com/netscape/cmscore/usrgrp/Group.java
index 25917e901da5b2993cf844d723627ef800c80bbf..fe5d9e1d04d7b69b1292f61ec8a6ac7f83e8dc38 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/usrgrp/Group.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/usrgrp/Group.java
@@ -39,7 +39,10 @@ public class Group implements IGroup {
@SuppressWarnings("unused")
private IUsrGrp mBase;
private String mName = null;
+
+ // TODO: replace Vector with Set
private Vector<String> mMembers = new Vector<String>();
+
private String mDescription = null;
private static final Vector<String> mNames = new Vector<String>();
@@ -71,6 +74,7 @@ public class Group implements IGroup {
}
public void addMemberName(String name) {
+ if (isMember(name)) return;
mMembers.addElement(name);
}
@@ -117,7 +121,17 @@ public class Group implements IGroup {
}
public void delete(String name) throws EBaseException {
- throw new EBaseException(CMS.getUserMessage("CMS_BASE_INVALID_ATTRIBUTE", name));
+ if (name.equals(ATTR_NAME)) {
+ mName = null;
+ } else if (name.equals(ATTR_ID)) {
+ mName = null;
+ } else if (name.equals(ATTR_MEMBERS)) {
+ mMembers.clear();
+ } else if (name.equals(ATTR_DESCRIPTION)) {
+ mDescription = null;
+ } else {
+ throw new EBaseException(CMS.getUserMessage("CMS_BASE_INVALID_ATTRIBUTE", name));
+ }
}
public Enumeration<String> getElements() {
diff --git a/base/server/cmscore/src/com/netscape/cmscore/usrgrp/UGSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/usrgrp/UGSubsystem.java
index 245115e755a735efeabcc7bb082f6d882159832e..a2655bf82f32a15e775d15fa04cf057a9958886c 100644
--- a/base/server/cmscore/src/com/netscape/cmscore/usrgrp/UGSubsystem.java
+++ b/base/server/cmscore/src/com/netscape/cmscore/usrgrp/UGSubsystem.java
@@ -1725,28 +1725,39 @@ public final class UGSubsystem implements IUGSubsystem {
LDAPConnection ldapconn = null;
try {
+ String dn = "cn=" + LDAPUtil.escapeRDNValue(grp.getGroupID()) + "," + getGroupBaseDN();
+ CMS.debug("dn: " + dn);
+
LDAPAttributeSet attrs = new LDAPAttributeSet();
String oc[] = { "top", "groupOfUniqueNames" };
attrs.add(new LDAPAttribute("objectclass", oc));
attrs.add(new LDAPAttribute("cn", group.getGroupID()));
- attrs.add(new LDAPAttribute("description", group.getDescription()));
+
+ String description = group.getDescription();
+ if (description != null) {
+ CMS.debug("description: " + description);
+ attrs.add(new LDAPAttribute("description", description));
+ }
+
Enumeration<String> e = grp.getMemberNames();
- if (e.hasMoreElements() == true) {
+ if (e.hasMoreElements()) {
LDAPAttribute attrMembers = new LDAPAttribute("uniquemember");
while (e.hasMoreElements()) {
String name = e.nextElement();
+ String memberDN = "uid=" + LDAPUtil.escapeRDNValue(name) + "," + getUserBaseDN();
+ CMS.debug("uniqueMember: " + memberDN);
+
// DOES NOT SUPPORT NESTED GROUPS...
- attrMembers.addValue("uid=" + LDAPUtil.escapeRDNValue(name) + "," +
- getUserBaseDN());
+ attrMembers.addValue(memberDN);
}
attrs.add(attrMembers);
}
- LDAPEntry entry = new LDAPEntry("cn=" + LDAPUtil.escapeRDNValue(grp.getGroupID()) +
- "," + getGroupBaseDN(), attrs);
+
+ LDAPEntry entry = new LDAPEntry(dn, attrs);
ldapconn = getConn();
ldapconn.add(entry);
@@ -1796,6 +1807,11 @@ public final class UGSubsystem implements IUGSubsystem {
}
}
+ /**
+ * Modifies an existing group in the database.
+ *
+ * @param group an existing group that has been modified in memory
+ */
public void modifyGroup(IGroup group) throws EUsrGrpException {
Group grp = (Group) group;
@@ -1806,39 +1822,38 @@ public final class UGSubsystem implements IUGSubsystem {
LDAPConnection ldapconn = null;
try {
- LDAPAttribute attrMembers = new LDAPAttribute("uniquemember");
+ String dn = "cn=" + LDAPUtil.escapeRDNValue(grp.getGroupID()) + "," + getGroupBaseDN();
+ CMS.debug("dn: " + dn);
+
LDAPModificationSet mod = new LDAPModificationSet();
- String desc = grp.getDescription();
-
- if (desc != null) {
- mod.add(LDAPModification.REPLACE,
- new LDAPAttribute("description", desc));
- }
+ // update description
+ String description = grp.getDescription();
+ mod.add(LDAPModification.REPLACE, new LDAPAttribute("description", description));
+ CMS.debug("description: " + description);
Enumeration<String> e = grp.getMemberNames();
- if (e.hasMoreElements() == true) {
- while (e.hasMoreElements()) {
- String name = e.nextElement();
+ // admin group cannot be empty
+ if (grp.getName().equalsIgnoreCase(SUPER_CERT_ADMINS) && !e.hasMoreElements()) {
+ throw new EUsrGrpException(CMS.getUserMessage("CMS_USRGRP_ILL_GRP_MOD"));
+ }
+
+ // update members
+ LDAPAttribute attrMembers = new LDAPAttribute("uniquemember");
+ while (e.hasMoreElements()) {
+ String name = e.nextElement();
+
+ String memberDN = "uid=" + LDAPUtil.escapeRDNValue(name) + "," + getUserBaseDN();
+ CMS.debug("uniqueMember: " + memberDN);
- // DOES NOT SUPPORT NESTED GROUPS...
- attrMembers.addValue("uid=" + LDAPUtil.escapeRDNValue(name) + "," +
- getUserBaseDN());
- }
- mod.add(LDAPModification.REPLACE, attrMembers);
- } else {
- if (!grp.getName().equalsIgnoreCase(SUPER_CERT_ADMINS)) {
- mod.add(LDAPModification.DELETE, attrMembers);
- } else {
- // not allowed
- throw new EUsrGrpException(CMS.getUserMessage("CMS_USRGRP_ILL_GRP_MOD"));
- }
+ // DOES NOT SUPPORT NESTED GROUPS...
+ attrMembers.addValue(memberDN);
}
+ mod.add(LDAPModification.REPLACE, attrMembers);
ldapconn = getConn();
- ldapconn.modify("cn=" + LDAPUtil.escapeRDNValue(grp.getGroupID()) +
- "," + getGroupBaseDN(), mod);
+ ldapconn.modify(dn, mod);
} catch (LDAPException e) {
log(ILogger.LL_FAILURE, CMS.getLogMessage("CMSCORE_USRGRP_MODIFY_GROUP", e.toString()));
--
1.8.4.2
More information about the Pki-devel
mailing list