0

Based on the Examples module, I tried to set up permissions on some user account fields so that only admins can read them.

However, the code below somehow allows users to register with the same email address multiple times. I can't figure out why it is doing this because I am only changing the access permissions; however, when I comment out the following code and clear cache, I cannot register the same email address twice, but when the code is active, I can.

I assume I must be doing something very incorrect to introduce such a problem; I would much appreciate any insight into what I'm doing wrong.

I defined two permissions, one for restricting view + edit access to admins, and another for allowing the user to view but not edit data.

First, I added the permissions to mymodule.permissions.yml:

'administer admin fields':
  title: Administer settings for admin fields.
'view any viewonly fields':
  title: View any viewonly fields.

Then I added this function to mymodule.module:

// Use statements to support hook_entity_field_access.
use Drupal\Core\Field\FieldDefinitionInterface;
use Drupal\Core\Session\AccountInterface;
use Drupal\Core\Field\FieldItemListInterface;
use Drupal\Core\Access\AccessResult;

/**
 * Implements hook_entity_field_access().
 */
function mymodule_entity_field_access($operation, FieldDefinitionInterface $field_definition, AccountInterface $account, FieldItemListInterface $items = NULL)
{
  {
    // Start of part I changed from examples module
    // Fields that only admins should be able to see and edit.
    $admin_fields = array(
      'user.user.field_ip',
      'user.user.field_nicknamehistory',
      'user.user.field_useragent',
      // Fields users can view but not edit.
      'user.user.field_age',
    );
    $view_not_edit_fields = array(
      'user.user.field_age',
    );

    if (method_exists($field_definition, 'getOriginalId') && !(in_array($field_definition->getOriginalId(), $admin_fields))) {
      return AccessResult::neutral();
    }
    // End of part I changed from examples module

    // First we'll check if the user has the 'superuser'
    // permissions that node provides. This way administrators
    // will be able to administer the content types.
    if ($account->hasPermission('bypass node access')) {
      return AccessResult::allowed();
    }
    if ($account->hasPermission('administer content types', $account)) {
      return AccessResult::allowed();
    }
    if ($account->hasPermission('administer admin fields', $account)) {
      return AccessResult::allowed();
    }

    // For anyone else, it depends on the desired operation.
    // Start of part I changed from examples module
    if (method_exists($field_definition, 'getOriginalId') && in_array($field_definition->getOriginalId(), $view_not_edit_fields)) {
      if ($operation == 'view' and $account->hasPermission('view any viewonly fields')) {
        return AccessResult::allowed();
      }
    }
    // End of part I changed from examples module

    // Anything else on this field is forbidden.
    return AccessResult::forbidden();
  }
}

And for reference, here is the code from the Examples module:

// Use statements to support hook_entity_field_access.
use Drupal\Core\Field\FieldDefinitionInterface;
use Drupal\Core\Session\AccountInterface;
use Drupal\Core\Field\FieldItemListInterface;
use Drupal\Core\Access\AccessResult;

// Interfaces used by entities to declare "ownership".
use Drupal\user\EntityOwnerInterface;
use Drupal\user\UserInterface;

// Use statements for hook_entity_test_access.
use Drupal\Core\Entity\EntityInterface;

/**
 * Implements hook_entity_field_access().
 *
 * We want to make sure that fields aren't being seen or edited
 * by those who shouldn't.
 */
function field_permission_example_entity_field_access($operation, FieldDefinitionInterface $field_definition, AccountInterface $account, FieldItemListInterface $items = NULL) {
  // Find out what field we're looking at.  If it isn't
  // our sticky note widget, tell Drupal we don't care about its access.
  if ($field_definition->getType() != 'field_permission_example_fieldnote') {
    return AccessResult::neutral();
  }

  // First we'll check if the user has the 'superuser'
  // permissions that node provides. This way administrators
  // will be able to administer the content types.
  if ($account->hasPermission('bypass node access')) {
    drupal_set_message(t('User can bypass node access.'));
    return AccessResult::allowed();
  }
  if ($account->hasPermission('administer content types', $account)) {
    drupal_set_message(t('User can administer content types.'));
    return AccessResult::allowed();
  }
  if ($account->hasPermission('administer the fieldnote field', $account)) {
    drupal_set_message(t('User can administer this field.'));
    return AccessResult::allowed();
  }

  // For anyone else, it depends on the desired operation.
  if ($operation == 'view' and $account->hasPermission('view any fieldnote')) {
    drupal_set_message(t('User can view any field note.'));
    return AccessResult::allowed();
  }

  if ($operation == 'edit' and $account->hasPermission('edit any fieldnote')) {
    drupal_set_message(t('User can edit any field note.'));
    return AccessResult::allowed();
  }

  // At this point, we need to know if the user "owns" the entity we're attached
  // to. If it's a user, we'll use the account name to test. Otherwise rely on
  // the entity implementing the EntityOwnerInterface. Anything else can't be
  // owned, and we'll refuse access.
  if ($items) {
    $entity = $items->getEntity();
    if ((($entity instanceof EntityOwnerInterface) and
         $entity->getOwner()->getAccountName() == $account->getAccountName()) or
        (($entity instanceof UserInterface) and
         $entity->name->value == $account->getAccountName())
        ) {
      if ($operation == 'view' and $account->hasPermission('view own fieldnote')) {
        drupal_set_message(t('User can view their own field note.'));
        return AccessResult::allowed();
      }
      if ($operation == 'edit' and $account->hasPermission('edit own fieldnote')) {
        drupal_set_message(t('User can edit their own field note.'));
        return AccessResult::allowed();
      }
    }
  }
  // Anything else on this field is forbidden.
  return AccessResult::forbidden();
}


/**
 * Implements hook_ENTITY_TYPE_access().
 *
 * Note: this routine is added so we can more easily test our access code. Core
 * defines an entity_test entity that is used for testing fields in core. We add
 * this routine to make the entity_test entity editable by our tests.
 */
function field_permission_example_entity_test_access(EntityInterface $entity, $operation, AccountInterface $account, $langcode) {
  if ($operation == 'edit') {
    $perms = [
      'administer the fieldnote field',
      'edit any fieldnote',
      'edit own fieldnote',
    ];
    foreach ($perms as $perm) {
      if ($account->hasPermission($perm)) {
        return AccessResult::allowed();
      }
    }
  }
  return AccessResult::neutral();
}
/**
 * @} End of "defgroup field_permission_example".
 */
1

I'm not sure why that happens, so just a few notes. You did ask for any insight. It will certainly help to simplify your code if nothing else. Might check again in more depth later and update the answer.

  • Note that permission names are global, you might want to use something a bit more specific.
  • getOriginalId() is not a method on FieldDefinitionInterface, you should not use that. Use getTargetEntityTypeId() == 'user' and ->getFieldName() then you can also drop the repeating 'user.user.' prefixes for your names. You should always work with the interfaces that you get, method_exists() is bad in 99% of the cases. That's what interfaces are for.
  • Do not implement entity level access checks. Just check access to fields, you can safely assume that the user has access to view/edit the entity, the calling code is responsible for checking that first if necessary.
  • Make sure that you add any cache contexts (e.g. user.permissions) and other cacheable metadata if applicable with allowed()->addCacheContexts(['user.permissions']). The entity view output for example will be cached based on that. And while user.permissions by default is always included, it's good practice to include that.
  • Your view own should however probably check if the user is really looking at his own user, and then you need the user cache context.
  • Do not implement entity level access checks. Do you mean I should call a different hook instead of hook_entity_field_access()? – Patrick Kenny Feb 19 '16 at 18:34
  • That was mostly in reference to the two node checks that you copied from the example (which I think isn't that good, for example because it doesn't contain useful examples like doing something based on the field name, and includes weird things like those node permissions). Why would you want someone to have access to those special fields just because they can administer node types? – Berdir Feb 19 '16 at 19:59
  • Thanks, I fixed it based on your comments. Note that I think you meant ->getName() instead of ->getFieldName(). – Patrick Kenny Feb 20 '16 at 6:58
0

Here's the working version, based on Berdir's comments:

/**
 * Implements hook_entity_field_access().
 *
 * We want to make sure that fields aren't being seen or edited
 * by those who shouldn't.
 */
function mymodule_entity_field_access($operation, FieldDefinitionInterface $field_definition, AccountInterface $account, FieldItemListInterface $items = NULL)
{
  // Fields that only admins should be able to see and edit.
  $admin_fields = array(
    'field_age',
    'field_nicknamehistory',
    'field_useragent',
  );
  $view_not_edit_fields = array(
    'field_age',
  );

  if ($field_definition->getTargetEntityTypeId() && in_array($field_definition->getName(), $admin_fields)) {
    if ($account->hasPermission('administer admin fields', $account)) {
      return AccessResult::allowed();
    }
    else if (in_array($field_definition->getName(), $view_not_edit_fields)) && ($operation == 'view' and $account->hasPermission('view any viewonly fields'))) {
      return AccessResult::allowed();
    }
    else {
      return AccessResult::forbidden();
    }
  }
  else {
    return AccessResult::neutral();
  }
}
  • 1
    I don't know why you are checking for node as well, do those fields really exist on nodes as well? Your old code was definitely user specific. I'd also try to limit the number of returns you have. For example, move the last neutral out of the switch so it always returns that, remove the ! from the admin fields check and make it the check for the else. If you don't need node, then you can also drop the second getTargetEntityTypeID() check. – Berdir Feb 20 '16 at 10:32
  • @Berdir Thanks, I updated it. Actually I am using it for nodes too on my site, but I tried to simplify the example when testing/debugging. I updated the answer to just show the user code more cleanly. – Patrick Kenny Feb 20 '16 at 10:48

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.