filefield_edit_access() and filefield_view_access() should use content_access() instead of checking content_permissions specific

+1. I am working on a module that provides fine-grained access control to fields based on TAC, and filefield's current implementation of access control prevents my module from restricting access to filefield downloads.

I'll test the patch within the next day or so.

Patch works perfectly, thanks.

Marking RTBC.

Thanks very much xjm and dsnopek. I think you are both 100% correct in these changes. I also agree with dsnopek's comment in #2 that doing the same check twice is unnecessary. In most cases you're going to be working with a single file and a single check, so doing the same check multiple times for the same file is unnecessary. There are a few situations where the lines could make things slightly more efficient, but I think generally they'll cause the opposite. So this patch is the same as #2 with the duplicate check removed. Please reopen if you find any problems with this change. I've committed it to CVS. Thanks again!

I've had issues with this fix for fields that appear in more than one content type. The array returned by content_fields($field_name), that's then sent to content_access() on line 479 of filefield.module returns data only relevant to one of those content types which can cause issues in your own implementation of hook_field_access.

For example, you have field_myfile in both the page and story content type. The field data sent to content_access() will have a type_name set to only one of these content types regardless of which content type you're actually uploading the file through.

My workaround was to pass the content type name to content_fields()

Thanks, this indeed looks like a problem. I'll give this a test next time I'm working on FileField.

Since the latest release of filefield I had problems on a website uploading images with imagefield. Before it worked fine.
Others then the admin are not allowed anymore to upload images in imagefields using AHAH and they got a message

An HTTP error 0 occurred.
/filefield/ahah/story/field_image/0

my workaround was to change in filefield.module line 478:

function filefield_edit_access($field_name) {
  if (!content_access('edit', content_fields($field_name))) {
    return FALSE;
  }
  // No content permissions to check, so let's fall back to a more general permission.
  return user_access('access content');
}

temporary to

function filefield_edit_access($field_name) {
  return TRUE;
}

So, in my case, it has to do something with the permissions (or checking the permissions in AHAH)
Because the above hack worked, I think this has something to do with this patch.

I found out what happened in my specific case:
- the filefield module version 3.3 checks since this release exclusively for 'access content' permissions, I gave my client permission to 'administer nodes', but not specific for 'node access' permissions.
Why? Administer nodes normally gives permission to access content, see comments in node.module:
* In determining access rights for a node, node_access() first checks
* whether the user has the "administer nodes" permission. Such users have
* unrestricted access to all nodes. Then the node module's hook_access()
* is called, and a TRUE or FALSE return value will grant or deny access.
.

Therefore, I propose the folllowing patch to make filefield more compatible with the node,module permissions hierarchy:

function filefield_edit_access($field_name) {
  if (!content_access('edit', content_fields($field_name))) {
    return FALSE;
  }
  // No content permissions to check, so let's fall back to a more general permission.
-  return (user_access('access content'));
+  return (user_access('access content')||user_access('administer nodes'));
}

That sounds good Hanno, though it should probably be combined with the patch in #8, since that's also a problem.

I have combined them. @Quicksketch I never contributed patches, so I'm not sure if the patch will work. Could you have a look?

Index: filefield.module
===================================================================
--- filefield.module
+++ filefield.module
@@ -36,7 +36,7 @@
     'page callback' => 'filefield_js',
     'page arguments' => array(2, 3, 4),
     'access callback' => 'filefield_edit_access',
-    'access arguments' => array(3),
+    'access arguments' => array(2, 3),
     'type' => MENU_CALLBACK,
   );
   $items['filefield/progress'] = array(
@@ -475,10 +475,10 @@
  * The content_permissions module provides nice fine-grained permissions for
  * us to check, so we can make sure that the user may actually edit the file.
  */
-function filefield_edit_access($field_name) {
-  if (!content_access('edit', content_fields($field_name))) {
+function filefield_edit_access($type_name, $field_name) {
+  if (!content_access('edit', content_fields($field_name, $type_name))) {
     return FALSE;
  }
  // No content permissions to check, so let's fall back to a more general permission.
-  return (user_access('access content'));
+  return (user_access('access content')||user_access('administer nodes'));
}

Thanks Hanno. That serves as a good start, we need to do the same thing for filefield_view_access(). I've committed this patch which wraps together #8, #11, plus similar changes to the filefield_view_access() function.

Sorry to reply to this already closed ticket, but this seems like a knowledgeable group and the other thread (HTTP error 0) is super long.

We've recently run into the HTTP error 0 issue as well, but narrowed it down to a problem somehow related to Organic Groups. When using Organic groups, users get assigned a role but only within the context of the group. So group users don't have site-wide permissions, just within the group. This appears to cause a problem with the AHAH javascript when using the Upload button. A user without site-wide edit access to the file field in question is thrown the HTTP error 0 when pressing the Upload button, but can successfully submit the whole edit form using the Preview or Save buttons. A user WITH site-wide edit access to the file field in question does NOT get the error.

When I give the "Authenticated User" role site-wide edit access to the file field in question the AHAH error goes away for all users. So there seems to be some disconnect with the permission check within the group context. I wonder if this isn't related to the content permission specific checks in the associated patch here.

Thoughts? Should I open another ticket or report this to another project?

Thanks,
-Tim

I'd say open a separate issue if you have a specific, unique problem, or reply to the existing one on that topic, unless you are confident your error is related to the patch in this issue. Doing otherwise only makes it harder for others to troubleshoot and find the info they need. (It is helpful to add references to related issues on both ends, though: #297035: HTTP error 0 .)

xjm,
Thanks. I'll reply in #297035.

-Tim