In Katello, we've been using an in-house solution (param_rules) to lock down parameters per action. The problem with our solution that it is not compatible with what Foreman, it's not compatible with the future of Rails, and it encourages a bad design (i.e. using params directly to update models). We have a few options though:
- Continue using our current solution.
- Adopt the Foremans model of using attr_accessible, attr_protected, etc
- Use strong_params (see http://weblog.rubyonrails.org/2012/3/21/strong-parameters/)
The problem I see with 2 is that it would be a huge amount of work to move param security from the controller to the model. Moreover, it probably would mess up our existing non-V2 controllers. Also, it's rather dated (more on this later).
I think 3 would be a good option provided it doesn't require too much work. While strong params is only in Rails 4+, there's a gem that provides this functionality (https://github.com/rails/strong_parameters) to Rails 3.2 although it's not packaged for Fedora.
That said, I think it could be a trivial code change since strong_parameters is much like how we currently use param_rules. Moreover, we'd be a step ahead in converting to Rails 4 and since it's part of Rails 4, it's a well-supported solution.
Thoughts? Also, I am wondering if Foreman has thought about strong params at all? I imagine that at some point Foreman will likely start to integrate it since it's a big part of Rails going forward.
David
I think katello kinda based it off strong params only. I am ok with 3.
A good controller to check how hard or easy it will be the RolesController in the UI.
I think it has the most complex of rules.
https://github.com/Katello/katello/blob/master/app/controllers/katello/roles_controller.rb#L50
Some how have a feeling we'll need more than strong params, because we may want logic that says
"if you provide param x we can accept only a.b"
···
----- Original Message -----
> From: "David Davis"
> To: "foreman-dev"
> Sent: Tuesday, December 17, 2013 10:02:07 AM
> Subject: [foreman-dev] Param security in V2
>
> In Katello, we've been using an in-house solution (param_rules) to lock down
> parameters per action. The problem with our solution that it is not
> compatible with what Foreman, it's not compatible with the future of Rails,
> and it encourages a bad design (i.e. using params directly to update
> models). We have a few options though:
>
> 1. Continue using our current solution.
> 2. Adopt the Foremans model of using attr_accessible, attr_protected, etc
> 3. Use strong_params (see
> http://weblog.rubyonrails.org/2012/3/21/strong-parameters/)
>
3 (strong_params) seems like the way to go for me, from what I've seen it
looks like it'd relieve us from the pressure of controlling params for
every model explicitly. As a side effect, It would also increase the
percent of code we won't have to change for Rails 4 
···
On Tue, Dec 17, 2013 at 4:02 PM, David Davis wrote:
In Katello, we’ve been using an in-house solution (param_rules) to lock
down parameters per action. The problem with our solution that it is not
compatible with what Foreman, it’s not compatible with the future of Rails,
and it encourages a bad design (i.e. using params directly to update
models). We have a few options though:
- Continue using our current solution.
- Adopt the Foremans model of using attr_accessible, attr_protected, etc
- Use strong_params (see
http://weblog.rubyonrails.org/2012/3/21/strong-parameters/)
The problem I see with 2 is that it would be a huge amount of work to move
param security from the controller to the model. Moreover, it probably
would mess up our existing non-V2 controllers. Also, it’s rather dated
(more on this later).
I think 3 would be a good option provided it doesn’t require too much
work. While strong params is only in Rails 4+, there’s a gem that provides
this functionality (https://github.com/rails/strong_parameters) to Rails
3.2 although it’s not packaged for Fedora.
That said, I think it could be a trivial code change since
strong_parameters is much like how we currently use param_rules. Moreover,
we’d be a step ahead in converting to Rails 4 and since it’s part of Rails
4, it’s a well-supported solution.
Thoughts? Also, I am wondering if Foreman has thought about strong params
at all? I imagine that at some point Foreman will likely start to integrate
it since it’s a big part of Rails going forward.
David
–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
–
Daniel Lobato
@elobatoss
blog.daniellobato.me
daniellobato.me
GPG: http://keys.gnupg.net/pks/lookup?op=get&search=0x7A92D6DD38D6DE30
> In Katello, we've been using an in-house solution (param_rules) to lock down parameters per action. The problem with our solution that it is not compatible with what Foreman, it's not compatible with the future of Rails, and it encourages a bad design (i.e. using params directly to update models). We have a few options though:
>
> 1. Continue using our current solution.
> 2. Adopt the Foremans model of using attr_accessible, attr_protected, etc
> 3. Use strong_params (see http://weblog.rubyonrails.org/2012/3/21/strong-parameters/)
+1 for 3.
···
On 17.12.13 16:02, David Davis wrote:
The problem I see with 2 is that it would be a huge amount of work to move param security from the controller to the model. Moreover, it probably would mess up our existing non-V2 controllers. Also, it’s rather dated (more on this later).
I think 3 would be a good option provided it doesn’t require too much work. While strong params is only in Rails 4+, there’s a gem that provides this functionality (https://github.com/rails/strong_parameters) to Rails 3.2 although it’s not packaged for Fedora.
That said, I think it could be a trivial code change since strong_parameters is much like how we currently use param_rules. Moreover, we’d be a step ahead in converting to Rails 4 and since it’s part of Rails 4, it’s a well-supported solution.
Thoughts? Also, I am wondering if Foreman has thought about strong params at all? I imagine that at some point Foreman will likely start to integrate it since it’s a big part of Rails going forward.
David
> 3 (strong_params) seems like the way to go for me, from what I've seen it
> looks like it'd relieve us from the pressure of controlling params for
> every model explicitly. As a side effect, It would also increase the
> percent of code we won't have to change for Rails 4
>
+1 long term for sure, what do you think the the effort to implement is ?
···
On Tue, Dec 17, 2013 at 5:40 PM, Daniel Lobato wrote:
On Tue, Dec 17, 2013 at 4:02 PM, David Davis daviddavis@redhat.comwrote:
In Katello, we’ve been using an in-house solution (param_rules) to lock
down parameters per action. The problem with our solution that it is not
compatible with what Foreman, it’s not compatible with the future of Rails,
and it encourages a bad design (i.e. using params directly to update
models). We have a few options though:
- Continue using our current solution.
- Adopt the Foremans model of using attr_accessible, attr_protected, etc
- Use strong_params (see
http://weblog.rubyonrails.org/2012/3/21/strong-parameters/)
The problem I see with 2 is that it would be a huge amount of work to
move param security from the controller to the model. Moreover, it probably
would mess up our existing non-V2 controllers. Also, it’s rather dated
(more on this later).
I think 3 would be a good option provided it doesn’t require too much
work. While strong params is only in Rails 4+, there’s a gem that provides
this functionality (https://github.com/rails/strong_parameters) to Rails
3.2 although it’s not packaged for Fedora.
That said, I think it could be a trivial code change since
strong_parameters is much like how we currently use param_rules. Moreover,
we’d be a step ahead in converting to Rails 4 and since it’s part of Rails
4, it’s a well-supported solution.
Thoughts? Also, I am wondering if Foreman has thought about strong params
at all? I imagine that at some point Foreman will likely start to integrate
it since it’s a big part of Rails going forward.
David
–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
–
Daniel Lobato
@elobatoss
blog.daniellobato.me
daniellobato.me
GPG: http://keys.gnupg.net/pks/lookup?op=get&search=0x7A92D6DD38D6DE30
–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
I have another question in regards to param security. This is mostly for Katello developers. Currently in our API we raise an exception if we see parameters that we don't want. The problem is that AngularJS sends the entire set of params back for update requests (which is actually the correct behavior for PUT) so we get stuff that we're not expecting and that we don't use to update the model (think Candlepin and Pulp data). So we can either:
- List all the attributes we are returning from the show request as being valid for the update action.
- Just filter out and ignore any attributes that we don't want.
I think #1 will be a fairly large amount of work. We'll have to list and maintain every attribute that appears in show.json.rabl in our param rule. If someone adds an attribute to show.json.rabl but forgets to add it to the update param rule, the updates will be broken from the UI.
#2 seems like less work but I think it would be less beneficial to users since they don't get immediate feedback if they've sent a bad param.
Thoughts?
David
···
----- Original Message -----
> From: "Petr Chalupa"
> To: foreman-dev@googlegroups.com
> Sent: Tuesday, December 17, 2013 12:10:08 PM
> Subject: Re: [foreman-dev] Param security in V2
>
>
>
> On 17.12.13 16:02, David Davis wrote:
> > In Katello, we've been using an in-house solution (param_rules) to lock
> > down parameters per action. The problem with our solution that it is not
> > compatible with what Foreman, it's not compatible with the future of
> > Rails, and it encourages a bad design (i.e. using params directly to
> > update models). We have a few options though:
> >
> > 1. Continue using our current solution.
> > 2. Adopt the Foremans model of using attr_accessible, attr_protected, etc
> > 3. Use strong_params (see
> > http://weblog.rubyonrails.org/2012/3/21/strong-parameters/)
>
> +1 for 3.
>
> >
> > The problem I see with 2 is that it would be a huge amount of work to move
> > param security from the controller to the model. Moreover, it probably
> > would mess up our existing non-V2 controllers. Also, it's rather dated
> > (more on this later).
> >
> > I think 3 would be a good option provided it doesn't require too much work.
> > While strong params is only in Rails 4+, there's a gem that provides this
> > functionality (https://github.com/rails/strong_parameters) to Rails 3.2
> > although it's not packaged for Fedora.
> >
> > That said, I think it could be a trivial code change since
> > strong_parameters is much like how we currently use param_rules. Moreover,
> > we'd be a step ahead in converting to Rails 4 and since it's part of Rails
> > 4, it's a well-supported solution.
> >
> > Thoughts? Also, I am wondering if Foreman has thought about strong params
> > at all? I imagine that at some point Foreman will likely start to
> > integrate it since it's a big part of Rails going forward.
> >
> > David
> >
>
> --
> You received this message because you are subscribed to the Google Groups
> "foreman-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to foreman-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
I think the bulk of the work could be done in one sprint. The only hairy
case I've encountered were BMC interfaces.
···
On Tue, Dec 17, 2013 at 4:43 PM, Ohad Levy wrote:
On Tue, Dec 17, 2013 at 5:40 PM, Daniel Lobato elobatocs@gmail.comwrote:
3 (strong_params) seems like the way to go for me, from what I’ve seen it
looks like it’d relieve us from the pressure of controlling params for
every model explicitly. As a side effect, It would also increase the
percent of code we won’t have to change for Rails 4
+1 long term for sure, what do you think the the effort to implement is ?
On Tue, Dec 17, 2013 at 4:02 PM, David Davis daviddavis@redhat.comwrote:
In Katello, we’ve been using an in-house solution (param_rules) to lock
down parameters per action. The problem with our solution that it is not
compatible with what Foreman, it’s not compatible with the future of Rails,
and it encourages a bad design (i.e. using params directly to update
models). We have a few options though:
- Continue using our current solution.
- Adopt the Foremans model of using attr_accessible, attr_protected, etc
- Use strong_params (see
http://weblog.rubyonrails.org/2012/3/21/strong-parameters/)
The problem I see with 2 is that it would be a huge amount of work to
move param security from the controller to the model. Moreover, it probably
would mess up our existing non-V2 controllers. Also, it’s rather dated
(more on this later).
I think 3 would be a good option provided it doesn’t require too much
work. While strong params is only in Rails 4+, there’s a gem that provides
this functionality (https://github.com/rails/strong_parameters) to
Rails 3.2 although it’s not packaged for Fedora.
That said, I think it could be a trivial code change since
strong_parameters is much like how we currently use param_rules. Moreover,
we’d be a step ahead in converting to Rails 4 and since it’s part of Rails
4, it’s a well-supported solution.
Thoughts? Also, I am wondering if Foreman has thought about strong
params at all? I imagine that at some point Foreman will likely start to
integrate it since it’s a big part of Rails going forward.
David
–
You received this message because you are subscribed to the Google
Groups “foreman-dev” group.
To unsubscribe from this group and stop receiving emails from it, send
an email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
–
Daniel Lobato
@elobatoss
blog.daniellobato.me
daniellobato.me
GPG: http://keys.gnupg.net/pks/lookup?op=get&search=0x7A92D6DD38D6DE30
–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
–
Daniel Lobato
@elobatoss
blog.daniellobato.me
daniellobato.me
GPG: http://keys.gnupg.net/pks/lookup?op=get&search=0x7A92D6DD38D6DE30
> From: "David Davis" <daviddavis@redhat.com>
> To: foreman-dev@googlegroups.com
> Sent: Friday, December 20, 2013 11:31:05 AM
> Subject: Re: [foreman-dev] Param security in V2
>
> I have another question in regards to param security. This is mostly for
> Katello developers. Currently in our API we raise an exception if we see
> parameters that we don't want. The problem is that AngularJS sends the
> entire set of params back for update requests (which is actually the correct
> behavior for PUT) so we get stuff that we're not expecting and that we don't
> use to update the model (think Candlepin and Pulp data). So we can either:
>
> 1. List all the attributes we are returning from the show request as being
> valid for the update action.
> 2. Just filter out and ignore any attributes that we don't want.
>
> I think #1 will be a fairly large amount of work. We'll have to list and
> maintain every attribute that appears in show.json.rabl in our param rule.
> If someone adds an attribute to show.json.rabl but forgets to add it to the
> update param rule, the updates will be broken from the UI.
>
> #2 seems like less work but I think it would be less beneficial to users
> since they don't get immediate feedback if they've sent a bad param.
>
> Thoughts?
>
> David
I'm all for option 2. With the apipie docs, there will be a ready source for API consumers to know exactly what is expected. Alternatively, we could enable something in development mode that would spit out warnings for unrecognized params to the log (kind of like the signature mismatch errors we get today with param_rules.)
···
----- Original Message -----
----- Original Message -----
From: “Petr Chalupa” pchalupa@redhat.com
To: foreman-dev@googlegroups.com
Sent: Tuesday, December 17, 2013 12:10:08 PM
Subject: Re: [foreman-dev] Param security in V2
On 17.12.13 16:02, David Davis wrote:
In Katello, we’ve been using an in-house solution (param_rules) to lock
down parameters per action. The problem with our solution that it is not
compatible with what Foreman, it’s not compatible with the future of
Rails, and it encourages a bad design (i.e. using params directly to
update models). We have a few options though:
- Continue using our current solution.
- Adopt the Foremans model of using attr_accessible, attr_protected, etc
- Use strong_params (see
http://weblog.rubyonrails.org/2012/3/21/strong-parameters/)
+1 for 3.
The problem I see with 2 is that it would be a huge amount of work to
move
param security from the controller to the model. Moreover, it probably
would mess up our existing non-V2 controllers. Also, it’s rather dated
(more on this later).
I think 3 would be a good option provided it doesn’t require too much
work.
While strong params is only in Rails 4+, there’s a gem that provides this
functionality (https://github.com/rails/strong_parameters) to Rails 3.2
although it’s not packaged for Fedora.
That said, I think it could be a trivial code change since
strong_parameters is much like how we currently use param_rules.
Moreover,
we’d be a step ahead in converting to Rails 4 and since it’s part of
Rails
4, it’s a well-supported solution.
Thoughts? Also, I am wondering if Foreman has thought about strong params
at all? I imagine that at some point Foreman will likely start to
integrate it since it’s a big part of Rails going forward.
David
–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
–
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
