Stop user from viewing items from table not allocated to their userId

Question

Trying to throw an error page if a user tries to manipulate the url to view a deck that does not belong to them.

So when a card is added to the deck table, the id of the current user is appended to the table along with the card.

Here i am getting the id of user, and then comparing that to the id of the userId property in the deck. If the current user's ID doesnt match up, then i want to throw an error page as seen below.

This doesnt work. any ideas? When i debug it says that item currentUserId and item.UserId have the same value, even when i use the url to move to a different deck that doesnt belong to them

My Controller:

    public ActionResult Details(int id)
    {
        var currentUserId = User.Identity.GetUserId();

        var deck = _context.Decks.SingleOrDefault(d => d.id == id);

        if (deck == null)
            return HttpNotFound();

        var userDecks = _context.Decks.Where(u => u.UserId == 
         currentUserId);


        foreach (var item in userDecks)
        {
            if (currentUserId != item.UserId)
            {
                return View("Error");
            }
        }

        return View(deck);
    }

My Model for reference:

public class Deck
{
    public int id { get; set; }

    public string Name { get; set; }

    public string Notes { get; set; }
    [DisplayName("Card")]
    public virtual List<Card> Card { get; set; }

    public string UserId { get; set;  }
}

Answer 1

You're checking for the same value twice, so it's going to be the same both times. First you get all decks matching the current user:

var userDecks = _context.Decks.Where(u => u.UserId == currentUserId);

Then you check if all th decks matching the current user... match the current user:

foreach (var item in userDecks)
{
    if (currentUserId != item.UserId)
    {
        return View("Error");
    }
}

Which they always will.

Note that in all this logic you've abandoned the original deck specified by the given id:

var deck = _context.Decks.SingleOrDefault(d => d.id == id);

Instead of checking all decks owned by the current user, you really only need to check that specified one. For example:

public ActionResult Details(int id)
{
    var currentUserId = User.Identity.GetUserId();

    var deck = _context.Decks.SingleOrDefault(d => d.id == id);

    if (deck == null)
        return HttpNotFound();

    if (deck.UserId != currentUserId)
        return View("Error");

    return View(deck);
}