Change the way 'PUT' endpoitns handle requests with incorrect parameters

Hello all,

Currently the Katello and Foreman APIs do not return any sort of error when
a user tries to update a parameter that cannot be updated or any fake
parameters. For example if you were to use a put statement on a
content-view with this body:

{
"label": "new label",
"fake_param": "fake data"
}

It returns a 200 response with the data for that content-view, with no
recognition of the attempted changes.

I think that it may be better to have a 400 response instead telling the
user the changes they tried to make cannot be taken as parameters. This is
a significant departure from the way it behaved before and would affect
both Katello and Foreman.

What do people think of changing it to behave this way?

If we were using strong parameters
(http://edgeapi.rubyonrails.org/classes/ActionController/StrongParameters.html)
like we're supposed to, it would give a 400 response code.

··· On Mon, May 2, 2016 at 11:42 AM, wrote: > Hello all, > > Currently the Katello and Foreman APIs do not return any sort of error when > a user tries to update a parameter that cannot be updated or any fake > parameters. For example if you were to use a put statement on a content-view > with this body: > > { > "label": "new label", > "fake_param": "fake data" > } > > It returns a 200 response with the data for that content-view, with no > recognition of the attempted changes. > > I think that it may be better to have a 400 response instead telling the > user the changes they tried to make cannot be taken as parameters. This is a > significant departure from the way it behaved before and would affect both > Katello and Foreman. > > What do people think of changing it to behave this way? > > -- > 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/d/optout.


Andrew Kofink
akofink@redhat.com

Some discussion here from the past https://groups.google.com/d/topic/foreman-dev/4LxKFB1xAdM/discussion

A source of problem is that, if implemented, the API would error on a PUT of a GET result in many cases.

We use apipie as the document of the supported API.

··· ----- Original Message ----- > Hello all, > > Currently the Katello and Foreman APIs do not return any sort of error when > a user tries to update a parameter that cannot be updated or any fake > parameters. For example if you were to use a put statement on a > content-view with this body: > > { > "label": "new label", > "fake_param": "fake data" > } > > It returns a 200 response with the data for that content-view, with no > recognition of the attempted changes. > > I think that it may be better to have a 400 response instead telling the > user the changes they tried to make cannot be taken as parameters. This is > a significant departure from the way it behaved before and would affect > both Katello and Foreman. > > What do people think of changing it to behave this way? > > -- > 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/d/optout. >

This depends on how the config “action_on_unpermitted_parameters” is
configured. I think by default it’s log in development and test and false
(do nothing) in production.

As Tom points out, in RESTful designs, PUT should really accept back
whatever is sent via GET. Unfortunately, this is hard to support in with
action_on_unpermitted_parameters set to raise.

In Rails 4, they introduced PATCH support partly for this reason (
http://weblog.rubyonrails.org/2012/2/26/edge-rails-patch-is-the-new-primary-http-method-for-updates/).
I really don’t think we should be validating the PUT requests—maybe PATCH
requests but we really don’t support those yet.

David

··· On Mon, May 2, 2016 at 11:47 AM, Andrew Kofink wrote:

If we were using strong parameters
(
http://edgeapi.rubyonrails.org/classes/ActionController/StrongParameters.html
)
like we’re supposed to, it would give a 400 response code.

On Mon, May 2, 2016 at 11:42 AM, zhunting@redhat.com wrote:

Hello all,

Currently the Katello and Foreman APIs do not return any sort of error
when
a user tries to update a parameter that cannot be updated or any fake
parameters. For example if you were to use a put statement on a
content-view
with this body:

{
“label”: “new label”,
“fake_param”: “fake data”
}

It returns a 200 response with the data for that content-view, with no
recognition of the attempted changes.

I think that it may be better to have a 400 response instead telling the
user the changes they tried to make cannot be taken as parameters. This
is a
significant departure from the way it behaved before and would affect
both
Katello and Foreman.

What do people think of changing it to behave this way?


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/d/optout.


Andrew Kofink
akofink@redhat.com


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/d/optout.

I should note, however, that I'd be completely in favor of making a real RESTful API to katello (and foreman) that was consistent from the outset. There was talk of following jsonapi.org since it would be a neutral third party that could be used as a conflict resolution source. I think that an API could actually be implemented as a plugin with a lot of work. Being done in a plugin would allow the interested devs the freedom to collaborate in unison without having to deal with legacy issues. If this is something you'd be interested, please lead the charge!

··· ----- Original Message ----- > This depends on how the config “action_on_unpermitted_parameters” is > configured. I think by default it’s log in development and test and false > (do nothing) in production. > > As Tom points out, in RESTful designs, PUT should really accept back > whatever is sent via GET. Unfortunately, this is hard to support in with > action_on_unpermitted_parameters set to raise. > > In Rails 4, they introduced PATCH support partly for this reason ( > http://weblog.rubyonrails.org/2012/2/26/edge-rails-patch-is-the-new-primary-http-method-for-updates/). > I really don’t think we should be validating the PUT requests—maybe PATCH > requests but we really don’t support those yet. > > > David > > On Mon, May 2, 2016 at 11:47 AM, Andrew Kofink wrote: > > > If we were using strong parameters > > ( > > http://edgeapi.rubyonrails.org/classes/ActionController/StrongParameters.html > > ) > > like we're supposed to, it would give a 400 response code. > > > > On Mon, May 2, 2016 at 11:42 AM, wrote: > > > Hello all, > > > > > > Currently the Katello and Foreman APIs do not return any sort of error > > when > > > a user tries to update a parameter that cannot be updated or any fake > > > parameters. For example if you were to use a put statement on a > > content-view > > > with this body: > > > > > > { > > > "label": "new label", > > > "fake_param": "fake data" > > > } > > > > > > It returns a 200 response with the data for that content-view, with no > > > recognition of the attempted changes. > > > > > > I think that it may be better to have a 400 response instead telling the > > > user the changes they tried to make cannot be taken as parameters. This > > is a > > > significant departure from the way it behaved before and would affect > > both > > > Katello and Foreman. > > > > > > What do people think of changing it to behave this way? > > > > > > -- > > > 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/d/optout. > > > > > > > > -- > > Andrew Kofink > > akofink@redhat.com > > > > -- > > 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/d/optout. > > > > -- > 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/d/optout. >