Skip to content
This repository was archived by the owner on Dec 9, 2022. It is now read-only.

Added ability to ignore security requirements on proxied controllers#43

Open
dbelcham wants to merge 1 commit intokristofferahl:masterfrom
dbelcham:master
Open

Added ability to ignore security requirements on proxied controllers#43
dbelcham wants to merge 1 commit intokristofferahl:masterfrom
dbelcham:master

Conversation

@dbelcham
Copy link
Copy Markdown

@dbelcham dbelcham commented Oct 9, 2012

Haven't had a huge amount of time to test this. Will try at work tomorrow. Looks like it will work though.

Fixes issue 42 #42

@chandu
Copy link
Copy Markdown
Collaborator

chandu commented Oct 13, 2012

I guess this needs more analysis. May be adding a feature to add a method like ForAllControllersImplementing() would be a better way to look at this issue, instead of just a proxy pattern. But again this introduces navigating the inheritance path issue and caching. @kristofferahl Your thoughts?

@kristofferahl
Copy link
Copy Markdown
Owner

Just had a quick look at this and I have to say I agree with @chandu on this. I'm not sure the proxy pattern is the way to go. It might seem like a small change but the ripple effects will be quite big in order for this to work at runtime and with testing.

If you're just interested in making it work at runtime you should be able to create your own ActionFilterAttribute based on https://github.com/kristofferahl/FluentSecurity/blob/develop/FluentSecurity/HandleSecurityAttribute.cs. In the OnActionExecuting method you could simply strip off the "Proxy" part of the controller name. I haven't tried this but it should work!?

When running your tests the controller names would not be ending with "Proxy" so setting expectations should work as usual. Does this sound like a way forward?

@kristofferahl
Copy link
Copy Markdown
Owner

Any progress on the suggested solution? If it works well I have an idea on how we could use this approach as a permanent solution to this issue. Let me know how things are moving along.

@rehoutm
Copy link
Copy Markdown

rehoutm commented Apr 24, 2013

Hello, is there any update how to solve the proxied controllers thing? Rewriting the handle security attribute seems not to be an option nowadays since a lot of stuff is now internal and thus cannot be simply rewriten....

@kristofferahl
Copy link
Copy Markdown
Owner

@rehoutm What parts are marked internal that you would need in order to make it work? I am open to making them public if there is a case where it is needed.

@rehoutm
Copy link
Copy Markdown

rehoutm commented Apr 30, 2013

@kristofferahl
I solved it by doing this - securityHandler = new SecurityHandler()
The internal part that made me to do it this way was ServiceLocator.Current.Resolve ( - ISecurityHandler - ) ()
(I don't know how to write angle brackets in comment :) )

I ended up doing it like that: https://gist.github.com/rehoutm/5488517#file-gistfile1-cs

@rehoutm
Copy link
Copy Markdown

rehoutm commented Apr 30, 2013

I think that there you could extract virtual method ParseControllerName from the OnAuthorization method, that would keep the internal stuff internal and let us do this kind of tweaks much easier...

@chandu
Copy link
Copy Markdown
Collaborator

chandu commented Apr 19, 2014

@rehoutm
FluentSecurity 2.0 has added support for securing controllers based on inheritance.
Is this and #42 still an issue for you?

@kristofferahl
Copy link
Copy Markdown
Owner

@chandu I think we need to give our users an easy way to handle this. It might not always be a case of inheritance. Perhaps registering a controller/action name modifyer or by creating an interface they can implement to have the controller/action names be resolved entirely by them. Something like:

public interface IControllerNameResolver
{
    string Resolve(Type controller);
}

public interface IActionNameResolver
{
   string Resolve(...);
}

What do you think?

@chandu
Copy link
Copy Markdown
Collaborator

chandu commented Apr 21, 2014

@kristofferahl I like the idea. This will make the action detection configurable and the framework can provide default resolvers (that matches how the names are resolved today).
Should this be a feature for next major/minor release?

@kristofferahl
Copy link
Copy Markdown
Owner

Yes, that is what I'm thinking. We should do the same for actually finding controllers and actions as well. However, I'm thinking it will be a big enough change that it will not be feasible to have it made before moving to 3.0.

@chandu
Copy link
Copy Markdown
Collaborator

chandu commented Apr 21, 2014

@kristofferahl Agreed.

@rehoutm
Copy link
Copy Markdown

rehoutm commented Apr 24, 2014

@chandu, @kristofferahl the inheritance itself does not solve the problem, because IIRC it is being parsed while registering the controllers (e.g. on App start), and afther that the resolver just uses the controller ful name and tries to match it against the data parsed before.

Which is not a solution, because the controllers proxied by Castle are not known on app start, and even worse, their name can change in runtime.

@rehoutm
Copy link
Copy Markdown

rehoutm commented Apr 24, 2014

The Controller name resolver is indeed a good idea, but not really sure about the action name resolver, I find it a bit useless, and in my current project, which is kinda large, I never ran into a problem with action name. The only behavirou I ever needed to alter was the controller name resolving part.

@kristofferahl
Copy link
Copy Markdown
Owner

Hey @rehoutm, I think we have solved the issue around proxied controllers. Can you send me an email at mail@fluentsecurity.net or ping us on twitter (@FluentSecurity) so we can send you some nuget packages to try. The fix is part of our work with FluentSecurity 3.0 so it's not yet available to the public but would love some feedback on it.

@kristofferahl
Copy link
Copy Markdown
Owner

@rehoutm Did you receive the email I sent on the 15th of May with the nuget packages I mentioned?

@rehoutm
Copy link
Copy Markdown

rehoutm commented May 20, 2014

Yes, thank you, didn't have the time to check it out yet, sorry...

On Tue, May 20, 2014 at 11:14 AM, Kristoffer Ahl
notifications@github.comwrote:

@rehoutm https://github.com/rehoutm Did you receive the email I sent on
the 15th of May with the nuget packages I mentioned?


Reply to this email directly or view it on GitHubhttps://github.com//pull/43#issuecomment-43603055
.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants