-
Couldn't load subscription status.
- Fork 378
Updating MSAL to send client info = 2 on client credential flow #5529
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.
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
base: main
Are you sure you want to change the base?
Conversation
src/client/Microsoft.Identity.Client/Internal/Requests/ClientCredentialRequest.cs
Show resolved
Hide resolved
added test
src/client/Microsoft.Identity.Client/Cache/Items/MsalAccessTokenCacheItem.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Cache/Items/MsalAccessTokenCacheItem.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Cache/Items/MsalAccessTokenCacheItem.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/TokenCache.ITokenCacheInternal.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| { | ||
| if (CreateClientInfoForS2S) | ||
| { | ||
| return Base64UrlHelpers.Encode("{\"xms_acb\":[\"value1\",\"value2\"]}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is xms_acb? Can you add some comment to explain this?
| var idtItem = tuple.Item2; | ||
| Account account = tuple.Item3; | ||
|
|
||
| atItem.AddAdditionalCacheParameters(clientInfoFromServer.AdditionalResponseParameters); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What values are we supposed to get from the server for client info?
| using Microsoft.Identity.Client.TelemetryCore.Internal.Events; | ||
| using Microsoft.Identity.Client.Utils; | ||
| using System.Security.Cryptography.X509Certificates; | ||
| using System.Linq; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Unused. Remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I think there can be some comments added to explain the response from ests in tests and code. Thanks!
Fixes #
#5537
Added:
[OAuth2Parameter.ClientInfo]= "2" to the body parameters in theClientCredentialRequestThis ensures that client credential requests now include the client info parameter with a value of "2".
This tells ESTS to include xms_acb in the token response
Changes proposed in this request
Testing
Performance impact
Documentation