dotnet / wcf

This repo contains the client-oriented WCF libraries that enable applications built on .NET Core to communicate with WCF services.
MIT License
1.71k stars 558 forks source link

Generated client lifetime scope #4998

Open mhallockPcty opened 1 year ago

mhallockPcty commented 1 year ago

I have an SSRS client I'm trying to put together for a legacy project upgrade from .NET Framework, and I'm having a little trouble finding the right documentation for my needs, hoping someone can help guide me to the right place. Im using a generated client regenerated through VS2022's add service reference screens.

Issue 1: Our SSRS implementation uses a FormsAuth scheme in front of it, meaning from a WCF standpoint here, it doesn't use any of the regular auth mechanisms (BasicHttpSecurityMode.None I believe), but instead through SSRS you can call a LogonAsync method that will return an AUTH COOKIE that needs to then be attached to each subsequent request along with managing sliding expirations, etc. by replacing the cookie when a new one comes back after a call, etc. Every "session" has a different user that matters to the report security context, so I need each "client" to have a separate auth context here.

I have a proof of concept working for this using an IClientMessageInspector added via an IEndPoint behavior and it works for a single client. I ALSO tried something on a per call basis via an OperationContextScope or something like that to put the headers on / strip them off on responses, but ran into some async issues that looked like SynchronizationContext type continuation issues that I couldn't figure out (and the IClientMessageInspector seemed more elegant anyway).

Buuuuut

Issue 2: This client is going to used as part of a report queue running around 500k report queue items a day across multiple parallel threads, each of which with this model, would be NEWING UP A NEW CLIENT when each queue item is processed. This would keep the auth context of my MessageInspector isolated per usage, but my understanding here is I'm setting myself up for socket exhaustion since each of those clients would instantiate its own HttpClient under the hood? I thought about making the client singleton, but that seems like my IMessageInspector approach to the auth scheme would then all share the same Cookie storage = bad....

I read about injecting in my behavior an IHttpMessageHandlerFactory to allow reuse of the underlying HttpMessageHandler to avoid the HttpClient socket exhaustion, but also read I'd still be recreating the ChannelFactory every time which might cause other issues? I'm a little stuck here on what my lifetime management should look like...

Semi-Issue 3: It is not uncommon for SSRS to timeout a call on us. My understanding is that if this happens, that Channel is basically dead. until you call a manual channel.Abort(). Im not sure this applies to me if I new up a new client every time as I think I get a whole new ChannelFactory/Channels then so between runs it'd be fine, but if I have to do some lifetime management here, this might come into play?

Im more concerned about the lifetime management issues as I'm going to be doing some stuff at scale here so I need to get that right, if I have to implement the auth part in some other way, Im open to suggestions.

mhallockPcty commented 1 year ago

I now understand my issue better to ask a better question.

SSRS is an ASMX service that uses COOKIE based authentication. This makes any client that uses it semi-stateful (they need to at least deal with the cookie lifetime between calls).

The client will be used in multiple ENVs, all of which will be logging in PER USER and need to establish and maintain that cookie in isolation of other thread callers. And its gonna get called a lot.

I can do that by just turning on AllowCookies on the binding and using a NEW client per thread... but I'm gonna hit the HttpClient socket exhaustion issues with that. The DEFAULT answer to that seems to be "use a static/singleton client", but that means my clients will share HttpClients under the hood, and this AllowCookies would cause issues since different callers would overwrite the shared Cookie storage therein all the time. Injecting an IHttpMessageHandlerFactory, I think, runs afoul of that too, since MS warns kinda loudly that doing so will break CookieContainer usage...

I could use a static client if I could inject PER CALL something that would do the cookie handling... but I believe IClientMessageInspector on a static client would ALSO be static and thus how do I get a PER CALL set of information into there to decorate cookies onto the call / extract cookie values for storage on response?

And herein is the crux of my issue, I need to handle the HttpClient/socket exhaustion issue, but still maintain a STATEFUL client for cookies. How do I do that best?

mhallockPcty commented 1 year ago

I have something I think works, but it is UGLY. Thoughts?

I register a STATIC client with AllowCookies=false, and with an IClientMessageInspector that handles taking off and adding on cookies. The message inspector BeforeSendRequest looks for an pulls a message property off the request, which is BASICALLY a POINTER to an externally managed object that will hold cookie information. It also PASSES that reference to the AfterReceiveReply correlationState so that it has direct reference to that external dependency as well (as I didn't think that property would still be on the reply.Properties there to pull again with how Im about to pass it in...)

  public class CookieClientMessageInspector : IClientMessageInspector
  {
    /// <inheritdoc />
    public void AfterReceiveReply(ref Message reply, object correlationState)
    {
      if (reply.Properties.TryGetValue(HttpResponseMessageProperty.Name, out object httpResponseMessageObject))
      {
        var httpResponseMessage = httpResponseMessageObject as HttpResponseMessageProperty;

        // If the response contains cookies, store it
        var responseCookieHeader = httpResponseMessage.Headers["Set-Cookie"];
        if (responseCookieHeader != null && !string.IsNullOrWhiteSpace(responseCookieHeader))
        {
          if (correlationState is not null)
          {
            var cookieStorage = correlationState as CookieStorageProperty;
            if (cookieStorage is not null)
            {
              // Note: Might need to clear these if theres something to set?
              cookieStorage.CookieContainer.SetCookies(cookieStorage.Uri, responseCookieHeader);

              Console.WriteLine($"Stored cookie from response: {responseCookieHeader}");
            }
          }
       }
      }
    }

    /// <inheritdoc />
    public object BeforeSendRequest(ref Message request, IClientChannel channel)
    {
      if (request.Properties.TryGetValue(CookieStorageProperty.Name, out object cookieStorageObject))
      {
        // Request contains a cookie container, lets use it
        var cookieStorage = cookieStorageObject as CookieStorageProperty;
        if (cookieStorage.CookieContainer != null)
        {
          Console.WriteLine($"Cookie for request: {cookieStorage.CookieContainer.GetCookieHeader(cookieStorage.Uri)}");
          HttpRequestMessageProperty httpRequestMessage;
          object httpRequestMessageObject;

          if (request.Properties.TryGetValue(HttpRequestMessageProperty.Name, out httpRequestMessageObject))
          {
            httpRequestMessage = httpRequestMessageObject as HttpRequestMessageProperty;
            httpRequestMessage.Headers.Add(HttpRequestHeader.Cookie, cookieStorage.CookieContainer.GetCookieHeader(cookieStorage.Uri));
          }
          else
          {
            httpRequestMessage = new HttpRequestMessageProperty();
            httpRequestMessage.Headers.Add(HttpRequestHeader.Cookie, cookieStorage.CookieContainer.GetCookieHeader(cookieStorage.Uri));
            request.Properties.Add(HttpRequestMessageProperty.Name, httpRequestMessage);
          }
        }
      }

      // Pass this as the correlationState to the AfterReceiveReply method as an accessor to the cookie storage object
      return cookieStorageObject;
    }
  }

Then in my CALLING code, I use an OperationContextScope around each ASYNC call to make sure said shared CookieStorageProperty is injected into a message property on both the LogonAsync call and the subsequent GetDataSourceContentsAsync. I can write a wrapper to clean this up, but this appears to pass the same reference into the message properties that both calls share, and the message inspector seems to modify it in isolation from other parallel threads THAT I CAN TELL (I'm Console.Logging in the Inspector to try and make sure I dont see the same cookie value used multiple times given the structure of this test):

      using (var client = GetStaticClient(url))
      {
        await Parallel.ForEachAsync(source, async (login, cancellationToken) =>
        {
          var cookieStorageProperty = new CookieStorageProperty
          {
            Uri = client.Endpoint.Address.Uri,
            CookieContainer = new System.Net.CookieContainer()
          };

          // Inject the scoped external cookie storage onto the individual call as a message property
          // for the IClientMessageInspector to later rip off and use BY REFERENCE
          Task logonTask;
          using (OperationContextScope scope = new OperationContextScope(client.InnerChannel))
          {
            if (!OperationContext.Current.OutgoingMessageProperties.ContainsKey(CookieStorageProperty.Name))
            {
              OperationContext.Current.OutgoingMessageProperties[CookieStorageProperty.Name] = cookieStorageProperty;
            }

            logonTask = client.LogonAsync(login.Key, login.Value);
          }

          // Await the logon task that is going to populate the cookie back into the cookie storage
          await logonTask;

          // Inject the now populated external cookie storage onto the NEXT individual call to be used
          // by the IClientMessageInspector to add the cookie to the request
          Task<GetDataSourceContentsResponse> myTask;
          using (OperationContextScope scope = new OperationContextScope(client.InnerChannel))
          {
            if (!OperationContext.Current.OutgoingMessageProperties.ContainsKey(CookieStorageProperty.Name))
            {
              OperationContext.Current.OutgoingMessageProperties[CookieStorageProperty.Name] = cookieStorageProperty;
            }

            var trustedHeader = new TrustedUserHeader();
            var itemPath = "/MyDataSetPath";
            myTask = client.GetDataSourceContentsAsync(trustedHeader, itemPath);
          }

          // Await the result. If during any call in between here the server sends back and updated sliding expiration cookie,
          // the scoped cookie storage should have been updated to the new cookie for use in subsequent calls
          var result = await myTask;

          Console.WriteLine(result.Definition.ConnectString);
        });
      }

This SEEMS to avoid the HttpClient issue (static client), and lets me INJECT on each call a cookie storage solution that essentially lets me completely remove the cookie handling from the underlying HttpClient and move it up to my IClientMessageInspector, and maintain a per-thread cookie storage so that multiple threads executing in parallel don't step on each other's cookie store...

It feels a little evil doing it this way, but does anyone have a better idea?

mconnew commented 1 year ago

This is a great way to do this, and your solution should work correctly, but I might be able to do you one better which will avoid needing to modify the calling code. BeforeSendRequest is passed the channel instance (as an IClientChannel) which implements IExtensibleObject<IContextChannel>. This interface allows you to do some really interesting tricks but most people don't know about it so there isn't a lot of examples out there on how to use it. When an object implements IExtensibleObject<T>, it allows you to attach and later retrieve other objects which implement IExtenstion<T>. Here's how I would implement a solution (I haven't run this code, just typing in a browser so apologies for missed typos):

public class CookieStorageHolder : IExtension<IClientChannel>
{
    public CookieStorageHolder(Uri uri) => Uri = uri;
    public void Attach(IClientChannel owner) { /* Intentionally empty */ }
    public void Detach(IClientChannel owner) { /* Intentionally empty */ }
    public CookieContainer CookieContainer { get; } = new CookieContainer();
    public Uri Uri { get; }
    public void UpdateCookies(WebHeaderCollection headers)
    {
        var responseCookieHeader = headers["Set-Cookie"];
        if (!string.IsNullOrWhiteSpace(responseCookieHeader))
        {
            CookieContainer.SetCookies(Uri, responseCookieHeader);
        }        
    }
    public void ApplyCookies(WebHeaderCollection headers)
    {
        headers.Add(HttpRequestHeader.Cookie, CookieContainer.GetCookieHeader(Uri));
    }
}

public class CookieClientMessageInspector : IClientMessageInspector
{
    public object BeforeSendRequest(ref Message request, IClientChannel channel)
    {
        var cookeHolder = channel.Extensions.Find<CookieStorageHolder>();
        if (cookieHolder is null)
        {
            cookieHolder = new CookieStorageHolder(channel.RemoteAddress.Uri);
            channel.Extensions.Add(cookieHolder);
            return cookieHolder; // No cookies to apply so return it as correlation object
        }
        HttpRequestMessageProperty httpRequestMessage;
        if (request.Properties.TryGetValue(HttpRequestMessageProperty.Name, out object httpRequestMessageObject))
        {
            httpRequestMessage = httpRequestMessageObject as HttpRequestMessageProperty;
        }
        else
        {
            httpRequestMessage = new HttpRequestMessageProperty();
            request.Properties.Add(HttpRequestMessageProperty.Name, httpRequestMessage);
        }
        cookieHolder.ApplyCookies(httpRequestMessage.Headers);
        return cookieHolder;
    }

    public void AfterReceiveReply(ref Message reply, object correlationState)
    {
        var cookieHolder = correlationState as CookieStorageHolder;
        if (reply.Properties.TryGetValue(HttpResponseMessageProperty.Name, out object httpResponseMessageObject))
        {
            var httpResponseMessage = httpResponseMessageObject as HttpResponseMessageProperty;
            cookieHolder.UpdateCookies(httpResponseMessage.Headers);
        }
    }
}

This solution doesn't require anything to be done up front with OperationContextScope before you call anything. Let me know how this works for you.

i8beef commented 1 year ago

So I would love to not have to wrap every call in an OperationContextScope to pass in an external managed, scoped cookie storage location... I'm unclear on what the LIFETIME of that IClientChannel is though.

Whatever I end up with must:

  1. Persist a cookie BETWEEN multiple calls on one "thread" (Probably the wrong word here... request? context scope?)
  2. Without leaking between, say, 30 different threads all using this static client at once with different logins

That LOOKS like it would be a context for a singe CHANNEL, and I thought in a static ChannelFactory that's a POOL which could imply reuse between threads and thus leakage between them? OR it coulld look like a context for a SINGLE call that wouldn't span between multiple calls on a single scope (e.g., Login, then call "SetParameters", then call "Render", then call Logout), but then I would think its no different than just shoving the cookie container in the IClientMessageInspector itself, except it'd only be for that CHANNEL and not ALL channels?

Just unclear on what the life cycle is of that...

mconnew commented 1 year ago

Sorry I missed your questions. IClientChannel is implemented by the channel returned from ChannelFactory<IService>.CreateChannel(). While the channel implements IService, it also implements IClientChannel. With HTTP, channels are relatively cheap. The expensive component is the ChannelFactory instance. Presuming you've already created a channel factory and it's available at this._factory, Your flow would be something like this:

await using(var channel = _factory.CreateChannel(url))
{
    await channel.LogonAsync(login.Key, login.Value);
    var trustedHeader = new TrustedUserHeader();
    var itemPath = "/MyDataSetPath";
    var result = await channel.GetDataSourceContentsAsync(trustedHeader, itemPath);
    Console.WriteLine(result.Definition.ConnectString);
    await channel.LogoutAsync();
}

The channels now implements IAsyncDisposable in a cleaner way, it swallows the exceptions you would normally need to handle if using Dispose. This means if you use await using(...) you can get some really clean code.

mhallockPcty commented 1 year ago

I think I'm missing a piece... so I am using a VS generated SOAP client here, and my understanding of what all goes on under the hood of that is incomplete. That is essentially a singleton for me here, and contains a ChannelFactory which I'm assuming is the expensive part / why it SHOULD be a singleton...

When I call client.GetDataSourceContentsAsync() instead of manually creating my channel via the contained ChannelFactory.CreateChannel inside it, does that wrapping generated client essentially call it under the hood in GetDataSourceContentsAsync and create (and close) a new channel-per-call essentially, complete with its own context for ONLY that one call? And thus opening the channel manually like this allows me to reuse that channel (and its context) for MULTIPLE calls?

What happens to that Channel context when it exits the using scope? Like, if that Channel is POOLED does it actually close and completely lose its scope so the next one starts fresh, or is there a possibility of scope overlap (e.g., if the next call fails to call "LogOn" to REPLACE scope accidentally, is it gonna carry the scope from the last usage)?

I like the pattern a bit more here, I'm just unclear on the scope lifetime of that Channel...