Resolve StrongParams rubocop TODOs for some admin controllers

Many admin controllers use params directly without using Rails' StrongParameters helper. This causes Rubocop exceptions, which were ignored when the rubocop was introduced in !155661 (merged)

Let's try to remove a bunch of admin controllers from .rubocop_todo/rails/strong_params.yml

  • app/controllers/admin/background_migrations_controller.rb
  • app/controllers/admin/batched_jobs_controller.rb
  • app/controllers/admin/broadcast_messages_controller.rb
  • app/controllers/admin/deploy_keys_controller.rb
  • app/controllers/admin/groups_controller.rb
  • app/controllers/admin/hook_logs_controller.rb
  • app/controllers/admin/hooks_controller.rb
  • app/controllers/admin/identities_controller.rb
  • app/controllers/admin/impersonation_tokens_controller.rb
  • app/controllers/admin/labels_controller.rb
  • app/controllers/admin/runner_projects_controller.rb
  • app/controllers/admin/sessions_controller.rb
  • app/controllers/admin/slacks_controller.rb
  • app/controllers/admin/spam_logs_controller.rb
  • app/controllers/admin/topics/avatars_controller.rb
  • app/controllers/admin/topics_controller.rb

The following were excluded from this effort for being, at a quick glance, too complicated or high risk for a "quick win" / easy MR review:

  • 'app/controllers/admin/abuse_reports_controller.rb' - being addressed in Resolves AbuseController StrongParam rubocop ex... (!161856 - merged)
  • app/controllers/admin/application_settings_controller.rb - lots of params being both overridden and potentially passed to ApplicationSettings::UpdateService
  • app/controllers/admin/applications_controller.rb - highish risk and a weird override of params[:owner] that I don't quickly understand
  • app/controllers/admin/projects_controller.rb - there are a number of parameters that go to ProjectsFinder; could be its own MR
  • app/controllers/admin/runners_controller.rb there is some relatively complex use of params in TagsFinder and GitRefsFinder
  • app/controllers/admin/users_controller.rb The logic in update is pretty straightforward, but probably still enough to warrant its own simple MR
  • app/controllers/admin/keys_controller.rb modifying this file led to undercoverage failure, so it needs an MR that includes new specs
    • #show #destroy #user and #key_params had no hits: https://gitlab.com/gitlab-org/gitlab/-/jobs/7533592435
    • There are no specs for the controller in spec/controllers/admin or spec/requests/admin
Edited Aug 08, 2024 by Nick Malcolm
Assignee Loading
Time tracking Loading