[ChangePassword] Applied code review.Fixed security problem.
authorJonathan Bueno <jonathan.bueno@openbravo.com>
Wed, 17 Feb 2016 09:52:43 +0100
changeset 28677 9dda34ecf279
parent 28676 0d920474c5da
child 28678 efc711fa6288
[ChangePassword] Applied code review.Fixed security problem.
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	Mon Feb 15 13:16:35 2016 +0100
+++ b/src/org/openbravo/authentication/basic/DefaultAuthenticationManager.java	Wed Feb 17 09:52:43 2016 +0100
@@ -62,13 +62,12 @@
     final Boolean resetPassword = Boolean.parseBoolean(vars.getStringParameter("resetPassword"));
     final String sUserId;
     if (resetPassword) {
-      final String userId = LoginUtils.getValidUserId(conn, vars.getStringParameter("loggedUser"),
-          vars.getStringParameter("user"));
-      sUserId = userId;
+      sUserId = vars.getSessionValue("#AD_User_ID");
+
     } else {
       sUserId = (String) request.getSession().getAttribute("#Authenticated_user");
+    }
 
-    }
     final String strAjax = vars.getStringParameter("IsAjaxCall");
     if (!StringUtils.isEmpty(sUserId) && !resetPassword) {
       return sUserId;
@@ -79,7 +78,8 @@
     final String pass;
     // Begins code related to login process
     if (resetPassword) {
-      user = vars.getStringParameter("loggedUser");
+      User userOB = OBDal.getInstance().get(User.class, sUserId);
+      user = userOB.getUsername();
     } else {
       user = vars.getStringParameter("user");
     }
@@ -112,6 +112,8 @@
       throw new AuthenticationException("IDENTIFICATION_FAILURE_TITLE", errorMsg);
     }
 
+    vars.setSessionValue("#AD_User_ID", userId);
+
     getUpdatePasswordDate(userId, variables.getLanguage(), this.conn);
 
     // Using the Servlet API instead of vars.setSessionValue to avoid breaking code
--- a/src/org/openbravo/base/secureApp/LoginHandler.java	Mon Feb 15 13:16:35 2016 +0100
+++ b/src/org/openbravo/base/secureApp/LoginHandler.java	Wed Feb 17 09:52:43 2016 +0100
@@ -69,17 +69,18 @@
     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");
     vars.setSessionObject("#loggingIn", "Y");
+
     final String user;
     final String password;
-    Client systemClient = OBDal.getInstance().get(Client.class, "0");
-    String language = systemClient.getLanguage().getLanguage();
+
     boolean isPasswordResetFlow = Boolean.parseBoolean(vars.getStringParameter("resetPassword"));
     if (isPasswordResetFlow) {
-      user = vars.getStringParameter("loggedUser");
+      user = vars.getSessionValue("#AD_User_ID");
     } else {
       user = vars.getStringParameter("user");
     }
@@ -92,15 +93,17 @@
 
     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 (isPasswordResetFlow) {
-            if (vars.getCommand().equalsIgnoreCase("FORCE_RESET_PASSWORD")) {
-              password = vars.getStringParameter("password");
-              updatePassword(user, password, myPool, language);
-            }
+          if (isPasswordResetFlow && StringUtils.isNotBlank(vars.getSessionValue("#AD_User_ID"))) {
+            password = vars.getStringParameter("password");
+            updatePassword(user, password, myPool, language);
           }
 
           AuthenticationManager authManager = AuthenticationManager.getAuthenticationManager(this);
@@ -523,13 +526,14 @@
    * 
    * @throws ServletException
    */
-  private static void updatePassword(String username, String unHashedPassword,
+  private static void updatePassword(String userId, String unHashedPassword,
       ConnectionProvider myPool, String language) throws ServletException {
     try {
       OBContext.setAdminMode();
       final OBCriteria<User> obc = OBDal.getInstance().createCriteria(User.class);
-      obc.add(Restrictions.eq(User.PROPERTY_USERNAME, username));
+      obc.add(Restrictions.eq(User.PROPERTY_ID, userId));
       obc.setFilterOnReadableClients(false);
+      obc.setFilterOnReadableOrganization(false);
       final User userOB = (User) obc.uniqueResult();
       String oldPassword = userOB.getPassword();
       String newPassword = FormatUtilities.sha1Base64(unHashedPassword);
--- a/src/org/openbravo/erpCommon/security/Login.html	Mon Feb 15 13:16:35 2016 +0100
+++ b/src/org/openbravo/erpCommon/security/Login.html	Wed Feb 17 09:52:43 2016 +0100
@@ -237,8 +237,7 @@
         document.getElementById('password').value = '';
       }
     }
-    if (result.resetPassword && document.getElementById('loggedUser').value===''){
-      document.getElementById('loggedUser').value=result.loggedUser;
+    if (result.resetPassword ){
       document.getElementById('resetPassword').value=result.resetPassword;
       document.getElementById('user').value = '';
       document.getElementById('user').type = 'password';
@@ -466,7 +465,6 @@
       <div class="Login_LogForm">
         <form method="post" action="../secureApp/LoginHandler.html" name="frmIdentificacion" id="frmFormulario" autocomplete="off">
           <input type="hidden" name="Command" value="" />
-          <input type="hidden" name="loggedUser" id="loggedUser" value="" />
           <input type="hidden" name="resetPassword" id="resetPassword" value="" />
           <div class="Login_LogForm_CompanyLogo_Container">
             <div class="Login_LogForm_CompanyLogo" id="CompanyLogo_Container" style="display: none;"><img class="Login_Logo_Company" src="../../../../../web/images/blank.gif" /></div>