OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.44k stars 2.4k forks source link

You can't pick users when you only have permissions to edit your own content. #11878

Closed Kees-Schouten closed 2 years ago

Kees-Schouten commented 2 years ago

You can't pick users when you only have permissions to edit your own content.

To Reproduce

I have a content type "Project" with a user content picker field. I'm only authorizated to edit my own created projects. When I create a new project I currently can't select users with the user picker field.

Expected behavior

I should be able to select users when I have the permission to create the content item (in this example "Project").

Proposed fix

https://github.com/OrchardCMS/OrchardCore/commit/f8fab3c2f8c5a606080b5333f70f949dcbb7a790

Screenshots

select users bug

hishamco commented 2 years ago

I think this is a bug. please create a PR for it, then e can review

Kees-Schouten commented 2 years ago

Created pull request:

https://github.com/OrchardCMS/OrchardCore/pull/11880

MikeAlhayek commented 2 years ago

I don't think this is a bug. Check the settings on your UserFieldPicker to ensure you it lists all the available users not specific roles which excludes yourself.

MikeAlhayek commented 2 years ago

@Kees-Schouten @hishamco The problem isn't permission related.

The problem occurs when a user attaches a UserPickerField to a content type, the field settings does not have default roles selected or have the DisplayAllUsers set to true so in this case, the search for a user return no records. The fix is to display all users in the UserPickerField by default to allow all users to show up. If the user want to override that behavior, then they can specific roles to select from. Without this, no users will show up in UserPickerField unless the user edit the field and set the settings.

I think the fix is ContentUserField should display all users by default unless the user specifically selected specific roles.

a temporary fix would be to edit the settings for UserPickerField and either select “Display All Users” or select specific roles to select from

Kees-Schouten commented 2 years ago

Although the change (Display All Users) is nice, it doesn't fix the bug. Unfortunately it's really an bug and I fixed it by using the code changes my PR. https://github.com/OrchardCMS/OrchardCore/pull/11880

Please follow my reproduction steps and see for your self. Make sure that role, in my case "ProjectManager" only has the permission to "Edit Project" and not "Edit Project for others".

After configuring the permissions that way, create an new Project and see that you can't select users.

MikeAlhayek commented 2 years ago

@Kees-Schouten Are you sure you are using the latest version of 1.5-preview from cloud smith? If you are, make sure you undo your change. Then, since you created your content type before this fix, you may have to edit the settings one more time to update your existing data. So edit your content type, then edit the users filed and make sure "Display All Users" is checked then save. Be sure to hit the save button even if the "Display All Users" is already selected to update the setting of the field in the database.

I also confirmed that the PR #11928 fixed this issue. I created content type called "Project" and attached a field called "Manager" to it. image

Then create a ProjectManager role like this and everything is working.

image

Kees-Schouten commented 2 years ago

Thanks the effort! Now I see where we misunderstand. When I enabled the checkboxes like you did in the section "Contents (OrchardCore.Contents)", the user picker works as expected. But In that case it's possible for the users to create all content types.

The bug occurs only when you select the checkboxes in the section "Project Content Type - Project". Because I want the users of the role "ProjectManager" only to be able to create content types of type "Project". In this case the UserPickerField doesn't work and stays empty. Project permissions

MikeAlhayek commented 2 years ago

@Kees-Schouten Even when I change the permission it still works as expected. Have you tried my previous recommendation?

Are you sure you are using the latest version of 1.5-preview from cloud smith? If you are, make sure you undo your change. Then, since you created your content type before this fix, you may have to edit the settings one more time to update your existing data. So edit your content type, then edit the users filed and make sure "Display All Users" is checked then save. Be sure to hit the save button even if the "Display All Users" is already selected to update the setting of the field in the database.

Kees-Schouten commented 2 years ago

Thanks for you fast response. Yes i'm running on main branch and I have your changes running, see:

image

I also saved multiples times the FieldSettings settings, Part settings and content type definitions.

Please check the UserPickerAdminController.cs and line 51, that's the following code:

image

It's clearly an permission problem because the debugger steps in to the Forbid response and as an result the UserPickerField stays empty. The projectmanager does not have the EditContent permission because I dont want him to create others types of content except Projects.

Kees-Schouten commented 2 years ago

I finally understand the problem (took me long enough lol).

You have to enable the permission "Edit own content" in the section "Contents (OrchardCore.Contents)". But that makes it possible for these users the Create all content of contenttypes which have don't have checked the option "Securable Determines if this content type can have custom permissions.".

I very cumbersome to enable this option for every content type and then give all the Roles the edit permissions for these content types except for the ProjectManager role.

It's debatable if this is an bug but atleast I don't like the user experience for managing the permissions this way.

MikeAlhayek commented 2 years ago

@Kees-Schouten no you don't need to grant the role permission to "Edit own content" in the section "Contents (OrchardCore.Contents)".

If you follow my previous instructions, it should work for you with no issues.

Kees-Schouten commented 2 years ago

@CrestApps your change, and correct me if I'm wrong, doesn't change anything related to the if statement:

if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditContent, contentItem))

That if statement is run in to when I don't have "Edit own content" permission and will result in Forbid response. When you remove the permission "Edit own content" your self and log off and log on again you should have the same result.

MikeAlhayek commented 2 years ago

@Kees-Schouten yes my change has nothing to do with the line you posted because that isn't the problem. The problem you keep referring to here is addressed here. So when the line if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditContent, contentItem)) is executed, we check to see if you own the given content, and if have CommonPermissions.EditOwnContent permission.

You never answered my previous questions so I can't provide you further recommendation

Are you sure you are using the latest version of 1.5-preview from cloud smith? If you are, make sure you undo your change. Then, since you created your content type before this fix, you may have to edit the settings one more time to update your existing data. So edit your content type, then edit the users filed and make sure "Display All Users" is checked then save. Be sure to hit the save button even if the "Display All Users" is already selected to update the setting of the field in the database.

Kees-Schouten commented 2 years ago

I can confirm that it works for a new content type. Also adding the user picker field afterwards and editing the user roles to show works afterwards.

For existing content types it doesn't work for me. I saved/changed the fields contentparts and content type multiple times but none seems to work for an existing content type.

I will edit the DB (PostgreSQL storage in Azure) my self to fix this problem.

MikeAlhayek commented 2 years ago

Click on your existing content-type, find the UserPicker field and click edit next to it. Not you’ll see the UserField setting. Make sure “Show All Users” is selected then click save. This should work. If you are saying that field is still not working. Please share a screen shots of your setting and the no working field

hishamco commented 2 years ago

I'm not sure why we still posting on a closed issue, please reopen or start a new discussion if something else related