Split out HMAC helper, remove HMAC from request with no commands

JimBobSquarePants · JimBobSquarePants · commit 884f76e0b069 · 2023-03-06T22:25:14.000+10:00
diff --git a/samples/ImageSharp.Web.Sample/Pages/Index.cshtml b/samples/ImageSharp.Web.Sample/Pages/Index.cshtml
@@ -60,13 +60,13 @@
         </div>
     </section>
         <section>
-        <h2>HMAC - No Commands</h2>
+        <h2>No Commands</h2>
         <div>
             <p>
-                <code>sixlabors.imagesharp.web.png?hmac=...</code>
+                <code>sixlabors.imagesharp.web.png</code>
             </p>
             <p>
-                <img src="sixlabors.imagesharp.web.png" width="300" imagesharp-hmac />
+                <img src="sixlabors.imagesharp.web.png" width="300" />
             </p>
         </div>
     </section>
diff --git a/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs b/src/ImageSharp.Web/Middleware/ImageSharpMiddleware.cs
@@ -244,13 +244,10 @@ private async Task Invoke(HttpContext httpContext, bool retry)
 
         // At this point we know that this is an image request designed for processing via this middleware.
         // Check for a token if required and reject if invalid.
-        if (checkHMAC)
+        if (checkHMAC && hmac != token)
         {
-            if (token == null || hmac != token)
-            {
-                SetBadRequest(httpContext);
-                return;
-            }
+            SetBadRequest(httpContext);
+            return;
         }
 
         IImageResolver? sourceImageResolver = await provider.GetAsync(httpContext);
diff --git a/src/ImageSharp.Web/RequestAuthorizationUtilities.cs b/src/ImageSharp.Web/RequestAuthorizationUtilities.cs
@@ -216,22 +216,13 @@ public void StripUnknownCommands(CommandCollection commands)
             this.StripUnknownCommands(commands);
         }
 
-        ImageCommandContext imageCommandContext = new(context, commands, this.commandParser, this.parserCulture);
-        return await this.options.OnComputeHMACAsync(imageCommandContext, secret);
-    }
-
-    internal string ComputeHMAC(string uri, CommandCollection commands, byte[] secret)
-    {
-        ToComponents(
-            new Uri(uri, UriKind.RelativeOrAbsolute),
-            out HostString host,
-            out PathString path,
-            out QueryString queryString);
+        if (commands.Count == 0)
+        {
+            return null;
+        }
 
-        HttpContext context = this.ToHttpContext(host, path, queryString, new(QueryHelpers.ParseQuery(queryString.Value)));
         ImageCommandContext imageCommandContext = new(context, commands, this.commandParser, this.parserCulture);
-
-        return AsyncHelper.RunSync(() => this.options.OnComputeHMACAsync(imageCommandContext, secret));
+        return await this.options.OnComputeHMACAsync(imageCommandContext, secret);
     }
 
     /// <summary>
@@ -243,8 +234,15 @@ internal string ComputeHMAC(string uri, CommandCollection commands, byte[] secre
     /// </remarks>
     /// <param name="context">Contains information about the current image request and parsed commands.</param>
     /// <returns>The computed HMAC.</returns>
-    internal Task<string> ComputeHMACAsync(ImageCommandContext context)
-        => this.options.OnComputeHMACAsync(context, this.options.HMACSecretKey);
+    internal async Task<string?> ComputeHMACAsync(ImageCommandContext context)
+    {
+        if (context.Commands.Count == 0)
+        {
+            return null;
+        }
+
+        return await this.options.OnComputeHMACAsync(context, this.options.HMACSecretKey);
+    }
 
     private static void ToComponents(
         Uri uri,
diff --git a/src/ImageSharp.Web/TagHelpers/HmacTokenTagHelper.cs b/src/ImageSharp.Web/TagHelpers/HmacTokenTagHelper.cs
@@ -0,0 +1,120 @@
+// Copyright (c) Six Labors.
+// Licensed under the Six Labors Split License.
+
+using System.Text;
+using System.Text.Encodings.Web;
+using Microsoft.AspNetCore.Mvc.Razor.TagHelpers;
+using Microsoft.AspNetCore.Mvc.Routing;
+using Microsoft.AspNetCore.Mvc.TagHelpers;
+using Microsoft.AspNetCore.Razor.TagHelpers;
+using Microsoft.Extensions.Options;
+using SixLabors.ImageSharp.Web.Middleware;
+
+namespace SixLabors.ImageSharp.Web.TagHelpers;
+
+/// <summary>
+/// A <see cref="TagHelper"/> implementation targeting &lt;img&gt; element that allows the automatic generation of HMAC image processing protection tokens.
+/// </summary>
+[HtmlTargetElement("img", Attributes = SrcAttributeName, TagStructure = TagStructure.WithoutEndTag)]
+public class HmacTokenTagHelper : UrlResolutionTagHelper
+{
+    private const string SrcAttributeName = "src";
+
+    private readonly ImageSharpMiddlewareOptions options;
+    private readonly RequestAuthorizationUtilities authorizationUtilities;
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="HmacTokenTagHelper" /> class.
+    /// </summary>
+    /// <param name="options">The middleware configuration options.</param>
+    /// <param name="authorizationUtilities">Contains helpers that allow authorization of image requests.</param>
+    /// <param name="urlHelperFactory">The URL helper factory.</param>
+    /// <param name="htmlEncoder">The HTML encorder.</param>
+    public HmacTokenTagHelper(
+        IOptions<ImageSharpMiddlewareOptions> options,
+        RequestAuthorizationUtilities authorizationUtilities,
+        IUrlHelperFactory urlHelperFactory,
+        HtmlEncoder htmlEncoder)
+        : base(urlHelperFactory, htmlEncoder)
+    {
+        Guard.NotNull(options, nameof(options));
+        Guard.NotNull(authorizationUtilities, nameof(authorizationUtilities));
+
+        this.options = options.Value;
+        this.authorizationUtilities = authorizationUtilities;
+    }
+
+    /// <inheritdoc/>
+    public override int Order => 2;
+
+    /// <summary>
+    /// Gets or sets the source of the image.
+    /// </summary>
+    /// <remarks>
+    /// Passed through to the generated HTML in all cases.
+    /// </remarks>
+    [HtmlAttributeName(SrcAttributeName)]
+    public string? Src { get; set; }
+
+    /// <inheritdoc />
+    public override void Process(TagHelperContext context, TagHelperOutput output)
+    {
+        Guard.NotNull(context, nameof(context));
+        Guard.NotNull(output, nameof(output));
+
+        output.CopyHtmlAttribute(SrcAttributeName, context);
+        this.ProcessUrlAttribute(SrcAttributeName, output);
+
+        byte[] secret = this.options.HMACSecretKey;
+        if (secret is null || secret.Length == 0)
+        {
+            return;
+        }
+
+        // Retrieve the TagHelperOutput variation of the "src" attribute in case other TagHelpers in the
+        // pipeline have touched the value. If the value is already encoded this ImageTagHelper may
+        // not function properly.
+        string? src = output.Attributes[SrcAttributeName]?.Value as string;
+        if (string.IsNullOrWhiteSpace(src))
+        {
+            return;
+        }
+
+        string? hmac = this.authorizationUtilities.ComputeHMAC(src, CommandHandling.Sanitize);
+        if (hmac is not null)
+        {
+            this.Src = AddQueryString(src, hmac);
+            output.Attributes.SetAttribute(SrcAttributeName, this.Src);
+        }
+    }
+
+    private static string AddQueryString(
+        ReadOnlySpan<char> uri,
+        string hmac)
+    {
+        ReadOnlySpan<char> uriToBeAppended = uri;
+        ReadOnlySpan<char> anchorText = default;
+
+        // If there is an anchor, then the query string must be inserted before its first occurrence.
+        int anchorIndex = uri.IndexOf('#');
+        if (anchorIndex != -1)
+        {
+            anchorText = uri[anchorIndex..];
+            uriToBeAppended = uri[..anchorIndex];
+        }
+
+        int queryIndex = uriToBeAppended.IndexOf('?');
+        bool hasQuery = queryIndex != -1;
+
+        StringBuilder sb = new();
+
+        sb.Append(uriToBeAppended)
+          .Append(hasQuery ? '&' : '?')
+          .Append(UrlEncoder.Default.Encode(RequestAuthorizationUtilities.TokenCommand))
+          .Append('=')
+          .Append(UrlEncoder.Default.Encode(hmac))
+          .Append(anchorText);
+
+        return sb.ToString();
+    }
+}
diff --git a/src/ImageSharp.Web/TagHelpers/ImageTagHelper.cs b/src/ImageSharp.Web/TagHelpers/ImageTagHelper.cs
@@ -16,44 +16,41 @@
 namespace SixLabors.ImageSharp.Web.TagHelpers;
 
 /// <summary>
-/// A TagHelper implementation targeting &lt;img&gt; element that allows the automatic generation of HMAC protected processing commands.
+/// A <see cref="TagHelper"/> implementation targeting &lt;img&gt; element that allows the automatic generation image processing commands.
 /// </summary>
 [HtmlTargetElement("img", Attributes = SrcAttributeName + "," + WidthAttributeName, TagStructure = TagStructure.WithoutEndTag)]
 [HtmlTargetElement("img", Attributes = SrcAttributeName + "," + HeightAttributeName, TagStructure = TagStructure.WithoutEndTag)]
 [HtmlTargetElement("img", Attributes = SrcAttributeName + "," + AnchorAttributeName, TagStructure = TagStructure.WithoutEndTag)]
-[HtmlTargetElement("img", Attributes = SrcAttributeName + "," + RModeAttributeName, TagStructure = TagStructure.WithoutEndTag)]
+[HtmlTargetElement("img", Attributes = SrcAttributeName + "," + ModeAttributeName, TagStructure = TagStructure.WithoutEndTag)]
 [HtmlTargetElement("img", Attributes = SrcAttributeName + "," + XyAttributeName, TagStructure = TagStructure.WithoutEndTag)]
-[HtmlTargetElement("img", Attributes = SrcAttributeName + "," + RColorAttributeName, TagStructure = TagStructure.WithoutEndTag)]
+[HtmlTargetElement("img", Attributes = SrcAttributeName + "," + ColorAttributeName, TagStructure = TagStructure.WithoutEndTag)]
 [HtmlTargetElement("img", Attributes = SrcAttributeName + "," + CompandAttributeName, TagStructure = TagStructure.WithoutEndTag)]
 [HtmlTargetElement("img", Attributes = SrcAttributeName + "," + OrientAttributeName, TagStructure = TagStructure.WithoutEndTag)]
 [HtmlTargetElement("img", Attributes = SrcAttributeName + "," + AutoOrientAttributeName, TagStructure = TagStructure.WithoutEndTag)]
 [HtmlTargetElement("img", Attributes = SrcAttributeName + "," + FormatAttributeName, TagStructure = TagStructure.WithoutEndTag)]
 [HtmlTargetElement("img", Attributes = SrcAttributeName + "," + BgColorAttributeName, TagStructure = TagStructure.WithoutEndTag)]
 [HtmlTargetElement("img", Attributes = SrcAttributeName + "," + QualityAttributeName, TagStructure = TagStructure.WithoutEndTag)]
-[HtmlTargetElement("img", Attributes = SrcAttributeName + "," + HMACAttributeName, TagStructure = TagStructure.WithoutEndTag)]
 public class ImageTagHelper : UrlResolutionTagHelper
 {
     private const string SrcAttributeName = "src";
     private const string AttributePrefix = "imagesharp-";
     private const string WidthAttributeName = AttributePrefix + ResizeWebProcessor.Width;
     private const string HeightAttributeName = AttributePrefix + ResizeWebProcessor.Height;
     private const string AnchorAttributeName = AttributePrefix + ResizeWebProcessor.Anchor;
-    private const string RModeAttributeName = AttributePrefix + ResizeWebProcessor.Mode;
+    private const string ModeAttributeName = AttributePrefix + ResizeWebProcessor.Mode;
     private const string XyAttributeName = AttributePrefix + ResizeWebProcessor.Xy;
-    private const string RColorAttributeName = AttributePrefix + ResizeWebProcessor.Color;
+    private const string ColorAttributeName = AttributePrefix + ResizeWebProcessor.Color;
     private const string CompandAttributeName = AttributePrefix + ResizeWebProcessor.Compand;
     private const string OrientAttributeName = AttributePrefix + ResizeWebProcessor.Orient;
     private const string SamplerAttributeName = AttributePrefix + ResizeWebProcessor.Sampler;
     private const string AutoOrientAttributeName = AttributePrefix + AutoOrientWebProcessor.AutoOrient;
     private const string FormatAttributeName = AttributePrefix + FormatWebProcessor.Format;
     private const string BgColorAttributeName = AttributePrefix + BackgroundColorWebProcessor.Color;
     private const string QualityAttributeName = AttributePrefix + QualityWebProcessor.Quality;
-    private const string HMACAttributeName = AttributePrefix + RequestAuthorizationUtilities.TokenCommand;
 
     private readonly ImageSharpMiddlewareOptions options;
     private readonly CultureInfo parserCulture;
     private readonly char separator;
-    private readonly RequestAuthorizationUtilities authorizationUtilities;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="ImageTagHelper"/> class.
@@ -77,10 +74,11 @@ public ImageTagHelper(
             ? CultureInfo.InvariantCulture
             : CultureInfo.CurrentCulture;
         this.separator = this.parserCulture.TextInfo.ListSeparator[0];
-
-        this.authorizationUtilities = authorizationUtilities;
     }
 
+    /// <inheritdoc/>
+    public override int Order => 1;
+
     /// <summary>
     /// Gets or sets the src.
     /// </summary>
@@ -111,7 +109,7 @@ public ImageTagHelper(
     /// <summary>
     /// Gets or sets the resize mode.
     /// </summary>
-    [HtmlAttributeName(RModeAttributeName)]
+    [HtmlAttributeName(ModeAttributeName)]
     public ResizeMode? ResizeMode { get; set; }
 
     /// <summary>
@@ -129,7 +127,7 @@ public ImageTagHelper(
     /// <summary>
     /// Gets or sets the color to use as a background when padding an image.
     /// </summary>
-    [HtmlAttributeName(RColorAttributeName)]
+    [HtmlAttributeName(ColorAttributeName)]
     public Color? PadColor { get; set; }
 
     /// <summary>
@@ -180,24 +178,17 @@ public ImageTagHelper(
     [HtmlAttributeName(QualityAttributeName)]
     public int? Quality { get; set; }
 
-    /// <summary>
-    /// Gets or sets a value indicating whether to append a HMAC token to the request.
-    /// This value is always <see langword="true"/>. HMAC token usage is controlled by populating the
-    /// <see cref="ImageSharpMiddlewareOptions.HMACSecretKey"/> property.
-    /// </summary>
-    [HtmlAttributeName(HMACAttributeName)]
-#pragma warning disable CA1822 // Mark members as static
-    public bool AppendHMAC { get => true; set => _ = true; }
-#pragma warning restore CA1822 // Mark members as static
-
     /// <inheritdoc />
     public override void Process(TagHelperContext context, TagHelperOutput output)
     {
         Guard.NotNull(context, nameof(context));
         Guard.NotNull(output, nameof(output));
 
         string? src = output.Attributes[SrcAttributeName]?.Value as string ?? this.Src;
-        if (string.IsNullOrWhiteSpace(src) || src.StartsWith("data", StringComparison.OrdinalIgnoreCase))
+        if (string.IsNullOrWhiteSpace(src)
+            || src.StartsWith("http", StringComparison.OrdinalIgnoreCase)
+            || src.StartsWith("ftp", StringComparison.OrdinalIgnoreCase)
+            || src.StartsWith("data", StringComparison.OrdinalIgnoreCase))
         {
             base.Process(context, output);
             return;
@@ -209,24 +200,16 @@ public override void Process(TagHelperContext context, TagHelperOutput output)
         CommandCollection commands = new();
         this.AddProcessingCommands(context, output, commands, this.parserCulture);
 
-        byte[] secret = this.options.HMACSecretKey;
-        if (commands.Count > 0 || secret?.Length > 0)
+        if (commands.Count > 0)
         {
             // Retrieve the TagHelperOutput variation of the "src" attribute in case other TagHelpers in the
             // pipeline have touched the value. If the value is already encoded this helper may
             // not function properly.
-            src = output.Attributes[SrcAttributeName].Value as string;
-            if (secret?.Length > 0)
-            {
-                string hash = this.authorizationUtilities.ComputeHMAC(src!, commands, secret);
-                commands.Add(RequestAuthorizationUtilities.TokenCommand, hash);
-            }
-
+            src = output.Attributes[SrcAttributeName]?.Value as string;
             src = AddQueryString(src, commands);
+            output.Attributes.SetAttribute(SrcAttributeName, src);
+            this.Src = src;
         }
-
-        this.Src = src;
-        output.Attributes.SetAttribute(SrcAttributeName, src);
     }
 
     /// <summary>
diff --git a/tests/ImageSharp.Web.Tests/TagHelpers/HmacTokenTagHelperTests.cs b/tests/ImageSharp.Web.Tests/TagHelpers/HmacTokenTagHelperTests.cs
diff --git a/tests/ImageSharp.Web.Tests/TagHelpers/ImageTagHelperTests.cs b/tests/ImageSharp.Web.Tests/TagHelpers/ImageTagHelperTests.cs
