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

Add check for unset correlation ID when configuring request headers #2435

Merged
merged 27 commits into from
Jul 29, 2024
Merged

Add check for unset correlation ID when configuring request headers #2435

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
a28a3f7
Add check for empty correlation ID when setting request headers, upda…
rmccahill Jun 20, 2024
046593a
Update changelog
rmccahill Jun 20, 2024
9fe242c
Update empty header test naming
rmccahill Jun 24, 2024
428f2ab
Merge branch 'dev' into robert/empty-correlation-id-fixes
rmccahill Jun 28, 2024
f221e24
Update changelog
rmccahill Jun 28, 2024
73f34ff
Merge branch 'dev' into robert/empty-correlation-id-fixes
Yuki-YuXin Jul 1, 2024
2e760ac
Add argument to pass MSAL consumer test
Yuki-YuXin Jul 1, 2024
c1d5a3f
Merge branch 'dev' into robert/empty-correlation-id-fixes
Yuki-YuXin Jul 3, 2024
b653ea6
Merge branch 'dev' into robert/empty-correlation-id-fixes
Yuki-YuXin Jul 16, 2024
e631037
Revert changes to trigger error log
Yuki-YuXin Jul 16, 2024
fed453c
getThreadCorrelationId
Yuki-YuXin Jul 17, 2024
c731847
getThreadCorrelationId UNSET
Yuki-YuXin Jul 17, 2024
3391582
Inside nativeAuthResponseHandler.getSignInInitiateResultFromHttpRespo…
Yuki-YuXin Jul 17, 2024
5976371
Merge branch 'dev' into robert/empty-correlation-id-fixes
Yuki-YuXin Jul 18, 2024
2685650
Modify the test
Yuki-YuXin Jul 18, 2024
d0a2618
Modify the remaining test
Yuki-YuXin Jul 18, 2024
abc1279
Modify the remaining test
Yuki-YuXin Jul 18, 2024
4cf05aa
Remove tests related with the request header nativeAuthRequestProvide…
Yuki-YuXin Jul 18, 2024
a9f32fe
Put dev version header into this branch
Yuki-YuXin Jul 22, 2024
77d3dd3
Only keep the minor changes
Yuki-YuXin Jul 22, 2024
acc8421
Merge branch 'dev' into robert/empty-correlation-id-fixes
Yuki-YuXin Jul 22, 2024
b4a923f
Revert "Remove tests related with the request header nativeAuthReques…
Yuki-YuXin Jul 24, 2024
c30e22e
"UNSET" -> UUID.randomUUID().toString() in getThreadCorrelationId()
Yuki-YuXin Jul 24, 2024
8181c6f
Revert "Modify the remaining test"
Yuki-YuXin Jul 24, 2024
3c74bb2
Revert "Modify the test"
Yuki-YuXin Jul 24, 2024
f0c1be5
Revert "Modify the remaining test"
Yuki-YuXin Jul 24, 2024
af209cc
Merge branch 'dev' into robert/empty-correlation-id-fixes
Yuki-YuXin Jul 29, 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
1 change: 1 addition & 0 deletions changelog.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ V.Next
- [MINOR] Updating YubiKit and CredMan versions (#2417)
- [PATCH] Adding check for OS version for passkeys (#2419)
- [MINOR] Platform Specific Extra Query Parameters (#2426)
- [PATCH] Add check for unset correlation ID when sending Native Auth requests (#2435)

Version 17.4.0
---------
Expand Down
7 changes: 6 additions & 1 deletion ...main/com/microsoft/identity/common/java/nativeauth/providers/NativeAuthRequestProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import com.microsoft.identity.common.java.nativeauth.providers.requests.signup.S
import com.microsoft.identity.common.java.nativeauth.providers.requests.signup.SignUpContinueRequest
import com.microsoft.identity.common.java.nativeauth.providers.requests.signup.SignUpStartRequest
import com.microsoft.identity.common.java.platform.Device
import com.microsoft.identity.common.java.util.StringUtil
import java.util.TreeMap

/**
Expand Down Expand Up @@ -310,7 +311,11 @@ class NativeAuthRequestProvider(private val config: NativeAuthOAuth2Configuratio
//region helpers
private fun getRequestHeaders(correlationId: String): Map<String, String?> {
val headers: MutableMap<String, String?> = TreeMap()
headers[AuthenticationConstants.AAD.CLIENT_REQUEST_ID] = correlationId

if (correlationId != "UNSET") {
Copy link
Contributor

Choose a reason for hiding this comment

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

The string 'UNSET' should be used from a constant file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having a look at the rest of the codebase and "UNSET" seems to be used in place instead of being used as a constant value. I'd rather stick with the convention here, given that it's a straightforward name and something that doesn't have a specific meaning or is used outside of the context of the SDK.

headers[AuthenticationConstants.AAD.CLIENT_REQUEST_ID] = correlationId
}

Yuki-YuXin marked this conversation as resolved.
Show resolved Hide resolved
headers[AuthenticationConstants.SdkPlatformFields.PRODUCT] = DiagnosticContext.INSTANCE.requestContext[AuthenticationConstants.SdkPlatformFields.PRODUCT]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this code change is wrong. And the one on the line below.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have no idea why the changes here. If that's wrong, I will revert it back.

headers[AuthenticationConstants.SdkPlatformFields.VERSION] = Device.getProductVersion()
headers.putAll(Device.getPlatformIdParameters())
Expand Down
Loading
Loading