-
Notifications
You must be signed in to change notification settings - Fork 252
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
GetHttpRequestMessage should return a message with the AuthorizationHeader prepopulated #263
Comments
This is worth investigating. It would be convenient to automatically get the token in both of the scenarios that you described. Can you (or anyone else that is reading this), let me know based on your experience, which approach is more fluent to you? I do wonder when the library doesn't support a scenario, what percentage of folks use the request builders to get a URL or whether they just provide the URL. |
It really depends on the cases:
In both cases the URL will at some point be carried by the message, so I don't think we need a ExecutePayloadAsync everywhere (which should be considered a service method, for technical, temporary reasons), except for deserialization reasons maybe. I hope that clarifies the ask. |
@baywet Having a middleware pipeline apply an auth header is probably better than returning the auth header in the request object. If there is ever a need to do request signing in the future then you would want to do that after the client code has manipulated the request. |
@darrelmiller thanks for the follow up. Agreed, as a developer using the SDK, having a client.ExecutePayloadAsync(myMessage) or something similar would be much simpler to me. This is why I was mentioning it as the first option. |
GetHttpRequestMessage(bool authenticate) so that customers can make use of the DelegateAuthorizationHandler. |
Hi @baywet . |
ah... my old issues coming back to haunt me 😂 The alternative already possible today would be to use the end async method from the request adapter. However the API is not super friendly for humans. |
The authentication middleware would make a ton of sense for our pizza model and people only using an augmented native client without models or fluent API. We'd need to reimplement some of the CAE work we've done however. |
@darrelmiller @RabebOthmani any thoughts on adding an authentication middleware in Kiota? |
I believe this can be closed with the introduction of the Taking a look at the implementation, at the link below, Calling the function should give back a httpRequestMessage that has the Authorization header populated to cater for the use case. var requestMessage = await graphClient.RequestAdapter.ConvertToNativeRequestAsync<HttpRequestMessage>(requestInformation); |
What if they want to make an arbitrary request (not described by the generated API) with the native client augmented by our middleware? They'll be missing auth at this point, won't they? |
That will be true if they create Closing this to be tracked via microsoft/kiota-dotnet#270 |
Expected behavior
When crafting requests manually because the SDK has some limitations, I'd expect the provided message to come with a Authorization header so I can run my request right away without having to retrieve a token (cached or not).
Or, if you're worried about exposing the token, I'd expect a message executor to accept my message to handle not only authorization, but also error mapping, transient errors... Something like
Actual behavior
Steps to reproduce the behavior
AB#7344
The text was updated successfully, but these errors were encountered: