From 2d63754924fee43300c218c20ac1f450ad6da558 Mon Sep 17 00:00:00 2001
From: neil_a_wilson <neil_a_wilson@localhost>
Date: Thu, 05 Jul 2007 22:59:03 +0000
Subject: [PATCH] Update the password modify extended operation so that it properly sets password policy state attributes that were not previously updated.  This primarily includes updating the last login time if that feature is enabled and the user provides the correct current password, or updating the auth failure times if that feature is enabled nad the user provides an incorrect current password.

---
 opendj-sdk/opends/src/server/org/opends/server/core/PasswordPolicyState.java                                                   |    5 +
 opendj-sdk/opends/src/server/org/opends/server/extensions/PasswordModifyExtendedOperation.java                                 |   50 +++++++++++-
 opendj-sdk/opends/src/server/org/opends/server/messages/ExtensionsMessages.java                                                |   17 ++++
 opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/extensions/PasswordModifyExtendedOperationTestCase.java |  153 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 218 insertions(+), 7 deletions(-)

diff --git a/opendj-sdk/opends/src/server/org/opends/server/core/PasswordPolicyState.java b/opendj-sdk/opends/src/server/org/opends/server/core/PasswordPolicyState.java
index d3a43fe..6c916bf 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/core/PasswordPolicyState.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/core/PasswordPolicyState.java
@@ -1342,7 +1342,10 @@
    */
   public void updateAuthFailureTimes()
   {
-    assert passwordPolicy.getLockoutFailureCount() > 0;
+    if (passwordPolicy.getLockoutFailureCount() <= 0)
+    {
+      return;
+    }
 
     if (debug)
     {
diff --git a/opendj-sdk/opends/src/server/org/opends/server/extensions/PasswordModifyExtendedOperation.java b/opendj-sdk/opends/src/server/org/opends/server/extensions/PasswordModifyExtendedOperation.java
index 6a290af..f4edbd0 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/extensions/PasswordModifyExtendedOperation.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/extensions/PasswordModifyExtendedOperation.java
@@ -64,6 +64,8 @@
 import org.opends.server.types.DirectoryException;
 import org.opends.server.types.DN;
 import org.opends.server.types.Entry;
+import org.opends.server.types.ErrorLogCategory;
+import org.opends.server.types.ErrorLogSeverity;
 import org.opends.server.types.InitializationException;
 import org.opends.server.types.LockManager;
 import org.opends.server.types.Modification;
@@ -72,6 +74,7 @@
 import org.opends.server.types.ResultCode;
 
 import static org.opends.server.extensions.ExtensionsConstants.*;
+import static org.opends.server.loggers.ErrorLogger.*;
 import static org.opends.server.loggers.debug.DebugLogger.*;
 import org.opends.server.loggers.debug.DebugTracer;
 import org.opends.server.types.DebugLogLevel;
@@ -617,12 +620,26 @@
           return;
         }
 
-        if (! pwPolicyState.passwordMatches(oldPassword))
+        if (pwPolicyState.passwordMatches(oldPassword))
+        {
+          pwPolicyState.setLastLoginTime();
+        }
+        else
         {
           operation.setResultCode(ResultCode.INVALID_CREDENTIALS);
 
           int msgID = MSGID_EXTOP_PASSMOD_INVALID_OLD_PASSWORD;
           operation.appendAdditionalLogMessage(getMessage(msgID));
+
+          pwPolicyState.updateAuthFailureTimes();
+          List<Modification> mods = pwPolicyState.getModifications();
+          if (! mods.isEmpty())
+          {
+            InternalClientConnection conn =
+                 InternalClientConnection.getRootConnection();
+            conn.processModify(userDN, mods);
+          }
+
           return;
         }
       }
@@ -1097,11 +1114,6 @@
       pwPolicyState.clearWarnedTime();
 
 
-      // Get the list of modifications from the password policy state and add
-      // them to the existing password modifications.
-      modList.addAll(pwPolicyState.getModifications());
-
-
       // If the LDAP no-op control was included in the request, then set the
       // appropriate response.  Otherwise, process the operation.
       if (noOpRequested)
@@ -1136,6 +1148,32 @@
         }
 
 
+        // If there were any password policy state changes, we need to apply
+        // them using a root connection because the end user may not have
+        // sufficient access to apply them.  This is less efficient than
+        // doing them all in the same modification, but it's safer.
+        List<Modification> pwPolicyMods = pwPolicyState.getModifications();
+        if (! pwPolicyMods.isEmpty())
+        {
+          InternalClientConnection rootConnection =
+               InternalClientConnection.getRootConnection();
+          ModifyOperation modOp =
+               rootConnection.processModify(userDN, pwPolicyMods);
+          if (modOp.getResultCode() != ResultCode.SUCCESS)
+          {
+            // At this point, the user's password is already changed so there's
+            // not much point in returning a non-success result.  However, we
+            // should at least log that something went wrong.
+            int    msgID   = MSGID_EXTOP_PASSMOD_CANNOT_UPDATE_PWP_STATE;
+            String message = getMessage(msgID, String.valueOf(userDN),
+                                        modOp.getResultCode(),
+                                        modOp.getErrorMessage());
+            logError(ErrorLogCategory.PASSWORD_POLICY,
+                     ErrorLogSeverity.SEVERE_WARNING, message, msgID);
+          }
+        }
+
+
         // If we've gotten here, then everything is OK, so indicate that the
         // operation was successful.  If a password was generated, then include
         // it in the response.
diff --git a/opendj-sdk/opends/src/server/org/opends/server/messages/ExtensionsMessages.java b/opendj-sdk/opends/src/server/org/opends/server/messages/ExtensionsMessages.java
index 4ac4dab..94afc25 100644
--- a/opendj-sdk/opends/src/server/org/opends/server/messages/ExtensionsMessages.java
+++ b/opendj-sdk/opends/src/server/org/opends/server/messages/ExtensionsMessages.java
@@ -5480,6 +5480,18 @@
 
 
   /**
+   * The message ID for the message that will be used if an error occurs while
+   * attempting to update the password policy state information in the user's
+   * entry.  This takes three arguments, which are the target user DN and the
+   * result code and error message from the internal modify operation attempting
+   * to update the state information.
+   */
+  public static final int MSGID_EXTOP_PASSMOD_CANNOT_UPDATE_PWP_STATE =
+       CATEGORY_MASK_EXTENSIONS | SEVERITY_MASK_MILD_WARNING | 528;
+
+
+
+  /**
    * Associates a set of generic messages with the message IDs defined in this
    * class.
    */
@@ -5771,6 +5783,11 @@
                     "The password modify operation was not actually " +
                     "performed in the Directory Server because the LDAP " +
                     "no-op control was present in the request");
+    registerMessage(MSGID_EXTOP_PASSMOD_CANNOT_UPDATE_PWP_STATE,
+                    "An error occurred while attempting to update the " +
+                    "password policy state information for user %s as part " +
+                    "of a password modify extended operation (result " +
+                    "code='%s', error message='%s')");
 
 
     registerMessage(MSGID_FILE_KEYMANAGER_DESCRIPTION_FILE,
diff --git a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/extensions/PasswordModifyExtendedOperationTestCase.java b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/extensions/PasswordModifyExtendedOperationTestCase.java
index 91a0e3f..4c234a8 100644
--- a/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/extensions/PasswordModifyExtendedOperationTestCase.java
+++ b/opendj-sdk/opends/tests/unit-tests-testng/src/server/org/opends/server/extensions/PasswordModifyExtendedOperationTestCase.java
@@ -51,6 +51,7 @@
 import org.opends.server.protocols.internal.InternalClientConnection;
 import org.opends.server.tools.LDAPPasswordModify;
 import org.opends.server.types.Attribute;
+import org.opends.server.types.AttributeType;
 import org.opends.server.types.AuthenticationInfo;
 import org.opends.server.types.DN;
 import org.opends.server.types.Entry;
@@ -2424,5 +2425,157 @@
     modifyOperation = conn.processModify(DN.decode(dnStr), mods);
     assertEquals(modifyOperation.getResultCode(), ResultCode.SUCCESS);
   }
+
+
+
+  /**
+   * Tests to ensure that if the user provides the correct old password, then
+   * the last login time will be updated if that feature is enabled.
+   *
+   * @throws  Exception  If an unexpected error occurs.
+   */
+  @Test()
+  public void testUpdateLastLoginTime()
+         throws Exception
+  {
+    TestCaseUtils.initializeTestBackend(true);
+
+    TestCaseUtils.applyModifications(
+      "dn: uid=test.user,o=test",
+      "changetype: add",
+      "objectClass: top",
+      "objectClass: person",
+      "objectClass: organizationalPerson",
+      "objectClass: inetOrgPerson",
+      "uid: test.user",
+      "givenName: Test",
+      "sn: User",
+      "cn: Test User",
+      "userPassword: oldpassword",
+      // FIXME -- I shouldn't have to add this ACI explicitly, but for some
+      //          reason the global ACIs are getting removed and never put back,
+      //          and without this ACI the user won't have permission to change
+      //          its own password.  If we ever change the access control syntax
+      //          then this will likely need to be updated, but I don't want to
+      //          have to give the user the bypass-acl privilege.
+      "aci: (targetattr=\"*\")(version 3.0; acl \"Self Modify Rights\"; " +
+           "allow (read,search,compare,write) userdn=\"ldap:///self\";)",
+      "",
+      "dn: cn=Default Password Policy,cn=Password Policies,cn=config",
+      "changetype: modify",
+      "replace: ds-cfg-last-login-time-attribute",
+      "ds-cfg-last-login-time-attribute: ds-pwp-last-login-time",
+      "-",
+      "replace: ds-cfg-last-login-time-format",
+      "ds-cfg-last-login-time-format: yyyyMMdd");
+
+    try
+    {
+      AttributeType lastLoginTimeAttr =
+           DirectoryServer.getAttributeType("ds-pwp-last-login-time", false);
+      assertNotNull(lastLoginTimeAttr);
+
+      DN userDN = DN.decode("uid=test.user,o=test");
+      Entry userEntry = DirectoryServer.getEntry(userDN);
+      assertNotNull(userEntry);
+      assertFalse(userEntry.hasAttribute(lastLoginTimeAttr));
+
+      String[] args =
+      {
+        "-h", "127.0.0.1",
+        "-p", String.valueOf(TestCaseUtils.getServerLdapPort()),
+        "-a", "dn:uid=test.user,o=test",
+        "-c", "oldpassword",
+        "-n", "newpassword"
+      };
+
+      int exitCode = LDAPPasswordModify.mainPasswordModify(args, false, null,
+                                                           System.err);
+      assertEquals(exitCode, 0);
+
+      userEntry = DirectoryServer.getEntry(userDN);
+      assertNotNull(userEntry);
+      assertTrue(userEntry.hasAttribute(lastLoginTimeAttr));
+    }
+    finally
+    {
+      TestCaseUtils.applyModifications(
+        "dn: cn=Default Password Policy,cn=Password Policies,cn=config",
+        "changetype: modify",
+        "replace: ds-cfg-last-login-time-attribute",
+        "-",
+        "replace: ds-cfg-last-login-time-format");
+    }
+  }
+
+
+
+  /**
+   * Tests to ensure that if the user provides an incorrect old password, then
+   * the auth failure times will be updated if that feature is enabled.
+   *
+   * @throws  Exception  If an unexpected error occurs.
+   */
+  @Test()
+  public void testUpdateAuthFailureTimes()
+         throws Exception
+  {
+    TestCaseUtils.initializeTestBackend(true);
+
+    TestCaseUtils.applyModifications(
+      "dn: uid=test.user,o=test",
+      "changetype: add",
+      "objectClass: top",
+      "objectClass: person",
+      "objectClass: organizationalPerson",
+      "objectClass: inetOrgPerson",
+      "uid: test.user",
+      "givenName: Test",
+      "sn: User",
+      "cn: Test User",
+      "userPassword: oldpassword",
+      "",
+      "dn: cn=Default Password Policy,cn=Password Policies,cn=config",
+      "changetype: modify",
+      "replace: ds-cfg-lockout-failure-count",
+      "ds-cfg-lockout-failure-count: 3");
+
+    try
+    {
+      AttributeType authFailureTimesAttr =
+           DirectoryServer.getAttributeType("pwdfailuretime", false);
+      assertNotNull(authFailureTimesAttr);
+
+      DN userDN = DN.decode("uid=test.user,o=test");
+      Entry userEntry = DirectoryServer.getEntry(userDN);
+      assertNotNull(userEntry);
+      assertFalse(userEntry.hasAttribute(authFailureTimesAttr));
+
+      String[] args =
+      {
+        "-h", "127.0.0.1",
+        "-p", String.valueOf(TestCaseUtils.getServerLdapPort()),
+        "-a", "dn:uid=test.user,o=test",
+        "-c", "wrongoldpassword",
+        "-n", "newpassword"
+      };
+
+      int exitCode = LDAPPasswordModify.mainPasswordModify(args, false, null,
+                                                           System.err);
+      assertFalse(exitCode == 0);
+
+      userEntry = DirectoryServer.getEntry(userDN);
+      assertNotNull(userEntry);
+      assertTrue(userEntry.hasAttribute(authFailureTimesAttr));
+    }
+    finally
+    {
+      TestCaseUtils.applyModifications(
+        "dn: cn=Default Password Policy,cn=Password Policies,cn=config",
+        "changetype: modify",
+        "replace: ds-cfg-lockout-failure-count",
+        "ds-cfg-lockout-failure-count: 0");
+    }
+  }
 }
 

--
Gitblit v1.10.0