SONAR-7713 Exclude static resources from token validation filter

2 files changed, 275 insertions, 53 deletions

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/web/ServletFilter.java b/sonar-plugin-api/src/main/java/org/sonar/api/web/ServletFilter.java
index 9b6f709620d..fdd28ba2186 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/web/ServletFilter.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/web/ServletFilter.java
@@ -19,12 +19,20 @@
  */
 package org.sonar.api.web;
 
-import com.google.common.base.Preconditions;
-import com.google.common.base.Strings;
-import org.sonar.api.ExtensionPoint;
-import org.sonar.api.server.ServerSide;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.ImmutableSet.copyOf;
+import static org.sonar.api.web.ServletFilter.UrlPattern.Pattern.Type.END_WITH_START;
+import static org.sonar.api.web.ServletFilter.UrlPattern.Pattern.Type.MATCH_ALL;
+import static org.sonar.api.web.ServletFilter.UrlPattern.Pattern.Type.START_WITH_STAR;
 
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
 import javax.servlet.Filter;
+import org.sonar.api.ExtensionPoint;
+import org.sonar.api.server.ServerSide;
 
 /**
  * @since 3.1
@@ -37,53 +45,146 @@ public abstract class ServletFilter implements Filter {
    * Override to change URL. Default is /*
    */
   public UrlPattern doGetPattern() {
-    return UrlPattern.create("/*");
+    return UrlPattern.builder().build();
   }
 
   public static final class UrlPattern {
-    private int code;
-    private String url;
-    private String urlToMatch;
-
-    private UrlPattern(String url) {
-      Preconditions.checkArgument(!Strings.isNullOrEmpty(url), "Empty url");
-      this.url = url;
-      this.urlToMatch = url.replaceAll("/?\\*", "");
-      if ("/*".equals(url)) {
-        code = 1;
-      } else if (url.startsWith("*")) {
-        code = 2;
-      } else if (url.endsWith("*")) {
-        code = 3;
-      } else {
-        code = 4;
-      }
-    }
-    
-    public static UrlPattern create(String pattern) {
-      return new UrlPattern(pattern);
+
+    private static final String MATCH_ALL_URL = "/*";
+
+    private final boolean matchAllUrls;
+    private final List<Pattern> includePatterns = new ArrayList<>();
+    private final List<Pattern> excludePatterns = new ArrayList<>();
+
+    private UrlPattern(Builder builder) {
+      this.includePatterns.addAll(builder.includePatterns.stream().map(Pattern::new).collect(Collectors.toList()));
+      this.excludePatterns.addAll(builder.excludePatterns.stream().map(Pattern::new).collect(Collectors.toList()));
+      this.matchAllUrls = excludePatterns.isEmpty() && includePatterns.size() == 1 && includePatterns.get(0).getUrl().equals(MATCH_ALL_URL);
     }
 
     public boolean matches(String path) {
-      switch (code) {
-        case 1:
-          return true;
-        case 2:
-          return path.endsWith(urlToMatch);
-        case 3:
-          return path.startsWith(urlToMatch);
-        default:
-          return path.equals(urlToMatch);
-      }
+      // Optimization for filter that match all urls
+      return matchAllUrls ||
+      // Otherwise we first verify if the url is not excluded, then we verify if the url is included
+        (!excludePatterns.stream().anyMatch(pattern -> pattern.matches(path)) && includePatterns.stream().anyMatch(pattern -> pattern.matches(path)));
     }
 
-    public String getUrl() {
-      return url;
+    /**
+     * @since 6.0
+     */
+    public Set<String> getIncludePatterns() {
+      return includePatterns.stream().map(Pattern::getUrl).collect(Collectors.toSet());
+    }
+
+    /**
+     * @since 6.0
+     */
+    public Set<String> getExcludePatterns() {
+      return excludePatterns.stream().map(Pattern::getUrl).collect(Collectors.toSet());
+    }
+
+    /**
+     * @deprecated since 6.0, use {@link #getIncludePatterns()} or {@link #getExcludePatterns()} instead
+     */
+    @Deprecated
+    public String getUrl(){
+      // Before 6.0, it was only possible to include one url
+      if (excludePatterns.isEmpty() && includePatterns.size() == 1 ) {
+        return includePatterns.get(0).getUrl();
+      }
+      throw new IllegalStateException("this method is deprecated and should not be used anymore");
     }
 
     @Override
     public String toString() {
-      return url;
+      return "UrlPattern{" +
+        "includePatterns=" + includePatterns +
+        ", excludePatterns=" + excludePatterns +
+        '}';
     }
+
+    public static UrlPattern create(String pattern) {
+      return new UrlPattern.Builder().setIncludePatterns(pattern).build();
+    }
+
+    public static Builder builder() {
+      return new Builder();
+    }
+
+    public static class Builder {
+      private Set<String> includePatterns = new HashSet<>();
+      private Set<String> excludePatterns = new HashSet<>();
+
+      private Builder() {
+        // By default, every urls is accepted
+        this.includePatterns = new HashSet<>();
+        this.includePatterns.add(MATCH_ALL_URL);
+        this.excludePatterns = new HashSet<>();
+      }
+
+      public Builder setIncludePatterns(String... includePatterns) {
+        this.includePatterns = copyOf(includePatterns);
+        return this;
+      }
+
+      public Builder setExcludePatterns(String... excludePatterns) {
+        this.excludePatterns = copyOf(excludePatterns);
+        return this;
+      }
+
+      public UrlPattern build() {
+        checkArgument(!includePatterns.isEmpty() || !excludePatterns.isEmpty(), "Empty urls");
+        checkNoEmptyValue(includePatterns);
+        checkNoEmptyValue(excludePatterns);
+        return new UrlPattern(this);
+      }
+
+      private static void checkNoEmptyValue(Set<String> list) {
+        checkArgument(!list.stream().anyMatch(String::isEmpty), "Empty url");
+      }
+    }
+
+    static class Pattern {
+
+      enum Type {
+        MATCH_ALL, START_WITH_STAR, END_WITH_START, ANY
+      }
+
+      private String url;
+      private String urlToMatch;
+      private Type urlType;
+
+      Pattern(String url) {
+        this.url = url;
+        this.urlToMatch = url.replaceAll("/?\\*", "");
+        if (MATCH_ALL_URL.equals(url)) {
+          this.urlType = MATCH_ALL;
+        } else if (url.startsWith("*")) {
+          this.urlType = START_WITH_STAR;
+        } else if (url.endsWith("*")) {
+          this.urlType = END_WITH_START;
+        } else {
+          this.urlType = Type.ANY;
+        }
+      }
+
+      boolean matches(String path) {
+        switch (urlType) {
+          case MATCH_ALL:
+            return true;
+          case START_WITH_STAR:
+            return path.endsWith(urlToMatch);
+          case END_WITH_START:
+            return path.startsWith(urlToMatch);
+          default:
+            return path.equals(urlToMatch);
+        }
+      }
+
+      String getUrl() {
+        return url;
+      }
+    }
+
   }
 }
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/web/ServletFilterTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/web/ServletFilterTest.java
index 9c6eabc990a..08add30b511 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/web/ServletFilterTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/web/ServletFilterTest.java
@@ -19,34 +19,34 @@
  */
 package org.sonar.api.web;
 
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ExpectedException;
+import static org.assertj.core.api.Assertions.assertThat;
 
+import java.io.IOException;
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
-
-import java.io.IOException;
-
-import static org.assertj.core.api.Assertions.assertThat;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 public class ServletFilterTest {
   @Rule
   public ExpectedException thrown = ExpectedException.none();
 
   @Test
-  public void matchAll() {
+  public void include_all() {
     ServletFilter.UrlPattern pattern = ServletFilter.UrlPattern.create("/*");
     assertThat(pattern.matches("/")).isTrue();
     assertThat(pattern.matches("/foo/ooo")).isTrue();
-    assertThat(pattern.toString()).isEqualTo("/*");
+
+    assertThat(pattern.getIncludePatterns()).containsOnly("/*");
+    assertThat(pattern.getExcludePatterns()).isEmpty();
   }
 
   @Test
-  public void matchEndOfUrl() {
+  public void includeend_of_url() {
     ServletFilter.UrlPattern pattern = ServletFilter.UrlPattern.create("*foo");
     assertThat(pattern.matches("/")).isFalse();
     assertThat(pattern.matches("/hello/foo")).isTrue();
@@ -56,7 +56,7 @@ public class ServletFilterTest {
   }
 
   @Test
-  public void matchBeginningOfUrl() {
+  public void include_beginning_of_url() {
     ServletFilter.UrlPattern pattern = ServletFilter.UrlPattern.create("/foo/*");
     assertThat(pattern.matches("/")).isFalse();
     assertThat(pattern.matches("/foo")).isTrue();
@@ -65,7 +65,7 @@ public class ServletFilterTest {
   }
 
   @Test
-  public void matchExactUrl() {
+  public void include_exact_url() {
     ServletFilter.UrlPattern pattern = ServletFilter.UrlPattern.create("/foo");
     assertThat(pattern.matches("/")).isFalse();
     assertThat(pattern.matches("/foo")).isTrue();
@@ -74,6 +74,110 @@ public class ServletFilterTest {
   }
 
   @Test
+  public void reject_all() throws Exception {
+    ServletFilter.UrlPattern pattern = ServletFilter.UrlPattern.builder()
+      .setExcludePatterns("/*")
+      .build();
+    assertThat(pattern.matches("/")).isFalse();
+    assertThat(pattern.matches("/foo/ooo")).isFalse();
+  }
+
+  @Test
+  public void reject_end_of_url() {
+    ServletFilter.UrlPattern pattern = ServletFilter.UrlPattern.builder()
+      .setExcludePatterns("*foo")
+      .build();
+
+    assertThat(pattern.matches("/")).isTrue();
+    assertThat(pattern.matches("/hello/foo")).isFalse();
+    assertThat(pattern.matches("/hello/bar")).isTrue();
+    assertThat(pattern.matches("/foo")).isFalse();
+    assertThat(pattern.matches("/foo2")).isTrue();
+  }
+
+  @Test
+  public void reject_beginning_of_url() {
+    ServletFilter.UrlPattern pattern = ServletFilter.UrlPattern.builder()
+      .setExcludePatterns("/foo/*")
+      .build();
+
+    assertThat(pattern.matches("/")).isTrue();
+    assertThat(pattern.matches("/foo")).isFalse();
+    assertThat(pattern.matches("/foo/bar")).isFalse();
+    assertThat(pattern.matches("/bar")).isTrue();
+  }
+
+  @Test
+  public void reject_exact_url() {
+    ServletFilter.UrlPattern pattern = ServletFilter.UrlPattern.builder()
+      .setExcludePatterns("/foo")
+      .build();
+
+    assertThat(pattern.matches("/")).isTrue();
+    assertThat(pattern.matches("/foo")).isFalse();
+    assertThat(pattern.matches("/foo/")).isTrue();
+    assertThat(pattern.matches("/bar")).isTrue();
+  }
+
+  @Test
+  public void use_multiple_include_patterns() throws Exception {
+    ServletFilter.UrlPattern pattern = ServletFilter.UrlPattern.builder()
+      .setIncludePatterns("/foo", "/foo2")
+      .build();
+    assertThat(pattern.matches("/")).isFalse();
+    assertThat(pattern.matches("/foo")).isTrue();
+    assertThat(pattern.matches("/foo2")).isTrue();
+    assertThat(pattern.matches("/foo/")).isFalse();
+    assertThat(pattern.matches("/bar")).isFalse();
+  }
+
+  @Test
+  public void use_multiple_exclude_patterns() throws Exception {
+    ServletFilter.UrlPattern pattern = ServletFilter.UrlPattern.builder()
+      .setExcludePatterns("/foo", "/foo2")
+      .build();
+    assertThat(pattern.matches("/")).isTrue();
+    assertThat(pattern.matches("/foo")).isFalse();
+    assertThat(pattern.matches("/foo2")).isFalse();
+    assertThat(pattern.matches("/foo/")).isTrue();
+    assertThat(pattern.matches("/bar")).isTrue();
+  }
+
+  @Test
+  public void use_include_and_exclude_patterns() throws Exception {
+    ServletFilter.UrlPattern pattern = ServletFilter.UrlPattern.builder()
+      .setIncludePatterns("/foo/*", "/foo/lo*")
+      .setExcludePatterns("/foo/login", "/foo/logout", "/foo/list")
+      .build();
+    assertThat(pattern.matches("/")).isFalse();
+    assertThat(pattern.matches("/foo")).isTrue();
+    assertThat(pattern.matches("/foo/login")).isFalse();
+    assertThat(pattern.matches("/foo/logout")).isFalse();
+    assertThat(pattern.matches("/foo/list")).isFalse();
+    assertThat(pattern.matches("/foo/locale")).isTrue();
+    assertThat(pattern.matches("/foo/index")).isTrue();
+  }
+
+  @Test
+  public void exclude_pattern_has_higher_priority_than_include_pattern() throws Exception {
+    ServletFilter.UrlPattern pattern = ServletFilter.UrlPattern.builder()
+      .setIncludePatterns("/foo")
+      .setExcludePatterns("/foo")
+      .build();
+    assertThat(pattern.matches("/foo")).isFalse();
+  }
+
+  @Test
+  public void url_pattern_cannot_contain_empty_patterns() {
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("Empty urls");
+    ServletFilter.UrlPattern.builder()
+      .setExcludePatterns()
+      .setIncludePatterns()
+      .build();
+  }
+
+  @Test
   public void url_pattern_cant_be_empty() {
     thrown.expect(IllegalArgumentException.class);
     thrown.expectMessage("Empty url");
@@ -83,13 +187,30 @@ public class ServletFilterTest {
   @Test
   public void filter_should_return_url_pattern() {
     ServletFilter filter = new FakeFilter();
-    assertThat(filter.doGetPattern().getUrl()).isEqualTo("/fake");
+    assertThat(filter.doGetPattern().getIncludePatterns()).containsOnly("/fake");
+    assertThat(filter.doGetPattern().getExcludePatterns()).isEmpty();
   }
 
   @Test
   public void filter_should_apply_to_all_urls_by_default() {
     ServletFilter filter = new DefaultFilter();
-    assertThat(filter.doGetPattern().getUrl()).isEqualTo("/*");
+    assertThat(filter.doGetPattern().getIncludePatterns()).containsOnly("/*");
+    assertThat(filter.doGetPattern().getExcludePatterns()).isEmpty();
+  }
+
+  @Test
+  public void get_url() throws Exception {
+    assertThat(ServletFilter.UrlPattern.create("/*").getUrl()).isEqualTo("/*");
+  }
+
+  @Test
+  public void fail_to_get_url_when_building_pattern_with_many_urls() throws Exception {
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("this method is deprecated and should not be used anymore");
+    ServletFilter.UrlPattern.builder()
+      .setIncludePatterns("/foo/*", "/foo/lo*")
+      .setExcludePatterns("/foo/login", "/foo/logout", "/foo/list")
+      .build().getUrl();
   }
 
   static class FakeFilter extends ServletFilter {