From 1b2df20fdd0d083f9cd2b32b28aecc4aa6264f86 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Fri, 29 Dec 2023 20:48:21 +0000 Subject: [PATCH] reviews --- .../CustomAuthenticationSuccessHandler.java | 14 +++----------- .../config/security/FirstLoginFilter.java | 7 ++----- .../config/security/IPRateLimitingFilter.java | 8 ++------ .../config/security/LoginAttemptService.java | 19 +++++++++++++++++-- .../security/RateLimitResetScheduler.java | 2 +- .../api/pipeline/ApiDocService.java | 7 ++++++- .../pipeline/PipelineDirectoryProcessor.java | 4 ++-- .../api/pipeline/PipelineProcessor.java | 4 ++-- .../SPDF/model/ApplicationProperties.java | 19 +++++++++++++++++++ .../software/SPDF/model/AttemptCounter.java | 12 ++++++------ .../software/SPDF/utils/RequestUriUtils.java | 16 ++++++++++++++++ src/main/resources/settings.yml.template | 4 +++- 12 files changed, 79 insertions(+), 37 deletions(-) create mode 100644 src/main/java/stirling/software/SPDF/utils/RequestUriUtils.java diff --git a/src/main/java/stirling/software/SPDF/config/security/CustomAuthenticationSuccessHandler.java b/src/main/java/stirling/software/SPDF/config/security/CustomAuthenticationSuccessHandler.java index ae97ec4a..cd2217e1 100644 --- a/src/main/java/stirling/software/SPDF/config/security/CustomAuthenticationSuccessHandler.java +++ b/src/main/java/stirling/software/SPDF/config/security/CustomAuthenticationSuccessHandler.java @@ -12,6 +12,7 @@ import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpSession; +import stirling.software.SPDF.utils.RequestUriUtils; @Component public class CustomAuthenticationSuccessHandler extends SavedRequestAwareAuthenticationSuccessHandler { @@ -28,7 +29,7 @@ public class CustomAuthenticationSuccessHandler extends SavedRequestAwareAuthent // Get the saved request HttpSession session = request.getSession(false); SavedRequest savedRequest = session != null ? (SavedRequest) session.getAttribute("SPRING_SECURITY_SAVED_REQUEST") : null; - if (savedRequest != null && !isStaticResource(savedRequest)) { + if (savedRequest != null && !RequestUriUtils.isStaticResource(savedRequest.getRedirectUrl())) { // Redirect to the original destination super.onAuthenticationSuccess(request, response, authentication); } else { @@ -38,15 +39,6 @@ public class CustomAuthenticationSuccessHandler extends SavedRequestAwareAuthent //super.onAuthenticationSuccess(request, response, authentication); } - - private boolean isStaticResource(SavedRequest savedRequest) { - String requestURI = savedRequest.getRedirectUrl(); - return requestURI.startsWith("/css/") - || requestURI.startsWith("/js/") - || requestURI.startsWith("/images/") - || requestURI.startsWith("/public/") - || requestURI.startsWith("/pdfjs/") - || requestURI.endsWith(".svg"); - } + } diff --git a/src/main/java/stirling/software/SPDF/config/security/FirstLoginFilter.java b/src/main/java/stirling/software/SPDF/config/security/FirstLoginFilter.java index 8e612464..65acf148 100644 --- a/src/main/java/stirling/software/SPDF/config/security/FirstLoginFilter.java +++ b/src/main/java/stirling/software/SPDF/config/security/FirstLoginFilter.java @@ -15,6 +15,7 @@ import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import stirling.software.SPDF.model.User; +import stirling.software.SPDF.utils.RequestUriUtils; @Component public class FirstLoginFilter extends OncePerRequestFilter { @@ -28,11 +29,7 @@ public class FirstLoginFilter extends OncePerRequestFilter { String method = request.getMethod(); String requestURI = request.getRequestURI(); // Check if the request is for static resources - boolean isStaticResource = requestURI.startsWith("/css/") - || requestURI.startsWith("/js/") - || requestURI.startsWith("/images/") - || requestURI.startsWith("/public/") - || requestURI.endsWith(".svg"); + boolean isStaticResource = RequestUriUtils.isStaticResource(requestURI); // If it's a static resource, just continue the filter chain and skip the logic below if (isStaticResource) { diff --git a/src/main/java/stirling/software/SPDF/config/security/IPRateLimitingFilter.java b/src/main/java/stirling/software/SPDF/config/security/IPRateLimitingFilter.java index 22bf653d..03e34b57 100644 --- a/src/main/java/stirling/software/SPDF/config/security/IPRateLimitingFilter.java +++ b/src/main/java/stirling/software/SPDF/config/security/IPRateLimitingFilter.java @@ -9,6 +9,7 @@ import jakarta.servlet.ServletException; import jakarta.servlet.ServletRequest; import jakarta.servlet.ServletResponse; import jakarta.servlet.http.HttpServletRequest; +import stirling.software.SPDF.utils.RequestUriUtils; public class IPRateLimitingFilter implements Filter { @@ -29,12 +30,7 @@ public class IPRateLimitingFilter implements Filter { String method = httpRequest.getMethod(); String requestURI = httpRequest.getRequestURI(); // Check if the request is for static resources - boolean isStaticResource = requestURI.startsWith("/css/") - || requestURI.startsWith("/js/") - || requestURI.startsWith("/images/") - || requestURI.startsWith("/public/") - || requestURI.startsWith("/pdfjs/") - || requestURI.endsWith(".svg"); + boolean isStaticResource = RequestUriUtils.isStaticResource(requestURI); // If it's a static resource, just continue the filter chain and skip the logic below if (isStaticResource) { diff --git a/src/main/java/stirling/software/SPDF/config/security/LoginAttemptService.java b/src/main/java/stirling/software/SPDF/config/security/LoginAttemptService.java index 2c86c65f..55a84449 100644 --- a/src/main/java/stirling/software/SPDF/config/security/LoginAttemptService.java +++ b/src/main/java/stirling/software/SPDF/config/security/LoginAttemptService.java @@ -2,15 +2,30 @@ package stirling.software.SPDF.config.security; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; +import jakarta.annotation.PostConstruct; +import stirling.software.SPDF.model.ApplicationProperties; import stirling.software.SPDF.model.AttemptCounter; @Service public class LoginAttemptService { - private final int MAX_ATTEMPTS = 10; - private final long ATTEMPT_INCREMENT_TIME = TimeUnit.MINUTES.toMillis(1); + + @Autowired + ApplicationProperties applicationProperties; + + private int MAX_ATTEMPTS; + private long ATTEMPT_INCREMENT_TIME; + + + @PostConstruct + public void init() { + MAX_ATTEMPTS = applicationProperties.getSecurity().getLoginAttemptCount(); + ATTEMPT_INCREMENT_TIME = TimeUnit.MINUTES.toMillis(applicationProperties.getSecurity().getLoginResetTimeMinutes()); + } + private final ConcurrentHashMap attemptsCache = new ConcurrentHashMap<>(); public void loginSucceeded(String key) { diff --git a/src/main/java/stirling/software/SPDF/config/security/RateLimitResetScheduler.java b/src/main/java/stirling/software/SPDF/config/security/RateLimitResetScheduler.java index 1a044582..3ef8ef31 100644 --- a/src/main/java/stirling/software/SPDF/config/security/RateLimitResetScheduler.java +++ b/src/main/java/stirling/software/SPDF/config/security/RateLimitResetScheduler.java @@ -11,7 +11,7 @@ public class RateLimitResetScheduler { this.rateLimitingFilter = rateLimitingFilter; } - @Scheduled(cron = "0 0 0 * * MON") // At 00:00 every Monday + @Scheduled(cron = "0 0 0 * * MON") // At 00:00 every Monday TODO: configurable public void resetRateLimit() { rateLimitingFilter.resetRequestCounts(); } diff --git a/src/main/java/stirling/software/SPDF/controller/api/pipeline/ApiDocService.java b/src/main/java/stirling/software/SPDF/controller/api/pipeline/ApiDocService.java index 78cda94d..43448f89 100644 --- a/src/main/java/stirling/software/SPDF/controller/api/pipeline/ApiDocService.java +++ b/src/main/java/stirling/software/SPDF/controller/api/pipeline/ApiDocService.java @@ -28,7 +28,12 @@ public class ApiDocService { private String getApiDocsUrl() { String contextPath = servletContext.getContextPath(); - return "http://localhost:8080" + contextPath + "/v1/api-docs"; + String port = System.getProperty("local.server.port"); + if(port == null || port.length() == 0) { + port="8080"; + } + + return "http://localhost:"+ port + contextPath + "/v1/api-docs"; } diff --git a/src/main/java/stirling/software/SPDF/controller/api/pipeline/PipelineDirectoryProcessor.java b/src/main/java/stirling/software/SPDF/controller/api/pipeline/PipelineDirectoryProcessor.java index a73e1ca7..dc45d4cb 100644 --- a/src/main/java/stirling/software/SPDF/controller/api/pipeline/PipelineDirectoryProcessor.java +++ b/src/main/java/stirling/software/SPDF/controller/api/pipeline/PipelineDirectoryProcessor.java @@ -109,7 +109,7 @@ public class PipelineDirectoryProcessor { private PipelineConfig readAndParseJson(Path jsonFile) throws IOException { String jsonString = new String(Files.readAllBytes(jsonFile), StandardCharsets.UTF_8); - logger.info("Reading JSON file: {}", jsonFile); + logger.debug("Reading JSON file: {}", jsonFile); return objectMapper.readValue(jsonString, PipelineConfig.class); } @@ -118,7 +118,7 @@ public class PipelineDirectoryProcessor { validateOperation(operation); File[] files = collectFilesForProcessing(dir, jsonFile, operation); if(files == null || files.length == 0) { - logger.info("No files detected for {} ", dir); + logger.debug("No files detected for {} ", dir); return; } List filesToProcess = prepareFilesForProcessing(files, processingDir); diff --git a/src/main/java/stirling/software/SPDF/controller/api/pipeline/PipelineProcessor.java b/src/main/java/stirling/software/SPDF/controller/api/pipeline/PipelineProcessor.java index 775de3b3..1ac7eef9 100644 --- a/src/main/java/stirling/software/SPDF/controller/api/pipeline/PipelineProcessor.java +++ b/src/main/java/stirling/software/SPDF/controller/api/pipeline/PipelineProcessor.java @@ -238,7 +238,7 @@ public class PipelineProcessor { for (File file : files) { Path path = Paths.get(file.getAbsolutePath()); - System.out.println("Reading file: " + path); // debug statement + logger.info("Reading file: " + path); // debug statement if (Files.exists(path)) { Resource fileResource = new ByteArrayResource(Files.readAllBytes(path)) { @@ -249,7 +249,7 @@ public class PipelineProcessor { }; outputFiles.add(fileResource); } else { - System.out.println("File not found: " + path); // debug statement + logger.info("File not found: " + path); } } logger.info("Files successfully loaded. Starting processing..."); diff --git a/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java b/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java index 735edc6f..36073c88 100644 --- a/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java +++ b/src/main/java/stirling/software/SPDF/model/ApplicationProperties.java @@ -106,6 +106,25 @@ public class ApplicationProperties { private Boolean enableLogin; private Boolean csrfDisabled; private InitialLogin initialLogin; + private int loginAttemptCount; + private long loginResetTimeMinutes; + + + public int getLoginAttemptCount() { + return loginAttemptCount; + } + + public void setLoginAttemptCount(int loginAttemptCount) { + this.loginAttemptCount = loginAttemptCount; + } + + public long getLoginResetTimeMinutes() { + return loginResetTimeMinutes; + } + + public void setLoginResetTimeMinutes(long loginResetTimeMinutes) { + this.loginResetTimeMinutes = loginResetTimeMinutes; + } public InitialLogin getInitialLogin() { return initialLogin != null ? initialLogin : new InitialLogin(); diff --git a/src/main/java/stirling/software/SPDF/model/AttemptCounter.java b/src/main/java/stirling/software/SPDF/model/AttemptCounter.java index 94f8ef4a..fd668b05 100644 --- a/src/main/java/stirling/software/SPDF/model/AttemptCounter.java +++ b/src/main/java/stirling/software/SPDF/model/AttemptCounter.java @@ -1,27 +1,27 @@ package stirling.software.SPDF.model; public class AttemptCounter { private int attemptCount; - private long firstAttemptTime; + private long lastAttemptTime; public AttemptCounter() { this.attemptCount = 1; - this.firstAttemptTime = System.currentTimeMillis(); + this.lastAttemptTime = System.currentTimeMillis(); } public void increment() { this.attemptCount++; - this.firstAttemptTime = System.currentTimeMillis(); + this.lastAttemptTime = System.currentTimeMillis(); } public int getAttemptCount() { return attemptCount; } - public long getFirstAttemptTime() { - return firstAttemptTime; + public long getlastAttemptTime() { + return lastAttemptTime; } public boolean shouldReset(long ATTEMPT_INCREMENT_TIME) { - return System.currentTimeMillis() - firstAttemptTime > ATTEMPT_INCREMENT_TIME; + return System.currentTimeMillis() - lastAttemptTime > ATTEMPT_INCREMENT_TIME; } } diff --git a/src/main/java/stirling/software/SPDF/utils/RequestUriUtils.java b/src/main/java/stirling/software/SPDF/utils/RequestUriUtils.java new file mode 100644 index 00000000..0046ee9f --- /dev/null +++ b/src/main/java/stirling/software/SPDF/utils/RequestUriUtils.java @@ -0,0 +1,16 @@ +package stirling.software.SPDF.utils; + +public class RequestUriUtils { + + public static boolean isStaticResource(String requestURI) { + + return requestURI.startsWith("/css/") + || requestURI.startsWith("/js/") + || requestURI.startsWith("/images/") + || requestURI.startsWith("/public/") + || requestURI.startsWith("/pdfjs/") + || requestURI.endsWith(".svg"); + + } + +} diff --git a/src/main/resources/settings.yml.template b/src/main/resources/settings.yml.template index cafea794..52d5e4de 100644 --- a/src/main/resources/settings.yml.template +++ b/src/main/resources/settings.yml.template @@ -5,7 +5,9 @@ security: enableLogin: false # set to 'true' to enable login csrfDisabled: true - + loginAttemptCount: 5 # lock user account after 5 tries + loginResetTimeMinutes : 120 # lock account for 2 hours after x attempts + system: defaultLocale: 'en-US' # Set the default language (e.g. 'de-DE', 'fr-FR', etc) googlevisibility: false # 'true' to allow Google visibility (via robots.txt), 'false' to disallow