You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by Don Brown <mr...@twdata.org> on 2007/12/09 13:30:05 UTC
[s2] Allowed methods next step
Since the commit for this feature involved a rather large XWork change
(properly immutable configuration objects [1]), I decided to commit
what I have and discuss the next steps.
First, due the aforementioned fix [1], Brian, your SmartURL's
migration work will probably be most affected. I changed the
configuration objects to be immutable using a static inner builder
class pattern. This makes construction a bit tricker, so pay
attention to the changes in the code and tests for the codebehind
plugin. The bright side is the construction code is much more
readable and nasty state bugs should be gone. You can do nifty things
like this:
ActionConfig config = new ActionConfig.Builder("mypackage", "foo/*/*",
"foo.BarAction")
.methodName("execute")
.addParam("someparam", "someval")
.addResultConfig(new
ResultConfig.Builder("success{1}", "foo.MyResult")
.addParams("location", "/foo.jsp")
.build())
.build();
As for the allowed methods, I originally suggested three options:
1. A new property/constant titled 'struts.restrictToDeclaredMethod'
that will instruct the ActionConfig (where the allowedMethods property
lives) to only allow the method that is explicitly defined (defaults
to 'execute'). If false, all methods will be allowed.
2. A new attribute on the <action> element called 'allowedMethods',
which takes a comma-separated list of method names to allow
3. A new @ActionMethod annotation for the codebehind plugin that
declares a method as callable
And after the comments, I see #2 is important and #3 I'll skip, since
Brian will be rewriting that stuff anyways.
To answer Matt's concern, yes, the default will be all public, no-arg
methods can be called, but what this will allow folks to do is limit
the methods that can be called, if they so choose. It also makes it
clearer to the developer what methods are being exposed through tools
like the config browser plugin. I'm also thinking it will be helpful
down the road when a plugin wants to move behind no-arg methods (I've
tried it, it can be pretty powerful).
See https://issues.apache.org/struts/browse/WW-2363
Any more thoughts?
Don
[1] http://jira.opensymphony.com/browse/XW-594
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: [s2] Allowed methods next step
Posted by Don Brown <mr...@twdata.org>.
On 12/11/07, Brian Pontarelli <br...@pontarelli.com> wrote:
> At first I thought this might be a problem because SmartURLs was
> sub-classing the ActionConfig object in order to add some additional
> information for performance reasons. However, I have a feeling that I
> can remove the sub-class. All the same, I think your change still allows
> sub-classing. Now I just need to figure out if I want to remove my
> sub-class or not :)
Yeah, try to do it without the subclass, and feel free to add more
builder methods as needed.
> I've got some work done on the new convention plugin and put together a
> short design doc for it because I kept getting lost in the soup of
> conventions and configuration overrides. Here's the URL for the design doc:
>
> http://svn.apache.org/viewvc/struts/sandbox/trunk/struts2-convention-plugin/design.txt?view=markup
Yep, looks fine to me.
> With respect to allowed methods, the new plugin only generates
> configuration for the execute method and any other method that is
> annotated. So, with the new allowedMethods property inside the
> ActionConfig, it should be a snap to just ensure that when the plugin is
> constructing the ActionConfig instances it locks down the Action
> accordingly.
Cool. To be honest, I'm thinking that maybe the feature would be
better simply as an Interceptor, and therefore, I might end up ripping
it out this weekend. Doing it as an interceptor would certainly be
the most flexible and consistent with how other features work.
Don
>
> -bp
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: [s2] Allowed methods next step
Posted by Brian Pontarelli <br...@pontarelli.com>.
Don Brown wrote:
> Since the commit for this feature involved a rather large XWork change
> (properly immutable configuration objects [1]), I decided to commit
> what I have and discuss the next steps.
>
> First, due the aforementioned fix [1], Brian, your SmartURL's
> migration work will probably be most affected. I changed the
> configuration objects to be immutable using a static inner builder
> class pattern. This makes construction a bit tricker, so pay
> attention to the changes in the code and tests for the codebehind
> plugin. The bright side is the construction code is much more
> readable and nasty state bugs should be gone. You can do nifty things
> like this:
>
> ActionConfig config = new ActionConfig.Builder("mypackage", "foo/*/*",
> "foo.BarAction")
> .methodName("execute")
> .addParam("someparam", "someval")
> .addResultConfig(new
> ResultConfig.Builder("success{1}", "foo.MyResult")
> .addParams("location", "/foo.jsp")
> .build())
> .build();
>
At first I thought this might be a problem because SmartURLs was
sub-classing the ActionConfig object in order to add some additional
information for performance reasons. However, I have a feeling that I
can remove the sub-class. All the same, I think your change still allows
sub-classing. Now I just need to figure out if I want to remove my
sub-class or not :)
> As for the allowed methods, I originally suggested three options:
>
> 1. A new property/constant titled 'struts.restrictToDeclaredMethod'
> that will instruct the ActionConfig (where the allowedMethods property
> lives) to only allow the method that is explicitly defined (defaults
> to 'execute'). If false, all methods will be allowed.
>
> 2. A new attribute on the <action> element called 'allowedMethods',
> which takes a comma-separated list of method names to allow
>
> 3. A new @ActionMethod annotation for the codebehind plugin that
> declares a method as callable
>
> And after the comments, I see #2 is important and #3 I'll skip, since
> Brian will be rewriting that stuff anyways.
>
I've got some work done on the new convention plugin and put together a
short design doc for it because I kept getting lost in the soup of
conventions and configuration overrides. Here's the URL for the design doc:
http://svn.apache.org/viewvc/struts/sandbox/trunk/struts2-convention-plugin/design.txt?view=markup
With respect to allowed methods, the new plugin only generates
configuration for the execute method and any other method that is
annotated. So, with the new allowedMethods property inside the
ActionConfig, it should be a snap to just ensure that when the plugin is
constructing the ActionConfig instances it locks down the Action
accordingly.
-bp
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: [s2] Allowed methods next step
Posted by Matt Raible <ma...@raibledesigns.com>.
On Dec 9, 2007 9:44 AM, Tom Schneider <sc...@gmail.com> wrote:
> CMA = container managed authentication for those who haven't memorized
> every three letter acronym under the sun.
>
> What about using an s2 interceptor to enforce role security?
This is what I've done in the past. Yes, it works. However, I think
this is something we should ship out-of-the-box to make it easier for
newbies. The interceptor can use request.isUserInRole() and if you're
using a non-standard security mechanism, you can create a filter and
requestwrapper that allows this to work for your security framework.
This is what Acegi/Spring Security and many others do and I think it's
the best solution.
Matt
> That way
> you could have an implementation for whatever security mechanism your
> using and it's not tied to the struts configuration. I suppose we could
> still have a place to store role metadata in the configuration, but I
> wouldn't want the specific security enforcement logic to be tied to the
> s2 itself.
> Tom
>
>
> Matt Raible wrote:
> > Is there anything on <action> that allows restricting roles? I
> > remember this being in S1 and I'm unsure if it exists in S2. If not,
> > it's something I believe we should add as I believe it's useful when
> > using CMA. When using Spring Security, I generally keep all my
> > configuration in my context file, but I can see why it's useful when
> > using CMA. If it is an attribute, I suppose having separate <action>
> > definitions is the best way to allow different roles for different
> > methods? If not, I could see some sort of "method:role1/role2"
> > shortcut being useful - but it might also make things more
> > complicated.
> >
> > Matt
> >
> > On Dec 9, 2007 5:30 AM, Don Brown <mr...@twdata.org> wrote:
> >
> >> Since the commit for this feature involved a rather large XWork change
> >> (properly immutable configuration objects [1]), I decided to commit
> >> what I have and discuss the next steps.
> >>
> >> First, due the aforementioned fix [1], Brian, your SmartURL's
> >> migration work will probably be most affected. I changed the
> >> configuration objects to be immutable using a static inner builder
> >> class pattern. This makes construction a bit tricker, so pay
> >> attention to the changes in the code and tests for the codebehind
> >> plugin. The bright side is the construction code is much more
> >> readable and nasty state bugs should be gone. You can do nifty things
> >> like this:
> >>
> >> ActionConfig config = new ActionConfig.Builder("mypackage", "foo/*/*",
> >> "foo.BarAction")
> >> .methodName("execute")
> >> .addParam("someparam", "someval")
> >> .addResultConfig(new
> >> ResultConfig.Builder("success{1}", "foo.MyResult")
> >> .addParams("location", "/foo.jsp")
> >> .build())
> >> .build();
> >>
> >> As for the allowed methods, I originally suggested three options:
> >>
> >> 1. A new property/constant titled 'struts.restrictToDeclaredMethod'
> >> that will instruct the ActionConfig (where the allowedMethods property
> >> lives) to only allow the method that is explicitly defined (defaults
> >> to 'execute'). If false, all methods will be allowed.
> >>
> >> 2. A new attribute on the <action> element called 'allowedMethods',
> >> which takes a comma-separated list of method names to allow
> >>
> >> 3. A new @ActionMethod annotation for the codebehind plugin that
> >> declares a method as callable
> >>
> >> And after the comments, I see #2 is important and #3 I'll skip, since
> >> Brian will be rewriting that stuff anyways.
> >>
> >> To answer Matt's concern, yes, the default will be all public, no-arg
> >> methods can be called, but what this will allow folks to do is limit
> >> the methods that can be called, if they so choose. It also makes it
> >> clearer to the developer what methods are being exposed through tools
> >> like the config browser plugin. I'm also thinking it will be helpful
> >> down the road when a plugin wants to move behind no-arg methods (I've
> >> tried it, it can be pretty powerful).
> >>
> >> See https://issues.apache.org/struts/browse/WW-2363
> >>
> >> Any more thoughts?
> >>
> >> Don
> >>
> >> [1] http://jira.opensymphony.com/browse/XW-594
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> >> For additional commands, e-mail: dev-help@struts.apache.org
> >>
> >>
> >>
> >
> >
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
--
http://raibledesigns.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: [s2] Allowed methods next step
Posted by Tom Schneider <sc...@gmail.com>.
CMA = container managed authentication for those who haven't memorized
every three letter acronym under the sun.
What about using an s2 interceptor to enforce role security? That way
you could have an implementation for whatever security mechanism your
using and it's not tied to the struts configuration. I suppose we could
still have a place to store role metadata in the configuration, but I
wouldn't want the specific security enforcement logic to be tied to the
s2 itself.
Tom
Matt Raible wrote:
> Is there anything on <action> that allows restricting roles? I
> remember this being in S1 and I'm unsure if it exists in S2. If not,
> it's something I believe we should add as I believe it's useful when
> using CMA. When using Spring Security, I generally keep all my
> configuration in my context file, but I can see why it's useful when
> using CMA. If it is an attribute, I suppose having separate <action>
> definitions is the best way to allow different roles for different
> methods? If not, I could see some sort of "method:role1/role2"
> shortcut being useful - but it might also make things more
> complicated.
>
> Matt
>
> On Dec 9, 2007 5:30 AM, Don Brown <mr...@twdata.org> wrote:
>
>> Since the commit for this feature involved a rather large XWork change
>> (properly immutable configuration objects [1]), I decided to commit
>> what I have and discuss the next steps.
>>
>> First, due the aforementioned fix [1], Brian, your SmartURL's
>> migration work will probably be most affected. I changed the
>> configuration objects to be immutable using a static inner builder
>> class pattern. This makes construction a bit tricker, so pay
>> attention to the changes in the code and tests for the codebehind
>> plugin. The bright side is the construction code is much more
>> readable and nasty state bugs should be gone. You can do nifty things
>> like this:
>>
>> ActionConfig config = new ActionConfig.Builder("mypackage", "foo/*/*",
>> "foo.BarAction")
>> .methodName("execute")
>> .addParam("someparam", "someval")
>> .addResultConfig(new
>> ResultConfig.Builder("success{1}", "foo.MyResult")
>> .addParams("location", "/foo.jsp")
>> .build())
>> .build();
>>
>> As for the allowed methods, I originally suggested three options:
>>
>> 1. A new property/constant titled 'struts.restrictToDeclaredMethod'
>> that will instruct the ActionConfig (where the allowedMethods property
>> lives) to only allow the method that is explicitly defined (defaults
>> to 'execute'). If false, all methods will be allowed.
>>
>> 2. A new attribute on the <action> element called 'allowedMethods',
>> which takes a comma-separated list of method names to allow
>>
>> 3. A new @ActionMethod annotation for the codebehind plugin that
>> declares a method as callable
>>
>> And after the comments, I see #2 is important and #3 I'll skip, since
>> Brian will be rewriting that stuff anyways.
>>
>> To answer Matt's concern, yes, the default will be all public, no-arg
>> methods can be called, but what this will allow folks to do is limit
>> the methods that can be called, if they so choose. It also makes it
>> clearer to the developer what methods are being exposed through tools
>> like the config browser plugin. I'm also thinking it will be helpful
>> down the road when a plugin wants to move behind no-arg methods (I've
>> tried it, it can be pretty powerful).
>>
>> See https://issues.apache.org/struts/browse/WW-2363
>>
>> Any more thoughts?
>>
>> Don
>>
>> [1] http://jira.opensymphony.com/browse/XW-594
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
>> For additional commands, e-mail: dev-help@struts.apache.org
>>
>>
>>
>
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: [s2] Allowed methods next step
Posted by Matt Raible <ma...@raibledesigns.com>.
Is there anything on <action> that allows restricting roles? I
remember this being in S1 and I'm unsure if it exists in S2. If not,
it's something I believe we should add as I believe it's useful when
using CMA. When using Spring Security, I generally keep all my
configuration in my context file, but I can see why it's useful when
using CMA. If it is an attribute, I suppose having separate <action>
definitions is the best way to allow different roles for different
methods? If not, I could see some sort of "method:role1/role2"
shortcut being useful - but it might also make things more
complicated.
Matt
On Dec 9, 2007 5:30 AM, Don Brown <mr...@twdata.org> wrote:
> Since the commit for this feature involved a rather large XWork change
> (properly immutable configuration objects [1]), I decided to commit
> what I have and discuss the next steps.
>
> First, due the aforementioned fix [1], Brian, your SmartURL's
> migration work will probably be most affected. I changed the
> configuration objects to be immutable using a static inner builder
> class pattern. This makes construction a bit tricker, so pay
> attention to the changes in the code and tests for the codebehind
> plugin. The bright side is the construction code is much more
> readable and nasty state bugs should be gone. You can do nifty things
> like this:
>
> ActionConfig config = new ActionConfig.Builder("mypackage", "foo/*/*",
> "foo.BarAction")
> .methodName("execute")
> .addParam("someparam", "someval")
> .addResultConfig(new
> ResultConfig.Builder("success{1}", "foo.MyResult")
> .addParams("location", "/foo.jsp")
> .build())
> .build();
>
> As for the allowed methods, I originally suggested three options:
>
> 1. A new property/constant titled 'struts.restrictToDeclaredMethod'
> that will instruct the ActionConfig (where the allowedMethods property
> lives) to only allow the method that is explicitly defined (defaults
> to 'execute'). If false, all methods will be allowed.
>
> 2. A new attribute on the <action> element called 'allowedMethods',
> which takes a comma-separated list of method names to allow
>
> 3. A new @ActionMethod annotation for the codebehind plugin that
> declares a method as callable
>
> And after the comments, I see #2 is important and #3 I'll skip, since
> Brian will be rewriting that stuff anyways.
>
> To answer Matt's concern, yes, the default will be all public, no-arg
> methods can be called, but what this will allow folks to do is limit
> the methods that can be called, if they so choose. It also makes it
> clearer to the developer what methods are being exposed through tools
> like the config browser plugin. I'm also thinking it will be helpful
> down the road when a plugin wants to move behind no-arg methods (I've
> tried it, it can be pretty powerful).
>
> See https://issues.apache.org/struts/browse/WW-2363
>
> Any more thoughts?
>
> Don
>
> [1] http://jira.opensymphony.com/browse/XW-594
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
--
http://raibledesigns.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org