Cookies not being sent with `secure: true`

[jkuester]

Describe the bug
In our api code for sending cookies to the user we have some logic for setting the secure attribute. (This attribute controls whether the cookie is only allowed to be sent via HTTPS and not just HTTP.) Unfortunately, this logic depends on the value of the NODE_ENV environment variable being set to production. Otherwise, we default to secure: false. However, it seems that we are no longer setting the NODE_ENV value anywhere in our containers/config. So, I do not think secure is ever getting set to true (even in production).

(It is also worth noting that we need secure: false when running the server locally for cht-core development since that is via HTTP.)

To Reproduce
Deploy a CHT server production-style (e.g. via Docker Helper).

Run this curl command with your instance info:

curl -v -k -X POST https://192-168-1-20.local-ip.medicmobile.org:37581 -H 'Content-Type: application/json' -d '{"user":"medic","password":"password"}' 2>&1 | grep -i set-cookie

See that the result is:

< set-cookie: login=force; Path=/; SameSite=Lax

Expected behavior

The result of the curl command should include Secure;. Like this:

< set-cookie: login=force; Path=/; Secure; SameSite=La

Additional context

This secure cookie logic based on NODE_ENV was added a long time ago: ae75e27.

I found this issue while reviewing this PR which is updating the logging to not be based on NODE_ENV.

---

I am not exactly sure what the best solution here is. We need to be able to toggle the secure value on for production and off for development. I wonder if maybe we just make NODE_ENV=production into our api Dockerfile and call it good... 🤔 I suppose any time we are running in Docker, we are running with HTTPS, correct?

[mrjones-plip]

Yikes! good find though - thanks @jkuester !

This secure cookie logic based on NODE_ENV was added a long time ago: ae75e27.

while good to know (looks to be from 2.x days), maybe just as important to know when the regression was added? Otherwise we can add every Affects: label we have from 4.0 - 5.1 and call it a day 🤷

maybe we just make NODE_ENV=production into our api Dockerfile and call it good

secure by default seems a very sane/safe way to go - agreed! In turn, forcing insecure cookies sent over HTTP to be an explicit change you make for dev env (eg, via old school local setup) also seems safe/sane way to go.

in a quick code audit, it looks like process.env.NODE_ENV is referenced 3 times so shouldn't be too hard to ensure we're not breaking anything.

[mrjones-plip]

I was freaking out out for a bit there because I thought that we were not only missing Secure; flag in the cookie, but also missing HttpOnly; !! Turns out your POC curl calls above needed to hit the /medic/login URL which then shows the correct cookie payload (and four cookies, not one). I believe we only care about the first one (AuthSession), but worth auditing to see if we can ensure they all have Secure;

$ curl -v  -X POST https://192-168-68-26.local-ip.medicmobile.org:10449/medic/login -H 'Content-Type: application/json' -d '{"user":"medic","password":"password"}' 2>&1 | grep -i cook

< set-cookie: AuthSession=bWVkaW-SNIPPED!-KN0; Max-Age=31536000; Path=/; Expires=Wed, 24 Mar 2027 21:44:10 GMT; HttpOnly; SameSite=Lax
< set-cookie: userCtx=%7B%22name%22%3A%22medic%22%2C%22roles%22%3A%5B%22_admin%22%2C%22admin%22%5D%2C%22home%22%3A%22%2Fadmin%2F%22%7D; Max-Age=31536000; Path=/; Expires=Wed, 24 Mar 2027 21:44:10 GMT; SameSite=Lax
< set-cookie: login=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; SameSite=Lax
< set-cookie: locale=en; Max-Age=31536000; Path=/; Expires=Wed, 24 Mar 2027 21:44:10 GMT; SameSite=Lax

[mrjones-plip]

tested this back to 4.2 (we can't go further back), so I'll just add Affects: tags that far back.