Skip to content

Commit bd7c53a

Browse files
authored
Fix Token Endpoint Server Telemetry Parsing, Fixes AB#3604499 (#3128)
During the acquire token flow, we make a call to /token endpoint to get an access token using PRT. As part of this call, there is a telemetry header returned by ests for telemetry purposes, which we want to send back up to oneauth. it seems that there is an issue in the parsing that i missed, particularly targeting msa accounts. This change fixes that, I validated it manually and Kevin is validating now. [AB#3604499](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3604499)
1 parent 8638b3b commit bd7c53a

5 files changed

Lines changed: 82 additions & 5 deletions

File tree

changelog.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
vNext
22
----------
3+
- [PATCH] Fix Token Endpoint Server Telemetry Parsing (#3128)
34
- [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)
45
- [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)
56
- [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
@@ -43,6 +43,8 @@
4343
import com.microsoft.identity.common.java.util.HeaderSerializationUtil;
4444
import com.microsoft.identity.common.java.util.ObjectMapper;
4545
import com.microsoft.identity.common.java.util.ResultUtil;
46+
import com.microsoft.identity.common.java.util.StringUtil;
47+
import com.microsoft.identity.common.java.logging.Logger;
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/cache/NameValueStorageFileManagerSimpleCacheImplConcurrencyTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,8 @@ public void tearDown() throws Exception {
202202
mocks.close();
203203
}
204204
updateFlightForTest(CommonFlight.USE_LOCKS_IN_NAME_VALUE_STORAGE, false);
205+
206+
CommonFlightsManager.INSTANCE.resetFlightsManager();
205207
}
206208

207209
/**

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,59 @@ 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+
149+
final HashMap<String, List<String>> headers = new HashMap<>();
150+
headers.put("Content-Type", Collections.singletonList("application/json; charset=utf-8"));
151+
headers.put(HttpConstants.HeaderField.X_MS_CLIENTDATA,
152+
Collections.singletonList(encodedClientDataHeader));
153+
154+
final HttpResponse response = new HttpResponse(200, MOCK_TOKEN_SUCCESS_RESPONSE, headers);
155+
final MicrosoftStsTokenResponseHandler handler = new MicrosoftStsTokenResponseHandler();
156+
157+
final Span mockSpan = mock(Span.class);
158+
when(mockSpan.setAttribute(Mockito.anyString(), Mockito.anyString())).thenReturn(mockSpan);
159+
160+
try (MockedStatic<SpanExtension> mockedExtension = Mockito.mockStatic(SpanExtension.class)) {
161+
mockedExtension.when(SpanExtension::current).thenReturn(mockSpan);
162+
163+
final TokenResult tokenResult = handler.handleTokenResponse(response);
164+
165+
Assert.assertNotNull(tokenResult);
166+
Assert.assertTrue(tokenResult.getSuccess());
167+
Assert.assertNotNull(tokenResult.getClientDataInfo());
168+
verify(mockSpan).setAttribute(AttributeName.server_error.name(), "0x800482A5");
169+
verify(mockSpan).setAttribute(AttributeName.account_type.name(), "MSA");
170+
}
171+
}
172+
173+
@SneakyThrows
174+
@Test
175+
public void testHandleTokenResponse_withMalformedPercentEncoding_doesNotCrash() {
176+
// Lone '%' is invalid percent-encoding; decode should fail gracefully and skip ClientDataInfo.
177+
final String malformedHeader = "e|AADSTS50058|%ZZ|us|public";
178+
179+
final HashMap<String, List<String>> headers = new HashMap<>();
180+
headers.put("Content-Type", Collections.singletonList("application/json; charset=utf-8"));
181+
headers.put(HttpConstants.HeaderField.X_MS_CLIENTDATA,
182+
Collections.singletonList(malformedHeader));
183+
184+
final HttpResponse response = new HttpResponse(200, MOCK_TOKEN_SUCCESS_RESPONSE, headers);
185+
final MicrosoftStsTokenResponseHandler handler = new MicrosoftStsTokenResponseHandler();
186+
187+
final TokenResult tokenResult = handler.handleTokenResponse(response);
188+
189+
Assert.assertNotNull(tokenResult);
190+
Assert.assertTrue(tokenResult.getSuccess());
191+
// Malformed encoding => null ClientDataInfo, but token result still valid.
192+
Assert.assertNull(tokenResult.getClientDataInfo());
193+
}
194+
142195
@SneakyThrows
143196
@Test
144197
public void testHandleTokenResponse_noClientDataHeader_doesNotCrash() {

0 commit comments

Comments
 (0)