feat: Add allowed origins config

[fredcarle]

Relevant issue(s)

Resolves #1355

Description

This PR adds the option to set allowed origins for the HTTP API using the config file or a CLI flag.

Tasks

• I made sure the code is well commented, particularly hard-to-understand areas.
• I made sure the repository-held documentation is changed accordingly.
• I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
• I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

Unit tests and curl with allowedOrigins set to http://test.com

# this will result in an unsuccessful Preflight request
curl -i http://localhost:9181/api/v1/ping -H "Origin: http://no.com" -H "Access-Control-Request-Method: GET" -X OPTIONS

# this will result in an successful Preflight request
curl -i http://localhost:9181/api/v1/ping -H "Origin: http://test.com" -H "Access-Control-Request-Method: GET" -X OPTIONS

Specify the platform(s) on which this was tested:

• MacOS

[AndrewSisley]

Note: The default has been set to * which will allow all origins to send requests to the API. I feel like that's a good default for development but not for prod for obvious reasons. Do we want to be restrictive by default or open by default?

My vote would be for the opposite, and have it locked down by default. Forgetting (or not knowing about this) could be an expensive mistake. I also think that locked down by default is the norm (I remember having to mess with stuff like this a few times to enable CORS, but never to block it).

[codecov]

Codecov Report

Merging #1408 (78e3de1) into develop (f0d7fe0) will decrease coverage by 0.05%.
The diff coverage is 80.00%.

@@             Coverage Diff             @@
##           develop    #1408      +/-   ##
===========================================
- Coverage    72.07%   72.02%   -0.05%     
===========================================
  Files          185      185              
  Lines        18175    18185      +10     
===========================================
- Hits         13099    13098       -1     
- Misses        4043     4049       +6     
- Partials      1033     1038       +5     

Impacted Files	Coverage Δ
cli/start.go	61.07% <66.66%> (+0.17%)	⬆️
config/config.go	73.30% <100.00%> (+0.05%)	⬆️

... and 6 files with indirect coverage changes

[AndrewSisley]

LGTM. Approving assuming that the default value conversation is resolved before merged :)

[fredcarle]

My vote would be for the opposite, and have it locked down by default. Forgetting (or not knowing about this) could be an expensive mistake. I also think that locked down by default is the norm (I remember having to mess with stuff like this a few times to enable CORS, but never to block it).

I'm happy with that 👍

[shahzadlone]

suggestion:

How about just origins. Mostly because we don't have a blockedorigins or any other option like that. Also allowedorigins is less readable, perhaps allowedOrigins if you want to stick with the "allowed" word.

Suggested change

	defradb start --allowedorigins=https://yourdomain.com
	defradb start --origins=https://yourdomain.com

[fredcarle]

The thing with --origins is that it's a little vague and the http header is called With-Allowed-Origins. I feel like it makes more sense to have almost the full representation here as well. allowedOrigins could be nicer.

[AndrewSisley]

nitpick: I have a very minor preference for allowed-origins, but only if it we dont already camel case any other existing multi-word params/flags

[shahzadlone]

I do like allowed-origins, would prefer that over allowedOrigins

[orpheuslummis]

one idea is corsdomain, and perhaps alternatively using --api.corsdomain or http.corsdomain depending on which top-level key we want to represent the http system.

when determining defradb's parameters, we have to keep in mind that they have three points of determination: environment variables, config file, CLI command.

[fredcarle]

I'd rather we stay closer to what it represents. allowed-origins is quite good.

[orpheuslummis]

so the fully qualified key from the config's perspective would be api.allowed-origins, so perhaps it can be --allowed-origins in the CLI but as an env. variable it would be DEFRA_API_ALLOWED_ORIGINS perhaps. i'm not sure yet about if it's necessary to quote such a key with a - in Yaml - it might have to be

[api]
"allowed-origins": ...

[fredcarle]

I don't think it is according to what I see here https://www.cloudbees.com/blog/yaml-tutorial-everything-you-need-get-started

[shahzadlone]

LGTM