Wrong Thread.CurrentPrincipal in async WCF end-method
Asked Answered
A

3

9

I have a WCF service which has its Thread.CurrentPrincipal set in the ServiceConfiguration.ClaimsAuthorizationManager.

When I implement the service asynchronously like this:

    public IAsyncResult BeginMethod1(AsyncCallback callback, object state)
    {
        // Audit log call (uses Thread.CurrentPrincipal)

        var task = Task<int>.Factory.StartNew(this.WorkerFunction, state);

        return task.ContinueWith(res => callback(task));
    }

    public string EndMethod1(IAsyncResult ar)
    {
        // Audit log result (uses Thread.CurrentPrincipal)

        return ar.AsyncState as string;
    }

    private int WorkerFunction(object state)
    {
        // perform work
    }

I find that the Thread.CurrentPrincipal is set to the correct ClaimsPrincipal in the Begin-method and also in the WorkerFunction, but in the End-method it's set to a GenericPrincipal.

I know I can enable ASP.NET compatibility for the service and use HttpContext.Current.User which has the correct principal in all methods, but I'd rather not do this.

Is there a way to force the Thread.CurrentPrincipal to the correct ClaimsPrincipal without turning on ASP.NET compatibility?

Awlwort answered 8/1, 2014 at 23:8 Comment(3)
Why such convoluted code? This code isn't really an asynchronous implementation of a service call. You get a synchronous call, wrap it in a task that runs in a threadpool thread (which does NOT preserve the principal), then convert it to an old-style APM pair of methods - why? Create a proper asynchronous method instead of the static WorkerFunction and pass the principal as a parameter instead of setting it in some arbitrary thread's propertiesArchimage
@PanagiotisKanavos: The code truly is asynchronous, because it is part of the WCF async pattern which operates on APM methods. Also, once the principal is set, it is preserved when passed to a Task or a threadpool thread because the user principal is stored in CallContext.Dill
@user18044 oops, I was thinking of .NET 4.5 where you can just define a Task<string> Method1Async to asynchronously implement Method1. This has no effect on how the clients generate their proxies and call your method, just on how the service code implements the contract. You can easily call client.Method1 and the call will get routed to MethodAsyncArchimage
D
4

Starting with a summary of WCF extension points, you'll see the one that is expressly designed to solve your problem. It is called a CallContextInitializer. Take a look at this article which gives CallContextInitializer sample code.

If you make an ICallContextInitializer extension, you will be given control over both the BeginXXX thread context AND the EndXXX thread context. You are saying that the ClaimsAuthorizationManager has correctly established the user principal in your BeginXXX(...) method. In that case, you then make for yourself a custom ICallContextInitializer which either assigns or records the CurrentPrincipal, depending on whether it is handling your BeginXXX() or your EndXXX(). Something like:

public object BeforeInvoke(System.ServiceModel.InstanceContext instanceContext, System.ServiceModel.IClientChannel channel, System.ServiceModel.Channels.Message request){
    object principal = null;
    if (request.Properties.TryGetValue("userPrincipal", out principal))
    {
        //If we got here, it means we're about to call the EndXXX(...) method.
        Thread.CurrentPrincipal = (IPrincipal)principal;
    }
    else
    {
        //If we got here, it means we're about to call the BeginXXX(...) method.
        request.Properties["userPrincipal"] = Thread.CurrentPrincipal;            
    }
    ...
 }

To clarify further, consider two cases. Suppose you implemented both an ICallContextInitializer and an IParameterInspector. Suppose that these hooks are expected to execute with a synchronous WCF service and with an async WCF service (which is your special case).

Below are the sequence of events and the explanation of what is happening:

Synchronous Case

ICallContextInitializer.BeforeInvoke();
IParemeterInspector.BeforeCall();
//...service executes...
IParameterInspector.AfterCall();
ICallContextInitializer.AfterInvoke();

Nothing surprising in the above code. But now look below at what happens with asynchronous service operations...

Asynchronous Case

ICallContextInitializer.BeforeInvoke();  //TryGetValue() fails, so this records the UserPrincipal.
IParameterInspector.BeforeCall();
//...Your BeginXXX() routine now executes...
ICallContextInitializer.AfterInvoke();

//...Now your Task async code executes (or finishes executing)...

ICallContextInitializercut.BeforeInvoke();  //TryGetValue succeeds, so this assigns the UserPrincipal.
//...Your EndXXX() routine now executes...
IParameterInspector.AfterCall();
ICallContextInitializer.AfterInvoke();

As you can see, the CallContextInitializer ensures you have opportunity to initialize values such as your CurrentPrincipal just before the EndXXX() routine runs. It therefore doesn't matter that the EndXXX() routine assuredly is executing on a different thread than did the BeginXXX() routine. And yes, the System.ServiceModel.Channels.Message object which is storing your user principal between Begin/End methods, is preserved and properly transmitted by WCF even though the thread changed.

Overall, this approach allows your EndXXX(IAsyncresult) to execute with the correct IPrincipal, without having to explicitly re-establish the CurrentPrincipal in the EndXXX() routine. And as with any WCF behavior, you can decide if this applies to individual operations, all operations on a contract, or all operations on an endpoint.

Dill answered 13/1, 2014 at 0:26 Comment(2)
Thanks for the elaborate answer and links to external info! Implementing the CallContextInitializer does indeed fix the issue. One question I have is why do you store the principal in the message properties and not in the call context using CallContext.SetData(...)? If no other answers come in, I'll award this answer the bounty.Awlwort
@user18044: CallContext only passes information from a "parent" thread on downward to all "child" threads created from it. In the case of WCF, the thread that executes your completion callback (from the BeginXXX() routine) is not the parent of the EndXXX() thread. Instead, the completion callback merely signals the WCF dispatcher to create its own thread to execute the EndXXX() routine.Dill
A
1

Not really the answer to my question, but an alternate approach of implementing the WCF service (in .NET 4.5) that does not exhibit the same issues with Thread.CurrentPrincipal.

    public async Task<string> Method1()
    {
        // Audit log call (uses Thread.CurrentPrincipal)

        try
        {
            return await Task.Factory.StartNew(() => this.WorkerFunction());
        }
        finally 
        {
            // Audit log result (uses Thread.CurrentPrincipal)
        }
    }

    private string WorkerFunction()
    {
        // perform work
        return string.Empty;
    }
Awlwort answered 16/1, 2014 at 22:41 Comment(2)
+1. I actually thought you might be restrained from exposing the Task-based WCF service API, because of the .NET version requirements.Gibson
No, we're on .NET 4.5, but we deliver an SDK that allows customers to implement their own WCF services running in our framework. We provide a service host that takes care of authN and authZ. However, I don't control how our customers implement the WCF service. I wanted to make sure it also worked correctly when they use Begin/End methods.Awlwort
F
0

The valid approach to this is to create an extension:

public class SLOperationContext : IExtension<OperationContext>
{
    private readonly IDictionary<string, object> items;

    private static ReaderWriterLockSlim _instanceLock = new ReaderWriterLockSlim();

    private SLOperationContext()
    {
        items = new Dictionary<string, object>();
    }

    public IDictionary<string, object> Items
    {
        get { return items; }
    }

    public static SLOperationContext Current
    {
        get
        {
            SLOperationContext context = OperationContext.Current.Extensions.Find<SLOperationContext>();
            if (context == null)
            {
                _instanceLock.EnterWriteLock();
                context = new SLOperationContext();
                OperationContext.Current.Extensions.Add(context);
                _instanceLock.ExitWriteLock();
            }
            return context;
        }
    }

    public void Attach(OperationContext owner) { }
    public void Detach(OperationContext owner) { }
}

Now this extension is used as a container for objects that you want to persist between thread switching as OperationContext.Current will remain the same.

Now you can use this in BeginMethod1 to save current user:

SLOperationContext.Current.Items["Principal"] = OperationContext.Current.ClaimsPrincipal;

And then in EndMethod1 you can get the user by typing:

ClaimsPrincipal principal = SLOperationContext.Current.Items["Principal"];

EDIT (Another approach):

public IAsyncResult BeginMethod1(AsyncCallback callback, object state)
{
    var task = Task.Factory.StartNew(this.WorkerFunction, state);

    var ec = ExecutionContext.Capture();

    return task.ContinueWith(res =>
        ExecutionContext.Run(ec, (_) => callback(task), null));
}
Fisc answered 11/1, 2014 at 9:17 Comment(8)
The whole idea of Thread.CurrentPrincipal is that it is globally available for each thread. Having to explicitly save and restore the variable kind of defeats the purpose IMO.Awlwort
It is available, except your continuation thread is unaware of the security context (principal) that instantiated first thread. Continuation thread is not necessarily same thread on which Begin... was invoked.Limousin
I am aware that the continuation thread may be different. My question was how to make sure that the Thread.CurrentPrincipal was set correctly for that thread. The solution proposed by Brent Arias above does that in a transparent way.Awlwort
Have you considered abandoning Begin/End+ContinueWith pattern and using async/await pattern instead? Because with the latter, and with the httpRuntime being set to 4.5, you get what you want out of the box. However, requirement is that you are able to utilize .NET 4.5 within your production environment. I'd be happy to provide you with the code sample if that's the case.Limousin
Thanks, appreciate the offer. I know how to create a Task based asynchronous WCF service. And doing so avoids the principal issue. However, I can not control how our customers create WCF services. They may use the Task based implementation, or may use the Begin/End APM methods. I just could not figure out how to make the principal work in the latter case.Awlwort
Ok, fair enough explanation. I've posted another approach, would you mind testing it? It places your current ExecutionContext into SynchronizationContext, and should also solve the issue. Core feature is doing ExecutionContext.Capture() and passing it as parameter when creating continuation task.Limousin
I already have tested your alternate approach. My question was first answered by someone that posted this solution. Unfortunately he deleted his answer after I told him in the comments that it did not work because the SecurityContext property in the captured ExecutionContext is null. At least this gave me an opportunity to re-add the lost info.Awlwort
I posted my async/await solution as an alternate answer here. I did not know how to make this work using .ContinueWith though.Awlwort

© 2022 - 2024 — McMap. All rights reserved.