User logout during maintenance mode

Rerolled for Drupal 10.

#14 creates an infinite redirect loop because $this->account->isAuthenticated() is always true now so it always sends a RedirectResponse.
I think MaintenanceModeSubscriber serves no other purpose and could be entirely removed.

Here is a reroll for Drupal 10.2.3.

samit.310@gmail.com → made their first commit to this issue’s fork.

HI,

We have permission provided by Drupal Use the site in maintenance mode(access site in maintenance mode) that allow any user to work on maintenance mode, A simple change can fix this user logout issue,

Here all the users who have Use the site in maintenance mode(access site in maintenance mode) permission, will not logout if the site goes to maintenance mode.

Here is the PR link: https://git.drupalcode.org/project/drupal/-/merge_requests/8876/diffs

Thanks
Samit K.

@samit.310@gmail.com But not logging out does not automatically mean that users should continue to use the website despite maintenance.

Example from practice:
Website is briefly switched to maintenance mode several times a day because automatic backups are performed. However, it is correct that during backups, no user administers the website and therefore should not have the permission “Use the site in maintenance mode”. But the users should not all be logged out just because maintenance mode was switched on briefly. If you have a website with a community where many users are logged in, then they are automatically logged out several times a day.

Only briefly skimmed but any change will need test coverage.

Also issue summary appears incomplete, recommend using default issue template. Sounds like from previous comment MR solution isn't along the idea so definitely needs a summary update.

Hi @handkerchief,

Agree with your point, but here the idea is the only trusted or some specific user roles have this permission Use the site in maintenance mode(access site in maintenance mode).

The functionality will remain the same for other users, meaning whenever the maintenance mode will on all the users except those with the above permission will logout.

Thanks
Samit K.

Rerolling #14 for drupal 10.3.6

Added tests and updated issue summary.

prudloff → changed the visibility of the branch 057267-user-logout-during-fix to hidden.

So believe this would be a feature request vs an actual bug

Not sure if this needs framework or product manager approval first but feedback appears to be addressed.

Did some git archaeology, this was added in #50901: User login under maintenance mode → .

One of the reasons given then was that if you are logged in as a regular auth user, you would not be able to logout to then login again as an admin account. Do we still prevent people from logging out when maintenance mode is on? If so maybe we need to exclude that route.

Also one small comment on the updated test coverage.

I could indeed reproduce this with the patch: logged-in users can't logout (/user/logout returns the maintenance page).
(A workaround is to open a new private browsing window but I see how this is a usability problem.)

I added _maintenance_access to the logout route in order to fix this.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

I rebased the MR.
The phpstan errors listed by the bot in #32 seem to be unrelated.

Think we need to deprecate the service first vs deleting everything. Probably needs a CR

I'm not sure how to deprecate an event subscriber since https://www.drupal.org/node/3357408 → .
It seems there is no way to prevent the event subscriber from handling events without removing it entirely (or disabling autowiring for the entire module).

The bc policy isn't very clear here, going to try to update it (relevant bit is here: https://www.drupal.org/about/core/policies/core-change-policies/bc-polic... → )

IMO when we're completely removing an event subscriber, we should just remove the whole thing without deprecation (as we would with a hook implementation).

In that case I assume we don't need a CR anymore, which I believe was the only outstanding item.

No I think we should add a CR for two reasons:

1. If someone has replaced or removed the event subscriber in custom code to try to fix this behaviour without a core patch (unlikely but not unheard of) it's a clear indication that workaround can be removed.

2. It's a change of visitor facing behaviour that's been like this for a very long time. I very much doubt anyone relies on it, but again a CR doesn't hurt.