kristofferahl / FluentSecurity

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

Global filters for controllers #78

Open hernangm opened 10 years ago

hernangm commented 10 years ago

It would be handy to have a way to exclude controllers globally so no policies would be applied to them when using methods such as ForAllControllersInNamespaceContainingType, ForActionsMatching, ForAllControllersInheriting.

The syntax would be something close to:

configuration.ExcludeControllers(controllerType => controllerType.Name.StartsWith("Util_"));

In my case, I am using T4MVC, a very useful utility that enables you to replace "magic strings" route references to strongly-typed ones. To achieve this, T4MVC creates new helper controllers that derives from existing ones, but that cannot be reached through a browser and should not be secured either. In this case, it would be great to have a way to exclude them globally.

kristofferahl commented 10 years ago

I get your point and it might be a useful feature. However, I would argue that you should be explicit with your security configuration. So instead of having them excluded, I would apply an IgnorePolicy to them instead.

You could use ForActionsMatching to do this:

SecurityConfigurator.Configure(configuration =>
{
    configuration.ForActionsMatching(action => action.ControllerType.Name.StartsWith("Util_"))
                    .Ignore();
});

Is this good enough for you?

hernangm commented 10 years ago

I totally agree with you about being explicit with security.

I may be dealing with an extreme use case due to T4MVC creating new controllers that derive from real ones to do its "magic". I found one real problem as these T4MVC controllers derive from non-abstract controllers (the application "real" controllers) and somehow this resulted in getting an extra policy added to every "real" controller's action.

I can provide you with test cases if you are interested. Let me know if I am not being clear.

In case you decide to add this feature, I can fork the repo and create it.

hernangm commented 10 years ago

I've kept thinking about this, and this

SecurityConfigurator.Configure(configuration =>
{
    configuration.ForActionsMatching(action => action.ControllerType.Name.StartsWith("Util_"))
                    .Ignore();
});

would be as explicit as this:

SecurityConfigurator.Configure(configuration =>
{
     configuration.ExcludeControllers(controllerType => controllerType.Name.StartsWith("Util_"));
});

with the sutile difference that the first one adds the ignore policy to the controllers, while the second one removes the controllers in the scan.

The first option is perfectly fine for most cases, however when dealing with base and derived controllers, one might want to have the controllers excluded at the scan level to avoid some issues.

I think this feature is worth considering.

kristofferahl commented 10 years ago

Currently we use an AssemblyScanner to find controllers and actions (internally) and it already has partial support for excluding stuff. It might be possible to simply expose that to let you do what you want. I'll think about it some more and have a look at it. Thanks for the input!

hernangm commented 10 years ago

Yes, I've been diving in the source code, and it seems pretty straight forward as most of the functionality is there. Take ur time to think about this.

kristofferahl commented 10 years ago

Reading through the issue again I think you have a valid point about actually having the option to exclude types when scanning. I'm still a bit unsure of how I would like this feature to be implemented though.

@hernangm Would you be interested in having a look at it and propose an implementation and maybe send a pull-request? If so, I would suggest we continue to use this issue to discuss the implementation.

hernangm commented 10 years ago

Yes, I would be happy to contribute with this. I can't promise I will have it ready soon, though, but I have added this to my to-do list.

kristofferahl commented 10 years ago

Great! Take your time!! Let me and @Chandu know if we can help in any way and please post your ideas as early as possible.