Flag reload and AJAX link types are missing a token

1-line fix to add tokens to a link! Nice!

We need tests for this, and as it turns out the reload link type has no coverage, here's a whole new test class.

In addition to checking the lifecycle of the flagging, we try to load the reload link without the token and check we get access denied.

Aw that's a pain :(

But then this is a problem in Flag with the reload link in 7x-3x, and indeed ever since it was introduced (or as long as Drupal core has had a page cache).

So we could legitimately fix the security regression, then slap on a warning on the UI description of the link to say don't use it with caching, and then think about how to fix the caching. Another possibility is offering an option to disable the token -- we've had several requests over the years to have this feature on 7x-3x.

Another issue asking to have the token turned off in D7: #2470373: How to bypass or alter CSRF token for a certain flag...

This affects the AJAX links too; the same attack could be crafted with that link type too.

Is there any way to have placeholders in the cached rendered content, which are replaced on output?

On earlier versions of Flag, anonymous users aren't allowed to access flag at all unless Session API module is present.

So now we've set a cache context with #2473921: FlagSimpleTest is failing due to render/page caching, can the need for a token be handled by changing the cache context to:

$build['#cache']['contexts'][] = 'user'

so the entity is cached per-user?

Though making the whole entity cached per-user is not brilliant for performance...

Well spotted!

However, this fix belongs in a new issue. And the whole of that stuff should be removed, as Views module is now in core, so you can't download it, and I don't think we need to tell people they can use Views to make lists.

+++ b/src/Controller/FlagListBuilder.php
@@ -105,7 +105,8 @@ class FlagListBuilder extends DraggableListBuilder {
+      // Fixed "@views-url" needed because of an exception in FlagLinkReloadTest.

BTW: we don't put comments code about the changes that have been made. Comments should be about the code now. For everything else, there's the git log and the issue queue.

It's a 5-minute fix -- I can take care of it after lunch today unless @tduong beats me to filing an issue and making a patch :)

Step one: created issue: #2619848: remove UI text about downloading Views.

(I'm not slacking off work, my caches are taking ages to clear ;)

Eh, that was back in April. I have no idea! I'd completely forgotten I wrote the tests too! :D

Though I see the test class needs a rename, as since I wrote that we've changed the link type tests to be called LinkTypeFOOTest. But given this is critical, that can be a follow-up.

Committed!

Thanks for that final reroll tduong :)