开发者

best practice for authorization code repeating in a controller

I have a controller that has method GetSignatories(), AddMe(), RemoveMe(), AddUser(), RemoveUser() and more to come in witch I have to verify every time if the contract is existing, if the user have access to the contract and if the requested version is existing. I'm wondering where should I put this code? In a Service or extracted in an other method of my controller? my problem is that I soometime return Unathorized or NotFound and don't know what would be the best way to do this.

Here's my controller :

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);
        }
}

For those who might be looking how to register the model binder in the global :

public static void RegisterModelBinders()
    {
        var session = (ISession)DependencyResolver.Current.GetService(typeof(ISession));
        var authService = (IAuthenticationService)DependencyResolver.Current.GetService(typeof(IAut开发者_运维问答henticationService));
        System.Web.Mvc.ModelBinders.Binders[typeof (Contract)] = new ContractModelBinder(session, authService);
    }


Indeed you have pretty repeating code. There are many ways to refactor this code, one of which consists of writing a custom model binder for the Contract model:

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;
    }
}

Once you have registered this model binder with the Contract model in Application_Start your controller simply becomes:

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);
    }
}

We successfully put it on a diet.


One possible solution would be to create an IContractService interface that has two methods, one to get the contract and another to validate it:

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

ValidationResult could be an enumeration, just to signal the caller of the method how to proceed:

public enum ValidationResult
{
    Valid,
    Unauthorized,
    NotFound
}

A possible Implementation of 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;
    }
}

Then in your controller you can still return the various HttpNotFound, etc to the view:

[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());
}
0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜