Tested successful thanks @Kubik-Rubik

No doubt about it that it works as described. Still the wrong thing to do. See no reason to replace bad code with bad code. As mentioned in the tracker, it should respect the show_authorised_links setting for content.

hmm i think the show_authorised_links is a different one. It should have a different PR and Tracker.
Just my two cents

Also this works for content - but what about tags of any other content type? e.g. weblinks/banners etc.

The show_unauthorised_links determines for content that the intro is public, the full article requires access. Ignoring this setting means you don't show relevant information while looking for it. But when you have found it in a different way, you show even more than the title, the complete intro. Don't think tags should be as open as it is now, it shouldn't be tighter either. Just fix it the proper way!

@sovainfo Yes, I think that you are right. We should take this option also into account. Will look into it in more detail and update the pull request. Thanks!

As stated on tracker already, show_unauthorised_links is a setting from the component (turned off by default). You can't use it in a module. You would have to create a new module parameter if you wish to have that function.

I agree with the PR here. Showing titles if the access doesn't match is wrong. I'd rather don't show titles even if show_unauthorised_links is enabled than showing unauthorised titles.

We can always add the feature in a separate PR if we want. That one would go into 3.4, while this one could go into 3.3.

@Bakual Okay, will open a new tracker then if I can figure it out. :-) Thanks!

Ok, won't bother to test it then. Already know that the results are going to be wrong when show_unauthorised_links is in effect. Seems there is no interest in doing the right thing!

@sovainfo No problem. Just send a pull request with the "right thing", we will be happy to test it!

@Kubik-Rubik Not being familiar with tags, I am not going to provide a PR with a real fix of the issue. Considering the remarks of @Bakual, I am not going to waste any more time on it.
I am convinced you'll be able to figure it out yourself (I would probably not have reacted at all otherwise). Just look at the models articles and category in com_content and the modules that use 'show_noauth' like mod_articles_popular. They do the right thing!

Also don't ignore what George mentioned. What about other content types?

Also this works for content - but what about tags of any other content type? e.g. weblinks/banners etc.

Good point. @Kubik-Rubik You need to check the core_access column in the #__ucm_content table. So remove the join over the #__content table and do the check using cc.core_access. Then it will work.

Added fix for all content types. In a further step the "show_noauth" option should also be considered.

While looking for the implementation of Show_Unauthorised_Links found the 'show_noauth' also used in mod_weblinks for the context of 'com_weblinks'.
The model of com_weblinks ignores the setting and always checks the access. So, I think it is save to say it is only implemented for com_content.

Thanks Viktor. Looks good now to me.

The one in mod_weblinks is basically a bug, since there is no such param in com_weblinks.
The parameter is only used in com_content and is likely one of the buggiest ones.
It is used in some of the com_content related modules, but in a way which doesn't take into account menu item settings. It will always use the global component settings.

I really don't think we should do the same thing in the similar tags module. It's a generic module which should be agnostic to any component (except maybe tags).

Doing the right thing and doing things right are two different things. It would be nice if the tags module would be the first doing both!
Agree that the calculated setting at the menu should be used.

Agree that the calculated setting at the menu should be used.

Not possible the way Joomla works...

One more thing I noticed while testing. core_access is basically an optional column for the various content types. If an extension uses a content type which doesn't support access levels, then its value will be 0 (zero).
In this case the item should show as well, but with this PR it will never show at all.
I'd suggest to use $query->where('(cc.core_access IN (' . $groups . ')) OR (cc.core_access = 0)');

@Bakual Ah, didn't know that this field is optional. Updated pull request with your suggestion!

@test: While the test is OK and the Registered article no longer shows up I see something else I wonder if it should.

Before the patch, I have 3 articles:

Joomla! Testing
Quick Icons
Captcha (registered access)

After applying the patch, I see this:
Similar Tags
Joomla! Testing
Quick Icons

Notice that:

1. The order has changed
2. An extra article is added to the list. This is the article I clicked from the All Modules menu. So should the list include the article one is actually looking at? It is not a similar one, it is the actual article I already am at.

This is the URL I used based on the Joomla! test data:

index.php?option=com_content&view=article&id=71&Itemid=472

I added a tag called similar to the Quick Icons and Captcha articles and I set Captcha to Registered.

@Bakual Done!
@roland-d Please test it again, thank you!

@Kubik-Rubik Thanks for the update. All good now and I moved the JC tracker to RTC.