nginx: Restructure how we manage uploaded file routes.

The overall goal of this change is to fix an issue where on Ubuntu
Trusty, we were accidentally overriding the configuration to serve
uploads from disk with the regular expressions for adding access
control headers.

However, while investigating this, it became clear that we could
considerably simplify the mental energy required to understand this
system by making the uploads-route file be unconditionally available
and included from `zulip-include/app` (which means the zulip_ops code
can share behavior here).

We also move the Access-Control-Allow-* headers to a separate include
file, to avoid duplicating it in 5 places.  Fixing this duplication
discovered a potential bug in the settings used for Tornado, where
DELETE was not allowed on a route that definitely expects DELETE.

Fixes #11758.

puppet/zulip/files/nginx/zulip-include-common/api_headers

@@ -0,0 +1,3 @@
+add_header Access-Control-Allow-Origin *;
+add_header Access-Control-Allow-Headers Authorization;
+add_header Access-Control-Allow-Methods 'GET, POST, DELETE, PUT, PATCH, HEAD';

puppet/zulip/files/nginx/zulip-include-frontend/app

@@ -27,15 +27,9 @@ location ~ /json/events {
 
 # Send longpoll requests to Tornado
 location /api/v1/events {
-
+    include /etc/nginx/zulip-include/api_headers;
-    add_header Access-Control-Allow-Origin *;
-    add_header Access-Control-Allow-Headers Authorization;
-    add_header Access-Control-Allow-Methods 'GET, POST';
 
     if ($request_method = 'OPTIONS') {
-        add_header Access-Control-Allow-Origin *;
-        add_header Access-Control-Allow-Headers Authorization;
-        add_header Access-Control-Allow-Methods 'GET, POST';
         add_header 'Content-Type' 'text/plain charset=UTF-8';
         add_header 'Content-Length' 0;
         return 204;
@@ -60,15 +54,20 @@ location / {
     uwsgi_pass django;
 }
 
-# Certain Django routes not under /api are shared between mobile and
+# These Django routes not under /api are shared between mobile and
-# web and thus need API headers added.  We don't collapse this with the
+# web, and thus need API headers added.  We can't easily collapse
-# above block for /events, because regular expressions take priority over
+# these blocks with the /api block, because regular expressions take
-# paths in nginx's order-of-operations, and we don't want to override the
+# priority over paths in nginx's order-of-operations, and we don't
-# tornado stuff.
+# want to override the tornado configuration for /api/v1/events.  The
-location ~ ^/(user_uploads|avatar|thumbnail)/ {
+# last is handled via uploads-route.
-    add_header Access-Control-Allow-Origin *;
+location /thumbnail {
-    add_header Access-Control-Allow-Headers Authorization;
+    include /etc/nginx/zulip-include/api_headers;
-    add_header Access-Control-Allow-Methods 'GET, POST, DELETE, PUT, PATCH, HEAD';
+
+    include uwsgi_params;
+    uwsgi_pass django;
+}
+location /avatar {
+    include /etc/nginx/zulip-include/api_headers;
 
     include uwsgi_params;
     uwsgi_pass django;
@@ -76,12 +75,11 @@ location ~ ^/(user_uploads|avatar|thumbnail)/ {
 
 # Send all API routes not covered above to Django via uWSGI
 location /api/ {
-    add_header Access-Control-Allow-Origin *;
+    include /etc/nginx/zulip-include/api_headers;
-    add_header Access-Control-Allow-Headers Authorization;
-    add_header Access-Control-Allow-Methods 'GET, POST, DELETE, PUT, PATCH, HEAD';
 
     include uwsgi_params;
     uwsgi_pass django;
 }
 
+include /etc/nginx/zulip-include/uploads.route;
 include /etc/nginx/zulip-include/app.d/*.conf;

puppet/zulip/files/nginx/zulip-include-maybe/uploads-route.direct

@@ -1,4 +1,10 @@
+# This Django route not under /api is shared between mobile and web
+# and thus needs API headers added, in addition to the configuration
+# required to have it serve files directly.
+
 location /user_uploads {
+    include /etc/nginx/zulip-include/api_headers;
+
     add_header X-Content-Type-Options nosniff;
     add_header Content-Security-Policy "default-src 'none'; style-src 'self' 'unsafe-inline'; img-src 'self'; object-src 'self'; plugin-types application/pdf;";
     include /etc/nginx/zulip-include/uploads.types;

puppet/zulip/files/nginx/zulip-include-maybe/uploads-route.internal

@@ -5,3 +5,14 @@ location /serve_uploads {
     include /etc/nginx/zulip-include/uploads.types;
     alias /home/zulip/uploads/files;
 }
+
+# This Django route not under /api is shared between mobile and web
+# and thus needs API headers added, in addition to the configuration
+# required to have this URL be served by Django.
+
+location /user_uploads {
+    include /etc/nginx/zulip-include/api_headers;
+
+    include uwsgi_params;
+    uwsgi_pass django;
+}

puppet/zulip/files/nginx/zulip-include-maybe/uploads-route.noserve

@@ -0,0 +1,10 @@
+# This Django route not under /api is shared between mobile and web
+# and thus needs API headers added, in addition to the configuration
+# required to have this URL be served by Django.
+
+location /user_uploads {
+    include /etc/nginx/zulip-include/api_headers;
+
+    include uwsgi_params;
+    uwsgi_pass django;
+}

puppet/zulip/manifests/nginx.pp

@@ -41,6 +41,11 @@ class zulip::nginx {
     'trusty' => 'puppet:///modules/zulip/nginx/zulip-include-maybe/uploads-route.direct',
     default  => 'puppet:///modules/zulip/nginx/zulip-include-maybe/uploads-route.internal',
   }
+  $no_serve_uploads = zulipconf('application_server', 'no_serve_uploads', '')
+  if $no_serve_uploads != '' {
+    # If we're not serving uploads locally, set the appropriate API headers for it.
+    $uploads_route = 'puppet:///modules/zulip/nginx/zulip-include-maybe/uploads-route.noserve'
+  }
 
   file { '/etc/nginx/zulip-include/uploads.route':
     ensure  => file,

puppet/zulip/templates/nginx/zulip-enterprise.template.erb

@@ -37,7 +37,4 @@ server {
 
     include /etc/nginx/zulip-include/certbot;
     include /etc/nginx/zulip-include/app;
-<% if @no_serve_uploads == '' -%>
-    include /etc/nginx/zulip-include/uploads.route;
-<% end -%>
 }