diff --git a/src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.Logger.cs b/src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.Logger.cs index b8cfa72de..78173444b 100644 --- a/src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.Logger.cs +++ b/src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.Logger.cs @@ -28,6 +28,17 @@ internal static class Logger DownstreamApiLoggingEventId.UnauthenticatedApiCall, "[MsIdWeb] An unauthenticated call was made to the Api with null Scopes"); + private static readonly Action s_reservedHeaderIgnored = + LoggerMessage.Define( + LogLevel.Warning, + DownstreamApiLoggingEventId.ReservedHeaderIgnored, + "[MsIdWeb] Header '{HeaderName}' supplied through ExtraHeaderParameters was ignored because the name is reserved for the library."); + + private static readonly Action s_duplicateHeaderIgnored = + LoggerMessage.Define( + LogLevel.Warning, + DownstreamApiLoggingEventId.DuplicateHeaderIgnored, + "[MsIdWeb] Header '{HeaderName}' supplied through ExtraHeaderParameters was ignored because the request already carries a value for it."); /// /// Logger for handling options exceptions in DownstreamApi. @@ -52,6 +63,25 @@ public static void HttpRequestError( public static void UnauthenticatedApiCall( ILogger logger, Exception? ex) => s_unauthenticatedApiCall(logger, ex); + + /// + /// Logs that an ExtraHeaderParameters entry was skipped because its name is reserved. + /// + /// Logger. + /// Header name that was ignored. + public static void ReservedHeaderIgnored( + ILogger logger, + string headerName) => s_reservedHeaderIgnored(logger, headerName, null); + + /// + /// Logs that an ExtraHeaderParameters entry was skipped because the request already + /// carries a value for the same header name. + /// + /// Logger. + /// Header name that was ignored. + public static void DuplicateHeaderIgnored( + ILogger logger, + string headerName) => s_duplicateHeaderIgnored(logger, headerName, null); } } } diff --git a/src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.cs b/src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.cs index 4886ebc82..f5e9eabf8 100644 --- a/src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.cs +++ b/src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.cs @@ -582,48 +582,63 @@ public Task CallApiForAppAsync( { Logger.UnauthenticatedApiCall(_logger, null); } - if (!string.IsNullOrEmpty(effectiveOptions.AcceptHeader)) - { - httpRequestMessage.Headers.Accept.ParseAdd(effectiveOptions.AcceptHeader); - } - - // Add extra headers if specified directly on DownstreamApiOptions - if (effectiveOptions.ExtraHeaderParameters != null) - { - foreach (var header in effectiveOptions.ExtraHeaderParameters) - { - httpRequestMessage.Headers.TryAddWithoutValidation(header.Key, header.Value); - } - } - - // Add extra query parameters if specified directly on DownstreamApiOptions - if (effectiveOptions.ExtraQueryParameters != null && effectiveOptions.ExtraQueryParameters.Count > 0) - { - var uriBuilder = new UriBuilder(httpRequestMessage.RequestUri!); - var existingQuery = uriBuilder.Query; - var queryString = new StringBuilder(existingQuery); - - foreach (var queryParam in effectiveOptions.ExtraQueryParameters) - { - if (queryString.Length > 1) // if there are existing query parameters - { - queryString.Append('&'); - } - else if (queryString.Length == 0) - { - queryString.Append('?'); - } - - queryString.Append(Uri.EscapeDataString(queryParam.Key)); - queryString.Append('='); - queryString.Append(Uri.EscapeDataString(queryParam.Value)); - } - - uriBuilder.Query = queryString.ToString().TrimStart('?'); - httpRequestMessage.RequestUri = uriBuilder.Uri; - } - - // Opportunity to change the request message + + if (!string.IsNullOrEmpty(effectiveOptions.AcceptHeader)) + { + httpRequestMessage.Headers.Accept.ParseAdd(effectiveOptions.AcceptHeader); + } + + // Add extra headers if specified directly on DownstreamApiOptions. + // Skip names that are reserved for the library or already present on + // the outgoing request to keep the library-set values authoritative. + if (effectiveOptions.ExtraHeaderParameters != null) + { + foreach (var header in effectiveOptions.ExtraHeaderParameters) + { + if (ReservedHeaderNames.IsReserved(header.Key)) + { + Logger.ReservedHeaderIgnored(_logger, header.Key); + continue; + } + + if (httpRequestMessage.Headers.Contains(header.Key)) + { + Logger.DuplicateHeaderIgnored(_logger, header.Key); + continue; + } + + httpRequestMessage.Headers.TryAddWithoutValidation(header.Key, header.Value); + } + } + + // Add extra query parameters if specified directly on DownstreamApiOptions + if (effectiveOptions.ExtraQueryParameters != null && effectiveOptions.ExtraQueryParameters.Count > 0) + { + var uriBuilder = new UriBuilder(httpRequestMessage.RequestUri!); + var existingQuery = uriBuilder.Query; + var queryString = new StringBuilder(existingQuery); + + foreach (var queryParam in effectiveOptions.ExtraQueryParameters) + { + if (queryString.Length > 1) // if there are existing query parameters + { + queryString.Append('&'); + } + else if (queryString.Length == 0) + { + queryString.Append('?'); + } + + queryString.Append(Uri.EscapeDataString(queryParam.Key)); + queryString.Append('='); + queryString.Append(Uri.EscapeDataString(queryParam.Value)); + } + + uriBuilder.Query = queryString.ToString().TrimStart('?'); + httpRequestMessage.RequestUri = uriBuilder.Uri; + } + + // Opportunity to change the request message effectiveOptions.CustomizeHttpRequestMessage?.Invoke(httpRequestMessage); } diff --git a/src/Microsoft.Identity.Web.DownstreamApi/DownstreamApiLoggingEventId.cs b/src/Microsoft.Identity.Web.DownstreamApi/DownstreamApiLoggingEventId.cs index a6865557d..d0a4b17e1 100644 --- a/src/Microsoft.Identity.Web.DownstreamApi/DownstreamApiLoggingEventId.cs +++ b/src/Microsoft.Identity.Web.DownstreamApi/DownstreamApiLoggingEventId.cs @@ -11,6 +11,8 @@ internal static class DownstreamApiLoggingEventId // DownstreamApi EventIds 100+ public static readonly EventId HttpRequestError = new(100, "HttpRequestError"); public static readonly EventId UnauthenticatedApiCall = new(101, "UnauthenticatedApiCall"); + public static readonly EventId ReservedHeaderIgnored = new(102, "ReservedHeaderIgnored"); + public static readonly EventId DuplicateHeaderIgnored = new(103, "DuplicateHeaderIgnored"); #pragma warning restore IDE1006 // Naming styles } } diff --git a/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net462/InternalAPI.Unshipped.txt b/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net462/InternalAPI.Unshipped.txt index 7dc5c5811..1b5f1352f 100644 --- a/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net462/InternalAPI.Unshipped.txt +++ b/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net462/InternalAPI.Unshipped.txt @@ -1 +1,7 @@ #nullable enable +Microsoft.Identity.Web.ReservedHeaderNames +static Microsoft.Identity.Web.DownstreamApi.Logger.DuplicateHeaderIgnored(Microsoft.Extensions.Logging.ILogger! logger, string! headerName) -> void +static Microsoft.Identity.Web.DownstreamApi.Logger.ReservedHeaderIgnored(Microsoft.Extensions.Logging.ILogger! logger, string! headerName) -> void +static Microsoft.Identity.Web.ReservedHeaderNames.IsReserved(string! headerName) -> bool +static readonly Microsoft.Identity.Web.DownstreamApiLoggingEventId.DuplicateHeaderIgnored -> Microsoft.Extensions.Logging.EventId +static readonly Microsoft.Identity.Web.DownstreamApiLoggingEventId.ReservedHeaderIgnored -> Microsoft.Extensions.Logging.EventId diff --git a/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net472/InternalAPI.Unshipped.txt b/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net472/InternalAPI.Unshipped.txt index 7dc5c5811..1b5f1352f 100644 --- a/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net472/InternalAPI.Unshipped.txt +++ b/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net472/InternalAPI.Unshipped.txt @@ -1 +1,7 @@ #nullable enable +Microsoft.Identity.Web.ReservedHeaderNames +static Microsoft.Identity.Web.DownstreamApi.Logger.DuplicateHeaderIgnored(Microsoft.Extensions.Logging.ILogger! logger, string! headerName) -> void +static Microsoft.Identity.Web.DownstreamApi.Logger.ReservedHeaderIgnored(Microsoft.Extensions.Logging.ILogger! logger, string! headerName) -> void +static Microsoft.Identity.Web.ReservedHeaderNames.IsReserved(string! headerName) -> bool +static readonly Microsoft.Identity.Web.DownstreamApiLoggingEventId.DuplicateHeaderIgnored -> Microsoft.Extensions.Logging.EventId +static readonly Microsoft.Identity.Web.DownstreamApiLoggingEventId.ReservedHeaderIgnored -> Microsoft.Extensions.Logging.EventId diff --git a/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net6.0/InternalAPI.Unshipped.txt b/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net6.0/InternalAPI.Unshipped.txt index 7dc5c5811..1b5f1352f 100644 --- a/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net6.0/InternalAPI.Unshipped.txt +++ b/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net6.0/InternalAPI.Unshipped.txt @@ -1 +1,7 @@ #nullable enable +Microsoft.Identity.Web.ReservedHeaderNames +static Microsoft.Identity.Web.DownstreamApi.Logger.DuplicateHeaderIgnored(Microsoft.Extensions.Logging.ILogger! logger, string! headerName) -> void +static Microsoft.Identity.Web.DownstreamApi.Logger.ReservedHeaderIgnored(Microsoft.Extensions.Logging.ILogger! logger, string! headerName) -> void +static Microsoft.Identity.Web.ReservedHeaderNames.IsReserved(string! headerName) -> bool +static readonly Microsoft.Identity.Web.DownstreamApiLoggingEventId.DuplicateHeaderIgnored -> Microsoft.Extensions.Logging.EventId +static readonly Microsoft.Identity.Web.DownstreamApiLoggingEventId.ReservedHeaderIgnored -> Microsoft.Extensions.Logging.EventId diff --git a/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net7.0/InternalAPI.Unshipped.txt b/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net7.0/InternalAPI.Unshipped.txt index 7dc5c5811..1b5f1352f 100644 --- a/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net7.0/InternalAPI.Unshipped.txt +++ b/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net7.0/InternalAPI.Unshipped.txt @@ -1 +1,7 @@ #nullable enable +Microsoft.Identity.Web.ReservedHeaderNames +static Microsoft.Identity.Web.DownstreamApi.Logger.DuplicateHeaderIgnored(Microsoft.Extensions.Logging.ILogger! logger, string! headerName) -> void +static Microsoft.Identity.Web.DownstreamApi.Logger.ReservedHeaderIgnored(Microsoft.Extensions.Logging.ILogger! logger, string! headerName) -> void +static Microsoft.Identity.Web.ReservedHeaderNames.IsReserved(string! headerName) -> bool +static readonly Microsoft.Identity.Web.DownstreamApiLoggingEventId.DuplicateHeaderIgnored -> Microsoft.Extensions.Logging.EventId +static readonly Microsoft.Identity.Web.DownstreamApiLoggingEventId.ReservedHeaderIgnored -> Microsoft.Extensions.Logging.EventId diff --git a/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net8.0/InternalAPI.Unshipped.txt b/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net8.0/InternalAPI.Unshipped.txt index 7dc5c5811..1b5f1352f 100644 --- a/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net8.0/InternalAPI.Unshipped.txt +++ b/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net8.0/InternalAPI.Unshipped.txt @@ -1 +1,7 @@ #nullable enable +Microsoft.Identity.Web.ReservedHeaderNames +static Microsoft.Identity.Web.DownstreamApi.Logger.DuplicateHeaderIgnored(Microsoft.Extensions.Logging.ILogger! logger, string! headerName) -> void +static Microsoft.Identity.Web.DownstreamApi.Logger.ReservedHeaderIgnored(Microsoft.Extensions.Logging.ILogger! logger, string! headerName) -> void +static Microsoft.Identity.Web.ReservedHeaderNames.IsReserved(string! headerName) -> bool +static readonly Microsoft.Identity.Web.DownstreamApiLoggingEventId.DuplicateHeaderIgnored -> Microsoft.Extensions.Logging.EventId +static readonly Microsoft.Identity.Web.DownstreamApiLoggingEventId.ReservedHeaderIgnored -> Microsoft.Extensions.Logging.EventId diff --git a/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net9.0/InternalAPI.Unshipped.txt b/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net9.0/InternalAPI.Unshipped.txt index 7dc5c5811..1b5f1352f 100644 --- a/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net9.0/InternalAPI.Unshipped.txt +++ b/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net9.0/InternalAPI.Unshipped.txt @@ -1 +1,7 @@ #nullable enable +Microsoft.Identity.Web.ReservedHeaderNames +static Microsoft.Identity.Web.DownstreamApi.Logger.DuplicateHeaderIgnored(Microsoft.Extensions.Logging.ILogger! logger, string! headerName) -> void +static Microsoft.Identity.Web.DownstreamApi.Logger.ReservedHeaderIgnored(Microsoft.Extensions.Logging.ILogger! logger, string! headerName) -> void +static Microsoft.Identity.Web.ReservedHeaderNames.IsReserved(string! headerName) -> bool +static readonly Microsoft.Identity.Web.DownstreamApiLoggingEventId.DuplicateHeaderIgnored -> Microsoft.Extensions.Logging.EventId +static readonly Microsoft.Identity.Web.DownstreamApiLoggingEventId.ReservedHeaderIgnored -> Microsoft.Extensions.Logging.EventId diff --git a/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/netstandard2.0/InternalAPI.Unshipped.txt b/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/netstandard2.0/InternalAPI.Unshipped.txt index 7dc5c5811..1b5f1352f 100644 --- a/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/netstandard2.0/InternalAPI.Unshipped.txt +++ b/src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/netstandard2.0/InternalAPI.Unshipped.txt @@ -1 +1,7 @@ #nullable enable +Microsoft.Identity.Web.ReservedHeaderNames +static Microsoft.Identity.Web.DownstreamApi.Logger.DuplicateHeaderIgnored(Microsoft.Extensions.Logging.ILogger! logger, string! headerName) -> void +static Microsoft.Identity.Web.DownstreamApi.Logger.ReservedHeaderIgnored(Microsoft.Extensions.Logging.ILogger! logger, string! headerName) -> void +static Microsoft.Identity.Web.ReservedHeaderNames.IsReserved(string! headerName) -> bool +static readonly Microsoft.Identity.Web.DownstreamApiLoggingEventId.DuplicateHeaderIgnored -> Microsoft.Extensions.Logging.EventId +static readonly Microsoft.Identity.Web.DownstreamApiLoggingEventId.ReservedHeaderIgnored -> Microsoft.Extensions.Logging.EventId diff --git a/src/Microsoft.Identity.Web.DownstreamApi/ReservedHeaderNames.cs b/src/Microsoft.Identity.Web.DownstreamApi/ReservedHeaderNames.cs new file mode 100644 index 000000000..f830f2fe7 --- /dev/null +++ b/src/Microsoft.Identity.Web.DownstreamApi/ReservedHeaderNames.cs @@ -0,0 +1,65 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; + +namespace Microsoft.Identity.Web +{ + /// + /// Reserved header names that callers must not provide through + /// . + /// The library either sets these itself, or they have host-level meaning that + /// should not be controlled by per-request configuration. + /// + internal static class ReservedHeaderNames + { + // Exact-match names (case-insensitive). + private static readonly HashSet s_exactNames = new(StringComparer.OrdinalIgnoreCase) + { + "Authorization", + "Cookie", + "Host", + "X-Original-URL", + "X-MS-CLIENT-PRINCIPAL", + "X-MS-CLIENT-PRINCIPAL-ID", + "X-MS-CLIENT-PRINCIPAL-NAME", + "X-MS-CLIENT-PRINCIPAL-IDP", + }; + + // Prefix-match names (case-insensitive). Any header name starting with one of + // these prefixes is treated as reserved. + private static readonly string[] s_prefixes = new[] + { + "X-Forwarded-", + "X-MS-TOKEN-AAD-", + }; + + /// + /// Returns when matches any + /// reserved exact name or reserved prefix. + /// + public static bool IsReserved(string headerName) + { + if (string.IsNullOrEmpty(headerName)) + { + return false; + } + + if (s_exactNames.Contains(headerName)) + { + return true; + } + + for (int i = 0; i < s_prefixes.Length; i++) + { + if (headerName.StartsWith(s_prefixes[i], StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + + return false; + } + } +} diff --git a/tests/Microsoft.Identity.Web.Test/DownstreamWebApiSupport/DownstreamApiTests.cs b/tests/Microsoft.Identity.Web.Test/DownstreamWebApiSupport/DownstreamApiTests.cs index fe356b089..bba8020c7 100644 --- a/tests/Microsoft.Identity.Web.Test/DownstreamWebApiSupport/DownstreamApiTests.cs +++ b/tests/Microsoft.Identity.Web.Test/DownstreamWebApiSupport/DownstreamApiTests.cs @@ -158,6 +158,134 @@ public async Task UpdateRequestAsync_WithScopes_AddsSamlAuthorizationHeaderToReq Assert.Equal(options.AcquireTokenOptions.ExtraQueryParameters, DownstreamApi.CallerSDKDetails); } + [Fact] + public async Task UpdateRequestAsync_ExtraHeaderParameters_CallerProvidedAuthorization_IsIgnored() + { + // Arrange + var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "https://example.com"); + var content = new StringContent("test content"); + var options = new DownstreamApiOptions + { + Scopes = ["scope1"], + BaseUrl = "https://localhost:44321/WeatherForecast", + ExtraHeaderParameters = new Dictionary + { + { "Authorization", "Bearer caller-supplied" } + } + }; + + // Act + await _input.UpdateRequestAsync(httpRequestMessage, content, options, false, new ClaimsPrincipal(), CancellationToken.None); + + // Assert + Assert.Single(httpRequestMessage.Headers.GetValues("Authorization")); + Assert.Equal("ey", httpRequestMessage.Headers.Authorization?.Parameter); + Assert.Equal("Bearer", httpRequestMessage.Headers.Authorization?.Scheme); + } + + [Theory] + [InlineData("Authorization")] + [InlineData("authorization")] + [InlineData("Cookie")] + [InlineData("Host")] + [InlineData("X-Original-URL")] + [InlineData("X-MS-CLIENT-PRINCIPAL")] + [InlineData("X-MS-CLIENT-PRINCIPAL-ID")] + [InlineData("X-MS-CLIENT-PRINCIPAL-NAME")] + [InlineData("X-MS-CLIENT-PRINCIPAL-IDP")] + public async Task UpdateRequestAsync_ExtraHeaderParameters_ReservedNames_AreIgnored(string headerName) + { + // Arrange + var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "https://example.com"); + var content = new StringContent("test content"); + var options = new DownstreamApiOptions + { + ExtraHeaderParameters = new Dictionary + { + { headerName, "caller-supplied-value" } + } + }; + + // Act + await _input.UpdateRequestAsync(httpRequestMessage, content, options, false, null, CancellationToken.None); + + // Assert + Assert.False(httpRequestMessage.Headers.Contains(headerName)); + } + + [Theory] + [InlineData("X-Forwarded-For")] + [InlineData("X-Forwarded-Host")] + [InlineData("X-Forwarded-Proto")] + [InlineData("x-forwarded-for")] + [InlineData("X-MS-TOKEN-AAD-ID-TOKEN")] + [InlineData("X-MS-TOKEN-AAD-ACCESS-TOKEN")] + [InlineData("x-ms-token-aad-refresh-token")] + public async Task UpdateRequestAsync_ExtraHeaderParameters_ReservedPrefixes_AreIgnored(string headerName) + { + // Arrange + var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "https://example.com"); + var content = new StringContent("test content"); + var options = new DownstreamApiOptions + { + ExtraHeaderParameters = new Dictionary + { + { headerName, "caller-supplied-value" } + } + }; + + // Act + await _input.UpdateRequestAsync(httpRequestMessage, content, options, false, null, CancellationToken.None); + + // Assert + Assert.False(httpRequestMessage.Headers.Contains(headerName)); + } + + [Fact] + public async Task UpdateRequestAsync_ExtraHeaderParameters_DuplicateOfLibrarySetHeader_IsIgnored() + { + // Arrange + var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "https://example.com"); + var content = new StringContent("test content"); + var options = new DownstreamApiOptions + { + AcceptHeader = "application/json", + ExtraHeaderParameters = new Dictionary + { + { "Accept", "text/xml" } + } + }; + + // Act + await _input.UpdateRequestAsync(httpRequestMessage, content, options, false, null, CancellationToken.None); + + // Assert + Assert.Single(httpRequestMessage.Headers.Accept); + Assert.Equal("application/json", httpRequestMessage.Headers.Accept.Single().MediaType); + } + + [Fact] + public async Task UpdateRequestAsync_ExtraHeaderParameters_AllowedHeader_IsForwarded() + { + // Arrange + var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "https://example.com"); + var content = new StringContent("test content"); + var options = new DownstreamApiOptions + { + ExtraHeaderParameters = new Dictionary + { + { "X-Custom-Tracking", "trace-id-123" } + } + }; + + // Act + await _input.UpdateRequestAsync(httpRequestMessage, content, options, false, null, CancellationToken.None); + + // Assert + Assert.True(httpRequestMessage.Headers.Contains("X-Custom-Tracking")); + Assert.Equal("trace-id-123", httpRequestMessage.Headers.GetValues("X-Custom-Tracking").Single()); + } + [Fact] public void SerializeInput_ReturnsCorrectHttpContent() {