Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the unnecessary code line because of fix in common #2135

Open
wants to merge 27 commits into
base: dev
Choose a base branch
from
+9 −4
Open

Remove the unnecessary code line because of fix in common #2135

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
87e051c
Comment the code to see the test
Yuki-YuXin Jul 1, 2024
44e0f07
Update the pr for test. Revert it back later.
Yuki-YuXin Jul 1, 2024
5d8f15c
Decomment because of the test failed
Yuki-YuXin Jul 1, 2024
304ba3b
Modify CommandParametersAdapter.java to move the temporary solution i…
Yuki-YuXin Jul 3, 2024
2780cc6
Merge branch 'dev' into yuki/company-with-robert-PR-test
SammyO Jul 15, 2024
e6614e3
Merge branch 'dev' into yuki/company-with-robert-PR-test
Yuki-YuXin Jul 16, 2024
a4ae188
Merge remote-tracking branch 'origin/yuki/company-with-robert-PR-test…
Yuki-YuXin Jul 16, 2024
11605bb
Revert changes to trigger error log
Yuki-YuXin Jul 16, 2024
b07663c
update submodule reference
Yuki-YuXin Jul 17, 2024
862c126
Use empty string "" to instead "UNSET" because inside Dispatcher.init…
Yuki-YuXin Jul 17, 2024
f87a447
Update the comment
Yuki-YuXin Jul 18, 2024
f799c82
update submodule reference
Yuki-YuXin Jul 18, 2024
73c241c
Merge branch 'dev' into yuki/company-with-robert-PR-test
Yuki-YuXin Jul 19, 2024
2a024b5
Revert back to DiagnosticContext.INSTANCE.threadCorrelationId because…
Yuki-YuXin Jul 22, 2024
c10ec23
update submodule reference
Yuki-YuXin Jul 22, 2024
0ce6946
Use "UNSET"
Yuki-YuXin Jul 22, 2024
74d3658
Merge branch 'dev' into yuki/company-with-robert-PR-test
Yuki-YuXin Jul 22, 2024
1262d2c
Tidy up imports
Yuki-YuXin Jul 24, 2024
b392bda
Revert ref branch
Yuki-YuXin Jul 24, 2024
0b1cd98
ref: dev
Yuki-YuXin Jul 24, 2024
2050eb2
Add try catch block according to AuthenticationResult
Yuki-YuXin Jul 24, 2024
319bd43
Add try catch block according to AuthenticationResult
Yuki-YuXin Jul 24, 2024
c6f864a
Address comment
Yuki-YuXin Jul 29, 2024
856c013
Update submodule reference
Yuki-YuXin Jul 29, 2024
d2f0162
Merge branch 'dev' into yuki/company-with-robert-PR-test
Yuki-YuXin Jul 29, 2024
43be7bf
Merge branch 'dev' into yuki/company-with-robert-PR-test
Yuki-YuXin Jul 31, 2024
f056483
Add changelog entry
Yuki-YuXin Jul 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion azure-pipelines/pull-request-validation/pr-msal.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert it back before the merge.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have this here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot change the targe branch in the DevOps portal thus change the command here.

endpoint: ANDROID_GITHUB

pool:
Expand Down
14 changes: 8 additions & 6 deletions msal/src/main/java/com/microsoft/identity/client/internal/CommandParametersAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.UUID;


/**
Expand Down Expand Up @@ -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())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic doesn't belong here. it's the responsibility of DiagnosticContext to generate an ID, and do additional null checks to ensure an actual ID is returned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the logic inside DiagnosticContext getThreadCorrelationId()

.build();
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions msal/src/main/java/com/microsoft/identity/nativeauth/statemachine/states/AccountState.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading