Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable CORS access to EdgeX microservices #281

Closed
magallardo opened this issue Oct 15, 2019 · 16 comments 路 Fixed by edgexfoundry/edgex-go#3678 or #290
Closed

Enable CORS access to EdgeX microservices #281

magallardo opened this issue Oct 15, 2019 · 16 comments 路 Fixed by edgexfoundry/edgex-go#3678 or #290
Assignees
Labels
3-high priority denoting release-blocking issues enhancement New feature or request
Projects
Milestone

Comments

@magallardo
Copy link

Hello,

馃殌 Feature Request

Relevant Package

This feature request is for the API Gateway

Description

The Edgex components are exposed through REST APIs and those services are usually called from a UI running on a browser. Most browser prevent calls to services at a different origin (CORS) thus making it difficult the interactions with services like Edgex.
There are basically 2 ways to solve the CORS issue. The first one is on the client side (UI application) by using a proxy server. Unfortunately this solution is static and need to know before hand the back end servers.
The second option is to enable the backend server to allow CORS.

Describe the solution you'd like

Currently I have been bypassing this issue by using a proxy server at the client site. However, this solution is not working due to the nature of IoT projects and solutions. The IoT solutions tend to be very dynamic and in my specific example, the number of instances where the Edgex framework is deployed is dynamic and can change. That makes the proxy option irrelevant as that solution is static.

So, the preferred solution will be to allow the user to enable/disable CORS as desired on the API Gateway. That will allow calling the API from UI clients without requiring to use a proxy.

Describe alternatives you've considered

As described above, I am already using a proxy solution but it only works for a static number of Edgex installations.

Thanks in advance,
Marcelo

@tingyuz tingyuz self-assigned this Oct 15, 2019
@tingyuz
Copy link
Contributor

tingyuz commented Feb 4, 2020

For future reference, the current implementation of API gateway is based on KONG. From the document it seems KONG has limited support to enable CORS/cross-origin resource sharing.

https://docs.konghq.com/hub/kong-inc/cors/

@tingyuz
Copy link
Contributor

tingyuz commented Feb 10, 2020

A quick verification/band aid style fix is to run Curl command against KONG. Here is the command that may work:

curl -X POST http://localhost:8001/plugins --data "name=cors" --data "config.origins=*"  --data "config.methods=GET"  --data "config.methods=POST" --data "config.headers=Accept" --data "config.headers=Accept-Version" --data "config.headers=Content-Length" --data "config.headers=Content-MD5" --data "config.headers=Content-Type" --data "config.headers=Date" --data "config.headers=X-Auth-Token" --data "config.exposed_headers=X-Auth-Token" --data "config.credentials=true" --data "config.max_age=3600"

@aj-n
Copy link

aj-n commented Feb 3, 2021

If anyone else is looking for a solution to this, simply using * for origins did not work for me. Only when restricted to specific origins (with proper http or https AND port) did it work (i.e. config.origins=https://localhost:3000)

Additionally, you will need to configure the OPTIONS and other methods to be allowed on all the routes. I accomplished this with a simple loop in a script

ROUTES=(
    "coredata"
    "metadata"
    "command"
    "notifications"
    "scheduler"
    "rulesengine"
)

# Loop download each
for route in "${ROUTES[@]}"; do
    curl -X PATCH "http://localhost:8001/routes/${route}/" \
        --data "methods=OPTIONS" \
        --data "methods=GET" \
        --data "methods=POST" \
        --data "methods=PUT" \
        --data "methods=DELETE"
done

@lenny-goodell
Copy link
Member

Example utility for CORS from Intel reference implementation
https://github.com/intel-iot-devkit/automated-checkout-utilities/blob/0dd0e1e344399f3042b1f64b82ccbc2d2e0333aa/utilities.go#L69

We might be able to handle this in the middleware for REST

@jpwhitemn
Copy link
Member

Discussed in Core WG meeting 5/13/21. To be done for Jakarta release. Looking at whether there is common code (such as in bootstrapping) or if this needs to be added to edgex-go, SDKs (and associated services) separately as part of the request handling process. For Kong, we'll need to add the Kong plugin (https://docs.konghq.com/hub/kong-inc/cors/) - again for Jakarta

@bnevis-i
Copy link
Collaborator

It was decided in Security WG on 9/15/2021 that the project wants CORS support to be enabled even in the non-security use case. Persuant to this direction, we want to back out the changes for edgexfoundry/edgex-go#1913 and re-implement them in the EdgeX middleware instead of in Kong.

As such, we will back out commit edgexfoundry/edgex-go@25b03d0 of PR edgexfoundry/edgex-go#3678

The following configuration options should be added to the service to control the CORS headers:

EnableCORS = false
CORSAllowCredentials = false
CORSAllowedOrigins = "https://localhost"
CORSAllowedMethods = "GET, POST, PUT, DELETE"
CORSAllowedHeaders = "Content-Type"
CORSExposedHeaders = ""
CORSPreflightMaxAge = 3600

Actual real-life example from a Kong user:

    curl -X POST http://localhost:8001/plugins/ \
    --data "name=cors"  \
    --data "config.origins=http://yyy.yyy.yyy.yyy:4200" \
    --data "config.methods=GET" \
    --data "config.methods=POST" \
    --data "config.methods=OPTIONS" \
    --data "config.headers=Accept" \
    --data "config.headers=Accept-Version" \
    --data "config.headers=Content-Length" \
    --data "config.headers=Content-MD5" \
    --data "config.headers=Content-Type" \
    --data "config.headers=Date" \
    --data "config.headers=X-Auth-Token" \
    --data "config.headers=Authorization" \
    --data "config.exposed_headers=X-Auth-Token" \
    --data "config.credentials=true" \
    --data "config.max_age=3600" 

@bnevis-i bnevis-i reopened this Sep 16, 2021
@bnevis-i bnevis-i changed the title Enable CORS for API Gateway Enable CORS access to EdgeX microservices Sep 16, 2021
bnevis-i referenced this issue in edgexfoundry/edgex-go Sep 16, 2021
Reverts #3678 due to strategy change.  See #1913.

It was decided to move CORS functionality to the
microservice level so that it can be applied both
in the security-enabled as well as security-disabled
states.

Signed-off-by: Bryon Nevis <bryon.nevis@intel.com>
@bnevis-i bnevis-i reopened this Oct 6, 2021
@bnevis-i
Copy link
Collaborator

bnevis-i commented Oct 6, 2021

Middleware implementation should be done in https://github.com/edgexfoundry/go-mod-bootstrap and in C SDK

@lenny-goodell
Copy link
Member

More details on the proposed implementation.

  1. Add new CORS configuration settings above to ServiceInfo. Suggest sub-struct CorsInfo.
  2. Implement adding the CORS info (when enabled) to response header in the bootstrap handler here:
    https://github.com/edgexfoundry/go-mod-bootstrap/blob/main/bootstrap/handlers/httpserver.go#L97

@lenny-goodell lenny-goodell transferred this issue from edgexfoundry/edgex-go Oct 6, 2021
@lenny-goodell lenny-goodell added 3-high priority denoting release-blocking issues enhancement New feature or request labels Oct 6, 2021
@lenny-goodell lenny-goodell added this to the Jakarta milestone Oct 6, 2021
@cloudxxx8 cloudxxx8 added this to New Issues in Core WG via automation Oct 7, 2021
@JamesKButcher JamesKButcher moved this from New Issues to Release Backlog in Core WG Oct 7, 2021
@JamesKButcher
Copy link

@cloudxxx8 please take a look at the amount of work here

@cloudxxx8
Copy link
Member

it's not hard to add this implementation, but we need more time to test it manually
according to the current test infrastructure, we can't add automation tests for this because it needs multiple machines

@JamesKButcher
Copy link

On Core WG call we agreed to add PATCH to the CORSAllowedMethods

i.e.

CORSAllowedMethods = "GET, POST, PUT, PATCH, DELETE"

@cloudxxx8 and team, please aim to complete in the next week if possible

@cloudxxx8 cloudxxx8 moved this from Release Backlog to In progress in Core WG Oct 15, 2021
@cloudxxx8 cloudxxx8 moved this from Release Backlog to In progress in Security WG Oct 15, 2021
@bnevis-i bnevis-i moved this from In progress to QA/Code Review in Security WG Oct 20, 2021
@cloudxxx8
Copy link
Member

fixed by #286 and #288

Core WG automation moved this from In progress to Done Oct 21, 2021
Security WG automation moved this from QA/Code Review to Done Oct 21, 2021
@bnevis-i bnevis-i reopened this Oct 22, 2021
Core WG automation moved this from Done to New Issues Oct 22, 2021
Security WG automation moved this from Done to New Issues Oct 22, 2021
@bnevis-i
Copy link
Collaborator

bnevis-i commented Oct 22, 2021

Reopening the issue because I realized that we're not done. There is actually logic we need to implement on the server side.

https://www.html5rocks.com/en/tutorials/cors//#toc-cors-server-flowchart

Also need to set "Vary: Origin" when supporting CORS.

Note that the consul API does not currently expose CORS headers. There was a PR to add the feature, though it is pretty low-level: https://github.com/hashicorp/consul/pull/558/files

@cloudxxx8
Copy link
Member

@jpwhitemn @lenny-intel we might need to postpone the code freeze date for this issue

@cloudxxx8 cloudxxx8 moved this from New Issues to In progress in Core WG Oct 29, 2021
@bnevis-i
Copy link
Collaborator

bnevis-i commented Nov 8, 2021

Waiting for edgexfoundry/edgex-docs#608

@bnevis-i bnevis-i moved this from In progress to QA/Code Review in Security WG Nov 8, 2021
@jpwhitemn
Copy link
Member

Issue has been resolved, but believe just Docs work to be done. Per TSC 11/10/21 - close this

Core WG automation moved this from In progress to Done Nov 10, 2021
Security WG automation moved this from QA/Code Review to Done Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3-high priority denoting release-blocking issues enhancement New feature or request
Projects
Security WG
  
Done
8 participants