Skip to content

Commit 3208520

Browse files
iNinjaCopilotCopilotpmaytak
authored
DownstreamApi: ignore caller-supplied reserved or duplicate headers in ExtraHeaderParameters (#3793)
When ExtraHeaderParameters contains an entry whose name is reserved for the library (Authorization, Cookie, Host, X-Forwarded-*, X-Original-URL, X-MS-CLIENT-PRINCIPAL*, X-MS-TOKEN-AAD-*) or is already present on the outgoing request (e.g. Accept set via AcceptHeader), the entry is now skipped and a warning is logged instead of being added alongside the library-set value. Adds ReservedHeaderNames helper and two LoggerMessage entries (EventIds 102, 103). All new types and members are internal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pmaytak <34331512+pmaytak@users.noreply.github.com>
1 parent 79a9fc1 commit 3208520

5 files changed

Lines changed: 241 additions & 1 deletion

File tree

‎src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.Logger.cs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,18 @@ internal static class Logger
3030
DownstreamApiLoggingEventId.UnauthenticatedApiCall,
3131
"[MsIdWeb] An unauthenticated call was made to the Api with null Scopes");
3232

33+
private static readonly Action<ILogger, string, Exception?> s_reservedHeaderIgnored =
34+
LoggerMessage.Define<string>(
35+
LogLevel.Warning,
36+
DownstreamApiLoggingEventId.ReservedHeaderIgnored,
37+
"[MsIdWeb] Header '{HeaderName}' supplied through ExtraHeaderParameters was ignored because the name is reserved for the library.");
38+
39+
private static readonly Action<ILogger, string, Exception?> s_duplicateHeaderIgnored =
40+
LoggerMessage.Define<string>(
41+
LogLevel.Warning,
42+
DownstreamApiLoggingEventId.DuplicateHeaderIgnored,
43+
"[MsIdWeb] Header '{HeaderName}' supplied through ExtraHeaderParameters was ignored because the request already carries a value for it.");
44+
3345
/// <summary>
3446
/// Logger for handling options exceptions in DownstreamApi.
3547
/// </summary>
@@ -57,6 +69,25 @@ public static void HttpRequestError(
5769
public static void UnauthenticatedApiCall(
5870
ILogger logger,
5971
Exception? ex) => s_unauthenticatedApiCall(logger, ex);
72+
73+
/// <summary>
74+
/// Logs that an ExtraHeaderParameters entry was skipped because its name is reserved.
75+
/// </summary>
76+
/// <param name="logger">Logger.</param>
77+
/// <param name="headerName">Header name that was ignored.</param>
78+
public static void ReservedHeaderIgnored(
79+
ILogger logger,
80+
string headerName) => s_reservedHeaderIgnored(logger, headerName, null);
81+
82+
/// <summary>
83+
/// Logs that an ExtraHeaderParameters entry was skipped because the request already
84+
/// carries a value for the same header name.
85+
/// </summary>
86+
/// <param name="logger">Logger.</param>
87+
/// <param name="headerName">Header name that was ignored.</param>
88+
public static void DuplicateHeaderIgnored(
89+
ILogger logger,
90+
string headerName) => s_duplicateHeaderIgnored(logger, headerName, null);
6091
}
6192
}
6293
}

‎src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.cs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -784,11 +784,25 @@ public Task<HttpResponseMessage> CallApiForAppAsync(
784784
httpRequestMessage.Headers.Accept.ParseAdd(effectiveOptions.AcceptHeader);
785785
}
786786

787-
// Add extra headers if specified directly on DownstreamApiOptions
787+
// Add extra headers if specified directly on DownstreamApiOptions.
788+
// Skip names that are reserved for the library or already present on
789+
// the outgoing request to keep the library-set values authoritative.
788790
if (effectiveOptions.ExtraHeaderParameters != null)
789791
{
790792
foreach (var header in effectiveOptions.ExtraHeaderParameters)
791793
{
794+
if (ReservedHeaderNames.IsReserved(header.Key))
795+
{
796+
Logger.ReservedHeaderIgnored(_logger, header.Key);
797+
continue;
798+
}
799+
800+
if (httpRequestMessage.Headers.Contains(header.Key))
801+
{
802+
Logger.DuplicateHeaderIgnored(_logger, header.Key);
803+
continue;
804+
}
805+
792806
httpRequestMessage.Headers.TryAddWithoutValidation(header.Key, header.Value);
793807
}
794808
}

‎src/Microsoft.Identity.Web.DownstreamApi/DownstreamApiLoggingEventId.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ internal static class DownstreamApiLoggingEventId
1111
// DownstreamApi EventIds 100+
1212
public static readonly EventId HttpRequestError = new(100, "HttpRequestError");
1313
public static readonly EventId UnauthenticatedApiCall = new(101, "UnauthenticatedApiCall");
14+
public static readonly EventId ReservedHeaderIgnored = new(102, "ReservedHeaderIgnored");
15+
public static readonly EventId DuplicateHeaderIgnored = new(103, "DuplicateHeaderIgnored");
1416
#pragma warning restore IDE1006 // Naming styles
1517
}
1618
}

‎src/Microsoft.Identity.Web.DownstreamApi/ReservedHeaderNames.cs‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
7+
namespace Microsoft.Identity.Web
8+
{
9+
/// <summary>
10+
/// Reserved header names that callers must not provide through
11+
/// <see cref="Microsoft.Identity.Abstractions.DownstreamApiOptions.ExtraHeaderParameters"/>.
12+
/// The library either sets these itself, or they have host-level meaning that
13+
/// should not be controlled by per-request configuration.
14+
/// </summary>
15+
internal static class ReservedHeaderNames
16+
{
17+
// Exact-match names (case-insensitive).
18+
private static readonly HashSet<string> s_exactNames = new(StringComparer.OrdinalIgnoreCase)
19+
{
20+
"Authorization",
21+
"Cookie",
22+
"Host",
23+
"X-Original-URL",
24+
"X-MS-CLIENT-PRINCIPAL",
25+
"X-MS-CLIENT-PRINCIPAL-ID",
26+
"X-MS-CLIENT-PRINCIPAL-NAME",
27+
"X-MS-CLIENT-PRINCIPAL-IDP",
28+
};
29+
30+
// Prefix-match names (case-insensitive). Any header name starting with one of
31+
// these prefixes is treated as reserved.
32+
private static readonly string[] s_prefixes = new[]
33+
{
34+
"X-Forwarded-",
35+
"X-MS-TOKEN-AAD-",
36+
};
37+
38+
/// <summary>
39+
/// Returns <see langword="true"/> when <paramref name="headerName"/> matches any
40+
/// reserved exact name or reserved prefix.
41+
/// </summary>
42+
public static bool IsReserved(string headerName)
43+
{
44+
if (string.IsNullOrEmpty(headerName))
45+
{
46+
return false;
47+
}
48+
49+
if (s_exactNames.Contains(headerName))
50+
{
51+
return true;
52+
}
53+
54+
for (int i = 0; i < s_prefixes.Length; i++)
55+
{
56+
if (headerName.StartsWith(s_prefixes[i], StringComparison.OrdinalIgnoreCase))
57+
{
58+
return true;
59+
}
60+
}
61+
62+
return false;
63+
}
64+
}
65+
}

‎tests/Microsoft.Identity.Web.Test/DownstreamWebApiSupport/DownstreamApiTests.cs‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,134 @@ public async Task UpdateRequestAsync_WithScopes_AddsSamlAuthorizationHeaderToReq
171171
Assert.Equal(options.AcquireTokenOptions.ExtraQueryParameters, DownstreamApi.CallerSDKDetails);
172172
}
173173

174+
[Fact]
175+
public async Task UpdateRequestAsync_ExtraHeaderParameters_CallerProvidedAuthorization_IsIgnored()
176+
{
177+
// Arrange
178+
var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "https://example.com");
179+
var content = new StringContent("test content");
180+
var options = new DownstreamApiOptions
181+
{
182+
Scopes = ["scope1"],
183+
BaseUrl = "https://localhost:44321/WeatherForecast",
184+
ExtraHeaderParameters = new Dictionary<string, string>
185+
{
186+
{ "Authorization", "Bearer caller-supplied" }
187+
}
188+
};
189+
190+
// Act
191+
await _input.UpdateRequestWithCertificateAsync(httpRequestMessage, content, options, false, new ClaimsPrincipal(), CancellationToken.None);
192+
193+
// Assert
194+
Assert.Single(httpRequestMessage.Headers.GetValues("Authorization"));
195+
Assert.Equal("ey", httpRequestMessage.Headers.Authorization?.Parameter);
196+
Assert.Equal("Bearer", httpRequestMessage.Headers.Authorization?.Scheme);
197+
}
198+
199+
[Theory]
200+
[InlineData("Authorization")]
201+
[InlineData("authorization")]
202+
[InlineData("Cookie")]
203+
[InlineData("Host")]
204+
[InlineData("X-Original-URL")]
205+
[InlineData("X-MS-CLIENT-PRINCIPAL")]
206+
[InlineData("X-MS-CLIENT-PRINCIPAL-ID")]
207+
[InlineData("X-MS-CLIENT-PRINCIPAL-NAME")]
208+
[InlineData("X-MS-CLIENT-PRINCIPAL-IDP")]
209+
public async Task UpdateRequestAsync_ExtraHeaderParameters_ReservedNames_AreIgnored(string headerName)
210+
{
211+
// Arrange
212+
var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "https://example.com");
213+
var content = new StringContent("test content");
214+
var options = new DownstreamApiOptions
215+
{
216+
ExtraHeaderParameters = new Dictionary<string, string>
217+
{
218+
{ headerName, "caller-supplied-value" }
219+
}
220+
};
221+
222+
// Act
223+
await _input.UpdateRequestWithCertificateAsync(httpRequestMessage, content, options, false, null, CancellationToken.None);
224+
225+
// Assert
226+
Assert.False(httpRequestMessage.Headers.Contains(headerName));
227+
}
228+
229+
[Theory]
230+
[InlineData("X-Forwarded-For")]
231+
[InlineData("X-Forwarded-Host")]
232+
[InlineData("X-Forwarded-Proto")]
233+
[InlineData("x-forwarded-for")]
234+
[InlineData("X-MS-TOKEN-AAD-ID-TOKEN")]
235+
[InlineData("X-MS-TOKEN-AAD-ACCESS-TOKEN")]
236+
[InlineData("x-ms-token-aad-refresh-token")]
237+
public async Task UpdateRequestAsync_ExtraHeaderParameters_ReservedPrefixes_AreIgnored(string headerName)
238+
{
239+
// Arrange
240+
var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "https://example.com");
241+
var content = new StringContent("test content");
242+
var options = new DownstreamApiOptions
243+
{
244+
ExtraHeaderParameters = new Dictionary<string, string>
245+
{
246+
{ headerName, "caller-supplied-value" }
247+
}
248+
};
249+
250+
// Act
251+
await _input.UpdateRequestWithCertificateAsync(httpRequestMessage, content, options, false, null, CancellationToken.None);
252+
253+
// Assert
254+
Assert.False(httpRequestMessage.Headers.Contains(headerName));
255+
}
256+
257+
[Fact]
258+
public async Task UpdateRequestAsync_ExtraHeaderParameters_DuplicateOfLibrarySetHeader_IsIgnored()
259+
{
260+
// Arrange
261+
var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "https://example.com");
262+
var content = new StringContent("test content");
263+
var options = new DownstreamApiOptions
264+
{
265+
AcceptHeader = "application/json",
266+
ExtraHeaderParameters = new Dictionary<string, string>
267+
{
268+
{ "Accept", "text/xml" }
269+
}
270+
};
271+
272+
// Act
273+
await _input.UpdateRequestWithCertificateAsync(httpRequestMessage, content, options, false, null, CancellationToken.None);
274+
275+
// Assert
276+
Assert.Single(httpRequestMessage.Headers.Accept);
277+
Assert.Equal("application/json", httpRequestMessage.Headers.Accept.Single().MediaType);
278+
}
279+
280+
[Fact]
281+
public async Task UpdateRequestAsync_ExtraHeaderParameters_AllowedHeader_IsForwarded()
282+
{
283+
// Arrange
284+
var httpRequestMessage = new HttpRequestMessage(HttpMethod.Get, "https://example.com");
285+
var content = new StringContent("test content");
286+
var options = new DownstreamApiOptions
287+
{
288+
ExtraHeaderParameters = new Dictionary<string, string>
289+
{
290+
{ "X-Custom-Tracking", "trace-id-123" }
291+
}
292+
};
293+
294+
// Act
295+
await _input.UpdateRequestWithCertificateAsync(httpRequestMessage, content, options, false, null, CancellationToken.None);
296+
297+
// Assert
298+
Assert.True(httpRequestMessage.Headers.Contains("X-Custom-Tracking"));
299+
Assert.Equal("trace-id-123", httpRequestMessage.Headers.GetValues("X-Custom-Tracking").Single());
300+
}
301+
174302
[Fact]
175303
public void SerializeInput_ReturnsCorrectHttpContent()
176304
{

0 commit comments

Comments
 (0)