Managing Credentials Within ChannelFactory Cache

[Ian-Hodges]

Hi
I've converted an old library to dotnet core and implemented a ChannelFactory caching solution. The services which it connects to require basic authorization added to each requests HTTP headers. The header is added to each request with a ClientMesageInspector. Because the ClientMessageInspector is immutable once the first channel is created the ChannelFactory cache stores each instance in a dictionary with a key based on the ChannelFactory endpoint, username and password combination. Also, due to the over head of creating the ChannelFactory I do not dispose of them. My concern is the usernames and passwords are kept in memory within the dictionary keys and also within the ClientMesageInspector added to the ChannelFactory. Is there a better solution to this?
Thanks in advance.
Bob.

[mconnew]

Would code along these lines work for you:

var factory = new ChannelFactory<IService>(new BasicHttpBinding(), "http://myserver/MyService.svc");
var proxy = factory.CreateChannel();
((ICommunicationObject)proxy).Open();
using (var scope = new OperationContextScope((IContextChannel)serviceProxy))
{
    var requestMessageProperty= new HttpRequestMessageProperty();
    OperationContext.Current.OutgoingMessageProperties[HttpRequestMessageProperty.Name] = requestMessageProperty;
    requestMessageProperty.Headers["Authentication"] = authenticationHeaderValue;
    result = proxy.Foo();
}

You can reuse the single proxy for multiple simultaneous requests without issue as HTTP doesn't bind to a single socket per channel.

[Ian-Hodges]

Many thanks for the reply. I've had a play and it looks like this works well. I can omit the password from the cache lookup key, and the password is only in memory for as long as needed. I read up a bit more and it seems this is the preferred method over an IClientMessageInspector when amending headers.
There is one snag though. We have an overload which calls services asynchronously. It looks like this is not an option as an await can continue on a different different thread to the OperationContextScope.

[Ian-Hodges]

Looking into the asynchronous snag, it looks like I can just move the using OperationContextScope to a new method. My code looks something like:

public async Task<TReturn> UseServiceAsync<TChannel, TReturn>(Func<TChannel, Task<TReturn>> code)
{
    BasicHttpBinding binding = new BasicHttpBinding();
    ChannelFactory<TChannel> chanFactory = new ChannelFactory<TChannel>(binding, new EndpointAddress("http://moo.com"));
    TChannel channel = chanFactory.CreateChannel();
    ((IClientChannel)channel).Open();
    TReturn result = await CallServiceAsync(code, channel);
    ((IClientChannel)channel).Close();

    return result;
}

private Task<TReturn> CallServiceAsync<TChannel, TReturn>(Func<TChannel, Task<TReturn>> code, TChannel channel)
{
    Task<TReturn> result;

    using (new OperationContextScope((IClientChannel)channel))
    {
        var requestMessageProperty = new HttpRequestMessageProperty();
        OperationContext.Current.OutgoingMessageProperties[HttpRequestMessageProperty.Name] = requestMessageProperty;
        requestMessageProperty.Headers["Authorization"] = "Basic " + Convert.ToBase64String(Encoding.ASCII.GetBytes(_userame + ":" + _password));
        result = code((TChannel)channel);
    }

    return result;
}

This appears to work fine after some initial testing.

[mconnew]

Your code is potentially dangerous and could be subject to a race condition if the actual service call isn't the first asynchronous operation inside the code delegate. For example, if the code delegate did something like this:

internal static async Task<string> SomeCode(IMyService channel)
{
    await Task.Delay(100); // Some call which completes asynchronously and takes a not-insignificant time to complete
    return await channel.GetMyStringAsync();
}

The first Task.Delay would cause the method to return with a Task which hasn't been completed yet. When returning to CallServiceAsync, Dispose is called on OperationContextScope which then resets OperationContext.Current on the current Thread. By the time you make your actual service call you've returned on a different thread and you've lost your OperationContext. We made a change in the full framework to move OperationContext/OperationContextScope to be propagated through async and it broke some people who were using some unusual coding patterns on the service side so we had to do a partial revert and make it switchable through app settings. None of that got brought to .NET Core so you are still facing the older broken async behavior. Here's an alternative mechanism which might be cleaner for you:

var factory = new ChannelFactory<IService>(new BasicHttpBinding(), "http://myserver/MyService.svc");
var proxy = factory.CreateChannel();
((ICommunicationObject)proxy).Open();
var opContext = new OperationContext((IClientChannel)proxy);
var prevOpContext = OperationContext.Current; // Optional if there's no way this might already be set
OperationContext.Current = opContext;
try
{
    var requestMessageProperty= new HttpRequestMessageProperty();
    OperationContext.Current.OutgoingMessageProperties[HttpRequestMessageProperty.Name] = requestMessageProperty;
    requestMessageProperty.Headers["Authentication"] = authenticationHeaderValue;
    result = await proxy.FooAsync();
    // If you want to read out things like the response HTTP headers, otherwise you can skip this
    OperationContext.Current = opContext;
    // You could also directly access opContext instead of setting and using OperationContext.Current
    var responseMessageProperty = OperationContext.Current.IncomingMessageProperties[HttpResponseMessageProperty.Name] as HttpResponseMessageProperty;
    Console.WriteLine("Status description: " + responseMesageProperty.StatusDescription);
}
finally
{
    OperationContext.Current = prevOpContext; // Or set to null if you didn't capture the previous context
}

I've opened an issue to track improving the OperationContext async behavior on .Net Core

[Ian-Hodges]

Thanks yet again for the reply. UseServiceAsync is exposed by a wrapper and normally called like so:

using (WCFWrapper wrapper = new WCFWrapper()
{
    Task<QueryResponse> task = wrapper.UseServiceAsync((IProxy svc) => svc.GetQueryAsync());
}

There's nothing stopping people amending it. I tried the following and it does cause issues.

using (WCFWrapper wrapper = new WCFWrapper())
{
    Task<QueryResponse> task = wrapper.UseServiceAsync(async (IProxy svc) => { await Task.Delay(5000); return await svc.GetQueryAsync(); });
}

It's not immediately obvious that there's a problem. I was expecting it to throw an exception, but instead found no Authorization header is sent with the request.

[mconnew]

You have several options. The easiest might be in your WCFWrapper class, save the value for the Authorization header in an AsyncLocal of an internal static property on WCFWrapper. Then in your message inspector, retrieve the value from the AsyncLocal and add the Authorization header like you did previously. After you've added the HTTP header, clear the value in the AsyncLocal to by setting it to null or string.Empty as there are ways to leak an AsyncLocal value to unrelated threads if you mix TPL coding patterns with async/await coding patterns and this is security data. Also make sure again that the value is cleared in the Dispose() of the WCFWrapper as AsyncLocal values get copied to new Task contexts and clearing it out in a child Task doesn't guarantee it's been cleared in a parent class.

[Ian-Hodges]

At first, I thought your 'alternative mechanism' was a suggestion that would get around the issue. I see now that it wasn't? I'll take a look at the AsyncLocal, but I'm happy with the way it's currently running. WCFWrapper is only used by our team of developers. I see no reason why they would put anything other than the single service method to invoke into it. In our documentation I can make a note about it too.

[mconnew]

@Bobmonga, that sounds like it should work then. It wasn't clear whether the usage of your wrapper was an internal usage or a library for external devs so I was trying to cover all the bases and make sure you understood the limitations of what would and wouldn't work.
I'm closing this issue now as your original issue of credentials being persisted (and although you didn't mention it, your memory footprint wouldn't have been great before and you are using a lot less RAM now) has been addressed and you have a solution which works.