msgraph-sdk-dotnet icon indicating copy to clipboard operation
msgraph-sdk-dotnet copied to clipboard

GetHttpRequestMessage should return a message with the AuthorizationHeader prepopulated

Open baywet opened this issue 6 years ago • 11 comments

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

await client.ExecutePayloadAsync(myHttpRequestMessage);

Actual behavior

Steps to reproduce the behavior

using(var message = client.Me.Events["id"].Request().GetHttpRequestMessage())
{
   message.Headers.Authorization?.Scheme; // null
}

AB#7344

baywet avatar Apr 27 '18 16:04 baywet

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. 791326

MIchaelMainer avatar Apr 27 '18 18:04 MIchaelMainer

It really depends on the cases:

  • The resource is "known" by the SDK, the URL path is the same, it's only a matter of some unsupported properties or so, in that case the resource.Request().GetHttpRequestMessage() is really useful as it would pre-bake most of the things. But I think this would be a rare percentage of cases (like the issue I'm having in #264 )
  • The resource is unknown, it's a different path (let's say event/cancel is not supported by the SDK), Then I have to generate the url myself, the payload... I'm most likely to forge the HttpRequestMessage myself. This is most likely to happen that the other one and should be resolved over time as the SDK implements more capabilities (ie: I update the package, get rid of my custom code and call the SDK instead in the future)

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. This method should add the bearer to the message as well as implement error handling, transient faults...etc saving me a lot of trouble and time, and having a consistent implementation to go from "I have the payload" to "the Graph got it and replied".

I hope that clarifies the ask.

baywet avatar Apr 27 '18 19:04 baywet

@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 avatar Jun 18 '18 22:06 darrelmiller

@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.

baywet avatar Jun 19 '18 12:06 baywet

GetHttpRequestMessage(bool authenticate) so that customers can make use of the DelegateAuthorizationHandler.

MIchaelMainer avatar Apr 29 '21 16:04 MIchaelMainer

Hi @baywet . Do you still believe this is a valid use case?

maisarissi avatar Aug 18 '22 14:08 maisarissi

ah... my old issues coming back to haunt me 😂 I do think we should introduce an authentication middleware in kiota http packages but not add it to the default middlewares. This way people who want to send requests with the native client would not have to handle authentication themselves.

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.

baywet avatar Aug 18 '22 15:08 baywet

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.

baywet avatar Aug 18 '22 15:08 baywet

@darrelmiller @RabebOthmani any thoughts on adding an authentication middleware in Kiota?

maisarissi avatar Aug 19 '22 19:08 maisarissi

I believe this can be closed with the introduction of the ConvertToNativeRequestAsync in the request adapter.

Taking a look at the implementation, at the link below, https://github.com/microsoft/kiota-http-dotnet/blob/2c6d3a411e4925f199f4da8dc10fd5861d69dec0/src/HttpClientRequestAdapter.cs#L458

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);

andrueastman avatar Apr 04 '23 09:04 andrueastman

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?

baywet avatar Apr 04 '23 10:04 baywet

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 HttpRequestMessage instances rather than RequestInformation instances without using ConvertToNativeRequestAsync.

Closing this to be tracked via https://github.com/microsoft/kiota-http-dotnet/issues/183

andrueastman avatar Apr 12 '24 09:04 andrueastman