[MGNLREST-275] Rest returning CORS error when should return 404 or 500 or not authorized. Created: 23/Sep/20  Updated: 23/Oct/23  Resolved: 12/Jan/21

Status: Closed
Project: Magnolia REST Framework
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Bartosz Staryga Assignee: Jaroslav Simak
Resolution: Obsolete Votes: 1
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File Screenshot 2020-09-23 at 09.03.13.png     PNG File Screenshot 2020-09-23 at 09.07.41.png     PNG File Screenshot 2020-12-04 at 14.45.27.png     PNG File Screenshot 2021-09-07 at 13.17.39.png     PNG File image-2021-09-07-14-34-53-662.png    
Issue Links:
Cloners
is cloned by PAGES-483 SPA cannot reach template endpoints b... Closed
relation
is related to MAGNOLIA-7969 CORS headers not added for unauthoriz... Closed
is related to MGNLSITE-103 CORS headers not added for unauthoriz... Closed
supersession
is superseded by MAGNOLIA-7969 CORS headers not added for unauthoriz... Closed
is superseded by MGNLSITE-103 CORS headers not added for unauthoriz... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Epic Link: Headless Phase 2
Sprint: HL & LD 17, HL & LD 18, HL & LD 19, HL & LD 20
Story Points: 3

 Description   

A delivery endpoint which is able to successfully return a response to a browser when there is no error in the request - should also successfully return an error response when a problem occurs. Currently, it does not. For some reason when the delivery endpoint returns an error message - such as in the case of a 404 or 500 - the browser shows a CORS error and does not return the error response as expected.

Details

When calling delivery rest endpoints and response is e.g.
"error":

{ "code": "notFound", "message": "Could not find resource for full path: http://127.0.0.1:8080/.rest/customer/journey/undefined?lang=en" }

or not authorized, or some 500 in rest call Magnolia returns CORS error.

It creates HUGE confusion in finding the real cause of error.
In couple cases blocked POC prospects for days as they did not knew what else to do.

CORS is correctly added naturally

Timeboxed for investigation: 3 SP



 Comments   
Comment by Bartosz Staryga [ 04/Dec/20 ]

jsimak to reproduce it in 6.2.5
1. Go to https://demo.magnolia-cms.com/
2. You should be able to open in browser normally https://demo.magnolia-cms.com/.rest/templateDefinition/v1/mtk:pages/basic
3. In Site app add to fallback: https://git.magnolia-cms.com/projects/DEMOS/repos/website-spa-demo/raw/_dev/config/cors.yaml?at=refs%2Fheads%2Fmaster enabling fully cors for test page you created.
4. Try to fetch same url as in point 2 from different domain - we get CORS errror. Reason is that rest-anonymous role does not have web access to that endpoint, not cors.
5. Add GET web access for rest-anonymous for /.rest/templateDefinition/v1/*
6. Do point 4 - now you can fetch it. So 4 was giving us cors error where it should give back unauthorized.

So issues is still there

Comment by Jaroslav Simak [ 04/Dec/20 ]

Workaround:

  • move new CORS filter before the uriSecurity.

Because user is not authorized by the uriSecurity, the request won't continue down the chain to process the CORS headers.
Solution would be to move the cors filter before uriSecurity, but CORS MEP states that the CORS filter is placed after uriSecurity.
I agree is confusing to see CORS error in the log, but there's also 401 printed in the console. Not sure how to handle this yet.

mgeljic Would you have any suggestions?

Comment by Mikaël Geljić [ 10/Dec/20 ]

We should rephrase/repurpose the ticket first:

  • URI security is the one bailing out, not delivery—400s/500s emitted by REST endpoints should get proper CORS response headers anyway.
  • pages-spa-rendering module should grant GET access to template-definitions endpoint for rest-anonymous by default.

Then about CORS filter placement:

Filter is placed after URISecurityFilter to prevent potential attacker from obtaining information on server capabilities description prior to potential attack.

iirc, preflight requests are not supposed to be behind authorization, so that would also speak for moving the CORS filter.
But according to the MEP concern about disclosure of capabilities, we should assess how much capabilities are exposed, and how much context-aware should preflight responses be?

  • on the one hand, CORS is not a security framework; allowing a method doesn't mean it will effectively be permitted.
  • on the other hand, the outcome can be resource/context dependent.
  • what types of attacks was the MEP concerned about? rather about misconfigurations, e.g. allowed-origin to '*'?

Filter is further placed after CacheFilter to allow caching of response specially in case of dynamic resolution of capabilities (unless `CORS filter` implementation signals that caching of response is not advisable (or has already passed its TTL)).

The cache concern should be secondary, should preflight requests be cached at all?

Comment by Jaroslav Simak [ 11/Dec/20 ]

My two cents:

  • I would not be concerned about security, the rule from MEP actually seems like security through obscurity, because one always can send actual CORS requests without first sending the pre-flight and actually try and guess what is allowed
  • Having CORS higher in the chain makes sense in order to support every CORS request
  • Caching might make sense and saves some time when resolving the OPTIONS headers, however, i would expect browsers to cache them based on cache response headers?

My final say is that we move CORS filter up in the filter chain.

Comment by Bartosz Staryga [ 07/Sep/21 ]

jsimakmdrapela in which Magnolia version this should be already reflected in Configuration app?
I just installed fresh latest and cors is still at the bottom:

Should I move CORS filter manually always?

Generated at Mon Feb 12 06:58:19 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.