SONAR-8247 Fix security headers

5 files changed, 48 insertions, 41 deletions

diff --git a/it/it-tests/src/test/java/it/Category4Suite.java b/it/it-tests/src/test/java/it/Category4Suite.java
index 9d9c8b61501..41c4373f731 100644
--- a/it/it-tests/src/test/java/it/Category4Suite.java
+++ b/it/it-tests/src/test/java/it/Category4Suite.java
@@ -29,7 +29,7 @@ import it.duplication.CrossProjectDuplicationsOnRemoveFileTest;
 import it.duplication.CrossProjectDuplicationsTest;
 import it.duplication.DuplicationsTest;
 import it.duplication.NewDuplicationsTest;
-import it.http.HttpHeadersTest;
+import it.serverSystem.HttpHeadersTest;
 import it.projectComparison.ProjectComparisonTest;
 import it.projectEvent.EventTest;
 import it.projectSearch.SearchProjectsTest;
diff --git a/it/it-tests/src/test/java/it/http/HttpHeadersTest.java b/it/it-tests/src/test/java/it/serverSystem/HttpHeadersTest.java
index bf06af57216..79f539a8a80 100644
--- a/it/it-tests/src/test/java/it/http/HttpHeadersTest.java
+++ b/it/it-tests/src/test/java/it/serverSystem/HttpHeadersTest.java
@@ -17,8 +17,7 @@
  * along with this program; if not, write to the Free Software Foundation,
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
-
-package it.http;
+package it.serverSystem;
 
 import com.google.common.base.Throwables;
 import com.sonar.orchestrator.Orchestrator;
@@ -77,6 +76,36 @@ public class HttpHeadersTest {
     assertCacheInBrowser(httpResponse);
   }
 
+  @Test
+  public void verify_security_headers_on_base_url() throws Exception {
+    verifySecurityHeaders(call(orchestrator.getServer().getUrl() + "/"));
+  }
+
+  @Test
+  public void verify_security_headers_on_ws() throws Exception {
+    verifySecurityHeaders(call(orchestrator.getServer().getUrl() + "/api/issues/search"));
+  }
+
+  @Test
+  public void verify_security_headers_on_ruby_ws() throws Exception {
+    verifySecurityHeaders(call(orchestrator.getServer().getUrl() + "/api/resources/index"));
+  }
+
+  @Test
+  public void verify_security_headers_on_images() throws Exception {
+    verifySecurityHeaders(call(orchestrator.getServer().getUrl() + "/images/logo.svg"));
+  }
+
+  @Test
+  public void verify_security_headers_on_css() throws Exception {
+    verifySecurityHeaders(call(orchestrator.getServer().getUrl() + "/css/sonar.css"));
+  }
+
+  @Test
+  public void verify_security_headers_on_js() throws Exception {
+    verifySecurityHeaders(call(orchestrator.getServer().getUrl() + "/js/bundles/main.js"));
+  }
+
   private static void assertCacheInBrowser(Response httpResponse) {
     CacheControl cacheControl = httpResponse.cacheControl();
     assertThat(cacheControl.mustRevalidate()).isFalse();
@@ -91,6 +120,16 @@ public class HttpHeadersTest {
     assertThat(cacheControl.noStore()).isTrue();
   }
 
+  /**
+   * SONAR-8247
+   */
+  private static void verifySecurityHeaders(Response httpResponse) {
+    assertThat(httpResponse.isSuccessful()).isTrue();
+    assertThat(httpResponse.headers().get("X-Frame-Options")).isEqualTo("SAMEORIGIN");
+    assertThat(httpResponse.headers().get("X-XSS-Protection")).isEqualTo("1; mode=block");
+    assertThat(httpResponse.headers().get("X-Content-Type-Options")).isEqualTo("nosniff");
+  }
+
   private static Response call(String url) {
     Request request = new Request.Builder().get().url(url).build();
     try {
diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/web/SecurityServletFilter.java b/server/sonar-server/src/main/java/org/sonar/server/platform/web/SecurityServletFilter.java
index a91d29e0262..c533f7e1ec1 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/platform/web/SecurityServletFilter.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/platform/web/SecurityServletFilter.java
@@ -56,7 +56,7 @@ public class SecurityServletFilter implements Filter {
       return;
     }
 
-    chain.doFilter(httpRequest, httpResponse);
+    // WARNING, headers must be added before the doFilter, otherwise they won't be added when response is already committed (for instance when a WS is called)
 
     // Clickjacking protection
     // See https://www.owasp.org/index.php/Clickjacking_Protection_for_Java_EE
@@ -69,6 +69,8 @@ public class SecurityServletFilter implements Filter {
     // MIME-sniffing
     // See https://www.owasp.org/index.php/List_of_useful_HTTP_headers
     httpResponse.addHeader("X-Content-Type-Options", "nosniff");
+
+    chain.doFilter(httpRequest, httpResponse);
   }
 
   @Override
diff --git a/server/sonar-web/src/main/webapp/WEB-INF/config/environment.rb b/server/sonar-web/src/main/webapp/WEB-INF/config/environment.rb
index cd529e41506..3c375e4607b 100644
--- a/server/sonar-web/src/main/webapp/WEB-INF/config/environment.rb
+++ b/server/sonar-web/src/main/webapp/WEB-INF/config/environment.rb
@@ -52,37 +52,6 @@ class EagerPluginLoader < Rails::Plugin::Loader
   end
 end
 
-
-#
-# Put response headers on all HTTP calls. This is done by the Java SecurityServlerFilter,
-# but for some reason Rack swallows the headers set on Java side.
-# See middleware configuration below.
-#
-class SecurityHeaders
-  def initialize(app)
-    @app = app
-  end
-
-  def call(env)
-    status, headers, body = @app.call(env)
-
-    # Clickjacking protection
-    # See https://www.owasp.org/index.php/Clickjacking_Protection_for_Java_EE
-    headers['X-Frame-Options']='SAMEORIGIN'
-
-    # Cross-site scripting
-    # See https://www.owasp.org/index.php/List_of_useful_HTTP_headers
-    headers['X-XSS-Protection']='1; mode=block'
-
-    # MIME-sniffing
-    # See https://www.owasp.org/index.php/List_of_useful_HTTP_headers
-    headers['X-Content-Type-Options']='nosniff';
-
-    [status, headers, body]
-  end
-end
-
-
 Rails::Initializer.run do |config|
   # Settings in config/environments/* take precedence over those specified here.
   # Application configuration should go into files in config/initializers
@@ -139,9 +108,6 @@ Rails::Initializer.run do |config|
   # Activate observers that should always be running
   # Please note that observers generated using script/generate observer need to have an _observer suffix
   # config.active_record.observers = :cacher, :garbage_collector, :forum_observer
-
-  # Add security related headers
-  config.middleware.use SecurityHeaders
 end
 
 
diff --git a/server/sonar-web/src/main/webapp/WEB-INF/web.xml b/server/sonar-web/src/main/webapp/WEB-INF/web.xml
index ab60f08440a..1a8aa2dc8ad 100644
--- a/server/sonar-web/src/main/webapp/WEB-INF/web.xml
+++ b/server/sonar-web/src/main/webapp/WEB-INF/web.xml
@@ -75,15 +75,15 @@
     <url-pattern>/*</url-pattern>
   </filter-mapping>
   <filter-mapping>
-    <filter-name>UserSessionFilter</filter-name>
+    <filter-name>SecurityFilter</filter-name>
     <url-pattern>/*</url-pattern>
   </filter-mapping>
   <filter-mapping>
-    <filter-name>ServletFilters</filter-name>
+    <filter-name>UserSessionFilter</filter-name>
     <url-pattern>/*</url-pattern>
   </filter-mapping>
   <filter-mapping>
-    <filter-name>SecurityFilter</filter-name>
+    <filter-name>ServletFilters</filter-name>
     <url-pattern>/*</url-pattern>
   </filter-mapping>
   <filter-mapping>