8

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?

  • add one more slash to php comment in page callback :) syntax highlighting is not working well – xurshid29 Apr 3 '14 at 13:01
  • 5
    I think, but am not positive, that if you add this path to a menu some very weird things will happen b/c the access callback is used to figure out if the menu item can be displayed for a particular user. – mpdonadio Apr 3 '14 at 13:02
6

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.

  • This sounds like an interesting approach... – Felix Eve Apr 4 '14 at 8:01
  • Confirmed this works. Way cleaner than redirecting in the access callback. Add to hook_menu_alter: $items['player/my_page']['delivery callback'] = 'player_my_page_delivery'; – wxactly Sep 26 '16 at 23:29
3

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.

  • This makes sense, and I think MPD's comment also makes a good point. – Felix Eve Apr 4 '14 at 8:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.