Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forced effective Manage Media permission #12057

Closed
mrpavlitzky opened this issue Jul 21, 2022 · 12 comments · Fixed by #15173
Closed

Forced effective Manage Media permission #12057

mrpavlitzky opened this issue Jul 21, 2022 · 12 comments · Fixed by #15173

Comments

@mrpavlitzky
Copy link

Issue

Checking Manage own media permission for a user implicitly grants that user to Manage media which results in his ability to delete other user's media which is unwanted

Expected behaviour

Isolate the own media management permission from the management of all media.

@hishamco
Copy link
Member

Does we implemented manage own and other media permissions completely?

/cc @matiasmolleja

@Skrypt
Copy link
Contributor

Skrypt commented Jul 26, 2022

Manage Own Media is what should work if not.

@sebastienros
Copy link
Member

Seems like ManageMedia is implied by ManageOwnMedia:
https://github.com/OrchardCMS/OrchardCore/blob/main/src/OrchardCore.Modules/OrchardCore.Media/Permissions.cs#L13

@sebastienros sebastienros added this to the 1.5 milestone Aug 11, 2022
@Skrypt
Copy link
Contributor

Skrypt commented Sep 26, 2022

Please, someone, fix this one for 1.5.

@hishamco
Copy link
Member

Manage own media should manage the owner media not others media, seems the issue he/she can manage others media, right?

@hishamco hishamco self-assigned this Sep 26, 2022
@hishamco
Copy link
Member

One more thing what are the main difference between ManageMedia and ManageMediaFolder? I didn't much difference

@Skrypt
Copy link
Contributor

Skrypt commented Oct 21, 2022

I suggest that we reassign this issue for 1.6 or that we close it if there will be no action taken.

@hishamco
Copy link
Member

Agree to move it into the next release

@sebastienros sebastienros modified the milestones: 1.5, 1.x Nov 3, 2022
@sebastienros sebastienros added the P1 label Nov 3, 2022
@ericrrichards
Copy link

Not to make this more complicated, but there's a wrinkle that I'm running into at the moment that feels related to this.

I have a content type with the CustomUserSettings stereotype.
This content type has an attached MediaField - editor = "attached"
For a user to edit their own settings and upload a picture to that MediaField, I have to give the role both the "Manage Own Custom User Settings - {content_type}" permission, and the "Manage Own Media" permission.

If I do this, the user can upload an image, but they can also get into other top-level Media Library files because of the implied Manage Media permission, which is not ideal.

But they can't get to the file that they uploaded to the MediaField, because that is underneath the /mediafields folder in the Media Library, and they don't have the "Manage Attached Media Fields Folder" permission.

So the CustomUserSettings MediaField image ends up living in /mediafields/{CustomUserSettings-ContentType}/{CustomUserSettings-ContentItemId}/

Would it be possible to change this for MediaFields on CustomUserSettings objects so that the file ended up under /_Users/{userid}/mediafields/{CustomUserSettings-ContentType}/{CustomUserSettings-ContentItemId}/?

@Piedone
Copy link
Member

Piedone commented Mar 22, 2024

Please check out this PR for an intended Secure Media feature that fixes this too: #15173.

@hishamco hishamco removed their assignment Mar 23, 2024
@Piedone
Copy link
Member

Piedone commented Apr 5, 2024

Please give your feedback about subfolder permissions here: #9369 (comment).

@Piedone
Copy link
Member

Piedone commented Apr 17, 2024

If anybody has any feedback on the #15173 PR, please let us know under it. Otherwise, I'll merge it in a week.

@MikeAlhayek MikeAlhayek modified the milestones: 2.x, 2.0 Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants