Cherrypick token endpoint telemetry fix, Fixes AB#3604499

changelog.txt

@@ -3,6 +3,7 @@ vNext
 
 Version 24.3.0-RC2
 ----------
+- [PATCH] Fix Token Endpoint Server Telemetry Parsing (#3128)
 - [PATCH] Emit ipc_strategy telemetry attribute for successful device registration IPC strategy and refactor execute flow to pack protocol request once before strategy retries (#3124)
 - [PATCH] Fix Edge browser selection on devices where Microsoft Edge is the default browser: add the rotated Edge signing certificate hash to the Edge BrowserDescriptor and accept multi-signer browsers when any signature intersects the safelist, instead of requiring strict set-equality (resolves MSAL #2414)
 - [MINOR] Refactor Auth Tab integration to use provider-based strategy selection. Adds AuthTabStrategyProvider and BrowserLaunchStrategy with Custom Tabs fallback. Compatible with androidx.browser:browser:1.7.0.

common4j/src/main/com/microsoft/identity/common/java/providers/microsoft/microsoftsts/AbstractMicrosoftStsTokenResponseHandler.java

@@ -31,6 +31,7 @@
 import com.microsoft.identity.common.java.exception.ClientException;
 import com.microsoft.identity.common.java.flighting.CommonFlight;
 import com.microsoft.identity.common.java.flighting.CommonFlightsManager;
+import com.microsoft.identity.common.java.logging.Logger;
 import com.microsoft.identity.common.java.net.HttpResponse;
 import com.microsoft.identity.common.java.opentelemetry.AttributeName;
 import com.microsoft.identity.common.java.opentelemetry.SpanExtension;
@@ -43,6 +44,7 @@
 import com.microsoft.identity.common.java.util.HeaderSerializationUtil;
 import com.microsoft.identity.common.java.util.ObjectMapper;
 import com.microsoft.identity.common.java.util.ResultUtil;
+import com.microsoft.identity.common.java.util.StringUtil;
 
 import java.net.HttpURLConnection;
 import java.util.HashMap;
@@ -109,11 +111,27 @@ public TokenResult handleTokenResponse(@NonNull final HttpResponse response) thr
             }
 
             final String clientDataHeader = response.getHeaderValue(X_MS_CLIENTDATA, 0);
-            if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_SERVER_CLIENT_DATA_TELEMETRY)) {
-                final ClientDataInfo clientDataInfo = ClientDataInfo.fromPipeDelimited(clientDataHeader);
-                if (null != clientDataInfo) {
-                    clientDataInfo.emitToSpan();
-                    result.setClientDataInfo(clientDataInfo);
+            if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_SERVER_CLIENT_DATA_TELEMETRY)
+                    && !StringUtil.isNullOrEmpty(clientDataHeader)) {
+                // eSTS URL-encodes the pipe-delimited value in the response header (e.g. "m%7C0x800482A5%7C%7C...").
+                // ClientDataInfo.fromPipeDelimited expects an already-decoded value (its contract matches the
+                // authorize-endpoint path, where UrlUtil#getParameters has already decoded the query param).
+                // Decode here so the token-endpoint header path produces the same shape.
+                String decodedClientDataHeader = null;
+                try {
+                    decodedClientDataHeader = StringUtil.urlFormDecode(clientDataHeader);
+                } catch (final Exception e) {
+                    // Malformed percent-encoding shouldn't break token parsing; swallow and fall through.
+                    Logger.warn(methodTag, "Failed to URL-decode x-ms-clientdata header: " + e.getMessage());
+                    // Emit that we failed to decode the clientdata value
+                    SpanExtension.current().setAttribute(AttributeName.server_error.name(), "msal_android_decoding_failed");
+                }
+                if (decodedClientDataHeader != null) {
+                    final ClientDataInfo clientDataInfo = ClientDataInfo.fromPipeDelimited(decodedClientDataHeader);
+                    if (null != clientDataInfo) {
+                        clientDataInfo.emitToSpan();
+                        result.setClientDataInfo(clientDataInfo);
+                    }
                 }
             }
 

common4j/src/main/com/microsoft/identity/common/java/telemetry/ClientDataInfo.java

@@ -130,6 +130,9 @@ public static ClientDataInfo fromPipeDelimited(@Nullable final String decodedVal
             return info;
         } catch (final Exception e) {
             Logger.warn(TAG, "Failed to parse clientdata pipe-delimited value: " + e.getMessage());
+            // Emit that we failed to parse the clientdata value
+            final Span span = SpanExtension.current();
+            span.setAttribute(AttributeName.server_error.name(), "msal_android_parsing_failed");
             return null;
         }
     }

common4j/src/test/com/microsoft/identity/common/java/providers/microsoft/microsoftsts/MicrosoftStsTokenResponseHandlerTest.java

@@ -139,6 +139,48 @@ public void testHandleTokenResponse_withClientDataHeader_attributesEmitted() {
         }
     }
 
+    @SneakyThrows
+    @Test
+    public void testHandleTokenResponse_withUrlEncodedClientDataHeader_attributesEmitted() {
+        // eSTS URL-encodes pipe separators in the response header (e.g. "%7C" for "|").
+        // Real-world example: x-ms-clientdata=[m%7C0x800482A5%7C%7Cmicrosoftonline.com%7Cnone]
+        final String encodedClientDataHeader = "m%7C0x800482A5%7C%7Cmicrosoftonline.com%7Cnone";
+        final HashMap<String, List<String>> headers = new HashMap<>();
+        headers.put("Content-Type", Collections.singletonList("application/json; charset=utf-8"));
+        headers.put(HttpConstants.HeaderField.X_MS_CLIENTDATA,
+                Collections.singletonList(encodedClientDataHeader));
+        final HttpResponse response = new HttpResponse(200, MOCK_TOKEN_SUCCESS_RESPONSE, headers);
+        final MicrosoftStsTokenResponseHandler handler = new MicrosoftStsTokenResponseHandler();
+        final Span mockSpan = mock(Span.class);
+        when(mockSpan.setAttribute(Mockito.anyString(), Mockito.anyString())).thenReturn(mockSpan);
+        try (MockedStatic<SpanExtension> mockedExtension = Mockito.mockStatic(SpanExtension.class)) {
+            mockedExtension.when(SpanExtension::current).thenReturn(mockSpan);
+            final TokenResult tokenResult = handler.handleTokenResponse(response);
+            Assert.assertNotNull(tokenResult);
+            Assert.assertTrue(tokenResult.getSuccess());
+            Assert.assertNotNull(tokenResult.getClientDataInfo());
+            verify(mockSpan).setAttribute(AttributeName.server_error.name(), "0x800482A5");
+            verify(mockSpan).setAttribute(AttributeName.account_type.name(), "MSA");
+        }
+    }
+    @SneakyThrows
+    @Test
+    public void testHandleTokenResponse_withMalformedPercentEncoding_doesNotCrash() {
+        // Lone '%' is invalid percent-encoding; decode should fail gracefully and skip ClientDataInfo.
+        final String malformedHeader = "e|AADSTS50058|%ZZ|us|public";
+        final HashMap<String, List<String>> headers = new HashMap<>();
+        headers.put("Content-Type", Collections.singletonList("application/json; charset=utf-8"));
+        headers.put(HttpConstants.HeaderField.X_MS_CLIENTDATA,
+                Collections.singletonList(malformedHeader));
+        final HttpResponse response = new HttpResponse(200, MOCK_TOKEN_SUCCESS_RESPONSE, headers);
+        final MicrosoftStsTokenResponseHandler handler = new MicrosoftStsTokenResponseHandler();
+        final TokenResult tokenResult = handler.handleTokenResponse(response);
+        Assert.assertNotNull(tokenResult);
+        Assert.assertTrue(tokenResult.getSuccess());
+        // Malformed encoding => null ClientDataInfo, but token result still valid.
+        Assert.assertNull(tokenResult.getClientDataInfo());
+    }
+
     @SneakyThrows
     @Test
     public void testHandleTokenResponse_noClientDataHeader_doesNotCrash() {