IFRCGo / go-frontend

MIT License
21 stars 5 forks source link

Properly handle image uploads through TinyMCE #2460

Open batpad opened 2 years ago

batpad commented 2 years ago

Following on from #2438 -

The last update we did with the Rich Text Editor implementation inadvertently gave users the ability to add images to their posts within rich text areas.

Since this was not a properly planned feature implementation, it relies on the default TinyMCE handling of image data, which is to embed the raw base64 encoded image data into the HTML. This is likely not the best solution going forward as we are storing rather large image blobs in the HTML and consequently in the database. Ideally, we would not do this, and upload the images somewhere else and include them with a normal <img src="https://path.to.image/foo.jpg"> type tag.

This would require us to handle file uploads on the backend, store them in Azure Blob Storage, and return a link to be embedded in the HTML.

There could be a few different ways to do this:

I would propose to go with the second approach outlined above - i.e. create a simple custom image handler route on the backend that uploads to Blob Storage and configure TinyMCE to use it. Whilst saying this, I think there is a lot to be concerned about when allowing users to upload arbitrary images to text fields, some of those are:

In general, introducing WYSIWYG editors like TinyMCE can have rather large implications and a large list of pros and cons that are likely out of the scope of this particular ticket, but that might inform decisions we take here. Looking forward to talking more and weighing our long-term options around rich text editing when we meet in person.

cc @tovari @szabozoltan69 @frozenhelium @thenav56

szabozoltan69 commented 2 years ago

8 days ago (on 15th Aug) I found 8 entities in production which used base64 images, and replaced them with the uploaded image. Since then these queries give no result; I will run them time to time to check if somebody uses pasted images:

select length(description) from api_fieldreport where description like '%base64%' order by 1 desc;
select length(situational_overview) from flash_update_flashupdate where situational_overview like '%base64%' order by 1 desc;
select id, length(summary) from api_event where summary like '%base64%' order by 2 desc;

Of course, a final solution (i.e. the 2nd one above) would be great.

thenav56 commented 2 years ago

For third point, some additional things we can do.