Limiting HTTP verbs on every action

Question

Is it a good practice to limit the available HTTP verbs for every action? My code is cleaner without [HttpGet], [HttpPost], [HttpPut], or [HttpDelete] decorating every action, but it might also be less robust or secure. I don't see this done in many tutorials or example code, unless the verb is explicitly required, like having two "Create" actions where the GET version returns a new form and the POST version inserts a new record.

Answer 1

Personally I try to respect RESTful conventions and specify the HTTP verb except for the GET actions which don't modify any state on the server thus allowing them to be invoked with any HTTP verb.

Answer 2

Yes, I believe it's a good practice to limit your actions only to the appropriate HTTP method it's supposed to handle, this will keep bad requests out of your system, reduce the effectiveness of possible attacks, improve the documentation of your code, enforce a RESTful design, etc.

Yes, using the [HttpGet], [HttpPost] .. attributes can make your code harder to read, specially if you also use other attributes like [OutputCache], [Authorize], etc.

I use a little trick with a custom IActionInvoker, instead of using attributes I prepend the HTTP method to the action method name, e.g.:

public class AccountController : Controller {

   protected override IActionInvoker CreateActionInvoker() {
      return new HttpMethodPrefixedActionInvoker();
   }

   public ActionResult GetLogOn() {
      ...
   }

   public ActionResult PostLogOn(LogOnModel model, string returnUrl) {
      ...
   }

   public ActionResult GetLogOff() {
      ...
   }

   public ActionResult GetRegister() {
      ...
   }

   public ActionResult PostRegister(RegisterModel model) {
      ...
   }

   [Authorize]
   public ActionResult GetChangePassword() {
      ...
   }

   [Authorize]
   public ActionResult PostChangePassword(ChangePasswordModel model) {
      ...
   }

   public ActionResult GetChangePasswordSuccess() {
      ...
   }
}

Note that this doesn't change the action names, which are still LogOn, LogOff, Register, etc.

Here's the code:

using System;
using System.Collections.Generic;
using System.Web.Mvc;

public class HttpMethodPrefixedActionInvoker : ControllerActionInvoker {

   protected override ActionDescriptor FindAction(ControllerContext controllerContext, ControllerDescriptor controllerDescriptor, string actionName) {

      var request = controllerContext.HttpContext.Request;

      string httpMethod = request.GetHttpMethodOverride()
         ?? request.HttpMethod;

      // Implicit support for HEAD method. 
      // Decorate action with [HttpGet] if HEAD support is not wanted (e.g. action has side effects)

      if (String.Equals(httpMethod, "HEAD", StringComparison.OrdinalIgnoreCase))
         httpMethod = "GET";

      string httpMethodAndActionName = httpMethod + actionName;

      ActionDescriptor adescr = base.FindAction(controllerContext, controllerDescriptor, httpMethodAndActionName);

      if (adescr != null)
         adescr = new ActionDescriptorWrapper(adescr, actionName);

      return adescr;
   }

   class ActionDescriptorWrapper : ActionDescriptor {

      readonly ActionDescriptor wrapped;
      readonly string realActionName;

      public override string ActionName {
         get { return realActionName; }
      }

      public override ControllerDescriptor ControllerDescriptor {
         get { return wrapped.ControllerDescriptor; }
      }

      public override string UniqueId {
         get { return wrapped.UniqueId; }
      }

      public ActionDescriptorWrapper(ActionDescriptor wrapped, string realActionName) {

         this.wrapped = wrapped;
         this.realActionName = realActionName;
      }

      public override object Execute(ControllerContext controllerContext, IDictionary<string, object> parameters) {
         return wrapped.Execute(controllerContext, parameters);
      }

      public override ParameterDescriptor[] GetParameters() {
         return wrapped.GetParameters();
      }

      public override object[] GetCustomAttributes(bool inherit) {
         return wrapped.GetCustomAttributes(inherit);
      }

      public override object[] GetCustomAttributes(Type attributeType, bool inherit) {
         return wrapped.GetCustomAttributes(attributeType, inherit);
      }

      public override bool Equals(object obj) {
         return wrapped.Equals(obj);
      }

      public override int GetHashCode() {
         return wrapped.GetHashCode();
      }

      public override ICollection<ActionSelector> GetSelectors() {
         return wrapped.GetSelectors();
      }

      public override bool IsDefined(Type attributeType, bool inherit) {
         return wrapped.IsDefined(attributeType, inherit);
      }

      public override string ToString() {
         return wrapped.ToString();
      }
   }
}