[ChangePassword] Applied code review.
authorJonathan Bueno <jonathan.bueno@openbravo.com>
Fri, 19 Feb 2016 12:56:49 +0100
changeset 28679 060c7af3f4dd
parent 28678 efc711fa6288
child 28680 e0aaaa3f6b1b
[ChangePassword] Applied code review.

Renamed trigger to be consistent, used expiration instead of expiry.
Used === to compare strings in Login.html.
Renamed getUpdatePasswordDate method to checkIfPasswordExpired in DefaultAuthenticationManager class.
Added throws in signature and javadoc for checkIfPasswordExpired and updatePassword methods.
Removed static parameter from checkIfPasswordExpired and updatePassword methods.
Removed extra brackets in expression in checkIfPasswordExpired method.
Exception AuthenticationExpirationPasswordException fixed. Removed extra variables, error is now loaded in the subclass AuthenticationException.
Removed empty line in checkIfPasswordExpired.
Removed check to support 2.50.
LoginUtils file keep unmodified.
src/org/openbravo/authentication/AuthenticationException.java
src/org/openbravo/authentication/AuthenticationExpirationPasswordException.java
src/org/openbravo/authentication/basic/DefaultAuthenticationManager.java
src/org/openbravo/base/secureApp/LoginHandler.java
src/org/openbravo/base/secureApp/LoginUtils.java
src/org/openbravo/erpCommon/security/Login.html
--- a/src/org/openbravo/authentication/AuthenticationException.java	Wed Feb 17 09:56:42 2016 +0100
+++ b/src/org/openbravo/authentication/AuthenticationException.java	Fri Feb 19 12:56:49 2016 +0100
@@ -44,6 +44,11 @@
     this.error = error;
   }
 
+  public AuthenticationException(String msg, OBError error, Boolean isLogExceptionNeeded) {
+    super(msg, isLogExceptionNeeded);
+    this.error = error;
+  }
+
   public OBError getOBError() {
     return error;
   }
--- a/src/org/openbravo/authentication/AuthenticationExpirationPasswordException.java	Wed Feb 17 09:56:42 2016 +0100
+++ b/src/org/openbravo/authentication/AuthenticationExpirationPasswordException.java	Fri Feb 19 12:56:49 2016 +0100
@@ -21,33 +21,23 @@
  */
 public class AuthenticationExpirationPasswordException extends AuthenticationException {
   private static final long serialVersionUID = 1L;
-  private OBError error;
-  private boolean passwordExpiration;
 
   public AuthenticationExpirationPasswordException(String msg) {
     super(msg);
-    this.error = null;
+  }
+
+  public AuthenticationExpirationPasswordException(String msg, OBError error) {
+    super(msg, error);
   }
 
   public AuthenticationExpirationPasswordException(String msg, Throwable cause) {
     super(msg, cause);
-    this.error = null;
   }
 
   public AuthenticationExpirationPasswordException(String msg, OBError error,
       boolean passwordExpiration) {
-    super(msg, false);
-    this.error = error;
-    this.passwordExpiration = passwordExpiration;
+    super(msg, error, false);
     this.getLogger().error(error.getTitle());
 
   }
-
-  public OBError getOBError() {
-    return error;
-  }
-
-  public boolean isPasswordExpiration() {
-    return passwordExpiration;
-  }
 }
\ No newline at end of file
--- a/src/org/openbravo/authentication/basic/DefaultAuthenticationManager.java	Wed Feb 17 09:56:42 2016 +0100
+++ b/src/org/openbravo/authentication/basic/DefaultAuthenticationManager.java	Fri Feb 19 12:56:49 2016 +0100
@@ -33,7 +33,6 @@
 import org.openbravo.base.secureApp.VariablesSecureApp;
 import org.openbravo.dal.service.OBCriteria;
 import org.openbravo.dal.service.OBDal;
-import org.openbravo.database.ConnectionProvider;
 import org.openbravo.erpCommon.utility.OBError;
 import org.openbravo.erpCommon.utility.Utility;
 import org.openbravo.model.ad.access.User;
@@ -114,7 +113,7 @@
 
     vars.setSessionValue("#AD_User_ID", userId);
 
-    getUpdatePasswordDate(userId, variables.getLanguage(), this.conn);
+    checkIfPasswordExpired(userId, variables.getLanguage());
 
     // Using the Servlet API instead of vars.setSessionValue to avoid breaking code
     // vars.setSessionValue always transform the key to upper-case
@@ -170,10 +169,13 @@
    *          The userId of the user to check expiration password date
    * @param language
    *          Default language for the user
-   * @param conn
-   *          ConnectionProvider used to translate messages.
+   * @throws AuthenticationExpirationPasswordException
+   *           AuthenticationExpirationPasswordException is thrown in case that expiration date is
+   *           reached
+   * 
    */
-  private static void getUpdatePasswordDate(String userId, String language, ConnectionProvider conn) {
+  private void checkIfPasswordExpired(String userId, String language)
+      throws AuthenticationExpirationPasswordException {
 
     Date total = null;
 
@@ -192,7 +194,7 @@
     }
 
     Date today = new Date();
-    if ((total != null) && (total.compareTo(today) <= 0)) {
+    if (total != null && total.compareTo(today) <= 0) {
       OBError errorMsg = new OBError();
       errorMsg.setType("Error");
       errorMsg.setTitle(Utility.messageBD(conn, "CPExpirationPassword", language));
--- a/src/org/openbravo/base/secureApp/LoginHandler.java	Wed Feb 17 09:56:42 2016 +0100
+++ b/src/org/openbravo/base/secureApp/LoginHandler.java	Fri Feb 19 12:56:49 2016 +0100
@@ -30,7 +30,6 @@
 import org.openbravo.dal.core.OBContext;
 import org.openbravo.dal.service.OBCriteria;
 import org.openbravo.dal.service.OBDal;
-import org.openbravo.database.ConnectionProvider;
 import org.openbravo.erpCommon.obps.ActivationKey;
 import org.openbravo.erpCommon.obps.ActivationKey.LicenseRestriction;
 import org.openbravo.erpCommon.security.Login;
@@ -103,7 +102,7 @@
         try {
           if (isPasswordResetFlow && StringUtils.isNotBlank(vars.getSessionValue("#AD_User_ID"))) {
             password = vars.getStringParameter("password");
-            updatePassword(user, password, myPool, language);
+            updatePassword(user, password, language);
           }
 
           AuthenticationManager authManager = AuthenticationManager.getAuthenticationManager(this);
@@ -476,31 +475,26 @@
       throws IOException, ServletException {
     String msg = (message != null && !message.equals("")) ? message : Utility.messageBD(myPool,
         "CPEmptyUserPassword", vars.getLanguage());
-    ;
 
-    if (OBVersion.getInstance().is30() && !doRedirect) {
-      // 3.0 instances show the message in the same login window, return a json object with the info
-      // to print the message
-      try {
-        JSONObject jsonMsg = new JSONObject();
-        jsonMsg.put("showMessage", true);
-        jsonMsg.put("target", action);
-        jsonMsg.put("messageType", msgType);
-        jsonMsg.put("messageTitle", title);
-        jsonMsg.put("messageText", msg);
-        jsonMsg.put("resetPassword", true);
-        jsonMsg.put("loggedUser", vars.getStringParameter("user"));
-        if ("Confirmation".equals(msgType)) {
-          jsonMsg.put("command", "FORCE_NAMED_USER");
-        }
-        response.setContentType("application/json;charset=UTF-8");
-        final PrintWriter out = response.getWriter();
-        out.print(jsonMsg.toString());
-        out.close();
-      } catch (JSONException e) {
-        log4j.error("Error setting login msg", e);
-        throw new ServletException(e);
+    try {
+      JSONObject jsonMsg = new JSONObject();
+      jsonMsg.put("showMessage", true);
+      jsonMsg.put("target", action);
+      jsonMsg.put("messageType", msgType);
+      jsonMsg.put("messageTitle", title);
+      jsonMsg.put("messageText", msg);
+      jsonMsg.put("resetPassword", true);
+      jsonMsg.put("loggedUser", vars.getStringParameter("user"));
+      if ("Confirmation".equals(msgType)) {
+        jsonMsg.put("command", "FORCE_NAMED_USER");
       }
+      response.setContentType("application/json;charset=UTF-8");
+      final PrintWriter out = response.getWriter();
+      out.print(jsonMsg.toString());
+      out.close();
+    } catch (JSONException e) {
+      log4j.error("Error setting login msg", e);
+      throw new ServletException(e);
     }
   }
 
@@ -510,24 +504,25 @@
   } // end of getServletInfo() method
 
   /**
-   * Update user password for username with unHashedPassword provided, throws
+   * Update user password for userId with unHashedPassword provided, throws
    * AuthenticationExpirationPasswordException in case the new password is the same that the old
    * one.
    * 
    * 
-   * @param username
-   *          the username
+   * @param userId
+   *          the userId
    * @param unHashedPassword
    *          the password, the unhashed password as it is entered by the user.
    * @param myPool
    *          ConnectionProvider used to translate messages.
    * @param language
    *          Default language for the user
+   * @throws ServletException
+   *           ServletException is thrown in case that password could not be hashed
    * 
-   * @throws ServletException
    */
-  private static void updatePassword(String userId, String unHashedPassword,
-      ConnectionProvider myPool, String language) throws ServletException {
+  private void updatePassword(String userId, String unHashedPassword, String language)
+      throws ServletException {
     try {
       OBContext.setAdminMode();
       final OBCriteria<User> obc = OBDal.getInstance().createCriteria(User.class);
--- a/src/org/openbravo/base/secureApp/LoginUtils.java	Wed Feb 17 09:56:42 2016 +0100
+++ b/src/org/openbravo/base/secureApp/LoginUtils.java	Fri Feb 19 12:56:49 2016 +0100
@@ -1,6 +1,6 @@
 /*
  ************************************************************************************
- * Copyright (C) 2001-2015 Openbravo S.L.U.
+ * Copyright (C) 2001-2014 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
--- a/src/org/openbravo/erpCommon/security/Login.html	Wed Feb 17 09:56:42 2016 +0100
+++ b/src/org/openbravo/erpCommon/security/Login.html	Fri Feb 19 12:56:49 2016 +0100
@@ -207,7 +207,7 @@
         return true;
       }   
       disableButton('buttonOK');
-      if (document.getElementById('resetPassword').value =='true'){
+      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);