-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Media: Add protection to restrict access to media in recycle bin (closes #2931) #20378
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
base: main
Are you sure you want to change the base?
Conversation
Protected requests for media in recycle bin.
…media recycle bin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements media recycle bin protection to restrict access to media files when they are moved to the recycle bin. The feature adds security by renaming trashed media files with a .deleted suffix and provides middleware to control access to these protected files.
- Adds new
EnableMediaRecycleBinProtectionconfiguration setting inImagingSettings - Implements file renaming functionality when media is moved to/from recycle bin
- Introduces middleware protection requiring authentication for trashed media access
Reviewed Changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/Umbraco.Core/Configuration/Models/ImagingSettings.cs |
Adds EnableMediaRecycleBinProtection configuration property |
src/Umbraco.Core/Constants-Conventions.cs |
Defines .deleted suffix constant for trashed media files |
src/Umbraco.Core/IO/MediaFileManager.cs |
Implements suffix/remove suffix operations for media files |
src/Umbraco.Core/IO/IFileSystem.cs |
Adds MoveFile method to interface with default implementation |
src/Umbraco.Web.Common/Middleware/ProtectRecycleBinMediaMiddleware.cs |
New middleware to authenticate access to protected media files |
src/Umbraco.Infrastructure/PropertyEditors/NotificationHandlers/FileUploadContentDeletedNotificationHandler.cs |
Handles file operations when media is moved to/from recycle bin |
src/Umbraco.Web.UI.Client/src/packages/media/media/property-editors/image-cropper/property-editor-ui-image-cropper.element.ts |
Updates frontend to handle protected media file paths |
| Multiple test files | Adds test coverage for new file system operations |
...tructure/PropertyEditors/NotificationHandlers/FileUploadContentDeletedNotificationHandler.cs
Outdated
Show resolved
Hide resolved
...kages/media/media/property-editors/image-cropper/property-editor-ui-image-cropper.element.ts
Outdated
Show resolved
Hide resolved
...kages/media/media/property-editors/image-cropper/property-editor-ui-image-cropper.element.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
… and file upload notification handlers.
…thub.com/umbraco/Umbraco-CMS into v16/feature/media-recycle-bin-protection
Prerequisites
Addresses #2931
Description
This PR attempts to address the long standing issue of media files that are associated with media items in the recycle bin being accessible.
There are a couple of prerequisites as part of the PR:
MoveFilemethod toIFileSystemand the various implementations, including a default for any third-party implementations that won't have yet implemented it.Then the implementation itself includes:
.deletedsuffix - somedia/xxx/test.pngbecomesmedia/xxx/test.deleted.png..deletedsuffix is removed.MoveFileimplementation onIFileSystem..deletedsuffix will be added in they are being viewed in the recycle bin..deletedsuffix and only allow access if there is a backoffice user logged in who has access to the "Media" section.IBackOfficeUserAccessoralways fails to find the current backoffice user in this context.Testing
With the code from this PR in place, verify that, with
Umbraco:Cms:Imaging:EnableMediaRecycleBinProtectionset totrue..deletedsuffix.To Do
IFileSystem.MoveFile.