Security, Thread.CurrentPrincipal, and ConfigureAwait(false)
R

1

10

Would using Thread.CurrentPrincipal's claims in a referenced library that uses ConfigureAwait(false) pose any problems or will the flowing of ExecutionContext's logical call context take care of me there? (my reading and testing so far indicates that it will).

Example WebAPI Controller Action:

[CustomAuthorizeThatSetsCurrentUsersClaimsToThreadCurrentContextAndHttpContextCurrentUser]
public async Task<Order> Get(int orderId)
{
    return await _orderBusinessLogicLibrary.LoadAsync(orderId); // defaults to .ConfigureAwait(true)
}

Example load functions from external, referenced library:

[ClaimsPrincipalPermission(
    SecurityAction.Demand,
    Operation="Read",
    Resource="Orders")]
[ClaimsPrincipalPermission(
    SecurityAction.Demand,
    Operation="Read",
    Resource="OrderItems")]
public async Task<Order> Load(int orderId)
{
    var order = await _repository.LoadOrderAsync(orderId).ConfigureAwait(false);

    // here's the key line.. assuming this lower-level function is also imposing
    // security constraints in the same way this method does, would
    // Thread.CurrentPrincipal still be correct inside the function below?
    order.Items = await _repository.LoadOrderItemsAsync(orderId).ConfigureAwait(false);
    return order;
}

Also, the answer can't be "well don't use ConfigureAwait(false) then!". That can cause other problems such as deadlock (Don't Block on Async Code).

Ratify answered 9/12, 2013 at 20:17 Comment(0)
M
13

From my tests, it appears that Thread.CurrentPrincipal will flow correctly, even if you use ConfigureAwait(false). The following WebAPI code sets the principal and then blocks on an async call, forcing another thread to resume the async method. That other thread does inherit the correct principal.

private async Task<string> Async()
{
    await Task.Delay(1000).ConfigureAwait(false);
    return "Thread " + Thread.CurrentThread.ManagedThreadId + ": " + Thread.CurrentPrincipal.Identity.Name + "\n";
}

public string Get(int id)
{
    var user = new ClaimsPrincipal(new ClaimsIdentity(
        new[]
        {
            new Claim(ClaimTypes.Name, "Bob"),
        }
    ));
    HttpContext.Current.User = user;
    Thread.CurrentPrincipal = user;

    var ret = "Thread " + Thread.CurrentThread.ManagedThreadId + ": " + Thread.CurrentPrincipal.Identity.Name + "\n";

    ret += Async().Result;

    return ret;
}

When I run this code on a new instance of IISExpress, I get:

"Thread 7: Bob\nThread 6: Bob\n"

However, I should point out that using ConfigureAwait(false) to avoid deadlock is not recommended. This is especially true on ASP.NET. If at all possible, use ConfigureAwait(false) and also use async all the way. Note that WebAPI is a fully-async stack and you should be able to do this.

Midships answered 10/12, 2013 at 3:13 Comment(2)
Great, I was seeing the same results. One question for clarification -- your last comment about not using ConfigureAwait(false) to avoid deadlock is specific to the use of .Result to block on the async method, correct? So you shouldn't switch from ConfigureAwait(true) to ConfigureAwait(false) to fix deadlock, you should make the whole stack fully async and would still want ConfigureAwait(true) on ASP.NET awaits in order to fully restore HttpContext and so on.Ratify
@MichaelTrotter: The default for ConfigureAwait is true. So if your async method needs the context, it should just not use ConfigureAwait at all. If it doesn't need the context, it should use ConfigureAwait(false). I recommend making the whole stack async and also using ConfigureAwait(false) on any method that doesn't need the context.Midships

© 2022 - 2024 — McMap. All rights reserved.