0

I am making a login-system. The method CheckCredentials() takes data from the login form:

public async Task<IActionResult> CheckCredentials(LoginFormViewModel loginForm)
{
    if (loginForm == null) return NotFound();

    AdminUser au = await db.AdminUsers
        .Include(p => p.Person)
        .Where(u =>
            u.Person.Email1 == loginForm.UserName &&
            u.Password == loginForm.Password)
        .FirstOrDefaultAsync().ConfigureAwait(false);
    if (au != null)
    {
        LogInAdminUser(au);
        return RedirectToLocal(loginForm.ReturnUrl ?? au.StartPage ?? "/Admin");
    }

    TempData["Message"] = "No such combination of username and password exists. Please try again.";
    return RedirectToAction("Login", new { loginForm.ReturnUrl });
}

If an AdminUser matching the given credentials is found, the void method LogInAdminUser() is called:

private async void LogInAdminUser(AdminUser au)
{
    HttpContext.Session.SetInt32("AdminUserId", au.Id);
    Login login = new Login
    {
        PersonId = au.PersonId,
        AdminUserId = au.Id,
        LogInTime = DateTime.Now,
        RemoteIpAddress = "some string",
        Browser = "some string"
    };
    db.Add(login);
    await db.SaveChangesAsync().ConfigureAwait(false); // This line causes the exception below
    return;
}

This exception is thrown on SaveChangesAsync():

System.ObjectDisposedException: '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. ObjectDisposed_ObjectName_Name'

The Login-class:

public class Login
{
    public int Id { get; set; }
    public int PersonId { get; set; }
    public int? AdminUserId { get; set; }
    public DateTime LogInTime { get; set; }
    public string RemoteIpAddress { get; set; }
    public string Browser { get; set; }
    public AdminUser AdminUser { get; set; }
}

What is happening here? I am not Disposeing anything anywhere in my code.

Update

This is how I create the db-context:

public class AdminController : BaseController
{
    private readonly KlubbNettContext db;
    private readonly IMapper auto;

    public AdminController(KlubbNettContext context, IMapper mapper, IHostingEnvironment env) : base(env)
    {
        db = context;
        auto = mapper;
    }

    // The rest of the controller methods here.
    // Both CheckCredentials() and LogInAdminUser() are among the
    // methods of this controller.
}
Stian
  • 1,522
  • 2
  • 22
  • 52
  • 2
    How are you creating the context?, How are you injecting it? Many times, this problem comes from declaring the context as transient and not scoped and then sharing entities between sevices – rekiem87 Dec 06 '19 at 19:08
  • @rekiem87 See update in the question. – Stian Dec 06 '19 at 19:15
  • 1
    And how do you configure your context, are you using services.AddDbContext in your startup class? – rekiem87 Dec 06 '19 at 19:17
  • your problem is async – Train Dec 06 '19 at 19:21
  • 1
    Also, storing passwords as plain text is not a good practice. Read Storing Passwords section [here](https://stackoverflow.com/questions/549/the-definitive-guide-to-form-based-website-authentication/477578#477578) – Matt.G Dec 06 '19 at 19:37

1 Answers1

2

LoginAdminUser is async, but you're not awaiting it when you call it. As a result, the processing of the action keeps going and returns before LoginAdminUser has completed. When the action returns, the context is disposed giving you the exception.

Long and short, await the call:

if (au != null)
{
    await LogInAdminUser(au);
    return RedirectToLocal(loginForm.ReturnUrl ?? au.StartPage ?? "/Admin");
}
Chris Pratt
  • 232,153
  • 36
  • 385
  • 444