Entity Framework / WebApi 2 permission check implementation

asp.net-web-api asp.net-web-api2 c# entity-framework entity-framework-6

Question

I'm writing a simple "Todo App" using ASP.NET WebApi 2 and Entity Framework 6.1.0-alpha1. My aim is to restrict access, each user shall only view/edit their own Todos.

Example:

    // GET api/Todo/5
    [ResponseType(typeof(Todo))]
    public async Task<IHttpActionResult> GetTodo(int id)
    {
        var todo = await _db.Todos.FindAsync(id);

        if (todo == null)
        {
            return NotFound();
        }

        if (todo.CreatorId != _currentUser.Id)
        {
            return StatusCode(HttpStatusCode.Forbidden); 
        }

        return Ok(todo);
    }

That is fine. Similar check added for delete, and on create it sets the CreatorId to the current user's id. However, I have a problem with updating.

I tried this:

    // PUT api/Todo/5
    public async Task<IHttpActionResult> PutTodo(int id, Todo todo)
    {
        if (!ModelState.IsValid)
        {
            return BadRequest(ModelState);
        }

        if (id != todo.Id)
        {
            return BadRequest();
        }


        // --- No exception if I remove this block - BEGIN ---
        var original = await _db.Todos.FindAsync(id);

        if (original.CreatorId != _currentUser.Id || original.CreatorId != todo.CreatorId)
        {
            return StatusCode(HttpStatusCode.Forbidden);
        }
        // --- No exception if I remove this block - END ---

        _db.Entry(todo).State = EntityState.Modified; // Exception thrown here

        try
        {
            await _db.SaveChangesAsync();
        }
        catch (DbUpdateConcurrencyException)
        {
            if (!TodoExists(id))
            {
                return NotFound();
            }
            else
            {
                throw;
            }
        }

        return StatusCode(HttpStatusCode.NoContent);
    }

However, a System.InvalidOperationException it thrown at the marked line:

System.InvalidOperationException

Attaching an entity of type 'ModernWeb.Domain.Models.Todo' failed because another entity of the same type already has the same primary key value. This can happen when using the 'Attach' method or setting the state of an entity to 'Unchanged' or 'Modified' if any entities in the graph have conflicting key values. This may be because some entities are new and have not yet received database-generated key values. In this case use the 'Add' method or the 'Added' entity state to track the graph and then set the state of non-new entities to 'Unchanged' or 'Modified' as appropriate.

If I remove the block with the FindByAsync(), it won't throw exception.

I also tried to use _db.Entry(todo).OriginalValue, but couldn't find a working syntax.

How can I overcome this problem? Any best practice for situations like this?

Accepted Answer

When you invoke FindAsync, the entity instance returned is already attached to the context. So there is no reason for _db.Entry(todo).State = EntityState.Modified;

Update

I think I see what you are trying to do here. Try this instead:

var original = await _db.Todos.AsNoTracking()
    .SingleOrDefaultAsync(x => x.Id == id);

if (original.CreatorId != _currentUser.Id || original.CreatorId != todo.CreatorId)
{
    return StatusCode(HttpStatusCode.Forbidden);
}
// --- No exception if I remove this block - END ---

_db.Entry(todo).State = EntityState.Modified;

When you invoke .AsNoTracking().SingleOrDefaultAsync instead of FindAsync, the original entity returned will not be attached to the context. Then you can set the one passed into the controller action as Modified, and since the context is not already tracking a different entity with the same id, you should not get that exception anymore.

As a secondary note, since the Todo entity passed into your argument already has an Id property, there shouldn't be a need to pass it in as a separate argument in the controller action. You should be able to do this:

public async Task<IHttpActionResult> PutTodo(Todo todo)
{
    if (!ModelState.IsValid || todo == null)
    {
        return BadRequest(ModelState);
    }

    var original = await _db.Todos.AsNoTracking()
        .SingleOrDefaultAsync(x => x.Id == todo.Id);

    if (original == null) return NotFound();

    if (original.CreatorId != _currentUser.Id || original.CreatorId != todo.CreatorId)
    {
        return StatusCode(HttpStatusCode.Forbidden);
    }

    _db.Entry(todo).State = EntityState.Modified; // Exception thrown here

    try
    {
        await _db.SaveChangesAsync();
    }
    catch (DbUpdateConcurrencyException)
    {
        if (!TodoExists(todo.Id))
        {
            return NotFound();
        }
        else
        {
            throw;
        }
    }

    return StatusCode(HttpStatusCode.NoContent);
}



Licensed under: CC-BY-SA with attribution
Not affiliated with Stack Overflow
Is this KB legal? Yes, learn why
Licensed under: CC-BY-SA with attribution
Not affiliated with Stack Overflow
Is this KB legal? Yes, learn why