Reject CSP reports without an `effective-directive`

This is one of the most useful pieces of information. This
directive tells us exactly what the issue is. Without this, it's
impossible for us to put a human friendly name on a report.

src/sentry/interfaces/csp.py

@@ -45,12 +45,16 @@ class Csp(Interface):
     >>>     "document_uri": "http://example.com/",
     >>>     "violated_directive": "style-src cdn.example.com",
     >>>     "blocked_uri": "http://example.com/style.css",
+    >>>     "effective_uri": "style-src",
     >>> }
     """
     @classmethod
     def to_python(cls, data):
         kwargs = {k: trim(data.get(k, None), 1024) for k in REPORT_KEYS}
 
+        if kwargs['effective_directive'] is None:
+            raise InterfaceValidationError("'effective_directive' is missing")
+
         # Some reports from Chrome report blocked-uri as just 'about'.
         # In this case, this is not actionable and is just noisy.
         # Observed in Chrome 45 and 46.

tests/integration/fixtures/csp/firefox_blocked_asset_output.json

@@ -1,16 +0,0 @@
-{
-  "message": "CSP Violation: blocked-uri 'http://localhost:8000/lol.css'",
-  "data": {
-    "sentry.interfaces.User": {"ip_address": "127.0.0.1"},
-    "sentry.interfaces.Csp": {
-      "blocked_uri": "http://localhost:8000/lol.css",
-      "violated_directive": "style-src http://cdn.example.com",
-      "document_uri": "http://localhost:8000/",
-      "original_policy": "default-src 'none'; style-src http://cdn.example.com; report-uri http://requestb.in/1im8m061"
-    },
-    "sentry.interfaces.Http": {
-      "url": "http://localhost:8000/",
-      "headers": [["User-Agent", "awesome"]]
-    }
-  }
-}

tests/integration/tests.py

@@ -392,41 +392,25 @@ class DepdendencyTest(TestCase):
         self.validate_dependency(*DEPENDENCY_TEST_DATA['pylibmc'])
 
 
-def make_csp_test(f, func):
-    path = os.path.join(os.path.dirname(__file__), 'fixtures/csp', f)
-
-    def run(self):
+def get_fixtures(name):
+    path = os.path.join(os.path.dirname(__file__), 'fixtures/csp', name)
+    try:
         with open(path + '_input.json', 'rb') as fp1:
             input = fp1.read()
+    except IOError:
+        input = None
+
+    try:
         with open(path + '_output.json', 'rb') as fp2:
             output = json.load(fp2)
-        func(self, input, output)
-    return run
+    except IOError:
+        output = None
 
-
-class _CspReportTestGenerator(type):
-    def __new__(cls, name, bases, attrs):
-        test_cases = []
-        for k, v in attrs.copy().iteritems():
-            if k.startswith('test_') and callable(v):
-                test_cases.append((k, v))
-                attrs.pop(k)
-        for f in attrs['fixtures']:
-            for name, func in test_cases:
-                test = make_csp_test(f, func)
-                test.__name__ = '%s:%s' % (name, f)
-                attrs[test.__name__] = test
-        return super(_CspReportTestGenerator, cls).__new__(cls, name, bases, attrs)
+    return input, output
 
 
 class CspReportTest(TestCase):
-    __metaclass__ = _CspReportTestGenerator
-    fixtures = (
-        'chrome_blocked_asset',
-        'firefox_blocked_asset'
-    )
-
-    def test_successful(self, input, output):
+    def assertReportCreated(self, input, output):
         resp = self._postCspWithHeader(input)
         assert resp.status_code == 201, resp.content
         assert Event.objects.count() == 1
@@ -434,3 +418,14 @@ class CspReportTest(TestCase):
         Event.objects.bind_nodes([e], 'data')
         assert e.message == output['message']
         self.assertDictContainsSubset(output['data'], e.data.data, e.data.data)
+
+    def assertReportRejected(self, input):
+        resp = self._postCspWithHeader(input)
+        assert resp.status_code == 403, resp.content
+
+    def test_chrome_blocked_asset(self):
+        self.assertReportCreated(*get_fixtures('chrome_blocked_asset'))
+
+    def test_firefox_missing_effective_uri(self):
+        input, _ = get_fixtures('firefox_blocked_asset')
+        self.assertReportRejected(input)

tests/sentry/interfaces/test_csp.py

@@ -17,6 +17,7 @@ class CspTest(TestCase):
             document_uri='http://example.com',
             violated_directive='style-src cdn.example.com',
             blocked_uri='http://example.com/lol.css',
+            effective_directive='style-src',
         ))
 
     def test_path(self):
@@ -46,30 +47,35 @@ class CspTest(TestCase):
         result = Csp.to_python(dict(
             document_uri='http://example.com/foo',
             violated_directive='style-src http://cdn.example.com',
+            effective_directive='style-src',
         ))
         assert result.get_violated_directive() == ('violated-directive', 'style-src http://cdn.example.com')
 
         result = Csp.to_python(dict(
             document_uri='http://example.com/foo',
             violated_directive='style-src cdn.example.com',
+            effective_directive='style-src',
         ))
         assert result.get_violated_directive() == ('violated-directive', 'style-src http://cdn.example.com')
 
         result = Csp.to_python(dict(
             document_uri='https://example.com/foo',
             violated_directive='style-src cdn.example.com',
+            effective_directive='style-src',
         ))
         assert result.get_violated_directive() == ('violated-directive', 'style-src https://cdn.example.com')
 
         result = Csp.to_python(dict(
             document_uri='http://example.com/foo',
             violated_directive='style-src https://cdn.example.com',
+            effective_directive='style-src',
         ))
         assert result.get_violated_directive() == ('violated-directive', 'style-src https://cdn.example.com')
 
         result = Csp.to_python(dict(
             document_uri='blob:example.com/foo',
             violated_directive='style-src cdn.example.com',
+            effective_directive='style-src',
         ))
         assert result.get_violated_directive() == ('violated-directive', 'style-src blob:cdn.example.com')
 
@@ -77,7 +83,7 @@ class CspTest(TestCase):
         result = Csp.to_python(dict(
             document_uri='http://example.com/foo',
             blocked_uri='http://example.com/lol.css',
-            effective_directive='style-src'
+            effective_directive='style-src',
         ))
         assert result.get_culprit_directive() == ('blocked-uri', 'http://example.com/lol.css')
 
@@ -95,11 +101,6 @@ class CspTest(TestCase):
         ))
         assert result.get_culprit_directive() == ('blocked-uri', 'self')
 
-        result = Csp.to_python(dict(
-            document_uri='http://example.com/foo',
-        ))
-        assert result.get_culprit_directive() == ('effective-directive', '<unknown>')
-
     @patch('sentry.interfaces.csp.Csp.get_culprit_directive')
     @patch('sentry.interfaces.csp.Csp.get_violated_directive')
     def test_get_hash(self, get_culprit, get_violated):