Skip to content

Commit ce8c4e7

Browse files
spetrescu84Copilot
andcommitted
Address PR review comments: improve warn message, document overwrite behavior, add tests
- NativeAuthCIAMAuthority: Improve misleading WARN log message to clearly explain that custom headers will not be applied when interceptor type does not match - RequestInterceptorHeaderUtils: Add detailed KDoc explaining case-insensitive merge semantics matching iOS behavior and why reserved SDK headers cannot be overwritten - JITInteractorRequestInterceptorTest: Use 3 base headers (was 2) so null interceptor size assertion is more meaningful - SignUpInteractorRequestInterceptorTest: Add test for reserved header overwrite protection and case-insensitive header merge Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1611df9 commit ce8c4e7

4 files changed

Lines changed: 92 additions & 6 deletions

File tree

common4j/src/main/com/microsoft/identity/common/java/nativeauth/authorities/NativeAuthCIAMAuthority.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ class NativeAuthCIAMAuthority (
126126
@Throws(ClientException::class)
127127
override fun createOAuth2Strategy(parameters: OAuth2StrategyParameters): NativeAuthOAuth2Strategy {
128128
if (parameters.mRequestInterceptor != null && parameters.mRequestInterceptor !is NativeAuthRequestInterceptor) {
129-
Logger.warn(TAG, "Ignoring non-native OAuth2RequestInterceptor for NativeAuthCIAMAuthority.")
129+
Logger.warn(TAG, "Request interceptor is not a NativeAuthRequestInterceptor instance; custom headers will not be applied to native auth requests.")
130130
}
131131

132132
val config = createNativeAuthOAuth2Configuration(

common4j/src/main/com/microsoft/identity/common/java/nativeauth/providers/interactors/RequestInterceptorHeaderUtils.kt

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,17 @@ import java.net.URL
2929
/**
3030
* Applies additional interceptor headers to the base request headers for native auth interactors.
3131
*
32-
* Uses case-insensitive merge semantics: interceptor headers replace matching base headers.
33-
* Interceptor headers are validated and normalized to lowercase by [NativeAuthHeaderValidator].
32+
* Uses case-insensitive merge semantics matching iOS behavior: interceptor headers replace
33+
* matching base headers regardless of casing. Interceptor headers are first validated and
34+
* normalized to lowercase by [NativeAuthHeaderValidator], which filters out any non-`x-` prefixed
35+
* headers and reserved prefixes (`x-ms-`, `x-client-`, `x-broker-`, `x-app-`). This ensures that
36+
* mandatory SDK headers (e.g., `Content-Type`, `x-client-SKU`) cannot be overwritten by the
37+
* interceptor, since they either lack the `x-` prefix or use a reserved prefix.
3438
*
3539
* @param requestUrl The outbound request URL.
3640
* @param headers The base request headers.
3741
* @param requestInterceptor Optional interceptor providing additional headers.
38-
* @return The merged headers map with interceptor values taking precedence.
42+
* @return The merged headers map with interceptor values taking precedence for valid custom headers.
3943
*/
4044
internal fun applyInterceptorHeaders(
4145
requestUrl: URL,
@@ -48,6 +52,8 @@ internal fun applyInterceptorHeaders(
4852
val validHeaders = NativeAuthHeaderValidator.filterValidHeaders(additionalHeaders)
4953
if (validHeaders.isEmpty()) return headers
5054

55+
// Case-insensitive merge: matches iOS's [NSMutableURLRequest setValue:forHTTPHeaderField:]
56+
// which replaces existing headers case-insensitively.
5157
val mergedHeaders = headers.toMutableMap()
5258
for ((field, value) in validHeaders) {
5359
val existingHeader = mergedHeaders.keys.firstOrNull { it.equals(field, ignoreCase = true) }

common4j/src/test/com/microsoft/identity/common/java/nativeauth/providers/interactors/JITInteractorRequestInterceptorTest.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ class JITInteractorRequestInterceptorTest {
5757

5858
private val baseHeaders = mapOf<String, String?>(
5959
"Content-Type" to "application/x-www-form-urlencoded",
60-
"x-client-SKU" to "MSAL.Android"
60+
"x-client-SKU" to "MSAL.Android",
61+
"Accept" to "application/json"
6162
)
6263

6364
private val testInterceptor = object : NativeAuthRequestInterceptor {
@@ -153,7 +154,7 @@ class JITInteractorRequestInterceptorTest {
153154
interactor.performIntrospect(createJITIntrospectParams())
154155

155156
assertTrue(capturedHeaders.isCaptured)
156-
assertEquals(2, capturedHeaders.captured.size)
157+
assertEquals(3, capturedHeaders.captured.size)
157158
assertFalse(capturedHeaders.captured.containsKey("x-akamai-sensor"))
158159
}
159160
// endregion

common4j/src/test/com/microsoft/identity/common/java/nativeauth/providers/interactors/SignUpInteractorRequestInterceptorTest.kt

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,4 +271,83 @@ class SignUpInteractorRequestInterceptorTest {
271271
assertEquals(testUrl, capturedUrls[0])
272272
}
273273
// endregion
274+
275+
// region reserved header overwrite protection
276+
@Test
277+
fun testInterceptorCannotOverwriteReservedBaseHeaders() {
278+
val overwriteInterceptor = object : NativeAuthRequestInterceptor {
279+
override fun additionalHeaders(requestUrl: URL): Map<String, String>? {
280+
return mapOf(
281+
"x-client-SKU" to "Evil.SDK",
282+
"x-ms-request-id" to "fake-id",
283+
"x-akamai-sensor" to "valid-data"
284+
)
285+
}
286+
}
287+
288+
val mockRequest = mockk<SignUpStartRequest>(relaxed = true)
289+
every { mockRequest.requestUrl } returns testUrl
290+
every { mockRequest.headers } returns baseHeaders
291+
every { mockRequestProvider.createSignUpStartRequest(any()) } returns mockRequest
292+
every { mockResponseHandler.getSignUpStartResultFromHttpResponse(any(), any()) } returns mockk(relaxed = true)
293+
294+
val capturedHeaders = setupHttpClientCapture()
295+
val interactor = createInteractor(interceptor = overwriteInterceptor)
296+
297+
interactor.performSignUpStart(mockk<SignUpStartCommandParameters>(relaxed = true))
298+
299+
assertTrue(capturedHeaders.isCaptured)
300+
val headers = capturedHeaders.captured
301+
// Reserved prefix headers from interceptor should be filtered, preserving base values
302+
assertEquals("MSAL.Android", headers["x-client-SKU"])
303+
assertFalse("x-ms- prefix should be filtered", headers.containsKey("x-ms-request-id"))
304+
// Valid custom header should be merged
305+
assertEquals("valid-data", headers["x-akamai-sensor"])
306+
}
307+
// endregion
308+
309+
// region case-insensitive header merge
310+
@Test
311+
fun testCaseInsensitiveHeaderMerge() {
312+
val baseHeadersWithCustom = mapOf<String, String?>(
313+
"Content-Type" to "application/x-www-form-urlencoded",
314+
"x-client-SKU" to "MSAL.Android",
315+
"x-existing-custom" to "old-value"
316+
)
317+
318+
val caseInterceptor = object : NativeAuthRequestInterceptor {
319+
override fun additionalHeaders(requestUrl: URL): Map<String, String>? {
320+
return mapOf(
321+
"X-Existing-Custom" to "new-value",
322+
"x-new-header" to "new-data"
323+
)
324+
}
325+
}
326+
327+
val mockRequest = mockk<SignUpStartRequest>(relaxed = true)
328+
every { mockRequest.requestUrl } returns testUrl
329+
every { mockRequest.headers } returns baseHeadersWithCustom
330+
every { mockRequestProvider.createSignUpStartRequest(any()) } returns mockRequest
331+
every { mockResponseHandler.getSignUpStartResultFromHttpResponse(any(), any()) } returns mockk(relaxed = true)
332+
333+
val capturedHeaders = setupHttpClientCapture()
334+
val interactor = createInteractor(interceptor = caseInterceptor)
335+
336+
interactor.performSignUpStart(mockk<SignUpStartCommandParameters>(relaxed = true))
337+
338+
assertTrue(capturedHeaders.isCaptured)
339+
val headers = capturedHeaders.captured
340+
// The original casing key should be replaced by the normalized (lowercase) key from validator
341+
assertFalse(
342+
"Original casing key should be removed",
343+
headers.containsKey("x-existing-custom") && headers.containsKey("x-Existing-Custom")
344+
)
345+
// The value should be the interceptor's new value (validator normalizes to lowercase)
346+
assertEquals("new-value", headers["x-existing-custom"])
347+
// New header should be added
348+
assertEquals("new-data", headers["x-new-header"])
349+
// Base reserved headers should be preserved
350+
assertEquals("MSAL.Android", headers["x-client-SKU"])
351+
}
352+
// endregion
274353
}

0 commit comments

Comments
 (0)