Preventing unauthorized logged users to edit/Delete in asp.net core

Question

I am trying to prevent unauthorized logged in users from seeing and editing other customers records but seem not working. I have asp.net core ApplicationUser tied to each category created. I am trying to match the currentUser Id and the applicationUser Id that is stored in the Database.

So if another user should put a category id in the URL that doesn't match with the currently logged ApplicationuserID. If that didn't match then redirects to index of category.

I don't know what I am doing wrong. I have tried different options and other suggestions online

// GET: Categories/Edit/5
        public async Task<IActionResult> Edit(int? id)
        {
            var currentUser = await _userManager.GetUserAsync(HttpContext.User);

            var Categories = _context.Categories.FirstOrDefault(m => m.ApplicationUserId == currentUser.Id);

            if (id == null)
            {
                return NotFound();
            }
            if (Categories.ApplicationUserId != currentUser.Id)
            {
                return RedirectToAction(nameof(Index));
                //return View(category);
            }
            else
            {
                var category = await _context.Categories.FindAsync(id);
                if (category == null)
                {
                    return NotFound();
                }
                return View(category);
            }
        }

The Httpedit

  [HttpPost]
        [ValidateAntiForgeryToken]
        public async Task<IActionResult> Edit(int id, [Bind("CategoryId,StoreId,ApplicationUserId,CategoryName,Description")] Category category)
        {
            var currentUser = await _userManager.GetUserAsync(HttpContext.User);
            var store = _context.Stores.FirstOrDefault(m => m.ApplicationUserId == currentUser.Id);
            var categories = _context.Categories.FirstOrDefault(m => m.ApplicationUserId == currentUser.Id);

            if (id != category.CategoryId)
            {
                return NotFound();
            }
            if (categories.ApplicationUserId != currentUser.Id)
            {
                return RedirectToAction(nameof(Index));
            }
            else
            {
                if (ModelState.IsValid)
                {
                    try
                    {

                        category.StoreId = store.Id;
                        category.ApplicationUserId = currentUser.Id;
                        _context.Update(category);
                        await _context.SaveChangesAsync();
                    }
                    catch (DbUpdateConcurrencyException)
                    {
                        if (!CategoryExists(category.CategoryId))
                        {
                            return NotFound();
                        }
                        else
                        {
                            throw;
                        }
                    }
                    return RedirectToAction(nameof(Index));
                }
            }
            return View(category);
        }

Thanks

Answer 1

You just need to change the flow some (and avoid unnecessary database calls).

Warning: I edited this in place and have not tested it at all!

// GET: Categories/Edit/5
public async Task<IActionResult> Edit(int? id)
{
    // Fail early here, no reason to check the DB if 
    // the user doesn't include the right information
    if (id == null)
    {
        return NotFound();
    }

    var currentUser = await _userManager.GetUserAsync(HttpContext.User);

    // First, look up the category the user is attempting to access
    var category = await _context.Categories.FindAsync(id);

    // Check that the category the user is accessing belongs to them
    if (category.ApplicationUserId != currentUser.Id)
    {        
        return RedirectToAction(nameof(Index));
    }

    return View(category);
}