Drupal Tips: Avoid infinite loop in user login

Written on January 3, 2023

Category: Drupal Tips

Author: David Rodríguez, @davidjguru

Picture from Unsplash, by user @tine999
Picture from Unsplash, user Tine Ivanič, @tine999

Last week I had to perform an intervention in a Drupal installation due to an unexpected bug. It was a problem that I hadn’t seen in Drupal for a long time… But the key really is that I was the one who created the issue so I think I’m ready to explain how it happened (my own code generated it) and how to fix it… I am very pleased to join you today to share with you my latest ridiculous mistake: the infinite login redirection in Drupal (and how to solve it)!…

Well, actually and to be fair, the title should be something like “what did I do to cause an infinite loop” or maybe “How I generated an endless loop of redirections in Drupal”, but it didn’t work well as a title (marketing issues) so better that way.

Acknowledgments

This post was composed from my current position as Senior Drupal Developer at FFW Agency, one of the biggest companies oriented to Open Source in the world and specifically focused in Drupal.

Checking the node / entity access

My little problem started from a very simple model. In fact my starting idea was quite simple: to check the visibility of certain types of nodes for some users based on some taxonomy terms, that is, depending on the taxonomy term (group 1, group 2, group 3, group 4) applied to a user (group 2), this user could only see nodes labeled with the same taxonomy term (group 2), and for this, better centralize the activity using well known Drupal hooks: hook_node_access for 8.x or hook_entity_access for 9.x and above. I was looking for something that can be summarized in the following image:

Managing node access by matching taxonomy terms
Managing node access by matching taxonomy terms

For those unfamiliar with its use, hook_entity_access works like an updated version of hook_node_access, more generalized for entities and with some restrictions on the parameters of CRUD operations to be processed (create, delete, update, view) respect to hook_node_access. But in fact both share a similar scheme of work:

  1. The hooks will receive some item-to-check related params: entity/node, operation and user account.
  2. The hooks will return an access result based in AccessResultInterface: allowed, forbidden, neutral…

Here is a comparison between these two access hooks:

Entity Access and Node Access comparison
Entity Access and Node Access comparison

So I started using the version hook_node_access():

/**
 * Implements hook_node_access().
 */
function my_custom_module_access_node_access(NodeInterface $node, $op, AccountInterface $account) {

And by the way, I’m only concerned about specific types using the selected vocabulary as field:

$specific_content_types = [
  'content_type_one',
  'content_type_two',
];

Then I’m getting the whole entity for the current user:

$current_user_entity = \Drupal::entityTypeManager()->getStorage('user')->load($account->id());

I would like to think about permissions, checking all scenarios where we will be neutral in access:

  1. If is another content type then is not our business.
  2. We’re only interested in management of views of nodes.
  3. Node has not associated value in taxonomy term field.
  4. Being a registered user the account has not value in taxonomy term field.
  5. Current User has permissions to bypass access restrictions.

I could generate some control like this:

#1  if ((!in_array($node->bundle(), $specific_content_types_content_types)) || 
#2     ($op != 'view') || 
#3     ($node->get('field_groups')->isEmpty()) ||
#4     (($account->isAuthenticated()) && ($current_user_entity->get('field_vocabulary')->isEmpty())) ||
#5     ($account->hasPermission('bypass content access'))) {

        return AccessResult::neutral();
    }

But what about other scenarios? I’m interested in forbidden and allowed cases. For instance, when the node could have loaded taxonomy terms in the key field (or maybe not) but user is not authenticated:

if ($account->isAnonymous()) {
  return AccessResult::forbidden();

And finally, the main key scenario and its derivatives: user and node have loaded values in the taxonomy term field and we have to look for a possible match between them, looping all the possible array of values. Here the user is authenticated and as a first step, we have to get the existing values in taxonomy term field:

$terms_node = $node->get('field_vocabulary')->getValue();
$terms_user = $current_user_entity->get('field_vocabulary')->getValue();

And as sub-case, we have to check if user is just registered but there is no value associated with the taxonomy field. In a hurry and without thinking too much, I chose this way of extracting the value of the field, being an entity reference field:

if ($account->isAuthenticated() && ($current_user_entity->get('field_vocabulary')->getValue()[0]['target_id'] == 0)) {
  return AccessResult::neutral();
}

You know, if any of the taxonomy terms from the node items is equal to any of the user account then we’ll have granted access to the node:

// Third: existing terms in node and user we'll match between terms.
foreach ($terms_node as $node_target) {
  foreach ($terms_user as $user_target) {
    if ($node_target['target_id'] == $user_target['target_id']) {
      return AccessResult::allowed();
    }
  }
}

And finally, while you’re a non-allowed user for the node we will redirect you to the home node:

// 101 is the ID of the node marked as home page.  
$url = Url::fromRoute('entity.node.canonical', ['node' => 101]);
$redirect = new RedirectResponse($url->toString());
$redirect->send();

These ideas, these implementations that seem to have a certain logic… did not take long to cause serious problems after the deployment of the website and testing… after a while, several colleagues started to raise the alarm.

The Issue

As I said in a previous paragraph, these changes caused bugs immediately after deployment in testing environments. The most obvious one was that after the user login, with the user already registered, visiting the nodes resulted in an infinite loop of redirection that prevented access to the website:

Infinite loop of redirection causing issues by blocking access
Infinite loop of redirection causing blocking access

At first glance, this problem can be recognised with a trained Drupal troubleshooting eye: the user is already logged in, an access is attempted and denied (forbidden) and a redirect is triggered, which will contain the login screen. But the user is already logged in, so it redirects endlessly until the login is closed.

So you have to stop, review and debug the code until you find the bugs… so please enable your xdebug configuration, F5 (in my VSCode installation) and go to run a debugging… in my case I found two main problems:

  1. The way I’m getting the taxonomy terms values.
  2. The way I’m executing redirection.

Let’s check the issues.

1. The way I’m getting the taxonomy terms values

The first issue was how I was getting the taxonomy term values from the different sources in order to look for a match. You know, I’m talking about the values from the Entity Reference Field (Taxonomy Term in the Drupal standard field is Entity Reference). At some point in my code I’m just checking if any value was set in the taxonomy term field:

($current_user_entity->get('field_groups')->isEmpty())

And this expression was avoiding the pass when the field wasn’t filled, passing the action to the next else clause ordering the redirection:

$url = Url::fromRoute('entity.node.canonical', ['node' => $return_page]);
$redirect = new RedirectResponse($url->toString());
$redirect->send();

We said that the field was not set, but this is not really true, due to the value being zero, and not empty. From this point of view, may can be more useful check values in a different way:

($current_user_entity->get('field_groups')->getvalue()[0]['target_id'] == 0)

But while that’s true the response is an array of values -being the taxonomy term field a multivalue field-, if the field was not filled then the response can be an unique value that you can check in a more single way using less expressions and reducing your code:

($current_user_entity->get('field_groups')->target_id == 0)

And remember you can obtain some more taxonomy terms related info if you need by using something like this:

$terms = $node->get('field_vocabulary')->referencedEntities();
foreach ($terms as $term) {
  $name = $term->getName();
  $tid = $term->id();
  $some_field_values = $term->get('specific_field_of_the_vocabulary_term')->getValue();
}

Now we have a more controlled processing of the value for taxonomy terms, checking it and giving permissions on access.

2. The way I’m executing redirection

The second issue (well, is not really an issue but something at risk of becoming technical debt). Now I’m talking about the redirection.Remember that when you want to redirect the user’s navigation, you can use various approaches. Let’s see…

  1. Calling for the 403 system page
system.403:
  path: '/system/403'
  defaults:
    _controller: '\Drupal\system\Controller\Http4xxController:on403'
    _title: 'Access denied'
  requirements:
    _access: 'TRUE'

And so you can use a redirection based in the internal system route:

$url = Url::fromRoute('system.403');
$response = new RedirectResponse($url->toString());
  1. Loading the related node ID for redirection
    $url = Url::fromRoute('entity.node.canonical', ['node' => 101]);
    $redirect = new RedirectResponse($url->toString());
    $redirect->send();
    

What was my initial approach? I thought I’d go and ask the system what page it had loaded for redirects. Something like:

$site_config = \Drupal::config('system.site');
if (is_null($site_config->get('page')['403'])) {
  // If a 403 error page is not configured then we will redirect to /home.
  $return_page = (int) filter_var($site_config->get('page')['front'], FILTER_SANITIZE_NUMBER_INT);
}
else {
  // If 403 page exists we will redirect to error page.
  $return_page = (int) filter_var($site_config->get('page')['403'], FILTER_SANITIZE_NUMBER_INT);
}

And finally I’ll load the return page for redirection:

$url = Url::fromRoute('entity.node.canonical', ['node' => $return_page]);
$redirect = new RedirectResponse($url->toString());
$redirect->send();

What was the problem here? Well firstly, we have to avoid sending responses directly (this can be problematic due to conflicts with two responses, one from you and another from the Drupal system by itself), Then I have to think about the context: while I’m in a Drupal multisite installation, I’m not pretty sure if every single site has been registered with a specific error page, so maybe a source of errors. I need some more neutral, more… “delegated”… So for me the solution was to use Symfony AccessDeniedHttpException.

  // Fourth: As you're a non-allowed user for this node we'll redirect you.
  throw new AccessDeniedHttpException();
  }
}

This will pass the default value “403” to its parent’ constructor and so, the HttpException will be called and it will generate an autonomous response. This can be a more clean solution ;-)

Read More

:wq!


Written on January 3, 2023