From 16d7cd4b4b74fba87b1d9a8e79a77100414c9f26 Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Tue, 03 Apr 2012 15:49:23 +0000
Subject: [PATCH] Fix OPENDJ-463: Unable to remove userPassword;deleted attributes
---
opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java | 28 +++++-
opends/tests/unit-tests-testng/src/server/org/opends/server/core/ModifyOperationTestCase.java | 164 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 184 insertions(+), 8 deletions(-)
diff --git a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
index 98ee77f..609c59b 100644
--- a/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
+++ b/opends/src/server/org/opends/server/workflowelement/localbackend/LocalBackendModifyOperation.java
@@ -23,7 +23,7 @@
*
*
* Copyright 2008-2011 Sun Microsystems, Inc.
- * Portions Copyright 2011 ForgeRock AS
+ * Portions Copyright 2011-2012 ForgeRock AS
*/
package org.opends.server.workflowelement.localbackend;
@@ -1098,18 +1098,32 @@
{
if (!isSynchronizationOperation())
{
- // If the attribute contains any options, then reject it. Passwords
- // will not be allowed to have options.
- // Skipped for internal operations.
+ // If the attribute contains any options and new values are going to
+ // be added, then reject it. Passwords will not be allowed to have
+ // options. Skipped for internal operations.
if (!isInternalOperation())
{
if (a.hasOptions())
{
- throw new DirectoryException(ResultCode.CONSTRAINT_VIOLATION,
- ERR_MODIFY_PASSWORDS_CANNOT_HAVE_OPTIONS.get());
+ switch (m.getModificationType())
+ {
+ case REPLACE:
+ if (!a.isEmpty())
+ {
+ throw new DirectoryException(ResultCode.CONSTRAINT_VIOLATION,
+ ERR_MODIFY_PASSWORDS_CANNOT_HAVE_OPTIONS.get());
+ }
+ // Allow delete operations to clean up after import.
+ break;
+ case ADD:
+ throw new DirectoryException(ResultCode.CONSTRAINT_VIOLATION,
+ ERR_MODIFY_PASSWORDS_CANNOT_HAVE_OPTIONS.get());
+ default:
+ // Allow delete operations to clean up after import.
+ break;
+ }
}
-
// If it's a self change, then see if that's allowed.
if (selfChange
&& (!pwPolicyState.getAuthenticationPolicy()
diff --git a/opends/tests/unit-tests-testng/src/server/org/opends/server/core/ModifyOperationTestCase.java b/opends/tests/unit-tests-testng/src/server/org/opends/server/core/ModifyOperationTestCase.java
index 489cc19..ac88a81 100644
--- a/opends/tests/unit-tests-testng/src/server/org/opends/server/core/ModifyOperationTestCase.java
+++ b/opends/tests/unit-tests-testng/src/server/org/opends/server/core/ModifyOperationTestCase.java
@@ -64,6 +64,7 @@
import static org.testng.Assert.*;
+import static org.opends.server.TestCaseUtils.TEST_BACKEND_ID;
import static org.opends.server.protocols.ldap.LDAPConstants.*;
@@ -4778,7 +4779,7 @@
retrieveSuccessfulOperationElements(modifyOperation);
}
- /**
+ /**
* Tests a modify operation that attempts change the user password doing
* a delete of all values followed of an add of a new value.
*
@@ -4968,7 +4969,168 @@
assertEquals(LDAPModify.mainModify(args, false, null, System.err), 0);
}
+ /**
+ * Tests that it is possible to delete userPassword attributes which have
+ * options. Options are not allowed for passwords, but we should allow users
+ * to clean them up, for example, after an import of legacy data.
+ *
+ * @throws Exception
+ * If an unexpected problem occurs.
+ */
+ @Test
+ public void testModifyDelPasswordAttributeWithOption()
+ throws Exception
+ {
+ // @formatter:off
+ TestCaseUtils.initializeTestBackend(true);
+ Entry e = TestCaseUtils.makeEntry(
+ "dn: cn=Test User,o=test",
+ "objectClass: top",
+ "objectClass: person",
+ "sn: User",
+ "cn: Test User",
+ "userPassword: password",
+ "userPassword;deleted: oldpassword");
+ Backend backend = DirectoryServer.getBackend(TEST_BACKEND_ID);
+ backend.addEntry(e, null); // Don't use add operation.
+ // Constraint violation.
+ assertEquals(TestCaseUtils.applyModifications(false,
+ "dn: cn=Test User,o=test",
+ "changetype: modify",
+ "delete: userPassword;deleted",
+ "-"
+ ), 0);
+ // @formatter:on
+
+ e = DirectoryServer.getEntry(DN.decode("cn=Test User,o=test"));
+ List<Attribute> attrList = e.getAttribute("userpassword");
+ assertNotNull(attrList);
+ assertEquals(attrList.size(), 1);
+ assertFalse(attrList.get(0).hasOptions());
+ assertEquals(attrList.get(0).size(), 1);
+ }
+
+ /**
+ * Tests that it is possible to delete userPassword attributes using an empty
+ * replace which have options. Options are not allowed for passwords, but we
+ * should allow users to clean them up, for example, after an import of legacy
+ * data.
+ *
+ * @throws Exception
+ * If an unexpected problem occurs.
+ */
+ @Test
+ public void testModifyReplaceEmptyPasswordAttributeWithOption()
+ throws Exception
+ {
+ // @formatter:off
+ TestCaseUtils.initializeTestBackend(true);
+ Entry e = TestCaseUtils.makeEntry(
+ "dn: cn=Test User,o=test",
+ "objectClass: top",
+ "objectClass: person",
+ "sn: User",
+ "cn: Test User",
+ "userPassword: password",
+ "userPassword;deleted: oldpassword");
+ Backend backend = DirectoryServer.getBackend(TEST_BACKEND_ID);
+ backend.addEntry(e, null); // Don't use add operation.
+
+ // Constraint violation.
+ assertEquals(TestCaseUtils.applyModifications(false,
+ "dn: cn=Test User,o=test",
+ "changetype: modify",
+ "replace: userPassword;deleted",
+ "-"
+ ), 0);
+ // @formatter:on
+
+ e = DirectoryServer.getEntry(DN.decode("cn=Test User,o=test"));
+ List<Attribute> attrList = e.getAttribute("userpassword");
+ assertNotNull(attrList);
+ assertEquals(attrList.size(), 1);
+ assertFalse(attrList.get(0).hasOptions());
+ assertEquals(attrList.get(0).size(), 1);
+ }
+
+ /**
+ * Tests that it is not possible to add userPassword attributes which have
+ * options. Options are not allowed for passwords.
+ *
+ * @throws Exception
+ * If an unexpected problem occurs.
+ */
+ @Test
+ public void testModifyAddPasswordAttributeWithOption()
+ throws Exception
+ {
+ // @formatter:off
+ TestCaseUtils.initializeTestBackend(true);
+ TestCaseUtils.addEntry(
+ "dn: cn=Test User,o=test",
+ "objectClass: top",
+ "objectClass: person",
+ "sn: User",
+ "cn: Test User",
+ "userPassword: password");
+
+ // Constraint violation.
+ assertEquals(TestCaseUtils.applyModifications(false,
+ "dn: cn=Test User,o=test",
+ "changetype: modify",
+ "add: userPassword;added",
+ "userPassword;added: newpassword",
+ "-"
+ ), 19);
+ // @formatter:on
+
+ Entry e = DirectoryServer.getEntry(DN.decode("cn=Test User,o=test"));
+ List<Attribute> attrList = e.getAttribute("userpassword");
+ assertNotNull(attrList);
+ assertEquals(attrList.size(), 1);
+ assertFalse(attrList.get(0).hasOptions());
+ assertEquals(attrList.get(0).size(), 1);
+ }
+
+ /**
+ * Tests that it is not possible to add userPassword attributes which have
+ * options. Options are not allowed for passwords.
+ *
+ * @throws Exception
+ * If an unexpected problem occurs.
+ */
+ @Test
+ public void testModifyReplaceWithValuesPasswordAttributeWithOption()
+ throws Exception
+ {
+ // @formatter:off
+ TestCaseUtils.initializeTestBackend(true);
+ TestCaseUtils.addEntry(
+ "dn: cn=Test User,o=test",
+ "objectClass: top",
+ "objectClass: person",
+ "sn: User",
+ "cn: Test User",
+ "userPassword: password");
+
+ // Constraint violation.
+ assertEquals(TestCaseUtils.applyModifications(false,
+ "dn: cn=Test User,o=test",
+ "changetype: modify",
+ "replace: userPassword;added",
+ "userPassword;added: newpassword",
+ "-"
+ ), 19);
+ // @formatter:on
+
+ Entry e = DirectoryServer.getEntry(DN.decode("cn=Test User,o=test"));
+ List<Attribute> attrList = e.getAttribute("userpassword");
+ assertNotNull(attrList);
+ assertEquals(attrList.size(), 1);
+ assertFalse(attrList.get(0).hasOptions());
+ assertEquals(attrList.get(0).size(), 1);
+ }
/**
* Tests that the binary option is automatically added to modifications if it
--
Gitblit v1.10.0