kristofferahl / FluentSecurity

Fluent Security configuration for ASP.NET MVC
MIT License
163 stars 47 forks source link

Action methods with void result can't be configured. #57

Closed michaelfagre closed 10 years ago

michaelfagre commented 11 years ago

It does not appear to be possible to set any policy for Actionmethods which have a void return signature.

FileController:

        [HttpPost]
        public void Delete(Guid attachmentId)
        {
            if (IsUploaded(attachmentId))
            {
                RemoveAttachment(attachmentId);
            }
        }

Configuration:

        configuration.For<FileController>()
            .Ignore();
        configuration.For<FileController>(x => x.Download(Guid.Empty))
            .DenyAnonymousAccess();
        configuration.For<FileController>(x => x.Upload(null, string.Empty, Guid.Empty))
            .DenyAnonymousAccess();
        configuration.For<UploaderController>(x => x.Delete(Guid.Empty))
            .DenyAnonymousAccess();

The configuration for Delete won't compile and displays an error of: "Cannot convert expression type 'void' to return type 'object'"

I tried to get around that by using ForActionsMatching:

        configuration.ForActionsMatching(x => x.ActionName == "Delete")
            .DenyAnonymousAccess();

That compiles but throws an "Security has not been configured for controller Web.Controllers.FileController, action Delete" at runtime when accessing the Action.

Am I doing anything wrong or isn't this implemented yet?

Edit: I'm using v2.0.0-alpha4

kristofferahl commented 11 years ago

@michaelfagre Still no progress on this I'm afraid. Been thinking about this and although it is perfectly valid to have a void return type for an action I would argue that returning an EmptyResult is a better solution and expresses intent better. I might be willing to accept a pull-request for this if you we're to send one but for the 2.0 release I doubt I will make this change.

chandu commented 11 years ago

Guys, Started working on this feature and wanted to get some feedback on what should be the default behavior.

Can you let me know your comments?

kristofferahl commented 11 years ago

@Chandu So sorry for not replying. Been busy, busy, busy.

From my point of view it should have the same behavior as other action methods.

1) There need to be an option to explicitly configure policies for void actions. 2) We also need to make sure that void actions are included in all the different scanning operations (ForAllControllers, ForAllControllersInAssembly, ForAll..., etc). Hopefully it should be enough to change GetActionMethods to also include void actions. 3) There might be a few other places in the configuration api where we also need to have an overload for void actions. So have a look at the classes in the Configuration namespace. 4) The same goes for the api in the TestHelper project.

Once again, big, big thanks @Chandu for supporting the project and helping us move it forward!

kristofferahl commented 10 years ago

This feature will be available in the final version of FluentSecurity 2.0. If you need it now there should be a nightly build package available for you by tomorrow (2.0.0-beta2-build248). https://www.myget.org/gallery/fluentsecurity Big thanks to @Chandu for taking this on!!!