Provide hook_seckit_options_alter() D8

Hi @jweowu & @mfb, I've attached a patch which attempts to replicate (D9 style) how the D7 version of seckit alters options via hook as mentioned in #12 ✨ Provide hook_seckit_options_alter() D8 Needs review . Some of the patterns in the D7 still seem to make sense currently so some logic is replicated. An initial test is included, but could be expanded on.

Since we're dealing with the configuration system and we don't want overrides appearing in the config form, getSeckitConfig() interacts with ImmutableConfig, and caches the overrides as advised since it runs more than once during the request cycle.

Including the "Needs manual testing" tag :^)

Ah, wasn't using the same pattern for the report path seen in the other tests. Updated patch.

@jnlar I tested out converting some code that used config.factory.override to use this hook instead, and seems to work fine. The one gotcha I ran into was that the style-src key is missing from the config install yml file, so my code has to check if it exists. We could open a separate issue for that.

Looks like you have a test so I also removed the "Needs tests" tag.

To flush caches, I would suggest adding an empty post_update function, rather than a hook_update_N() with drupal_flush_all_caches(), reason being that the way you wrote it will AFAIK result in flushing the caches twice.

You can remove the can remove the core_version_requirement key from the test module.

There are some coding standards messages for your patch, but looks like the 2.x branch already has plenty of those so not sure if it's an issue for the maintainers.

@mfb Thanks for reviewing.

Opening a separate issue for adding the style-src key in config install sounds good.

The way I wrote it will indeed flush the caches twice. I've removed the hook_update_N() in favour of an empty hook_post_update_NAME().

Wouldn't want to creep in more coding standards issues so I've looked at the ones in the modified files.

Created an issue 🐛 style-src key missing in seckit.settings.yml Fixed for adding the CSP setting style-src to seckit.settings.yml

We've been using #18 to adjust the CSP to allow for some rich legacy experiences that take over the page. We're taking configuration from Composer files in separate repositories to adjust the CSP rules for particular page requests -- highly custom but very effective. This patch has been of great benefit to us; and given that the patch provides new tests for this case, and that they're all passing, I'm willing to RTBC this.

As of 2.0.2 this patch no longer applies to 2.x

I've tried rerolling #18 against the latest 2.x. It seems to work but I reckon it needs a bit of a look. If anyone else wants to check the branch out and make changes that's all good.

I've tried #22 and it seems to work well for my use case at least (Adding to CSP domains list)

Hi all,

Pulled in #22 and the changes look OK on my end :^) CI is passing + I've done some manual testing of the hook with things such as per request nonces in the CSP and blocking based on the CSRF Origin header.

I've created a quick patch based on the merge request as an interim solution.