fix(#8868): don't forward all headers for get requests

api/src/auth.js

@@ -1,15 +1,23 @@
-const request = require('request-promise-native');
+const rpn = require('request-promise-native');
 const _ = require('lodash');
 const db = require('./db');
 const environment = require('./environment');
 const config = require('./config');
 const { roles, users } = require('@medic/user-management')(config, db);
 
+const contentLengthRegex = /^content-length$/i;
+
 const get = (path, headers) => {
+  const getHeaders = { ...headers };
+  Object
+    .keys(getHeaders)
+    .filter(header => contentLengthRegex.test(header))
+    .forEach(header => delete getHeaders[header]);
+
   const url = new URL(path, environment.serverUrlNoAuth);
-  return request.get({
+  return rpn.get({
     url: url.toString(),
-    headers: headers,
+    headers: getHeaders,
     json: true
   });
 };
@@ -95,7 +103,7 @@ module.exports = {
     const authUrl = new URL(environment.serverUrlNoAuth);
     authUrl.username = username;
     authUrl.password = password;
-    return request.head({
+    return rpn.head({
       uri: authUrl.toString(),
       resolveWithFullResponse: true
     })

api/tests/mocha/auth.spec.js

@@ -1,14 +1,24 @@
-const request = require('request-promise-native');
+const rpn = require('request-promise-native');
 const url = require('url');
 const chai = require('chai');
 const sinon = require('sinon');
 const auth = require('../../src/auth');
 const config = require('../../src/config');
 const environment = require('../../src/environment');
 
+let req;
+
 describe('Auth', () => {
 
   beforeEach(() => {
+    req = {
+      headers: {
+        host: 'localhost:5988',
+        'user-agent': 'curl/8.6.0',
+        accept: '*/*',
+        'content-type': 'application/json',
+      },
+    };
     sinon.stub(environment, 'serverUrlNoAuth').get(() => 'http://abc.com');
   });
 
@@ -19,7 +29,7 @@ describe('Auth', () => {
   describe('check', () => {
 
     it('returns error when not logged in', () => {
-      const get = sinon.stub(request, 'get').rejects({ statusCode: 401 });
+      const get = sinon.stub(rpn, 'get').rejects({ statusCode: 401 });
       return auth.check({ }).catch(err => {
         chai.expect(get.callCount).to.equal(1);
         chai.expect(get.args[0][0].url).to.equal('http://abc.com/_session');
@@ -29,7 +39,7 @@ describe('Auth', () => {
     });
 
     it('returns error with incomplete session', () => {
-      const get = sinon.stub(request, 'get').resolves();
+      const get = sinon.stub(rpn, 'get').resolves();
       return auth.check({ }).catch(err => {
         chai.expect(get.callCount).to.equal(1);
         chai.expect(get.args[0][0].url).to.equal('http://abc.com/_session');
@@ -39,7 +49,7 @@ describe('Auth', () => {
     });
 
     it('returns error when no user context', () => {
-      const get = sinon.stub(request, 'get').resolves({ roles: [] });
+      const get = sinon.stub(rpn, 'get').resolves({ roles: [] });
       return auth.check({ }).catch(err => {
         chai.expect(get.callCount).to.equal(1);
         chai.expect(err.message).to.equal('Failed to authenticate');
@@ -48,7 +58,7 @@ describe('Auth', () => {
     });
 
     it('returns error when request errors', () => {
-      const get = sinon.stub(request, 'get').rejects({ error: 'boom' });
+      const get = sinon.stub(rpn, 'get').rejects({ error: 'boom' });
       return auth.check({ }).catch(err => {
         chai.expect(get.callCount).to.equal(1);
         chai.expect(get.args[0][0].url).to.equal('http://abc.com/_session');
@@ -58,7 +68,7 @@ describe('Auth', () => {
 
     it('returns error when it has insufficient privilege', () => {
       const userCtx = { userCtx: { name: 'steve', roles: [ 'xyz' ] } };
-      const get = sinon.stub(request, 'get').resolves(userCtx);
+      const get = sinon.stub(rpn, 'get').resolves(userCtx);
       sinon.stub(config, 'get').returns({ can_edit: ['abc'] });
       return auth.check({headers: []}, 'can_edit').catch(err => {
         chai.expect(get.callCount).to.equal(1);
@@ -69,7 +79,7 @@ describe('Auth', () => {
 
     it('returns username for admin', () => {
       const userCtx = { userCtx: { name: 'steve', roles: [ '_admin' ] } };
-      const get = sinon.stub(request, 'get').resolves(userCtx);
+      const get = sinon.stub(rpn, 'get').resolves(userCtx);
       return auth.check({headers: []}, 'can_edit').then(ctx => {
         chai.expect(get.callCount).to.equal(1);
         chai.expect(ctx.name).to.equal('steve');
@@ -78,7 +88,7 @@ describe('Auth', () => {
 
     it('returns username of non-admin user', () => {
       const userCtx = { userCtx: { name: 'laura', roles: [ 'xyz', 'district_admin' ] } };
-      const get = sinon.stub(request, 'get').resolves(userCtx);
+      const get = sinon.stub(rpn, 'get').resolves(userCtx);
       sinon.stub(config, 'get').returns({ can_edit: ['district_admin'] });
       return auth.check({headers: []}, 'can_edit').then(ctx => {
         chai.expect(get.callCount).to.equal(1);
@@ -89,7 +99,7 @@ describe('Auth', () => {
     it('accepts multiple required roles', () => {
       const userCtx = { userCtx: { name: 'steve', roles: [ 'xyz', 'district_admin' ] } };
       sinon.stub(url, 'format').returns('http://abc.com');
-      const get = sinon.stub(request, 'get').resolves(userCtx);
+      const get = sinon.stub(rpn, 'get').resolves(userCtx);
       sinon.stub(config, 'get').returns({
         can_export_messages: ['district_admin'],
         can_export_contacts: ['district_admin'],
@@ -103,7 +113,7 @@ describe('Auth', () => {
     it('checks all required roles', () => {
       const userCtx = { userCtx: { name: 'steve', roles: [ 'xyz', 'district_admin' ] } };
       sinon.stub(url, 'format').returns('http://abc.com');
-      const get = sinon.stub(request, 'get').resolves(userCtx);
+      const get = sinon.stub(rpn, 'get').resolves(userCtx);
       sinon.stub(config, 'get').returns({
         can_export_messages: ['district_admin'],
         can_export_server_logs: ['national_admin'],
@@ -116,4 +126,79 @@ describe('Auth', () => {
     });
   });
 
+  describe('getUserCtx', () => {
+    it('should return userCtx when authentication is successful', async () => {
+      sinon.stub(rpn, 'get').resolves({ userCtx: { name: 'user', roles: ['userrole'] }});
+
+      const result = await auth.getUserCtx(req);
+      chai.expect(result).to.deep.equal({ name: 'user', roles: ['userrole'] });
+      chai.expect(rpn.get.args).to.deep.equal([[{
+        url: 'http://abc.com/_session',
+        json: true,
+        headers: {
+          host: 'localhost:5988',
+          'user-agent': 'curl/8.6.0',
+          accept: '*/*',
+          'content-type': 'application/json',
+        },
+      }]]);
+    });
+
+    it('should clean content-length headers before forwarding', async () => {
+      sinon.stub(rpn, 'get').resolves({ userCtx: { name: 'theuser', roles: ['userrole'] }});
+
+      req.headers['content-length'] = 100;
+      req.headers['Content-Length'] = 22;
+      req.headers['Content-length'] = 44;
+      req.headers['content-Length'] = 82;
+
+      const result = await auth.getUserCtx(req);
+      chai.expect(result).to.deep.equal({ name: 'theuser', roles: ['userrole'] });
+      chai.expect(rpn.get.args).to.deep.equal([[{
+        url: 'http://abc.com/_session',
+        json: true,
+        headers: {
+          host: 'localhost:5988',
+          'user-agent': 'curl/8.6.0',
+          accept: '*/*',
+          'content-type': 'application/json',
+        },
+      }]]);
+    });
+
+    it('should throw a custom 401 error', async () => {
+      sinon.stub(rpn, 'get').rejects({ statusCode: 401, error: 'not logged in' });
+
+      await chai.expect(auth.getUserCtx(req)).to.be.rejected.and.eventually.deep.equal({
+        code: 401,
+        message: 'Not logged in',
+        err: { statusCode: 401, error: 'not logged in' }
+      });
+
+      chai.expect(rpn.get.callCount).to.equal(1);
+    });
+
+    it('should throw non-401 errors', async () => {
+      sinon.stub(rpn, 'get').rejects({ statusCode: 400, error: 'invalid' });
+
+      await chai.expect(auth.getUserCtx(req)).to.be.rejected.and.eventually.deep.equal({
+        statusCode: 400,
+        error: 'invalid'
+      });
+
+      chai.expect(rpn.get.callCount).to.equal(1);
+    });
+
+    it('should throw 500 when auth is invalid', async () => {
+      sinon.stub(rpn, 'get').resolves({ userCtx: { invalid: 'userctx' }});
+
+      await chai.expect(auth.getUserCtx(req)).to.be.rejected.and.eventually.deep.equal({
+        code: 500,
+        message: 'Failed to authenticate'
+      });
+
+      chai.expect(rpn.get.callCount).to.equal(1);
+    });
+  });
+
 });

tests/integration/haproxy/keep-alive-script/Dockerfile

@@ -0,0 +1,8 @@
+FROM alpine:3.19
+
+RUN apk add --update --no-cache curl
+
+COPY cmd.sh /
+RUN chmod +x /cmd.sh
+
+CMD ["/cmd.sh"]

tests/integration/haproxy/keep-alive-script/cmd.sh

@@ -0,0 +1,8 @@
+#!/bin/sh
+
+set -e
+
+data='{"user":"'$USER'","password":"'$PASSWORD'"}'
+
+curl -v POST 'http://api:5988/medic/login' -d $data -H 'Content-Type:application/json'
+

tests/integration/haproxy/keep-alive-script/docker-compose.yml

@@ -0,0 +1,15 @@
+version: '3'
+
+services:
+  login_call:
+    build: .
+    networks:
+      net:
+    environment:
+      - USER
+      - PASSWORD
+
+networks:
+  net:
+    name: cht-net-e2e
+    external: true

tests/integration/haproxy/keep-alive.spec.js

@@ -0,0 +1,45 @@
+const { spawn } = require('child_process');
+const path = require('path');
+const constants = require('@constants');
+
+const runDockerCommand = (command, params, env=process.env) => {
+  return new Promise((resolve, reject) => {
+    const cmd = spawn(command, params, { cwd: path.join(__dirname, 'keep-alive-script'), env });
+    const output = [];
+    const log = (data) => output.push(data.toString());
+    cmd.on('error', reject);
+    cmd.stdout.on('data', log);
+    cmd.stderr.on('data', log);
+    cmd.on('close', () => resolve(output.join(' ')));
+  });
+};
+
+const runScript = async () => {
+  const env = { ...process.env };
+  env.USER = constants.USERNAME;
+  env.PASSWORD = constants.PASSWORD;
+  return await runDockerCommand('docker-compose', ['up', '--build', '--force-recreate'], env);
+};
+const getLogs = async () => {
+  return await runDockerCommand('docker-compose', ['logs', '--no-log-prefix']);
+};
+
+describe('logging in through API directly', () => {
+  after(async () => {
+    await runDockerCommand('docker-compose', ['down', '--remove-orphans']);
+  });
+
+  it('should allow logins', async () => {
+    await runScript();
+    const logs = await getLogs();
+
+    console.log(logs);
+
+    expect(logs).to.not.include('HTTP/1.1 400 Bad Request');
+
+    expect(logs).to.include('HTTP/1.1 302 Found');
+    expect(logs).to.include('Connection: keep-alive');
+    expect(logs).to.include('Set-Cookie: AuthSession=');
+    expect(logs).to.include('Set-Cookie: userCtx=');
+  });
+});