Project

General

Profile

Bug #17256

Non-admin user can't edit his own profile if he has more than 5 roles

Added by Marek Hulán almost 2 years ago. Updated about 1 month ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Authorization
Target version:
Difficulty:
Triaged:
Bugzilla link:
Team Backlog:
Fixed in Releases:
Found in Releases:

Description

With current Foreman user can't edit his/her own profile if the account is granted more than 5 roles, the exception follows

 | ActionView::Template::Error (PG::InvalidColumnReference: ERROR:  for SELECT DISTINCT, ORDER BY expressions must appear in select list
 | LINE 1: ...ocation'))  ORDER BY filters.role_id, filters.id, "roles"."n...
 |                                                              ^
 | : SELECT DISTINCT "filters".* FROM "filters" INNER JOIN "filterings" ON "filterings"."filter_id" = "filters"."id" INNER JOIN "permissions" ON "permissions"."id" = "filterings"."permission_id" INNER JOIN "roles" ON "filters"."role_id" = "roles"."id" INNER JOIN "cached_user_roles" ON "roles"."id" = "cached_user_roles"."role_id" LEFT JOIN taxable_taxonomies ON (filters.id = taxable_taxonomies.taxable_id AND taxable_type = 'Filter') LEFT JOIN taxonomies ON (taxonomies.id = taxable_taxonomies.taxonomy_id) WHERE "roles"."builtin" = $1 AND "roles"."id" IN (62, 17, 16, 18, 21, 6, 5, 10, 8, 31, 7, 57, 19, 20, 4, 3, 12, 1, 2, 11, 9) AND "cached_user_roles"."user_id" = $2 AND (permissions.resource_type = 'Role') AND (permissions.name = 'view_roles') AND (taxable_taxonomies.id IS NULL OR (taxonomies.type = 'Organization') OR (taxonomies.type = 'Location'))  ORDER BY filters.role_id, filters.id, "roles"."name" ASC):
 |     72: 
 |     73:     <div class='tab-pane' id='roles'>
 |     74:       <%= checkbox_f f, :admin if User.current.can_change_admin_flag? %>
 |     75:       <%= multiple_checkboxes f, :roles, @user, Role.givable.for_current_user,
 |     76:                                { :label => _('Roles')}, {:disabled => @editing_self ? Role.givable.for_current_user.pluck(:id) : false } %>
 |     77:     </div>
 |     78:     <%= render 'taxonomies/loc_org_tabs', :f => f, :obj => @user,
 |   app/services/authorizer.rb:46:in `find_collection'
 |   app/models/concerns/authorizable.rb:27:in `authorized_as'
 |   app/models/concerns/authorizable.rb:65:in `authorized'
 |   app/services/association_authorizer.rb:10:in `authorized_associations'
 |   app/helpers/form_helper.rb:71:in `multiple_selects'
 |   app/helpers/form_helper.rb:52:in `multiple_checkboxes'
 |   app/views/users/_form.html.erb:75:in `block in _861f0cf4d8ce7a662cb6dc52f4cf250e'
 |   app/helpers/form_helper.rb:286:in `form_for'
 |   app/views/users/_form.html.erb:3:in `_861f0cf4d8ce7a662cb6dc52f4cf250e'
 |   app/views/users/edit.html.erb:3:in `_30ea7a6fadeacb8b083429b851c07a17'
 |   app/controllers/concerns/application_shared.rb:14:in `set_timezone'
 |   app/models/concerns/foreman/thread_session.rb:32:in `clear_thread'
 |   lib/middleware/catch_json_parse_errors.rb:8:in `call'
 |   lib/middleware/tagged_logging.rb:18:in `call'

This is caused by role default scope adding ordering by name and the fact that multiple_selects pass resource scope with default order. It would be fixed by #16971 https://github.com/theforeman/foreman/pull/3955/files#diff-a1d6431830287bc2d6a5c744edbf1eb7R65 but since in authorizer it does not make sense to enforce order. We don't rely on it since we only check their presence and use them to generate scope search. So we can safely reorder(nil) there which might gain also some small performance boost.


Related issues

Related to Foreman - Bug #16971: CVE-2016-7077 - Association lists (for < 6 items) shown without authorization/filtersClosed2016-10-17

Associated revisions

Revision 6ba1d116 (diff)
Added by Marek Hulán almost 2 years ago

Fixes #17256 - ignore order in Authorizer

Revision 4e2ed14e (diff)
Added by Marek Hulán over 1 year ago

Fixes #17256 - ignore order in Authorizer

(cherry picked from commit 6ba1d11667fcdd8d7345d487f3c831e0d31e587d)

History

#1 Updated by Marek Hulán almost 2 years ago

  • Related to Bug #16971: CVE-2016-7077 - Association lists (for < 6 items) shown without authorization/filters added

#2 Updated by The Foreman Bot almost 2 years ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/4002 added

#3 Updated by Anonymous almost 2 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100

#4 Updated by Dominic Cleal almost 2 years ago

  • Legacy Backlogs Release (now unused) set to 203

Also available in: Atom PDF