2

When creating a scaffolded item (CRUD) Visual Studio creates multiple pages, I have a question in regards to the editing page. Here it creates the default layout to update your model which can be modified to meet your needs. The problem I see is that it creates a hidden input field for your Id. Isn't this a security issue since the input control can be edited? If this is edited, when you save by theory it would update a different item (hence a security issue)? Also What if I have a second field that shouldn't be edited for example "CreatedBy" should I just be creating another hidden field? If this field is also edited i will lose my original CreatedBy user.

Also if I remove these hidden input boxes to remove the security threat the issue I face is that the automatic validation will fail because it won't retain my Id or CreatedBy user on the model. This is also an issue when updating because the Id would also be lost. Whats the best and the proper way to handle this?

Below is a sample of the automatic code created by visual studio when you create a scaffolded item (CRUD):

...
<form method="post">
    <div asp-validation-summary="ModelOnly" class="text-danger"></div>
    <input type="hidden" asp-for="Test.Id" />
    <div class="form-group">
        <label asp-for="Test.Created" class="control-label"></label>
        <input asp-for="Test.Created" class="form-control" />
        <span asp-validation-for="Test.Created" class="text-danger"></span>
    </div>
    <div class="form-group">
        <label asp-for="Test.CreatedBy" class="control-label"></label>
        <input asp-for="Test.CreatedBy" class="form-control" />
        <span asp-validation-for="Test.CreatedBy" class="text-danger"></span>
    </div>
    <div class="form-group">
        <label asp-for="Test.Blahblah" class="control-label"></label>
        <input asp-for="Test.Blahblah" class="form-control" />
        <span asp-validation-for="Test.Blahblah" class="text-danger"></span>
    </div>
    <div class="form-group">
        <input type="submit" value="Save" class="btn btn-primary" />
    </div>
</form>
...

Anyhow I know this is something basic and I have been looking online for an answer to this but haven't been able to find one. I have found ways to check specific properties during validation, but this still will not ensure that I don't lose the Id and CreatedBy fields assuming I remove the hidden inputs.

It seems as if my only option is to have a security issue but I refuse to believe this is the correct method. Anyhow thank for the help!

Orlando
  • 935
  • 2
  • 20
  • 42
  • 2
    This is something that needs to be validated on the backend. You will need to write the logic that says "should the user be able to update this record?" and if so update it, otherwise tell the user what they're doing is not valid. – emagers Apr 29 '19 at 15:43
  • @EricMagers So I should remove the hidden input controls and validate it manually? If I do this how will I know which item I am updating since the binded model loses the value for the Id property? – Orlando Apr 29 '19 at 16:53
  • Leave the hidden input, but validate that the record exists and that the user should be able to update it on the backend. Trust, but verify. – emagers Apr 29 '19 at 17:32
  • 1
    @Lord-Link, You could keep the id as a hidden input, but no need to keep CreatedBy. you can fetch fields in backend if you don't need to update them. if you have concern about security maybe using AntoforgeryToken is the solution: https://stackoverflow.com/questions/44783702/what-is-the-use-of-html-antiforgerytoken – Saeid Apr 29 '19 at 17:32
  • Is adding the id to the url feasible in your case? If so, you can add the id as part of the action. If the user tries to change the id, they will be presented with a brand new page, circumventing the problem. – Shai Cohen Apr 29 '19 at 18:18
  • @ShaiCohen I am doing a post request so I wouldn't be able to pass it in the url – Orlando Apr 30 '19 at 12:52
  • Absolutely you can. Add it to both get and post actions and it will come through as a parameter. Would you like a code sample? – Shai Cohen Apr 30 '19 at 13:32
  • @ShaiCohen Even if it is possible I still don't think it would resolve this specific case since someone could change the urls parameters just as well as the hidden input field I would have the same result in the end. I'm not to sure how I would confirm that they didn't change the value as you mentioned. Maybe there is something I'm not understanding. – Orlando Apr 30 '19 at 13:40
  • 1
    If they change the querystring, they would need to make a whole new GET request for that that change to "stick". While the accepted answer is definitely best practice (I follow it religiously) you still haven't solved your original issue of the user changing values. Additionally you need to run the "is user allowed to change this" logic both on the GET and POST actions – Shai Cohen Apr 30 '19 at 14:01
  • @ShaiCohen I understand that they would have to make a whole new GET request for a change in the id to happen, however changing the hidden input this is also true. The accepted answer I selected it because reading into the articles it shows that you can omit a change to specific fields for example the CreatedBy therefore you wouldn't need to have a hidden input making it impossible for the user to change it. I understand that the Id field is still a hidden field since this is required to know what item you are updating but this is where you would verify if the user has the right to change it. – Orlando Apr 30 '19 at 15:02
  • @ShaiCohen So where i mentioned that the hidden input also requires another request is because my code is on a post back and not running client side code to process the change. Just wanted to clarify that. – Orlando Apr 30 '19 at 15:03
  • 1
    Not true. F12. Developer tools. I can change anything I want and it will be posted back using the same url. – Shai Cohen Apr 30 '19 at 15:04
  • @ShaiCohen Exactly I agree with you but that requires a post back which would mean a post request. I think I know what you mean though as for using the get parameter and then processing a post request you can still use the get parameter and verify it. – Orlando Apr 30 '19 at 15:12

1 Answers1

2

There are 2 different things:

  1. The Id to update.

    There is no way to prevent sending the Id to a server when updating a dataset, because HTTP is stateless. Using workarounds with things like Session or TempData might prevent that, but may introduce different problems for users that work with multiple tabs (e.g. user loads object A, then in new tab loads object B, then saves object A in tab 1, which causes B to be overwritten since that's the last Id loaded in the session).

    The solution here is validation and authorization (e.g. Resource-based authorization).

  1. Over-binding/"mass assignment"

    This is the issue when a client sends more information than it should (e.g. CreatedBy). The solution for this is either a dedicated ViewModel containing only the editable fields or using attributes like [Editable] and [BindNever]. That way the Modelbinding won't bind fields that should be read-only. Here is a blogpost explaining that issue.

    So the best way is to remove the hidden fields not used(e.g. CreatedBy) before sending to the client (unless you want to show it somewhere). Then to update the model you load the data from your database with the Id, update the posted editable properties (and other fields you want to set, e.g. a last updated date set in code) then save the model.

hiiru
  • 416
  • 3
  • 8