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);
}
1 Answer 1
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.
-
\$\begingroup\$ Thanks, Robh - this makes sense. I'm sure I'm being overly paranoid on multiple levels, as I'm trying to authorize before even sending the request to the service in an already unlikely scenario. However, that's why I posted here, I'm curious what others do when it comes to security. \$\endgroup\$Jonathan H– Jonathan H2015年07月13日 14:01:06 +00:00Commented Jul 13, 2015 at 14:01
AllowAnonymous
attribute nor theAuthorize
attribute. \$\endgroup\$