0

For third party authentication, I need a custom Authorize attribute. Here a repository (SessionManager) class is required to check if the user is logged in.

[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true, Inherited = true)]
public class VBAuthorizeAttribute : AuthorizeAttribute, IAuthorizationFilter {
    public async void OnAuthorization(AuthorizationFilterContext context) {
        var sessionManager = (VBSessionManager)context.HttpContext.RequestServices.GetService(typeof(VBSessionManager));
        var user = await sessionManager.GetCurrentSessionAsync();
        if (user == null) {
            context.Result = new UnauthorizedResult();
            return;
        }
    }
}

In the like sessionManager.GetCurrentSessionAsync() the following exception occur:

Cannot access a disposed object. A common cause of this error is disposing a context that was resolved from dependency injection and then later trying to use the same context instance elsewhere in your application. This may occur if you are calling Dispose() on the context, or wrapping the context in a using statement. If you are using dependency injection, you should let the dependency injection container take care of disposing context instances. Object name: 'AsyncDisposer'.

I'm aware of this and don't to any disposing on my own. VBSessionManager got my DbContext injected in its constructor. Inside GetCurrentSessionAsync cookies were checked with LinQ database queries. So no calling of Dispose, using directives or something like that.

Injection in VBSessionManager

public class VBSessionManager {
    readonly VBDbContext db;
    readonly IHttpContextAccessor contextAccessor;
    const string sessionHashCookieName = "xxx";
    VBSession currentSession;

    public VBSessionManager(VBDbContext db, IHttpContextAccessor contextAccessor) {
        this.db = db;
        this.contextAccessor = contextAccessor;
    }

    public async Task<VBSession> GetCurrentSessionAsync() {
        if (currentSession == null) {
            string sessionCookie = GetCookieWithoutPrefix(sessionHashCookieName);
            currentSession = await GetSessionAsync(sessionCookie);

            if (currentSession == null) {
                var cookieUser = GetUserFromCookiePassword().Result;
                // No session detected
                if (cookieUser == null) {
                    return null;
                }
                currentSession = db.Sessions.FirstOrDefault(s => s.UserId == cookieUser.Id);
            }
        }
        return currentSession;
    }
    // ...
}

Injection of services

        services.AddDbContext<VBDbContext>(options => {
            string connectionString = Configuration.GetValue<string>("VBConnectionString");
            options.UseMySql(connectionString,
                    mySqlOptions => {
                        mySqlOptions.ServerVersion(new Version(10, 2, 19), ServerType.MariaDb);
                    }
            );
            bool isDev = CurrentEnvironment.IsDevelopment();
            options.EnableSensitiveDataLogging(isDev);
        });

        services.AddScoped<VBSessionManager>();
Lion
  • 11,498
  • 13
  • 55
  • 113
  • 1
    Try switching from `Scoped` to `Singleton` or `Transient`. For `DI` I reference this (https://docs.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection?view=aspnetcore-2.2) – Ryan Wilson Dec 19 '18 at 15:46
  • I edited the injection of my DbContext in the question. – Lion Dec 19 '18 at 15:47
  • @RyanWilson Tried `Transient` same problem. Singleton doesn't seem good practice since `DbContext` classes are designed to short liveness (see eg https://codereview.stackexchange.com/a/15818/169176) – Lion Dec 19 '18 at 15:57
  • Can you show your code of `GetCurrentSessionAsync()`? The dbcontext may have been disposed from that. – wannadream Dec 19 '18 at 16:07
  • @wannadream The method is added in my question. – Lion Dec 19 '18 at 16:29

4 Answers4

1

It seems that the usage of async cause problems. When I change OnAuthorization to a sync method like this, I don't get any errors:

[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true, Inherited = true)]
    public class VBAuthorizeAttribute : AuthorizeAttribute, IAuthorizationFilter {
        public void OnAuthorization(AuthorizationFilterContext context) {
            var sessionManager = (VBSessionManager)context.HttpContext.RequestServices.GetService(typeof(VBSessionManager));
            var user = sessionManager.GetCurrentSessionAsync().Result;
            if (user == null) {
                context.Result = new UnauthorizedResult();
                return;
            }
        }
    }

Don't know if those attributes (or maybe only the AuthorizeAttribute) isn't designed to work async. For me the current workaround is to use syn method. I also think that this shouldn't decrease performance. But if someone know about the backgrounds and even have a idea how we can use the attribute async, I'd be happy about another answear.

Lion
  • 11,498
  • 13
  • 55
  • 113
  • Just for reference here: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/await. An `await` expression does not block the thread on which it is executing. Instead, it causes the compiler to sign up the rest of the async method as a continuation on the awaited task. Control then returns to the caller of the async method. When the task completes, it invokes its continuation, and execution of the async method resumes where it left off. Therefore, when the continuation is invoked here, the dbcontext might have been disposed already. – wannadream Dec 19 '18 at 16:40
1
public async void OnAuthorization(AuthorizationFilterContext context) {

Of importance here is the use of async void, which is, according to David Fowler, ALWAYS bad. With the setup you have here, the call to OnAuthorization itself cannot be awaited, which means that something like the following is happening:

  1. Scoped instances of VBSessionManager and VBDbContext are being created some amount of time before invoking your OnAuthorization method.
  2. Your OnAuthorization executes and makes a call to VBSessionManager.GetCurrentSessionAsync, returning before said method has a chance to complete (due to the use of async/await).
  3. As OnAuthorization has completed, the IDisposable-implementing VBDbContext is disposed.
  4. The code inside VBSessionManager.GetCurrentSessionAsync is still running - it attempts to use the instance of VBDbContext that has been disposed of.

The reason async void is being used in your situation is simply because that's what is declared in the IAuthorizationFilter interface - you want to use await and the only way to do that is to mark your implementation method as async (you can't make it async Task because that wouldn't implement the interface).

In terms of a solution to this, I'd agree with Gabriel Luci that using policy-based authorisation would be the way to go.

Kirk Larkin
  • 60,745
  • 11
  • 150
  • 162
  • 1
    Note that there also is an [`IAsyncAuthorizationFilter`](https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.mvc.filters.iasyncauthorizationfilter.onauthorizationasync?view=aspnetcore-2.1#Microsoft_AspNetCore_Mvc_Filters_IAsyncAuthorizationFilter_OnAuthorizationAsync_Microsoft_AspNetCore_Mvc_Filters_AuthorizationFilterContext_) which should be used here to allow the filter to be properly asynchronous. – poke Dec 19 '18 at 17:12
1
public class VBAuthorizeAttribute : AuthorizeAttribute, IAuthorizationFilter
{
    public async void OnAuthorization(AuthorizationFilterContext context)
    {
        // …

        await something;

        // …
    }
}

Having a method async void is almost always a bad idea. Asynchronous methods should return a Task to make callers able to determine the result of the asynchronous process.

Since you are implementing IAuthorizationFilter, you are implementing a synchronous authorization filter. You use this when you do not need to do something asynchronously. This is for example true if you just need to look at some of the parameters and then have some ruling to determine whether access is allowed or not.

If you require asynchronous processes, you should not make the void method asynchronous but instead implement IAsyncAuthorizationFilter. This is the interface for implementing an asynchronous authorization filter. In that case, the method you need to implement looks a bit different:

Task OnAuthorizationAsync(AuthorizationFilterContext context)

As you can see, this method returns a Task so it can properly do asynchronous processes. In your case, where you want to await something inside of the method, you can just do it:

public class VBAuthorizeAttribute : AuthorizeAttribute, IAsyncAuthorizationFilter
{
    public async Task OnAuthorizationAsync(AuthorizationFilterContext context)
    {
        // …

        await something;

        // …
    }
}

Now, with a proper asynchronous method that returns a Task, the calling system will be able to consume the method properly and the continuation of the request handling will wait for your authorization filter to be processed.

poke
  • 307,619
  • 61
  • 472
  • 533
0

The OnAuthorization method is not supposed to be used for verifying authorization. It's just a notification that "hey, authorization is happening now".

That said, some have used it for this. But since you declared it as async void, nothing is waiting for this method to finish. That's the root of your exception: by the time the database call is made the request is already finished and the context is disposed. You can just remove the async....

But the proper solution is use IAuthorizationHandler, which is designed for, like the name implies, handling authorization. It has a HandleAsync method, which is a proper async method that is actually awaited (it waits for your decision on authorization before continuing).

Take a look at this answer from a Microsoft employee. You setup the handler, then use it with the regular AuthorizeAttribute like this:

[Authorize(Policy = "MyCustomPolicy")]
Gabriel Luci
  • 28,970
  • 3
  • 37
  • 58