59

Basically, I have a table which contains a few properties for a company. This is the "master" table and their ID is used in many other tables. I basically find their ID via this method:

private Company currentcompany()
    {
        Company cuco = db.Companies.Single(x => x.username == User.Identity.Name);
        return cuco;
    }

I need to give users the ability to update various details about themselves stored in this table, which I did perfectly well - however, I noticed a big security hole!

Using Tamper Data on Firefox (And I imagine Fidler/many others), I could easily change the hidden ID and modify another companies details.

To stop this, I added the following lines to the modify action:

        Company cuco = currentcompany();

        if (company.id != cuco.id)
        {
            return Content("Security Error");
        }

(FYI - Company is a model/POCO representing a company, and company itself is the form data.)

After adding this, if I edit the ID in the form data, it works as expected and brings up "Security Error", however, if there isn't an error and I go on, I get the error in the question.

"An object with the same key already exists in the ObjectStateManager. The ObjectStateManager cannot track multiple objects with the same key."

I believe this is because EF is somehow detecting and keeping the first data pull, but I am just un sure on how to correct it.

Any advice?

edit- --update--

If you can understand what I am trying to achieve, is there a better way of going around this?

Wil
  • 9,259
  • 11
  • 47
  • 80
  • FYI - I have read similar questions on the related bar, but cannot see a solution. :( – Wil May 17 '11 at 15:55

4 Answers4

150

If you load the entity from the context you cannot attach an entity with the same key again. The first entity is still kept in internal context cache and context can hold only one instance with given key value per type (it is called identity map and I described it here in other situation).

You can solve it by detaching former instance but you don't have to. If you only need to save new values you can use this:

  • ObjectContext API: context.YourEntitySet.ApplyCurrentValues(newEntity);
  • DbContext API: context.Entry(oldEntity).CurrentValues.SetValues(newEntity);
Community
  • 1
  • 1
Ladislav Mrnka
  • 349,807
  • 56
  • 643
  • 654
  • 6
    You rock... I think you have posted a good answer to every EF question I have had on this site! I am just getting a little fed up asking so many EF questions and feel like I should know better! Apart from the odd blog and questions here, I can't find many resources and just wondering if you can recommend anything that can help me? ... Also, for this actual problem, would you say that doing the anti forgery token would be enough / what would you do in this situation? I was always under the impression that anything from the client should be treated as – Wil May 17 '11 at 20:43
  • not trusted and My method of checking the DB and comparing ID's would work perfectly if it wasn't for this! (And, I am going to read up about those API's and see if I can do something in the morning. – Wil May 17 '11 at 20:43
  • Believe me, you don't ask so many questions. There are other who beat you ten times :) Forget about anti-forgery token. It is used for Cross site request forgery defence - I was wrong in this part. If you want to pass Id in hidden field you must secure it manually (encrypt/decrypt). – Ladislav Mrnka May 17 '11 at 20:52
  • Hmmm... Shame in real life apps it isn't as easy as the Movie database/ Music shop samples make it out to be!!! So, from the examples I gave in my question, would you drop the database checking all together and just go for an encrypted ID? (Otherwise, I think, if/when I get my head around these apis, it will probably be the best approach - as i said, it did appear to work at first.) – Wil May 17 '11 at 20:57
  • In general you should always check permission in your application. My initial idea was completely bad. – Ladislav Mrnka May 17 '11 at 21:01
  • Thanks - Ok... Will certainly be reading up about these APIs in the morning. Again, thanks so much for your help - I feel like Karma is working for me! The help I give on Super User.com is being returned on Stack Overflow! I just feel out of my depth on this and you have helped me so much... Many thanks again and will mark as answer in the morning after I have read up/learnt about these. – Wil May 17 '11 at 21:06
  • The second one, DbContext worked perfectly... I just wish I was better at this :( ... I have been looking but I can't see any EF4.1 books (guessing I will have to wait!)... I was just wondering will I miss much if I get a EF 4.0 book as I just want to learn as much as I can? – Wil May 18 '11 at 15:31
  • 1
    There is no EFv4.1 book. Check links in this answer for some resources: http://stackoverflow.com/questions/5974863/linq-for-entities-framework-4-1-tutorials-ebooks-links/5974973#5974973 There are several very good EFv4 books but the problem is that EFv4.1 offers very different API - DbContext whereas EFv4 is using ObjectContext API. – Ladislav Mrnka May 18 '11 at 15:36
  • Well, as long as you (and others!) don't mind answering questions, if EF 4.1's ObjectContext is significantly different, do you think it will be a waste of time getting any EF 4 book, or would it give me a head start/teach me the basics? – Wil May 18 '11 at 15:47
  • I ran into the same brick wall until I found this answer. Does this work even if the oldEntity is not currently in the context? – BonyT Jun 20 '12 at 11:07
  • @BonyT: If the oldEntity is not in the context you should not get this exception because you don't have entity with the same key in the context's cache. – Ladislav Mrnka Jun 20 '12 at 12:01
  • But that means you have to know if the entity IS or ISN'T already in the cache - unless you use the above exception to control flow - and if MS start advocating going down that route for anything I'm switching to Java! – BonyT Jun 20 '12 at 12:12
  • @BonyT: You should always know it because your code creates that context and use it to get entities. And to make it clear - I'm not MS representative. – Ladislav Mrnka Jun 20 '12 at 12:15
  • Lol - I'm having big issues getting EF to work as you can see from my other question. – BonyT Jun 20 '12 at 12:37
  • @LadislavMrnka So how I get my `oldEntity`? – mhesabi Mar 31 '14 at 11:12
5

Just a bit of help for you if you don't know how to find the oldEntity as according to Ladislav:

var entityKey = context.NewEntitySet.Create().GetType().GetProperty("Id").GetValue(newEntity);

factory.Entry(context.Set<NewEntityType>().Find(entityKey)).CurrentValues.SetValues(newEntity);
Serj Sagan
  • 24,625
  • 17
  • 133
  • 166
1

As the error clearly states - you need to make sure to get existing item and modify that item data with updated information and save it. You will not run into this issue if you update very specific information and not key information such as ID and ForeignKey ID
Below code should do the job!

 public virtual void Update(TEntity entity)
    {
        _context.Entry(entity).State = EntityState.Modified;

        this._context.SaveChanges();
    }

Usage will be -

 public async Task<bool> UpdateVendorItem(string userId, Vendor modified)
    {

        try
        {
            var existing = await this.GetVendors().SingleOrDefaultAsync(a => a.Id == modified.Id);


            //Set updated info
            existing.VendorName = modified.VendorName;

            //Update address information
            existing.Address.AddressLine1 = modified.Address.AddressLine1;
            ... 

            await _vendorRepository.UpdateAsync(existing);

    ...
sandeep talabathula
  • 3,008
  • 3
  • 26
  • 37
0

Just saw this issue yesterday. You need to detach cuco before the update. Check out the solution in this answer

Community
  • 1
  • 1
Steve Mallory
  • 4,059
  • 1
  • 25
  • 31