[CORE] Feat/issue 193 - Enable cors #201
Conversation
…o the routes through a decorator
| @@ -18,6 +19,11 @@ const fastify = Fastify({ | |||
| logger: !isTestEnv, | |||
| }); | |||
|
|
|||
| // Enable cors | |||
| fastify.register(require('@fastify/cors'), { | |||
| origin: configCORS.domain, | |||
There was a problem hiding this comment.
I would like to add these options
- credentials: this enables to share cookies in requests, I suggest to talk with other teams and know if that's the case this should set to true. Here more information
- methods: fastify-cors doesn't say how will behave if this parameter is omitted, so I think we should add it
allowHeaders: the default behavior is
reflecting the headers specified in the request's Access-Control-Request-Headers header.
Here you can find more information about it.
Is that what you want?
- optionSuccessStatus: to avoid to having conflict with default 204 code with legacy browsers
- exposedHeaders: this just if we create custom headers. Do you talk with the rest of cohort if someone is going to need that?
There was a problem hiding this comment.
thanks for the comments. i will read all the documentation to make a configuration closer to production.
There was a problem hiding this comment.
I was searching about optionSuccesStatus, I think you can omit that value since almost all browsers accept 204 value (default value). Here you can see 204 code compatibility with browsers
There was a problem hiding this comment.
For the moment I only set the credentials to true to activate the cookies.
But the other keys I think it is better that they stay like this so as not to put more barriers and interrupt the progress of the other squads.
I don't think it's feasible to change the key "domain" to list because our requirement is to use only one domain and not several.
There was a problem hiding this comment.
Okay, the documentation doesn't say about default behavior in that field, so I would recommend to add all the methods (to make a fast solution)
| const prefix = require('./environment'); | ||
|
|
||
| module.exports = { | ||
| domain: process.env[`${prefix}CORS_DOMAIN`], |
There was a problem hiding this comment.
Do you talk with our cohort about third services needed? If we need more that one, this value should be an array. In order to accomplish that we need to separate values with commas and use split function.
Here more information about creating an array using env vars
Enable CORS.
Resolve issue 193.