Skip to content

Commit fff60fc

Browse files
authored
Cherrypick token endpoint telemetry fix, Fixes AB#3604499 (#3129)
Original PR: #3128 [AB#3604499](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3604499)
1 parent 0c035c5 commit fff60fc

4 files changed

Lines changed: 69 additions & 5 deletions

File tree

changelog.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ vNext
33

44
Version 24.3.0-RC2
55
----------
6+
- [PATCH] Fix Token Endpoint Server Telemetry Parsing (#3128)
67
- [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)
78
- [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)
89
- [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

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import com.microsoft.identity.common.java.exception.ClientException;
3232
import com.microsoft.identity.common.java.flighting.CommonFlight;
3333
import com.microsoft.identity.common.java.flighting.CommonFlightsManager;
34+
import com.microsoft.identity.common.java.logging.Logger;
3435
import com.microsoft.identity.common.java.net.HttpResponse;
3536
import com.microsoft.identity.common.java.opentelemetry.AttributeName;
3637
import com.microsoft.identity.common.java.opentelemetry.SpanExtension;
@@ -43,6 +44,7 @@
4344
import com.microsoft.identity.common.java.util.HeaderSerializationUtil;
4445
import com.microsoft.identity.common.java.util.ObjectMapper;
4546
import com.microsoft.identity.common.java.util.ResultUtil;
47+
import com.microsoft.identity.common.java.util.StringUtil;
4648

4749
import java.net.HttpURLConnection;
4850
import java.util.HashMap;
@@ -109,11 +111,27 @@ public TokenResult handleTokenResponse(@NonNull final HttpResponse response) thr
109111
}
110112

111113
final String clientDataHeader = response.getHeaderValue(X_MS_CLIENTDATA, 0);
112-
if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_SERVER_CLIENT_DATA_TELEMETRY)) {
113-
final ClientDataInfo clientDataInfo = ClientDataInfo.fromPipeDelimited(clientDataHeader);
114-
if (null != clientDataInfo) {
115-
clientDataInfo.emitToSpan();
116-
result.setClientDataInfo(clientDataInfo);
114+
if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_SERVER_CLIENT_DATA_TELEMETRY)
115+
&& !StringUtil.isNullOrEmpty(clientDataHeader)) {
116+
// eSTS URL-encodes the pipe-delimited value in the response header (e.g. "m%7C0x800482A5%7C%7C...").
117+
// ClientDataInfo.fromPipeDelimited expects an already-decoded value (its contract matches the
118+
// authorize-endpoint path, where UrlUtil#getParameters has already decoded the query param).
119+
// Decode here so the token-endpoint header path produces the same shape.
120+
String decodedClientDataHeader = null;
121+
try {
122+
decodedClientDataHeader = StringUtil.urlFormDecode(clientDataHeader);
123+
} catch (final Exception e) {
124+
// Malformed percent-encoding shouldn't break token parsing; swallow and fall through.
125+
Logger.warn(methodTag, "Failed to URL-decode x-ms-clientdata header: " + e.getMessage());
126+
// Emit that we failed to decode the clientdata value
127+
SpanExtension.current().setAttribute(AttributeName.server_error.name(), "msal_android_decoding_failed");
128+
}
129+
if (decodedClientDataHeader != null) {
130+
final ClientDataInfo clientDataInfo = ClientDataInfo.fromPipeDelimited(decodedClientDataHeader);
131+
if (null != clientDataInfo) {
132+
clientDataInfo.emitToSpan();
133+
result.setClientDataInfo(clientDataInfo);
134+
}
117135
}
118136
}
119137

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,9 @@ public static ClientDataInfo fromPipeDelimited(@Nullable final String decodedVal
130130
return info;
131131
} catch (final Exception e) {
132132
Logger.warn(TAG, "Failed to parse clientdata pipe-delimited value: " + e.getMessage());
133+
// Emit that we failed to parse the clientdata value
134+
final Span span = SpanExtension.current();
135+
span.setAttribute(AttributeName.server_error.name(), "msal_android_parsing_failed");
133136
return null;
134137
}
135138
}

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,48 @@ public void testHandleTokenResponse_withClientDataHeader_attributesEmitted() {
139139
}
140140
}
141141

142+
@SneakyThrows
143+
@Test
144+
public void testHandleTokenResponse_withUrlEncodedClientDataHeader_attributesEmitted() {
145+
// eSTS URL-encodes pipe separators in the response header (e.g. "%7C" for "|").
146+
// Real-world example: x-ms-clientdata=[m%7C0x800482A5%7C%7Cmicrosoftonline.com%7Cnone]
147+
final String encodedClientDataHeader = "m%7C0x800482A5%7C%7Cmicrosoftonline.com%7Cnone";
148+
final HashMap<String, List<String>> headers = new HashMap<>();
149+
headers.put("Content-Type", Collections.singletonList("application/json; charset=utf-8"));
150+
headers.put(HttpConstants.HeaderField.X_MS_CLIENTDATA,
151+
Collections.singletonList(encodedClientDataHeader));
152+
final HttpResponse response = new HttpResponse(200, MOCK_TOKEN_SUCCESS_RESPONSE, headers);
153+
final MicrosoftStsTokenResponseHandler handler = new MicrosoftStsTokenResponseHandler();
154+
final Span mockSpan = mock(Span.class);
155+
when(mockSpan.setAttribute(Mockito.anyString(), Mockito.anyString())).thenReturn(mockSpan);
156+
try (MockedStatic<SpanExtension> mockedExtension = Mockito.mockStatic(SpanExtension.class)) {
157+
mockedExtension.when(SpanExtension::current).thenReturn(mockSpan);
158+
final TokenResult tokenResult = handler.handleTokenResponse(response);
159+
Assert.assertNotNull(tokenResult);
160+
Assert.assertTrue(tokenResult.getSuccess());
161+
Assert.assertNotNull(tokenResult.getClientDataInfo());
162+
verify(mockSpan).setAttribute(AttributeName.server_error.name(), "0x800482A5");
163+
verify(mockSpan).setAttribute(AttributeName.account_type.name(), "MSA");
164+
}
165+
}
166+
@SneakyThrows
167+
@Test
168+
public void testHandleTokenResponse_withMalformedPercentEncoding_doesNotCrash() {
169+
// Lone '%' is invalid percent-encoding; decode should fail gracefully and skip ClientDataInfo.
170+
final String malformedHeader = "e|AADSTS50058|%ZZ|us|public";
171+
final HashMap<String, List<String>> headers = new HashMap<>();
172+
headers.put("Content-Type", Collections.singletonList("application/json; charset=utf-8"));
173+
headers.put(HttpConstants.HeaderField.X_MS_CLIENTDATA,
174+
Collections.singletonList(malformedHeader));
175+
final HttpResponse response = new HttpResponse(200, MOCK_TOKEN_SUCCESS_RESPONSE, headers);
176+
final MicrosoftStsTokenResponseHandler handler = new MicrosoftStsTokenResponseHandler();
177+
final TokenResult tokenResult = handler.handleTokenResponse(response);
178+
Assert.assertNotNull(tokenResult);
179+
Assert.assertTrue(tokenResult.getSuccess());
180+
// Malformed encoding => null ClientDataInfo, but token result still valid.
181+
Assert.assertNull(tokenResult.getClientDataInfo());
182+
}
183+
142184
@SneakyThrows
143185
@Test
144186
public void testHandleTokenResponse_noClientDataHeader_doesNotCrash() {

0 commit comments

Comments
 (0)