From c7789533a01d95bf867e89bd84c1be0bd47713b4 Mon Sep 17 00:00:00 2001 From: Andrew DeMaria Date: Sun, 29 Oct 2017 15:10:49 -0600 Subject: [PATCH] Don't require csrf for search endpoint Reasoning: - It doesn't change state and is not a sensitive endpoint - It really should be changed to GET but that is a bit more intrusive change that can be done at another time - The search csrf token is stored on the top.jsp page for a long time. If the user keeps this tab open for a while it is possible the csrf token will change on their session with other requests going on such that the search csrf token becomes wrong/stale. Signed-off-by: Andrew DeMaria --- .../security/CsrfSecurityRequestMatcher.java | 29 ++++++++++--------- .../src/main/webapp/WEB-INF/jsp/top.jsp | 1 - 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/airsonic-main/src/main/java/org/airsonic/player/security/CsrfSecurityRequestMatcher.java b/airsonic-main/src/main/java/org/airsonic/player/security/CsrfSecurityRequestMatcher.java index 262b1307..8a624c6f 100644 --- a/airsonic-main/src/main/java/org/airsonic/player/security/CsrfSecurityRequestMatcher.java +++ b/airsonic-main/src/main/java/org/airsonic/player/security/CsrfSecurityRequestMatcher.java @@ -6,6 +6,8 @@ import org.springframework.stereotype.Component; import javax.servlet.http.HttpServletRequest; +import java.util.ArrayList; +import java.util.Collection; import java.util.regex.Pattern; /** @@ -19,24 +21,23 @@ import java.util.regex.Pattern; @Component public class CsrfSecurityRequestMatcher implements RequestMatcher { private Pattern allowedMethods = Pattern.compile("^(GET|HEAD|TRACE|OPTIONS)$"); - private RegexRequestMatcher dwrRequestMatcher = new RegexRequestMatcher("/dwr/.*\\.dwr", "POST"); - private RegexRequestMatcher restRequestMatcher = new RegexRequestMatcher("/rest/.*\\.view(\\?.*)?", "POST"); + private Collection whiteListedMatchers; + + public CsrfSecurityRequestMatcher() { + Collection whiteListedMatchers = new ArrayList<>(); + whiteListedMatchers.add(new RegexRequestMatcher("/dwr/.*\\.dwr", "POST")); + whiteListedMatchers.add(new RegexRequestMatcher("/rest/.*\\.view(\\?.*)?", "POST")); + whiteListedMatchers.add(new RegexRequestMatcher("/search(?:\\.view)?", "POST")); + this.whiteListedMatchers = whiteListedMatchers; + } @Override public boolean matches(HttpServletRequest request) { - boolean requireCsrfToken = true; - - if(allowedMethods.matcher(request.getMethod()).matches()){ - requireCsrfToken = false; - } else { - if (dwrRequestMatcher.matches(request)) { - requireCsrfToken = false; - } else if (restRequestMatcher.matches(request)) { - requireCsrfToken = false; - } - } + boolean skipCSRF = + allowedMethods.matcher(request.getMethod()).matches() || + whiteListedMatchers.stream().anyMatch(matcher -> matcher.matches(request)); - return requireCsrfToken; + return !skipCSRF; } } \ No newline at end of file diff --git a/airsonic-main/src/main/webapp/WEB-INF/jsp/top.jsp b/airsonic-main/src/main/webapp/WEB-INF/jsp/top.jsp index 2b2d4980..79629551 100644 --- a/airsonic-main/src/main/webapp/WEB-INF/jsp/top.jsp +++ b/airsonic-main/src/main/webapp/WEB-INF/jsp/top.jsp @@ -125,7 +125,6 @@
- " alt="${search}" title="${search}">