POST request from mobile app never allowed

[Martimiz]

ISSUE:
I'm building a mobile Flutter app that uses Silverstripe GraphQL to pull data via a POST request. Once I enable CORS, the request from the app will always fail because the app doesn't send an Origin header.

Please note that the app doesn't send a preflight check, as it isn't a browser, just a plain POST request. But the Silverstripe GraphQL Controller seems to handle a POST request in a similar manner to a preflight check, by calling the addCorsHeaders() function for it, requiring an Origin.

So the only way to to get through is manually adding an Origin Header to the POST, but afaik this is not according to the rules, and could even be considered a spoofing attempt, if a take the app to the app store...

https://developer.mozilla.org/en-US/docs/Glossary/Forbidden_header_name

As far as I understand it by now, CORS is purely a browser thing, and handled in preflight. So I think restricting the actual POST request might maybe be handled separately, but at least the Origin thing can hopefully be tackled...

Thanks, Martine

Acceptance criteria

• Can do GraphQL request without Origin header.
• Security concerns are explicitly considered during peer review.

PRs

• FIX: Don't 403 error when origin is on CORS-disallowed (fixes #519) #527

[blueo]

Yeah I tend to agree here. The preflight request and validation is a browser concern so I suspect there should be some way to make a straight POST without a 'fake' origin.

[Martimiz]

Guys, maybe this just isn’t a priority? Maybe noone will ever build an app as a website companion using SilverStripe GraphQL, except for me?

I know I can just override the GraphQL controller, and I’ll probably have to, but I’m kind of curious as to the why 😉

[maxime-rainville]

Had a glance. I suspect we would just need to remove these lines Controller::validateOrigin.

silverstripe-graphql/src/Controller.php

Lines 234 to 236 in 6b4c9a2

	if (empty($allowedOrigins) || empty($origin)) {
	return false;
	}

And maybe tweak this line in Controller::addCorsHeaders to handle null origin.

silverstripe-graphql/src/Controller.php

Line 197 in 6b4c9a2

$response->addHeader('Access-Control-Allow-Origin', $origin);

Given that I don't quite know what I'm doing and that this touches some key security feature, I'm not quite ready to just jump in there and make the change. But I would be keen to support this use case.

[kinglozzer]

I’m not sure why we check CORS server side at all, isn’t the point of CORS that it’s a set of headers for the browser to check? I.e. it protects the site making the requests, not the site receiving them.

The OPTIONS request outputs a list of valid hosts which an “attacker” (I’m using that term loosely here!) could use to set the Referer header manually, so I don’t see any security benefit in checking the origin headers server side.

[michalkleiner]

I would probably agree that the server doesn't need to check CORS headers. The server sends them, as instructed by the dev via config, to tell the browser what origins it can load content for your site/app.
It'd be up to the webserver configuration to which requests it responds, what domains are the requestors trying to reach and whether it services them or not. The Origin header can be spoofed.

So I would say keep adding them as per the configuration (if not null), but we probably don't need to be checking for them back, or make that particular check configurable. If we receive a legitimate request that the server can handle based on its config and it goes to graphql, it should respond to it.

[Martimiz]

Can we assume that, if there is no Origin header (nor other cors headers) in the request, then it isn't a browser cors request? In which case there is no need anyway to add cors headers to the response?

[michalkleiner]

Can we assume that, if there is no Origin header (nor other cors headers) in the request, then it isn't a browser cors request? In which case there is no need anyway to add cors headers to the response?

Adding headers to the response is not an issue here, right?

[Martimiz]

Adding headers to the response is not an issue here, right?

No, not if the issue is getting the request through, sure!

It’s me wondering about the logic behind it and how it might be reflected in code…

[kinglozzer]

Proposed fix: #527

[Martimiz]

Nice! Thanks

[sabina-talipova]

@maxime-rainville , there is a quite interesting discussion about this Flutter CORS issue

[maxime-rainville]

Closed by accident. The problem is still there