Skip to content

Commit a709062

Browse files
melissaahnmelissaahn
committed
changelog
2 parents 3f63dbc + e172a33 commit a709062

File tree

9 files changed

+235
-6
lines changed

9 files changed

+235
-6
lines changed

.github/workflows/integration/release-fix.yml renamed to .github/workflows/release-fix.yml

File renamed without changes.

.github/workflows/integration/release-integration.yml renamed to .github/workflows/release-integration.yml

File renamed without changes.

‎changelog.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ vNext
99
- [MINOR] Fix for SDL violation in device pop scenarios, Fixes AB#3284510 (#2744)
1010
- [MINOR] Adds Authentication Constants to be used for broker latency timestamp in response (#2831)
1111
- [MINOR] ExtraTokenBodyParameters (#2825)
12+
- [MINOR] Implemented Common logic to retrieve broker latency duration from result Broker Bundle (#2835)
1213
- [MINOR] Add support for WebApps getToken API (#2803)
1314

1415
Version 23.1.1

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

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
// THE SOFTWARE.
2323
package com.microsoft.identity.common.internal.result;
2424

25+
import static com.microsoft.identity.common.java.AuthenticationConstants.Broker.BROKER_REQUEST_RECEIVED_TIMESTAMP;
26+
import static com.microsoft.identity.common.java.AuthenticationConstants.Broker.BROKER_RESPONSE_GENERATION_TIMESTAMP;
2527
import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.Broker.BROKER_ACCOUNTS;
2628
import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.Broker.BROKER_ACCOUNTS_COMPRESSED;
2729
import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.Broker.BROKER_ACTIVITY_NAME;
@@ -60,6 +62,7 @@
6062
import com.microsoft.identity.common.internal.util.GzipUtil;
6163
import com.microsoft.identity.common.internal.util.WebAppsUtil;
6264
import com.microsoft.identity.common.java.authorities.AzureActiveDirectoryAudience;
65+
import com.microsoft.identity.common.java.broker.BrokerPerformanceMetrics;
6366
import com.microsoft.identity.common.java.cache.CacheRecord;
6467
import com.microsoft.identity.common.java.cache.ICacheRecord;
6568
import com.microsoft.identity.common.java.commands.AcquirePrtSsoTokenBatchResult;
@@ -121,8 +124,10 @@ public class MsalBrokerResultAdapter implements IBrokerResultAdapter {
121124
private static final String TAG = MsalBrokerResultAdapter.class.getSimpleName();
122125
public static final Gson GSON = new Gson();
123126

127+
private static final Long INVALID_TIMESTAMP = -1L;
124128
private static final String DCF_NOT_SUPPORTED_ERROR = "deviceCodeFlowAuthRequest() not supported in BrokerMsalController";
125129
private static final String WEBAPPS_ENTRY_IS_NULL_ERROR = "WebApps entry in the bundle is null";
130+
126131
interface IBooleanCallback {
127132
boolean getResult();
128133
}
@@ -482,15 +487,53 @@ public BaseException getBaseExceptionFromBundle(@NonNull final Bundle resultBund
482487
}
483488

484489
final String exceptionType = brokerResult.getExceptionType();
490+
final BrokerPerformanceMetrics metrics = getBrokerPerformanceMetricsFromBundle(resultBundle);
491+
final BaseException baseException;
485492

486493
if (!StringUtil.isNullOrEmpty(exceptionType)) {
487-
return getBaseExceptionFromExceptionType(exceptionType, brokerResult);
494+
baseException = getBaseExceptionFromExceptionType(exceptionType, brokerResult);
488495
} else {
489496
// This code is here for legacy purposes where old versions of broker (3.1.8 or below)
490497
// wouldn't return exception type in the result.
491498
Logger.info(methodTag, "Exception type is not returned from the broker, " +
492499
"using error codes to transform to the right exception");
493-
return getBaseExceptionFromErrorCodes(brokerResult);
500+
baseException = getBaseExceptionFromErrorCodes(brokerResult);
501+
}
502+
503+
// Attach broker performance metrics if available
504+
if (metrics != null) {
505+
baseException.setBrokerPerformanceMetrics(metrics);
506+
}
507+
508+
return baseException;
509+
}
510+
511+
/**
512+
* Extracts broker performance metrics from the result bundle if available.
513+
*
514+
* @param resultBundle The result bundle returned from the broker.
515+
* @return {@link BrokerPerformanceMetrics} if available, null otherwise.
516+
*/
517+
@Nullable
518+
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
519+
public BrokerPerformanceMetrics getBrokerPerformanceMetricsFromBundle(@NonNull final Bundle resultBundle) {
520+
final long brokerRequestReceivedTimestamp = resultBundle.getLong(
521+
BROKER_REQUEST_RECEIVED_TIMESTAMP,
522+
INVALID_TIMESTAMP
523+
);
524+
final long brokerResponseGenerationTimestamp = resultBundle.getLong(
525+
BROKER_RESPONSE_GENERATION_TIMESTAMP,
526+
INVALID_TIMESTAMP
527+
);
528+
529+
if (brokerRequestReceivedTimestamp != INVALID_TIMESTAMP && brokerResponseGenerationTimestamp != INVALID_TIMESTAMP) {
530+
return new BrokerPerformanceMetrics(
531+
brokerRequestReceivedTimestamp,
532+
brokerResponseGenerationTimestamp
533+
);
534+
} else {
535+
Logger.warn(TAG, "Broker performance metrics not found in the result bundle.");
536+
return null;
494537
}
495538
}
496539

@@ -873,6 +916,7 @@ public Bundle extractInteractiveRequestBundleFromResultBundle(@NonNull final Bun
873916

874917
/**
875918
* Get authorizationResult from resultBundle for Device Code Flow
919+
*
876920
* @param resultBundle The bundle to interpret
877921
* @return authorizationResult {@link AuthorizationResult}
878922
* @throws BaseException
@@ -898,6 +942,7 @@ public AuthorizationResult getDeviceCodeFlowAuthResultFromResultBundle(@NonNull
898942

899943
/**
900944
* Get acquireTokenResult from resultBundle for Device Code Flow
945+
*
901946
* @param resultBundle The bundle to interpret
902947
* @return acquireTokenResult {@link AcquireTokenResult}
903948
* @throws BaseException
@@ -948,7 +993,11 @@ AcquireTokenResult getAcquireTokenResultFromResultBundle(@NonNull final Bundle r
948993
acquireTokenResult.setLocalAuthenticationResult(
949994
resultAdapter.authenticationResultFromBundle(resultBundle)
950995
);
951-
996+
// Set broker performance metrics if available
997+
final BrokerPerformanceMetrics metrics = resultAdapter.getBrokerPerformanceMetricsFromBundle(resultBundle);
998+
if (metrics != null) {
999+
acquireTokenResult.setBrokerPerformanceMetrics(metrics);
1000+
}
9521001
return acquireTokenResult;
9531002
}
9541003

@@ -1010,6 +1059,7 @@ public List<ICacheRecord> getAccountsFromResultBundle(@NonNull final Bundle bund
10101059
/**
10111060
* Get resource account record from the result bundle. If successful, new account
10121061
* record part of ICachedRecord is returned.
1062+
*
10131063
* @param bundle The result bundle from the broker.
10141064
* @throws BaseException
10151065
*/

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,4 +381,46 @@ class MsalBrokerResultAdapterTests {
381381
assertEquals(mockErrorCode, receivedException.errorCode)
382382
assertEquals(mockErrorMessage, receivedException.message)
383383
}
384+
385+
@Test
386+
fun testGetBrokerPerformanceMetricsFromBundle_WithValidTimestamps() {
387+
val mockRequestReceivedTimestamp = 123456789L
388+
val mockResponseGenerationTimestamp = 987654321L
389+
val resultAdapter = MsalBrokerResultAdapter()
390+
391+
val resultBundle = android.os.Bundle().apply {
392+
putLong(
393+
com.microsoft.identity.common.java.AuthenticationConstants.Broker.BROKER_REQUEST_RECEIVED_TIMESTAMP,
394+
mockRequestReceivedTimestamp
395+
)
396+
putLong(
397+
com.microsoft.identity.common.java.AuthenticationConstants.Broker.BROKER_RESPONSE_GENERATION_TIMESTAMP,
398+
mockResponseGenerationTimestamp
399+
)
400+
}
401+
402+
val metrics = resultAdapter.getBrokerPerformanceMetricsFromBundle(resultBundle)
403+
404+
assertNotNull(metrics)
405+
406+
// Validate timestamp values
407+
assertEquals(mockRequestReceivedTimestamp, metrics!!.brokerRequestReceivedTimestamp)
408+
assertEquals(mockResponseGenerationTimestamp, metrics.brokerResponseGenerationTimestamp)
409+
410+
// Validate calculated broker handling time
411+
val expectedBrokerHandlingTime = mockResponseGenerationTimestamp - mockRequestReceivedTimestamp
412+
assertEquals(expectedBrokerHandlingTime, metrics.brokerHandlingTime)
413+
414+
// Validate response latency (should be >= 0)
415+
assertNotNull(metrics.responseLatency)
416+
assertTrue(metrics.responseLatency >= 0)
417+
}
418+
419+
@Test
420+
fun testGetBrokerPerformanceMetricsFromBundle_WithoutTimestamps() {
421+
val resultAdapter = MsalBrokerResultAdapter()
422+
val resultBundle = android.os.Bundle() // Empty bundle, no timestamps
423+
val metrics = resultAdapter.getBrokerPerformanceMetricsFromBundle(resultBundle)
424+
assertNull(metrics)
425+
}
384426
}

‎common4j/src/main/com/microsoft/identity/common/java/broker/BrokerPerformanceMetrics.kt‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// All rights reserved.
3+
//
4+
// This code is licensed under the MIT License.
5+
//
6+
// Permission is hereby granted, free of charge, to any person obtaining a copy
7+
// of this software and associated documentation files(the "Software"), to deal
8+
// in the Software without restriction, including without limitation the rights
9+
// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
10+
// copies of the Software, and to permit persons to whom the Software is
11+
// furnished to do so, subject to the following conditions :
12+
//
13+
// The above copyright notice and this permission notice shall be included in
14+
// all copies or substantial portions of the Software.
15+
//
16+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
22+
// THE SOFTWARE.
23+
package com.microsoft.identity.common.java.broker
24+
25+
import com.microsoft.identity.common.java.logging.Logger
26+
27+
/**
28+
* Holds broker performance metrics derived from timestamps in authentication flows.
29+
* Calculates processing and latency durations at construction time.
30+
*/
31+
class BrokerPerformanceMetrics(
32+
/**
33+
* Timestamp when the broker received the authentication request (epoch milliseconds).
34+
* Used to calculate broker handling time.
35+
*/
36+
val brokerRequestReceivedTimestamp: Long,
37+
38+
/**
39+
* Timestamp when the broker generated the authentication response (epoch milliseconds).
40+
* Used to calculate both broker handling time and response latency.
41+
*/
42+
val brokerResponseGenerationTimestamp: Long
43+
) {
44+
/**
45+
* Time spent by broker processing the request (in milliseconds).
46+
* Calculated as: brokerResponseGenerationTimestamp - brokerRequestReceivedTimestamp
47+
*/
48+
val brokerHandlingTime: Long
49+
50+
/**
51+
* Time for the response to reach and be processed by the client (in milliseconds).
52+
* Calculated as: currentTime - brokerResponseGenerationTimestamp
53+
*/
54+
val responseLatency: Long
55+
56+
init {
57+
// Validation to avoid negative durations due to clock skew or invalid timestamps
58+
brokerHandlingTime =
59+
if (brokerResponseGenerationTimestamp < brokerRequestReceivedTimestamp) {
60+
Logger.warn(TAG, "Invalid timestamps: response before request")
61+
0
62+
} else {
63+
brokerResponseGenerationTimestamp - brokerRequestReceivedTimestamp
64+
}
65+
responseLatency = System.currentTimeMillis() - brokerResponseGenerationTimestamp
66+
}
67+
68+
override fun toString(): String {
69+
return "BrokerPerformanceMetrics(" +
70+
"brokerHandlingTime=$brokerHandlingTime, " +
71+
"responseLatency=$responseLatency, " +
72+
"brokerRequestReceivedTimestamp=$brokerRequestReceivedTimestamp, " +
73+
"brokerResponseGenerationTimestamp=$brokerResponseGenerationTimestamp)"
74+
}
75+
76+
companion object {
77+
private val TAG = BrokerPerformanceMetrics::class.java.simpleName
78+
}
79+
}

‎common4j/src/main/com/microsoft/identity/common/java/broker/IBrokerPerformanceMetricsProvider.kt‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// All rights reserved.
3+
//
4+
// This code is licensed under the MIT License.
5+
//
6+
// Permission is hereby granted, free of charge, to any person obtaining a copy
7+
// of this software and associated documentation files(the "Software"), to deal
8+
// in the Software without restriction, including without limitation the rights
9+
// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
10+
// copies of the Software, and to permit persons to whom the Software is
11+
// furnished to do so, subject to the following conditions :
12+
//
13+
// The above copyright notice and this permission notice shall be included in
14+
// all copies or substantial portions of the Software.
15+
//
16+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
22+
// THE SOFTWARE.
23+
package com.microsoft.identity.common.java.broker
24+
25+
/**
26+
* Holds broker performance metrics derived from timestamps in authentication flows.
27+
* Calculates processing and latency durations at construction time.
28+
* Mainly used to pass metrics between broker and client.
29+
*/
30+
interface IBrokerPerformanceMetricsProvider {
31+
val brokerPerformanceMetrics: BrokerPerformanceMetrics?
32+
}

‎common4j/src/main/com/microsoft/identity/common/java/exception/BaseException.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import com.microsoft.identity.common.java.telemetry.Telemetry;
2727
import com.microsoft.identity.common.java.telemetry.events.ErrorEvent;
2828
import com.microsoft.identity.common.java.util.StringUtil;
29+
import com.microsoft.identity.common.java.broker.BrokerPerformanceMetrics;
30+
import com.microsoft.identity.common.java.broker.IBrokerPerformanceMetricsProvider;
2931

3032
import java.util.ArrayList;
3133
import java.util.Arrays;
@@ -38,7 +40,7 @@
3840
import lombok.NonNull;
3941
import lombok.experimental.Accessors;
4042

41-
public class BaseException extends Exception implements IErrorInformation, ITelemetryAccessor {
43+
public class BaseException extends Exception implements IErrorInformation, ITelemetryAccessor, IBrokerPerformanceMetricsProvider {
4244

4345
// This is needed for backward compatibility with older versions of MSAL (pre 3.0.0)
4446
// When MSAL converts the result bundle it looks for this value to know about exception type
@@ -79,6 +81,8 @@ public class BaseException extends Exception implements IErrorInformation, ITele
7981

8082
private final List<Map<String, String>> mTelemetry = new ArrayList<>();
8183

84+
private BrokerPerformanceMetrics mBrokerPerformanceMetrics;
85+
8286
/**
8387
* {@link Exception#addSuppressed(Throwable)} requires API19 in Android, so we're creating our own.
8488
*/
@@ -216,6 +220,15 @@ public String getUsername() {
216220
return mUsername;
217221
}
218222

223+
public void setBrokerPerformanceMetrics(final BrokerPerformanceMetrics brokerPerformanceMetrics) {
224+
this.mBrokerPerformanceMetrics = brokerPerformanceMetrics;
225+
}
226+
227+
@Override
228+
public BrokerPerformanceMetrics getBrokerPerformanceMetrics() {
229+
return this.mBrokerPerformanceMetrics;
230+
}
231+
219232
public void setUsername(@Nullable final String username) {
220233
this.mUsername = username;
221234
}

‎common4j/src/main/com/microsoft/identity/common/java/result/AcquireTokenResult.java‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,10 @@
2525
import com.microsoft.identity.common.java.WarningType;
2626
import com.microsoft.identity.common.java.providers.oauth2.TokenResult;
2727
import com.microsoft.identity.common.java.providers.oauth2.AuthorizationResult;
28-
import com.microsoft.identity.common.java.result.ILocalAuthenticationResult;
28+
import com.microsoft.identity.common.java.broker.BrokerPerformanceMetrics;
29+
import com.microsoft.identity.common.java.broker.IBrokerPerformanceMetricsProvider;
2930

30-
public class AcquireTokenResult {
31+
public class AcquireTokenResult implements IBrokerPerformanceMetricsProvider {
3132

3233
private ILocalAuthenticationResult mLocalAuthenticationResult;
3334
private TokenResult mTokenResult;
@@ -37,6 +38,8 @@ public class AcquireTokenResult {
3738

3839
private Boolean mSucceeded = false;
3940

41+
private BrokerPerformanceMetrics mBrokerPerformanceMetrics;
42+
4043
public void setLocalAuthenticationResult(ILocalAuthenticationResult result) {
4144
this.mLocalAuthenticationResult = result;
4245
this.mSucceeded = true;
@@ -54,6 +57,15 @@ public void setTokenResult(TokenResult tokenResult) {
5457
this.mTokenResult = tokenResult;
5558
}
5659

60+
public void setBrokerPerformanceMetrics(BrokerPerformanceMetrics brokerPerformanceMetrics) {
61+
this.mBrokerPerformanceMetrics = brokerPerformanceMetrics;
62+
}
63+
64+
@Override
65+
public BrokerPerformanceMetrics getBrokerPerformanceMetrics() {
66+
return this.mBrokerPerformanceMetrics;
67+
}
68+
5769
// Suppressing rawtype warnings due to the generic type AuthorizationResult
5870
@SuppressWarnings(WarningType.rawtype_warning)
5971
public AuthorizationResult getAuthorizationResult() {

0 commit comments

Comments
 (0)