Would redirecting users in hook_menu's access callback have any undesirable effects?

Question

This answer led to a discussion about the best way to redirect a user to a specific page if they shouldn't have access to the page they are trying to access.

One option is to set the access callback to true and then redirect users in the page callback. Whilst this seems valid I think it is mixing access functionality with page building functionality in the page callback.

E.g.

function hook_menu() {
    $items['player/my_page'] = array(
        'title' => t('My Page'), // note this is a required parameter
        'access callback' => TRUE,
        'page callback' => 'some_function',
    );
    return $items;
}

function some_function() {
    global $user;   
    if(!$user->uid) { // here checking if the user is logged in but could be checking for a specific permission or field value
        $dest = drupal_get_destination();
        drupal_goto('user/login', $dest); // this remembers where the user is coming from
    }
    // carry on building rest of page
}

Another option is to set the access callback function to call a function that checks if the user has access, but then instead of returning false it would redirect the user to another page. This is good as it is separating access logic and page building logic. However the purpose of an access callback is to return a boolean value, so this is breaking that logic by redirecting the user.

E.g.

function hook_menu() {
    $items['player/my_page'] = array(
        'title' => t('My Page'), // note this is a required parameter
        'access callback' => 'check_access',
        'page callback' => 'some_function',
    );
    return $items;
}

function check_access() {
    global $user;
    // here checking if the user is logged in but could be checking for a specific permission or field value
    if(!$user->uid) {
        $dest = drupal_get_destination();
        drupal_goto('user/login', $dest);
    }
    return TRUE;
}

Are there any undesirable effects from redirecting users in the access callback that I'm not aware of?

What do you think is best practice here?

Answer 1

I think you could do this by altering the delivery callback function. If the access callback returns FALSE that's what's passed to the delivery callback. If you wanted this redirect behaviour just on specific pages you could modify the delivery callback just for those pages via hook_menu() or hook_menu_alter(). If you want the behaviour to be global, you can use a hook_page_delivery_callback_alter() to change it.

Here's an example delivery callback.

function custom_deliver_html_page($page_callback_result) {
  if ($page_callback_result === MENU_ACCESS_DENIED) {
    drupal_goto('<front>');
  }
  drupal_deliver_html_page($page_callback_result);
 }

Btw this is untested, and I've never actually altered the delivery callback myself previously.

Answer 2

I would not do it in the access callback. If another developer want to alter the page_callback at some point in the future, they will end up scratching their heads as to why the callback is not working when you are redirecting in access callback.