In #1958950: file_entity_access fail check with "administer files" permission it was pointed out that the following code lives in file_entity_access() in the beginning portion of the logic:
if (!$file && $op == 'create') {
return user_access('create files', $account) || user_access('bypass file access', $account);
}
This check is redundant because:
- 'bypass file access' is checked just a couple lines below in the same function as expected since it should return TRUE regardless of $op, $file, or $account.
- 'create files' is checked in file_entity_file_entity_access()
Comments
Comment #1
dave reidComment #2
dave reidLet's add a test for the 'administer files' permission as well since it came up in the other issue.
Comment #4
dave reidTry this again.
Comment #5
dave reidComment #7
devin carlson commentedGood catch!
#4 looks good; it just needs the permissions to match the comment.
Comment #8
dave reidI was confused when testing this. file_entity when installed should be adding the 'view files' permission to the authenticated user role. Why when I create a new testing user should they not be able to view files?
Comment #9
devin carlson commentedThat makes sense to me. I know node.module cheats by having the
access contentpermission added to both anonymous and authenticated roles by the standard and minimal installation profiles.Should automatically granting view permissions during installation be done in this issue or in a follow-up?
Also,
testFileEntityAccess()basically tests each permission in isolation, so should any automatically granted permissions be removed during the test setup?Comment #10
devin carlson commented#7: 1959026-7-file-entity-access-redundant-check.patch queued for re-testing.
Comment #12
devin carlson commented#7: 1959026-7-file-entity-access-redundant-check.patch queued for re-testing.
Comment #13
devin carlson commentedCommitted #7 to 7.x-2.x. Leaving any permission/test changes to follow-ups.