I've run into this problem a few times now so thought I would post it to the issue queue. For the provided rules integration for entities the 'create entity' and 'fetch entity' actions use the entity_access function to determine if the action can be performed. Currently rule treats a return value of 'NULL' (which means the module providing an entity did not provide an access callback) as FALSE. I would argue that it should be considered TRUE since by not providing an access callback the module developer was not assuming anything regarding restrictions to that entity. However, other modules certainly can use 'alter' functions to add that kind of restrictions in.
Let me know what you think?
I will also upload a patch to implement this if you so choose.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | rules-alternate-use-of-entity_access-1526198-2.patch | 2.49 KB | acrazyanimal |
| #1 | rules-alternate-use-of-entity_access-1526198-1.patch | 1.29 KB | acrazyanimal |
Comments
Comment #1
acrazyanimal commentedComment #2
acrazyanimal commentedI missed a similar call to determine the access for the 'data set value' action. Here's a new patch that includes an update for this as well.
Also, I noticed that the implementation of propertyAccess in entity.wrapper.inc of the Entity API was inconsistent with the rest of the entity access related call implementations. This affects rules use of EntityMetadataWrapper::access so I'm mentioning it here as well. I've reopened an issue in the entity module here: #976360: UI fails without 'access callback' defined [PATCH]
Comment #3
acrazyanimal commentedJust realized this is set to 'task' and should probably be a classified as a bug.
Comment #4
mgiffordThis patch applies nicely to the git repository. Looking at the patch, it makes sense. Ran it through Coder and there were a lot of issues, but not related to the patch in question.
Is there a good test case to address this?
Comment #5
fagoI'm not sure about that - as it might just be that the module has not implement the access callback, but access restrictions should be in place. That way the default of assuming TRUE might lead to security issues. So let's better stay with assuming FALSE and fix entity types not providing the access callback.
Comment #6
mitchell commentedUpdated component.
Comment #7
acrazyanimal commented@fago: Crud! *sad face* ... To me it would seem that rules would be trying to fix other modules security mistakes. However, if an access callback is a required parameter in the entity info array, then I guess it does make sense to expect one and protect the security of the site if one does not exist.
Perhaps there should be a check somewhere to make sure an access callback was provided?