$return = entity_access('update', 'file', $entity); does not return anything as entity of type file does not have an access callback defined, so attempting to PUT a file always returns access denied.

Fixed by always returning TRUE for entities of type file in _uuid_services_entity_access():

    // File does not seem to have an access callback by default.
    if($entity_type == 'file' && user_access('save file information')) {
      return TRUE;
    }

Comments

aspilicious’s picture

Status: Needs review » Needs work

Hmmm: http://drupalcontrib.org/api/drupal/contributions!entity!entity.module/f...

Return value
boolean Whether access is allowed or not. If the entity type does not specify any access information, NULL is returned.

I don't think hacking away file is a good generic plan. If it returns NULL it means there is no access information available so we just should return true. Entity module (or core or whatever module that is responsible) should add a callback for files.

Or am I missing something?

aspilicious’s picture

Status: Needs work » Needs review
StatusFileSize
new1.06 KB

And a patch, that implements #1

Status: Needs review » Needs work

The last submitted patch, 1469716-entity-access-3.patch, failed testing.

aspilicious’s picture

Status: Needs work » Needs review

That test always fails... even without this patch.

recidive’s picture

Status: Needs review » Needs work

@aspilicious: we need to add a check to see if $return is !== FALSE, or we are just giving access to everyone for everything.

Something like that:

    $return = entity_access($op, $entity_type, $entity);

    // If there is no entity access information available, return TRUE.
    if (empty($return) && $return !== FALSE) {
      return TRUE;
    }

    return $return;

or check for === NULL

    $return = entity_access($op, $entity_type, $entity);

    // If there is no entity access information available, return TRUE.
    if ($return === NULL) {
      return TRUE;
    }

    return $return;
aspilicious’s picture

StatusFileSize
new859 bytes

You're totaly correct!

aspilicious’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 1469716-entity-access-6.patch, failed testing.

aspilicious’s picture

Version: 7.x-1.0-alpha3 » 7.x-1.x-dev
Status: Needs work » Needs review

Stupid tests, nothing to do with this patch

dixon_’s picture

Status: Needs review » Needs work

Hmm, I have my doubts here.

What if a module implements an access callback, and the access callback might be constructed in a way that it only returns TRUE when access is to be given and nothing (NULL) in all other cases. NULL in that case would mean access denied, but here we will let this through.

Maybe we should do something similar to what entity_access is doing -- fetch the entity info, check if we have 'access callback'. If we don't, always return TRUE. But if we do have an access callback return what that callback is giving us.

recidive’s picture

@dixon_ in the long run your solution looks the more appropriate.

dixon_’s picture

Status: Needs work » Fixed

I rolled and committed a patch with my logic explained in #10. I also took the opportunity to wrap more code in the try {} statement to catch potential errors in other parts of the access callback.

http://drupalcode.org/project/uuid.git/commit/22c3ae3f24e19ad411f14cb08b...

Thanks

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

dillix’s picture

Priority: Normal » Critical
Status: Closed (fixed) » Active

There is a mistake in uuid_services.module:

elseif (!empty($entity_id)) {
      $entities = entity_load($entity_type, $entity_ids);
      $entity = reset($entities);
    }

Should be:

elseif (!empty($entity_ids)) {
      $entities = entity_load($entity_type, $entity_ids);
      $entity = reset($entities);
    }
carlhinton’s picture

StatusFileSize
new555 bytes

Here's a patch to implement #14

carlhinton’s picture

Status: Active » Reviewed & tested by the community
dillix’s picture

+1 for RTBC

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 15: uuid-fix-make-local-1469716-14.patch, failed testing.

dkingofpa’s picture

Issue summary: View changes
Priority: Critical » Normal
Status: Needs work » Closed (fixed)

@dillix, @CarlHinton Please don't re-open a Closed (fixed) issue. Instead, you can open a new issue with your patch. Setting metadata back to the closed values.