Remove wildcard from Access-Control-Allow-Origin header

[zsaltys]

Currently all graphql endpoints respond with header: Access-Control-Allow-Origin: *. This allows to make calls from other origins outside data.all domain. In reality this is very low or non existent security risk as * prevents passing credentials with cookies so it would be impossible to see anything sensitive unless a request was being made to something that is not protected by authentication. However our security team is still flagging this up and it should be a simple fix and probably a good practice to fix.

These are current places where this header is mentioned: https://github.com/search?q=repo%3Adata-dot-all%2Fdataall%20Access-Control-Allow-Origin&type=code

[noah-paige]

Thanks for opening this issue - should be a good security enhancement that we can look to pick up sometime after our v2.3 release

[zsaltys]

@mourya-33 maybe something you could pick up?

[noah-paige]

Hi @zsaltys @mourya-33 - it may prove challenging to implement the above with the ongoing development of dataall programmatic tools, including the CLI and SDK initiatives

With the above in mind - the origin of the API request to the api endpoint and associated back-end resources is not limited to a single domain / client origin

I also want to note that we protect the Cloudfront Domain, API GW endpoint, and Cognito User Pool (if applicable) with WAF rules to protect against common threats and known bad actors by default

[zsaltys]

@noah-paige I don't think SDK initiatives would care what the header of this value is only browsers should.. Also the SDK should always send the same origin the one we provide in the redirect. This also solves the issue of having origin filter on WAF for the api gateway.

[noah-paige]

Hi @zsaltys - for requests that would originate from SDK/CLI, I would be hesitant from enforcing a particular custom Origin header as it (1) is not the true origin of the request, (2) could open some additional security implications, (3) adds additional complexity and restrictions to the requests made from CLI/SDK, and (4) is best practice to omit this Origin header for non-browser requests.

That being said - if we want to pick up the following security enhancement I believe we can still do as follows to better our security posture:

• [1] Have a configurable way of restricting Access-Control-Allow-Origin to dataall domain
  • Or maybe can have some more intelligent way of restricting allowable origins to dataall domain only if Origin header is present (to note - the origin header is automatically added and cannot be modified programmatically via the browser (link))
• [2] For the client making requests from SDK/CLI have a general purpose way of setting custom headers that are particular for a individual's use case

cc: @petrkalos @anmolsgandhi @mourya-33

[zsaltys]

@noah-paige this would work us to make it configurable.. Then we put in our Access-Control-Allow-Origin the origin we expect... And in SDK we have to specify the origin as well because our WAF checks that the origin is set. So let's go with this approach and make the Access-Control-Allow-Origin configurable and it can be * by default.

[zsaltys]

@anmolsgandhi @mourya-33 it would really be very good to have this at least merged and ideally released before Sept 1st.

[anmolsgandhi]

@zsaltys Noted and it does align with the potential timeline in my mind as well. @noah-paige can you work with @mourya-33 based on the recommendation you provided and prioritize it?

[noah-paige]

@mourya-33 - would you have the bandwidth to pick up this enhancement for our next release v2.7? I can help work together for any review required as well

cc: @anmolsgandhi

[mourya-33]

I am on this already @noah-paige . Making this part of cdk.json as a configuration which defaults to *