Aspnetcore: InvalidOperationException - Operations that change non-concurrent collections must have exclusive acces

[Mortana89]

Hi,

During testing of our microservices, we, from time to time, stumble upon the following error.
We're using asp.net core 2.2.0, and are using a custom authorization Middleware piece that populates the policies. Basically, we have currently three policies:
Two that validate an incoming id (something like tenant id), and one that validates a policy annotation of a controller action with the users allowed policies. The latter is currently not completely implemented and just returns true as we haven't finished this development yet.
After a restart of the service fabric nodes, the issue goes away until eventually it pops up again, totally random.

The mentioned line in our custom Middleware:

```c#
public class StoreContextIdMiddleware
{
private readonly RequestDelegate _next;

    public StoreContextIdMiddleware(RequestDelegate next)
    {
        _next = next;
    }

    public async Task Invoke(HttpContext context, IHeaderContextId contextId)
    {
        try
        {
            contextId.TenantId = context.Request.Headers.FirstOrDefault(x => x.Key == "zf-tuuid").Value.ToString();
            contextId.OrganisationId =
                context.Request.Headers.FirstOrDefault(x => x.Key == "zf-ouuid").Value.ToString();
        }
        catch (Exception)
        {
        }

        await _next.Invoke(context);
    }

The Stacktrace:

System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.
at System.Collections.Generic.Dictionary2.FindEntry(TKey key) at System.Collections.Generic.Dictionary2.ContainsKey(TKey key)
at Microsoft.AspNetCore.Authorization.AuthorizationOptions.GetPolicy(String name)
at ZFH.Common.Authorization.AuthorizationPolicyProvider.GetPolicyAsync(String policyName)
at Microsoft.AspNetCore.Authorization.AuthorizationPolicy.CombineAsync(IAuthorizationPolicyProvider policyProvider, IEnumerable`1 authorizeData)
at Microsoft.AspNetCore.Mvc.Authorization.AuthorizeFilter.GetEffectivePolicyAsync(AuthorizationFilterContext context)
at Microsoft.AspNetCore.Mvc.Authorization.AuthorizeFilter.OnAuthorizationAsync(AuthorizationFilterContext context)
at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeFilterPipelineAsync()
at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeAsync()
at Microsoft.AspNetCore.Builder.RouterMiddleware.Invoke(HttpContext httpContext)
at ZFH.Common.Authorization.StoreContextIdMiddleware.Invoke(HttpContext context, IHeaderContextId contextId)
at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)
```

[Eilon]

Can you show what ZFH.Common.Authorization.AuthorizationPolicyProvider.GetPolicyAsync(String policyName) looks like?

[Mortana89]

Sure:

```c#
public class AuthorizationPolicyProvider : DefaultAuthorizationPolicyProvider
{
private readonly AuthorizationOptions _options;

    public AuthorizationPolicyProvider(IOptions<AuthorizationOptions> options) : base(options)
    {
        _options = options.Value;
    }

    public override async Task<AuthorizationPolicy> GetPolicyAsync(string policyName)
    {
        // Check static policies first
        var policy = await base.GetPolicyAsync(policyName);

        if (policy == null)
        {
            string[] policyScopes = policyName.Split(":");
            AuthorizationPolicyBuilder builder = new AuthorizationPolicyBuilder();

            // An adm policy defines a system admin. He doesn't require a check on scope of org/tenant, as this is us!
            // But we allow some granularity to prevent other system admins of doing stuff that is not allowed
            if (policyScopes[0] == "adm")
            {
                builder.AddRequirements(new AdminRequirement())
                    .AddRequirements(new PermissionRequirement(policyScopes[1]));

            // For a tid policy, we need to check if the contexts tenant id is an allowed tenant for this user
            } else if (policyScopes[0] == "tid")
            {
                builder.AddRequirements(new TenantRequirement())
                    .AddRequirements(new PermissionRequirement(policyScopes[1]));
            // As for a oid policy, we check if the user has access to an organisation
            } else if (policyScopes[0] == "oid")
            {
                builder.AddRequirements(new OrganisationRequirement())
                    .AddRequirements(new TenantRequirement())
                    .AddRequirements(new PermissionRequirement(policyScopes[1]));
            }

            policy = builder.Build();

            // Add policy to the AuthorizationOptions, so we don't have to re-create it each time
            _options.AddPolicy(policyName, policy);
        }

        return policy;
    }
}

```

[Eilon]

Thanks for the details @Mortana89 , we will take a look.

cc @Tratcher @HaoK

[rynowak]

Options aren't intended to be modified after statup. Is there a reason why you need _options in that code at all? Are you using it as a cache?

Nevermind, you have a comment explaining just that 😁 . I'd suggest just creating your own ConcurrentDictionary<string, Policy> and using that as a cache. We expect options types to be treated as readonly after startup code runs.

[Mortana89]

Thanks for the update, I'll implement the concurrentdictionary.
Can we make the options a private readonly variable to prevent changes from outside that don't go via constructor(startup) then? As I got a part of this code from a tutorial I found, I suspect more people might run into this issue.

I see I declare it locally, stupid :D

[Narshe1412]

@Mortana89 could you share the dictionary implementation? I actually just ran into this same issue.
Thanks!

[Mortana89]

Sure, it's pretty easy;

`public class AuthorizationPolicyProvider : DefaultAuthorizationPolicyProvider
{
private readonly AuthorizationOptions _options;
private readonly ConcurrentDictionary _policies;

    public AuthorizationPolicyProvider(IOptions<AuthorizationOptions> options) : base(options)
    {
        _options = options.Value;

        _policies = new ConcurrentDictionary<string, AuthorizationPolicy>();
    }

    public override async Task<AuthorizationPolicy> GetPolicyAsync(string policyName)
    {
        // Check static policies first
        _policies.TryGetValue(policyName, out var policy);

        if (policy == null)
        {
            string[] policyScopes = policyName.Split(":");
            AuthorizationPolicyBuilder builder = new AuthorizationPolicyBuilder();

            // An adm policy defines a system admin. He doesn't require a check on scope of org/tenant, as this is us!
            // But we allow some granularity to prevent other system admins of doing stuff that is not allowed
            if (policyScopes[0] == "adm")
            {
                builder.AddRequirements(new AdminRequirement())
                    .AddRequirements(new PermissionRequirement(policyScopes[1]));

            // For a tid policy, we need to check if the contexts tenant id is an allowed tenant for this user
            } else if (policyScopes[0] == "tid")
            {
                builder.AddRequirements(new TenantRequirement())
                    .AddRequirements(new PermissionRequirement(policyScopes[1]));
            // As for a oid policy, we check if the user has access to an organisation
            } else if (policyScopes[0] == "oid")
            {
                builder.AddRequirements(new OrganisationRequirement())
                    .AddRequirements(new TenantRequirement())
                    .AddRequirements(new PermissionRequirement(policyScopes[1]));
            }

            policy = builder.Build();

            // Add policy to the AuthorizationOptions, so we don't have to re-create it each time
            _policies.TryAdd(policyName, policy);
        }

        return policy;
    }
}`

[Narshe1412]

Oh brilliant, I had it on a different place :)
It's working perfect for us too!

[Narshe1412]

Btw, this was happening just in a couple of machines. We had no errors in any Dev or QA machines, neither in production. If anyone stumbles upon this issue and can shed some light in why it may not be 100% reproducible all help welcomed.
Thanks again @Mortana89