Media: Add protection to restrict access to media in recycle bin (closes #2931) (#20378)

* Add MoveFile it IFileSystem and implement on file systems.

* Rename media file on move to recycle bin.

* Rename file on restore from recycle bin.

* Add configuration to enabled recycle bin media protection.

* Expose backoffice authentication as cookie for non-backoffice usage.
Protected requests for media in recycle bin.

* Display protected image when viewing image cropper in the backoffice media recycle bin.

* Code tidy and comments.

* Apply suggestions from code review

Co-authored-by: Copilot <[email protected]>

* Introduced helper class to DRY up repeated code between image cropper and file upload notification handlers.

* Reverted client-side and management API updates.

* Moved update of path to media file in recycle bin with deleted suffix to the server.

* Separate integration tests for add and remove.

* Use interpolated strings.

* Renamed variable.

* Move EnableMediaRecycleBinProtection to ContentSettings.

* Tidied up comments.

* Added TODO for 18.

---------

Co-authored-by: Copilot <[email protected]>

Author: AndyButland

src/Umbraco.Cms.Api.Common/Security/ExposeBackOfficeAuthenticationOpenIddictServerEventsHandler.cs

@@ -0,0 +1,84 @@
+using System.Security.Claims;
+using Microsoft.AspNetCore.Authentication;
+using Microsoft.AspNetCore.Http;
+using Microsoft.Extensions.Options;
+using OpenIddict.Server;
+using Umbraco.Cms.Core.Configuration.Models;
+using Umbraco.Cms.Core.Security;
+using Umbraco.Extensions;
+
+namespace Umbraco.Cms.Infrastructure.Security;
+
+/// <summary>
+/// Provides OpenIddict server event handlers to expose the backoffice authentication token via a custom authentication scheme.
+/// </summary>
+public class ExposeBackOfficeAuthenticationOpenIddictServerEventsHandler : IOpenIddictServerHandler<OpenIddictServerEvents.GenerateTokenContext>,
+    IOpenIddictServerHandler<OpenIddictServerEvents.ApplyRevocationResponseContext>
+{
+    private readonly IHttpContextAccessor _httpContextAccessor;
+    private readonly string[] _claimTypes;
+    private readonly TimeSpan _timeOut;
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="ExposeBackOfficeAuthenticationOpenIddictServerEventsHandler"/> class.
+    /// </summary>
+    public ExposeBackOfficeAuthenticationOpenIddictServerEventsHandler(
+        IHttpContextAccessor httpContextAccessor,
+        IOptions<GlobalSettings> globalSettings,
+        IOptions<BackOfficeIdentityOptions> backOfficeIdentityOptions)
+    {
+        _httpContextAccessor = httpContextAccessor;
+        _timeOut = globalSettings.Value.TimeOut;
+
+        // These are the type identifiers for the claims required by the principal
+        // for the custom authentication scheme.
+        // We make available the ID, user name and allowed applications (sections) claims.
+        _claimTypes =
+        [
+            backOfficeIdentityOptions.Value.ClaimsIdentity.UserIdClaimType,
+            backOfficeIdentityOptions.Value.ClaimsIdentity.UserNameClaimType,
+            Core.Constants.Security.AllowedApplicationsClaimType,
+        ];
+    }
+
+    /// <inheritdoc/>
+    /// <remarks>
+    /// Event handler for when access tokens are generated (created or refreshed).
+    /// </remarks>
+    public async ValueTask HandleAsync(OpenIddictServerEvents.GenerateTokenContext context)
+    {
+        // Only proceed if this is a back-office sign-in.
+        if (context.Principal.Identity?.AuthenticationType != Core.Constants.Security.BackOfficeAuthenticationType)
+        {
+            return;
+        }
+
+        // Create a new principal with the claims from the authenticated principal.
+        var principal = new ClaimsPrincipal(
+            new ClaimsIdentity(
+                context.Principal.Claims.Where(claim => _claimTypes.Contains(claim.Type)),
+                Core.Constants.Security.BackOfficeExposedAuthenticationType));
+
+        // Sign-in the new principal for the custom authentication scheme.
+        await _httpContextAccessor
+            .GetRequiredHttpContext()
+            .SignInAsync(Core.Constants.Security.BackOfficeExposedAuthenticationType, principal, GetAuthenticationProperties());
+    }
+
+    /// <inheritdoc/>
+    /// <remarks>
+    /// Event handler for when access tokens are revoked.
+    /// </remarks>
+    public async ValueTask HandleAsync(OpenIddictServerEvents.ApplyRevocationResponseContext context)
+        => await _httpContextAccessor
+            .GetRequiredHttpContext()
+            .SignOutAsync(Core.Constants.Security.BackOfficeExposedAuthenticationType, GetAuthenticationProperties());
+
+    private AuthenticationProperties GetAuthenticationProperties()
+        => new()
+        {
+            IsPersistent = true,
+            IssuedUtc = DateTimeOffset.UtcNow,
+            ExpiresUtc = DateTimeOffset.UtcNow.Add(_timeOut)
+        };
+}

src/Umbraco.Cms.Api.Management/DependencyInjection/BackOfficeAuthBuilderExtensions.cs

@@ -1,5 +1,7 @@
-using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.DependencyInjection;
+using OpenIddict.Server;
 using Umbraco.Cms.Api.Common.DependencyInjection;
 using Umbraco.Cms.Api.Management.Configuration;
 using Umbraco.Cms.Api.Management.Handlers;
@@ -50,6 +52,7 @@ private static IUmbracoBuilder AddBackOfficeLogin(this IUmbracoBuilder builder)
     {
         builder.Services
             .AddAuthentication()
+
             // Add our custom schemes which are cookie handlers
             .AddCookie(Constants.Security.BackOfficeAuthenticationType)
             .AddCookie(Constants.Security.BackOfficeExternalAuthenticationType, o =>
@@ -58,6 +61,15 @@ private static IUmbracoBuilder AddBackOfficeLogin(this IUmbracoBuilder builder)
                 o.ExpireTimeSpan = TimeSpan.FromMinutes(5);
             })
 
+            // Add a cookie scheme that can be used for authenticating backoffice users outside the scope of the backoffice.
+            .AddCookie(Constants.Security.BackOfficeExposedAuthenticationType, options =>
+            {
+                options.Cookie.Name = Constants.Security.BackOfficeExposedCookieName;
+                options.Cookie.HttpOnly = true;
+                options.Cookie.SecurePolicy = CookieSecurePolicy.Always;
+                options.SlidingExpiration = true;
+            })
+
             // Although we don't natively support this, we add it anyways so that if end-users implement the required logic
             // they don't have to worry about manually adding this scheme or modifying the sign in manager
             .AddCookie(Constants.Security.BackOfficeTwoFactorAuthenticationType, options =>
@@ -71,6 +83,22 @@ private static IUmbracoBuilder AddBackOfficeLogin(this IUmbracoBuilder builder)
                 o.ExpireTimeSpan = TimeSpan.FromMinutes(5);
             });
 
+        // Add OpnIddict server event handler to refresh the cookie that exposes the backoffice authentication outside the scope of the backoffice.
+        builder.Services.AddSingleton<ExposeBackOfficeAuthenticationOpenIddictServerEventsHandler>();
+        builder.Services.Configure<OpenIddictServerOptions>(options =>
+        {
+            options.Handlers.Add(
+                OpenIddictServerHandlerDescriptor
+                    .CreateBuilder<OpenIddictServerEvents.GenerateTokenContext>()
+                    .UseSingletonHandler<ExposeBackOfficeAuthenticationOpenIddictServerEventsHandler>()
+                    .Build());
+            options.Handlers.Add(
+                OpenIddictServerHandlerDescriptor
+                    .CreateBuilder<OpenIddictServerEvents.ApplyRevocationResponseContext>()
+                    .UseSingletonHandler<ExposeBackOfficeAuthenticationOpenIddictServerEventsHandler>()
+                    .Build());
+        });
+
         builder.Services.AddScoped<BackOfficeSecurityStampValidator>();
         builder.Services.ConfigureOptions<ConfigureBackOfficeCookieOptions>();
         builder.Services.ConfigureOptions<ConfigureBackOfficeSecurityStampValidatorOptions>();

src/Umbraco.Cms.Api.Management/Mapping/Media/MediaMapDefinition.cs

@@ -1,27 +1,49 @@
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Options;
 using Umbraco.Cms.Api.Management.Mapping.Content;
 using Umbraco.Cms.Api.Management.ViewModels.Media;
 using Umbraco.Cms.Api.Management.ViewModels.Media.Collection;
 using Umbraco.Cms.Api.Management.ViewModels.MediaType;
+using Umbraco.Cms.Core.Configuration.Models;
 using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Mapping;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.Mapping;
 using Umbraco.Cms.Core.PropertyEditors;
+using Umbraco.Cms.Core.PropertyEditors.ValueConverters;
 using Umbraco.Extensions;
 
 namespace Umbraco.Cms.Api.Management.Mapping.Media;
 
 public class MediaMapDefinition : ContentMapDefinition<IMedia, MediaValueResponseModel, MediaVariantResponseModel>, IMapDefinition
 {
     private readonly CommonMapper _commonMapper;
+    private ContentSettings _contentSettings;
 
     public MediaMapDefinition(
         PropertyEditorCollection propertyEditorCollection,
         CommonMapper commonMapper,
-        IDataValueEditorFactory dataValueEditorFactory)
+        IDataValueEditorFactory dataValueEditorFactory,
+        IOptionsMonitor<ContentSettings> contentSettings)
         : base(propertyEditorCollection, dataValueEditorFactory)
-        => _commonMapper = commonMapper;
+    {
+        _commonMapper = commonMapper;
+        _contentSettings = contentSettings.CurrentValue;
+        contentSettings.OnChange(x => _contentSettings = x);
+    }
+
+    [Obsolete("Please use the non-obsolete constructor. Scheduled for removal in Umbraco 18.")]
+    public MediaMapDefinition(
+        PropertyEditorCollection propertyEditorCollection,
+        CommonMapper commonMapper,
+        IDataValueEditorFactory dataValueEditorFactory)
+        : this(
+              propertyEditorCollection,
+              commonMapper,
+              dataValueEditorFactory,
+              StaticServiceProvider.Instance.GetRequiredService<IOptionsMonitor<ContentSettings>>())
+    {
+    }
 
     [Obsolete("Please use the non-obsolete constructor. Scheduled for removal in Umbraco 18.")]
     public MediaMapDefinition(
@@ -48,6 +70,39 @@ private void Map(IMedia source, MediaResponseModel target, MapperContext context
         target.Values = MapValueViewModels(source.Properties);
         target.Variants = MapVariantViewModels(source);
         target.IsTrashed = source.Trashed;
+
+        // If protection for media files in the recycle bin is enabled, and the media item is trashed, amend the value of the file path
+        // to have the `.deleted` suffix that will have been added to the persisted file.
+        if (target.IsTrashed && _contentSettings.EnableMediaRecycleBinProtection)
+        {
+            foreach (MediaValueResponseModel valueModel in target.Values
+                .Where(x => x.EditorAlias.Equals(Core.Constants.PropertyEditors.Aliases.ImageCropper)))
+            {
+                if (valueModel.Value is not null &&
+                    valueModel.Value is ImageCropperValue imageCropperValue &&
+                    string.IsNullOrWhiteSpace(imageCropperValue.Src) is false)
+                {
+                    valueModel.Value = new ImageCropperValue
+                    {
+                        Crops = imageCropperValue.Crops,
+                        FocalPoint = imageCropperValue.FocalPoint,
+                        TemporaryFileId = imageCropperValue.TemporaryFileId,
+                        Src = SuffixMediaPath(imageCropperValue.Src, Core.Constants.Conventions.Media.TrashedMediaSuffix),
+                    };
+                }
+            }
+        }
+    }
+
+    private static string SuffixMediaPath(string filePath, string suffix)
+    {
+        int lastDotIndex = filePath.LastIndexOf('.');
+        if (lastDotIndex == -1)
+        {
+            return filePath + suffix;
+        }
+
+        return filePath[..lastDotIndex] + suffix + filePath[lastDotIndex..];
     }
 
     // Umbraco.Code.MapAll -Flags

src/Umbraco.Core/Configuration/Models/ContentSettings.cs

@@ -31,6 +31,9 @@ public class ContentSettings
     internal const bool StaticShowDomainWarnings = true;
     internal const bool StaticShowUnroutableContentWarnings = true;
 
+    // TODO (V18): Consider enabling this by default and documenting as a behavioural breaking change.
+    private const bool StaticEnableMediaRecycleBinProtection = false;
+
     /// <summary>
     ///     Gets or sets a value for the content notification settings.
     /// </summary>
@@ -158,4 +161,16 @@ public class ContentSettings
     /// </summary>
     [DefaultValue(StaticShowUnroutableContentWarnings)]
     public bool ShowUnroutableContentWarnings { get; set; } = StaticShowUnroutableContentWarnings;
+
+    /// <summary>
+    /// Gets or sets a value indicating whether to enable or disable the recycle bin protection for media.
+    /// </summary>
+    /// <remarks>
+    /// When set to true, this will:
+    ///  - Rename media moved to the recycle bin to have a .deleted suffice (e.g. image.jpg will be renamed to image.deleted.jpg).
+    ///  - On restore, the media file will be renamed back to its original name.
+    ///  - A middleware component will be enabled to prevent access to media files in the recycle bin unless the user is authenticated with access to the media section.
+    /// </remarks>
+    [DefaultValue(StaticEnableMediaRecycleBinProtection)]
+    public bool EnableMediaRecycleBinProtection { get; set; } = StaticEnableMediaRecycleBinProtection;
 }

src/Umbraco.Core/Configuration/Models/ImagingSettings.cs

@@ -1,6 +1,8 @@
 // Copyright (c) Umbraco.
 // See LICENSE for more details.
 
+using System.ComponentModel;
+
 namespace Umbraco.Cms.Core.Configuration.Models;
 
 /// <summary>

src/Umbraco.Core/Constants-Conventions.cs

@@ -100,6 +100,11 @@ public static class Media
             ///     The default height/width of an image file if the size can't be determined from the metadata
             /// </summary>
             public const int DefaultSize = 200;
+
+            /// <summary>
+            ///     Suffix added to media files when moved to the recycle bin when recycle bin media protection is enabled.
+            /// </summary>
+            public const string TrashedMediaSuffix = ".deleted";
         }
 
         /// <summary>

src/Umbraco.Core/Constants-Security.cs

@@ -73,6 +73,17 @@ public static class Security
         public const string BackOfficeTokenAuthenticationType = "UmbracoBackOfficeToken";
         public const string BackOfficeTwoFactorAuthenticationType = "UmbracoTwoFactorCookie";
         public const string BackOfficeTwoFactorRememberMeAuthenticationType = "UmbracoTwoFactorRememberMeCookie";
+
+        /// <summary>
+        /// Authentication type and scheme used for backoffice users when it is exposed out of the backoffice context via a cookie.
+        /// </summary>
+        public const string BackOfficeExposedAuthenticationType = "UmbracoBackOfficeExposed";
+
+        /// <summary>
+        /// Represents the name of the authentication cookie used to expose the backoffice authentication token outside of the backoffice context.
+        /// </summary>
+        public const string BackOfficeExposedCookieName = "UMB_UCONTEXT_EXPOSED";
+
         public const string EmptyPasswordPrefix = "___UIDEMPTYPWORD__";
 
         public const string DefaultMemberTypeAlias = "Member";

src/Umbraco.Core/IO/IFileSystem.cs

@@ -168,10 +168,42 @@ public interface IFileSystem
     /// <param name="copy">A value indicating whether to move (default) or copy.</param>
     void AddFile(string path, string physicalPath, bool overrideIfExists = true, bool copy = false);
 
+    /// <summary>
+    /// Moves a file from the specified source path to the specified target path.
+    /// </summary>
+    /// <param name="source">The path of the file or directory to move.</param>
+    /// <param name="target">The destination path where the file or directory will be moved.</param>
+    /// <param name="overrideIfExists">A value indicating what to do if the file already exists.</param>
+    void MoveFile(string source, string target, bool overrideIfExists = true)
+    {
+        // Provide a default implementation for implementations of IFileSystem that do not implement this method.
+        if (FileExists(source) is false)
+        {
+            throw new FileNotFoundException($"File at path '{source}' could not be found.");
+        }
+
+        if (FileExists(target))
+        {
+            if (overrideIfExists)
+            {
+                DeleteFile(target);
+            }
+            else
+            {
+                throw new IOException($"A file at path '{target}' already exists.");
+            }
+        }
+
+        using (Stream sourceStream = OpenFile(source))
+        {
+            AddFile(target, sourceStream);
+        }
+
+        DeleteFile(source);
+    }
+
     // TODO: implement these
     //
     // void CreateDirectory(string path);
     //
-    //// move or rename, directory or file
-    // void Move(string source, string target);
 }