Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CKFinder Adapter should support adding custom headers to the request #887

Closed
peledies opened this issue Mar 9, 2018 · 10 comments
Closed
Labels
package:adapter-ckfinder resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@peledies
Copy link

peledies commented Mar 9, 2018

Request

Need the ability to add additional headers to the ckfinder upload adaptor.

Explanation

When using the ckfinder upload adaptor to upload files to a Laravel application, the XcsrfToken needs to be sent in the headers to ensure authentication is respected.

Desired configuration

ClassicEditor.create( document.querySelector( '#editor' ), {
	ckfinder: {
		uploadUrl: '/image/upload'
		headers: [
			XcsrfToken: 'examplestring'
		]
	}
})
.then( editor => {})
.catch( error => {});
@wwalc wwalc changed the title Easy Upload not so easy CKFinder Adapter should support adding custom headers to the request Mar 12, 2018
@pjasiun
Copy link

pjasiun commented Mar 12, 2018

Hi,

First of all, Easy Image and CKFider are 2 separate solutions. Easy Image uses CKEditor Cloud Services as a backend. For CKFider you need to configure your own backend. This is why we change the topic of this ticket.

However, the request makes sense for CKFinder adapter to make headers configurable.

@pjasiun pjasiun added type:improvement This issue reports a possible enhancement of an existing feature. status:confirmed package:adapter-ckfinder labels Mar 12, 2018
@Reinmar
Copy link
Member

Reinmar commented Mar 12, 2018

Doesn't CKFinder's backend connector support CSRF token itself? And isn't it built into this upload adapter already? If this was a generic upload adapter, then it should support setting headings, but that's not this adapter's goal.

@Reinmar
Copy link
Member

Reinmar commented Mar 12, 2018

cc @jodator

@peledies
Copy link
Author

@Reinmar im sure the CKFinder backend components do support the CSRF token. However, the issue is that I don't want to use additional software to handle something as simple as file upload that is already pretty well fleshed out with Laravel. adding the ability to configure the headers would be ideal.

@jodator
Copy link
Contributor

jodator commented Mar 12, 2018

The CKFinder does already own CSRF protection and if used with provided connector it will work.

Assuming from the conversion that @peledies is using CKFinder upload adapter plugin without CKFinder backend in custom Laravel application (just a guess).

I think that @peledies needs to implement own upload adapter to manage uploads to a custom Laravel application as both mentioned plugins (easy image and CKFinder upload adapter) are dedicated solutions.

AFAIK we do not provide generic file upload adapter.

@Reinmar
Copy link
Member

Reinmar commented Mar 12, 2018

So, I think that we can close this ticket as invalid. Thanks.

@Reinmar Reinmar closed this as completed Mar 12, 2018
@Reinmar Reinmar added resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). and removed status:confirmed labels Mar 12, 2018
@peledies
Copy link
Author

@jodator you are correct

Assuming from the conversion that @peledies is using CKFinder upload adapter plugin without CKFinder backend in custom Laravel application (just a guess).

Where can I find a detailed example of implementing my own upload adapter? the documentation is kind of sparse in its alpha state.

@Reinmar
Copy link
Member

Reinmar commented Mar 12, 2018

Unfortunately, it's on our long TODO list for docs (it's a part of #618). But, based on the source code of https://github.com/ckeditor/ckeditor5-adapter-ckfinder/ you really shouldn't have a problem implementing your own adapter. I know people do that.

@peledies
Copy link
Author

Cool, ill look into that. Thanks for the feedback.

Keep up the good work and good luck on that TODO list.

@PayteR
Copy link

PayteR commented May 2, 2018

Or just add exception for you upload url

namespace App\Http\Middleware;

use Illuminate\Foundation\Http\Middleware\VerifyCsrfToken as Middleware;

class VerifyCsrfToken extends Middleware
{
    /**
     * The URIs that should be excluded from CSRF verification.
     *
     * @var array
     */
    protected $except = [
        'your-upload/path'
    ];
}

And here is example, how to implement it into Laravel controller with http://laravel-mediable.readthedocs.io/en/latest/uploader.html uploader

/**
 * @param Request $request
 * @return \Illuminate\Http\JsonResponse
 */
public function upload(Request $request)
{
    try {
        $file = $request->file('upload');

        $uploader = \MediaUploader::fromSource($file);
        $uploader->setAllowedMimeTypes(['image/jpeg', 'image/gif', 'image/png']);
        $uploader->setAllowedExtensions(['jpg', 'jpeg', 'png', 'gif']);
        $media = $uploader->upload();

        return response()->json([
            'uploaded' => true,
            'url' => $media->getUrl()
        ]);
    } catch (\Exception $e) {
        return response()->json(
            [
                'uploaded' => false,
                'error' => [
                    'message' => $e->getMessage()
                ]
            ]
        );
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:adapter-ckfinder resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

No branches or pull requests

5 participants