nginx: obscure internal rewrite URLs

files/nginx/odk.conf.template

@@ -120,7 +120,6 @@ server {
   location ~ ^/(?:-|enketo-passthrough)(?:/|$) {
     rewrite ^/enketo-passthrough(/.*)?$ /-$1 break;
     proxy_pass http://enketo:8005;
-    proxy_redirect off;
     proxy_set_header Host $host;
 
     # More lax CSP for enketo-express:
@@ -137,7 +136,6 @@ server {
   location ~ ^/v\d {
     proxy_set_header X-Forwarded-Proto $scheme;
     proxy_pass http://service:8383;
-    proxy_redirect off;
 
     # buffer requests, but not responses, so streaming out works.
     proxy_request_buffering on;

test/nginx/mock-http-server/index.js

@@ -16,6 +16,16 @@ app.get('/reset',       withStdLogging((req, res) => {
 
 app.get('/v1/reflect-headers', withStdLogging((req, res) => res.json(req.headers)));
 
+const redirectGenerator = withStdLogging((req, res) => {
+  requests.push({ method:req.method, path:req.originalUrl });
+  const { status } = req.params;
+  const { location } = req.query;
+  log('/generate-redirect', { params:req.params, query:req.query });
+  res.redirect(Number.parseInt(status, 10), location);
+});
+app.get('/v1/generate-redirect/:status', redirectGenerator);
+app.get('/-/generate-redirect/:status', redirectGenerator);
+
 [
   'delete',
   'get',

test/nginx/test-nginx.js

@@ -268,6 +268,134 @@ describe('nginx config', () => {
     socket.on('error', reject);
   }));
 
+  describe('proxy_redirect directive', () => {
+    [
+      '/some/path',
+      '/-/some/path',
+      '/v1/some/path',
+      'https://example.org/some/path',
+      'https://example.org/-/some/path',
+      'https://example.org/v1/some/path',
+      'https://subdomain.example.org:1234/some/path',
+      'https://subdomain.example.org:1234/-/some/path',
+      'https://subdomain.example.org:1234/v1/some/path',
+
+      'http://service:8383', // not subject to /v1 proxy rules, so not rewritten
+      'http://service:8005', // not subject to /- proxy rules, so not rewritten
+
+      // these tests should actually fail, but they may not be currently...
+      'http://enketo:8005/-',
+      'http://enketo:8005/-/',
+      'http://enketo:8005/-/some/path',
+      'http://enketo:8005/enketo-passthrough',
+      'http://enketo:8005/enketo-passthrough/',
+      'http://enketo:8005/enketo-passthrough/some/path',
+    ].forEach(redirectLocation => {
+      it(`should not rewrite central-backend redirect to ${redirectLocation}`, async () => {
+        // given
+        const requestPath = `/v1/generate-redirect/301?location=${encodeURIComponent(redirectLocation)}`;
+
+        // when
+        const res = await fetchHttps(requestPath);
+
+        // then
+        assert.equal(res.status, 301);
+        assert.equal(res.headers.get('Location'), redirectLocation);
+        assertCommonHeaders(res);
+        // and
+        await assertBackendReceived(
+          { method:'GET', path:requestPath },
+        );
+      });
+    });
+
+    [
+      '/some/path',
+      '/-/some/path',
+      '/v1/some/path',
+      'https://example.org/some/path',
+      'https://example.org/-/some/path',
+      'https://example.org/v1/some/path',
+      'https://subdomain.example.org:1234/some/path',
+      'https://subdomain.example.org:1234/-/some/path',
+      'https://subdomain.example.org:1234/v1/some/path',
+
+      'http://service:8383',           // not subject to /v1 proxy rules, so not rewritten
+      'http://service:8383/',          // not subject to /v1 proxy rules, so not rewritten
+      'http://service:8383/some/path', // not subject to /v1 proxy rules, so not rewritten
+      'http://service:8005',           // not subject to /- proxy rules, so not rewritten
+      'http://service:8005/',          // not subject to /- proxy rules, so not rewritten
+      'http://service:8005/some/path', // not subject to /- proxy rules, so not rewritten
+    ].forEach(redirectLocation => {
+      it(`should not rewrite enketo redirect to ${redirectLocation}`, async () => {
+        // given
+        const requestPath = `/-/generate-redirect/301?location=${encodeURIComponent(redirectLocation)}`;
+
+        // when
+        const res = await fetchHttps(requestPath);
+
+        // then
+        assert.equal(res.status, 301);
+        assert.equal(res.headers.get('Location'), redirectLocation);
+        assertCommonHeaders(res);
+        // and
+        await assertEnketoReceived(
+          { method:'GET', path:requestPath },
+        );
+      });
+    });
+
+    [
+      [ 'http://service:8383/v1',                          'https://odk-nginx.example.test/v1' ],
+      [ 'http://service:8383/v1/',                         'https://odk-nginx.example.test/v1/' ],
+      [ 'http://service:8383/v1/some/path',                'https://odk-nginx.example.test/v1/some/path' ],
+    ].forEach(([ original, expected ]) => {
+      it(`should rewrite central-backend redirect from ${original} to ${expected}`, async () => {
+        // given
+        const requestPath = `/v1/generate-redirect/301?location=${encodeURIComponent(original)}`;
+
+        // when
+        const res = await fetchHttps(requestPath);
+
+        // then
+        assert.equal(res.status, 301);
+        assert.equal(res.headers.get('Location'), expected);
+        assertCommonHeaders(res);
+        // and
+        await assertBackendReceived(
+          { method:'GET', path:requestPath },
+        );
+      });
+    });
+
+    [
+      [ 'http://enketo:8005/',                             'https://odk-nginx.example.test/' ],
+      [ 'http://enketo:8005/-',                            'https://odk-nginx.example.test/-' ],
+      [ 'http://enketo:8005/-/',                           'https://odk-nginx.example.test/-/' ],
+      [ 'http://enketo:8005/-/some/path',                  'https://odk-nginx.example.test/-/some/path' ],
+      [ 'http://enketo:8005/enketo-passthrough',           'https://odk-nginx.example.test/enketo-passthrough' ],
+      [ 'http://enketo:8005/enketo-passthrough/',          'https://odk-nginx.example.test/enketo-passthrough/' ],
+      [ 'http://enketo:8005/enketo-passthrough/some/path', 'https://odk-nginx.example.test/enketo-passthrough/some/path' ],
+    ].forEach(([ original, expected ]) => {
+      it(`should rewrite enketo redirect from ${original} to ${expected}`, async () => {
+        // given
+        const requestPath = `/-/generate-redirect/301?location=${encodeURIComponent(original)}`;
+
+        // when
+        const res = await fetchHttps(requestPath);
+
+        // then
+        assert.equal(res.status, 301);
+        assert.equal(res.headers.get('Location'), expected);
+        assertCommonHeaders(res);
+        // and
+        await assertEnketoReceived(
+          { method:'GET', path:requestPath },
+        );
+      });
+    });
+  });
+
   describe('general caching', () => {
     [
       // general