1

I have a model class that has an AreDuesPaid property that I want only administrators to be able to see and edit.

The class looks something like this:

public class ClubMember
{
    [ScaffoldColumn(false)]
    public int Id { get; set; }

    [Display(Name = "First Name")]
    [Required(ErrorMessage = "First name is required")]
    public string FirstName { get; set; }

    [Display(Name = "Last Name")]
    [Required(ErrorMessage = "Last name is required")]
    public string LastName { get; set; }

    [Display(Name = "Email Address")]
    [DataType(DataType.EmailAddress)]
    public string EmailAddress { get; set; }

    [DataType(DataType.PhoneNumber)]
    [Display(Name = "Phone Number")]
    public string PhoneNumber { get; set; }

    [Authorize(Roles="Administrator")] // error: this can only be used for methods
    public bool AreDuesPaid{ get; set; }
}

I thought maybe I could use the Authorize attribute, but the compiler tells me this is only for methods.

So, I'm wondering, how can I limit access to a particular property when using DisplayForModel() and EditorForModel() to auto-scaffold views?

Do I need to create entirely separate views and view models or is there an easier way?

devuxer
  • 41,681
  • 47
  • 180
  • 292

2 Answers2

1

It dosent make sense to make permission restrictions for properties. Instead you should check whether the user that is logged in is in the role "Administrator" when you render the view (make up some if statements in the view, so if the user is in role "Administrator" then show the value of AreDuesPaid, else hide it)

ebb
  • 9,297
  • 18
  • 72
  • 123
  • But I think this makes it impossible to use `DisplayForModel()` and `EditorForModel()`. – devuxer Apr 26 '11 at 20:24
  • 1
    I'd also be very careful about hiding things in the view, because a malicious user might still post data from your "protected" properties and have them written into your class before saving. Permissions do not belong in UI code. – Danny Tuppeny Apr 28 '11 at 15:34
  • @Danny, You'll have to explain that a bit more in detail. The HttpPost `ActionResult` is normally decorated with an `[Authorize]` attribute, or a custom one which allows you to specify which roles should be allowed to call the `ActionResult`. – ebb May 03 '11 at 18:22
  • @ebb You said to "hide" the field if the user is not an Admin. The non-Admin presumably can save the other fields. Hiding a field from the View does not stop a malicious user adding to the data POSTed back to the server, which would then be set in the Model passed in to the save Action. – Danny Tuppeny May 03 '11 at 19:01
  • @Danny, as this question was about where to handle the logic that determines whether a certain field should be visible for admins only. I assumed the OP was aware of how to configure an ActionResult for admins only, and another one for "regular" users. – ebb May 03 '11 at 19:44
  • @ebb The question mentioned editing, so just adding if statements to the view would leave security issues. I was just clarifying, so other people reading your comment don't go and implement things in an insecure way. – Danny Tuppeny May 03 '11 at 19:48
  • @Danny, You keep running in circles... As I said in my last comment if statements in a view would be the best (if not only) way to control what to show for specific user roles. Also as I mentioned in my last comment there's absolutely no security issues by using if statements in your view, as long you have created specific ActionResult that suits each model, and is decorated with proper attributes. – ebb May 03 '11 at 20:05
  • You asked me to explain my comment in more detail, so I was clarifying. Until I asked (and you posted a comment), your response could've been misinterpreted and built insecurely. I was adding information for other people reading. If you didn't want my explanation, why ask for it? ;) – Danny Tuppeny May 04 '11 at 05:58
  • @Danny, I have a question about your original comment...how could a malicious user without access to the source code know an admin-only property exists if it is never available in any view? In this particular example, how would a user know `AreDuesPaid` even exists if there is no mention of it in any html the user has access to? – devuxer May 10 '11 at 18:00
  • @DanM, Never trust your users - rule number 1. – ebb May 10 '11 at 21:49
  • @DanM This is known as "security by obscurity". Do you really want to bet your security on users not finding out? All it takes is someone to accidentally expose the property there, a .NET error message to reveal it, or someone to see on StackOverflow you have a property called AreDuesPaid that you didn't secure properly ;-) – Danny Tuppeny May 11 '11 at 06:00
  • @Danny...All good points (though my StackOverflow question was a simplified version of my real code). I already took some steps to secure my HTTP Post pages based on your original comment, but I was just curious what the extent of the risk was. Thanks for your help. – devuxer May 11 '11 at 06:38
1

Here is the solution I ended up going with:

  1. Use view models instead of models. So, in this case, I created a class called ViewModels.ClubMember.EditorModel.
  2. For any properties that are admin-only, place them in a separate class. In this case, ViewModels.ClubMember.AdminEditorModel
  3. Make AdminEditorModel a property on EditorModel.
  4. When requesting an EditorModel, the service that gets the model checks the credentials of the logged-in user. If the user is an admin, the AdminEditorModel property is populated, otherwise, it gets set to null.
  5. In the view, use EditorForModel() to render the EditorModel, then, if the AdminEditorModel is not null, use EditorFor(model => model.AdminEditorModel) to scaffold the remaining properties. (Note: AdminEditorModel will not be automatically scaffolded when doing EditorForModel() because the default object template specifically ignores complex properties.)

This took some work to implement, but so far, I'm finding it to be a fairly clean solution. It seems to work well with validation, too.

Update

Based on some of the comment on ebb's answer (especially Danny's), I decided to sure things up a little more by using completely separate pages for admins vs. regular users. Each page gets it's own view model (EditorModel and AdminEditorModel). This took even more work then my initial solution (I had to add new actions to my controller and add mappings between view models and models, but the end result is yet another level cleaner and definitely more secure.

devuxer
  • 41,681
  • 47
  • 180
  • 292