Clean login failure logging

3 files changed, 28 insertions, 386 deletions

diff --git a/src/main/java/org/traccar/helper/LogAction.java b/src/main/java/org/traccar/helper/LogAction.java
index 21fe4c3fa..16d55ec60 100644
--- a/src/main/java/org/traccar/helper/LogAction.java
+++ b/src/main/java/org/traccar/helper/LogAction.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017 Anton Tananaev (anton@traccar.org)
+ * Copyright 2017 - 2020 Anton Tananaev (anton@traccar.org)
  * Copyright 2017 Andrey Kunitsyn (andrey@traccar.org)
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -38,14 +38,13 @@ public final class LogAction {
 
     private static final String ACTION_LOGIN = "login";
     private static final String ACTION_LOGOUT = "logout";
-    private static final String ACTION_FAILED_LOGIN_NO_IP = "Failed Login Attempt. IP address: failed to retrieve";
 
     private static final String ACTION_DEVICE_ACCUMULATORS = "resetDeviceAccumulators";
 
     private static final String PATTERN_OBJECT = "user: %d, action: %s, object: %s, id: %d";
     private static final String PATTERN_LINK = "user: %d, action: %s, owner: %s, id: %d, property: %s, id: %d";
     private static final String PATTERN_LOGIN = "user: %d, action: %s";
-    private static final String PATTERN_FAILED_LOGIN = "Failed Login Attempt. IP address: %s";
+    private static final String PATTERN_LOGIN_FAILED = "login failed from: %s";
     private static final String PATTERN_DEVICE_ACCUMULATORS = "user: %d, action: %s, deviceId: %d";
 
     public static void create(long userId, BaseModel object) {
@@ -76,18 +75,13 @@ public final class LogAction {
         logLoginAction(ACTION_LOGOUT, userId);
     }
 
-    public static void failedLogin(String ipAddress) {
-
-        if (ipAddress == null || ipAddress.isEmpty()) {
-            LOGGER.info(ACTION_FAILED_LOGIN_NO_IP);
-        } else {
-            LOGGER.info(String.format(
-                    PATTERN_FAILED_LOGIN, ipAddress));
+    public static void failedLogin(String remoteAddress) {
+        if (remoteAddress == null || remoteAddress.isEmpty()) {
+            remoteAddress = "unknown";
         }
-
+        LOGGER.info(String.format(PATTERN_LOGIN_FAILED, remoteAddress));
     }
 
-
     public static void resetDeviceAccumulators(long userId, long deviceId) {
         LOGGER.info(String.format(
                 PATTERN_DEVICE_ACCUMULATORS, userId, ACTION_DEVICE_ACCUMULATORS, deviceId));
diff --git a/src/main/java/org/traccar/helper/ServletHelper.java b/src/main/java/org/traccar/helper/ServletHelper.java
index f951d17c0..e3481e249 100644
--- a/src/main/java/org/traccar/helper/ServletHelper.java
+++ b/src/main/java/org/traccar/helper/ServletHelper.java
@@ -5,24 +5,23 @@ import javax.servlet.http.HttpServletRequest;
 
 public final class ServletHelper {
 
-
     private ServletHelper() {
     }
 
     public static String retrieveRemoteAddress(HttpServletRequest request) {
 
         if (request != null) {
-            String ipAddress = request.getHeader("X-FORWARDED-FOR");
+            String remoteAddress = request.getHeader("X-FORWARDED-FOR");
 
-            if (ipAddress != null && !ipAddress.isEmpty()) {
-                return ipAddress.substring(0, ipAddress.indexOf(",")); //Removes the additional data
+            if (remoteAddress != null && !remoteAddress.isEmpty()) {
+                return remoteAddress.substring(0, remoteAddress.indexOf(",")); // removes the additional data
             } else {
-                ipAddress = request.getRemoteAddr();
-                return ipAddress;
+                remoteAddress = request.getRemoteAddr();
+                return remoteAddress;
             }
-
         } else {
             return null;
         }
     }
+
 }
diff --git a/src/test/java/org/traccar/helper/ServletHelperTest.java b/src/test/java/org/traccar/helper/ServletHelperTest.java
index 963be998c..7359bf3dd 100644
--- a/src/test/java/org/traccar/helper/ServletHelperTest.java
+++ b/src/test/java/org/traccar/helper/ServletHelperTest.java
@@ -1,57 +1,40 @@
 package org.traccar.helper;
 
-
-import org.junit.Before;
+import org.apache.struts.mock.MockHttpServletRequest;
 import org.junit.Test;
 
-
-import javax.servlet.*;
-import javax.servlet.http.*;
-
-import java.io.BufferedReader;
-import java.io.IOException;
-import java.io.UnsupportedEncodingException;
-import java.security.Principal;
-import java.util.*;
+import java.util.HashMap;
+import java.util.Map;
 
 import static org.junit.Assert.assertEquals;
 
 public class ServletHelperTest {
 
-
-    private MockHttpServletRequestForRemoteAddr mockHttpServletRequest;
-
-
-    @Before
-    public void init() {
-        mockHttpServletRequest = new MockHttpServletRequestForRemoteAddr();
-    }
-
     @Test
     public void testIpBehindReverseProxy() {
-        mockHttpServletRequest.setRemoteAddr("147.120.1.5");
-        mockHttpServletRequest.addHeader("X-FORWARDED-FOR", "231.23.45.65, 10.20.10.33, 10.20.20.34");
+        MockRequest request = new MockRequest();
+        request.setRemoteAddress("147.120.1.5");
+        request.addHeader("X-FORWARDED-FOR", "231.23.45.65, 10.20.10.33, 10.20.20.34");
 
-        assertEquals("231.23.45.65", ServletHelper.retrieveRemoteAddress(mockHttpServletRequest));
+        assertEquals("231.23.45.65", ServletHelper.retrieveRemoteAddress(request));
     }
 
     @Test
     public void testNormalIp() {
-        mockHttpServletRequest.setRemoteAddr("231.23.45.65");
-        assertEquals("231.23.45.65", ServletHelper.retrieveRemoteAddress(mockHttpServletRequest));
+        MockRequest request = new MockRequest();
+        request.setRemoteAddress("231.23.45.65");
 
+        assertEquals("231.23.45.65", ServletHelper.retrieveRemoteAddress(request));
     }
 
-    /**
-     * This mock implementation only supports IP address-related operations.
-     */
-    private final class MockHttpServletRequestForRemoteAddr implements HttpServletRequest {
+    private final static class MockRequest extends MockHttpServletRequest {
+
+        private String remoteAddress;
 
-        private String remoteAddr;
         private Map<String, String> headers = new HashMap<>();
 
-        public void setRemoteAddr(String remoteAddr) {
-            this.remoteAddr = remoteAddr;
+        public void setRemoteAddress(String remoteAddress) {
+            this.remoteAddress = remoteAddress;
         }
 
         public void addHeader(String name, String value) {
@@ -65,343 +48,9 @@ public class ServletHelperTest {
 
         @Override
         public String getRemoteAddr() {
-            return remoteAddr;
-        }
-
-        @Override
-        public String getAuthType() {
-            return null;
-        }
-
-        @Override
-        public Cookie[] getCookies() {
-            return new Cookie[0];
-        }
-
-        @Override
-        public long getDateHeader(String name) {
-            return 0;
-        }
-
-        @Override
-        public Enumeration<String> getHeaders(String name) {
-            return null;
-        }
-
-        @Override
-        public Enumeration<String> getHeaderNames() {
-            return null;
-        }
-
-        @Override
-        public int getIntHeader(String name) {
-            return 0;
-        }
-
-        @Override
-        public String getMethod() {
-            return null;
-        }
-
-        @Override
-        public String getPathInfo() {
-            return null;
-        }
-
-        @Override
-        public String getPathTranslated() {
-            return null;
-        }
-
-        @Override
-        public String getContextPath() {
-            return null;
-        }
-
-        @Override
-        public String getQueryString() {
-            return null;
-        }
-
-        @Override
-        public String getRemoteUser() {
-            return null;
-        }
-
-        @Override
-        public boolean isUserInRole(String role) {
-            return false;
-        }
-
-        @Override
-        public Principal getUserPrincipal() {
-            return null;
-        }
-
-        @Override
-        public String getRequestedSessionId() {
-            return null;
-        }
-
-        @Override
-        public String getRequestURI() {
-            return null;
-        }
-
-        @Override
-        public StringBuffer getRequestURL() {
-            return null;
-        }
-
-        @Override
-        public String getServletPath() {
-            return null;
-        }
-
-        @Override
-        public HttpSession getSession(boolean create) {
-            return null;
-        }
-
-        @Override
-        public HttpSession getSession() {
-            return null;
-        }
-
-        @Override
-        public String changeSessionId() {
-            return null;
-        }
-
-        @Override
-        public boolean isRequestedSessionIdValid() {
-            return false;
-        }
-
-        @Override
-        public boolean isRequestedSessionIdFromCookie() {
-            return false;
-        }
-
-        @Override
-        public boolean isRequestedSessionIdFromURL() {
-            return false;
-        }
-
-        @Override
-        public boolean isRequestedSessionIdFromUrl() {
-            return false;
-        }
-
-        @Override
-        public boolean authenticate(HttpServletResponse response) throws IOException, ServletException {
-            return false;
-        }
-
-        @Override
-        public void login(String username, String password) throws ServletException {
-
-        }
-
-        @Override
-        public void logout() throws ServletException {
-
-        }
-
-        @Override
-        public Collection<Part> getParts() throws IOException, ServletException {
-            return null;
-        }
-
-        @Override
-        public Part getPart(String name) throws IOException, ServletException {
-            return null;
-        }
-
-        @Override
-        public <T extends HttpUpgradeHandler> T upgrade(Class<T> handlerClass) throws IOException, ServletException {
-            return null;
-        }
-
-        @Override
-        public Object getAttribute(String name) {
-            return null;
+            return remoteAddress;
         }
 
-        @Override
-        public Enumeration<String> getAttributeNames() {
-            return null;
-        }
-
-        @Override
-        public String getCharacterEncoding() {
-            return null;
-        }
-
-        @Override
-        public void setCharacterEncoding(String env) throws UnsupportedEncodingException {
-
-        }
-
-        @Override
-        public int getContentLength() {
-            return 0;
-        }
-
-        @Override
-        public long getContentLengthLong() {
-            return 0;
-        }
-
-        @Override
-        public String getContentType() {
-            return null;
-        }
-
-        @Override
-        public ServletInputStream getInputStream() throws IOException {
-            return null;
-        }
-
-        @Override
-        public String getParameter(String name) {
-            return null;
-        }
-
-        @Override
-        public Enumeration<String> getParameterNames() {
-            return null;
-        }
-
-        @Override
-        public String[] getParameterValues(String name) {
-            return new String[0];
-        }
-
-        @Override
-        public Map<String, String[]> getParameterMap() {
-            return null;
-        }
-
-        @Override
-        public String getProtocol() {
-            return null;
-        }
-
-        @Override
-        public String getScheme() {
-            return null;
-        }
-
-        @Override
-        public String getServerName() {
-            return null;
-        }
-
-        @Override
-        public int getServerPort() {
-            return 0;
-        }
-
-        @Override
-        public BufferedReader getReader() throws IOException {
-            return null;
-        }
-
-        @Override
-        public String getRemoteHost() {
-            return null;
-        }
-
-        @Override
-        public void setAttribute(String name, Object o) {
-
-        }
-
-        @Override
-        public void removeAttribute(String name) {
-
-        }
-
-        @Override
-        public Locale getLocale() {
-            return null;
-        }
-
-        @Override
-        public Enumeration<Locale> getLocales() {
-            return null;
-        }
-
-        @Override
-        public boolean isSecure() {
-            return false;
-        }
-
-        @Override
-        public RequestDispatcher getRequestDispatcher(String path) {
-            return null;
-        }
-
-        @Override
-        public String getRealPath(String path) {
-            return null;
-        }
-
-        @Override
-        public int getRemotePort() {
-            return 0;
-        }
-
-        @Override
-        public String getLocalName() {
-            return null;
-        }
-
-        @Override
-        public String getLocalAddr() {
-            return null;
-        }
-
-        @Override
-        public int getLocalPort() {
-            return 0;
-        }
-
-        @Override
-        public ServletContext getServletContext() {
-            return null;
-        }
-
-        @Override
-        public AsyncContext startAsync() throws IllegalStateException {
-            return null;
-        }
-
-        @Override
-        public AsyncContext startAsync(ServletRequest servletRequest, ServletResponse servletResponse) throws IllegalStateException {
-            return null;
-        }
-
-        @Override
-        public boolean isAsyncStarted() {
-            return false;
-        }
-
-        @Override
-        public boolean isAsyncSupported() {
-            return false;
-        }
-
-        @Override
-        public AsyncContext getAsyncContext() {
-            return null;
-        }
-
-        @Override
-        public DispatcherType getDispatcherType() {
-            return null;
-        }
     }
 
 }