[Change Password] Code review applied:
authorAlberto Santos <alberto.santos@openbravo.com>
Tue, 02 Feb 2016 14:11:27 +0100
changeset 28799 f62d9ee30060
parent 28798 8289713e0e69
child 28800 75cb35d38d02
[Change Password] Code review applied:
Rename property Lastupdatepassworddate to lastPasswordUpdate.
Be consistent in naming: change expiry to expiration.
Use 'userID' instead of username in getUpdatePasswordDate function.
Check sameOldPassword in the rigth place.
src-db/database/model/tables/AD_USER.xml
src-db/database/sourcedata/AD_COLUMN.xml
src-db/database/sourcedata/AD_ELEMENT.xml
src-db/database/sourcedata/AD_FIELD.xml
src/org/openbravo/authentication/AuthenticationExpirationPasswordException.java
src/org/openbravo/authentication/AuthenticationExpiryPasswordException.java
src/org/openbravo/authentication/basic/DefaultAuthenticationManager.java
src/org/openbravo/base/secureApp/LoginHandler.java
--- a/src-db/database/model/tables/AD_USER.xml	Thu Jan 28 12:40:57 2016 +0100
+++ b/src-db/database/model/tables/AD_USER.xml	Tue Feb 02 14:11:27 2016 +0100
@@ -157,7 +157,7 @@
         <default><![CDATA[N]]></default>
         <onCreateDefault><![CDATA['N']]></onCreateDefault>
       </column>
-      <column name="LASTUPDATEPASSWORDDATE" primaryKey="false" required="true" type="TIMESTAMP" size="7" autoIncrement="false">
+      <column name="LASTPASSWORDUPDATE" primaryKey="false" required="true" type="TIMESTAMP" size="7" autoIncrement="false">
         <default><![CDATA[SYSDATE]]></default>
         <onCreateDefault/>
       </column>
--- a/src-db/database/sourcedata/AD_COLUMN.xml	Thu Jan 28 12:40:57 2016 +0100
+++ b/src-db/database/sourcedata/AD_COLUMN.xml	Tue Feb 02 14:11:27 2016 +0100
@@ -245884,10 +245884,10 @@
 <!--1FEFAB2F6215496FA9026D5FF7706F6F-->  <AD_CLIENT_ID><![CDATA[0]]></AD_CLIENT_ID>
 <!--1FEFAB2F6215496FA9026D5FF7706F6F-->  <AD_ORG_ID><![CDATA[0]]></AD_ORG_ID>
 <!--1FEFAB2F6215496FA9026D5FF7706F6F-->  <ISACTIVE><![CDATA[Y]]></ISACTIVE>
-<!--1FEFAB2F6215496FA9026D5FF7706F6F-->  <NAME><![CDATA[Lastupdatepassworddate]]></NAME>
+<!--1FEFAB2F6215496FA9026D5FF7706F6F-->  <NAME><![CDATA[LastPasswordUpdate]]></NAME>
 <!--1FEFAB2F6215496FA9026D5FF7706F6F-->  <DESCRIPTION><![CDATA[Latest date of user password change]]></DESCRIPTION>
 <!--1FEFAB2F6215496FA9026D5FF7706F6F-->  <HELP><![CDATA[Latest date of user password change]]></HELP>
-<!--1FEFAB2F6215496FA9026D5FF7706F6F-->  <COLUMNNAME><![CDATA[Lastupdatepassworddate]]></COLUMNNAME>
+<!--1FEFAB2F6215496FA9026D5FF7706F6F-->  <COLUMNNAME><![CDATA[LastPasswordUpdate]]></COLUMNNAME>
 <!--1FEFAB2F6215496FA9026D5FF7706F6F-->  <AD_TABLE_ID><![CDATA[114]]></AD_TABLE_ID>
 <!--1FEFAB2F6215496FA9026D5FF7706F6F-->  <AD_REFERENCE_ID><![CDATA[15]]></AD_REFERENCE_ID>
 <!--1FEFAB2F6215496FA9026D5FF7706F6F-->  <FIELDLENGTH><![CDATA[19]]></FIELDLENGTH>
--- a/src-db/database/sourcedata/AD_ELEMENT.xml	Thu Jan 28 12:40:57 2016 +0100
+++ b/src-db/database/sourcedata/AD_ELEMENT.xml	Tue Feb 02 14:11:27 2016 +0100
@@ -33051,9 +33051,9 @@
 <!--D570CE6C7BB4426BBEBDA54C7D1EFF3B-->  <AD_CLIENT_ID><![CDATA[0]]></AD_CLIENT_ID>
 <!--D570CE6C7BB4426BBEBDA54C7D1EFF3B-->  <AD_ORG_ID><![CDATA[0]]></AD_ORG_ID>
 <!--D570CE6C7BB4426BBEBDA54C7D1EFF3B-->  <ISACTIVE><![CDATA[Y]]></ISACTIVE>
-<!--D570CE6C7BB4426BBEBDA54C7D1EFF3B-->  <COLUMNNAME><![CDATA[Lastupdatepassworddate]]></COLUMNNAME>
-<!--D570CE6C7BB4426BBEBDA54C7D1EFF3B-->  <NAME><![CDATA[Last Update Password Date]]></NAME>
-<!--D570CE6C7BB4426BBEBDA54C7D1EFF3B-->  <PRINTNAME><![CDATA[Last Update Password Date]]></PRINTNAME>
+<!--D570CE6C7BB4426BBEBDA54C7D1EFF3B-->  <COLUMNNAME><![CDATA[LastPasswordUpdate]]></COLUMNNAME>
+<!--D570CE6C7BB4426BBEBDA54C7D1EFF3B-->  <NAME><![CDATA[Last Password Update]]></NAME>
+<!--D570CE6C7BB4426BBEBDA54C7D1EFF3B-->  <PRINTNAME><![CDATA[Last Password Update]]></PRINTNAME>
 <!--D570CE6C7BB4426BBEBDA54C7D1EFF3B-->  <DESCRIPTION><![CDATA[Latest date of user password change]]></DESCRIPTION>
 <!--D570CE6C7BB4426BBEBDA54C7D1EFF3B-->  <HELP><![CDATA[Latest date of user password change]]></HELP>
 <!--D570CE6C7BB4426BBEBDA54C7D1EFF3B-->  <AD_MODULE_ID><![CDATA[0]]></AD_MODULE_ID>
--- a/src-db/database/sourcedata/AD_FIELD.xml	Thu Jan 28 12:40:57 2016 +0100
+++ b/src-db/database/sourcedata/AD_FIELD.xml	Tue Feb 02 14:11:27 2016 +0100
@@ -204405,7 +204405,7 @@
 <!--41AD871370F04BA7AE780F013DEE70DF-->  <AD_CLIENT_ID><![CDATA[0]]></AD_CLIENT_ID>
 <!--41AD871370F04BA7AE780F013DEE70DF-->  <AD_ORG_ID><![CDATA[0]]></AD_ORG_ID>
 <!--41AD871370F04BA7AE780F013DEE70DF-->  <ISACTIVE><![CDATA[Y]]></ISACTIVE>
-<!--41AD871370F04BA7AE780F013DEE70DF-->  <NAME><![CDATA[Last Update Password Date]]></NAME>
+<!--41AD871370F04BA7AE780F013DEE70DF-->  <NAME><![CDATA[Last Password Update]]></NAME>
 <!--41AD871370F04BA7AE780F013DEE70DF-->  <DESCRIPTION><![CDATA[Latest date of user password change]]></DESCRIPTION>
 <!--41AD871370F04BA7AE780F013DEE70DF-->  <HELP><![CDATA[Latest date of user password change]]></HELP>
 <!--41AD871370F04BA7AE780F013DEE70DF-->  <ISCENTRALLYMAINTAINED><![CDATA[Y]]></ISCENTRALLYMAINTAINED>
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/src/org/openbravo/authentication/AuthenticationExpirationPasswordException.java	Tue Feb 02 14:11:27 2016 +0100
@@ -0,0 +1,44 @@
+/*
+ ************************************************************************************
+ * Copyright (C) 2015 Openbravo S.L.U.
+ * Licensed under the Apache Software License version 2.0
+ * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to  in writing,  software  distributed
+ * under the License is distributed  on  an  "AS IS"  BASIS,  WITHOUT  WARRANTIES  OR
+ * CONDITIONS OF ANY KIND, either  express  or  implied.  See  the  License  for  the
+ * specific language governing permissions and limitations under the License.
+ ************************************************************************************
+ */
+
+package org.openbravo.authentication;
+
+import org.openbravo.erpCommon.utility.OBError;
+
+/**
+ * This exception is used in case password for user has expired. Exception is launched in case that
+ * last update password date for user plus validity days defined for client has been reached
+ * 
+ */
+public class AuthenticationExpirationPasswordException extends AuthenticationException {
+  private static final long serialVersionUID = 1L;
+  private OBError error;
+
+  public AuthenticationExpirationPasswordException(String msg) {
+    super(msg);
+    this.error = null;
+  }
+
+  public AuthenticationExpirationPasswordException(String msg, Throwable cause) {
+    super(msg, cause);
+    this.error = null;
+  }
+
+  public AuthenticationExpirationPasswordException(String msg, OBError error) {
+    super(msg);
+    this.error = error;
+  }
+
+  public OBError getOBError() {
+    return error;
+  }
+}
\ No newline at end of file
--- a/src/org/openbravo/authentication/AuthenticationExpiryPasswordException.java	Thu Jan 28 12:40:57 2016 +0100
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,44 +0,0 @@
-/*
- ************************************************************************************
- * Copyright (C) 2015 Openbravo S.L.U.
- * Licensed under the Apache Software License version 2.0
- * You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
- * Unless required by applicable law or agreed to  in writing,  software  distributed
- * under the License is distributed  on  an  "AS IS"  BASIS,  WITHOUT  WARRANTIES  OR
- * CONDITIONS OF ANY KIND, either  express  or  implied.  See  the  License  for  the
- * specific language governing permissions and limitations under the License.
- ************************************************************************************
- */
-
-package org.openbravo.authentication;
-
-import org.openbravo.erpCommon.utility.OBError;
-
-/**
- * This exception is used in case password for user has expired. Exception is launched in case that
- * last update password date for user plus validity days defined for client has been reached
- * 
- */
-public class AuthenticationExpiryPasswordException extends AuthenticationException {
-  private static final long serialVersionUID = 1L;
-  private OBError error;
-
-  public AuthenticationExpiryPasswordException(String msg) {
-    super(msg);
-    this.error = null;
-  }
-
-  public AuthenticationExpiryPasswordException(String msg, Throwable cause) {
-    super(msg, cause);
-    this.error = null;
-  }
-
-  public AuthenticationExpiryPasswordException(String msg, OBError error) {
-    super(msg);
-    this.error = error;
-  }
-
-  public OBError getOBError() {
-    return error;
-  }
-}
\ No newline at end of file
--- a/src/org/openbravo/authentication/basic/DefaultAuthenticationManager.java	Thu Jan 28 12:40:57 2016 +0100
+++ b/src/org/openbravo/authentication/basic/DefaultAuthenticationManager.java	Tue Feb 02 14:11:27 2016 +0100
@@ -25,7 +25,7 @@
 import org.apache.log4j.Logger;
 import org.hibernate.criterion.Restrictions;
 import org.openbravo.authentication.AuthenticationException;
-import org.openbravo.authentication.AuthenticationExpiryPasswordException;
+import org.openbravo.authentication.AuthenticationExpirationPasswordException;
 import org.openbravo.authentication.AuthenticationManager;
 import org.openbravo.base.HttpBaseUtils;
 import org.openbravo.base.secureApp.LoginUtils;
@@ -118,20 +118,17 @@
     vars.setSessionValue("#AD_SESSION_ID", sessionId);
     vars.setSessionValue("#LogginIn", "Y");
 
-    Date lastUpdatePasswordDate = getUpdatePasswordDate(user);
+    Date lastUpdatePassword = getUpdatePasswordDate(userId);
+    Date today = new Date();
 
-    if (lastUpdatePasswordDate != null) {
+    if ((lastUpdatePassword != null) && (lastUpdatePassword.compareTo(today) <= 0)) {
+      OBError errorMsg = new OBError();
+      errorMsg.setType("Error");
+      errorMsg.setTitle("IDENTIFICATION_FAILURE_TITLE");
+      errorMsg.setMessage("IDENTIFICATION_FAILURE_MSG");
+      throw new AuthenticationExpirationPasswordException("IDENTIFICATION_FAILURE_TITLE", errorMsg);
+    }
 
-      Date today = new Date();
-      if (lastUpdatePasswordDate.compareTo(today) <= 0) {
-        OBError errorMsg = new OBError();
-        errorMsg.setType("Error");
-        errorMsg.setTitle("IDENTIFICATION_FAILURE_TITLE");
-        errorMsg.setMessage("IDENTIFICATION_FAILURE_MSG");
-        throw new AuthenticationExpiryPasswordException("IDENTIFICATION_FAILURE_TITLE", errorMsg);
-
-      }
-    }
     if (!StringUtils.isEmpty(strAjax) && StringUtils.isEmpty(userId)) {
       bdErrorAjax(response, "Error", "",
           Utility.messageBD(this.conn, "NotLogged", variables.getLanguage()));
@@ -172,26 +169,26 @@
   }
 
   /**
-   * Returns the expiry password date from login and unHashedPassword parameters
+   * Returns the expiration password date from login and unHashedPassword parameters
    * 
    * @param username
    *          the username
-   * @return the expiry password date or null in case validity days are not applicable
+   * @return the expiration password date or null in case validity days are not applicable
    */
-  private static Date getUpdatePasswordDate(String username) {
+  private static Date getUpdatePasswordDate(String id) {
 
     Date total;
 
     final OBCriteria<User> obc = OBDal.getInstance().createCriteria(User.class);
     obc.setFilterOnReadableClients(false);
     obc.setFilterOnReadableOrganization(false);
-    obc.add(Restrictions.eq(User.PROPERTY_USERNAME, username));
+    obc.add(Restrictions.eq(User.PROPERTY_ID, id));
     final User userOB = (User) obc.uniqueResult();
-    Date lastUpdateDate = userOB.getLastupdatepassworddate();
+    Date lastUpdatePassword = userOB.getLastPasswordUpdate();
     Long validityDays = userOB.getClient().getDaystopasswordexpiration();
     if (validityDays != null && validityDays > 0) {
       Calendar expirationDate = Calendar.getInstance();
-      expirationDate.setTimeInMillis(lastUpdateDate.getTime());
+      expirationDate.setTimeInMillis(lastUpdatePassword.getTime());
       expirationDate.add(Calendar.DATE, validityDays.intValue());
       total = expirationDate.getTime();
       return total;
--- a/src/org/openbravo/base/secureApp/LoginHandler.java	Thu Jan 28 12:40:57 2016 +0100
+++ b/src/org/openbravo/base/secureApp/LoginHandler.java	Tue Feb 02 14:11:27 2016 +0100
@@ -24,7 +24,7 @@
 import org.codehaus.jettison.json.JSONObject;
 import org.hibernate.criterion.Restrictions;
 import org.openbravo.authentication.AuthenticationException;
-import org.openbravo.authentication.AuthenticationExpiryPasswordException;
+import org.openbravo.authentication.AuthenticationExpirationPasswordException;
 import org.openbravo.authentication.AuthenticationManager;
 import org.openbravo.base.HttpBaseServlet;
 import org.openbravo.base.exception.OBException;
@@ -78,11 +78,24 @@
     Boolean sameOldPassword = false;
     final String user;
     final String password;
+    Client systemClient = OBDal.getInstance().get(Client.class, "0");
+    String language = systemClient.getLanguage().getLanguage();
     if (resetPassword) {
       password = vars.getStringParameter("password");
       user = vars.getStringParameter("loggedUser");
       if (!vars.getSessionValue("#AD_Session_ID").equalsIgnoreCase("")) {
         sameOldPassword = updatePassword(user, password);
+        if (sameOldPassword) {
+          OBError errorMsg = new OBError();
+          String msg = Utility.messageBD(myPool, "CPUpdatePassword", language);
+          String title = Utility.messageBD(myPool, "CPDifferentPassword", language);
+          errorMsg.setType("Error");
+          errorMsg.setTitle(title);
+          errorMsg.setMessage(msg);
+
+          throw new AuthenticationExpirationPasswordException(Utility.messageBD(myPool,
+              "CPSamePasswordThanOld", language), errorMsg);
+        }
       }
     } else {
       user = vars.getStringParameter("user");
@@ -96,25 +109,10 @@
 
     OBContext.setAdminMode();
     try {
-      Client systemClient = OBDal.getInstance().get(Client.class, "0");
-
-      String language = systemClient.getLanguage().getLanguage();
-
       if (user.equals("") && !OBVersion.getInstance().is30()) {
         res.sendRedirect(res.encodeRedirectURL(strDireccion + "/security/Login_F1.html"));
       } else {
         try {
-          if (sameOldPassword) {
-            OBError errorMsg = new OBError();
-            String msg = Utility.messageBD(myPool, "CPUpdatePassword", language);
-            String title = Utility.messageBD(myPool, "CPDifferentPassword", language);
-            errorMsg.setType("Error");
-            errorMsg.setTitle(title);
-            errorMsg.setMessage(msg);
-
-            throw new AuthenticationExpiryPasswordException(Utility.messageBD(myPool,
-                "CPSamePasswordThanOld", language), errorMsg);
-          }
           AuthenticationManager authManager = AuthenticationManager.getAuthenticationManager(this);
 
           final String strUserAuth = authManager.authenticate(req, res);
@@ -125,7 +123,7 @@
           }
           checkLicenseAndGo(res, vars, strUserAuth, user, sessionId, doRedirect);
 
-        } catch (AuthenticationExpiryPasswordException aepe) {
+        } catch (AuthenticationExpirationPasswordException aepe) {
 
           final OBError errorMsg = aepe.getOBError();
           if (errorMsg != null) {
@@ -138,7 +136,7 @@
                   doRedirect);
             } else {
               String msg = Utility.messageBD(myPool, "CPUpdatePassword", language);
-              String title = Utility.messageBD(myPool, "CPExpiryPassword", language);
+              String title = Utility.messageBD(myPool, "CPExpirationPassword", language);
               goToUpdatePassword(res, vars, msg, title, "Error", "../security/Login_FS.html",
                   doRedirect);
             }