search

jooby-project / jooby

The modular web framework for Java and Kotlin
https://jooby.io
Apache License 2.0
1.71k stars 196 forks source link

CORS Handler always throws 403 forbidden, when PUT or DELETE is used as HTTP Verb. #1413

Closed jerouris closed 5 years ago

jerouris commented 5 years ago

Found a possible back in CORS. When PUT or DELETE is used as HTTP Verb, cors will always throw as 403 forbidden. Here: isPreflight() will return false because it is not OPTIONS (https://github.com/jooby-project/jooby/blob/5bd3b293802651b84915cb3d14bdb71f0d5a23b8/jooby/src/main/java/io/jooby/CorsHandler.java#L112-L114) and isSimple() will return false (not POST, GET or HEAD) (https://github.com/jooby-project/jooby/blob/5bd3b293802651b84915cb3d14bdb71f0d5a23b8/jooby/src/main/java/io/jooby/CorsHandler.java#L83-L87) thus return 403, forbidden: (https://github.com/jooby-project/jooby/blob/5bd3b293802651b84915cb3d14bdb71f0d5a23b8/jooby/src/main/java/io/jooby/CorsHandler.java#L62-L81)

jknack commented 5 years ago

my understanding is that either you do a simple cors request (GET, POST, HEAD) or you need a preflight

jknack commented 5 years ago

Look at simple Requests: https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS

jerouris commented 5 years ago

According to Chrome network monitor, here is the preFlight (verb OPTIONS):

OPTIONS /api/v1/machines/123 HTTP/1.1 Host: localhost:8080 Connection: keep-alive Access-Control-Request-Method: PUT Origin: http://localhost:4200 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.97 Safari/537.36 DNT: 1 Access-Control-Request-Headers: authorization Accept: / Sec-Fetch-Site: same-site Sec-Fetch-Mode: cors Referer: http://localhost:4200/machines Accept-Encoding: gzip, deflate, br Accept-Language: el-GR,el;q=0.9,en;q=0.8

Which returns 200 OK (Correct)

and then the second (actual) request:

PUT /api/v1/machines/123 HTTP/1.1 Host: localhost:8080 Connection: keep-alive Content-Length: 0 Accept: application/json, text/plain, / Origin: http://localhost:4200 Authorization: Bearer eyJhbGciOiJSUzI1NiJ9.eyJqdGkiOiI4MTVlZjg1Yi1lZDMzLTQ0NTYtOTQxOC1kNjI4ZjRmNTRkYWIiLCJpYXQiOjE1NzM0Nzg4NDksInN1YiI6ImFkbWluaXN0cmF0b3IiLCJSb2xlIjoiQURNSU5JU1RSQVRPUiIsIkNvbXBhbnlJZCI6ImludGVyc3lzIn0.rgGRRJTOEsvC4ccfdeoL8PFfqPDc_H0aDc_Bw6GV2Y3bGSblmffasvQRkD-zPur_fcreFg2F5BIkS-4CPmAE-cpK-Wo4WEJEdw-HRDFKI2oMM9s87K1VQXY-hHs0FsCZDxr7-3t-MMskj9ZFAP24CEVSF26rFm73v_uMCighA2MgeBvVHPsIiAT-SXonIAIB2hOzbMUBW3iI1zj4MgYnwP6x9bbaQj1oitZftKyhtwGC9tCz_RPagw_hQle9xVTQ6UVK-1ZxWAejGrDws4toW_Zy5Wn9_ZFyQsFpdPb8mKADrovrPp-mIMEtlQ7HFfMcsEpjPiY9vyqj5wYohcSRKg User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.97 Safari/537.36 DNT: 1 Sec-Fetch-Site: same-site Sec-Fetch-Mode: cors Referer: http://localhost:4200/machines Accept-Encoding: gzip, deflate, br Accept-Language: el-GR,el;q=0.9,en;q=0.8

Which returns forbidden: HTTP/1.1 403 Forbidden Server: U Content-Type: text/plain Content-Length: 0 Date: Mon, 11 Nov 2019 13:57:03 GMT

jerouris commented 5 years ago

From MDN (Emphasis mine): The Cross-Origin Resource Sharing standard works by adding new HTTP headers that let servers describe which origins are permitted to read that information from a web browser. Additionally, for HTTP request methods that can cause side-effects on server data (in particular, HTTP methods other than GET, or POST with certain MIME types), the specification mandates that browsers "preflight" the request, soliciting supported methods from the server with the HTTP OPTIONS request method, and then, upon "approval" from the server, sending the actual request. Servers can also inform clients whether "credentials" (such as Cookies and HTTP Authentication) should be sent with requests.

jerouris commented 5 years ago

and here: https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#Examples_of_access_control_scenarios The preflight example

jknack commented 5 years ago

think the simple vs preflight handling is OK, but think we have a bug in the workflow/execution and we are not calling next.apply(), just sending an 403

can you look and send a PR?

jerouris commented 5 years ago

I would remove the isSimple() check. This is anyway a browser responsibility to decide when to create a pre-flight or not, according to MDN. Do you agree?

jknack commented 5 years ago

don't think so. do you know if this works in 1.x? I ported the code, so probably I missed something

jerouris commented 5 years ago

Yes, it worked properly in 1.x. I've found out while porting the App from Jooby 1 to Jooby 2

jknack commented 5 years ago

so yea, I missed something them (isSimple() is there in 1.x)

jerouris commented 5 years ago

Implementation is different in Jooby 1.x. Looks like there is no verb restriction in the non-preflight part https://github.com/jooby-project/jooby/blob/1b3dea6a7b4581d436ad71646ad0cd4bfb0c0e06/jooby/src/main/java/org/jooby/handlers/CorsHandler.java#L282-L307

jknack commented 5 years ago

merged your pull, add back simple cors handling (as optional thing) and handle normal options. Please try it and let me know if works OK

jerouris commented 5 years ago

Tested and it has an issue again, because of isSimple(). PUT, DELETE and other "non-simple" requests do not have the "Access-Control-Allow-Origin" header which makes them not acceptable by browsers.

From MDN: HTTP/1.1 200 OK Date: Mon, 01 Dec 2008 01:15:40 GMT Server: Apache/2 Access-Control-Allow-Origin: https://foo.example Vary: Accept-Encoding, Origin Content-Encoding: gzip Content-Length: 235 Keep-Alive: timeout=2, max=99 Connection: Keep-Alive Content-Type: text/plain