-
Notifications
You must be signed in to change notification settings - Fork 369
Adding WithExtraBodyParameters api #5389
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.Extensions.Msal/Shared/StorageCreationPropertiesBuilder.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ApiConfig/AcquireTokenForClientParameterBuilder.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 so far, some minor comments and TODOs are left
/// <param name="builder"></param> | ||
/// <param name="extrabodyparams">List of additional body parameters</param> | ||
/// <returns></returns> | ||
public static AcquireTokenForClientParameterBuilder WithExtraBodyParameters( |
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.
Thx, can you also pls log a feature request to the same for WithExtraQueryParameters? i.e. associate tokens with those params?
@@ -27,10 +28,10 @@ internal class AcquireTokenCommonParameters | |||
public IAuthenticationOperation AuthenticationOperation { get; set; } = new BearerAuthenticationOperation(); | |||
public IDictionary<string, string> ExtraHttpHeaders { get; set; } | |||
public PoPAuthenticationConfiguration PopAuthenticationConfiguration { get; set; } | |||
public Func<OnBeforeTokenRequestData, Task> OnBeforeTokenRequestHandler { get; internal set; } | |||
public IList<Func<OnBeforeTokenRequestData, Task>> OnBeforeTokenRequestHandler { get; internal set; } |
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.
Why is this changed to a list object? Are we expecting to have this set multiple times?
} | ||
} | ||
|
||
private Task<string> GetPdpAuthorization() |
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 does Pdp mean?
Fixes #
Changes proposed in this request
This PR adds a new extensibility API
WithExtraBodyParameters(Dictionary<string, Func<CancellationToken, Task<string>>> extrabodyparams)
The body params are in the form of
Dictionary<string, Func<CancellationToken, Task<string>>>
because this will give developers an opportunity to provide a delegate to achieve the dictionary value because it is possible that it may change depending on when it is acquired.These parameters are added to the body parameters of the outgoing request and they are added to the cache key. This way, the tokens acquired with these parameters will only be retrieved when the same parameters are provided.
Testing
Unit/Manual
Documentation