External application is redirected to frontpage in maintenance mode

Is this change limited to json API or does it impact all users?

IMO, redirecting a JSON request to the front page doesn't make sense; for a user using a browser, that could make sense, as the front page would show the site is under maintenance. For a JSON request, returning 503 as status should be sufficient, except in the case a JSON request is supposed to interact with the site as a normal user would.

It would help to find the reason for redirecting users to the front page when the site is under maintenance. There should be an issue that introduced that behavior, even if it could be for an old Drupal version.

For the moment, I would focus on requests done through the REST API, for which the redirect doesn't have sense, IMO. (A HTTP status should be sufficient for the code consuming the API to understand the requested data isn't temporary available.)

I've bumped into this issue when using JSON:API queries on a site, where the access to the front page is also restricted for non-logged-in users.

So when the maintenance mode is enabled, we're receiving an infinite redirect loop:

$ curl --location --request GET 'https://example.com/router/schema?_format=json' --header 'api-key: [secret]'
curl: (47) Maximum (50) redirects followed

But the correct response is 503 HTTP instead of this!

We're using the "key_auth" module for authentification, the related issue about this is #3319099: Key auth produces "Trying to destroy uninitialized session" warning in maintenance mode.

So, from my perspective, in such cases we should just show a 503 error, and the redirect to the <front> looks strange to me. And for our projects just removing the redirect works well for all cases.

At least, we should get rid of it for API requests such as REST and JSON:API.

But I see no problems with removing it for regular users too.

So I'm marking the issue as "Reviewed and tested" because see no problems with the current patch.

And if anybody can describe the reasons why, or cases where it should be kept - please comment!

Also, I see a forced logout call in this function:

  public function onMaintenanceModeRequest(RequestEvent $event) {
    // If the site is offline, log out unprivileged users.
    if ($this->account->isAuthenticated()) {
      user_logout();

It seems to me that the maintenance mode works well without doing the user_logout() - the user just sees the 503 response with the explanation, without losing their session.

So, can anybody please explain why we should here also forcibly kill the current session for logged-in users in maintenance mode?

If there are no significant reasons, maybe get rid of it too?

And if we remove also the user_logout() - we don't need the event subscriber at all! I've attached my patch with the removed subscriber.

And here is rerolled patch with the removed MaintenanceModeSubscriber for 9.4.x branch.

The same patches, but with adopted tests to new behavior (503 error with 'Site under maintenance' message instead of 302 redirect).

Regarding

b) I don't understand what benefits the redirect does (why is below). It was introduced in #1998228. Remove it, patch is below.

From patch attached to that issue there is a clean reason for logout:

-function user_menu_site_status_alter(&$menu_site_status, $path) {
-  if ($menu_site_status == MENU_SITE_OFFLINE) {
-    // If the site is offline, log out unprivileged users.
-    if (user_is_logged_in() && !user_access('access site in maintenance mode')) {
-      module_load_include('pages.inc', 'user', 'user');
-      user_logout();
-    }
-
-    if (user_is_anonymous()) {
-      switch ($path) {
-        case 'user':
-          // Forward anonymous user to login page.
-          drupal_goto('user/login');
-        case 'user/login':
-        case 'user/password':
-          // Disable offline mode.
-          $menu_site_status = MENU_SITE_ONLINE;
-          break;
-        default:
-          if (strpos($path, 'user/reset/') === 0) {
-            // Disable offline mode.
-            $menu_site_status = MENU_SITE_ONLINE;
-          }
-          break;
-      }
-    }
-  }
-  if (user_is_logged_in()) {
-    if ($path == 'user/login') {
-      // If user is logged in, redirect to 'user' instead of giving 403.
-      drupal_goto('user');
-    }
-    if ($path == 'user/register') {
-      // Authenticated user should be redirected to user edit page.
-      drupal_goto('user/' . $GLOBALS['user']->uid . '/edit');
-    }
-  }
-}

So logout is happened only for non-eligible users i.e. w/o "access site in maintenance mode".
9.5.x branch contains https://git.drupalcode.org/project/drupal/-/blob/9.5.x/core/lib/Drupal/C...

if (!$this->maintenanceMode->exempt($this->account)) {

So subscriber will be not triggered for users with "access site in maintenance mode". So we do not require additional event subscriber.
Regarding why redirect is happening - maybe it is elated to some kind of SEO optimizations made to have Drupal correctly indexed in search results.

From patch attached to that issue there is a clean reason for logout:

The reason is described as "If the site is offline, log out unprivileged users." but I still can't understand why we should force the logout for all users, instead of just showing the "Site is offline" message on the blank page?

For example, if some user (without permission to access in maintenance mode) is successfully logged in and is actively working with the site, and a little later the site goes into maintenance mode for a couple of minutes, and exactly in this time the user opens some page - we're forcibly dropping the user's session. So after the maintenance mode window is finished, the user should do the log in procedure again! What's the purpose of this?

Regarding why redirect is happening - maybe it is elated to some kind of SEO optimizations made to have Drupal correctly indexed in search results.

For SEO 503 error works much better than the redirect to frontpage, so the 302 redirect looks very strange for me.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

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

Have to agree, not sure the redirect is needed so the change LGTM.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch 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.

Hello,

I faced this problem today.
We have routes that are called in ajax, some of them are called every 20 seconds on some lesson page in order to save the time spent by the student on the lesson.
In the middle of a deployment procedure, with the site under maintenance, an error was caused by this ajax route when it shouldn't have. Because there is no way the code of this ajax route could possibly cause this specific error.

After debugging I discovered that the first time the ajax route is called, when the student is still authenticated, the homepage was returned in HTML by the ajax call. Any call after that would return a nice 503 service unavailable response. So it was the building of the homepage that was responsible for that error when calling the ajax route the first time.

I debugged a little further and discovered the MaintenanceModeSubscriber.php of the user module that forces the log out and forces the redirect to the homepage.

I am happy to discover that this issue exists, but I'm skeptical to see that no final decision could be made so far :/
Personally I also believe that this redirect to the front page should not be made, and that displaying the maintenance page is enough and better.
Even though I understand the point of "a user on a public computer that could not log out...", I also find it very frustrating to all our students that they get logged out every time we put the site under maintenance mode. So I would be in favor not to log them out anymore.

I actually think this could be introduced as a config checkbox on the maintenance mode page : "automatically log out unprivileged users who try to access the site while in maintenance mode" (can be safer for users who logged in on a public computer and cannot log out while site is in maintenance)
In the mean time, I think I'm going to try the patch that gets rid of the forced log out and the forced redirect to front page.

Nicolas

Review the patch, created MR with respective changes. Please review, moving to NR

Thank you the patch applies cleanly on Drupal 10.3.2 and fixes the problem of the HTML of the front page being sent in response of the ajax call when the user is logged out. Now we immediately have the simple 503 service unavailable error which is what we want.

I don't think the redirect was added in #1998228: Remove hook_menu_site_status_alter() in favor of request listeners, it was moved in that issue. As far as I can tell the original redirect was added in #363580: OpenID login fails when in maintenance mode. However this has been moved around so much since then, and changed from what it used to do, that I agree it doesn't make any sense any more.

The one situation I can see is that say you are on /user or /some/admin/path and you get logged out by maintenance mode, if you refresh the page you'll get a 403 and you might not know why. But equally, you might not know why you got redirected to the front page either.

Committed/pushed to 11.x and backported back through to 10.3.x, thanks!

I agree with not removing the user logout due to the public computer issue in this change because it's not necessary to fix the bug - however I think we could open a follow-up to discuss the pros/cons of removing the logout. The public computer issue is very unlikely (and any networking or site issue that's not maintenance mode could result in someone not being able to log out at a precise moment), but people being logged out in the middle of something and losing progress is not.