Merge pull request #4023 from freedomofpress/3977-allow-delete-for-jo…

…urnalist-interface

Allow DELETE HTTP method for journalist interface

install_files/ansible-base/roles/app/templates/sites-available/journalist.conf

@@ -31,18 +31,10 @@ Header set X-Content-Type-Options: nosniff
 Header set X-Download-Options: noopen
 Header set X-Content-Security-Policy: "default-src 'self'"
 Header set Content-Security-Policy: "default-src 'self'"
-Header unset Etag
 
 # Limit the max submitted size of requests.
 LimitRequestBody 524288000
 
-#Redirect error pages to ensure headers are sent
-ErrorDocument 400 /notfound
-ErrorDocument 401 /notfound
-ErrorDocument 403 /notfound
-ErrorDocument 404 /notfound
-ErrorDocument 500 /error
-
 <Directory />
   Options None
   AllowOverride None
@@ -53,11 +45,11 @@ ErrorDocument 500 /error
 <Directory /var/www/>
   Options {{ apache_dir_options | default('None') }}
   AllowOverride None
-  <Limit GET POST HEAD>
+  <Limit GET POST HEAD DELETE>
     Order allow,deny
     allow from {{ securedrop_app_apache_allow_from }}
   </Limit>
-  <LimitExcept GET POST HEAD>
+  <LimitExcept GET POST HEAD DELETE>
     Order deny,allow
     Deny from all
   </LimitExcept>
@@ -66,11 +58,11 @@ ErrorDocument 500 /error
 <Directory /var/www/securedrop>
   Options {{ apache_dir_options | default('None') }}
   AllowOverride None
-  <Limit GET POST HEAD>
+  <Limit GET POST HEAD DELETE>
     Order allow,deny
     allow from {{ securedrop_app_apache_allow_from }}
   </Limit>
-  <LimitExcept GET POST HEAD>
+  <LimitExcept GET POST HEAD DELETE>
     Order deny,allow
     Deny from all
   </LimitExcept>

molecule/testinfra/staging/app/apache/test_apache_journalist_interface.py

@@ -16,7 +16,6 @@
   "Header set X-Content-Security-Policy: \"default-src 'self'\"",
   "Header set Content-Security-Policy: \"default-src 'self'\"",
   'Header set Referrer-Policy "no-referrer"',
-  'Header unset Etag',
 ]
 
 
@@ -51,11 +50,11 @@ def test_apache_headers_journalist_interface(host, header):
 <Directory /var/www/>
   Options None
   AllowOverride None
-  <Limit GET POST HEAD>
+  <Limit GET POST HEAD DELETE>
     Order allow,deny
     allow from {apache_allow_from}
   </Limit>
-  <LimitExcept GET POST HEAD>
+  <LimitExcept GET POST HEAD DELETE>
     Order deny,allow
     Deny from all
   </LimitExcept>
@@ -64,11 +63,11 @@ def test_apache_headers_journalist_interface(host, header):
 <Directory {securedrop_code}>
   Options None
   AllowOverride None
-  <Limit GET POST HEAD>
+  <Limit GET POST HEAD DELETE>
     Order allow,deny
     allow from {apache_allow_from}
   </Limit>
-  <LimitExcept GET POST HEAD>
+  <LimitExcept GET POST HEAD DELETE>
     Order deny,allow
     Deny from all
   </LimitExcept>

securedrop/journalist_app/api.py

@@ -309,7 +309,7 @@ def get_current_user():
         user = get_user_object(request)
         return jsonify(user.to_json()), 200
 
-    def _handle_http_exception(error):
+    def _handle_api_http_exception(error):
         # Workaround for no blueprint-level 404/5 error handlers, see:
         # https://github.com/pallets/flask/issues/503#issuecomment-71383286
         response = jsonify({'error': error.name,
@@ -318,6 +318,6 @@ def _handle_http_exception(error):
         return response, error.code
 
     for code in default_exceptions:
-        api.errorhandler(code)(_handle_http_exception)
+        api.errorhandler(code)(_handle_api_http_exception)
 
     return api

securedrop/tests/test_journalist.py

@@ -2024,3 +2024,9 @@ def test_does_set_cookie_headers(journalist_app, test_journo):
         observed_headers = response.headers
         assert 'Set-Cookie' in observed_headers.keys()
         assert 'Cookie' in observed_headers['Vary']
+
+
+def test_app_error_handlers_defined(journalist_app):
+    for status_code in [400, 401, 403, 404, 500]:
+        # This will raise KeyError if an app-wide error handler is not defined
+        assert journalist_app.error_handler_spec[None][status_code]

securedrop/tests/test_journalist_api.py

@@ -244,7 +244,16 @@ def test_user_without_token_cannot_post_protected_endpoints(journalist_app,
             assert response.status_code == 403
 
 
-def test_api_404(journalist_app, journalist_api_token):
+def test_api_error_handlers_defined(journalist_app):
+    """Ensure the expected error handler is defined in the API blueprint"""
+    for status_code in [400, 401, 403, 404, 500]:
+        result = journalist_app.error_handler_spec['api'][status_code]
+
+        expected_error_handler = '_handle_api_http_exception'
+        assert result.values()[0].__name__ == expected_error_handler
+
+
+def test_api_error_handler_404(journalist_app, journalist_api_token):
     with journalist_app.test_client() as app:
         response = app.get('/api/v1/invalidendpoint',
                            headers=get_api_headers(journalist_api_token))