Gizra / drupal-starter

Drupal 10 Starter with best practices
76 stars 39 forks source link

Add seckit and HSTS [3h] #280

Closed amitaibu closed 1 year ago

amitaibu commented 2 years ago

@mariano-dagostino I believe you've implemented it on other projects. Do you see any downside in including it by default with Drupal-stater?

mariano-dagostino commented 2 years ago

@amitaibu The only issue I see is you need to be aware of seckit blocking remote URL calls by your js code. Therefore if you have some elm or js app that do a get requests to an external api, you need to whitelist it in the seckit config.

This also applies for iframe content, so google maps, youtube embeds, etc need to be whitelisted as well.

amitaibu commented 2 years ago

you need to whitelist it in the seckit config.

Let's add that to the README.

so google maps, youtube embeds, etc

Good point. Let's add to the README, but also let's whitelist those services (Youtube, Maps, Google Analytics, come to mind)

mariano-dagostino commented 2 years ago

@amitaibu I've been checking this today, it seems there is a big catch. If you use Ckeditor you need to enabled script-src: 'unsafe-inline` which is not a good practice.

I've been doing some research and there is an alternative module to seckit called CSP I've never used it, but it seems it adds this unsafe-inline option conditionally when CKeditor is in place.

Anyways now rethinking my answer I think Seckit is not a good candidate for drupal starter. So either we try CSP, or close this issue.

1h

amitaibu commented 2 years ago

Seckit seems to be more popular and has more options to harden the site. Can you give it ~2h to see what would be a possible path for it and CKeditor.

mariano-dagostino commented 1 year ago

@amitaibu We may have more luck with CKEditor 5 (now included in core) https://ckeditor.com/docs/ckeditor5/latest/installation/advanced/csp.html

Screenshot_2022-10-24_10-49-04

I don't know if CKEditor for core is already doing this. I checked the issue queue and haven't found anything related to this topic. Anyways seems the most promising path of action.

+0.5h

amitaibu commented 1 year ago

@mariano-dagostino let's revive this when you have time.

mariano-dagostino commented 1 year ago

@amitaibu It seems Ckeditor 5 works fine with seckit. I'm not 100% sure if the functionality remains the same between upgrades, but so far the editor works and no error is triggered in the console.

2023-05-26_17-25_1

2023-05-26_17-25

+1.5

GizraNotifier commented 1 year ago

The latest merged PR #512 just got deployed successfully to Pantheon qa environment