betfair / cougar

Cougar is a framework for making building network exposed service interfaces easy.
http://betfair.github.io/cougar
Apache License 2.0
27 stars 18 forks source link

CORS support #87

Closed jorgemsrs closed 9 years ago

jorgemsrs commented 9 years ago

Cougar as a modern RPC machine should support CORS out of the box.

CORS stands for Cross-site HTTP requests and relates to the ability to interact with cross domain resources in a safe and standardised way. Our existing web apps are currently using data frames to do cross domain requests which is an insecure and hacky way to solve the problem. jsonp is insecure and clunky as well. cors is the modern and safe way to allow cross domain exchanges.

Additional references:

jorgemsrs commented 9 years ago

Using the baseline service

 curl -X GET http://0:8080/cougarBaseline/v2/propertyEcho?propertyName=cougar.client.http.uuidparentsheader

works as expected. When one runs

curl -X OPTIONS http://0:8080/cougarBaseline/v2/propertyEcho?propertyName=cougar.client.http.uuidparentsheader

the behaviour is somewhat unexpected for me because it succeeds as well. The extensions clearly defines GET as the verb to use (file BaselineService-extensions line 33) and this is where i find it confusing.

I'm thinking of a possible impl for cors to submit for your judgment and it would be following the lines of com.betfair.cougar.transport.jetty.GzipHandler. The issue with it is that it would hijack the OPTIONS verb for cors handling and would break the behaviour described above...

eswdd commented 9 years ago

What is the behaviour with OPTIONS?

If currently it behaves same as GET then this is a bug so we should be comfortable breaking it. On 10 Nov 2014 10:53, "Jorge Silva" notifications@github.com wrote:

Using the baseline service

curl -X GET http://0:8080/cougarBaseline/v2/propertyEcho?propertyName=cougar.client.http.uuidparentsheader

works as expected. When one runs

curl -X OPTIONS http://0:8080/cougarBaseline/v2/propertyEcho?propertyName=cougar.client.http.uuidparentsheader

the behaviour is somewhat unexpected for me because the extensions clearly defines GET as the verb to use (file BaselineService-extensions line 33).

I'm thinking of a possible impl for cors to submit for your judgment and it would be following the lines of com.betfair.cougar.transport.jetty.GzipHandler. The issue with it is that it would hijack the OPTIONS verb for cors handling and would break the behaviour described above...

— Reply to this email directly or view it on GitHub https://github.com/betfair/cougar/issues/87#issuecomment-62368764.

jorgemsrs commented 9 years ago

It executes similar code paths. OPTIONS brings an additional Vary header

With OPTIONS

curl -v -X OPTIONS http://0:8080/cougarBaseline/v2/propertyEcho?propertyName=cougar.client.http.uuidparentsheader
* Adding handle: conn: 0x7fc863803000
* Adding handle: send: 0
* Adding handle: recv: 0
* Curl_addHandleToPipeline: length: 1
* - Conn 0 (0x7fc863803000) send_pipe: 1, recv_pipe: 0
* About to connect() to 0 port 8080 (#0)
*   Trying 0.0.0.0...
* Connected to 0 (0.0.0.0) port 8080 (#0)
> OPTIONS /cougarBaseline/v2/propertyEcho?propertyName=cougar.client.http.uuidparentsheader HTTP/1.1
> User-Agent: curl/7.30.0
> Host: 0:8080
> Accept: */*
> 
< HTTP/1.1 200 OK
< Date: Mon, 10 Nov 2014 13:40:30 GMT
* Server Cougar 2 - 3.2-SNAPSHOT is not blacklisted
< Server: Cougar 2 - 3.2-SNAPSHOT
< Cache-Control: no-cache
< Content-Type: application/json
< Content-Length: 16
< 
* Connection #0 to host 0 left intact
"X-UUID-Parents"

With GET

curl -v -X GET http://0:8080/cougarBaseline/v2/propertyEcho?propertyName=cougar.client.http.uuidparentsheader
* Adding handle: conn: 0x7f8503803000
* Adding handle: send: 0
* Adding handle: recv: 0
* Curl_addHandleToPipeline: length: 1
* - Conn 0 (0x7f8503803000) send_pipe: 1, recv_pipe: 0
* About to connect() to 0 port 8080 (#0)
*   Trying 0.0.0.0...
* Connected to 0 (0.0.0.0) port 8080 (#0)
> GET /cougarBaseline/v2/propertyEcho?propertyName=cougar.client.http.uuidparentsheader HTTP/1.1
> User-Agent: curl/7.30.0
> Host: 0:8080
> Accept: */*
> 
< HTTP/1.1 200 OK
< Date: Mon, 10 Nov 2014 13:41:31 GMT
* Server Cougar 2 - 3.2-SNAPSHOT is not blacklisted
< Server: Cougar 2 - 3.2-SNAPSHOT
< Cache-Control: no-cache
< Content-Type: application/json
< Vary: Accept-Encoding, User-Agent
< Content-Length: 16
< 
* Connection #0 to host 0 left intact
"X-UUID-Parents"
jorgemsrs commented 9 years ago

a minimum working solution is open for discussion at #89. It has no tests. It should serve to see if the rationale is correctly adapted to the cougar reality.

eswdd commented 9 years ago

@bfmike - do you have any concerns/thoughts from a security perspective on adding CORS support to Cougar?

I suggest that once this is added we deprecate the data frame solution and remove it in Cougar 4 (whenever that occurs).

eswdd commented 9 years ago

All merged and tests passing.