Merge branch 'main' into nebharg/fic-otel-enricher

Author: neha-bhargava

docs/sni_mtls_pop_token_design.md

@@ -88,7 +88,6 @@ IConfidentialClientApplication app = ConfidentialClientApplicationBuilder.Create
     .WithAuthority(authority)
     .WithAzureRegion("westus")
     .WithCertificate(certificate, true)
-    .WithExperimentalFeatures(true)
     .Build();
 
 AuthenticationResult result = await app.AcquireTokenForClient(scopes).WithMtlsProofOfPossession()

src/client/Microsoft.Identity.Client/Internal/Logger/LoggerHelper.cs

@@ -76,7 +76,7 @@ public static ILoggerAdapter CreateLogger(
 
         public static string FormatLogMessage(string message, bool piiEnabled, string correlationId, string clientInformation)
         {
-            return string.Format(
+            string formattedMessage = string.Format(
                 CultureInfo.InvariantCulture,
                 "{0} MSAL {1} {2} {3} {4} [{5}{6}]{7} {8}",
                 piiEnabled,
@@ -88,6 +88,8 @@ public static string FormatLogMessage(string message, bool piiEnabled, string co
                 correlationId,
                 clientInformation,
                 message);
+
+            return formattedMessage;
         }
 
         internal static string GetPiiScrubbedExceptionDetails(Exception ex)

src/client/Microsoft.Identity.Client/Internal/Logger/MsalLoggerExtensions.cs

@@ -30,7 +30,12 @@ public static void Error(this ILoggerAdapter logger, string message)
 
         public static void ErrorPiiWithPrefix(this ILoggerAdapter logger, Exception exWithPii, string prefix)
         {
-            logger.Log(LogLevel.Error, prefix + exWithPii, prefix + LoggerHelper.GetPiiScrubbedExceptionDetails(exWithPii));
+            if (!logger.IsLoggingEnabled(LogLevel.Error))
+            {
+                return;
+            }
+
+            logger.Log(LogLevel.Error, prefix + TokenScrubber.Scrub(exWithPii?.ToString()), prefix + LoggerHelper.GetPiiScrubbedExceptionDetails(exWithPii));
         }
 
         public static void ErrorPii(this ILoggerAdapter logger, string messageWithPii, string messageScrubbed)
@@ -40,7 +45,12 @@ public static void ErrorPii(this ILoggerAdapter logger, string messageWithPii, s
 
         public static void ErrorPii(this ILoggerAdapter logger, Exception exWithPii)
         {
-            logger.Log(LogLevel.Error, exWithPii.ToString(), LoggerHelper.GetPiiScrubbedExceptionDetails(exWithPii));
+            if (!logger.IsLoggingEnabled(LogLevel.Error))
+            {
+                return;
+            }
+
+            logger.Log(LogLevel.Error, TokenScrubber.Scrub(exWithPii?.ToString()), LoggerHelper.GetPiiScrubbedExceptionDetails(exWithPii));
         }
 
         public static void Warning(this ILoggerAdapter logger, string message)
@@ -55,12 +65,22 @@ public static void WarningPii(this ILoggerAdapter logger, string messageWithPii,
 
         public static void WarningPii(this ILoggerAdapter logger, Exception exWithPii)
         {
-            logger.Log(LogLevel.Warning, exWithPii.ToString(), LoggerHelper.GetPiiScrubbedExceptionDetails(exWithPii));
+            if (!logger.IsLoggingEnabled(LogLevel.Warning))
+            {
+                return;
+            }
+
+            logger.Log(LogLevel.Warning, TokenScrubber.Scrub(exWithPii?.ToString()), LoggerHelper.GetPiiScrubbedExceptionDetails(exWithPii));
         }
 
         public static void WarningPiiWithPrefix(this ILoggerAdapter logger, Exception exWithPii, string prefix)
         {
-            logger.Log(LogLevel.Warning, prefix + exWithPii, prefix + LoggerHelper.GetPiiScrubbedExceptionDetails(exWithPii));
+            if (!logger.IsLoggingEnabled(LogLevel.Warning))
+            {
+                return;
+            }
+
+            logger.Log(LogLevel.Warning, prefix + TokenScrubber.Scrub(exWithPii?.ToString()), prefix + LoggerHelper.GetPiiScrubbedExceptionDetails(exWithPii));
         }
 
         public static void Info(this ILoggerAdapter logger, string message)
@@ -97,7 +117,12 @@ public static void InfoPii(this ILoggerAdapter logger, Func<string> messageWithP
 
         public static void InfoPii(this ILoggerAdapter logger, Exception exWithPii)
         {
-            logger.Log(LogLevel.Info, exWithPii?.ToString(), LoggerHelper.GetPiiScrubbedExceptionDetails(exWithPii));
+            if (!logger.IsLoggingEnabled(LogLevel.Info))
+            {
+                return;
+            }
+
+            logger.Log(LogLevel.Info, TokenScrubber.Scrub(exWithPii?.ToString()), LoggerHelper.GetPiiScrubbedExceptionDetails(exWithPii));
         }
 
         public static void Verbose(this ILoggerAdapter logger, Func<string> messageProducer)

src/client/Microsoft.Identity.Client/Internal/Logger/TokenScrubber.cs

@@ -0,0 +1,128 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+using System.Text;
+
+namespace Microsoft.Identity.Client.Internal.Logger
+{
+    /// <summary>
+    /// Redacts opaque tokens that eSTS/CCS have tagged with a Highly Identifiable Token (HIT) tag
+    /// from MSAL's own log output before the text reaches any logging sink.
+    /// </summary>
+    /// <remarks>
+    /// eSTS/CCS embed the "EvoStsArtifacts" marker (MSA uses the literal "MsaArtifacts") inside the
+    /// base64/base64url encoded token. Because the marker can appear at any of three byte-offset
+    /// alignments once base64 encoded, all three renderings are matched. The marker sits between the
+    /// token header and body, so a match is expanded both left and right over the token charset to
+    /// cover the full token run before replacing it with <see cref="Placeholder"/>.
+    /// </remarks>
+    internal static class TokenScrubber
+    {
+        internal const string Placeholder = "[Redacted opaque token]";
+
+        private const string DisableSwitchName = "Microsoft.Identity.Client.DisableOpaqueTokenScrubbing";
+
+        // Base64 renderings of the "EvoStsArtifacts" prefix at the three possible byte-offset
+        // alignments, plus the MSA literal "MsaArtifacts". Ordinal, case-sensitive.
+        private static readonly string[] s_patterns = new[]
+        {
+            "RXZvU3RzQXJ0aWZhY3Rz", // offset 0
+            "V2b1N0c0FydGlmYWN0c",  // offset 1
+            "dm9TdHNBcnRpZmFjdH",   // offset 2
+            "MsaArtifacts",         // MSA literal
+        };
+
+        private static readonly bool s_disabled = AppContext.TryGetSwitch(DisableSwitchName, out bool isDisabled) && isDisabled;
+
+        /// <summary>
+        /// Redacts any HIT-tagged opaque tokens found in <paramref name="message"/>.
+        /// </summary>
+        /// <param name="message">The log message that may contain a tagged opaque token.</param>
+        /// <returns>
+        /// The same string reference when nothing was redacted (fast path, no allocation); otherwise a new
+        /// string with each tagged token run replaced by <see cref="Placeholder"/>.
+        /// </returns>
+        /// <example>
+        /// <code>
+        /// string scrubbed = TokenScrubber.Scrub("Set-Cookie: esctx-x=AQAB...RXZvU3RzQXJ0aWZhY3Rz...; path=/");
+        /// // scrubbed == "Set-Cookie: [Redacted opaque token]; path=/"
+        /// </code>
+        /// </example>
+        public static string Scrub(string message)
+        {
+            if (s_disabled || string.IsNullOrEmpty(message))
+            {
+                return message;
+            }
+
+            // Single search for the first match. On the no-match fast path this returns the same
+            // reference with no allocation and without scanning the string twice.
+            int matchStart = FindEarliestMatch(message, 0, out int matchLength);
+            if (matchStart < 0)
+            {
+                return message;
+            }
+
+            var sb = new StringBuilder(message.Length);
+            int index = 0;
+
+            while (matchStart >= 0)
+            {
+                // Expand the match left and right over the token charset to cover the whole run.
+                int runStart = matchStart;
+                while (runStart > index && IsTokenChar(message[runStart - 1]))
+                {
+                    runStart--;
+                }
+
+                int runEnd = matchStart + matchLength; // exclusive
+                while (runEnd < message.Length && IsTokenChar(message[runEnd]))
+                {
+                    runEnd++;
+                }
+
+                // Append text before the run, then the placeholder.
+                sb.Append(message, index, runStart - index);
+                sb.Append(Placeholder);
+
+                index = runEnd;
+                matchStart = index < message.Length ? FindEarliestMatch(message, index, out matchLength) : -1;
+            }
+
+            if (index < message.Length)
+            {
+                sb.Append(message, index, message.Length - index);
+            }
+
+            return sb.ToString();
+        }
+
+        private static int FindEarliestMatch(string message, int startIndex, out int matchLength)
+        {
+            int earliest = -1;
+            matchLength = 0;
+
+            for (int i = 0; i < s_patterns.Length; i++)
+            {
+                int found = message.IndexOf(s_patterns[i], startIndex, StringComparison.Ordinal);
+                if (found >= 0 && (earliest < 0 || found < earliest))
+                {
+                    earliest = found;
+                    matchLength = s_patterns[i].Length;
+                }
+            }
+
+            return earliest;
+        }
+
+        private static bool IsTokenChar(char c)
+        {
+            // base64 + base64url + '=' padding: [A-Za-z0-9+/_-=]
+            return (c >= 'A' && c <= 'Z') ||
+                   (c >= 'a' && c <= 'z') ||
+                   (c >= '0' && c <= '9') ||
+                   c == '+' || c == '/' || c == '_' || c == '-' || c == '=';
+        }
+    }
+}

tests/Microsoft.Identity.Test.Unit/PublicApiTests/LoggerTests.cs

@@ -440,6 +440,86 @@ private void AfterCacheAccessWithLogging(TokenCacheNotificationArgs args)
 
             args.IdentityLogger.Log(entry);
         }
+
+        private const string TaggedOpaqueToken = "AQABCQEAAAAdDD7nRXZvU3RzQXJ0aWZhY3RzDQAAAAAAwEvgQ_kqa0hyAA";
+
+        private static string GetPiiLoggedOutput(Action<ILoggerAdapter> logAction)
+        {
+            var testLogger = new TestIdentityLogger();
+            ILoggerAdapter logger = new IdentityLoggerAdapter(testLogger, Guid.Empty, null, null, enablePiiLogging: true);
+            logAction(logger);
+            return testLogger.StringBuilder.ToString();
+        }
+
+        [TestMethod]
+        [Description("Exception messages that carry an ESTS-tagged opaque token must be scrubbed before logging.")]
+        public void ErrorPii_Exception_WithTaggedToken_IsScrubbed()
+        {
+            // Arrange
+            var exception = new MsalServiceException("some_error", "server rejected the request")
+            {
+                ResponseBody = "{\"error\":\"invalid_grant\",\"opaque\":\"" + TaggedOpaqueToken + "\"}"
+            };
+
+            // Act
+            string output = GetPiiLoggedOutput(logger => logger.ErrorPii(exception));
+
+            // Assert
+            Assert.Contains(TokenScrubber.Placeholder, output);
+            Assert.DoesNotContain("RXZvU3RzQXJ0aWZhY3Rz", output);
+            Assert.DoesNotContain(TaggedOpaqueToken, output);
+        }
+
+        [TestMethod]
+        [Description("Info-level exception logging must also scrub ESTS-tagged opaque tokens.")]
+        public void InfoPii_Exception_WithTaggedToken_IsScrubbed()
+        {
+            // Arrange
+            var exception = new MsalServiceException("some_error", "server rejected the request")
+            {
+                ResponseBody = "{\"opaque\":\"" + TaggedOpaqueToken + "\"}"
+            };
+
+            // Act
+            string output = GetPiiLoggedOutput(logger => logger.InfoPii(exception));
+
+            // Assert
+            Assert.Contains(TokenScrubber.Placeholder, output);
+            Assert.DoesNotContain("RXZvU3RzQXJ0aWZhY3Rz", output);
+        }
+
+        [TestMethod]
+        [Description("Warning-level exception logging must also scrub ESTS-tagged opaque tokens.")]
+        public void WarningPii_Exception_WithTaggedToken_IsScrubbed()
+        {
+            // Arrange
+            var exception = new MsalServiceException("some_error", "server rejected the request")
+            {
+                ResponseBody = "{\"opaque\":\"" + TaggedOpaqueToken + "\"}"
+            };
+
+            // Act
+            string output = GetPiiLoggedOutput(logger => logger.WarningPii(exception));
+
+            // Assert
+            Assert.Contains(TokenScrubber.Placeholder, output);
+            Assert.DoesNotContain("RXZvU3RzQXJ0aWZhY3Rz", output);
+        }
+
+        [TestMethod]
+        [Description("Tactical scope: ordinary (non-exception, non-ESTS) log messages are NOT scrubbed.")]
+        public void InfoPii_PlainMessage_WithTaggedToken_IsNotScrubbed()
+        {
+            // Arrange
+            string message = "diagnostic value " + TaggedOpaqueToken;
+
+            // Act
+            string output = GetPiiLoggedOutput(logger => logger.InfoPii(message, string.Empty));
+
+            // Assert
+            Assert.Contains(TaggedOpaqueToken, output);
+            Assert.DoesNotContain(TokenScrubber.Placeholder, output);
+        }
 
     }
 }