6

Closed

There should be a way to set permissions for non-creatable types too

description

It makes sense to set type-level permissions even if a type is not creatable. So IMO even non-creatable types should be listed on pages of roles and there shouldn't be a check in Orchard.Core.Contents.Security.AuthorizationEventHandler ln 28.
Closed Sep 12 at 11:49 AM by Piedone
Fixed by securable metadata in 1.x.

comments

jao28 wrote Feb 13 at 12:59 AM

This is similar to https://orchard.codeplex.com/workitem/19905 where I have a "hack" solution - be great to have it resolved (and I think this one is proposing a better solution, even though it will provide a lot more permissions to manage).

Piedone wrote Jun 11 at 12:48 AM

Piedone wrote Jul 2 at 2:54 PM

mjy78 wrote Jul 25 at 2:17 AM

I just added quite a lengthy comment to work item 19905 discussing this which I probably should have added here. Please take a look and let me know your thoughts.

https://orchard.codeplex.com/workitem/19905

mjy78 wrote Jul 28 at 3:31 PM

The following fork contains a a fix for this. It adds a "Permissions Enabled" option to the content type settings.

https://orchard.codeplex.com/SourceControl/network/forks/mjy78/security

j3ffb wrote Aug 5 at 2:29 PM

I saw this discussed on the recent bug triaging and pull request meeting. Just wanted to say that the main reason I needed a fix for this issue was to be able to give permissions to individual Lists and also to give permissions to the Image content type, so users could set captions and alt text.

I think Sébastien said he has implemented a custom permission for managing individual lists (I have tried the latest 1.x code and can't see that it works) and perhaps the Image permission could be included in the 'Managing Media' permission or a new custom permission. However I do still agree with Zoltán that perhaps these custom permissions are not required and is additional code that could be handles by allowing dynamic permissions. Obviously custom permissions are needed to secure certain functionality in modules but for CRUD actions on Content Types could this just be left up to dynamic permissions? Could the creatable flag just be for whether it appears in the new list? If so, perhaps update the hint text to be clearer - 'Determines if this content type is listed in the 'New' admin menu. I think the current text is misleading as Taxonomies can be created through the UI, just not via the New menu.

Perhaps the above solution of having a flag on the type (perhaps 'Dynamic Permissions') would be a good compromise, and will work for modules that already manage this via their own permission set.

mjy78 wrote Aug 6 at 1:28 AM

Unfortunately I've been missing the triage meetings - 5am my local time - and I'm not a morning person :)

One of the changes that I've implemented in the above referenced fork adds a "Permissions Enabled" option to the content type (similar to the existing "Creatable" and "Draftable" options). I think the approach of controlling the behaviour of security on content types explicitly through its own setting, rather than this functionality being determining indirectly through some other setting (ie. currently "Creatable") is much more intuitive and puts security of content types in the mind of the end user.

The naming of the setting I've created is probably not optimal. I considered "Securable" but in my mind this implies that without this setting enabled the content types are not securable, when in fact they still can be secured via the global permissions that apply to all content types.

Piedone wrote Aug 6 at 2:36 PM

Just wanted to add some usage examples:
  • In the Shoutbox module we have a WriteMessage permission just so we can specify who can post messages through the Shoutbox. Wouldn't be necessary with dynamic permissions what we don't have now since messages are not Creatable.
  • Orchard.Comments: the AddComment permission is just needed so we specify who can post a comment. Wouldn't bee needed with dynamic permissions for Comments.
  • We have many permissions in Orchard.Blogs that are direct re-implementations of some general content permissions: PublishBlogPost, PublishOwnBlogPost, EditBlogPost, EditOwnBlogPost...
Many custom permissions implemented solely as an inferior workaround for not having all the dynamic content permissions.

sebastienros wrote Aug 6 at 8:02 PM

These dynamic permissions are defined for the backend, and if there is no reason to edit a content item in the backend, OR there is already a specific way to list/manage these items in the backend then they are not required. This is the intent of the Creatable metadata, one can create it using the backend default screens, hence why these permissions are linked to it.

"Shouldn't be necessary with dynamic permission" is a nice statement, which also implies that some other content types would be listed by would have nothing to do here. "Creatable" is the way to have these items get the dynamic permission, and because they are creatable, we add a nice useful link to the menu.

This is why I could see a discussion about separating Creatable from "Visible in the New menu", as I think mjy78 is suggesting.

Piedone wrote Aug 6 at 8:50 PM

So do we agree that there is a need for enabling arbitrary content types to have dynamic permissions? If yes, then we can discuss the specific implementation, like having a different metadata configuration for specifically this.

sebastienros wrote Aug 6 at 9:57 PM

"Arbitrary" yes.