From 87e051c526758cad5c735464dd8b785cb2b7ac1f Mon Sep 17 00:00:00 2001 From: yuxin Date: Mon, 1 Jul 2024 16:22:53 +0100 Subject: [PATCH 01/20] Comment the code to see the test --- .../identity/nativeauth/statemachine/states/AccountState.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt index 9898aaf50..1f48b39f4 100644 --- a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt +++ b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt @@ -309,12 +309,12 @@ class AccountState private constructor( correlationId = correlationId ) - val privateCorrelationId = if (correlationId == "UNSET") { UUID.randomUUID().toString() } else { correlationId } +// val privateCorrelationId = if (correlationId == "UNSET") { UUID.randomUUID().toString() } else { correlationId } val acquireTokenSilentParameters = AcquireTokenSilentParameters.Builder() .forAccount(currentAccount) .fromAuthority(currentAccount.authority) - .withCorrelationId(UUID.fromString(privateCorrelationId)) + .withCorrelationId(UUID.fromString(correlationId)) .forceRefresh(forceRefresh) .withScopes(scopes) .build() From 44e0f07168fe206d9852f4240e156ddd7dce9f70 Mon Sep 17 00:00:00 2001 From: yuxin Date: Mon, 1 Jul 2024 16:42:23 +0100 Subject: [PATCH 02/20] Update the pr for test. Revert it back later. --- azure-pipelines/pull-request-validation/pr-msal.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines/pull-request-validation/pr-msal.yml b/azure-pipelines/pull-request-validation/pr-msal.yml index 373b3c72f..15eb70400 100644 --- a/azure-pipelines/pull-request-validation/pr-msal.yml +++ b/azure-pipelines/pull-request-validation/pr-msal.yml @@ -30,7 +30,7 @@ resources: - repository: common type: github name: AzureAD/microsoft-authentication-library-common-for-android - ref: dev + ref: robert/empty-correlation-id-fixes endpoint: ANDROID_GITHUB pool: From 5d8f15c84dccdd9b631ce7912db835087e65b49a Mon Sep 17 00:00:00 2001 From: yuxin Date: Mon, 1 Jul 2024 17:07:41 +0100 Subject: [PATCH 03/20] Decomment because of the test failed --- .../identity/nativeauth/statemachine/states/AccountState.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt index 1f48b39f4..9898aaf50 100644 --- a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt +++ b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt @@ -309,12 +309,12 @@ class AccountState private constructor( correlationId = correlationId ) -// val privateCorrelationId = if (correlationId == "UNSET") { UUID.randomUUID().toString() } else { correlationId } + val privateCorrelationId = if (correlationId == "UNSET") { UUID.randomUUID().toString() } else { correlationId } val acquireTokenSilentParameters = AcquireTokenSilentParameters.Builder() .forAccount(currentAccount) .fromAuthority(currentAccount.authority) - .withCorrelationId(UUID.fromString(correlationId)) + .withCorrelationId(UUID.fromString(privateCorrelationId)) .forceRefresh(forceRefresh) .withScopes(scopes) .build() From 304ba3be8457ec8f31da48f1acc6e9d9bf272f77 Mon Sep 17 00:00:00 2001 From: yuxin Date: Wed, 3 Jul 2024 10:47:03 +0100 Subject: [PATCH 04/20] Modify CommandParametersAdapter.java to move the temporary solution in AccountState.kt --- .../client/internal/CommandParametersAdapter.java | 14 ++++++++------ .../nativeauth/statemachine/states/AccountState.kt | 3 +-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java b/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java index 9a76b272a..3c7fdcaf6 100644 --- a/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java +++ b/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java @@ -88,6 +88,8 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.UUID; /** @@ -360,8 +362,8 @@ public static SignUpStartCommandParameters createSignUpStartCommandParameters( .challengeType(configuration.getChallengeTypes()) .userAttributes(userAttributes) // Start of the flow, so there is no correlation ID to use from a previous API response. - // Set it to a default value. - .correlationId(DiagnosticContext.INSTANCE.getThreadCorrelationId()) + // If default value "UNSET", generate a new UUID as correlationId. + .correlationId(Objects.equals(DiagnosticContext.INSTANCE.getThreadCorrelationId(), "UNSET") ? UUID.randomUUID().toString() : DiagnosticContext.INSTANCE.getThreadCorrelationId()) .build(); } @@ -563,8 +565,8 @@ public static SignInStartCommandParameters createSignInStartCommandParameters( .challengeType(configuration.getChallengeTypes()) .scopes(scopes) // Start of the flow, so there is no correlation ID to use from a previous API response. - // Set it to a default value. - .correlationId(DiagnosticContext.INSTANCE.getThreadCorrelationId()) + // If default value "UNSET", generate a new UUID as correlationId. + .correlationId(Objects.equals(DiagnosticContext.INSTANCE.getThreadCorrelationId(), "UNSET") ? UUID.randomUUID().toString() : DiagnosticContext.INSTANCE.getThreadCorrelationId()) .build(); return commandParameters; @@ -792,8 +794,8 @@ public static ResetPasswordStartCommandParameters createResetPasswordStartComman .challengeType(configuration.getChallengeTypes()) .clientId(configuration.getClientId()) // Start of the flow, so there is no correlation ID to use from a previous API response. - // Set it to a default value. - .correlationId(DiagnosticContext.INSTANCE.getThreadCorrelationId()) + // If default value "UNSET", generate a new UUID as correlationId. + .correlationId(Objects.equals(DiagnosticContext.INSTANCE.getThreadCorrelationId(), "UNSET") ? UUID.randomUUID().toString() : DiagnosticContext.INSTANCE.getThreadCorrelationId()) .build(); return commandParameters; diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt index 9898aaf50..f6a63d5e1 100644 --- a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt +++ b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt @@ -309,12 +309,11 @@ class AccountState private constructor( correlationId = correlationId ) - val privateCorrelationId = if (correlationId == "UNSET") { UUID.randomUUID().toString() } else { correlationId } val acquireTokenSilentParameters = AcquireTokenSilentParameters.Builder() .forAccount(currentAccount) .fromAuthority(currentAccount.authority) - .withCorrelationId(UUID.fromString(privateCorrelationId)) + .withCorrelationId(UUID.fromString(correlationId)) .forceRefresh(forceRefresh) .withScopes(scopes) .build() From 11605bbe169eab2fd87e958fd1ce807273a8ef6f Mon Sep 17 00:00:00 2001 From: yuxin Date: Tue, 16 Jul 2024 14:29:14 +0100 Subject: [PATCH 05/20] Revert changes to trigger error log --- .../identity/client/internal/CommandParametersAdapter.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java b/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java index 3c7fdcaf6..9f125f081 100644 --- a/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java +++ b/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java @@ -363,7 +363,7 @@ public static SignUpStartCommandParameters createSignUpStartCommandParameters( .userAttributes(userAttributes) // Start of the flow, so there is no correlation ID to use from a previous API response. // If default value "UNSET", generate a new UUID as correlationId. - .correlationId(Objects.equals(DiagnosticContext.INSTANCE.getThreadCorrelationId(), "UNSET") ? UUID.randomUUID().toString() : DiagnosticContext.INSTANCE.getThreadCorrelationId()) + .correlationId(DiagnosticContext.INSTANCE.getThreadCorrelationId()) .build(); } @@ -566,7 +566,7 @@ public static SignInStartCommandParameters createSignInStartCommandParameters( .scopes(scopes) // Start of the flow, so there is no correlation ID to use from a previous API response. // If default value "UNSET", generate a new UUID as correlationId. - .correlationId(Objects.equals(DiagnosticContext.INSTANCE.getThreadCorrelationId(), "UNSET") ? UUID.randomUUID().toString() : DiagnosticContext.INSTANCE.getThreadCorrelationId()) + .correlationId(DiagnosticContext.INSTANCE.getThreadCorrelationId()) .build(); return commandParameters; @@ -795,7 +795,7 @@ public static ResetPasswordStartCommandParameters createResetPasswordStartComman .clientId(configuration.getClientId()) // Start of the flow, so there is no correlation ID to use from a previous API response. // If default value "UNSET", generate a new UUID as correlationId. - .correlationId(Objects.equals(DiagnosticContext.INSTANCE.getThreadCorrelationId(), "UNSET") ? UUID.randomUUID().toString() : DiagnosticContext.INSTANCE.getThreadCorrelationId()) + .correlationId(DiagnosticContext.INSTANCE.getThreadCorrelationId()) .build(); return commandParameters; From b07663c08e6a29834b674f4fd9f0df01a3309c0d Mon Sep 17 00:00:00 2001 From: yuxin Date: Wed, 17 Jul 2024 11:48:32 +0100 Subject: [PATCH 06/20] update submodule reference --- common | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common b/common index e72fbf36a..c7318476f 160000 --- a/common +++ b/common @@ -1 +1 @@ -Subproject commit e72fbf36a7d69993425165b85558d2fc8824074d +Subproject commit c7318476f2066bba8f1df86cb082c23e3b14af52 From 862c1268c201c608cb351bd9c6adf9d6c1db54c6 Mon Sep 17 00:00:00 2001 From: yuxin Date: Wed, 17 Jul 2024 18:02:08 +0100 Subject: [PATCH 07/20] Use empty string "" to instead "UNSET" because inside Dispatcher.initializeDiagnosticContext final String correlationId = StringUti?.isNullOrEmpty(requestCorrelationId)?UUID.randomUUID().toString():requestCorrelationId; --- .../NativeAuthPublicClientApplication.kt | 10 +++++----- .../statemachine/states/AccountState.kt | 2 +- .../network/nativeauth/GetAccessTokenTests.kt | 16 ++++++++++++++++ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplication.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplication.kt index c1bb3f361..2de8faa2b 100644 --- a/msal/src/main/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplication.kt +++ b/msal/src/main/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplication.kt @@ -262,7 +262,7 @@ class NativeAuthPublicClientApplication( GetAccountResult.AccountFound( resultValue = AccountState.createFromAccountResult( account = account, - correlationId = DiagnosticContext.INSTANCE.threadCorrelationId, + correlationId = "", config = nativeAuthConfig ) ) @@ -274,7 +274,7 @@ class NativeAuthPublicClientApplication( errorType = ErrorTypes.CLIENT_EXCEPTION, errorMessage = "MSAL client exception occurred in getCurrentAccount.", exception = e, - correlationId = DiagnosticContext.INSTANCE.threadCorrelationId + correlationId = "" ) } } @@ -341,7 +341,7 @@ class NativeAuthPublicClientApplication( return@withContext SignInError( errorType = ErrorTypes.INVALID_USERNAME, errorMessage = "Empty or blank username", - correlationId = "UNSET" + correlationId = "" ) } @@ -582,7 +582,7 @@ class NativeAuthPublicClientApplication( return@withContext SignUpError( errorType = ErrorTypes.INVALID_USERNAME, errorMessage = "Empty or blank username", - correlationId = "UNSET" + correlationId = "" ) } @@ -810,7 +810,7 @@ class NativeAuthPublicClientApplication( return@withContext ResetPasswordError( errorType = ErrorTypes.INVALID_USERNAME, errorMessage = "Empty or blank username", - correlationId = "UNSET" + correlationId = "" ) } diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt index f6a63d5e1..5ffea2e20 100644 --- a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt +++ b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt @@ -77,7 +77,7 @@ class AccountState private constructor( constructor (parcel: Parcel) : this ( account = parcel.serializable() as IAccount, - correlationId = parcel.readString() ?: "UNSET", + correlationId = parcel.readString() ?: "", config = parcel.serializable() as NativeAuthPublicClientApplicationConfiguration ) diff --git a/msal/src/test/java/com/microsoft/identity/client/e2e/tests/network/nativeauth/GetAccessTokenTests.kt b/msal/src/test/java/com/microsoft/identity/client/e2e/tests/network/nativeauth/GetAccessTokenTests.kt index c69c85d4c..bde9e9953 100644 --- a/msal/src/test/java/com/microsoft/identity/client/e2e/tests/network/nativeauth/GetAccessTokenTests.kt +++ b/msal/src/test/java/com/microsoft/identity/client/e2e/tests/network/nativeauth/GetAccessTokenTests.kt @@ -27,8 +27,10 @@ import com.microsoft.identity.client.e2e.shadows.ShadowBaseController import com.microsoft.identity.client.e2e.utils.assertState import com.microsoft.identity.internal.testutils.nativeauth.NativeAuthCredentialHelper import com.microsoft.identity.nativeauth.statemachine.errors.GetAccessTokenError +import com.microsoft.identity.nativeauth.statemachine.errors.GetAccountError import com.microsoft.identity.nativeauth.statemachine.errors.SignInError import com.microsoft.identity.nativeauth.statemachine.results.GetAccessTokenResult +import com.microsoft.identity.nativeauth.statemachine.results.GetAccountResult import com.microsoft.identity.nativeauth.statemachine.results.SignInResult import kotlinx.coroutines.test.runTest import org.junit.Assert @@ -437,4 +439,18 @@ class GetAccessTokenTests : NativeAuthPublicClientApplicationAbstractTest() { val tokenWithCustomerScope2 = authResult4.accessToken Assert.assertNotEquals(tokenWithCustomerScope, tokenWithCustomerScope2) // New token received } + @Test + fun getCurrentAccount() = runTest { + val accountResult = application.getCurrentAccount() + assertState(accountResult) + val username = NativeAuthCredentialHelper.nativeAuthSignInUsername + val password = getSafePassword() + val result = application.signIn( + username = username, + password = password.toCharArray() + ) + assertState(result) + val accountResult2 = application.getCurrentAccount() + assertState(accountResult2) + } } \ No newline at end of file From f87a44713d0af2185cf59dd4fa2a49de72e95f18 Mon Sep 17 00:00:00 2001 From: yuxin Date: Thu, 18 Jul 2024 12:21:55 +0100 Subject: [PATCH 08/20] Update the comment --- .../client/internal/CommandParametersAdapter.java | 6 +++--- .../network/nativeauth/GetAccessTokenTests.kt | 14 -------------- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java b/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java index 9f125f081..bcc680595 100644 --- a/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java +++ b/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java @@ -362,7 +362,7 @@ public static SignUpStartCommandParameters createSignUpStartCommandParameters( .challengeType(configuration.getChallengeTypes()) .userAttributes(userAttributes) // Start of the flow, so there is no correlation ID to use from a previous API response. - // If default value "UNSET", generate a new UUID as correlationId. + // If default value is empty, generate a new UUID as correlationId. .correlationId(DiagnosticContext.INSTANCE.getThreadCorrelationId()) .build(); } @@ -565,7 +565,7 @@ public static SignInStartCommandParameters createSignInStartCommandParameters( .challengeType(configuration.getChallengeTypes()) .scopes(scopes) // Start of the flow, so there is no correlation ID to use from a previous API response. - // If default value "UNSET", generate a new UUID as correlationId. + // If default value is empty, generate a new UUID as correlationId. .correlationId(DiagnosticContext.INSTANCE.getThreadCorrelationId()) .build(); @@ -794,7 +794,7 @@ public static ResetPasswordStartCommandParameters createResetPasswordStartComman .challengeType(configuration.getChallengeTypes()) .clientId(configuration.getClientId()) // Start of the flow, so there is no correlation ID to use from a previous API response. - // If default value "UNSET", generate a new UUID as correlationId. + // If default value is empty, generate a new UUID as correlationId. .correlationId(DiagnosticContext.INSTANCE.getThreadCorrelationId()) .build(); diff --git a/msal/src/test/java/com/microsoft/identity/client/e2e/tests/network/nativeauth/GetAccessTokenTests.kt b/msal/src/test/java/com/microsoft/identity/client/e2e/tests/network/nativeauth/GetAccessTokenTests.kt index bde9e9953..1d60601d5 100644 --- a/msal/src/test/java/com/microsoft/identity/client/e2e/tests/network/nativeauth/GetAccessTokenTests.kt +++ b/msal/src/test/java/com/microsoft/identity/client/e2e/tests/network/nativeauth/GetAccessTokenTests.kt @@ -439,18 +439,4 @@ class GetAccessTokenTests : NativeAuthPublicClientApplicationAbstractTest() { val tokenWithCustomerScope2 = authResult4.accessToken Assert.assertNotEquals(tokenWithCustomerScope, tokenWithCustomerScope2) // New token received } - @Test - fun getCurrentAccount() = runTest { - val accountResult = application.getCurrentAccount() - assertState(accountResult) - val username = NativeAuthCredentialHelper.nativeAuthSignInUsername - val password = getSafePassword() - val result = application.signIn( - username = username, - password = password.toCharArray() - ) - assertState(result) - val accountResult2 = application.getCurrentAccount() - assertState(accountResult2) - } } \ No newline at end of file From f799c8218ff8d6ff407d6c75b9a11d96b3564060 Mon Sep 17 00:00:00 2001 From: yuxin Date: Thu, 18 Jul 2024 12:22:26 +0100 Subject: [PATCH 09/20] update submodule reference --- common | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common b/common index c7318476f..abc127929 160000 --- a/common +++ b/common @@ -1 +1 @@ -Subproject commit c7318476f2066bba8f1df86cb082c23e3b14af52 +Subproject commit abc12792931b421234c9e1bb39b182becf71a91b From 2a024b5ddb0355326ab403bd86a29aa01ed67b0d Mon Sep 17 00:00:00 2001 From: yuxin Date: Mon, 22 Jul 2024 13:55:37 +0100 Subject: [PATCH 10/20] Revert back to DiagnosticContext.INSTANCE.threadCorrelationId because the common PR made changes in getThreadCorrelationId(), if it is null or UNSET, will generate a random UUID --- .../nativeauth/NativeAuthPublicClientApplication.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplication.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplication.kt index 2de8faa2b..b219a5b30 100644 --- a/msal/src/main/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplication.kt +++ b/msal/src/main/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplication.kt @@ -262,7 +262,7 @@ class NativeAuthPublicClientApplication( GetAccountResult.AccountFound( resultValue = AccountState.createFromAccountResult( account = account, - correlationId = "", + correlationId = DiagnosticContext.INSTANCE.threadCorrelationId, config = nativeAuthConfig ) ) @@ -274,7 +274,7 @@ class NativeAuthPublicClientApplication( errorType = ErrorTypes.CLIENT_EXCEPTION, errorMessage = "MSAL client exception occurred in getCurrentAccount.", exception = e, - correlationId = "" + correlationId = DiagnosticContext.INSTANCE.threadCorrelationId ) } } @@ -341,7 +341,7 @@ class NativeAuthPublicClientApplication( return@withContext SignInError( errorType = ErrorTypes.INVALID_USERNAME, errorMessage = "Empty or blank username", - correlationId = "" + correlationId = DiagnosticContext.INSTANCE.threadCorrelationId ) } @@ -582,7 +582,7 @@ class NativeAuthPublicClientApplication( return@withContext SignUpError( errorType = ErrorTypes.INVALID_USERNAME, errorMessage = "Empty or blank username", - correlationId = "" + correlationId = DiagnosticContext.INSTANCE.threadCorrelationId ) } @@ -810,7 +810,7 @@ class NativeAuthPublicClientApplication( return@withContext ResetPasswordError( errorType = ErrorTypes.INVALID_USERNAME, errorMessage = "Empty or blank username", - correlationId = "" + correlationId = DiagnosticContext.INSTANCE.threadCorrelationId ) } From c10ec234d353eb1ddd2d9679db782dc95cd6f941 Mon Sep 17 00:00:00 2001 From: yuxin Date: Mon, 22 Jul 2024 14:07:24 +0100 Subject: [PATCH 11/20] update submodule reference --- common | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common b/common index e1ffcbb15..4cf05aa7c 160000 --- a/common +++ b/common @@ -1 +1 @@ -Subproject commit e1ffcbb15ec6fd49e18a3c9475c2e0ac6e318687 +Subproject commit 4cf05aa7ce080fd77cf41d28de8cd960556b13ba From 0ce6946ef4dab2eca2ad687317f58d983fd2650d Mon Sep 17 00:00:00 2001 From: yuxin Date: Mon, 22 Jul 2024 21:48:49 +0100 Subject: [PATCH 12/20] Use "UNSET" --- .../nativeauth/NativeAuthPublicClientApplication.kt | 6 +++--- .../identity/nativeauth/statemachine/states/AccountState.kt | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplication.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplication.kt index b219a5b30..c1bb3f361 100644 --- a/msal/src/main/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplication.kt +++ b/msal/src/main/java/com/microsoft/identity/nativeauth/NativeAuthPublicClientApplication.kt @@ -341,7 +341,7 @@ class NativeAuthPublicClientApplication( return@withContext SignInError( errorType = ErrorTypes.INVALID_USERNAME, errorMessage = "Empty or blank username", - correlationId = DiagnosticContext.INSTANCE.threadCorrelationId + correlationId = "UNSET" ) } @@ -582,7 +582,7 @@ class NativeAuthPublicClientApplication( return@withContext SignUpError( errorType = ErrorTypes.INVALID_USERNAME, errorMessage = "Empty or blank username", - correlationId = DiagnosticContext.INSTANCE.threadCorrelationId + correlationId = "UNSET" ) } @@ -810,7 +810,7 @@ class NativeAuthPublicClientApplication( return@withContext ResetPasswordError( errorType = ErrorTypes.INVALID_USERNAME, errorMessage = "Empty or blank username", - correlationId = DiagnosticContext.INSTANCE.threadCorrelationId + correlationId = "UNSET" ) } diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt index 5ffea2e20..f6a63d5e1 100644 --- a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt +++ b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt @@ -77,7 +77,7 @@ class AccountState private constructor( constructor (parcel: Parcel) : this ( account = parcel.serializable() as IAccount, - correlationId = parcel.readString() ?: "", + correlationId = parcel.readString() ?: "UNSET", config = parcel.serializable() as NativeAuthPublicClientApplicationConfiguration ) From 1262d2cd630838588b4b2429edaa69845223b121 Mon Sep 17 00:00:00 2001 From: yuxin Date: Wed, 24 Jul 2024 14:11:54 +0100 Subject: [PATCH 13/20] Tidy up imports --- .../identity/client/internal/CommandParametersAdapter.java | 2 -- .../client/e2e/tests/network/nativeauth/GetAccessTokenTests.kt | 2 -- 2 files changed, 4 deletions(-) diff --git a/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java b/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java index bcc680595..160a44d1c 100644 --- a/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java +++ b/msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java @@ -88,8 +88,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Objects; -import java.util.UUID; /** diff --git a/msal/src/test/java/com/microsoft/identity/client/e2e/tests/network/nativeauth/GetAccessTokenTests.kt b/msal/src/test/java/com/microsoft/identity/client/e2e/tests/network/nativeauth/GetAccessTokenTests.kt index 61eea68c9..587e0caee 100644 --- a/msal/src/test/java/com/microsoft/identity/client/e2e/tests/network/nativeauth/GetAccessTokenTests.kt +++ b/msal/src/test/java/com/microsoft/identity/client/e2e/tests/network/nativeauth/GetAccessTokenTests.kt @@ -28,10 +28,8 @@ import com.microsoft.identity.client.e2e.utils.assertState import com.microsoft.identity.internal.testutils.nativeauth.ConfigType import com.microsoft.identity.internal.testutils.nativeauth.api.models.NativeAuthTestConfig import com.microsoft.identity.nativeauth.statemachine.errors.GetAccessTokenError -import com.microsoft.identity.nativeauth.statemachine.errors.GetAccountError import com.microsoft.identity.nativeauth.statemachine.errors.SignInError import com.microsoft.identity.nativeauth.statemachine.results.GetAccessTokenResult -import com.microsoft.identity.nativeauth.statemachine.results.GetAccountResult import com.microsoft.identity.nativeauth.statemachine.results.SignInResult import kotlinx.coroutines.test.runTest import org.junit.Assert From b392bda7ac93e56e8ffcac0309bf85c8f6d090e2 Mon Sep 17 00:00:00 2001 From: yuxin Date: Wed, 24 Jul 2024 14:12:35 +0100 Subject: [PATCH 14/20] Revert ref branch --- azure-pipelines/pull-request-validation/pr-msal.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/azure-pipelines/pull-request-validation/pr-msal.yml b/azure-pipelines/pull-request-validation/pr-msal.yml index 1f324e503..42748b2bc 100644 --- a/azure-pipelines/pull-request-validation/pr-msal.yml +++ b/azure-pipelines/pull-request-validation/pr-msal.yml @@ -30,7 +30,6 @@ resources: - repository: common type: github name: AzureAD/microsoft-authentication-library-common-for-android - ref: robert/empty-correlation-id-fixes endpoint: ANDROID_GITHUB pool: From 0b1cd98d09e1f3800db467b5bd19b741d366a86d Mon Sep 17 00:00:00 2001 From: yuxin Date: Wed, 24 Jul 2024 14:13:58 +0100 Subject: [PATCH 15/20] ref: dev --- azure-pipelines/pull-request-validation/pr-msal.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/azure-pipelines/pull-request-validation/pr-msal.yml b/azure-pipelines/pull-request-validation/pr-msal.yml index 42748b2bc..69eda1d9a 100644 --- a/azure-pipelines/pull-request-validation/pr-msal.yml +++ b/azure-pipelines/pull-request-validation/pr-msal.yml @@ -30,6 +30,7 @@ resources: - repository: common type: github name: AzureAD/microsoft-authentication-library-common-for-android + ref: dev endpoint: ANDROID_GITHUB pool: From 2050eb2c8bf32426e407ea1fd19f294cc9b696db Mon Sep 17 00:00:00 2001 From: yuxin Date: Wed, 24 Jul 2024 14:35:02 +0100 Subject: [PATCH 16/20] Add try catch block according to AuthenticationResult --- .../identity/nativeauth/statemachine/states/AccountState.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt index f6a63d5e1..289619bfb 100644 --- a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt +++ b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt @@ -309,6 +309,11 @@ class AccountState private constructor( correlationId = correlationId ) + try { + UUID.fromString(correlationId) + } catch (e: IllegalArgumentException) { + throw IllegalArgumentException("Correlation id is not a valid UUID.") + } val acquireTokenSilentParameters = AcquireTokenSilentParameters.Builder() .forAccount(currentAccount) From 319bd43c80a38eb732769fe1e14da4160ba2494c Mon Sep 17 00:00:00 2001 From: yuxin Date: Wed, 24 Jul 2024 14:36:08 +0100 Subject: [PATCH 17/20] Add try catch block according to AuthenticationResult --- .../identity/nativeauth/statemachine/states/AccountState.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt index 289619bfb..3a7bc640b 100644 --- a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt +++ b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt @@ -312,6 +312,7 @@ class AccountState private constructor( try { UUID.fromString(correlationId) } catch (e: IllegalArgumentException) { + Logger.error(TAG, "Correlation id is not a valid UUID", e) throw IllegalArgumentException("Correlation id is not a valid UUID.") } From c6f864ac749917f72e3a09148aed4686a3061d76 Mon Sep 17 00:00:00 2001 From: yuxin Date: Mon, 29 Jul 2024 09:37:49 +0100 Subject: [PATCH 18/20] Address comment --- .../nativeauth/statemachine/states/AccountState.kt | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt index 3a7bc640b..f930d43a0 100644 --- a/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt +++ b/msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt @@ -45,6 +45,7 @@ import com.microsoft.identity.common.java.dto.AccountRecord import com.microsoft.identity.common.java.eststelemetry.PublicApiId import com.microsoft.identity.common.java.exception.BaseException import com.microsoft.identity.common.java.exception.ServiceException +import com.microsoft.identity.common.java.logging.DiagnosticContext import com.microsoft.identity.common.java.logging.LogSession import com.microsoft.identity.common.java.logging.Logger import com.microsoft.identity.common.java.result.ILocalAuthenticationResult @@ -309,17 +310,12 @@ class AccountState private constructor( correlationId = correlationId ) - try { - UUID.fromString(correlationId) - } catch (e: IllegalArgumentException) { - Logger.error(TAG, "Correlation id is not a valid UUID", e) - throw IllegalArgumentException("Correlation id is not a valid UUID.") - } + val privateCorrelationId = if (correlationId == "UNSET") { DiagnosticContext.INSTANCE.getThreadCorrelationId() } else { correlationId } val acquireTokenSilentParameters = AcquireTokenSilentParameters.Builder() .forAccount(currentAccount) .fromAuthority(currentAccount.authority) - .withCorrelationId(UUID.fromString(correlationId)) + .withCorrelationId(UUID.fromString(privateCorrelationId)) .forceRefresh(forceRefresh) .withScopes(scopes) .build() @@ -383,6 +379,9 @@ class AccountState private constructor( } } } catch (e: Exception) { + if (e is IllegalArgumentException) { + Logger.error(TAG, "Correlation id is not a valid UUID", e) + } GetAccessTokenError( errorType = ErrorTypes.CLIENT_EXCEPTION, errorMessage = "MSAL client exception occurred in getAccessToken.", From 856c013b1b58a679157efe1b9ef2fe7326973bcc Mon Sep 17 00:00:00 2001 From: yuxin Date: Mon, 29 Jul 2024 11:05:28 +0100 Subject: [PATCH 19/20] Update submodule reference --- common | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common b/common index 46804199b..c8318496e 160000 --- a/common +++ b/common @@ -1 +1 @@ -Subproject commit 46804199b631023896624539aef04031b1c5bcab +Subproject commit c8318496e4b5cfb0e86325f72c03e93a4e2e5c44 From f056483a05511b9176edbc280c0121035ea35b8a Mon Sep 17 00:00:00 2001 From: yuxin Date: Wed, 31 Jul 2024 14:44:45 +0100 Subject: [PATCH 20/20] Add changelog entry --- changelog | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog b/changelog index 3a94ebf9b..7686aa8b6 100644 --- a/changelog +++ b/changelog @@ -2,6 +2,7 @@ MSAL Wiki : https://github.com/AzureAD/microsoft-authentication-library-for-andr vNext ---------- +- [PATCH] Add check for unset correlation ID when sending Native Auth requests (#2135) Version 5.4.2 ---------