CORS config parameter parsing

[simoami]

Hi @AnandChowdhary
It took me a while to get the app set up locally after the recent updates. The issue was that all xhr requests were failing because CORS couldn't be enabled even with proper config.

curl -v 'http://localhost:8080/v1/auth/login' -X OPTIONS -H 'Access-Control-Request-Method: POST'
--
< HTTP/1.1 200 OK
< X-DNS-Prefetch-Control: off
< X-Frame-Options: SAMEORIGIN
< Strict-Transport-Security: max-age=31536000; includeSubDomains; preload
< X-Download-Options: noopen
< X-Content-Type-Options: nosniff
< X-XSS-Protection: 1; mode=block
< X-Api-Version: 1.3.147
< X-RateLimit-Limit-Type: public
< X-RateLimit-Limit: 60
< X-RateLimit-Remaining: 59
< Date: Mon, 16 Mar 2020 17:36:24 GMT
< X-RateLimit-Reset: 1584380244
< Allow: POST
< Content-Type: text/html; charset=utf-8
< Content-Length: 4
< ETag: W/"4-Yf+Bwwqjx254r+pisuO9HfpJ6FQ"
< X-Response-Time: 6.500ms
< Connection: keep-alive

didn't return the necessary headers to allow requests to be sent from the UI.

My .env config:

# Remove CORS headers without API key
DISALLOW_OPEN_CORS = false

Upon close inspection, it looks like the value false was coming in as a string so it failed the condition here: https://github.com/staart/packages/blob/master/packages/server/index.ts#L52

Also a side issue during compilation:

☐  pending   Compiling TypeScript
.staart/src/__staart.ts(2,34): error TS7016: Could not find a declaration file for module 'regenerator-runtime'. '/Users/smoujami/projects/dukketta/staart/migration/api/.staart/node_modules/regenerator-runtime/runtime.js' implicitly has an 'any' type.
  Try `npm install @types/regenerator-runtime` if it exists or add a new declaration (.d.ts) file containing `declare module 'regenerator-runtime';`
✔  success   Listening on 8080

[AnandChowdhary]

Hi @simoami, thanks for opening the issue! If this is the case, I should handle other boolean environment variables too, like DISABLE_HELMET below it (thought they work right now because !"true" is false, but !"false" is not true), I'll fix it to be more precise.

The TypeScript warning is not an error, so it should compile, but that's something already on my todo and I'm going to fix it ASAP.

[AnandChowdhary]

For now, if you remove the environment variable, it should work, because !undefined and !"" are both true, so CORS will be injected. This can be a temporary fix until I deploy an update to @staart/server.

[AnandChowdhary]

I've fixed this, but I'll also check the codebase for other environment variables booleans.

[AnandChowdhary]

I'll open another issue for the TypeScript warning so we can check on it

[simoami]

Thank you! I should point out that this was working before. Perhaps dotenv isn't parsing properly?

…

On Mon, Mar 16, 2020, 2:15 PM Anand Chowdhary ***@***.***> wrote: I've fixed this, but I'll also check the codebase for other environment variables booleans. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1046 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAPRT6DQQPIACGGENB7R4XTRHZUEVANCNFSM4LMP5JNQ> .

[AnandChowdhary]

Maybe something to do with the fact that it was in @staart/api before, and is now in a separate @staart/server package and the .env was directly used with process.env.VAR instead of from the config.ts file which had smart fallbacks.

If anything similar doesn't work again, I'll reopen this issue, but we should be good to go for now.

[simoami]

Ah got it. That must be it then. Thanks for fixing