feat: improved csrf check (#585)

* fix: ensure Referer header is provided or check allowed user agent * feat: allow java user agent
obiba · Jan 11, 2025 · 66f79ea · 66f79ea
1 parent 92b4e05
commit 66f79ea
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 15 deletions.
diff --git a/agate-core/src/main/resources/config/application.yml b/agate-core/src/main/resources/config/application.yml
@@ -45,3 +45,6 @@ login:
   trialTime: 300
   banTime: 300
   otpTimeout: 600
+
+csrf:
+  allowed-agents: curl,python,java
diff --git a/agate-rest/src/main/java/org/obiba/agate/web/rest/config/JerseyConfiguration.java b/agate-rest/src/main/java/org/obiba/agate/web/rest/config/JerseyConfiguration.java
@@ -37,7 +37,7 @@ public JerseyConfiguration(Environment environment) {
     ///register(LoggingFilter.class);
     register(AuthenticationInterceptor.class);
     register(AuditInterceptor.class);
-    register(new CSRFInterceptor(environment.acceptsProfiles(Profiles.of(Constants.SPRING_PROFILE_PRODUCTION)), environment.getProperty("csrf.allowed", "")));
+    register(new CSRFInterceptor(environment.acceptsProfiles(Profiles.of(Constants.SPRING_PROFILE_PRODUCTION)), environment.getProperty("csrf.allowed", ""), environment.getProperty("csrf.allowed-agents", "")));
     // validation errors will be sent to the client
     property(ServerProperties.BV_SEND_ERROR_IN_RESPONSE, true);
   }

diff --git a/agate-rest/src/main/java/org/obiba/agate/web/rest/security/CSRFInterceptor.java b/agate-rest/src/main/java/org/obiba/agate/web/rest/security/CSRFInterceptor.java
@@ -32,24 +32,32 @@ public class CSRFInterceptor implements ContainerRequestFilter {
 
   private static final String REFERER_HEADER = "Referer";
 
+  private static final String USER_AGENT_HEADER = "User-Agent";
+
   private static final Pattern localhostPattern = Pattern.compile("^http[s]?://localhost:");
 
   private static final Pattern loopbackhostPattern = Pattern.compile("^http[s]?://127\\.0\\.0\\.1:");
 
   private final boolean productionMode;
 
-  private final List<String> csrfAllowed;
+  private final List<String> csrfAllowedHosts;
+
+  private final List<String> csrfAllowedAgents;
 
-  public CSRFInterceptor(boolean productionMode, String csrfAllowed) {
+  public CSRFInterceptor(boolean productionMode, String csrfAllowedHosts, String csrfAllowedAgents) {
     this.productionMode = productionMode;
-    this.csrfAllowed = Strings.isNullOrEmpty(csrfAllowed) ? Lists.newArrayList() : Splitter.on(",").splitToList(csrfAllowed.trim());
+    this.csrfAllowedHosts = Strings.isNullOrEmpty(csrfAllowedHosts) ? Lists.newArrayList() : Splitter.on(",").splitToList(csrfAllowedHosts.trim());
+    this.csrfAllowedAgents = Strings.isNullOrEmpty(csrfAllowedAgents) ? Lists.newArrayList() : Splitter.on(",").splitToList(csrfAllowedAgents.trim());
   }
 
   @Override
-  public void filter(ContainerRequestContext requestContext) throws IOException {
-    if (!productionMode || csrfAllowed.contains("*")) return;
+  public void filter(ContainerRequestContext requestContext)
+      throws IOException {
+    if (!productionMode || csrfAllowedHosts.contains("*")) return;
 
     String host = requestContext.getHeaderString(HOST_HEADER);
+    if (matchesLocalhost(host)) return;
+
     String referer = requestContext.getHeaderString(REFERER_HEADER);
     if (referer != null) {
       String refererHostPort = "";
@@ -60,26 +68,33 @@ public void filter(ContainerRequestContext requestContext) throws IOException {
         // malformed url
       }
       // explicitly ok
-      if (csrfAllowed.contains(refererHostPort)) return;
-
-      boolean forbidden = false;
-      if (!matchesLocalhost(host) && !referer.startsWith(String.format("https://%s/", host))) {
-        forbidden = true;
-      }
+      if (csrfAllowedHosts.contains(refererHostPort)) return;
 
+      boolean forbidden = !referer.startsWith(String.format("https://%s/", host));
       if (forbidden) {
         log.warn("CSRF detection: Host={}, Referer={}", host, referer);
         log.info(">> You can add {} to csrf.allowed setting", refererHostPort);
         throw new ForbiddenException();
       }
+    } else {
+      String userAgent = requestContext.getHeaderString(USER_AGENT_HEADER);
+      if (Strings.isNullOrEmpty(userAgent) || !matchesUserAgent(userAgent)) {
+        log.warn("CSRF detection: Host={}, User-Agent={}", host, userAgent);
+        log.info(">> Ensure 'Referer' HTTP header is set or allow this 'User-Agent' with 'csrf.allowed-agents' setting");
+        throw new ForbiddenException("CSRF error");
+      }
     }
   }
 
-
   private boolean matchesLocalhost(String host) {
     return localhostPattern.matcher(host).matches()
         || loopbackhostPattern.matcher(host).matches()
         || host.startsWith("localhost:")
         || host.startsWith("127.0.0.1:");
   }
-}
+
+  private boolean matchesUserAgent(String userAgent) {
+    return csrfAllowedAgents.stream().anyMatch(ua -> userAgent.toLowerCase().contains(ua.toLowerCase()));
+  }
+
+}
diff --git a/agate-webapp/src/main/java/org/obiba/agate/web/controller/AdminController.java b/agate-webapp/src/main/java/org/obiba/agate/web/controller/AdminController.java
@@ -51,7 +51,7 @@ private void includeEntryPoints(ModelAndView mv) throws IOException {
     for (Resource resource : resources) {
       String fileName = resource.getFilename();
       if (fileName != null && fileName.startsWith("index-")) {
-        log.info("Quasar entrypoint: {}", fileName);
+        log.debug("Quasar entrypoint: {}", fileName);
         if (fileName.endsWith(".js")) {
           mv.getModel().put("entryPointJS", fileName);
         } else if (fileName.endsWith(".css")) {