beckn / protocol-server

protocol-server
5 stars 11 forks source link

CORS Issues #60

Closed sareenEminds closed 1 week ago

sareenEminds commented 8 months ago

Name: protocol-server enhancement Title: "CORS issues"

Description Currently there is a restriction on the protocol server to allow only the same domain to connect to the protocol server. To be able to enable all traffic to connect the protocol server should be able to accept all connections

Goals: To use '*' in the whitelisting to enable all traffic to connect to the protocol server

Expected Outcome: Protocol server should be able to accept all network connections

Project: Beckn

Organization Name: Beckn Open Collective

Domain: Others

Tech Skills Needed: • Understanding of protocol server

sreekanthmr commented 8 months ago

Name: protocol-server enhancement Title: "CORS issues"

Description Currently there is a restriction on the protocol server to allow only the same domain to connect to the protocol server. To be able to enable all traffic to connect the protocol server should be able to accept all connections

Goals: To use '*' in the whitelisting to enable all traffic to connect to the protocol server

Expected Outcome: Protocol server should be able to accept all network connections

Project: Beckn

Organization Name: Beckn Open Collective

Domain: Others

Tech Skills Needed: • Understanding of protocol server

We are also facing the same issue. Any update to this?

faizmagic commented 8 months ago

@sreekanthmr , this issue has been prioritised and the team is working on fixing the same. Will update you as soon as it is done.

CC: @shreyvishal @sareenEminds @vbabuEM @vishi24

shreyvishal commented 8 months ago

This issue has been resolved. We have tested this and was running. After testing can you please close this @faizmagic cc: @sareenEminds @faizmagic

sreekanthmr commented 8 months ago

This issue has been resolved. We have tested this and was running. After testing can you please close this @faizmagic cc: @sareenEminds @faizmagic

I updated the code and tried and still the same response. I also checked the Beckn BAP sandbox and the result is same

Access to XMLHttpRequest at 'https://ps-bap-client.becknprotocol.io/confirm' from origin 'http://localhost:8100/' has been blocked by CORS policy: Request header field appversion is not allowed by Access-Control-Allow-Headers in preflight response.

shreyvishal commented 8 months ago

This issue has been resolved.

Fix Details:

  1. We have used cors npm package and with that we have whitelisted all the urls and accepting requests from these url by using wild card entry "*" as allow origin. If you are using nginx as deployment tool then you have to also enable cors in nginx config.
  2. For proof of the fix is attached below. In that you can see the protocol-server is accepting the requests from other third party urls and processing their request. Protocol Server Logs

cc: @faizmagic @sareenEminds

sreekanthmr commented 8 months ago

This issue has been resolved.

Fix Details:

  1. We have used cors npm package and with that we have whitelisted all the urls and accepting requests from these url by using wild card entry "*" as allow origin. If you are using nginx as deployment tool then you have to also enable cors in nginx config.
  2. For proof of the fix is attached below. In that you can see the protocol-server is accepting the requests from other third party urls and processing their request. Protocol Server Logs

cc: @faizmagic @sareenEminds

  1. Installed cors npm package
  2. Implemented the cors related changes in app.ts
  3. Pure nodejs implementation without nginx

Using an Angular frontend make a call to protocol server bpp client and we get the same error Using postman the requests go through. I'm assuming the logs that you are seeing are from postman and we can see our own requests and responses from postman there.

vbabuEM commented 8 months ago

Sreekanth, can you add the OPTIONS and POST request/response headers and body for the operation. That should help in debugging the CORS issue. Theoretically, the OPTIONS preflight check should list the headers that need to be allowed and those headers will be allowed by the server. These two request traces might help identify the problem. @sreekanthmr

vbabuEM commented 8 months ago

@Shivani-Dhoke , @shreyvishal , In my opinion, a single line configuration of app.use(cors()) is sufficient to enable CORS the right way for protocol server. I dont think we are achieving any different in the current code with call to both app.options and app.use

sreekanthmr commented 8 months ago

@Shivani-Dhoke , @shreyvishal , In my opinion, a single line configuration of app.use(cors()) is sufficient to enable CORS the right way for protocol server. I dont think we are achieving any different in the current code with call to both app.options and app.use

Requests go through Postman and node but not from browser apps. Irrespective of the framework, when you make a request to the protocol server from a browser-based application, it fails. I have checked using vanilla http and axios.

sreekanthmr commented 8 months ago

@faizmagic. As you are aware cors errors are related to browser-originated requests and you cannot test them using node or postman. Here is a bare Angular app that I have created to test a request protocol server. Please use the same for your tests.

https://github.com/kuza-tech/cors-check/tree/master

Look in console log for cors error or when there is no error a response.

faizmagic commented 8 months ago

@vishi24 @shreyvishal , can you confirm if the necessary testing was done from browser, or it was tested via nodejs code. as sreekanth mentioned CORS issue happens only when http request is invoked directly from browser. Can you please test this again and fix if there are issues.

Please use the sample code provided by sreekanth

CC: @vbabuEM @sareenEminds

sreekanthmr commented 8 months ago

Looks like the cors issue is resolved. It was related to invalid redirect in preflight. Observed that though we are using http in the request, the location in the preflight headers is pointing to https. I changed the urls to https the cors errors have disappeared but I'm getting empty responses. This only happnes when I move to https. So, essentially there is no response from our bpp when pointing to https.

{ "context": { "ttl": "PT10M",

    "action": "search",
    "timestamp": "2024-02-05T07:14:18.182Z",
    "message_id": "15bf593b-d9a7-4864-8b68-05dfab6385a9",
    "transaction_id": "beb65a81-6967-4c12-baa4-f1c03896a2b9",
    "domain": "retail:1.1.0",
    "version": "1.1.0",
    "bap_id": "kuza-agrinet-bap.com",
    "bap_uri": "https://bap-network.kuza.one",
    "location": {
        "country": {
            "code": "IND"
        },
        "city": {
            "code": "std:080"
        }
    }
},
"responses": []

}

sreekanthmr commented 8 months ago

Now I'm getting the response. It was related to an issue in the certificate chain.

Cors issue resolved Here are is how I resolved it

  1. Installed cors package (npm i cors) and added app.use(cors()) to app.ts before routing. I'll add specific cors settings to restrict it to a specific methods and url before going to production.
  2. Still the issue was not resolved. My investigations revealed that preflight is failing with 307 redirection error. When I looked at the location under preflight response headers, I noticed that the url returned is https where as the url that I'm calling is http.
  3. So, requests to http urls might work from node but not browser.

SSL issue resolve Subsequent to cors fixes, I got https errors and here is how I resolved it

  1. To enable SSL, we need to do proxy pass from Nginx server to Node url and configure SSL
  2. Unlike Apache, Nginx has only one certificate attribute, which should include intermediate and bundle together. So, we need to chain the certificates in the correct order and link it from Nginx.
  3. By the way I tried proxy pass from Apache but it did not work reliably. So, when you have multiple domains to proxy pass, use Nginx instead of Apache
faizmagic commented 5 months ago

Recently tekidi team encountered CORS issue and their dev-ops team was able to fix the same. @vbabuEM , its important we document their fixes and approach taken by tekidi team, also further investigate if their is any code improvements which is required and hence re-opening this ticket

vbabuEM commented 1 week ago

Here is a mail from Tekdi on how they solved it.

HI @venkatesh.babu@eminds.ai I will share the updated nginx configuration file which solved the CORS issue and will also share previously used nginx configuration files here. Changes made in file named dragon-foods-bap-client.becknprotocol.io.conf

Wide-open CORS config for nginx

if ($request_method = 'OPTIONS') { add_header 'Access-Control-Allow-Origin' '*' always; add_header 'Access-Control-Allow-Credentials' 'true' always; add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS' always;

add_header 'Access-Control-Allow-Headers' '*' always;

add_header 'Access-Control-Allow-Headers' 'Authorization, Access-Control-Allow-Origin, Access-Control-Allow-Methods, X-Authorization, X-Compatibility-Mode, Content-Type, Accept,X-Auth,';

add_header 'Access-Control-Max-Age' 1728000;
add_header 'Content-Type' 'text/plain charset=UTF-8';
add_header 'Content-Length' 0;
return 204;

}

if ($request_method = 'POST') {

add_header 'Access-Control-Allow-Origin' '*';

add_header 'Access-Control-Allow-Credentials' 'true' always;

add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS' always;

add_header 'Access-Control-Allow-Headers' '$http_access_control_request_headers' always;

add_header 'Access-Control-Allow-Headers' 'Authorization, Access-Control-Allow-Origin, Access-Control-Allow-Methods, X-Authorization, X-Compatibility-Mode, Content-Type, Accept,X-Auth,';

add_header 'Access-Control-Expose-Headers' 'Authorization, Access-Control-Allow-Origin, Access-Control-Allow-Methods, X-Authorization, X-Compatibility-Mode, Content-Type, Accept,X-Auth,';

}

if ($request_method = 'GET') {

add_header 'Access-Control-Allow-Origin' '*';

add_header 'Access-Control-Allow-Credentials' 'true' always;

add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS' always;

add_header 'Access-Control-Allow-Headers' 'Authorization, Access-Control-Allow-Origin, Access-Control-Allow-Methods, X-Authorization, X-Compatibility-Mode, Content-Type, Accept,X-Auth,';

add_header 'Access-Control-Expose-Headers' 'Authorization, Access-Control-Allow-Origin, Access-Control-Allow-Methods, X-Authorization, X-Compatibility-Mode, Content-Type, Accept,X-Auth,';

}

  1. Here previously ($request_method = 'GET') and ($request_method = 'POST') are not hashed out but now in new file these are hashed out.
  2. Also in ($request_method = 'OPTIONS') This line is added : add_header 'Access-Control-Allow-Headers' 'Authorization, Access-Control-Allow-Origin, Access-Control-Allow-Methods, X-Authorization, X- Compatibility-Mode, Content-Type, Accept,X-Auth,'; I am sharing file with you dragon-foods-bap-client.becknprotocol.io.conf

thanks and regards.

vbabuEM commented 1 week ago

Most of the CORS issues we face is with wrong configuration of Nginx. Some people have large configurations which is mostly not needed. However if you do need it, please study the mail attached above and change it suitably.