svn commit: r1470127 - in /ofbiz/trunk/framework: common/config/SecurityextUiLabels.xml common/src/org/ofbiz/common/login/LoginServices.java webapp/src/org/ofbiz/webapp/control/LoginWorker.java

Author: jleroux
Date: Sat Apr 20 09:00:44 2013
New Revision: 1470127

URL: http://svn.apache.org/r1470127
Log:
A patch from Leon for "Some enhancement to password change." https://issues.apache.org/jira/browse/OFBIZ-5176

1. Make "password must be different from last passwords" function work. (--// FIXME: switching to salt-based hashing breaks this history lookup below)
2. When there's error occurs, return "requirePasswordChange" instead of "error". Then, "password change" form will not be redirected to "login" form if there's any kind of "error".
3. Fix one "deprecated" findByAnd call.
4. Return the "error" message instead of "event" message when password expires.
5. Chinese translation for "password expiration alert" and "password expired" message.

jleroux: I only reviewed (no tests) but seems good to me

Modified:
    ofbiz/trunk/framework/common/config/SecurityextUiLabels.xml
    ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java
    ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/LoginWorker.java

Modified: ofbiz/trunk/framework/common/config/SecurityextUiLabels.xml
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/config/SecurityextUiLabels.xml?rev=1470127&r1=1470126&r2=1470127&view=diff
==============================================================================
--- ofbiz/trunk/framework/common/config/SecurityextUiLabels.xml (original)
+++ ofbiz/trunk/framework/common/config/SecurityextUiLabels.xml Sat Apr 20 09:00:44 2013
@@ -169,10 +169,16 @@
     <property key="loginevents.password_expiration_alert">
         <value xml:lang="en">Alert: Your password will expire on ${passwordExpirationDate}. Update password before it expired.</value>
         <value xml:lang="fr">Votre mot de passe va expirer le ${passwordExpirationDate}, modifiez le avant.</value>
+        <value xml:lang="zh">注意：您的密码将在${passwordExpirationDate}过期，请在这之前更改密码。</value>
+        <value xml:lang="zh_CN">注意：您的密码将在${passwordExpirationDate}过期，请在这之前更改密码。</value>
+        <value xml:lang="zh_TW">注意：您的密碼將在${passwordExpirationDate}過期，請在這之前更改密碼。</value>
     </property>
     <property key="loginevents.password_expired_message">
         <value xml:lang="en">Alert: Your password expired on ${passwordExpirationDate}. Update your password.</value>
         <value xml:lang="fr">Votre mot de passe a expiré le  ${passwordExpirationDate}, modifiez le.</value>
+        <value xml:lang="zh">您的密码已于${passwordExpirationDate}过期，请更改密码。</value>
+        <value xml:lang="zh_CN">您的密码已于${passwordExpirationDate}过期，请更改密码。</value>
+        <value xml:lang="zh_TW">您的密碼已於${passwordExpirationDate}過期，請更改密碼。</value>
     </property>
     <property key="loginevents.password_hint_is">
         <value xml:lang="de">Der Passwort-Hinweis ist: ${passwordHint}.</value>

Modified: ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java?rev=1470127&r1=1470126&r2=1470127&view=diff
==============================================================================
--- ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java (original)
+++ ofbiz/trunk/framework/common/src/org/ofbiz/common/login/LoginServices.java Sat Apr 20 09:00:44 2013
@@ -925,19 +925,15 @@ public class LoginServices {
         if (passwordChangeHistoryLimit > 0 && userLogin != null) {
             Debug.logInfo(" checkNewPassword Checking if user is tyring to use old password " + passwordChangeHistoryLimit, module);
             Delegator delegator = userLogin.getDelegator();
-            String newPasswordHash = newPassword;
-            if (useEncryption) {
-                // FIXME: switching to salt-based hashing breaks this history lookup below
-                newPasswordHash = HashCrypt.cryptUTF8(getHashType(), null, newPassword);
-            }
             try {
-                List<GenericValue> pwdHistList = delegator.findByAnd("UserLoginPasswordHistory", UtilMisc.toMap("userLoginId",userLogin.getString("userLoginId"),"currentPassword",newPasswordHash), null, false);
-                Debug.logInfo(" checkNewPassword pwdHistListpwdHistList " + pwdHistList.size(), module);
-                if (pwdHistList.size() >0) {
-                    Map<String, Integer> messageMap = UtilMisc.toMap("passwordChangeHistoryLimit", passwordChangeHistoryLimit);
-                    errMsg = UtilProperties.getMessage(resource,"loginservices.password_must_be_different_from_last_passwords", messageMap, locale);
-                    errorMessageList.add(errMsg);
-                    Debug.logInfo(" checkNewPassword errorMessageListerrorMessageList " + pwdHistList.size(), module);
+                List<GenericValue> pwdHistList = delegator.findByAnd("UserLoginPasswordHistory", UtilMisc.toMap("userLoginId",userLogin.getString("userLoginId")), UtilMisc.toList("-fromDate"), false);
+                for (GenericValue pwdHistValue : pwdHistList) {
+                    if (checkPassword(pwdHistValue.getString("currentPassword"), useEncryption, newPassword)) {
+                        Map<String, Integer> messageMap = UtilMisc.toMap("passwordChangeHistoryLimit", passwordChangeHistoryLimit);
+                        errMsg = UtilProperties.getMessage(resource,"loginservices.password_must_be_different_from_last_passwords", messageMap, locale);
+                        errorMessageList.add(errMsg);
+                        break;
+                    }
                 }
             } catch (GenericEntityException e) {
                 Debug.logWarning(e, "", module);

Modified: ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/LoginWorker.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/LoginWorker.java?rev=1470127&r1=1470126&r2=1470127&view=diff
==============================================================================
--- ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/LoginWorker.java (original)
+++ ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/LoginWorker.java Sat Apr 20 09:00:44 2013
@@ -314,9 +314,10 @@ public class LoginWorker {
         if (UtilValidate.isEmpty(password)) {
             unpwErrMsgList.add(UtilProperties.getMessage(resourceWebapp, "loginevents.password_was_empty_reenter", UtilHttp.getLocale(request)));
         }
+        boolean requirePasswordChange = "Y".equals(request.getParameter("requirePasswordChange"));
         if (!unpwErrMsgList.isEmpty()) {
             request.setAttribute("_ERROR_MESSAGE_LIST_", unpwErrMsgList);
-            return "error";
+            return  requirePasswordChange ? "requirePasswordChange" : "error";
         }
 
         boolean setupNewDelegatorEtc = false;
@@ -405,7 +406,7 @@ public class LoginWorker {
         if (ModelService.RESPOND_SUCCESS.equals(result.get(ModelService.RESPONSE_MESSAGE))) {
             GenericValue userLogin = (GenericValue) result.get("userLogin");
 
-            if ("Y".equals(request.getParameter("requirePasswordChange"))) {
+            if (requirePasswordChange) {
                 Map<String, Object> inMap = UtilMisc.<String, Object>toMap("login.username", username, "login.password", password, "locale", UtilHttp.getLocale(request));
                 inMap.put("userLoginId", username);
                 inMap.put("currentPassword", password);
@@ -419,7 +420,7 @@ public class LoginWorker {
                     Map<String, String> messageMap = UtilMisc.toMap("errorMessage", e.getMessage());
                     String errMsg = UtilProperties.getMessage(resourceWebapp, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
                     request.setAttribute("_ERROR_MESSAGE_", errMsg);
-                    return "error";
+                    return "requirePasswordChange";
                 }
                 if (ServiceUtil.isError(resultPasswordChange)) {
                     String errorMessage = (String) resultPasswordChange.get(ModelService.ERROR_MESSAGE);
@@ -429,7 +430,7 @@ public class LoginWorker {
                         request.setAttribute("_ERROR_MESSAGE_", errMsg);
                     }
                     request.setAttribute("_ERROR_MESSAGE_LIST_", resultPasswordChange.get(ModelService.ERROR_MESSAGE_LIST));
-                    return "error";
+                    return "requirePasswordChange";
                 } else {
                     try {
                         userLogin.refresh();
@@ -439,7 +440,7 @@ public class LoginWorker {
                         Map<String, String> messageMap = UtilMisc.toMap("errorMessage", e.getMessage());
                         String errMsg = UtilProperties.getMessage(resourceWebapp, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
                         request.setAttribute("_ERROR_MESSAGE_", errMsg);
-                        return "error";
+                        return "requirePasswordChange";
                     }
                 }
             }
@@ -451,7 +452,7 @@ public class LoginWorker {
 
             // check to see if a password change is required for the user
             Map<String, Object> userLoginSession = checkMap(result.get("userLoginSession"), String.class, Object.class);
-            if (userLogin != null && "Y".equals(userLogin.getString("requirePasswordChange"))) {
+            if (userLogin != null && requirePasswordChange) {
                 return "requirePasswordChange";
             }
             String autoChangePassword = UtilProperties.getPropertyValue("security.properties", "user.auto.change.password.enable", "false");
@@ -478,7 +479,7 @@ public class LoginWorker {
             Map<String, String> messageMap = UtilMisc.toMap("errorMessage", (String) result.get(ModelService.ERROR_MESSAGE));
             String errMsg = UtilProperties.getMessage(resourceWebapp, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
             request.setAttribute("_ERROR_MESSAGE_", errMsg);
-            return "error";
+            return requirePasswordChange ? "requirePasswordChange" : "error";
         }
     }
 
@@ -1051,7 +1052,7 @@ public class LoginWorker {
         if (reqToChangePwdInDays > 0) {
             List<GenericValue> passwordHistories = null;
             try {
-                passwordHistories = delegator.findByAnd("UserLoginPasswordHistory", UtilMisc.toMap("userLoginId", userName));
+                passwordHistories = delegator.findByAnd("UserLoginPasswordHistory", UtilMisc.toMap("userLoginId", userName), null, false);
             } catch (GenericEntityException e) {
                 Debug.logError(e, "Cannot get user's password history record: " + e.getMessage(), module);
             }
@@ -1065,7 +1066,7 @@ public class LoginWorker {
                     if (now.after(passwordExpirationDate)) {
                         Map<String, String> messageMap = UtilMisc.toMap("passwordExpirationDate", passwordExpirationDate.toString());
                         String errMsg = UtilProperties.getMessage(resourceWebapp, "loginevents.password_expired_message", messageMap, UtilHttp.getLocale(request));
-                        request.setAttribute("_EVENT_MESSAGE_", errMsg);
+                        request.setAttribute("_ERROR_MESSAGE_", errMsg);
                         return "requirePasswordChange";
                     } else {
                         Map<String, String> messageMap = UtilMisc.toMap("passwordExpirationDate", passwordExpirationDate.toString());