Project

General

Profile

Feature #22285

Present error message when passing integer when array expected for API call

Added by Tomer Brisker over 1 year ago. Updated 3 months ago.

Status:
New
Priority:
Normal
Assignee:
Category:
API
Target version:
-
Difficulty:
Triaged:
No
Bugzilla link:
Team Backlog:
Fixed in Releases:
Found in Releases:

Description

Summary from BZ: when passing an integer for a parameter expecting an array (e.g. organization_ids), strong params filters out the value and continues silently instead of failing the request.

Cloned from https://bugzilla.redhat.com/show_bug.cgi?id=1401090

Description of problem:
A typo in API call generates a backtrace errors instead of simple error message with proper syntax suggestion

Steps to Reproduce:
1. Execute API call with a typo

NOK (fails)
  1. curl -H "Accept:application/json,version=2" -H "Content-Type:application/json" -X PUT -u username:password -k -d "{\"user\":{\"location_ids\":[3], \"organization_ids\":1}}" https://sat6.example.com/api/users/12
OK (works)
  1. curl -H "Accept:application/json,version=2" -H "Content-Type:application/json" -X PUT -u username:password -k -d "{\"user\":{\"location_ids\":[3], \"organization_ids\":[1]}}" https://sat6.example.com/api/users/12

Actual results:

  1. curl -H "Accept:application/json,version=2" -H "Content-Type:application/json" -X PUT -u username:password -k -d "{\"user\":{\"location_ids\":[3], \"organization_ids\":1}}" https://sat6.example.com/api/users/12

2016-12-01 17:13:33 [app] [I] Started PUT "/api/users/12" for <IP> at 2016-12-01 17:13:33 -0500
2016-12-01 17:13:33 [app] [I] Processing by Api::V2::UsersController#update as JSON
2016-12-01 17:13:33 [app] [I] Parameters: {"user"=>{"location_ids"=>[3], "organization_ids"=>1}, "apiv"=>"v2", "id"=>"12"}
2016-12-01 17:13:33 [app] [I] Authorized user admin(Admin User)
2016-12-01 17:13:33 [app] [W] Action failed | NoMethodError: undefined method `uniq' for 1:Fixnum | /usr/share/foreman/app/models/concerns/dirty_associations.rb:34:in `block (2 levels) in dirty_has_many_associations' | /opt/rh/rh-ror41/root/usr/share/gems/gems/activerecord-4.1.5/lib/active_record/attribute_assignment.rb:45:in `public_send'
<-- snip --> | /opt/theforeman/tfm/root/usr/share/gems/gems/logging-1.8.2/lib/logging/diagnostic_context.rb:323:in `block in create_with_logging_context'
2016-12-01 17:13:33 [app] [I] Rendered api/v2/errors/standard_error.json.rabl within api/v2/layouts/error_layout (1.2ms)
2016-12-01 17:13:33 [app] [I] Completed 500 Internal Server Error in 52ms (Views: 2.5ms | ActiveRecord: 7.6ms)

Expected results:

  1. curl -H "Accept:application/json,version=2" -H "Content-Type:application/json" -X PUT -u username:password -k -d "{\"user\":{\"location_ids\":[3], \"organization_ids\":1}}" https://usl10149341.am.hedani.net/api/users/12

[E] 'organization_ids must be array' or something similar with proper syntax suggestion

Additional info:

None


Related issues

Related to Discovery - Refactor #22325: Fix tests after strong params set to raiseClosed2018-01-19
Related to Foreman - Feature #3917: Add strong_parameters to foremanClosed2013-12-19
Related to foreman-tasks - Refactor #22438: Remove KeepParamsClosed2018-01-28
Related to Foreman - Feature #3026: Enable apipie API parameter validationNew2013-09-04
Related to Foreman - Bug #25755: log when unpermitted params are passed in all environmentsClosed
Blocked by Foreman Remote Execution - Bug #22531: Correctly handle strong paramsAssigned2018-02-07

Associated revisions

Revision 15b6d27d (diff)
Added by Tomer Brisker about 1 year ago

Refs #22285 - Remove keep_param

This workaround was needed in Rails 5.0, but 5.1 already supports
filtering on arbitrary hash params.

Revision bd941767 (diff)
Added by Tomer Brisker about 1 year ago

Refs #22285 - add common parameters to strong_params whitelist

Despite the name, this list only prevents errors from being raised from
these parameters which are still filtered.

Revision 69c5fa93 (diff)
Added by Tomer Brisker about 1 year ago

Refs #22285 - Allow unwrapped params on API

Revision 98331f74 (diff)
Added by Tomer Brisker about 1 year ago

Refs #22285 - Correct parameter filtering for strong params

- Allow $resource_id param on parameters controller
- Permit user_id on access token controller
- Allow params in templete combination controller
Allows `config_template_id` and `:provisioning_template_id`
- Fix compute attribute param filter
Ignore top-level compute_resource_id and compute_profile_id that are
passed from the url and aren't used for assignment.
- Allow arbitrary compute_atrributes for nic

Revision 5cbba9ec (diff)
Added by Tomer Brisker about 1 year ago

Refs #22285 - Prepare tests for strong params enforcement

- Fix broken hosts api tests
- Fix role cloning tests
- Fix ssh key controller api test
- Fix http proxies controller test
- Fix lookup key override api controller test
- Fix puppetclass controller api test
- fix operatingsystem controller api test
- Fix hostgroups controller test
- Fix integration tests with template fields
We remove the template fields using JS, so we have to test these forms
with JS or they fail because the template fields are sent
- Fix host instegration tests
- fix hosts controller tests
- Fix images controller parameter handling
- Fix interface controller test
- Fix puppetclass controller tests
- Fix variable LK controller test

Revision e9fa04b9 (diff)
Added by Tomer Brisker about 1 year ago

Refs #22285 - Prepare UI for strong params enforcement

- Fix current_url_params helper for strong params
Explicitly permit only the permitted parameters, don't try to permit all
parameters (and raise exception for other parameters)
- Don't send editor params to server
This causes issues with strong params, and we don't need these fields
anyways.
- Fix host form for strong params
- Disable cr fields on hosts form
These fields are only used to determine in JS what capabilities the
current compute resource allows, they don't need to be sent to the
server.
- Correctly calculate object type in host form
Otherwise, discovered hosts add a field named
`discovered_host[puppetclass_ids][]` that fails on smart params which
expects the whole form to be submitted under the `host` hash.
- Remove extranous role_id input on filter form

History

#1 Updated by The Foreman Bot over 1 year ago

  • Assignee set to Tomer Brisker
  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/5183 added

#2 Updated by Tomer Brisker over 1 year ago

  • Related to Refactor #22325: Fix tests after strong params set to raise added

#3 Updated by Tomer Brisker over 1 year ago

  • Related to Feature #3917: Add strong_parameters to foreman added

#4 Updated by Tomer Brisker about 1 year ago

#5 Updated by Tomer Brisker about 1 year ago

  • Blocked by Bug #22531: Correctly handle strong params added

#6 Updated by The Foreman Bot about 1 year ago

  • Pull request https://github.com/theforeman/foreman/pull/5330 added

#7 Updated by Marek Hulán about 1 year ago

  • Related to Feature #3026: Enable apipie API parameter validation added

#8 Updated by Marek Hulán about 1 year ago

  • Subject changed from Present error message when passing integer when array expected for API call to Present error message when passing integer when array expected for API call

It might be also good time to revisit enabling apipie validations.

#9 Updated by Tomer Brisker 6 months ago

  • Category changed from 19 to API

#10 Updated by Tomer Brisker 4 months ago

  • Related to Bug #25755: log when unpermitted params are passed in all environments added

#11 Updated by The Foreman Bot 3 months ago

  • Status changed from Ready For Testing to New
  • Pull request deleted (https://github.com/theforeman/foreman/pull/5183)

Also available in: Atom PDF