Opened 5 years ago

Closed 5 years ago

#12612 closed New Feature (fixed)

Support for cross domain file uploads

Reported by: Piotr Jasiun Owned by: Artur Delura
Priority: Normal Milestone: CKEditor 4.5.0 Beta
Component: General Version:
Keywords: Cc:

Description

Check if Cross domain file uploads (CORS) works and fix it if not.

Change History (11)

comment:1 Changed 5 years ago by Piotr Jasiun

Status: newconfirmed

comment:2 Changed 5 years ago by Artur Delura

My ckfinder instance is located under http://localhost domain. My sample, from which I want upload my image, is located under http://ckeditor.dev domain.

My CKEditor configuration is:

var editor = CKEDITOR.replace( 'editor1', {
	extraPlugins: 'uploadimage',
	imageUploadUrl: 'http://localhost/ckfinder/core/connector/php/connector.php?command=QuickUpload&type=Images&responseType=json'
} );

By default CORS doesn't work. To make it work I added extra response header in C:\wamp\www\ckfinder\core\connector\php\php5\Core\Connector.php at the beginning of method executeCommand.

header('Access-Control-Allow-Origin: http://ckeditor.dev');

As you can see we got hardcoded value here. It should by dynamic based on request Origin header.

Going further: as response I've got following JSON:

{
	"fileName": "Tulips.jpg",
	"uploaded": 1,
	"url": "/ckfinder/userfiles/images/Tulips.jpg"
}

After uploading image, plugin try to download one but from wrong domain: ckeditor.dev, I'm not sure whether we should handle it on front-end or on back-end. WDYT? Front-end - just load image from proper domain - based on upload request information or on back-end - response url might be more strict - with domain or extra property origin might be provided.

Last edited 5 years ago by Artur Delura (previous) (diff)

comment:3 Changed 5 years ago by Artur Delura

Owner: set to Artur Delura
Status: confirmedassigned

comment:4 Changed 5 years ago by Artur Delura

Okay, after quick talk with @wwalc I know that we can set $baseUrl property which should include domain as well. Then uploading works.

Last edited 5 years ago by Artur Delura (previous) (diff)

comment:5 Changed 5 years ago by Piotr Jasiun

If it can be handled on the server side we should do nothing. This ticket is only about changes which may (or not) be needed on the client side. According to this article: https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS we should check withCredentials parameter.

On the other hand user may overwrite sendRequest methods if he want to modify request (withCredentials). I think that the best choose is to add withCredentials example in the documentation. In the general documentation should be short description what is needed to support CORS and in the sendRequest example how to set withCredentials.

Last edited 5 years ago by Piotr Jasiun (previous) (diff)

comment:6 in reply to:  2 ; Changed 5 years ago by Wiktor Walc

Replying to a.delura:

By default CORS doesn't work. To make it work I added extra response header in C:\wamp\www\ckfinder\core\connector\php\php5\Core\Connector.php at the beginning of method executeCommand.

header('Access-Control-Allow-Origin: http://ckeditor.dev');

As you can see we got hardcoded value here. It should by dynamic based on request Origin header.


It doesn't concern CKEditor as pjasiun wrote already, but just for anyone reading this: Access-Control-Allow-Origin should not be dynamic based on request Origin header (as it might be insecure), but configurable on the server side, so that user could set a trusted single domain only or * if he wants to allow any domain.


According to this article: ​https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS we should check withCredentials parameter.


withCredentials would have to be accompanied by yet another header on the server side: Access-Control-Allow-Credentials: true. Not a big deal though.

withCredentials may require some additional research/testing on Safari in order to document eventual issues properly:

comment:7 in reply to:  6 Changed 5 years ago by Piotr Jasiun

Replying to wwalc:

withCredentials would have to be accompanied by yet another header on the server side: Access-Control-Allow-Credentials: true. Not a big deal though.

Also CORS should be checked with all browsers we support file loader so IE 10+, Chrome, Safari and Firefox.

comment:8 Changed 5 years ago by Artur Delura

To allow simplest CORS we don't have to do nothing on front-end. To make authenticated request to server (send cookies) we have to add withCredentials flag to XHR. It will be easily available after closing ticket:12952.

Most stuff have to be done on server side. I checked uploading on safari as well, and it works (with no authentication). Issue which @wwalc mentioned should be checked when work on server side will start.

comment:9 Changed 5 years ago by Piotr Jasiun

So after closing #12952 we need to add docs how to add withCredentials flag, and we will close this ticket. Thank you for the research.

comment:10 Changed 5 years ago by Artur Delura

I added into docs already in ticket:12952.

comment:11 Changed 5 years ago by Piotr Jasiun

Resolution: fixed
Status: assignedclosed

Research is complete and the documentation updates land in #12952. Thanks!

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy