Preview links not rewriting

[willguv]

Flagged by Jamie

@stephen-cox I've created a preview link for a guide that has 11 pages. I clicked 'Add all pages to this guide' and when I view this preview link logged in I can see all the pages. When I'm logged out I can only see the first page. Does it only work for users who are logged in? Most colleagues I use this with will not have an account on the website.

Originally posted by @Dixonj1 in #600

[willguv]

@stephen-cox I think it's urgent we track down this problem and patch/ suggest a fix to maintainers. This feature working properly is more important to content editors than character counts. Can you let me kow how we go about this please? Thanks

[andybroomfield]

Just confirming the issue applies to a fresh copy of localgov drupal with localgov_demo enabled.

Steps to reproduce:

1. Create an unpublished guide overview page
2. Create several unpublished guide pages, link them to the guide overview.
3. On the guide overview page, click preview link
4. On the preview link page, click and all pages for this guide
5. Copy the preview link and open in a private browser window (anon user)

Expected result

Page shows with links to all guide pages in the top navigation box.

Actual result

Anon user can only preview the top level page, navigaton box does not show additional links. (Though if they paste the links in it does give them access, so it's not a permission issue).

And pasting in the link to the second page directly.

[willguv]

Thanks for testing this @andybroomfield !

[andybroomfield]

The troublesome code is this line in localgov_guides/src/Plugin/Block/GuidesAbstractBaseBlock.php:

return ($guide_node instanceof NodeInterface) && $guide_node->access('view');

If you remove && $guide_node->access('view') it now works as expected. That will also mean that anyone else can see the link also regardless on if they have a preview link or not!

I suspect it's the same in other places. We check to see if a page can be accessed before showing the link, but we are only concerned with the page in it's normal state, not accounting for the preview link token.

Not sure resolution, maybe a custom access checker that can account for preview links?

[mccrodp]

I have been able to reproduce this - thanks for helpful steps above. Now I'll dig deeper into @andybroomfield's latest comment above to see if I can replicate and investigate possible access solutions before reporting back.

[willguv]

Thanks @mccrodp

Looping in @stephen-cox who's chatting with Paul from Haringey about the issues he's been having

[mccrodp]

I suspect it's the same in other places. We check to see if a page can be accessed before showing the link, but we are only concerned with the page in it's normal state, not accounting for the preview link token.

Not sure resolution, maybe a custom access checker that can account for preview links?

There is a PreviewLinkAccessCheck class in the Drupal\preview_link\Access\ namespace. There's also an interesting class PreviewLinkEntityHooks class in Drupal\preview_link\ namespace that has a preview_link_entity_access hook by the look of it. Both of these seem to have ways to determine access using the token, rather than standard entity access that's currently in use: $guide_node->access('view').

However, I don't see documentation for preview_link module and been out of this space a while. I don't know how to call Preview link's access method.

I presume we want a custom access checker that: when Preview Link module is enabled, uses the preview link access check instead of default entity access check. Please correct if I'm wrong 😄

[stephen-cox]

Looking into this I have found a potential bug with the Preview Link module. The entity access check it adds has a condition to restrict it to the current entity, so when it checks if the user has access to the other preview links it doesn't allow it. As access to these pages isn't allowed, they aren't displayed in the navigation. Removing this check allows access, but there is a danger that this allows too much.

The condition causing the issue is here: https://git.drupalcode.org/project/preview_link/-/blob/2.1.x/src/PreviewLinkEntityHooks.php?ref_type=heads#L69

Removing the $route_entity->id() === $entity->id() condition allows the blocks to work as expected with preview links.

I will create an issue in the Preview Link issue queue with a potential fix and see what the module maintainers think of this.

[stephen-cox]

I have created an issue for this on drupal.org: https://www.drupal.org/project/preview_link/issues/3481523

The outstanding question I have is, is it safe to just drop the check for the current page or should we check whether the entity belongs to a preview link?

The later sounds safer, so I will look at how feasible this is, but any thoughts are welcome.

[willguv]

Hi @stephen-cox what's the latest on this please? Thanks

[stephen-cox]

This should have been fixed for Guides, Step by steps, Subsites and Publications with https://github.com/localgovdrupal/localgov/pull/785

There's still a problem with Directories, for which we don't have an easy answer. Discussion for this here https://github.com/localgovdrupal/localgov_directories/issues/404

[willguv]

Hi @stephen-cox is there an update on Publications please?

[stephen-cox]

Hi @willguv This should work with publications. Have you had reports that it's not working?

[willguv]

Hi @stephen-cox, not any recent issues - will go back to the folks who mentioned it

Any news on getting Directories to work? Thanks again

[stephen-cox]

Discussion on directories is here: https://github.com/localgovdrupal/localgov_directories/issues/404

I don't know if it's going to be possible to ship with this working as there are serious concerns about putting unpublished content in a Search API index, which would be required for this to work with directories.

[willguv]

Thanks @stephen-cox, I'll call this done and surface 404 in this project