PR feedback

docs/docfx/articles/session-affinity.md

@@ -94,7 +94,7 @@ There are three built-in affinity polices that format and store the key differen
 
 ### Key Protection
 
-The HashCookie policy uses a SHA-256 hash to produce a standard, obscured output format for the cookie value. This is not a strong privacy protection and sensitive data should not be included in destination ids.
+The `HashCookie` policy uses a SHA-256 hash to produce a standard, obscured output format for the cookie value. This is not a strong privacy protection and sensitive data should not be included in destination ids. The `HashCookie` policy does not conceal the total number of unique destinations behind the proxy and should not be used if that's a concern.
 
 The Cookie and CustomHeader policies encrypt the key using Data Protection. This provides strong privacy protections for the key, but requires [additional configuration](https://learn.microsoft.com/aspnet/core/security/data-protection/configuration/overview) when more than once proxy instance is in use.
 

src/ReverseProxy/SessionAffinity/AffinityHelpers.cs

@@ -0,0 +1,27 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System;
+using Microsoft.AspNetCore.Http;
+using Yarp.ReverseProxy.Configuration;
+using Yarp.ReverseProxy.Utilities;
+
+namespace Yarp.ReverseProxy.SessionAffinity;
+
+internal static class AffinityHelpers
+{
+    internal static CookieOptions CreateCookieOptions(SessionAffinityCookieConfig? config, bool isHttps, IClock clock)
+    {
+        return new CookieOptions
+        {
+            Path = config?.Path ?? "/",
+            SameSite = config?.SameSite ?? SameSiteMode.Unspecified,
+            HttpOnly = config?.HttpOnly ?? true,
+            MaxAge = config?.MaxAge,
+            Domain = config?.Domain,
+            IsEssential = config?.IsEssential ?? false,
+            Secure = config?.SecurePolicy == CookieSecurePolicy.Always || (config?.SecurePolicy == CookieSecurePolicy.SameAsRequest && isHttps),
+            Expires = config?.Expiration is not null ? clock.GetUtcNow().Add(config.Expiration.Value) : default(DateTimeOffset?),
+        };
+    }
+}

src/ReverseProxy/SessionAffinity/BaseSessionAffinityPolicy.cs

@@ -145,37 +145,4 @@ private static string Pad(string text)
         }
         return text + new string('=', padding);
     }
-
-    private static class Log
-    {
-        private static readonly Action<ILogger, string, Exception?> _affinityCannotBeEstablishedBecauseNoDestinationsFound = LoggerMessage.Define<string>(
-            LogLevel.Warning,
-            EventIds.AffinityCannotBeEstablishedBecauseNoDestinationsFoundOnCluster,
-            "The request affinity cannot be established because no destinations are found on cluster `{clusterId}`.");
-
-        private static readonly Action<ILogger, Exception?> _requestAffinityKeyDecryptionFailed = LoggerMessage.Define(
-            LogLevel.Error,
-            EventIds.RequestAffinityKeyDecryptionFailed,
-            "The request affinity key decryption failed.");
-
-        private static readonly Action<ILogger, string, Exception?> _destinationMatchingToAffinityKeyNotFound = LoggerMessage.Define<string>(
-            LogLevel.Warning,
-            EventIds.DestinationMatchingToAffinityKeyNotFound,
-            "Destination matching to the request affinity key is not found on cluster `{backnedId}`. Configured failure policy will be applied.");
-
-        public static void AffinityCannotBeEstablishedBecauseNoDestinationsFound(ILogger logger, string clusterId)
-        {
-            _affinityCannotBeEstablishedBecauseNoDestinationsFound(logger, clusterId, null);
-        }
-
-        public static void RequestAffinityKeyDecryptionFailed(ILogger logger, Exception? ex)
-        {
-            _requestAffinityKeyDecryptionFailed(logger, ex);
-        }
-
-        public static void DestinationMatchingToAffinityKeyNotFound(ILogger logger, string clusterId)
-        {
-            _destinationMatchingToAffinityKeyNotFound(logger, clusterId, null);
-        }
-    }
 }

src/ReverseProxy/SessionAffinity/CookieSessionAffinityPolicy.cs

@@ -39,17 +39,7 @@ protected override (string? Key, bool ExtractedSuccessfully) GetRequestAffinityK
 
     protected override void SetAffinityKey(HttpContext context, ClusterState cluster, SessionAffinityConfig config, string unencryptedKey)
     {
-        var affinityCookieOptions = new CookieOptions
-        {
-            Path = config.Cookie?.Path ?? "/",
-            SameSite = config.Cookie?.SameSite ?? SameSiteMode.Unspecified,
-            HttpOnly = config.Cookie?.HttpOnly ?? true,
-            MaxAge = config.Cookie?.MaxAge,
-            Domain = config.Cookie?.Domain,
-            IsEssential = config.Cookie?.IsEssential ?? false,
-            Secure = config.Cookie?.SecurePolicy == CookieSecurePolicy.Always || (config.Cookie?.SecurePolicy == CookieSecurePolicy.SameAsRequest && context.Request.IsHttps),
-            Expires = config.Cookie?.Expiration is not null ? _clock.GetUtcNow().Add(config.Cookie.Expiration.Value) : default(DateTimeOffset?),
-        };
+        var affinityCookieOptions = AffinityHelpers.CreateCookieOptions(config.Cookie, context.Request.IsHttps, _clock);
         context.Response.Cookies.Append(config.AffinityKeyName, Protect(unencryptedKey), affinityCookieOptions);
     }
 }

src/ReverseProxy/SessionAffinity/HashCookieSessionAffinityPolicy.cs

@@ -14,7 +14,7 @@
 
 namespace Yarp.ReverseProxy.SessionAffinity;
 
-internal sealed class HashCookieSessionAffinityPolicy : ISessionAffinityPolicy
+internal sealed partial class HashCookieSessionAffinityPolicy : ISessionAffinityPolicy
 {
     private static readonly object AffinityKeyId = new();
     private readonly ConditionalWeakTable<DestinationState, string> _hashes = new();
@@ -42,20 +42,7 @@ public void AffinitizeResponse(HttpContext context, ClusterState cluster, Sessio
         if (!context.Items.ContainsKey(AffinityKeyId))
         {
             var affinityKey = GetDestinationHash(destination);
-
-            // Nothing is written to the response
-            var affinityCookieOptions = new CookieOptions
-            {
-                Path = config.Cookie?.Path ?? "/",
-                SameSite = config.Cookie?.SameSite ?? SameSiteMode.Unspecified,
-                HttpOnly = config.Cookie?.HttpOnly ?? true,
-                MaxAge = config.Cookie?.MaxAge,
-                Domain = config.Cookie?.Domain,
-                IsEssential = config.Cookie?.IsEssential ?? false,
-                Secure = config.Cookie?.SecurePolicy == CookieSecurePolicy.Always || (config.Cookie?.SecurePolicy == CookieSecurePolicy.SameAsRequest && context.Request.IsHttps),
-                Expires = config.Cookie?.Expiration is not null ? _clock.GetUtcNow().Add(config.Cookie.Expiration.Value) : default(DateTimeOffset?),
-            };
-
+            var affinityCookieOptions = AffinityHelpers.CreateCookieOptions(config.Cookie, context.Request.IsHttps, _clock);
             context.Response.Cookies.Append(config.AffinityKeyName, affinityKey, affinityCookieOptions);
         }
     }
@@ -106,27 +93,4 @@ private string GetDestinationHash(DestinationState d)
             return Convert.ToHexString(hashBytes);
         });
     }
-
-    private static class Log
-    {
-        private static readonly Action<ILogger, string, Exception?> _affinityCannotBeEstablishedBecauseNoDestinationsFound = LoggerMessage.Define<string>(
-            LogLevel.Warning,
-            EventIds.AffinityCannotBeEstablishedBecauseNoDestinationsFoundOnCluster,
-            "The request affinity cannot be established because no destinations are found on cluster `{clusterId}`.");
-
-        private static readonly Action<ILogger, string, Exception?> _destinationMatchingToAffinityKeyNotFound = LoggerMessage.Define<string>(
-            LogLevel.Warning,
-            EventIds.DestinationMatchingToAffinityKeyNotFound,
-            "Destination matching to the request affinity key is not found on cluster `{backnedId}`. Configured failure policy will be applied.");
-
-        public static void AffinityCannotBeEstablishedBecauseNoDestinationsFound(ILogger logger, string clusterId)
-        {
-            _affinityCannotBeEstablishedBecauseNoDestinationsFound(logger, clusterId, null);
-        }
-
-        public static void DestinationMatchingToAffinityKeyNotFound(ILogger logger, string clusterId)
-        {
-            _destinationMatchingToAffinityKeyNotFound(logger, clusterId, null);
-        }
-    }
 }

src/ReverseProxy/SessionAffinity/Log.cs

@@ -0,0 +1,40 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System;
+using Microsoft.Extensions.Logging;
+
+namespace Yarp.ReverseProxy.SessionAffinity;
+
+internal static class Log
+{
+    private static readonly Action<ILogger, string, Exception?> _affinityCannotBeEstablishedBecauseNoDestinationsFound = LoggerMessage.Define<string>(
+        LogLevel.Warning,
+        EventIds.AffinityCannotBeEstablishedBecauseNoDestinationsFoundOnCluster,
+        "The request affinity cannot be established because no destinations are found on cluster `{clusterId}`.");
+
+    private static readonly Action<ILogger, Exception?> _requestAffinityKeyDecryptionFailed = LoggerMessage.Define(
+        LogLevel.Error,
+        EventIds.RequestAffinityKeyDecryptionFailed,
+        "The request affinity key decryption failed.");
+
+    private static readonly Action<ILogger, string, Exception?> _destinationMatchingToAffinityKeyNotFound = LoggerMessage.Define<string>(
+        LogLevel.Warning,
+        EventIds.DestinationMatchingToAffinityKeyNotFound,
+        "Destination matching to the request affinity key is not found on cluster `{clusterId}`. Configured failure policy will be applied.");
+
+    public static void AffinityCannotBeEstablishedBecauseNoDestinationsFound(ILogger logger, string clusterId)
+    {
+        _affinityCannotBeEstablishedBecauseNoDestinationsFound(logger, clusterId, null);
+    }
+
+    public static void RequestAffinityKeyDecryptionFailed(ILogger logger, Exception? ex)
+    {
+        _requestAffinityKeyDecryptionFailed(logger, ex);
+    }
+
+    public static void DestinationMatchingToAffinityKeyNotFound(ILogger logger, string clusterId)
+    {
+        _destinationMatchingToAffinityKeyNotFound(logger, clusterId, null);
+    }
+}