Logout option is displayed for anonymous users

This is interesting, and in a way the opposite of #2267603: "Login" link shows up for logged-in users as well.

Wrong tag - sorry!

They are, let us say, thematically related but may have different causes.

Please review. The solution is conflicting with https://www.drupal.org/project/drupal/issues/2193803.

Seems like the patch on this comment fixes this issue as well: https://www.drupal.org/project/drupal/issues/2267603#comment-13893903

The solution to add _user_is_logged_in: 'TRUE' has an exception that it is redirecting the user to "Access denied" page which should not be the case , the anonymous user should get redirected to "Home page".

The tests are failing because of that , so made some changes to the user subscriber and updated few tests.

Thanks.

Fixed some Coding standard issues.

Applied #15 patch working fine sharing screenshot...

Patch #15 test is stuck.

Verified and tested patch#15. Patch applied successfully.

Testing Steps:
1. Goto Structure > Menus > Main navigation
2. Create a link with title "Logout" and link "/user/logout"
3. Logout from the site
4. Check "Logout" link is displayed for anonymous user
5. Now apply the patch
6. Again, check "Logout" link is not displayed for anonymous user
7. Login to the site and check "Logout" link is displayed for authenticated user

Testing Results:
1. "Logout" link is not displayed for anonymous user
2. Authenticated user is directed to homepage on logging out.

Just checked the patch, it indeed works, but there is a CI error happening with the tests runs for patch #15. It could be something not related with issue in hand(like a SSH key error), but since it didn't perform correctly, I'm moving the issue to "Needs Work". If its not related, please disconsider this point, and keep moving the issue.

Thanks for the patch! I've slightly improved the asserts by using linkByHref(Not)Exists

Patch #22 applied cleanly and working as expected and sharing screenshot for reference.

+++ b/core/modules/user/src/EventSubscriber/AccessDeniedSubscriber.php
@@ -72,6 +72,25 @@ public function onException(ExceptionEvent $event) {
+    $redirect_403_path = Url::fromRoute('<front>', [], ['absolute' => TRUE]);
+    if ($this->account->isAnonymous()) {
+      $event->setResponse(new RedirectResponse($redirect_403_path->toString()));
+    }

On further inspection I actually don't think this is right. This is effectively redirecting a user to the frontpage on any 403 that isn't the logout page which is a change in behaviour

+++ b/core/modules/user/src/EventSubscriber/AccessDeniedSubscriber.php
@@ -72,6 +72,25 @@ public function onException(ExceptionEvent $event) {
+    if (!($exception instanceof AccessDeniedHttpException) && $route_name !== 'user.logout') {

Sorry it's this bit that's not right. It should be || so we don't redirect if it's not a 403 OR it's not the user.logout route. I think it'd make more sense to invert the conditions and put the whole block inside it rather than the early return.

Here's an updated patch

1. Moves the response alter into the existing onException as it was already doing something very similar
2. Adds further test coverage to ensure non-logout links don't redirect

Removes extra whitespace -_-

This looks great, thanks!

The test coverage seems pretty solid. I can't think of any aspect of this bug that isn't reflected in the new tests.

I only have 1 tiny ubernit that stopped me from RTBC'ing:

+++ b/core/modules/menu_ui/tests/src/Functional/MenuUiTest.php
@@ -492,6 +493,30 @@ public function doMenuTests() {
+    // Create user with required permissions

Could be:

// Create a user with the required permissions.

(or just remove this comment entirely, the code is self-documenting enough).

Thanks again!
-Derek

Bah, this is trivial enough, I'm just gonna do it and self-RTBC... ;)

This looks good, and the correct redirection fix for #2193803: Anonymous users receive access denied when attempting to logout which introduced the bug in the first place trying to fix a diferent one.

Committed/pushed to 9.2.x and cherry-picked to 9.1.x, thanks!