Fix New-line characters are not allowed in header values (#209) (#213)

Author: skg-marimuthu

src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs

@@ -224,8 +224,8 @@ private async Task<bool> EnsureGraphHeadersAsync(string tenantId, CancellationTo
             _logger.LogDebug("Successfully acquired Graph token via Azure CLI");
         }
 
-        // Trim token to remove any newline characters that may cause header validation errors
-        token = token.Trim();
+        // Remove all newline characters and trim whitespace to prevent header validation errors
+        token = token.ReplaceLineEndings(string.Empty).Trim();
 
         _httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token);
         // NOTE: Do NOT add "ConsistencyLevel: eventual" header here.

src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTests.cs

@@ -192,6 +192,201 @@ public async Task LookupServicePrincipalAsync_DoesNotIncludeConsistencyLevelHead
             "ConsistencyLevel header should NOT be present for simple service principal lookup queries. " +
             "This header is only needed for advanced Graph query capabilities and causes HTTP 400 errors otherwise.");
     }
+
+    [Theory]
+    [InlineData("token-with-trailing-newline\n")]
+    [InlineData("token-with-trailing-crlf\r\n")]
+    [InlineData("token\nwith\nembedded\nnewlines")]
+    [InlineData("token\r\nwith\r\nembedded\r\ncrlf")]
+    [InlineData("token\rwith\rcarriage\rreturns")]
+    [InlineData("\nleading-newline-token")]
+    [InlineData("\r\nleading-crlf-token")]
+    [InlineData("  token-with-whitespace  \n")]
+    public async Task GraphGetAsync_SanitizesTokenWithNewlineCharacters(string tokenWithNewlines)
+    {
+        // This test verifies that tokens containing newline characters (\r, \n, \r\n)
+        // are properly sanitized before being used in HTTP Authorization headers.
+        // Without this fix, System.FormatException is thrown:
+        // "New-line characters are not allowed in header values."
+        // Regression test for newline character issue in token handling.
+
+        // Arrange
+        HttpRequestMessage? capturedRequest = null;
+        var handler = new CapturingHttpMessageHandler((req) => capturedRequest = req);
+        var logger = Substitute.For<ILogger<GraphApiService>>();
+        var executor = Substitute.For<CommandExecutor>(Substitute.For<ILogger<CommandExecutor>>());
+
+        // Mock az CLI to return a token WITH newline characters (simulating real-world issue)
+        executor.ExecuteAsync(Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string?>(), Arg.Any<bool>(), Arg.Any<bool>(), Arg.Any<CancellationToken>())
+            .Returns(callInfo =>
+            {
+                var cmd = callInfo.ArgAt<string>(0);
+                var args = callInfo.ArgAt<string>(1);
+
+                if (cmd == "az" && args != null && args.StartsWith("account show", StringComparison.OrdinalIgnoreCase))
+                {
+                    return Task.FromResult(new CommandResult
+                    {
+                        ExitCode = 0,
+                        StandardOutput = "{}",
+                        StandardError = string.Empty
+                    });
+                }
+
+                if (cmd == "az" && args != null && args.Contains("get-access-token", StringComparison.OrdinalIgnoreCase))
+                {
+                    // Return token WITH newline characters - this simulates the real-world issue
+                    return Task.FromResult(new CommandResult
+                    {
+                        ExitCode = 0,
+                        StandardOutput = tokenWithNewlines,
+                        StandardError = string.Empty
+                    });
+                }
+
+                return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty });
+            });
+
+        var service = new GraphApiService(logger, executor, handler);
+
+        // Queue a successful response
+        using var queuedResponse = new HttpResponseMessage(HttpStatusCode.OK)
+        {
+            Content = new StringContent("{\"value\":[]}")
+        };
+        handler.QueueResponse(queuedResponse);
+
+        // Act - This should NOT throw FormatException even with newlines in token
+        var result = await service.GraphGetAsync("tenant-123", "/v1.0/me");
+
+        // Assert
+        capturedRequest.Should().NotBeNull("HTTP request should have been sent");
+        capturedRequest!.Headers.Authorization.Should().NotBeNull("Authorization header should be set");
+        capturedRequest.Headers.Authorization!.Scheme.Should().Be("Bearer");
+
+        // The token in the header should NOT contain any newline characters
+        var actualToken = capturedRequest.Headers.Authorization.Parameter;
+        actualToken.Should().NotBeNull();
+        actualToken.Should().NotContain("\r", "Token should not contain carriage return characters");
+        actualToken.Should().NotContain("\n", "Token should not contain newline characters");
+        actualToken.Should().NotStartWith(" ", "Token should not have leading whitespace");
+        actualToken.Should().NotEndWith(" ", "Token should not have trailing whitespace");
+    }
+
+    [Fact]
+    public async Task GraphGetAsync_TokenFromTokenProvider_SanitizesNewlines()
+    {
+        // This test verifies that tokens from IMicrosoftGraphTokenProvider are also sanitized.
+        // The token provider path uses a different code branch in EnsureGraphHeadersAsync.
+
+        // Arrange
+        HttpRequestMessage? capturedRequest = null;
+        var handler = new CapturingHttpMessageHandler((req) => capturedRequest = req);
+        var logger = Substitute.For<ILogger<GraphApiService>>();
+        var executor = Substitute.For<CommandExecutor>(Substitute.For<ILogger<CommandExecutor>>());
+        var tokenProvider = Substitute.For<IMicrosoftGraphTokenProvider>();
+
+        // Mock token provider to return a token WITH embedded newlines
+        tokenProvider.GetMgGraphAccessTokenAsync(
+            Arg.Any<string>(),
+            Arg.Any<IEnumerable<string>>(),
+            Arg.Any<bool>(),
+            Arg.Any<string?>(),
+            Arg.Any<CancellationToken>())
+            .Returns("token-from-provider\r\nwith-embedded-newlines\n");
+
+        var service = new GraphApiService(logger, executor, handler, tokenProvider);
+
+        // Queue a successful response
+        using var queuedResponse = new HttpResponseMessage(HttpStatusCode.OK)
+        {
+            Content = new StringContent("{\"value\":[]}")
+        };
+        handler.QueueResponse(queuedResponse);
+
+        // Act - Call with scopes to trigger token provider path
+        var result = await service.GraphGetAsync("tenant-123", "/v1.0/me", default, new[] { "User.Read" });
+
+        // Assert
+        capturedRequest.Should().NotBeNull("HTTP request should have been sent");
+        capturedRequest!.Headers.Authorization.Should().NotBeNull("Authorization header should be set");
+
+        var actualToken = capturedRequest.Headers.Authorization!.Parameter;
+        actualToken.Should().NotBeNull();
+        actualToken.Should().NotContain("\r", "Token should not contain carriage return characters");
+        actualToken.Should().NotContain("\n", "Token should not contain newline characters");
+    }
+
+    [Fact]
+    public async Task CheckServicePrincipalCreationPrivilegesAsync_SanitizesTokenWithNewlines()
+    {
+        // This test verifies that CheckServicePrincipalCreationPrivilegesAsync also
+        // sanitizes tokens with newlines. This method has its own token handling code
+        // separate from EnsureGraphHeadersAsync.
+
+        // Arrange
+        HttpRequestMessage? capturedRequest = null;
+        var handler = new CapturingHttpMessageHandler((req) => capturedRequest = req);
+        var logger = Substitute.For<ILogger<GraphApiService>>();
+        var executor = Substitute.For<CommandExecutor>(Substitute.For<ILogger<CommandExecutor>>());
+
+        // Mock az CLI to return a token WITH newline characters
+        executor.ExecuteAsync(Arg.Any<string>(), Arg.Any<string>(), Arg.Any<string?>(), Arg.Any<bool>(), Arg.Any<bool>(), Arg.Any<CancellationToken>())
+            .Returns(callInfo =>
+            {
+                var cmd = callInfo.ArgAt<string>(0);
+                var args = callInfo.ArgAt<string>(1);
+
+                if (cmd == "az" && args != null && args.StartsWith("account show", StringComparison.OrdinalIgnoreCase))
+                {
+                    return Task.FromResult(new CommandResult
+                    {
+                        ExitCode = 0,
+                        StandardOutput = "{}",
+                        StandardError = string.Empty
+                    });
+                }
+
+                if (cmd == "az" && args != null && args.Contains("get-access-token", StringComparison.OrdinalIgnoreCase))
+                {
+                    // Return token WITH embedded newlines
+                    return Task.FromResult(new CommandResult
+                    {
+                        ExitCode = 0,
+                        StandardOutput = "privileges-check-token\r\n\n",
+                        StandardError = string.Empty
+                    });
+                }
+
+                return Task.FromResult(new CommandResult { ExitCode = 0, StandardOutput = string.Empty, StandardError = string.Empty });
+            });
+
+        var service = new GraphApiService(logger, executor, handler);
+
+        // Queue a successful response for the directory roles query
+        using var queuedResponse = new HttpResponseMessage(HttpStatusCode.OK)
+        {
+            Content = new StringContent("{\"value\":[{\"displayName\":\"Application Administrator\"}]}")
+        };
+        handler.QueueResponse(queuedResponse);
+
+        // Act - This should NOT throw FormatException
+        var (hasPrivileges, roles) = await service.CheckServicePrincipalCreationPrivilegesAsync("tenant-123");
+
+        // Assert
+        capturedRequest.Should().NotBeNull("HTTP request should have been sent");
+        capturedRequest!.Headers.Authorization.Should().NotBeNull("Authorization header should be set");
+
+        var actualToken = capturedRequest.Headers.Authorization!.Parameter;
+        actualToken.Should().NotBeNull();
+        actualToken.Should().NotContain("\r", "Token should not contain carriage return characters");
+        actualToken.Should().NotContain("\n", "Token should not contain newline characters");
+        actualToken.Should().Be("privileges-check-token", "Token should be sanitized to just the token value");
+
+        // Also verify the method returns correct results
+        hasPrivileges.Should().BeTrue("User has Application Administrator role");
+        roles.Should().Contain("Application Administrator");
+    }
 }
 
 // Simple test handler that returns queued responses sequentially