Sins Against Drupal 3

This is part of my ongoing series about ways Drupal can be badly misused. These are generally times someone tried to solve an otherwise interesting problem in just about the worst possible way. All of these will start with a description of the problem, how not to solve it, and then ideas about how to solve it well.

I present these at SC Drupal Users Group meetings from time to time as an entertaining way to discuss ways we can all improve our skills.

This one was presenting during our October event here in Aiken, SC.

The Problem

Provide a custom authentication solution that allows staff to have one backend and members another.

The Sinful Solution

In order to force staff to use the staff login page, during login form validation check to see if the user is a staff member, by authenticating the user, checking their groups, and logging out staff.

The Code

/**
* Prevents staff members from logging in outside of staff login page. <<-- Why?
*/
function my_auth_staff_boot($form, &$form_state) { // NOT actually a hook_boot (thankfully) called as login form validator...
  user_authenticate($form_state['values']);
  global $user;
  if (in_array('An Employee', $user->roles)) {
    form_set_error($form['#id'], l(t('Staff must log in via staff-login', 'staff-login')), TRUE);
    drupal_set_message('Staff must log in via ' . l(t('staff-login', 'staff-login')), 'error', TRUE);
    // Load the user pages in case they have not been loaded.
    module_load_include('inc', 'user', 'user.pages');
    user_logout();
  }
}

Why is this so bad?
This code actually completes the login process before kicking the user out. Why would you ever want to do that to your users? What did they do to you? It also loads an extra file for no apparent reason just before kicking the user back out.
Better Solutions
The goal here is to control what backend the user logs into, and shouldn’t control the page they login from. So the place to look for solutions are modules that already do this and so I propose mimicking the LDAP or GAuth modules’ approaches. LDAP attaches a validator to the form and takes over authentication, but LDAP supports lots of options so the code there is too extensive to use for a clear example. So for discussion I pulled out elements of the GAuth module (although there is still lots of trimming to make this understandable).
The GAuth module adds a submit button to the form and handles all processing for that form directly.
/**
* Implements hook_form_alter().
*/
function gauth_login_form_alter(&$form, &$form_state, $form_id) {
  if ($form_id == 'user_login' || $form_id == 'user_login_block') {
    $form['submit_google'] = array(
      '#type' => 'submit',
      '#value' => t(''),
      '#submit' => array('gauth_login_user_login_submit'),
      '#limit_validation_errors' => array(),
      '#weight' => 1000,
    );
    drupal_add_css(drupal_get_path('module', 'gauth_login') . '/gauth_login.css');
  }
}

/**
* Login using google, submit handler
*/
function gauth_login_user_login_submit() {
  if (variable_get('gauth_login_client_id', FALSE)) {
// .. skipping resource validation ...

  $client = new Google_Client();
// .. skipping client setup ...
  $url = $client->createAuthUrl();
  // Send the user off to Google for processing
  drupal_goto($url);
  }
  // ... skip errors
}

From there we pass through a menu router from the main module, and an API hook to get:
function gauth_login_gauth_google_response() {
  if (isset($_GET['state'])) {
// Skipping some error traps...
    $redirect_url = isset($state['destination']) ? $state['destination'] : '';
    if (isset($_GET['code'])) {
// Skipping a bunch of Client setup...
      $oauth = new Google_Service_Oauth2($client);
      $info = $oauth->userinfo->get();
      if ($uid = gauth_login_load_google_id($info['id'])) {
        $form_state['uid'] = $uid;
        user_login_submit(array(), $form_state); // << That right there with the $form_state['uid'] set does the magic.
      }
      else {
// Skipping other options....
      }
    }
    drupal_goto($redirect_url); // << be nice and handle the destination parameter
  }
}

Share your sins
I’m always looking for new material to include in this series. If you would like to submit a problem with a terrible solution, please remove any personally identifying information about the developer or where the code is running (the goal is not to embarrass individuals), post them as a gist (or a similar public code sharing tool), and leave me a comment here about the problem with a link to the code. I’ll do my best to come up with a reasonable solution and share it with SC DUG and then here. I’m presenting next month so if you have something we want me to look at you should share it soon.
If there are security issues in the code you want to share, please report those to the site owner before you tell anyone else so they can fix it. And please make sure no one could get from the code back to the site in case they ignore your advice.

This work is licensed under a Creative Commons Attribution-NonCommercial-ShareAlike 4.0 International