First I will lead off with that I created an administrator role that has been granted all permissions on the site. I have a user that has this role. Also, all caches have been cleared several times and there are no pending updates required for the db.

As user1 I had created a new entity type 'organization' and its first bundle 'corporate membership'. After switching to my non user1 administrator user I could no longer see the 'manage fields' and 'manage display' options. I also tried entering the url to those pages '/admin/structure/eck/organization/corporate_membership/fields' and was denied access.

Upon further investigation I found a few more issues with permissions which I will summarize here:

Entity Type page (/admin/structure/eck):
- Can create a new entity, but once it is created I cannot access it.

{Entity Type} Bundles page:
- 'Edit {entity type}' tab not available. example: on /admin/structure/eck/organization
- Denied access to edit page /admin/structure/eck/organization/edit
- 'Add a(n) {Entity type} Bundle' allows me to create a new bundle with a label, but after saving when brought to the .../edit page for the bundle I get 'You are not authorized to access this page.'

{Bundle Type} Entities page:
- 'Manage fields' and 'Manage display' tabs are not visible and access denied to those pages.

The problems where I create an entity or bundle and then cannot access it later is due to the dynamic nature of how the entity permissions work. So I would suggest having add/edit/view/delete permissions for all entities and bundles. i.e. 'edit all entity types'.

p.s. Just want to say that this is a super awesome module and will be a very powerful tool.

CommentFileSizeAuthor
#5 permissions-overhaul-1336754.patch19.66 KBacrazyanimal
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fmizzell’s picture

fmizzell CreditAttribution: fmizzell commented

First of all thank you so much for the through review. Second, I feel your pain. I am currently trying to create useful roles for my site and am running into some of the same problems that you have so lets see how much of this I can help you with, and we will see what all I can do in ECK to make this better.

Ok, First of all as you notice user1 and the "administrator" role do not behave the same. This is not something that ECK can fix, we should check the core issue queue to see if anyone has complained about this, or create an issue ourselves.

Now, about not having access to "manage field" and "manage display". These two tabs are created by the Field UI module, and there are no permissions associated with that module :(. So we are out of luck on that one.. and once again should check the core issue queue or add an issue.

You are right.. all of the other problems are caused by the dynamic nature of ECK permissions. the "administrator" role is advertised as having all permissions, but it will not get the new permissions from a new entity type created with ECK. I guess we can solve that by adding all ECK permissions to the "administrator" role when they are created. I have not checked into how difficult that would be, but I imagine that it is trivial. Also, I like your idea of having a catch all permission for handling everything.

I am currently working on a major clean up of ECKs code and I will add this too the list.. I will post back once the new features are in.

PS. Do not trust the main administration page of permissions, as you can see, once a permission is given to the "authenticated" role, all of the checkboxes of that permission for other roles get automatically checked. Well I have found this to be deceiving, and that if something is not working it is important to go to that role's page and give the permission if it is not checked. Good Luck.

acrazyanimal’s picture

acrazyanimal CreditAttribution: acrazyanimal commented

Cool beans, thanks for responding so swiftly. I was actually working on a patch to eck permissions today for my own purposes. I didn't get 100% finished, but I was intending on sharing it here sometime tomorrow if that will help you out any. Here are a couple points of discussion:

1. You have administer/add/edit/view permissions for each entity type and bundle type that are created. It seems to me that the 'administer' permission should be all encompassing at the particular level it deals with (parent entity, bundle, entity). This is not the way the code currently works however. In order to add a bundle of a particular type you need explicitly the add {bundle type} permission. It would make sense to also allow anyone with the administer permission to add as well. Same with edit, delete and view.

2. I think it would also make sense to grant roles that have the 'administer entities' permission to also add entities, etc. So the 'administer entities' permission would become the all encompassing permission for the parent entity level.

3. If an 'administer bundles' permission were added as an all encompassing permission for all bundle types then that would go along way to solving the issue where you add a bundle, but cannot edit, view or delete it. I don't think it would make sense to have the dynamic permissions automatically granted to an administrator role as I could foresee that being a security issue.

4. In the case of the 'add' permissions I can see two options here and I don't think you can have it both ways. One, if a user has the 'add' permission it automatically also assumes they have edit, delete and view permissions; in this case then it becomes redundant with the 'administer' permission. Or, second, keep it the way it is and if a role only has the 'add' permission then they can only add, period. I can't really envision how common this case would be, but I can see creating a role that has a combo of some of those permissions, like add, edit and view, but not delete for example. However, in this case if a person does not have the edit role then they should not be forwarded (via drupal_goto) to the edit page, but rather the page they clicked 'add whatever' from.

Anyway, that pretty much summarizes the changes I was in the process of making today. Let me know if you have any comments.

I will also look into the fields ui issues.

peace,
Jesse

acrazyanimal’s picture

acrazyanimal CreditAttribution: acrazyanimal commented

looks like the 'manage fields' and 'manage display' tabs are not showing up due to the wrong permission being passed into the entity info hook for the bundles. 'access arguments' => array('administer entities') on line 599 of eck.entity_type.inc. I haven't tested fixing this, but will test and add to my patch tomorrow.

fmizzell’s picture

fmizzell CreditAttribution: fmizzell commented

I definitely think that having some more comprehensive permissions at each level would be very useful.
So, like you said, if someone has an "administer entity types" permissions, they can do everything with ECK without having to set up individual permissions. Right now "administer entity types" will give you access to the list of all the entity types , so this permission should maybe be named "view entity types list" or something like that. That would leave "administer entity types" free to be used as you described, and it would make more sense.

So if we have an entity type called "Post", with a bundle called "Blog" we should also have a comprehensive permission to "administer post", and one to "administer post blog". I believe this is exactly what you are proposing, I just want to make sure I got it right.

Now, on the whole discussion on 4. I am sure that we could offer a few permissions for common cases, but at this point if you really need such granularity in the permissions, it won't hurt you to click 3 boxes over one to get exactly what you need. So I would not worry about it, just implementing the more comprehensive permissions would be a great improvement over what we have right now.

acrazyanimal’s picture

acrazyanimal CreditAttribution: acrazyanimal commented
FileSize
permissions-overhaul-1336754.patch19.66 KB

Ok here is a patch for the permissions. I actually changed quite a bit so I hope you don't mind. There are now lots of permissions but it gives you very fine grained control. So all 'administer' permissions are all encompassing at the particular level they are at. Then there are edit, add, list and delete permissions that only grant those functions. The new list permissions (shown in the permissions configuration page as View list of {something}) just gives access to a role to see the list on that page. For example 'View List of membership Bundles' would give a user the ability to see the bundles table on the '/admin/structure/eck/membership' page.

The attached patch is against the 7.x-1.x branch using git diff. Anyway try it out and hopefully you like it.

fmizzell’s picture

fmizzell CreditAttribution: fmizzell commented
Status: Active » Needs review
fmizzell’s picture

fmizzell CreditAttribution: fmizzell commented
Version: 7.x-1.1-alpha3 » 7.x-2.x-dev
Status: Needs review » Fixed

Finally got a chance to checkout the patch.. thank you so much for your work.. it looks awesome.. and it is a great start to a better permissions handling.. I am going to open a general Issue to continue to discuss a better permissions system if we need too #1374150: Better Permissions. Once again thanks @acrazyanimal. The patch was applied to 7.x-2.x

Status: Fixed » Closed (fixed)

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

  • Commit 90bbc6f on 7.x-2.x, change_paths, own_permissions, 7.x-2.x-property-widgets, 7.x-3.x, entity_reference, 7.x-3.x-settings, 7.x-2.0.x, 7.x-2.1.x authored by acrazyanimal, committed by fmizzell: 
    #1336754 Better permissions

  • Commit 90bbc6f on 7.x-2.x, change_paths, own_permissions, 7.x-2.x-property-widgets, 7.x-3.x, entity_reference, 7.x-3.x-settings, 7.x-2.0.x, 7.x-2.1.x authored by acrazyanimal, committed by fmizzell: 
    #1336754 Better permissions