[ChangePassword] Code review applied.
authorJonathan Bueno <jonathan.bueno@openbravo.com>
Mon, 15 Feb 2016 12:11:46 +0100
changeset 28674 5970284b4e82
parent 28673 d5385407e873
child 28675 1cf700dfd92e
[ChangePassword] Code review applied.

Updated javadocs of methods.
Added check to avoid security problem when updating password.
src/org/openbravo/authentication/basic/DefaultAuthenticationManager.java
src/org/openbravo/base/secureApp/LoginHandler.java
src/org/openbravo/erpCommon/security/Login.html
--- a/src/org/openbravo/authentication/basic/DefaultAuthenticationManager.java	Fri Feb 12 13:44:48 2016 +0100
+++ b/src/org/openbravo/authentication/basic/DefaultAuthenticationManager.java	Mon Feb 15 12:11:46 2016 +0100
@@ -161,24 +161,24 @@
   }
 
   /**
-   * Returns the expiration password date from login and unHashedPassword parameters
+   * Checks the expiration password date from userId, throws
+   * AuthenticationExpirationPasswordException in case that expiration date is reached
    * 
+   * @param userId
+   *          The userId of the user to check expiration password date
+   * @param language
+   *          Default language for the user
    * @param conn
-   * 
-   * @param string
-   * 
-   * @param username
-   *          the username
-   * @return the expiration password date or null in case validity days are not applicable
+   *          ConnectionProvider used to translate messages.
    */
-  private static void getUpdatePasswordDate(String id, String language, ConnectionProvider conn) {
+  private static void getUpdatePasswordDate(String userId, String language, ConnectionProvider conn) {
 
     Date total = null;
 
     final OBCriteria<User> obc = OBDal.getInstance().createCriteria(User.class);
     obc.setFilterOnReadableClients(false);
     obc.setFilterOnReadableOrganization(false);
-    obc.add(Restrictions.eq(User.PROPERTY_ID, id));
+    obc.add(Restrictions.eq(User.PROPERTY_ID, userId));
     final User userOB = (User) obc.uniqueResult();
     Date lastUpdatePassword = userOB.getLastPasswordUpdate();
     Long validityDays = userOB.getClient().getDaysToPasswordExpiration();
--- a/src/org/openbravo/base/secureApp/LoginHandler.java	Fri Feb 12 13:44:48 2016 +0100
+++ b/src/org/openbravo/base/secureApp/LoginHandler.java	Mon Feb 15 12:11:46 2016 +0100
@@ -69,7 +69,6 @@
     log4j.debug("start doPost");
     doOptions(req, res);
     final VariablesSecureApp vars = new VariablesSecureApp(req);
-
     // Empty session
     req.getSession().removeAttribute("#Authenticated_user");
     vars.removeSessionValue("#AD_Role_ID");
@@ -98,8 +97,10 @@
       } else {
         try {
           if (isPasswordResetFlow) {
-            password = vars.getStringParameter("password");
-            updatePassword(user, password, myPool, language);
+            if (vars.getCommand().equalsIgnoreCase("FORCE_RESET_PASSWORD")) {
+              password = vars.getStringParameter("password");
+              updatePassword(user, password, myPool, language);
+            }
           }
 
           AuthenticationManager authManager = AuthenticationManager.getAuthenticationManager(this);
@@ -506,9 +507,9 @@
   } // end of getServletInfo() method
 
   /**
-   * Returns a boolean indicating if password has been changed for user. The password must be
-   * different from previous one, otherwise method will return false
-   * 
+   * Update user password for username with unHashedPassword provided, throws
+   * AuthenticationExpirationPasswordException in case the new password is the same that the old
+   * one.
    * 
    * 
    * @param username
@@ -516,9 +517,10 @@
    * @param unHashedPassword
    *          the password, the unhashed password as it is entered by the user.
    * @param myPool
+   *          ConnectionProvider used to translate messages.
    * @param language
-   * @return false in case that password has changed succesfully in database, true in case password
-   *         were the same than the old one
+   *          Default language for the user
+   * 
    * @throws ServletException
    */
   private static void updatePassword(String username, String unHashedPassword,
--- a/src/org/openbravo/erpCommon/security/Login.html	Fri Feb 12 13:44:48 2016 +0100
+++ b/src/org/openbravo/erpCommon/security/Login.html	Mon Feb 15 12:11:46 2016 +0100
@@ -203,11 +203,15 @@
       setWindowElementFocus(document.getElementById('user'))
     } else {
       if (document.getElementById('user').value === '' || document.getElementById('password').value === '') {
-          setLoginMessage('Error', identificationFailureTitle, errorEmptyContent);
+        setLoginMessage('Error', identificationFailureTitle, errorEmptyContent);
         return true;
       }   
       disableButton('buttonOK');
-      submitXmlHttpRequest(loginResult, document.frmIdentificacion, 'DEFAULT', '../secureApp/LoginHandler.html', false, null, null);
+      if (document.getElementById('resetPassword').value =='true'){
+        submitXmlHttpRequest(loginResult, document.frmIdentificacion, 'FORCE_RESET_PASSWORD', '../secureApp/LoginHandler.html', false, null, null);
+      }else{
+        submitXmlHttpRequest(loginResult, document.frmIdentificacion, 'DEFAULT', '../secureApp/LoginHandler.html', false, null, null);
+      }
     }
     
     return false;