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, 2014 at 10:49 AM by Piedone
Fixed by securable metadata in 1.x.

comments

jao28 wrote Feb 12, 2014 at 11:59 PM

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 10, 2014 at 11:48 PM

Piedone wrote Jul 2, 2014 at 1:54 PM

mjy78 wrote Jul 25, 2014 at 1: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, 2014 at 2: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, 2014 at 1: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, 2014 at 12: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, 2014 at 1: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, 2014 at 7: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, 2014 at 7: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, 2014 at 8:57 PM

"Arbitrary" yes.

jayharris wrote Jan 16 at 3:59 PM

Is there anything new on this? @mjy78's code seems reasonable. A lack of solution is causing a lot of pain, and I would prefer not to deviate from core Orchard too much.

@mjy78: Have you considered putting this in a Module in the mean time? Though the Deny functionality might be a bit difficult, I think the PermissionEnabled portion might be relatively easy to extract until a permanent solution is in place. If you don't have time but are agreeable to it, I would be happy to take your changesets and begin building a Module for it.

Piedone wrote Jan 16 at 4:32 PM

What do you mean? This is fixed.

mjy78 wrote Feb 28 at 10:54 AM

@jayharris as @Piedone mentions, I believe the latest code base now has a "Securable" option on the content type which is the same as the "Permissions Enabled" option I had added in the fork here:

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

There are still two other changes that I implemented in that fork that I would like to see considered for the product. These are discussed in my comments here: https://orchard.codeplex.com/workitem/19905

But in summary they are...
  1. Display a warning/info message in the admin if a user creates a custom form set to a) save a content item when the form is submitted and b) where the content type used by the form is NOT DRAFTABLE (so it gets published when the form is submitted) and c) where the content type has no permissions preventing it from being viewed publicly. My reasoning here is that I don't believe it's immediately obvious that when you create a custom form in Orchard that you may be allowing the form data to be publicly accessible. In the majority of use cases for forms, I suspect the intention would not be for the submitted information to be publicly accessible (think enquiry forms, contact forms, etc with user's email addresses and other private info).
  2. Add the ability to "DENY" access to certain permissions.
@jayharris If I get the opportunity I will raise separate pull requests for these two changes. The user warning I think is a pretty basic change and really needs to be in place. The "Deny" functionality probably needs more scrutiny, although I've been using it without problems for a while now.

Piedone wrote Feb 28 at 11:33 AM

mjy78 please be so kind and add these to a new issue, this is not tracked any more.

mjy78 wrote Mar 8 at 9:49 PM

Done.
  1. Security Warning on Custom Forms - https://orchard.codeplex.com/workitem/21244
  2. Explicitly DENY (negative exception) security permissions - https://orchard.codeplex.com/workitem/21245

Piedone wrote Mar 8 at 10:05 PM

Thank you!