Skip to content

Commit 1586383

Browse files
fadidurahCopilotCopilot
authored
[Hotfix 24.2.1] Wire ClientDataInfo through AcquireTokenResult (#3109), Fixes AB#3604499 (#3118)
Cherry-picks #3109 (squash commit `26de108`) onto `working/release/24.2.1` for the 24.2.1 hotfix. Propagates the parsed `x-ms-clientdata` (token endpoint) and `clientdata` query parameter (authorize endpoint) data from the response handlers through `TokenResult`, `MicrosoftStsAuthorizationResult`, and ultimately onto `AcquireTokenResult` so callers can access server-side telemetry (error, sub-error, account type, cloud instance, data boundary). All propagation is gated behind the `ENABLE_SERVER_CLIENT_DATA_TELEMETRY` flight. ### Conflict resolution notes The hotfix branch does not contain the onboarding-blob feature (PR #3088 / #3111) that landed on `dev` alongside #3109. To keep this hotfix scoped to ClientDataInfo only, the following PR-side additions were dropped during cherry-pick: - `BrokerResult.ONBOARDING_BLOB` constant (kept only `CLIENT_DATA_INFO`) - `MsalBrokerResultAdapterTests.testOnboardingBlob_*` tests - `AcquireTokenResultTest.onboardingBlob_*` tests - Other `vNext` changelog entries unrelated to #3109 All `ClientDataInfo` plumbing from #3109 is preserved unchanged. [AB#3604499](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3604499) --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent c5c0c6a commit 1586383

19 files changed

Lines changed: 675 additions & 24 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.2.1-RC1
55
----------
6+
- [PATCH] Wire ClientDataInfo through AcquireTokenResult, exceptions (#3109)
67
- [MINOR] Add additional step ID and blocking error constants for full onboarding telemetry coverage (#3117)
78
- [MINOR] Add onboarding telemetry blob fields to BrokerRequest/BrokerResult and command parameters for client↔broker IPC transport (#3111)
89
- [MINOR] Add onboarding telemetry recorder, field keys, and session persistence for mobile onboarding flow (#3088)

‎common/src/main/java/com/microsoft/identity/common/internal/broker/BrokerResult.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ private static class SerializedNames {
9797
static final String SPE_RING = "spe_ring";
9898
static final String CLI_TELEM_ERRORCODE = "cli_telem_error_code";
9999
static final String CLI_TELEM_SUB_ERROR_CODE = "cli_telem_suberror_code";
100+
static final String CLIENT_DATA_INFO = "client_data_info";
100101
static final String ONBOARDING_BLOB = "onboarding_blob";
101102
}
102103

@@ -237,6 +238,13 @@ private static class SerializedNames {
237238
@SerializedName(SerializedNames.REFRESH_TOKEN_AGE)
238239
private String mRefreshTokenAge;
239240

241+
/**
242+
* Server client data info from x-ms-clientdata response header (pipe-delimited format).
243+
*/
244+
@Nullable
245+
@SerializedName(SerializedNames.CLIENT_DATA_INFO)
246+
private String mClientDataInfoRaw;
247+
240248
/**
241249
* Boolean to indicate if the request succeeded without exceptions.
242250
*/
@@ -355,6 +363,7 @@ private BrokerResult(@NonNull final Builder builder) {
355363
mCachedAt = builder.mCachedAt;
356364
mSpeRing = builder.mSpeRing;
357365
mRefreshTokenAge = builder.mRefreshTokenAge;
366+
mClientDataInfoRaw = builder.mClientDataInfoRaw;
358367
mSuccess = builder.mSuccess;
359368
mTenantProfileData = builder.mTenantProfileData;
360369
mServicedFromCache = builder.mServicedFromCache;
@@ -435,6 +444,11 @@ public String getSpeRing() {
435444
return mSpeRing;
436445
}
437446

447+
@Nullable
448+
public String getClientDataInfoRaw() {
449+
return mClientDataInfoRaw;
450+
}
451+
438452
public long getCachedAt() {
439453
return mCachedAt;
440454
}
@@ -532,6 +546,7 @@ public static class Builder {
532546
private long mCachedAt;
533547
private String mSpeRing;
534548
private String mRefreshTokenAge;
549+
private String mClientDataInfoRaw;
535550
private boolean mSuccess;
536551
private String mNegotiatedBrokerProtocolVersion;
537552
private List<ICacheRecord> mTenantProfileData;
@@ -645,6 +660,11 @@ public Builder refreshTokenAge(final String refreshTokenAge) {
645660
return this;
646661
}
647662

663+
public Builder clientDataInfoRaw(@Nullable final String clientDataInfoRaw) {
664+
this.mClientDataInfoRaw = clientDataInfoRaw;
665+
return this;
666+
}
667+
648668
public Builder success(boolean success) {
649669
this.mSuccess = success;
650670
return this;

‎common/src/main/java/com/microsoft/identity/common/internal/controllers/LocalMSALController.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,15 @@
5757
import com.microsoft.identity.common.java.result.AcquireTokenResult;
5858
import com.microsoft.identity.common.java.result.GenerateShrResult;
5959
import com.microsoft.identity.common.java.result.LocalAuthenticationResult;
60+
import com.microsoft.identity.common.java.telemetry.ClientDataInfo;
6061
import com.microsoft.identity.common.java.ui.PreferredAuthMethod;
6162
import com.microsoft.identity.common.java.util.ThreadUtils;
63+
import com.microsoft.identity.common.java.flighting.CommonFlight;
64+
import com.microsoft.identity.common.java.flighting.CommonFlightsManager;
6265
import com.microsoft.identity.common.java.providers.RawAuthorizationResult;
6366
import com.microsoft.identity.common.java.providers.microsoft.microsoftsts.MicrosoftStsAuthorizationRequest;
6467
import com.microsoft.identity.common.java.providers.microsoft.microsoftsts.MicrosoftStsAuthorizationResponse;
68+
import com.microsoft.identity.common.java.providers.microsoft.microsoftsts.MicrosoftStsAuthorizationResult;
6569
import com.microsoft.identity.common.java.providers.microsoft.microsoftsts.MicrosoftStsTokenRequest;
6670
import com.microsoft.identity.common.java.providers.oauth2.AuthorizationRequest;
6771
import com.microsoft.identity.common.java.providers.oauth2.AuthorizationResult;
@@ -205,6 +209,23 @@ public AcquireTokenResult acquireToken(
205209
false
206210
)
207211
);
212+
213+
// Set ClientDataInfo on the LocalAuthenticationResult for IPC propagation.
214+
// Prefer the token-endpoint value (later, more authoritative); fall back to the
215+
// authorize-endpoint value.
216+
final LocalAuthenticationResult localResult =
217+
(LocalAuthenticationResult) acquireTokenResult.getLocalAuthenticationResult();
218+
if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_SERVER_CLIENT_DATA_TELEMETRY)) {
219+
if (tokenResult != null && tokenResult.getClientDataInfo() != null) {
220+
localResult.setClientDataInfo(tokenResult.getClientDataInfo());
221+
} else if (result instanceof MicrosoftStsAuthorizationResult) {
222+
final ClientDataInfo authClientData =
223+
((MicrosoftStsAuthorizationResult) result).getClientDataInfo();
224+
if (authClientData != null) {
225+
localResult.setClientDataInfo(authClientData);
226+
}
227+
}
228+
}
208229
}
209230
}
210231

@@ -764,6 +785,13 @@ public AcquireTokenResult acquireDeviceCodeFlowToken(
764785
false
765786
)
766787
);
788+
789+
// Set ClientDataInfo on the LocalAuthenticationResult for IPC propagation
790+
if (tokenResult != null && CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_SERVER_CLIENT_DATA_TELEMETRY)) {
791+
final LocalAuthenticationResult localResult =
792+
(LocalAuthenticationResult) acquireTokenResult.getLocalAuthenticationResult();
793+
localResult.setClientDataInfo(tokenResult.getClientDataInfo());
794+
}
767795
} catch (Exception error) {
768796
Telemetry.emit(
769797
new ApiEndEvent()

‎common/src/main/java/com/microsoft/identity/common/internal/result/MsalBrokerResultAdapter.java‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@
9898
import com.microsoft.identity.common.java.result.GenerateShrResult;
9999
import com.microsoft.identity.common.java.result.ILocalAuthenticationResult;
100100
import com.microsoft.identity.common.java.result.LocalAuthenticationResult;
101+
import com.microsoft.identity.common.java.telemetry.ClientDataInfo;
101102
import com.microsoft.identity.common.java.ui.PreferredAuthMethod;
102103
import com.microsoft.identity.common.java.util.BrokerProtocolVersionUtil;
103104
import com.microsoft.identity.common.java.util.HeaderSerializationUtil;
@@ -284,6 +285,18 @@ public Bundle bundleFromAuthenticationResultForWebApps(@NonNull final ILocalAuth
284285
.success(true)
285286
.servicedFromCache(authenticationResult.isServicedFromCache());
286287

288+
// Serialize ClientDataInfo as raw pipe-delimited string for IPC transfer.
289+
// The raw field is populated by ClientDataInfo.fromPipeDelimited(), the only
290+
// path that populates the parsed fields, so it is safe to ship as-is.
291+
if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_SERVER_CLIENT_DATA_TELEMETRY)
292+
&& authenticationResult instanceof LocalAuthenticationResult) {
293+
final ClientDataInfo clientDataInfo =
294+
((LocalAuthenticationResult) authenticationResult).getClientDataInfo();
295+
if (clientDataInfo != null) {
296+
brokerResultBuilder.clientDataInfoRaw(clientDataInfo.getRaw());
297+
}
298+
}
299+
287300
if (shouldRemoveRefreshTokenFromResult(authenticationResult, negotiatedBrokerProtocolVersion)){
288301
brokerResultBuilder.tenantProfileRecords(
289302
removeRefreshTokenFromCacheRecords(
@@ -411,6 +424,13 @@ public Bundle bundleFromBaseException(@NonNull final BaseException exception,
411424
.speRing(exception.getSpeRing())
412425
.refreshTokenAge(exception.getRefreshTokenAge());
413426

427+
// Serialize ClientDataInfo (server telemetry from x-ms-clientdata) so it
428+
// survives the broker IPC boundary on error paths.
429+
if (exception.getClientDataInfo() != null
430+
&& CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_SERVER_CLIENT_DATA_TELEMETRY)) {
431+
builder.clientDataInfoRaw(exception.getClientDataInfo().getRaw());
432+
}
433+
414434
if (exception instanceof ServiceException) {
415435
final ServiceException serviceException = (ServiceException) exception;
416436
builder.subErrorCode(serviceException.getSubErrorCode())
@@ -483,12 +503,23 @@ public ILocalAuthenticationResult authenticationResultFromBrokerResult(@NonNull
483503
throw new ClientException(INVALID_BROKER_BUNDLE, "getTenantProfileData is null.");
484504
}
485505

486-
return new LocalAuthenticationResult(
506+
final LocalAuthenticationResult localAuthResult = new LocalAuthenticationResult(
487507
tenantProfileCacheRecords.get(0),
488508
tenantProfileCacheRecords,
489509
SdkType.MSAL,
490510
brokerResult.isServicedFromCache()
491511
);
512+
513+
// Deserialize ClientDataInfo from the broker result if available
514+
if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_SERVER_CLIENT_DATA_TELEMETRY)) {
515+
final ClientDataInfo clientDataInfo =
516+
ClientDataInfo.fromPipeDelimited(brokerResult.getClientDataInfoRaw());
517+
if (clientDataInfo != null) {
518+
localAuthResult.setClientDataInfo(clientDataInfo);
519+
}
520+
}
521+
522+
return localAuthResult;
492523
}
493524

494525
@NonNull
@@ -523,6 +554,15 @@ public BaseException getBaseExceptionFromBundle(@NonNull final Bundle resultBund
523554
baseException.setBrokerPerformanceMetrics(metrics);
524555
}
525556

557+
// Restore ClientDataInfo (server telemetry) from the broker result so callers
558+
// catching the exception can inspect server-side error context.
559+
if (!StringUtil.isNullOrEmpty(brokerResult.getClientDataInfoRaw())
560+
&& CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_SERVER_CLIENT_DATA_TELEMETRY)) {
561+
baseException.setClientDataInfo(
562+
ClientDataInfo.fromPipeDelimited(brokerResult.getClientDataInfoRaw())
563+
);
564+
}
565+
526566
// Set broker app info if available
527567
if (resultBundle.containsKey(AuthenticationConstants.Broker.BROKER_VERSION)) {
528568
baseException.setBrokerAppVersion(
@@ -1034,7 +1074,9 @@ public AcquireTokenResult getDeviceCodeFlowTokenResultFromResultBundle(@NonNull
10341074

10351075
if (resultBundle.getBoolean(AuthenticationConstants.Broker.BROKER_REQUEST_V2_SUCCESS)) {
10361076
final AcquireTokenResult acquireTokenResult = new AcquireTokenResult();
1037-
acquireTokenResult.setLocalAuthenticationResult(authenticationResultFromBundle(resultBundle));
1077+
final ILocalAuthenticationResult authResult = authenticationResultFromBundle(resultBundle);
1078+
acquireTokenResult.setLocalAuthenticationResult(authResult);
1079+
10381080
span.setStatus(StatusCode.OK);
10391081
return acquireTokenResult;
10401082
} else if (brokerResult.getErrorCode().equals(ErrorStrings.DEVICE_CODE_FLOW_AUTHORIZATION_PENDING_ERROR_CODE)) {

‎common/src/test/java/com/microsoft/identity/common/internal/request/MsalBrokerResultAdapterTests.kt‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import com.microsoft.identity.common.java.exception.ClientException
3636
import com.microsoft.identity.common.java.exception.UiRequiredException
3737
import com.microsoft.identity.common.java.request.SdkType
3838
import com.microsoft.identity.common.java.result.LocalAuthenticationResult
39+
import com.microsoft.identity.common.java.telemetry.ClientDataInfo
3940
import com.microsoft.identity.common.java.util.SchemaUtil
4041
import com.microsoft.identity.internal.testutils.MockRecords
4142
import lombok.SneakyThrows
@@ -659,6 +660,83 @@ class MsalBrokerResultAdapterTests {
659660
)
660661
}
661662

663+
// ==================== ClientDataInfo IPC round-trip tests (PR #3109) ====================
664+
665+
private val clientDataRaw = "m|AADSTS50058|login_required|us|public"
666+
667+
private fun newCacheRecord() = CacheRecord.builder()
668+
.account(MockRecords.getMockAccountRecord_AAD())
669+
.idToken(MockRecords.getMockIdTokenRecord_AAD())
670+
.accessToken(MockRecords.getMockAccessTokenRecord_AAD())
671+
.refreshToken(MockRecords.getMockRefreshTokenRecord_AAD())
672+
.build()
673+
674+
@Test
675+
fun testClientDataInfo_RoundTripsThroughBrokerResult_OnSuccess() {
676+
val cacheRecord = newCacheRecord()
677+
val cacheRecords: MutableList<ICacheRecord> = arrayListOf(cacheRecord)
678+
val authResult = LocalAuthenticationResult(cacheRecord, cacheRecords, SdkType.MSAL, false)
679+
authResult.clientDataInfo = ClientDataInfo.fromPipeDelimited(clientDataRaw)
680+
681+
val brokerResult = getInstance().buildBrokerResultFromAuthenticationResult(authResult, "16.0")
682+
assertEquals("Raw payload should be serialized into BrokerResult", clientDataRaw, brokerResult.clientDataInfoRaw)
683+
}
684+
685+
@Test
686+
fun testClientDataInfo_NullOnLocalAuthResult_ResultsInNullOnBrokerResult() {
687+
val cacheRecord = newCacheRecord()
688+
val cacheRecords: MutableList<ICacheRecord> = arrayListOf(cacheRecord)
689+
val authResult = LocalAuthenticationResult(cacheRecord, cacheRecords, SdkType.MSAL, false)
690+
// No ClientDataInfo set
691+
692+
val brokerResult = getInstance().buildBrokerResultFromAuthenticationResult(authResult, "16.0")
693+
assertNull(brokerResult.clientDataInfoRaw)
694+
}
695+
696+
@Test
697+
fun testClientDataInfo_RoundTripsThroughBaseExceptionBundle() {
698+
val exception = ClientException("invalid_grant", "token failure")
699+
exception.clientDataInfo = ClientDataInfo.fromPipeDelimited(clientDataRaw)
700+
701+
val resultAdapter = MsalBrokerResultAdapter()
702+
val resultBundle = resultAdapter.bundleFromBaseException(exception, null)
703+
val brokerResult = resultAdapter.brokerResultFromBundle(resultBundle)
704+
assertEquals(clientDataRaw, brokerResult.clientDataInfoRaw)
705+
706+
val received = resultAdapter.getBaseExceptionFromBundle(resultBundle)
707+
assertNotNull("ClientDataInfo should be reconstructed on the exception", received.clientDataInfo)
708+
assertEquals("AADSTS50058", received.clientDataInfo!!.error)
709+
assertEquals("login_required", received.clientDataInfo!!.subError)
710+
assertEquals(clientDataRaw, received.clientDataInfo!!.raw)
711+
}
712+
713+
@Test
714+
fun testClientDataInfo_NullOnException_NotInBundle() {
715+
val exception = ClientException("invalid_grant", "token failure")
716+
// No ClientDataInfo set
717+
718+
val resultAdapter = MsalBrokerResultAdapter()
719+
val resultBundle = resultAdapter.bundleFromBaseException(exception, null)
720+
val received = resultAdapter.getBaseExceptionFromBundle(resultBundle)
721+
assertNull(received.clientDataInfo)
722+
}
723+
724+
@Test
725+
fun testClientDataInfo_RoundTripsThroughGetAcquireTokenResultFromResultBundle() {
726+
val cacheRecord = newCacheRecord()
727+
val cacheRecords: MutableList<ICacheRecord> = arrayListOf(cacheRecord)
728+
val authResult = LocalAuthenticationResult(cacheRecord, cacheRecords, SdkType.MSAL, false)
729+
authResult.clientDataInfo = ClientDataInfo.fromPipeDelimited(clientDataRaw)
730+
731+
val resultAdapter = MsalBrokerResultAdapter()
732+
val resultBundle = resultAdapter.bundleFromAuthenticationResult(authResult, "16.0")
733+
734+
val acquireTokenResult = resultAdapter.getAcquireTokenResultFromResultBundle(resultBundle)
735+
assertNotNull("ClientDataInfo should be present on AcquireTokenResult", acquireTokenResult.clientDataInfo)
736+
assertEquals("AADSTS50058", acquireTokenResult.clientDataInfo!!.error)
737+
assertEquals(clientDataRaw, acquireTokenResult.clientDataInfo!!.raw)
738+
}
739+
662740
@Test
663741
fun testOnboardingBlob_RoundTripsThroughBundle() {
664742
val blobJson = """{"schema_version":"1.0.0","session_correlation_id":"abc-123","onboarding_mode":"brokered","blocking_errors":["BROKER_INSTALLATION_TRIGGERED"]}"""

‎common4j/src/main/com/microsoft/identity/common/java/controllers/BaseController.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@
5858
import com.microsoft.identity.common.java.exception.ClientException;
5959
import com.microsoft.identity.common.java.exception.ErrorStrings;
6060
import com.microsoft.identity.common.java.exception.ServiceException;
61+
import com.microsoft.identity.common.java.flighting.CommonFlight;
62+
import com.microsoft.identity.common.java.flighting.CommonFlightsManager;
6163
import com.microsoft.identity.common.java.foci.FociQueryUtilities;
6264
import com.microsoft.identity.common.java.logging.DiagnosticContext;
6365
import com.microsoft.identity.common.java.logging.Logger;
@@ -251,6 +253,11 @@ public AcquireTokenResult acquireTokenWithPassword(@NonNull final RopcTokenComma
251253
Telemetry.emit(new CacheEndEvent());
252254
}
253255

256+
// Set server client data info on the authentication result for IPC propagation
257+
if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_SERVER_CLIENT_DATA_TELEMETRY)) {
258+
authenticationResult.setClientDataInfo(tokenResult.getClientDataInfo());
259+
}
260+
254261
// Set the AuthenticationResult on the final result object
255262
acquireTokenResult.setLocalAuthenticationResult(authenticationResult);
256263
}
@@ -528,6 +535,11 @@ protected void renewAccessToken(@NonNull final SilentTokenCommandParameters para
528535
Telemetry.emit(new CacheEndEvent());
529536
}
530537

538+
// Set server client data info on the authentication result for IPC propagation
539+
if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_SERVER_CLIENT_DATA_TELEMETRY)) {
540+
authenticationResult.setClientDataInfo(tokenResult.getClientDataInfo());
541+
}
542+
531543
// Set the AuthenticationResult on the final result object
532544
acquireTokenSilentResult.setLocalAuthenticationResult(authenticationResult);
533545
} else {

0 commit comments

Comments
 (0)