Issue #1538462 by c960657, Niklas Fiekas, nod_: Fixed Cannot log in with OpenID due to 'required' attribute.

core/modules/openid/lib/Drupal/openid/Tests/OpenIDFunctionalTest.php

@@ -157,9 +157,9 @@ class OpenIDFunctionalTest extends OpenIDTestBase {
 
     $this->drupalLogout();
 
-    // Test logging in via the user/login page.
+    // Test logging in via the user/login/openid page.
     $edit = array('openid_identifier' => $identity);
-    $this->drupalPost('user/login', $edit, t('Log in'));
+    $this->drupalPost('user/login/openid', $edit, t('Log in'));
 
     // Check we are on the OpenID redirect form.
     $this->assertTitle(t('OpenID redirect'), 'OpenID redirect page was displayed.');
@@ -169,7 +169,8 @@ class OpenIDFunctionalTest extends OpenIDTestBase {
 
     $this->assertLink(t('Log out'), 0, 'User was logged in.');
 
-    // Verify user was redirected away from user/login to an accessible page.
+    // Verify user was redirected away from user/login/openid to an accessible
+    // page.
     $this->assertResponse(200);
 
     $this->drupalLogout();
@@ -199,9 +200,9 @@ class OpenIDFunctionalTest extends OpenIDTestBase {
     // Enable maintenance mode.
     config('system.maintenance')->set('enabled', TRUE)->save();
 
-    // Test logging in via the user/login page while the site is offline.
+    // Test logging in via the user/login/openid page while the site is offline.
     $edit = array('openid_identifier' => $identity);
-    $this->drupalPost('user/login', $edit, t('Log in'));
+    $this->drupalPost('user/login/openid', $edit, t('Log in'));
 
     // Check we are on the OpenID redirect form.
     $this->assertTitle(t('OpenID redirect'), 'OpenID redirect page was displayed.');
@@ -211,7 +212,8 @@ class OpenIDFunctionalTest extends OpenIDTestBase {
 
     $this->assertLink(t('Log out'), 0, 'User was logged in.');
 
-    // Verify user was redirected away from user/login to an accessible page.
+    // Verify user was redirected away from user/login/openid to an accessible
+    // page.
     $this->assertText(t('Operating in maintenance mode.'));
     $this->assertResponse(200);
   }

core/modules/openid/lib/Drupal/openid/Tests/OpenIDTestBase.php

@@ -19,7 +19,7 @@ abstract class OpenIDTestBase extends WebTestBase {
    *
    * @var array
    */
-  public static $modules = array('block', 'openid');
+  public static $modules = array('block', 'node', 'openid');
 
   function setUp() {
     parent::setUp();
@@ -40,6 +40,10 @@ abstract class OpenIDTestBase extends WebTestBase {
       ))
       ->execute();
 
+    // Use a different front page than login page for testing OpenID login from
+    // the user login block.
+    config('system.site')->set('page.front', 'node')->save();
+
     // Create Basic page and Article node types.
     if ($this->profile != 'standard') {
       $this->drupalCreateContentType(array('type' => 'page', 'name' => 'Basic page'));
@@ -53,7 +57,7 @@ abstract class OpenIDTestBase extends WebTestBase {
   function submitLoginForm($identity) {
     // Fill out and submit the login form.
     $edit = array('openid_identifier' => $identity);
-    $this->drupalPost('', $edit, t('Log in'));
+    $this->drupalPost('', $edit, t('Log in'), array(), array(), 'openid-login-form');
 
     // Check we are on the OpenID redirect form.
     $this->assertTitle(t('OpenID redirect'), 'OpenID redirect page was displayed.');

core/modules/openid/openid.css

@@ -1,44 +1,21 @@
-
+input[name="openid_identifier"] {
-#edit-openid-identifier {
   background-image: url("login-bg.png");
   background-position: left 50%; /* LTR */
   background-repeat: no-repeat;
   padding-left: 20px; /* LTR */
 }
-div.form-item-openid-identifier {
+.js form.user-login-block.openid-login-form {
-  display: block;
-}
-.js #user-login-form div.form-item-openid-identifier,
-.js #user-login div.form-item-openid-identifier {
   display: none;
 }
-#user-login-form ul {
+form.user-login-block a.openid-link,
-  margin-top: 0;
+form.user-login-block a.user-link {
-}
-#user-login ul {
-  margin: 0 0 5px;
-}
-#user-login ul li {
-  margin: 0;
-}
-#user-login-form .openid-links {
-  padding-bottom: 0;
-}
-#user-login .openid-links {
-  padding-left: 0; /* LTR */
-}
-#user-login-form .openid-links li,
-#user-login .openid-links li {
   display: none;
-  list-style: none;
 }
-.js #user-login-form li.openid-link,
+.js form.user-login-block a.openid-link,
-.js #user-login li.openid-link {
+.js form.user-login-block a.user-link {
   display: block;
-  margin-left: 0; /* LTR */
 }
-#user-login-form li.openid-link a,
+form.user-login-block a.openid-link {
-#user-login li.openid-link a {
   background-image: url("login-bg.png");
   background-position: left top; /* LTR */
   background-repeat: no-repeat;

core/modules/openid/openid.js

@@ -4,49 +4,34 @@
 
 Drupal.behaviors.openid = {
   attach: function (context) {
-    var $context = $(context);
+    var $login = $('#user-login-form');
-    var loginElements = $('.form-item-name, .form-item-pass, li.openid-link');
+    var $openid = $('#openid-login-form');
-    var openidElements = $('.form-item-openid-identifier, li.user-link');
-    var cookie = $.cookie('Drupal.visitor.openid_identifier');
 
-    // This behavior attaches by ID, so is only valid once on a page.
+    var cookie = $.cookie('Drupal.visitor.openid_identifier');
     if (cookie || location.hash === '#openid-login') {
-      var $edit_openid_identifier = $('#edit-openid-identifier');
+      $openid.show()
-      if (cookie) {
+        .find('[name="openid_identifier"]').once('openid')
-        $edit_openid_identifier.val(cookie);
+        .val(cookie);
-      }
+      $login.hide();
-      $edit_openid_identifier.once('openid', function() {
-        loginElements.hide();
-        // Use .css('display', 'block') instead of .show() to be Konqueror friendly.
-        openidElements.css('display', 'block');
-      });
     }
 
+    // Switch between the default login form and the OpenID login form.
+    $('#block-user-login').once('openid').on('click', '.openid-link, .user-link', function (e) {
+      $openid.toggle();
+      $login.toggle();
 
-    $context.find('li.openid-link')
+      var $showForm = $(this).hasClass('openid-link') ? $openid : $login;
-      .once('openid')
+      $showForm.find('input:first').focus();
-      .click(function (e) {
+      // Clear input fields and reset any validation errors.
-        e.preventDefault();
+      $showForm[0].reset();
-        loginElements.hide();
+
-        openidElements.css('display', 'block');
+      // Reset error state.
-        // Remove possible error message.
+      $('#messages').find('div.error').hide();
-        $('#edit-name, #edit-pass').removeClass('error');
+      $('#block-user-login').find('input').removeClass('error');
-        $('div.messages.error').hide();
+
-        // Set focus on OpenID Identifier field.
+      // Forget saved identifier.
-        $('#edit-openid-identifier')[0].focus();
+      $.cookie('Drupal.visitor.openid_identifier', null);
-      });
+    });
-    $context.find('li.user-link')
-      .once('openid')
-      .click(function (e) {
-        e.preventDefault();
-        openidElements.hide();
-        loginElements.css('display', 'block');
-        // Clear OpenID Identifier field and remove possible error message.
-        $('#edit-openid-identifier').val('').removeClass('error');
-        $('div.messages.error').css('display', 'block');
-        // Set focus on username field.
-        $('#edit-name')[0].focus();
-      });
   }
 };
 

core/modules/openid/openid.module

@@ -16,6 +16,13 @@ function openid_menu() {
     'type' => MENU_CALLBACK,
     'file' => 'openid.pages.inc',
   );
+  $items['user/login/openid'] = array(
+    'title' => 'OpenID',
+    'page callback' => 'drupal_get_form',
+    'page arguments' => array('openid_login_form'),
+    'access callback' => 'user_is_anonymous',
+    'type' => MENU_LOCAL_TASK,
+  );
   $items['user/%user/openid'] = array(
     'title' => 'OpenID identities',
     'page callback' => 'openid_user_identities',
@@ -41,7 +48,7 @@ function openid_menu() {
  */
 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') {
+  if ($menu_site_status == MENU_SITE_OFFLINE && user_is_anonymous() && in_array($path, array('openid/authenticate', 'user/login/openid'))) {
     $menu_site_status = MENU_SITE_ONLINE;
   }
 }
@@ -120,56 +127,84 @@ function openid_user_logout($account) {
 }
 
 /**
- * Implements hook_form_FORM_ID_alter().
+ * Implements hook_block_view_MODULE_DELTA_alter().
+ *
+ * Adds the OpenID login form to the user login block.
+ *
+ * @see user_login_block()
  */
-function openid_form_user_login_block_alter(&$form, &$form_state) {
+function openid_block_view_user_login_alter(&$block) {
-  _openid_user_login_form_alter($form, $form_state);
+  // Only alter the block when it is non-empty, i.e. when no user is logged in.
+  if (!isset($block['content'])) {
+    return;
+  }
+
+  $block['content']['openid_login_form'] = drupal_get_form('openid_login_form');
+  $block['content']['openid_login_form']['#attributes']['class'][] = 'user-login-block';
+  $block['content']['openid_login_form']['openid_identifier']['#size'] = $block['content']['user_login_form']['name']['#size'];
+
+  // Add links between the forms.
+  $block['content']['user_login_form']['openid_link'] = array(
+    '#type' => 'link',
+    '#title' => t('Log in using OpenID'),
+    '#href' => '#openid-login',
+    '#options' => array('external' => TRUE),
+    '#attributes' => array('class' => array('openid-link')),
+    '#attached' => array('css' => array(
+      drupal_get_path('module', 'openid') . '/openid.css'),
+    ),
+    '#weight' => 5,
+  );
+  $block['content']['openid_login_form']['user_link'] = array(
+    '#type' => 'link',
+    '#title' => t('Cancel OpenID login'),
+    '#href' => '#nogo',
+    '#options' => array('external' => TRUE),
+    '#attributes' => array('class' => array('user-link')),
+    '#weight' => 5,
+  );
+  $block['content']['openid_login_form']['links'] = $block['content']['user_login_form']['links'];
 }
 
 /**
- * Implements hook_form_FORM_ID_alter().
+ * Form constructor for the OpenID login form.
+ *
+ * @see openid_login_form_submit()
  */
-function openid_form_user_login_alter(&$form, &$form_state) {
+function openid_login_form($form) {
-  _openid_user_login_form_alter($form, $form_state);
-}
-
-function _openid_user_login_form_alter(&$form, &$form_state) {
   $form['#attached']['library'][] = array('openid', 'drupal.openid');
-  if (!empty($form_state['input']['openid_identifier'])) {
-    $form['name']['#required'] = FALSE;
-    $form['pass']['#required'] = FALSE;
-    unset($form['#submit']);
-    $form['#validate'] = array('openid_login_validate');
-  }
-
-  $items = array();
-  $items[] = array(
-    'data' => l(t('Log in using OpenID'), '#openid-login', array('external' => TRUE)),
-    'class' => array('openid-link'),
-  );
-  $items[] = array(
-    'data' => l(t('Cancel OpenID login'), '#', array('external' => TRUE)),
-    'class' => array('user-link'),
-  );
-
-  $form['openid_links'] = array(
-    '#theme' => 'item_list',
-    '#items' => $items,
-    '#attributes' => array('class' => array('openid-links')),
-    '#weight' => 1,
-  );
-
-  $form['links']['#weight'] = 2;
 
   $form['openid_identifier'] = array(
     '#type' => 'textfield',
-    '#title' => t('Log in using OpenID'),
+    '#title' => t('OpenID'),
-    '#size' => $form['name']['#size'],
     '#maxlength' => 255,
-    '#weight' => -1,
     '#description' => l(t('What is OpenID?'), 'http://openid.net/', array('external' => TRUE)),
+    '#required' => TRUE,
   );
-  $form['openid.return_to'] = array('#type' => 'hidden', '#value' => url('openid/authenticate', array('absolute' => TRUE, 'query' => user_login_destination())));
+
+  $form['openid.return_to'] = array(
+    '#type' => 'hidden',
+    '#value' => url('openid/authenticate', array('absolute' => TRUE, 'query' => user_login_destination())),
+  );
+
+  $form['actions'] = array('#type' => 'actions');
+  $form['actions']['submit'] = array('#type' => 'submit', '#value' => t('Log in'));
+
+  return $form;
+}
+
+/**
+ * Form submit handler for openid_login_form().
+ *
+ * @see openid_login_form()
+ */
+function openid_login_form_submit($form, &$form_state) {
+  $return_to = $form_state['values']['openid.return_to'];
+  if (empty($return_to)) {
+    $return_to = url('', array('absolute' => TRUE));
+  }
+
+  openid_begin($form_state['values']['openid_identifier'], $return_to, $form_state['values']);
 }
 
 /**
@@ -289,18 +324,6 @@ function openid_form_user_register_form_alter(&$form, &$form_state) {
   }
 }
 
-/**
- * Login form _validate hook
- */
-function openid_login_validate($form, &$form_state) {
-  $return_to = $form_state['values']['openid.return_to'];
-  if (empty($return_to)) {
-    $return_to = url('', array('absolute' => TRUE));
-  }
-
-  openid_begin($form_state['values']['openid_identifier'], $return_to, $form_state['values']);
-}
-
 /**
  * The initial step of OpenID authentication responsible for the following:
  *  - Perform discovery on the claimed OpenID.
@@ -1079,7 +1102,7 @@ function openid_cron() {
  * Implements hook_library_info().
  */
 function openid_library_info() {
-  $libraries['openid'] = array(
+  $libraries['drupal.openid'] = array(
     'title' => 'OpenID',
     'version' => VERSION,
     'js' => array(
@@ -1090,7 +1113,8 @@ function openid_library_info() {
     ),
     'dependencies' => array(
       array('system', 'drupal'),
-      array('system', 'jquery.cookie')
+      array('system', 'jquery.cookie'),
+      array('system', 'jquery.once'),
     ),
   );
 

core/modules/user/user.module

@@ -747,16 +747,21 @@ function user_login_block($form) {
     '#size' => 15,
     '#required' => TRUE,
   );
-  $form['actions'] = array('#type' => 'actions');
-  $form['actions']['submit'] = array('#type' => 'submit',
-    '#value' => t('Log in'),
-  );
   $items = array();
   if (config('user.settings')->get('register') != USER_REGISTER_ADMINISTRATORS_ONLY) {
     $items[] = l(t('Create new account'), 'user/register', array('attributes' => array('title' => t('Create a new user account.'))));
   }
   $items[] = l(t('Request new password'), 'user/password', array('attributes' => array('title' => t('Request new password via e-mail.'))));
-  $form['links'] = array('#theme' => 'item_list', '#items' => $items);
+  $form['links'] = array(
+    '#theme' => 'item_list',
+    '#items' => $items,
+    '#weight' => 10,
+  );
+  $form['actions'] = array('#type' => 'actions');
+  $form['actions']['submit'] = array('#type' => 'submit',
+    '#value' => t('Log in'),
+    '#weight' => 20,
+  );
   return $form;
 }
 
@@ -837,7 +842,7 @@ function user_block_view($delta = '') {
       if (!$user->uid && !(arg(0) == 'user' && !is_numeric(arg(1)))) {
 
         $block['subject'] = t('User login');
-        $block['content'] = drupal_get_form('user_login_block');
+        $block['content']['user_login_form'] = drupal_get_form('user_login_block');
       }
       return $block;
 
@@ -1179,6 +1184,12 @@ function user_menu() {
     'access callback' => 'user_is_anonymous',
     'type' => MENU_DEFAULT_LOCAL_TASK,
   );
+  // Other authentication methods may add pages below user/login/.
+  $items['user/login/default'] = array(
+    'title' => 'Username and password',
+    'type' => MENU_DEFAULT_LOCAL_TASK,
+    'weight' => -10,
+  );
 
   $items['user/register'] = array(
     'title' => 'Create new account',
@@ -3078,7 +3089,8 @@ function user_modules_uninstalled($modules) {
  */
 function user_login_destination() {
   $destination = drupal_get_destination();
-  if ($destination['destination'] == 'user/login') {
+  // Modules may provide login pages under the "user/login/" path prefix.
+  if (preg_match('@^user/login(/.*|)$@', $destination['destination'])) {
     $destination['destination'] = 'user';
   }
   return $destination;