apigee / apigeelint

Static code analysis for Apigee proxy bundles to encourage API developers to use best practices and avoid anti-patterns.
Apache License 2.0
91 stars 71 forks source link

apigeelint warning for CORS - PO032 #421

Closed mj-tirupathi closed 5 months ago

mj-tirupathi commented 6 months ago

Hi,

We had to add a CORS policy in our proxy and there's a configuration, "{request.header.origin}" in the policy.

We had to run apigeelint for the proxy using the command, 'apigeelint -f table.js --profile apigeex -s path/to/proxy'. When we ran the lint, it was throwing a warning, "Using {request.header.origin} in AllowOrigins defeats the purpose of CORS."

We've a few queries now and could you please help us with the below?

Thanks in advance! Manoj Sai T.

priyanka212 commented 6 months ago

@ssvaidyanathan Please revert to this. It will be helpful.

ssvaidyanathan commented 6 months ago

@priyanka212 / @mj-tirupathi - Check the caution in our public docs regarding the usage of wildcard *. When you use request.header.origin - you are basically allowing any origin header.

It defeats the CORS purpose by setting that value. If you know the list of origins, you should list those or populate that in a variable and use that within the policy.

Its not best practice, hence the tool is throwing a warning. You can ignore it if you dont want to change. Its just a warning

aramkrishna commented 6 months ago

@ssvaidyanathan I faced similar issue and see this comment now, My understanding is overall CORS, the Access-Control-Allow-Origin header is used to control which origins are allowed to access a resource from a different domain.

{**request.header.origin**}: to my knowledge, this syntax suggests that the value of the Access-Control-Allow-Origin header is dynamically set to the value of the Origin header in the incoming request. _This means that the server will respond with the exact origin that made the request, allowing only that specific origin to access the resource. To my understanding, this approach enables more fine-grained control over the allowed origins or comma separated ones. But in case of *: indicates that the value of the Access-Control-Allow-Origin header is set to a wildcard "*", which means that any origin is allowed to access the resource. This approach allows unrestricted access to the resource from any domain, which may be useful in certain scenarios but may also introduce potential security risks. Overall error was "Using {request.header.origin} in AllowOrigins defeats the purpose of CORS." but this error should only throw for * and not sure, why we get it for {**request.header.origin**} ? {**request.header.origin**} This means that the server will respond with the exact origin that made the request, allowing only that specific origin to access the resource. To my understanding, this approach enables more fine-grained control over the allowed origins or comma separated ones & i believe this is approach good for CORS and will work even if i move from say dev to higher envs without any changes, but for comma separated it may change hence this option is more safe (than * or comma separated option). Please educate if missing anything or overlooked ?
ssvaidyanathan commented 6 months ago

Adding @DinoChiesa to this thread as well

@aramkrishna - yes you are right. * is not encouraged at all.

Regarding request.header.origin, you can still initiate requests from many different hosts which is still open. As the spec suggests www.example.com, example.com, example.com:8080 all are different origins. Do you want it to enable for all those just because you have it configured with request.header.origin?

DinoChiesa commented 6 months ago

{request.header.origin} This means that the server will respond with the exact origin that made the request, allowing only that specific origin to access the resource. To my understanding, this approach enables more fine-grained control over the allowed origins

If you use {request.header.origin} in your CORS policy, within the <AllowOrigins> element, you're basically telling Apigee to allow cross-origin requests originating from any domain that inquires. The browser always injects the origin header into the cross-origin request (preflight or simple). The value of that origin is accessible in the context variable request.header.origin. It's the responsibility of the server to respond with a set of allowed origins in the CORS-related response headers. By using {request.header.origin} inside the <AllowOrigins> element, you're telling Apigee: allow the origin that was sent in. If Apigee always responds "the origin you're using, is fine", then there is no restriction at all, is there? It's not fine-grained control. It's a blanket "accept any origin" approach.

i believe this is approach good for CORS and will work even if i move from say dev to higher envs without any changes, but for comma separated it may change hence this option is more safe (than * or comma separated option). Please educate if missing anything or overlooked ?

If by "good for CORS and will work", you mean to say "Apigee will allow all cross-origin requests", then I agree. If you want to use CORS to actually restrict some traffic, then you should not use {request.header.origin} inside the <AllowOrigins> element.

ssvaidyanathan commented 5 months ago

Closing this issue as its not related to the plugin