HiveBrain v1.2.0
Get Started
← Back to all entries
principlecsharpMinor

MVC4 approach to checking authorization after POST

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
aftermvc4authorizationcheckingpostapproach

Problem

I have a very simple form that is designed to update account expiration dates. I'm currently creating a View Model and sending that to the form, however, I still have to pass along a GUID so I know what object is being referenced on POST back.

What I'm worried about it is someone altering the guid (via JS, jQuery, etc..) to match an entity that they're not authorized for. After the form is posted, I'm checking to see if the user making the request matches the username of the entity that maps from the ViewModel (I realize at some point administrators and future permissions will need to be added, I'm keeping it simple for design-pattern sake).

Is this the proper way to do this, or is there a more efficient out of the box approach?

Entity:

public class FtpTrack
{
    [Key]
    public int id { get; set; }
    public string domainUserName { get; set; }
    public string ftpAccountName { get; set; }
    public DateTime expirationDate { get; set; }
}


ViewModel

public class FtpTrackViewModel
{
    public int guid { get; set; }
    public DateTime expirationDate { get; set; }
}


Controller

[HttpPost]
[ValidateAntiForgeryToken]
public ActionResult Edit(FtpTrackViewModel ftptrackVM)
{
    if (ModelState.IsValid)
    {
        FtpTrack ftptrack = db.FtpTracks.Find(ftptrackVM.guid);
        if (ftptrack.domainUserName == User.Identity.Name.Split('\\')[1])
        {
            ftptrack.expirationDate = ftptrackVM.expirationDate;
            db.Entry(ftptrack).State = EntityState.Modified;
            db.SaveChanges();
            return RedirectToAction("Index");
        }
    }
    return View(ftptrackVM);
}

Solution

Authorising actions should be done by your business layer or whatever you want to call it. Your web layer need only say this person is authenticated and is trying to perform this action:

public ActionResult Edit(FtpTrackViewModel ftptrackVM)
{
    if (ModelState.IsValid)
    {
        var requestingUserName = User.Identity.Name.Split('\\')[1]);
        someFtpTrackService.UpdateExpirationDate(ftptrackVM, requestingUserName);
        return RedirectToAction("Index");
    }
    return View(ftptrackVM);
}


Then it's the job of your service to check the user can do the action that the web layer is asking:

public class SomeFtpTrackService
{
    public SomeFtpTrackService(/* dependencies */)
    {
        // set up dependencies (assign to fields)
    }

    public void UpdateExpirationDate(FtpTrackViewModel ftpTrack, string userName) 
    {
        var track = db.FtpTracks.Find(ftptrackVM.guid);
        if (track.DomainUserName != userName) 
        {
            // throw new ... 
        }
        // ...
    }
}


Not that I've Pascal cased domainUserName to DomainUserName. I've treated user and name as separate words here (userName) but if you implement full authentication on the site, you might find you move towards an actual username (one word).

Having said all of the above, the chance of someone randomly changing a GUID and getting another entity is infinitesimally small.

Code Snippets

public ActionResult Edit(FtpTrackViewModel ftptrackVM)
{
    if (ModelState.IsValid)
    {
        var requestingUserName = User.Identity.Name.Split('\\')[1]);
        someFtpTrackService.UpdateExpirationDate(ftptrackVM, requestingUserName);
        return RedirectToAction("Index");
    }
    return View(ftptrackVM);
}
public class SomeFtpTrackService
{
    public SomeFtpTrackService(/* dependencies */)
    {
        // set up dependencies (assign to fields)
    }

    public void UpdateExpirationDate(FtpTrackViewModel ftpTrack, string userName) 
    {
        var track = db.FtpTracks.Find(ftptrackVM.guid);
        if (track.DomainUserName != userName) 
        {
            // throw new ... 
        }
        // ...
    }
}

Context

StackExchange Code Review Q#96536, answer score: 2

Revisions (0)

No revisions yet.