#363580 by rfay, chx, Berdir, Rob Loach, Gábor Hojtsy, Shawn DeArmond: Fixed OpenID login fails when in maintenance mode, and 403 errors on certain user paths for logged-in users.

9 files changed, 209 insertions, 16 deletions

diff --git a/includes/menu.inc b/includes/menu.inc
index 77719a33e..9af697d4c 100644
--- a/includes/menu.inc
+++ b/includes/menu.inc
@@ -239,6 +239,11 @@ define('MENU_ACCESS_DENIED', 3);
 define('MENU_SITE_OFFLINE', 4);
 
 /**
+ * Internal menu status code -- Everything is working fine.
+ */
+define('MENU_SITE_ONLINE', 5);
+
+/**
  * @} End of "Menu status codes".
  */
 
@@ -447,10 +452,17 @@ function menu_get_item($path = NULL, $router_item = NULL) {
  *   the result to the caller (FALSE).
  */
 function menu_execute_active_handler($path = NULL, $deliver = TRUE) {
-  if (_menu_site_is_offline()) {
-    $page_callback_result = MENU_SITE_OFFLINE;
-  }
-  else {
+  // Check if site is offline.
+  $page_callback_result = _menu_site_is_offline() ? MENU_SITE_OFFLINE : MENU_SITE_ONLINE;
+
+  // Allow other modules to change the site status but not the path because that
+  // would not change the global variable. hook_url_inbound_alter() can be used
+  // to change the path. Code later will not use the $read_only_path variable.
+  $read_only_path = !empty($path) ? $path : $_GET['q'];
+  drupal_alter('menu_site_status', $page_callback_result, $read_only_path);
+
+  // Only continue if the site status is not set.
+  if ($page_callback_result == MENU_SITE_ONLINE) {
     // Rebuild if we know it's needed, or if the menu masks are missing which
     // occurs rarely, likely due to a race condition of multiple rebuilds.
     if (variable_get('menu_rebuild_needed', FALSE) || !variable_get('menu_masks', array())) {
@@ -3387,15 +3399,7 @@ function _menu_site_is_offline($check_only = FALSE) {
       }
     }
     else {
-      // Anonymous users get a FALSE at the login prompt, TRUE otherwise.
-      if (user_is_anonymous()) {
-        return ($_GET['q'] != 'user' && $_GET['q'] != 'user/login');
-      }
-      // Logged in users are unprivileged here, so they are logged out.
-      if (!$check_only) {
-        require_once DRUPAL_ROOT . '/' . drupal_get_path('module', 'user') . '/user.pages.inc';
-        user_logout();
-      }
+      return TRUE;
     }
   }
   return FALSE;
diff --git a/modules/openid/openid.module b/modules/openid/openid.module
index 9a6e7edfb..99d37cf4c 100644
--- a/modules/openid/openid.module
+++ b/modules/openid/openid.module
@@ -39,6 +39,16 @@ function openid_menu() {
 }
 
 /**
+ * Implements hook_menu_site_status_alter().
+ */
+function openid_menu_site_status_alter(&$menu_site_status, $path) {
+  // Allow access to openid/authenticate even if site is in offline mode.
+  if ($menu_site_status == MENU_SITE_OFFLINE && user_is_anonymous() && $path == 'openid/authenticate') {
+    $menu_site_status = MENU_SITE_ONLINE;
+  }
+}
+
+/**
  * Implements hook_help().
  */
 function openid_help($path, $arg) {
diff --git a/modules/openid/openid.test b/modules/openid/openid.test
index cf90161fd..68313ae7e 100644
--- a/modules/openid/openid.test
+++ b/modules/openid/openid.test
@@ -147,6 +147,38 @@ class OpenIDFunctionalTestCase extends OpenIDWebTestCase {
   }
 
   /**
+   * Test login using OpenID during maintenance mode.
+   */
+  function testLoginMaintenanceMode() {
+    $this->web_user = $this->drupalCreateUser(array('access site in maintenance mode'));
+    $this->drupalLogin($this->web_user);
+
+    // Use a User-supplied Identity that is the URL of an XRDS document.
+    $identity = url('openid-test/yadis/xrds', array('absolute' => TRUE));
+    $this->addIdentity($identity);
+    $this->drupalLogout();
+
+    // Enable maintenance mode.
+    variable_set('maintenance_mode', 1);
+
+    // Test logging in via the user/login page while the site is offline.
+    $edit = array('openid_identifier' => $identity);
+    $this->drupalPost('user/login', $edit, t('Log in'));
+
+    // Check we are on the OpenID redirect form.
+    $this->assertTitle(t('OpenID redirect'), t('OpenID redirect page was displayed.'));
+
+    // Submit form to the OpenID Provider Endpoint.
+    $this->drupalPost(NULL, array(), t('Send'));
+
+    $this->assertLink($this->web_user->name, 0, t('User was logged in.'));
+
+    // Verify user was redirected away from user/login to an accessible page.
+    $this->assertText(t('Operating in maintenance mode.'));
+    $this->assertResponse(200);
+  }
+
+  /**
    * Test deleting an OpenID identity from a user's profile.
    */
   function testDelete() {
diff --git a/modules/openid/tests/openid_test.module b/modules/openid/tests/openid_test.module
index 261a1aa1e..5beb5fa41 100644
--- a/modules/openid/tests/openid_test.module
+++ b/modules/openid/tests/openid_test.module
@@ -65,6 +65,16 @@ function openid_test_menu() {
 }
 
 /**
+ * Implements hook_menu_site_status_alter().
+ */
+function openid_test_menu_site_status_alter(&$menu_site_status, $path) {
+  // Allow access to openid endpoint and identity even in offline mode.
+  if ($menu_site_status == MENU_SITE_OFFLINE && user_is_anonymous() && in_array($path, array('openid-test/yadis/xrds', 'openid-test/endpoint'))) {
+    $menu_site_status = MENU_SITE_ONLINE;
+  }
+}
+
+/**
  * Menu callback; XRDS document that references the OP Endpoint URL.
  */
 function openid_test_yadis_xrds() {
diff --git a/modules/simpletest/tests/menu.test b/modules/simpletest/tests/menu.test
index 0e6da766b..fdf529733 100644
--- a/modules/simpletest/tests/menu.test
+++ b/modules/simpletest/tests/menu.test
@@ -91,6 +91,39 @@ class MenuRouterTestCase extends DrupalWebTestCase {
   }
 
   /**
+   * Make sure the maintenance mode can be bypassed using hook_menu_site_status_alter().
+   *
+   * @see hook_menu_site_status_alter().
+   */
+  function testMaintenanceModeLoginPaths() {
+    variable_set('maintenance_mode', TRUE);
+
+    $offline_message = t('@site is currently under maintenance. We should be back shortly. Thank you for your patience.', array('@site' => variable_get('site_name', 'Drupal')));
+    $this->drupalLogout();
+    $this->drupalGet('node');
+    $this->assertText($offline_message);
+    $this->drupalGet('menu_login_callback');
+    $this->assertText('This is menu_login_callback().', t('Maintenance mode can be bypassed through hook_login_paths().'));
+  }
+
+  /**
+   * Test that an authenticated user hitting 'user/login' gets redirected to
+   * 'user' and 'user/register' gets redirected to the user edit page.
+   */
+  function testAuthUserUserLogin() {
+    $loggedInUser = $this->drupalCreateUser(array());
+    $this->drupalLogin($loggedInUser);
+
+    $this->DrupalGet('user/login');
+    // Check that we got to 'user'.
+    $this->assertTrue($this->url == url('user', array('absolute' => TRUE)), t("Logged-in user redirected to q=user on accessing q=user/login"));
+
+    // user/register should redirect to user/UID/edit.
+    $this->DrupalGet('user/register');
+    $this->assertTrue($this->url == url('user/' . $this->loggedInUser->uid . '/edit', array('absolute' => TRUE)), t("Logged-in user redirected to q=user/UID/edit on accessing q=user/register"));
+  }
+
+  /**
    * Test the theme callback when it is set to use an optional theme.
    */
   function testThemeCallbackOptionalTheme() {
@@ -491,4 +524,3 @@ class MenuTreeDataTestCase extends DrupalUnitTestCase {
     return $this->assert($link1['mlid'] == $link2['mlid'], $message ? $message : t('First link is identical to second link'));
   }
 }
-
diff --git a/modules/simpletest/tests/menu_test.module b/modules/simpletest/tests/menu_test.module
index ee8f2ea1b..8bab8a208 100644
--- a/modules/simpletest/tests/menu_test.module
+++ b/modules/simpletest/tests/menu_test.module
@@ -189,6 +189,12 @@ function menu_test_menu() {
     'type' => MENU_LOCAL_TASK,
   );
 
+  $items['menu_login_callback'] = array(
+    'title' => 'Used as a login path',
+    'page callback' => 'menu_login_callback',
+    'access callback' => TRUE,
+  );
+
   return $items;
 }
 
@@ -329,3 +335,20 @@ function menu_test_static_variable($value = NULL) {
   }
   return $variable;
 }
+
+/**
+ * Implements hook_menu_site_status_alter().
+ */
+function menu_test_menu_site_status_alter(&$menu_site_status, $path) {
+  // Allow access to ?q=menu_login_callback even if in maintenance mode.
+  if ($menu_site_status == MENU_SITE_OFFLINE && $path == 'menu_login_callback') {
+    $menu_site_status = MENU_SITE_ONLINE;
+  }
+}
+
+/**
+ * Menu callback to be used as a login path.
+ */
+function menu_login_callback() {
+  return 'This is menu_login_callback().';
+}
diff --git a/modules/system/system.api.php b/modules/system/system.api.php
index 497eb9155..f2616ec13 100644
--- a/modules/system/system.api.php
+++ b/modules/system/system.api.php
@@ -3961,5 +3961,30 @@ function hook_filetransfer_backends() {
 }
 
 /**
+ * Control site status before menu dispatching.
+ *
+ * The hook is called after checking whether the site is offline but before 
+ * the current router item is retrieved and executed by
+ * menu_execute_active_handler(). If the site is in offline mode,
+ * $menu_site_status is set to MENU_SITE_OFFLINE.
+ *
+ * @param $menu_site_status
+ *   Supported values are MENU_SITE_OFFLINE, MENU_ACCESS_DENIED,
+ *   MENU_NOT_FOUND and MENU_SITE_ONLINE. Any other value than
+ *   MENU_SITE_ONLINE will skip the default menu handling system and be passed
+ *   for delivery to drupal_deliver_page() with a NULL
+ *   $default_delivery_callback.
+ * @param $path
+ *   Contains the system path that is going to be loaded. This is read only,
+ *   use hook_url_inbound_alter() to change the path.
+ */
+function hook_menu_site_status_alter(&$menu_site_status, $path) {
+  // Allow access to my_module/authentication even if site is in offline mode.
+  if ($menu_site_status == MENU_SITE_OFFLINE && user_is_anonymous() && $path == 'my_module/authentication') {
+    $menu_site_status = MENU_SITE_ONLINE;
+  }
+}
+
+/**
  * @} End of "addtogroup hooks".
  */
diff --git a/modules/system/system.test b/modules/system/system.test
index 7d6619802..e456e3744 100644
--- a/modules/system/system.test
+++ b/modules/system/system.test
@@ -770,8 +770,6 @@ class SiteMaintenanceTestCase extends DrupalWebTestCase {
     $this->assertText($offline_message);
     $this->drupalGet('user/register');
     $this->assertText($offline_message);
-    $this->drupalGet('user/password');
-    $this->assertText($offline_message);
 
     // Verify that user is able to log in.
     $this->drupalGet('user');
@@ -804,6 +802,23 @@ class SiteMaintenanceTestCase extends DrupalWebTestCase {
     $this->drupalLogout();
     $this->drupalGet('');
     $this->assertRaw($offline_message, t('Found the site offline message.'));
+
+    // Verify that custom site offline message is not displayed on user/password.
+    $this->drupalGet('user/password');
+    $this->assertText(t('Username or e-mail address'), t('Anonymous users can access user/password'));
+
+    // Submit password reset form.
+    $edit = array(
+      'name' => $this->user->name,
+    );
+    $this->drupalPost('user/password', $edit, t('E-mail new password'));
+    $mails = $this->drupalGetMails();
+    $start = strpos($mails[0]['body'], 'user/reset/'. $this->user->uid);
+    $path = substr($mails[0]['body'], $start, 66 + strlen($this->user->uid));
+
+    // Log in with temporary login link.
+    $this->drupalPost($path, array(), t('Log in'));
+    $this->assertText($user_message);
   }
 }
 
diff --git a/modules/user/user.module b/modules/user/user.module
index 0fa22cb09..226f2f152 100644
--- a/modules/user/user.module
+++ b/modules/user/user.module
@@ -1741,6 +1741,48 @@ function user_menu() {
 }
 
 /**
+ * Implements hook_menu_site_status_alter().
+ */
+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');
+    }
+  }
+}
+
+/**
  * Implements hook_init().
  */
 function user_init() {