2011-04-09 51 views
4

我有一個控制器,它具有方法GetSignatories(),AddMe(),RemoveMe(),AddUser(),RemoveUser()等等,以便我必須每次驗證如果合同存在,用戶是否有權訪問合同以及是否存在請求的版本。我想知道我應該在哪裏放這個代碼?在一個服務中或者用我的控制器的其他方法提取?我的問題是,我soometime返回Unathorized或NotFound,不知道什麼是最好的方式來做到這一點。在控制器中重複授權代碼的最佳做法

這裏是我的控制器:

public partial class ContractController : Controller 
    { 

     private ISession _session; 
     private IAuthenticationService _authService; 

     public V1Controller(ISession session, IAuthenticationService authService) 
     { 
      _session = session; 
      _authService = authService; 
     } 

     [HttpPost] 
     [Authorize(Roles = "User")] 
     [ValidateAntiForgeryToken] 
     public virtual ActionResult GetSignatories(string contractId, int version) 
     { 
      //NOTE Should be extracted 
      var contract = _session.Single<Contract>(contractId); 
      if (contract == null) return HttpNotFound(); 
      var user = _authService.LoggedUser(); 
      if (contract.CreatedBy == null || !contract.CreatedBy.Id.HasValue || contract.CreatedBy.Id.Value != user.Id) return new HttpUnauthorizedResult(); 
      if (contract.Versions.Count < version) return HttpNotFound(); 
      /*NOTE END Should be extracted */ 

      return Json(contract.CurrentVersion.Tokens.Select(x => x.User).ToList()); 
     } 

     [HttpPost] 
     [Authorize(Roles = "User")] 
     [ValidateAntiForgeryToken] 
     public virtual ActionResult AddMe(string contractId, int version) 
     { 
      //NOTE Should be extracted 
      var contract = _session.Single<Contract>(contractId); 
      if (contract == null) return HttpNotFound(); 
      var user = _authService.LoggedUser(); 
      if (contract.CreatedBy == null || !contract.CreatedBy.Id.HasValue || contract.CreatedBy.Id.Value != user.Id) return new HttpUnauthorizedResult(); 
      if (contract.Versions.Count < version) return HttpNotFound(); 
      /*NOTE END Should be extracted */ 

      return AddUserToContract(contract, new UserSummary(user)); 
     } 

     [HttpPost] 
     [Authorize(Roles = "User")] 
     [ValidateAntiForgeryToken] 
     public virtual ActionResult RemoveMe(string contractId, int version) 
     { 
      //NOTE Should be extracted 
      var contract = _session.Single<Contract>(contractId); 
      if (contract == null) return HttpNotFound(); 
      var user = _authService.LoggedUser(); 
      if (contract.CreatedBy == null || !contract.CreatedBy.Id.HasValue || contract.CreatedBy.Id.Value != user.Id) return new HttpUnauthorizedResult(); 
      if (contract.Versions.Count < version) return HttpNotFound(); 
      /*NOTE END Should be extracted */ 

      return RemoveUserFromContract(contract, new UserSummary(user)); 
     } 

     [HttpPost] 
     [Authorize(Roles = "User")] 
     [ValidateAntiForgeryToken] 
     public virtual ActionResult AddUser(string contractId, int version, UserSummary userSummary) 
     { 
      //NOTE Should be extracted 
      var contract = _session.Single<Contract>(contractId); 
      if (contract == null) return HttpNotFound(); 
      var user = _authService.LoggedUser(); 
      if (contract.CreatedBy == null || !contract.CreatedBy.Id.HasValue || contract.CreatedBy.Id.Value != user.Id) return new HttpUnauthorizedResult(); 
      if (contract.Versions.Count < version) return HttpNotFound(); 
      /*NOTE END Should be extracted */ 


      return AddUserToContract(contract, userSummary); 
     } 

     [HttpPost] 
     [Authorize(Roles = "User")] 
     [ValidateAntiForgeryToken] 
     public virtual ActionResult RemoveUser(string contractId, int version, UserSummary userSummary) 
     { 
      //NOTE Should be extracted 
      var contract = _session.Single<Contract>(contractId); 
      if (contract == null) return HttpNotFound(); 
      var user = _authService.LoggedUser(); 
      if (contract.CreatedBy == null || !contract.CreatedBy.Id.HasValue || contract.CreatedBy.Id.Value != user.Id) return new HttpUnauthorizedResult(); 
      if (contract.Versions.Count < version) return HttpNotFound(); 
      /*NOTE END Should be extracted */ 


      return RemoveUserFromContract(contract, userSummary); 
     } 
} 

對於那些誰可能會尋求如何在全球註冊模型綁定:

public static void RegisterModelBinders() 
    { 
     var session = (ISession)DependencyResolver.Current.GetService(typeof(ISession)); 
     var authService = (IAuthenticationService)DependencyResolver.Current.GetService(typeof(IAuthenticationService)); 
     System.Web.Mvc.ModelBinders.Binders[typeof (Contract)] = new ContractModelBinder(session, authService); 
    } 
+0

對於那些希望在這裏ModelBinder的如何進行單元測試是一個良好的開端:http://stackoverflow.com/questions/1992629/unit-testing-custom-model-binder-in-asp-net-mvc-2 – VinnyG 2011-04-09 18:57:06

回答

2

你的確有相當的重複代碼。有許多方法來重構這個代碼,其中一個由編寫自定義模型綁定的Contract型號:

public class ContractModelBinder : DefaultModelBinder 
{ 
    private readonly ISession _session; 
    private readonly IAuthenticationService _authService; 
    public ContractModelBinder(ISession session, IAuthenticationService authService) 
    { 
     _session = session; 
     _authService = authService; 
    } 

    public override object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext) 
    { 
     string contractId = null; 
     int version = 0; 
     var contractIdValue = bindingContext.ValueProvider.GetValue("contractId"); 
     var versionValue = bindingContext.ValueProvider.GetValue("version"); 
     if (versionValue != null) 
     { 
      version = int.Parse(versionValue.AttemptedValue); 
     } 
     if (contractIdValue != null) 
     { 
      contractId = contractIdValue.AttemptedValue; 
     } 

     var contract = _session.Single<Contract>(contractId); 
     if (contract == null) 
     { 
      throw new HttpException(404, "Not found"); 
     } 
     var user = _authService.LoggedUser(); 
     if (contract.CreatedBy == null || 
      !contract.CreatedBy.Id.HasValue || 
      contract.CreatedBy.Id.Value != user.Id 
     ) 
     { 
      throw new HttpException(401, "Unauthorized"); 
     } 

     if (contract.Versions.Count < version) 
     { 
      throw new HttpException(404, "Not found"); 
     } 
     return contract; 
    } 
} 

一旦在Application_Start註冊這個模型綁定與Contract模式控制器簡單地變爲:

public class ContractController : Controller 
{ 
    [HttpPost] 
    [Authorize(Roles = "User")] 
    [ValidateAntiForgeryToken] 
    public ActionResult GetSignatories(Contract contract) 
    { 
     return Json(contract.CurrentVersion.Tokens.Select(x => x.User).ToList()); 
    } 

    [HttpPost] 
    [Authorize(Roles = "User")] 
    [ValidateAntiForgeryToken] 
    public ActionResult AddMe(Contract contract) 
    { 
     var user = contract.CreatedBy; 
     return AddUserToContract(contract, new UserSummary(user)); 
    } 

    [HttpPost] 
    [Authorize(Roles = "User")] 
    [ValidateAntiForgeryToken] 
    public ActionResult RemoveMe(Contract contract) 
    { 
     var user = contract.CreatedBy; 
     return RemoveUserFromContract(contract, new UserSummary(user)); 
    } 

    [HttpPost] 
    [Authorize(Roles = "User")] 
    [ValidateAntiForgeryToken] 
    public ActionResult AddUser(Contract contract, UserSummary userSummary) 
    { 
     return AddUserToContract(contract, userSummary); 
    } 

    [HttpPost] 
    [Authorize(Roles = "User")] 
    [ValidateAntiForgeryToken] 
    public ActionResult RemoveUser(Contract contract, UserSummary userSummary) 
    { 
     return RemoveUserFromContract(contract, userSummary); 
    } 
} 

我們成功地put it on a diet

+1

確實很短的代碼,但是你確定模型綁定器是否適合這個?畢竟有很多業務邏輯。把它放在一個服務類似乎是一種更常見的方法... – 2011-04-09 16:11:50

+0

@Adrian Grigore,嚴格來說是0的商業邏輯。這是授權邏輯,實際上授權過濾器可能是另一種可能的解決方案,但模型綁定器的優點是您可以在控制器操作中獲得結果,以便您可以直接使用它們。如果您使用服務,那麼您仍然會在這些操作中擁有大量重複邏輯,並且它們不會太乾。 – 2011-04-09 16:13:48

+0

它似乎是最好的解決方案,你將如何處理HttpException?項目文件夾中的活頁夾在哪裏? – VinnyG 2011-04-09 16:14:19

2

一個可能的解決方案是創建一個IContractService接口,有兩種方法,一種獲得合同,另一個對其進行驗證:

public IContractService 
{ 
    Contract GetContract(int id); 
    ValidationResult ValidateContract(Contract contract); 
} 

ValidationResult可能是一個枚舉,只是信號的呼叫者方法如何進行:

public enum ValidationResult 
{ 
    Valid, 
    Unauthorized, 
    NotFound 
} 

一種可能實現的IContractService

public class ContractService : IContractService 
{ 
    private readonly ISession _session; 
    private readonly IAuthenticationService _authService; 

    // Use Dependency Injection for this! 
    public ContractService(ISession session, IAuthenticationService authService) 
    { 
     _session = session; 
     _authService = authService; 
    } 

    public Contract GetContract(int id) 
    { 
     var contract = _session.Single<Contract>(contractId); 

     // hanlde somwhere else whether it's null or not 
     return contract; 
    } 

    public ValidationResult ValidateContract(Contract contract) 
    { 
     var user = _authService.LoggedUser(); 
     if (contract.CreatedBy == null || !contract.CreatedBy.Id.HasValue || 
      contract.CreatedBy.Id.Value != user.Id) 
       return ValidationResult.Unauthorized; 

     if (contract.Versions.Count < version) 
      return ValidationResult.NotFound; 

     return ValidationResult.Valid; 
    } 
} 

然後在您的控制器仍然可以返回各種HttpNotFound等的觀點:

[HttpPost, Authorize(Roles = "User"), ValidateAntiForgeryToken] 
public virtual ActionResult GetSignatories(string contractId, int version) 
{ 
    //NOTE Should be extracted 
    var contract = _contractService.GetContract(contractId); 

    if (contract == null) 
     return HttpNotFound(); 

    var result = _contractService.ValidateContract(contract); 

    if (result == ValidationResult.Unauthorized) 
     return new HttpUnauthorizedResult(); 

    if (result == ValidationResult.NotFound) 
     return HttpNotFound(); 

    return Json(contract.CurrentVersion.Tokens.Select(x => x.User).ToList()); 
} 
+1

你會如何在我的控制器中執行GetSignatories()方法?如果我明白了,該服務只會取代最後3行,而我的控制器中會有儘可能多的代碼? – VinnyG 2011-04-09 16:24:50

+0

實際上這是正確的。這主要將驗證邏輯移出控制器,它不屬於該控制器。將ActionResult'返回到視圖仍然是控制器的工作。我會更新我的答案,以包含「GetSignatoires」操作。 – 2011-04-09 16:32:30

+0

我認爲這兩種解決方案都很好,但我更喜歡Darin,因爲我的代碼較少。感謝您的幫助! – VinnyG 2011-04-09 17:10:05