[ofbiz-plugins] branch trunk updated: Improved: Enforce noninstantiability to LdapLoginWorker, OFBizSolrLoginWorker class. (#23)

[surajk]

This is an automated email from the ASF dual-hosted git repository.

surajk pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-plugins.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 9c447f2  Improved: Enforce noninstantiability to LdapLoginWorker, OFBizSolrLoginWorker class. (#23)
9c447f2 is described below

commit 9c447f248699e5942bc3b4c2ce4cb94d088fdf95
Author: Suraj Khurana <[hidden email]>
AuthorDate: Sun Jun 7 10:55:56 2020 +0530

    Improved: Enforce noninstantiability to LdapLoginWorker, OFBizSolrLoginWorker class. (#23)
   
    (OFBIZ-11747)
   
    Made class as final, added private constructor and made static variables as private.
---
 .../org/apache/ofbiz/ldap/LdapLoginWorker.java     | 36 +++++++++++----------
 .../AbstractOFBizAuthenticationHandler.java        |  6 ++--
 .../ofbiz/solr/control/OFBizSolrLoginWorker.java   | 37 ++++++++++------------
 3 files changed, 39 insertions(+), 40 deletions(-)

diff --git a/ldap/src/main/java/org/apache/ofbiz/ldap/LdapLoginWorker.java b/ldap/src/main/java/org/apache/ofbiz/ldap/LdapLoginWorker.java
index 7c3dce4..6e1bdb4 100644
--- a/ldap/src/main/java/org/apache/ofbiz/ldap/LdapLoginWorker.java
+++ b/ldap/src/main/java/org/apache/ofbiz/ldap/LdapLoginWorker.java
@@ -44,9 +44,13 @@ import org.xml.sax.SAXException;
 /**
  * Common LDAP Login Workers
  */
-public class LdapLoginWorker extends LoginWorker {
+public final class LdapLoginWorker {
 
-    private static final String ldapConfig = "plugins/ldap/config/ldap.xml";
+    private final static String MODULE = LdapLoginWorker.class.getName();
+    private static final String RESOURCE = "SecurityextUiLabels";
+    private static final String LDAP_CONFIG = "plugins/ldap/config/ldap.xml";
+
+    protected LdapLoginWorker() { }
 
     /**
      * An HTTP WebEvent handler that checks to see is a userLogin is logged in.
@@ -77,14 +81,14 @@ public class LdapLoginWorker extends LoginWorker {
                 } catch (Exception e) {
                     Debug.logError(e, "Error calling checkLogin service", MODULE);
                     Map<String, String> messageMap = UtilMisc.toMap("errorMessage", e.getMessage());
-                    String errMsg = UtilProperties.getMessage(resourceWebapp, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
+                    String errMsg = UtilProperties.getMessage(RESOURCE, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
                     request.setAttribute("_ERROR_MESSAGE_", errMsg);
                 }
             }
 
-            if (!hasBasePermission(userLogin, request) || isFlaggedLoggedOut(userLogin, userLogin.getDelegator()) || hasLdapLoggedOut) {
+            if (!LoginWorker.hasBasePermission(userLogin, request) || LoginWorker.isFlaggedLoggedOut(userLogin, userLogin.getDelegator()) || hasLdapLoggedOut) {
                 Debug.logInfo("User does not have permission or is flagged as logged out", MODULE);
-                doBasicLogout(userLogin, request, response);
+                LoginWorker.doBasicLogout(userLogin, request, response);
                 userLogin = null;
             }
         }
@@ -117,7 +121,7 @@ public class LdapLoginWorker extends LoginWorker {
             } catch (Exception e) {
                 Debug.logError(e, "Error calling userLogin service", MODULE);
                 Map<String, String> messageMap = UtilMisc.toMap("errorMessage", e.getMessage());
-                String errMsg = UtilProperties.getMessage(resourceWebapp, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
+                String errMsg = UtilProperties.getMessage(RESOURCE, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
                 request.setAttribute("_ERROR_MESSAGE_", errMsg);
             }
         }
@@ -147,7 +151,7 @@ public class LdapLoginWorker extends LoginWorker {
         // invalidate the security group list cache
         GenericValue userLogin = (GenericValue) request.getSession().getAttribute("userLogin");
 
-        doBasicLogout(userLogin, request, response);
+        LoginWorker.doBasicLogout(userLogin, request, response);
 
         Element rootElement = getRootElement(request);
 
@@ -161,47 +165,47 @@ public class LdapLoginWorker extends LoginWorker {
             } catch (Exception e) {
                 Debug.logError(e, "Error calling userLogin service", MODULE);
                 Map<String, String> messageMap = UtilMisc.toMap("errorMessage", e.getMessage());
-                String errMsg = UtilProperties.getMessage(resourceWebapp, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
+                String errMsg = UtilProperties.getMessage(RESOURCE, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
                 request.setAttribute("_ERROR_MESSAGE_", errMsg);
             }
         }
 
         if (request.getAttribute("_AUTO_LOGIN_LOGOUT_") == null) {
-            return autoLoginCheck(request, response);
+            return LoginWorker.autoLoginCheck(request, response);
         }
         return result;
     }
 
     protected static Element getRootElement(HttpServletRequest request) {
         if (Debug.infoOn()) {
-            Debug.logInfo("LDAP config file: " + ldapConfig, MODULE);
+            Debug.logInfo("LDAP config file: " + LDAP_CONFIG, MODULE);
         }
-        File configFile = new File(ldapConfig);
+        File configFile = new File(LDAP_CONFIG);
         FileInputStream configFileIS = null;
         Element rootElement = null;
         try {
             configFileIS = new FileInputStream(configFile);
-            Document configDoc = UtilXml.readXmlDocument(configFileIS, "LDAP configuration file " + ldapConfig);
+            Document configDoc = UtilXml.readXmlDocument(configFileIS, "LDAP configuration file " + LDAP_CONFIG);
             rootElement = configDoc.getDocumentElement();
         } catch (FileNotFoundException e) {
             Debug.logError(e, "Error calling userLogin service", MODULE);
             Map<String, String> messageMap = UtilMisc.toMap("errorMessage", e.getMessage());
-            String errMsg = UtilProperties.getMessage(resourceWebapp, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
+            String errMsg = UtilProperties.getMessage(RESOURCE, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
             request.setAttribute("_ERROR_MESSAGE_", errMsg);
         } catch (SAXException e) {
             Debug.logError(e, "Error calling userLogin service", MODULE);
             Map<String, String> messageMap = UtilMisc.toMap("errorMessage", e.getMessage());
-            String errMsg = UtilProperties.getMessage(resourceWebapp, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
+            String errMsg = UtilProperties.getMessage(RESOURCE, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
             request.setAttribute("_ERROR_MESSAGE_", errMsg);
         } catch (ParserConfigurationException e) {
             Debug.logError(e, "Error calling userLogin service", MODULE);
             Map<String, String> messageMap = UtilMisc.toMap("errorMessage", e.getMessage());
-            String errMsg = UtilProperties.getMessage(resourceWebapp, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
+            String errMsg = UtilProperties.getMessage(RESOURCE, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
             request.setAttribute("_ERROR_MESSAGE_", errMsg);
         } catch (IOException e) {
             Debug.logError(e, "Error calling userLogin service", MODULE);
             Map<String, String> messageMap = UtilMisc.toMap("errorMessage", e.getMessage());
-            String errMsg = UtilProperties.getMessage(resourceWebapp, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
+            String errMsg = UtilProperties.getMessage(RESOURCE, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
             request.setAttribute("_ERROR_MESSAGE_", errMsg);
         } finally {
             if (configFileIS != null) {
diff --git a/ldap/src/main/java/org/apache/ofbiz/ldap/commons/AbstractOFBizAuthenticationHandler.java b/ldap/src/main/java/org/apache/ofbiz/ldap/commons/AbstractOFBizAuthenticationHandler.java
index c207f87..cf287ea 100644
--- a/ldap/src/main/java/org/apache/ofbiz/ldap/commons/AbstractOFBizAuthenticationHandler.java
+++ b/ldap/src/main/java/org/apache/ofbiz/ldap/commons/AbstractOFBizAuthenticationHandler.java
@@ -41,7 +41,7 @@ import org.apache.ofbiz.entity.GenericEntityException;
 import org.apache.ofbiz.entity.GenericValue;
 import org.apache.ofbiz.entity.util.EntityQuery;
 import org.apache.ofbiz.entity.util.EntityUtilProperties;
-import org.apache.ofbiz.ldap.LdapLoginWorker;
+import org.apache.ofbiz.webapp.control.LoginWorker;
 import org.apache.ofbiz.service.GenericServiceException;
 import org.apache.ofbiz.service.LocalDispatcher;
 import org.apache.ofbiz.service.ModelService;
@@ -145,10 +145,10 @@ public abstract class AbstractOFBizAuthenticationHandler implements InterfaceOFB
         if (ModelService.RESPOND_SUCCESS.equals(loginResult.get(ModelService.RESPONSE_MESSAGE))) {
             GenericValue userLogin = (GenericValue) loginResult.get("userLogin");
             Map<String, Object> userLoginSession = checkMap(loginResult.get("userLoginSession"), String.class, Object.class);
-            return LdapLoginWorker.doMainLogin(request, response, userLogin, userLoginSession);
+            return LoginWorker.doMainLogin(request, response, userLogin, userLoginSession);
         } else {
             Map<String, String> messageMap = UtilMisc.toMap("errorMessage", (String) loginResult.get(ModelService.ERROR_MESSAGE));
-            String errMsg = UtilProperties.getMessage(LdapLoginWorker.resourceWebapp, "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
+            String errMsg = UtilProperties.getMessage("SecurityextUiLabels", "loginevents.following_error_occurred_during_login", messageMap, UtilHttp.getLocale(request));
             throw new Exception(errMsg);
         }
     }
diff --git a/solr/src/main/java/org/apache/ofbiz/solr/control/OFBizSolrLoginWorker.java b/solr/src/main/java/org/apache/ofbiz/solr/control/OFBizSolrLoginWorker.java
index 1dd08fd..dcc278b 100644
--- a/solr/src/main/java/org/apache/ofbiz/solr/control/OFBizSolrLoginWorker.java
+++ b/solr/src/main/java/org/apache/ofbiz/solr/control/OFBizSolrLoginWorker.java
@@ -27,9 +27,10 @@ import org.apache.ofbiz.webapp.control.LoginWorker;
 /**
  * OFBiz Solr Login Workers
  */
-public class OFBizSolrLoginWorker extends LoginWorker {
+public final class OFBizSolrLoginWorker{
 
-    public final static String MODULE = OFBizSolrLoginWorker.class.getName();
+    private final static String MODULE = OFBizSolrLoginWorker.class.getName();
+    protected OFBizSolrLoginWorker() { }
 
     /**
      * An HTTP WebEvent handler that logs in a userLogin. This should run before the security check.
@@ -42,32 +43,26 @@ public class OFBizSolrLoginWorker extends LoginWorker {
     public static String login(HttpServletRequest request, HttpServletResponse response) {
         String result = LoginWorker.login(request, response);
         if ("success".equals(result)) {
-            // send the redirect
-            try {            
-                response.setStatus(HttpServletResponse.SC_MOVED_TEMPORARILY);
-                response.setHeader("Location", request.getContextPath());
-                response.setHeader("Connection", "close");
-            } catch (IllegalStateException ise) {
-                Debug.logError(ise.getMessage(), MODULE);
-                return "error";
-            }
+            result = setResponse(request, response);
         }
         return result;
     }
-
     public static String extensionCheckLogin(HttpServletRequest request, HttpServletResponse response) {
         String result = LoginWorker.extensionCheckLogin(request, response);
         if ("success".equals(result)) {
-            // send the redirect
-            try {            
-                response.setStatus(HttpServletResponse.SC_MOVED_TEMPORARILY);
-                response.setHeader("Location", request.getContextPath());
-                response.setHeader("Connection", "close");
-            } catch (IllegalStateException ise) {
-                Debug.logError(ise.getMessage(), MODULE);
-                return "error";
-            }
+            result = setResponse(request, response);
         }
         return result;
     }
+    private static String setResponse(HttpServletRequest request, HttpServletResponse response) {
+        try {
+            response.setStatus(HttpServletResponse.SC_MOVED_TEMPORARILY);
+            response.setHeader("Location", request.getContextPath());
+            response.setHeader("Connection", "close");
+        } catch (IllegalStateException ise) {
+            Debug.logError(ise.getMessage(), MODULE);
+            return "error";
+        }
+        return "success";
+    }
 }