diff --git a/.reek.yml b/.reek.yml index 9329f55ef5..aa0ba8b4ed 100644 --- a/.reek.yml +++ b/.reek.yml @@ -39,3 +39,6 @@ directories: "app/mailers": InstanceVariableAssumption: enabled: false + "test": + InstanceVariableAssumption: + enabled: false diff --git a/.rubocop.yml b/.rubocop.yml index e4c738d766..1cc9becde0 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -17,6 +17,10 @@ Performance: Metrics: Enabled: true +Metrics/BlockLength: + Exclude: + - 'test/**/*_test.rb' + Lint/AssignmentInCondition: AllowSafeAssignment: true diff --git a/app/controllers/admin/api/account_features_controller.rb b/app/controllers/admin/api/account_features_controller.rb index e1da6e07a7..5d1055172f 100644 --- a/app/controllers/admin/api/account_features_controller.rb +++ b/app/controllers/admin/api/account_features_controller.rb @@ -41,7 +41,7 @@ def destroy protected def feature_params - params.fetch(:feature) + params.require(:feature).permit(:name, :system_name, :description) end def features diff --git a/app/controllers/admin/api/api_docs_services_controller.rb b/app/controllers/admin/api/api_docs_services_controller.rb index da4294f921..72e1fbed8c 100644 --- a/app/controllers/admin/api/api_docs_services_controller.rb +++ b/app/controllers/admin/api/api_docs_services_controller.rb @@ -27,7 +27,7 @@ def index # ActiveDocs Spec Create # POST /admin/api/active_docs.json def create - @api_docs_service = current_account.api_docs_services.create(api_docs_params(:system_name), without_protection: true) + @api_docs_service = current_account.api_docs_services.create(api_docs_params(:system_name)) respond_with(@api_docs_service) end @@ -40,7 +40,7 @@ def show # ActiveDocs Spec Update # PUT /admin/api/active_docs/{id}.json def update - @api_docs_service.update(api_docs_params, without_protection: true) + @api_docs_service.update(api_docs_params) respond_with(@api_docs_service) end diff --git a/app/controllers/admin/api/application_plan_metric_limits_controller.rb b/app/controllers/admin/api/application_plan_metric_limits_controller.rb index 75aafed67d..49a4d3ef99 100644 --- a/app/controllers/admin/api/application_plan_metric_limits_controller.rb +++ b/app/controllers/admin/api/application_plan_metric_limits_controller.rb @@ -61,7 +61,7 @@ def usage_limit end def usage_limit_params - params.fetch(:usage_limit) + params.fetch(:usage_limit).permit(:period, :value) end end diff --git a/app/controllers/admin/api/registry/policies_controller.rb b/app/controllers/admin/api/registry/policies_controller.rb index 32f6b1aa7d..b2c58bc619 100644 --- a/app/controllers/admin/api/registry/policies_controller.rb +++ b/app/controllers/admin/api/registry/policies_controller.rb @@ -54,6 +54,8 @@ def authorize_policies def policy_params policy_params = params.require(:policy) final_params = policy_params.permit(:name, :version) + # permit! is necessary here because the schema field accepts arbitrary nested JSON structures + # that cannot be whitelisted in advance. final_params.merge(schema: policy_params.require(:schema)).permit! end diff --git a/app/controllers/admin/api/service_features_controller.rb b/app/controllers/admin/api/service_features_controller.rb index caaeadf0b5..fc70941513 100644 --- a/app/controllers/admin/api/service_features_controller.rb +++ b/app/controllers/admin/api/service_features_controller.rb @@ -26,7 +26,7 @@ def show # Service Feature Update # PUT /admin/api/services/{service_id}/features/{id}.xml def update - feature.update(feature_params) + feature.update(feature_update_params) respond_with(feature) end @@ -42,7 +42,11 @@ def destroy protected def feature_params - params.fetch(:feature) + params.require(:feature).permit(:name, :system_name, :description, :scope) + end + + def feature_update_params + feature_params.except(:scope) end def features diff --git a/app/controllers/admin/api/sso_tokens_controller.rb b/app/controllers/admin/api/sso_tokens_controller.rb index 96dcfaec33..8cb8676e55 100644 --- a/app/controllers/admin/api/sso_tokens_controller.rb +++ b/app/controllers/admin/api/sso_tokens_controller.rb @@ -1,6 +1,8 @@ +# frozen_string_literal: true + class Admin::Api::SSOTokensController < Admin::Api::BaseController - wrap_parameters :sso_token, :include => [:user_id, :username, :expires_in, :redirect_url, :protocol], :format => [ :url_encoded_form ] + wrap_parameters :sso_token, include: %i[user_id username expires_in redirect_url protocol], format: [:url_encoded_form] # parameters: # * user_id @@ -8,7 +10,7 @@ class Admin::Api::SSOTokensController < Admin::Api::BaseController # * provider_key # * protocol def create - sso_token = SSOToken.new **sso_token_params.to_h + sso_token = SSOToken.new(sso_token_params) sso_token.account = domain_account sso_token.save diff --git a/app/controllers/admin/api/web_hooks_controller.rb b/app/controllers/admin/api/web_hooks_controller.rb index 797f18e84f..3d7d0cb387 100644 --- a/app/controllers/admin/api/web_hooks_controller.rb +++ b/app/controllers/admin/api/web_hooks_controller.rb @@ -19,7 +19,7 @@ def webhook end def allowed_params - %w(url active provider_actions) + WebHook.switchable_attributes + %w[url active provider_actions] + WebHook.switchable_attributes end def webhook_params diff --git a/app/controllers/admin/api_docs/base_controller.rb b/app/controllers/admin/api_docs/base_controller.rb index d1994d05bd..c5a0ad70f9 100644 --- a/app/controllers/admin/api_docs/base_controller.rb +++ b/app/controllers/admin/api_docs/base_controller.rb @@ -18,7 +18,7 @@ def new end def create - @api_docs_service = api_docs_services.new(api_docs_params(:system_name), without_protection: true) + @api_docs_service = api_docs_services.new(api_docs_params(:system_name)) if @api_docs_service.save redirect_to preview_admin_api_docs_service_path(@api_docs_service), success: t('admin.api_docs.create.success') else @@ -72,7 +72,7 @@ def edit; end def update respond_to do |format| - if api_docs_service.update(api_docs_params, without_protection: true) + if api_docs_service.update(api_docs_params) msg = t('admin.api_docs.update.success') format.html { redirect_to preview_admin_api_docs_service_path(api_docs_service), success: msg } format.js do diff --git a/app/controllers/admin/fields_definitions_controller.rb b/app/controllers/admin/fields_definitions_controller.rb index 39eb170fc1..526477f9dd 100644 --- a/app/controllers/admin/fields_definitions_controller.rb +++ b/app/controllers/admin/fields_definitions_controller.rb @@ -4,19 +4,24 @@ class Admin::FieldsDefinitionsController < Sites::BaseController respond_to :html activate_menu :audience, :accounts, :fields_definitions + before_action :set_fields_definition_params, only: %i[new create update] + before_action :validate_target, only: [:new] + def index - @possible_targets = FieldsDefinition.targets + @possible_targets = available_targets - respond_with(field_definitions) + respond_with(fields_definitions) end def new - @fields_definition = field_definitions.build(field_definition_params) + target = fields_definition_params[:target] + + @fields_definition = fields_definitions.build(target: target) + target_class = @fields_definition.target_class - @optional_fields = @fields_definition.target_class.builtin_fields - - current_account.fields_definitions.by_target(target).map{ |f|f.name } + @optional_fields = target_class.builtin_fields - existing_fields_names_by_target(target) - @required_fields = @fields_definition.target_class.required_fields + @required_fields = target_class.required_fields @optional_fields.unshift "[new field]" @@ -24,14 +29,14 @@ def new end def edit - @optional_fields = field_definition.target_class.builtin_fields - @required_fields = field_definition.target_class.required_fields + @optional_fields = fields_definition.target_class.builtin_fields + @required_fields = fields_definition.target_class.required_fields - respond_with(field_definition) + respond_with(fields_definition) end def create - @fields_definition = field_definitions.build(field_definition_params) + @fields_definition = fields_definitions.build(fields_definition_params) if @fields_definition.save flash[:success] = t('.success') @@ -48,22 +53,20 @@ def create def update @required_fields = [] - if field_definition.update(field_definition_params) - @required_fields = field_definition.target_class.required_fields - end + @required_fields = fields_definition.target_class.required_fields if fields_definition.update(fields_definition_params) - respond_with(field_definition, location: admin_fields_definitions_path) + respond_with(fields_definition, location: admin_fields_definitions_path) end def destroy - field_definition.destroy - respond_with(field_definition, location: admin_fields_definitions_path) + fields_definition.destroy + respond_with(fields_definition, location: admin_fields_definitions_path) end def sort - fields = current_account.fields_definitions.find(field_definition_params).index_by(&:id) + fields = current_account.fields_definitions.find(sort_params).index_by(&:id) - field_definition_params.each_with_index do |field_id, index| + sort_params.each_with_index do |field_id, index| fields.fetch(field_id.to_i).update_attribute(:pos, index + 1) end @@ -72,19 +75,38 @@ def sort private - def field_definition_params - params[:fields_definition] || {} - end + attr_reader :fields_definition_params - def target - field_definition_params[:target] + def sort_params + @sort_params ||= params.permit(fields_definition: [])[:fields_definition] || [] end - def field_definitions + def fields_definitions @fields_definitions ||= current_account.fields_definitions end - def field_definition - @fields_definition ||= field_definitions.find(params[:id]) + def fields_definition + @fields_definition ||= fields_definitions.find(params[:id]) + end + + def available_targets + @available_targets ||= FieldsDefinition.targets + end + + def existing_fields_names_by_target(target) + current_account.fields_definitions.by_target(target).map(&:name) + end + + def set_fields_definition_params + fd_params = params.fetch(:fields_definition) + if fd_params.respond_to? :permit + @fields_definition_params = fd_params.permit(:target, :name, :label, :required, :hidden, :read_only, :choices_for_views) + else + render_error "invalid fields definition", status: :bad_request unless fd_params.is_a?(Hash) + end + end + + def validate_target + render_error "invalid fields definition target", status: :bad_request unless available_targets.include?(fields_definition_params[:target]) end end diff --git a/app/controllers/api/metric_visibilities_controller.rb b/app/controllers/api/metric_visibilities_controller.rb index 48a1724e28..d461b07585 100644 --- a/app/controllers/api/metric_visibilities_controller.rb +++ b/app/controllers/api/metric_visibilities_controller.rb @@ -76,8 +76,4 @@ def find_metric def authorize_section authorize! :manage, :plans end - - def usage_limit_params - params.require(:usage_limit) - end end diff --git a/app/controllers/api/pricing_rules_controller.rb b/app/controllers/api/pricing_rules_controller.rb index 46b2b0b8b4..54042b4c6c 100644 --- a/app/controllers/api/pricing_rules_controller.rb +++ b/app/controllers/api/pricing_rules_controller.rb @@ -76,6 +76,6 @@ def authorize_action end def pricing_rule_params - params.require(:pricing_rule) + params.require(:pricing_rule).permit(:min, :max, :cost_per_unit) end end diff --git a/app/controllers/api/usage_limits_controller.rb b/app/controllers/api/usage_limits_controller.rb index fb1d497c3b..38aad3a17d 100644 --- a/app/controllers/api/usage_limits_controller.rb +++ b/app/controllers/api/usage_limits_controller.rb @@ -79,6 +79,6 @@ def authorize_action end def usage_limit_params - params.require(:usage_limit) + params.require(:usage_limit).permit(:period, :value) end end diff --git a/app/controllers/buyers/invoices_controller.rb b/app/controllers/buyers/invoices_controller.rb index ef0e041d79..d88bd4839f 100644 --- a/app/controllers/buyers/invoices_controller.rb +++ b/app/controllers/buyers/invoices_controller.rb @@ -40,7 +40,7 @@ def create def update @invoice = @account.invoices.find(params[:id]) - if @invoice.update(params[:invoice]) + if @invoice.update(invoice_params) redirect_to admin_buyers_account_invoice_url(@account, @invoice), success: t('.success') else render :edit @@ -52,4 +52,8 @@ def update def find_account @account = current_account.buyers.find(params[:account_id]) end + + def invoice_params + params.require(:invoice).permit(:period) + end end diff --git a/app/controllers/finance/api/invoices_controller.rb b/app/controllers/finance/api/invoices_controller.rb index 5b37cbd237..b12bb98e89 100644 --- a/app/controllers/finance/api/invoices_controller.rb +++ b/app/controllers/finance/api/invoices_controller.rb @@ -51,7 +51,7 @@ def charge # Invoice Update # PUT /api/invoices/{id}.xml def update - invoice.update(invoice_params_update, without_protection: true) + invoice.update(invoice_params_update) respond_with(invoice) end diff --git a/app/controllers/finance/provider/invoices_controller.rb b/app/controllers/finance/provider/invoices_controller.rb index ee89baa6ec..3bde8274e0 100644 --- a/app/controllers/finance/provider/invoices_controller.rb +++ b/app/controllers/finance/provider/invoices_controller.rb @@ -61,7 +61,7 @@ def create end def update - if @invoice.update(params[:invoice]) + if @invoice.update(invoice_params) redirect_to admin_finance_invoice_url(@invoice), success: t('.success') else render :edit @@ -96,4 +96,8 @@ def find_buyer(options = {}) def find_invoice @invoice = collection.find(params[:id]) end + + def invoice_params + @invoice_params ||= params.require(:invoice).permit(:friendly_id, :period) + end end diff --git a/app/controllers/provider/admin/account/payment_gateways/braintree_blue_controller.rb b/app/controllers/provider/admin/account/payment_gateways/braintree_blue_controller.rb index a1157455dd..caf52c4485 100644 --- a/app/controllers/provider/admin/account/payment_gateways/braintree_blue_controller.rb +++ b/app/controllers/provider/admin/account/payment_gateways/braintree_blue_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Provider::Admin::Account::PaymentGateways::BraintreeBlueController < Provider::Admin::Account::BaseController after_action :check_multiple_payment_failures, only: [:hosted_success] @@ -7,8 +9,7 @@ class Provider::Admin::Account::PaymentGateways::BraintreeBlueController < Provi prepend_before_action :deny_on_premises activate_menu :account, :billing, :payment_details - def show - end + def show; end def edit current_account.require_billing_information! @@ -33,8 +34,7 @@ def update end def hosted_success - customer_info = params.require(:customer).permit!.to_h - braintree_response = braintree_blue_crypt.confirm(customer_info, params.require(:braintree).require(:nonce)) + braintree_response = braintree_blue_crypt.confirm(customer_params.to_h, params.require(:braintree).require(:nonce)) @payment_result = braintree_response&.success? if @payment_result @@ -97,4 +97,9 @@ def hack_errors current_account.billing_address.errors.instance_variable_set('@errors', new_errors) end + + def customer_params + billing_address_params = %i[company street_address postal_code locality region country_name] + params.require(:customer).permit(:first_name, :last_name, :phone, credit_card: { billing_address: billing_address_params }) + end end diff --git a/app/controllers/provider/admin/cms/groups_controller.rb b/app/controllers/provider/admin/cms/groups_controller.rb index 53b85c3026..c89af41778 100644 --- a/app/controllers/provider/admin/cms/groups_controller.rb +++ b/app/controllers/provider/admin/cms/groups_controller.rb @@ -1,7 +1,10 @@ +# frozen_string_literal: true + class Provider::Admin::CMS::GroupsController < Provider::Admin::CMS::BaseController - before_action :available_groups, :only => [:edit, :new ] - before_action :available_sections, :only => [:edit, :new ] + before_action :available_groups, :only => %i[edit new] + before_action :available_sections, :only => %i[edit new] before_action :authorize_groups + before_action :validate_section_ids, only: %i[create update] activate_menu :audience, :cms, :groups @@ -24,7 +27,7 @@ def edit end def create - @group = current_account.provided_groups.build(sections_params) + @group = current_account.provided_groups.build(group_params) if @group.save redirect_to({ action: :index }, success: t('.success')) @@ -38,7 +41,7 @@ def create def update @group = current_account.provided_groups.find(params[:id]) - if @group.update(sections_params) + if @group.update(group_params) redirect_to({ action: :index }, success: t('.success')) else available_groups @@ -53,32 +56,30 @@ def destroy redirect_to({ action: :index }, success: t('.success')) end - - protected - def sections_params - params[:cms_group].dup.tap do |params| - if section_ids = params[:section_ids].presence - section_ids.reject!(&:empty?) - params[:sections] = section_ids.map do |sec_id| - current_account.sections.find(sec_id.to_i) - end - end - end - + def group_params + @group_params ||= params.require(:cms_group).permit(:name, section_ids: []) end def available_groups - @available_groups= current_account.provided_groups + @available_groups = current_account.provided_groups end def available_sections - @available_sections= current_account.provided_sections + @available_sections = current_account.provided_sections end def authorize_groups authorize! :manage, :groups end + # Ensure that all section IDs provided in the parameters belong to the account, which is the group owner + # Providing a non-existent section ID, or the one that belongs to another provider results in a Not Found error + def validate_section_ids + section_ids = group_params[:section_ids]&.reject(&:empty?)&.uniq + return if section_ids.blank? + + render_error(:not_found, status: :not_found) unless current_account.sections.where(id: section_ids).count == section_ids.size + end end diff --git a/app/controllers/provider/admin/cms/redirects_controller.rb b/app/controllers/provider/admin/cms/redirects_controller.rb index 2da32dfb18..e88f2dc16c 100644 --- a/app/controllers/provider/admin/cms/redirects_controller.rb +++ b/app/controllers/provider/admin/cms/redirects_controller.rb @@ -16,7 +16,7 @@ def edit end def create - @redirect = redirects.build(params[:cms_redirect]) + @redirect = redirects.build(redirect_params) if @redirect.save redirect_to({ action: :index }, success: t('.success')) @@ -26,7 +26,7 @@ def create end def update - if redirect.update(params[:cms_redirect]) + if redirect.update(redirect_params) redirect_to provider_admin_cms_redirects_path, success: t('.success') else render :edit @@ -48,4 +48,7 @@ def redirects @_redirects ||= current_account.redirects end + def redirect_params + @redirect_params ||= params.require(:cms_redirect).permit(:source, :target) + end end diff --git a/app/controllers/provider/admin/messages/inbox_controller.rb b/app/controllers/provider/admin/messages/inbox_controller.rb index 6db4c1309b..3d9f7ee4fb 100644 --- a/app/controllers/provider/admin/messages/inbox_controller.rb +++ b/app/controllers/provider/admin/messages/inbox_controller.rb @@ -26,7 +26,7 @@ def destroy def reply reply = @message.reply - reply.attributes = message_params + reply.assign_attributes(message_params.merge(origin: "web")) if reply.save && reply.deliver redirect_to({ action: :index }, success: t('.success')) @@ -43,7 +43,7 @@ def mark_as_read private def message_params - params.fetch(:message).merge(:origin => "web") + params.require(:message).permit(:subject, :body) end def find_message diff --git a/app/controllers/provider/admin/onboarding/wizard/product_controller.rb b/app/controllers/provider/admin/onboarding/wizard/product_controller.rb index e167d6192d..49200ffd91 100644 --- a/app/controllers/provider/admin/onboarding/wizard/product_controller.rb +++ b/app/controllers/provider/admin/onboarding/wizard/product_controller.rb @@ -26,6 +26,6 @@ def update private def service_params - params.require(:service) + params.require(:service).permit(:name) end end diff --git a/app/controllers/provider/admin/webhooks_controller.rb b/app/controllers/provider/admin/webhooks_controller.rb index a185636913..db8c3f31f8 100644 --- a/app/controllers/provider/admin/webhooks_controller.rb +++ b/app/controllers/provider/admin/webhooks_controller.rb @@ -39,7 +39,7 @@ def show end def create - @webhook ||= current_account.build_web_hook(params[:web_hook]) + @webhook ||= current_account.build_web_hook(webhook_params) if @webhook.save redirect_to({ action: :edit }, success: t('.success')) @@ -49,7 +49,7 @@ def create end def update - if @webhook.update(params[:web_hook]) + if @webhook.update(webhook_params) redirect_to({ action: :edit }, success: t('.success')) else flash.now[:danger] = t('.error') @@ -59,6 +59,10 @@ def update protected + def webhook_params + params.require(:web_hook).permit(%w[url active provider_actions] + WebHook.switchable_attributes) + end + def find_webhook @webhook = current_account.web_hook end diff --git a/app/controllers/sites/developer_portals_controller.rb b/app/controllers/sites/developer_portals_controller.rb index c2e0b6aa99..6694405700 100644 --- a/app/controllers/sites/developer_portals_controller.rb +++ b/app/controllers/sites/developer_portals_controller.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Sites::DeveloperPortalsController < Sites::BaseController activate_menu :audience, :cms @@ -17,7 +19,7 @@ def update private def settings_params - params[:settings] + params.require(:settings).permit(:cms_escape_draft_html, :cms_escape_published_html) end def settings diff --git a/app/controllers/sites/forums_controller.rb b/app/controllers/sites/forums_controller.rb index a145e80a08..00f352dee4 100644 --- a/app/controllers/sites/forums_controller.rb +++ b/app/controllers/sites/forums_controller.rb @@ -8,7 +8,7 @@ def edit end def update - if @settings.update(params[:settings]) + if @settings.update(settings_params) redirect_to edit_admin_site_forum_url, success: t('.success') else flash.now[:danger] = t('.error') @@ -29,4 +29,8 @@ def find_settings def active_settings_menu activate_menu :audience, :cms, :forum_settings end + + def settings_params + params.require(:settings).permit(:forum_enabled, :forum_public, :anonymous_posts_enabled) + end end diff --git a/app/controllers/sites/settings_controller.rb b/app/controllers/sites/settings_controller.rb index 1564c475fa..ca7feedb1c 100644 --- a/app/controllers/sites/settings_controller.rb +++ b/app/controllers/sites/settings_controller.rb @@ -2,7 +2,6 @@ class Sites::SettingsController < Sites::BaseController provider_required before_action :find_settings - before_action :find_service, :only => [:edit, :policies, :accessrules] layout 'provider' activate_menu :audience, :finance, :credit_card_policies @@ -11,22 +10,23 @@ def show redirect_to :action => :edit end - def edit - end - - def accessrules - end + def edit; end def update - if @settings.update(params[:settings]) + if @settings.update(settings_params) redirect_to edit_admin_site_settings_path, success: t('.success') else - render :accessrules + flash.now[:danger] = t('.error') + render action: 'edit' end end private + def settings_params + params.require(:settings).permit(:cc_terms_path, :cc_privacy_path, :cc_refunds_path) + end + def find_settings @settings = current_account.settings end diff --git a/app/controllers/sites/usage_rules_controller.rb b/app/controllers/sites/usage_rules_controller.rb index 4e714db860..3d0acbd8b6 100644 --- a/app/controllers/sites/usage_rules_controller.rb +++ b/app/controllers/sites/usage_rules_controller.rb @@ -7,7 +7,7 @@ def edit end def update - if @settings.update(params[:settings]) + if @settings.update(settings_params) redirect_back_or_to admin_site_settings_url, success: t('.success') else render :edit @@ -20,4 +20,13 @@ def find_settings @settings = current_account.settings end + def settings_params + allowed_attrs = %i[ + useraccountarea_enabled signups_enabled public_search + account_plans_ui_visible change_account_plan_permission + service_plans_ui_visible change_service_plan_permission + account_approval_required hide_service cas_server_url + ] + params.require(:settings).permit(*allowed_attrs) + end end diff --git a/app/lib/switches.rb b/app/lib/switches.rb index 6851e07beb..c2d9d01802 100644 --- a/app/lib/switches.rb +++ b/app/lib/switches.rb @@ -164,7 +164,6 @@ def assign_switches(*switch_names) included do SWITCHES.each do |name| attr_name = "#{name}_switch" - attr_protected attr_name # Switches State Machine # diff --git a/app/lib/system_name.rb b/app/lib/system_name.rb index d6d16fef97..3ac3ff95af 100644 --- a/app/lib/system_name.rb +++ b/app/lib/system_name.rb @@ -20,7 +20,6 @@ def validates_system_name(opts = {}) end def has_system_name(opts = {}) - attr_protected :system_name if opts.delete(:protected) self._system_human_name = opts.delete(:human_name) || :name validates_system_name(opts) end diff --git a/app/models/access_token.rb b/app/models/access_token.rb index 7cdaeb0cb3..461396d7b0 100644 --- a/app/models/access_token.rb +++ b/app/models/access_token.rb @@ -106,8 +106,6 @@ def self.allowed_scopes after_initialize :generate_if_missing, if: :new_record? - attr_accessible :owner, :name, :scopes, :permission, :expires_at - def self.compute_digest(plaintext_value) return nil if plaintext_value.blank? diff --git a/app/models/alert.rb b/app/models/alert.rb index c38bb312d0..1738492338 100644 --- a/app/models/alert.rb +++ b/app/models/alert.rb @@ -15,8 +15,6 @@ class Alert < ApplicationRecord has_one :user_account, through: :cinstance - attr_protected :account_id, :cinstance_id, :tenant_id - validates :account, :timestamp, :state, presence: true validates :utilization, :level, :alert_id, :cinstance, presence: true validates :alert_id, uniqueness: { scope: :account_id, case_sensitive: true } diff --git a/app/models/api_docs/service.rb b/app/models/api_docs/service.rb index b3cb34f33f..57a4c39afc 100644 --- a/app/models/api_docs/service.rb +++ b/app/models/api_docs/service.rb @@ -8,7 +8,6 @@ class ApiDocs::Service < ApplicationRecord belongs_to :account, required: true belongs_to :service, class_name: '::Service', inverse_of: :api_docs_services - attr_accessible :account, :body, :name, :description, :published, :skip_swagger_validations attr_readonly :system_name self.table_name = "api_docs_services" diff --git a/app/models/application_key.rb b/app/models/application_key.rb index ef467d2227..f6f6093dab 100644 --- a/app/models/application_key.rb +++ b/app/models/application_key.rb @@ -7,8 +7,6 @@ class ApplicationKey < ApplicationRecord belongs_to :application, :class_name => 'Cinstance', :inverse_of => :application_keys - attr_accessible :application, :value - validates :application, presence: true # The following characters are accepted: diff --git a/app/models/authentication_provider/redhat_customer_portal.rb b/app/models/authentication_provider/redhat_customer_portal.rb index 9ba81721ff..bf9bdf20b0 100644 --- a/app/models/authentication_provider/redhat_customer_portal.rb +++ b/app/models/authentication_provider/redhat_customer_portal.rb @@ -5,8 +5,6 @@ class AuthenticationProvider::RedhatCustomerPortal < AuthenticationProvider::Key CONFIG_ATTRIBUTES = %i[client_id client_secret realm system_name skip_ssl_certificate_verification].freeze private_constant :CONFIG_ATTRIBUTES - attr_accessible(*CONFIG_ATTRIBUTES, :account) - def self.build(options = {}) config = ThreeScale.config.redhat_customer_portal.to_h .slice(*CONFIG_ATTRIBUTES) diff --git a/app/models/authentication_provider/service_discovery_provider.rb b/app/models/authentication_provider/service_discovery_provider.rb index 500223504a..c0d39ac8cd 100644 --- a/app/models/authentication_provider/service_discovery_provider.rb +++ b/app/models/authentication_provider/service_discovery_provider.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class AuthenticationProvider::ServiceDiscoveryProvider < AuthenticationProvider - attr_accessible :account, :token_url, :authorize_url, :user_info_url, :client_id, :client_secret, :system_name, - :kind, :skip_ssl_certificate_verification def self.build(options = {}) new do |provider| diff --git a/app/models/backend_event.rb b/app/models/backend_event.rb index a57a3e1bf1..7711211e2a 100644 --- a/app/models/backend_event.rb +++ b/app/models/backend_event.rb @@ -2,8 +2,6 @@ class BackendEvent < ApplicationRecord serialize :data - attr_accessible :id, :data - validates :id, presence: true validates :data, length: { maximum: 65535 } end diff --git a/app/models/cms/group.rb b/app/models/cms/group.rb index 3894acb668..91079d32d0 100644 --- a/app/models/cms/group.rb +++ b/app/models/cms/group.rb @@ -1,6 +1,4 @@ class CMS::Group < ApplicationRecord - attr_accessible :provider, :sections, :name, :accounts - # This is BuyerGroup self.table_name = :cms_groups diff --git a/app/models/cms/group_section.rb b/app/models/cms/group_section.rb index 4f478cbcbd..5be54918e3 100644 --- a/app/models/cms/group_section.rb +++ b/app/models/cms/group_section.rb @@ -1,7 +1,6 @@ class CMS::GroupSection < ApplicationRecord self.table_name = :cms_group_sections - attr_accessible :section, :group belongs_to :group, :class_name => 'CMS::Group' belongs_to :section, :class_name => 'CMS::Section' end diff --git a/app/models/cms/permission.rb b/app/models/cms/permission.rb index e228b10781..72950f735c 100644 --- a/app/models/cms/permission.rb +++ b/app/models/cms/permission.rb @@ -1,11 +1,10 @@ -class CMS::Permission < ApplicationRecord - attr_accessible :group, :account +# frozen_string_literal: true +class CMS::Permission < ApplicationRecord self.table_name = :cms_permissions validates :name, length: { maximum: 255 } belongs_to :group, :class_name => 'CMS::Group', inverse_of: :permissions belongs_to :account, inverse_of: :permissions - end diff --git a/app/models/cms/redirect.rb b/app/models/cms/redirect.rb index 5a69d8d5c7..b763aab170 100644 --- a/app/models/cms/redirect.rb +++ b/app/models/cms/redirect.rb @@ -7,8 +7,6 @@ class CMS::Redirect < ApplicationRecord validates :source, uniqueness: { scope: [:provider_id], case_sensitive: true }, length: { maximum: 255 } validates :target, length: { maximum: 255 } - attr_accessible :source, :target - include NormalizePathAttribute verify_path_format :source, :target end diff --git a/app/models/feature.rb b/app/models/feature.rb index 0f3f8c5985..efd7c1a8e2 100644 --- a/app/models/feature.rb +++ b/app/models/feature.rb @@ -23,8 +23,6 @@ class Feature < ApplicationRecord where(:scope => object.class.model_name.to_s) end - attr_protected :featurable_id, :featurable_type, :tenant_id, :audit_ids - validates :system_name, uniqueness: { scope: [:featurable_id, :featurable_type], case_sensitive: true } validates :system_name, :name, length: { maximum: 255 } diff --git a/app/models/features_plan.rb b/app/models/features_plan.rb index e11fccc147..68400a8aab 100644 --- a/app/models/features_plan.rb +++ b/app/models/features_plan.rb @@ -18,8 +18,6 @@ def hash [self.class, feature_id, plan_id].hash end - attr_protected :plan_id, :feature_id, :plan_type, :tenant_id - private def feature_scope_matches_plan_class? diff --git a/app/models/fields_definition.rb b/app/models/fields_definition.rb index ddf5663d7b..1103613b4e 100644 --- a/app/models/fields_definition.rb +++ b/app/models/fields_definition.rb @@ -48,7 +48,6 @@ def self.editable_by(user) default_scope { by_position } scope :by_position, -> { order(:pos) } - attr_protected :account_id, :tenant_id attr_readonly :account_id, :tenant_id, :target, :name # The attribute `pos` is exposed as `position` in the API diff --git a/app/models/finance/billing_strategy.rb b/app/models/finance/billing_strategy.rb index 2a73a3b719..73326c6eca 100644 --- a/app/models/finance/billing_strategy.rb +++ b/app/models/finance/billing_strategy.rb @@ -20,8 +20,6 @@ class << self belongs_to :account alias_method :provider, :account - attr_protected :account_id, :tenant_id, :audit_ids - accepts_nested_attributes_for :account validates :currency, inclusion: { in: CURRENCIES.values, message: :invalid } diff --git a/app/models/forum.rb b/app/models/forum.rb index 95199ffa06..1cecd60e33 100644 --- a/app/models/forum.rb +++ b/app/models/forum.rb @@ -27,8 +27,6 @@ class Forum < ApplicationRecord scope :ordered, -> { order('position') } - attr_protected :topics_count, :posts_count, :account_id, :tenant_id - def latest_posts posts.joins(:topic).order('posts.created_at DESC') end diff --git a/app/models/invitation.rb b/app/models/invitation.rb index 4e431b1828..bd5698338d 100644 --- a/app/models/invitation.rb +++ b/app/models/invitation.rb @@ -18,8 +18,6 @@ class Invitation < ApplicationRecord # so after first call it sets the sent_at attribute and breaks the infinite loop after_commit :notify_invitee, :on => :create, :unless => :sent? - attr_protected :accepted_at, :tenant_id - default_scope -> { ordering { System::Database.oracle? ? sent_at.desc.op('', sql('NULLS LAST')) : sent_at.desc } } scope :pending, -> { where(accepted_at: nil) } diff --git a/app/models/invoice.rb b/app/models/invoice.rb index 738ac61b17..fbe59c15f0 100644 --- a/app/models/invoice.rb +++ b/app/models/invoice.rb @@ -39,8 +39,6 @@ class InvalidInvoiceStateException < RuntimeError; end has_attached_file :pdf, url: ':url_root/:class/:id/:attachment/:style/:basename.:extension' do_not_validate_attachment_file_type :pdf - attr_accessible :provider_account, :buyer_account, :friendly_id, :period - validates :provider_account, :buyer_account, :friendly_id, presence: true validates :period, presence: { :message => 'Billing period format should be YYYY-MM' } diff --git a/app/models/line_item.rb b/app/models/line_item.rb index 1cc21d41a1..0f7eee6fd2 100644 --- a/app/models/line_item.rb +++ b/app/models/line_item.rb @@ -7,8 +7,6 @@ class LineItem < ApplicationRecord audited associated_with: :invoice, allow_mass_assignment: true - attr_accessible :name, :description, :cost, :finished_at, :quantity, :started_at - validates :name, :description, :type, :contract_type, length: { maximum: 255 } validates :type, inclusion: {in: [LineItem::PlanCost, LineItem::VariableCost].flat_map { |klass| [klass, klass.to_s]}, allow_blank: true} diff --git a/app/models/message.rb b/app/models/message.rb index be3e67bc54..52e5f5ca49 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -37,8 +37,6 @@ class Message < ApplicationRecord after_save :update_recipients after_initialize :default_values - attr_protected :tenant_id - scope :visible, -> { where(hidden_at: nil) } scope :hidden, -> { where.not(hidden_at: nil) } scope :latest_first, -> { order(created_at: :desc) } diff --git a/app/models/message_recipient.rb b/app/models/message_recipient.rb index abb92a810e..232451cd82 100644 --- a/app/models/message_recipient.rb +++ b/app/models/message_recipient.rb @@ -29,8 +29,6 @@ class MessageRecipient < ApplicationRecord validates :message_id, :kind, :receiver_id, :receiver_type, presence: true validates :receiver_type, :kind, :state, length: { maximum: 255 } - attr_protected :message_id, :receiver_id, :receiver_type, :tenant_id - # Make this class look like the actual message delegate :sender, :subject, :body, :recipients, :to, :cc, :bcc, :created_at, :sender_name, :to => :message diff --git a/app/models/metric.rb b/app/models/metric.rb index 5db3afa2b2..2e0c822d33 100644 --- a/app/models/metric.rb +++ b/app/models/metric.rb @@ -26,7 +26,6 @@ class Metric < ApplicationRecord acts_as_tree - attr_protected :service_id, :parent_id, :tenant_id, :audit_ids validates :unit, presence: true, unless: :child? validates :friendly_name, uniqueness: { scope: %i[owner_type owner_id], case_sensitive: true }, presence: true validates :system_name, :unit, :friendly_name, :owner_type, length: { maximum: 255 } diff --git a/app/models/moderatorship.rb b/app/models/moderatorship.rb index 45d095309c..4a8d9381ee 100644 --- a/app/models/moderatorship.rb +++ b/app/models/moderatorship.rb @@ -4,8 +4,6 @@ class Moderatorship < ApplicationRecord validates :user_id, :forum_id, presence: true validate :uniqueness_of_relationship - attr_protected :forum_id, :user_id, :tenant_id - protected def uniqueness_of_relationship diff --git a/app/models/onboarding.rb b/app/models/onboarding.rb index b7f3072338..0aadd9ca15 100644 --- a/app/models/onboarding.rb +++ b/app/models/onboarding.rb @@ -1,6 +1,4 @@ class Onboarding < ApplicationRecord - attr_accessible :wizard_state - belongs_to :account validates :account_id, presence: true diff --git a/app/models/partner.rb b/app/models/partner.rb index b50046f7c3..4aea0b6107 100644 --- a/app/models/partner.rb +++ b/app/models/partner.rb @@ -1,8 +1,7 @@ class Partner < ApplicationRecord include SystemName - has_system_name protected: true, uniqueness_scope: true - attr_accessible :api_key, :name + has_system_name uniqueness_scope: true has_many :providers, class_name: "Account" has_many :application_plans diff --git a/app/models/payment_detail.rb b/app/models/payment_detail.rb index 40b9cdb2f8..4600bd5f8d 100644 --- a/app/models/payment_detail.rb +++ b/app/models/payment_detail.rb @@ -12,8 +12,6 @@ class PaymentDetail < ApplicationRecord audited allow_mass_assignment: true - attr_protected :account_id, :audit_ids - validates :credit_card_partial_number, length: { :maximum => 4, :allow_blank => true, :message => "must be the final 4 digits only" } diff --git a/app/models/payment_transaction.rb b/app/models/payment_transaction.rb index 3b298bf2da..6c8f367236 100644 --- a/app/models/payment_transaction.rb +++ b/app/models/payment_transaction.rb @@ -13,8 +13,6 @@ class PaymentTransaction < ApplicationRecord validates :currency, length: {maximum: 4} validates :message, :reference, :action, length: {maximum: 255} - attr_protected :account_id, :invoice_id, :success, :test, :tenant_id - scope :failed, -> { where(:success => false) } scope :succeeded, -> { where(:success => true) } scope :oldest_first, -> { order(:created_at) } diff --git a/app/models/plan.rb b/app/models/plan.rb index 9a7fbbc09a..0a915e9d3a 100644 --- a/app/models/plan.rb +++ b/app/models/plan.rb @@ -31,8 +31,6 @@ class PeriodRangeCalculationError < StandardError; end validates :state, inclusion: { in: %w(hidden published) } before_validation(:on => :create) { set_state_to_hidden_if_nil } # otherwise the validation above fails as state machine hasnt kicked in yet (ugly) - attr_protected :issuer_id, :original_id, :type, :issuer_type, :tenant_id, :audit_ids, :state - state_machine :initial => :hidden do state :hidden state :published diff --git a/app/models/plan_metric.rb b/app/models/plan_metric.rb index 2250582d2f..d4ab8bb13b 100644 --- a/app/models/plan_metric.rb +++ b/app/models/plan_metric.rb @@ -2,8 +2,6 @@ class PlanMetric < ApplicationRecord belongs_to :plan, :polymorphic => true belongs_to :metric - attr_protected :plan_id, :metric_id, :plan_type, :tenant_id - validates :plan, presence: true validates :metric, presence: true end diff --git a/app/models/post.rb b/app/models/post.rb index 83cc5fcecf..7ccb462b8d 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -4,8 +4,6 @@ class Post < ApplicationRecord after_commit(:on => :destroy) { update_cached_fields } after_commit :update_topic_delta_index - attr_accessible :body, :markup_type, :anonymous_user - include Indices::TopicIndex::ForPost # author of post diff --git a/app/models/pricing_rule.rb b/app/models/pricing_rule.rb index 4ff59d202d..f8b631968f 100644 --- a/app/models/pricing_rule.rb +++ b/app/models/pricing_rule.rb @@ -5,8 +5,6 @@ class PricingRule < ApplicationRecord belongs_to :plan belongs_to :metric - attr_protected :metric_id, :plan_id, :plan_type, :tenant_id, :audit_ids - # TODO: add validations that check that the intervals are well defined. TODO: # min must be always > 0 # diff --git a/app/models/profile.rb b/app/models/profile.rb index 81463cfc9f..fe5b4df767 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -13,8 +13,6 @@ class Profile < ApplicationRecord # with the account validation that causes profile#valid? to be false before_save :validate_presence_of_account - attr_protected :account_id, :tenant_id, :audit_ids - after_initialize :set_company_size audited diff --git a/app/models/referrer_filter.rb b/app/models/referrer_filter.rb index d4c719c958..15041784b5 100644 --- a/app/models/referrer_filter.rb +++ b/app/models/referrer_filter.rb @@ -12,8 +12,6 @@ class ReferrerFilter < ApplicationRecord validate :keys_limit_reached - attr_accessible :application, :value - attr_readonly :value after_commit :destroy_backend_value, on: :destroy, unless: :destroyed_by_association diff --git a/app/models/service.rb b/app/models/service.rb index 413e0597ac..3f693e22c1 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -69,8 +69,6 @@ class Service < ApplicationRecord # rubocop:disable Metrics/ClassLength belongs_to :default_service_plan, class_name: 'ServicePlan' belongs_to :default_application_plan, class_name: 'ApplicationPlan' - attr_protected :account_id, :tenant_id, :audit_ids - # LEGACY alias plans application_plans diff --git a/app/models/settings.rb b/app/models/settings.rb index 4ca2fb1040..17ee8c8c3f 100644 --- a/app/models/settings.rb +++ b/app/models/settings.rb @@ -4,8 +4,6 @@ class Settings < ApplicationRecord audited allow_mass_assignment: true - attr_protected :account_id, :tenant_id, :product, :audit_ids, :sso_key - validates :product, inclusion: { in: %w[connect enterprise].freeze } validates :change_account_plan_permission, :change_service_plan_permission, inclusion: { in: %w[request none credit_card request_credit_card direct].freeze } validates :bg_colour, :link_colour, :text_colour, :menu_bg_colour, :link_label, :link_url, :menu_link_colour, :token_api, diff --git a/app/models/sso_token.rb b/app/models/sso_token.rb index ee8a11075e..64bd7c12ef 100644 --- a/app/models/sso_token.rb +++ b/app/models/sso_token.rb @@ -1,10 +1,11 @@ +# frozen_string_literal: true + class SSOToken include ActiveModel::Validations - include ActiveModel::MassAssignmentSecurity + include ActiveModel::ForbiddenAttributesProtection include ActiveModel::Serializers::Xml attr_accessor :user_id, :username, :expires_in, :redirect_url, :protocol, :account - attr_accessible :user_id, :username, :expires_in, :redirect_url, :protocol attr_reader :encrypted_token, :expires_at validates :expires_in, :numericality => { :only_integer => true, :greater_than => 30.seconds, :less_than_or_equal_to => 1.day, :allow_nil => true } @@ -15,9 +16,9 @@ class SSOToken validate :one_of_user_id_or_username_is_required validate :account_is_provider_and_user_of_provider, :if => Proc.new {|o| o.account && o.user_id || o.username } - def initialize(**attributes) - assign_attributes({:expires_in => 10.minutes, :protocol => 'https'}.merge(attributes)) - @new_record= true + def initialize(attributes = {}) + assign_attributes(attributes.reverse_merge(expires_in: 10.minutes, protocol: 'https')) + @new_record = true end def save @@ -47,9 +48,9 @@ def to_xml options = {} xml.to_xml end - def assign_attributes values - sanitize_for_mass_assignment(values, nil).each do |k, v| - send("#{k}=", v) + def assign_attributes(values) + sanitize_for_mass_assignment(values).each do |key, value| + send("#{key}=", value) end end @@ -75,42 +76,42 @@ def sso_url!(host: nil, port: nil) protected - def generate_token - @expires_at= Time.now.utc + expires_in.to_i - @encrypted_token= ThreeScale::SSO::Encryptor.new(account.settings.sso_key, expires_at.to_i).encrypt_token user_id, username - @new_record= false - end + def generate_token + @expires_at= Time.now.utc + expires_in.to_i + @encrypted_token= ThreeScale::SSO::Encryptor.new(account.settings.sso_key, expires_at.to_i).encrypt_token user_id, username + @new_record= false + end private - def parsable_redirect_url - Addressable::URI.parse redirect_url - rescue - errors.add :redirect_url, :invalid - end + def parsable_redirect_url + Addressable::URI.parse redirect_url + rescue + errors.add :redirect_url, :invalid + end - def one_of_user_id_or_username_is_required - if user_id.nil? && username.nil? - errors.add :base, :one_of_user_id_or_username_is_required - end + def one_of_user_id_or_username_is_required + if user_id.nil? && username.nil? + errors.add :base, :one_of_user_id_or_username_is_required end + end - def account_is_provider_and_user_of_provider - unless account.is_a?(Account) && account.provider? - errors.add :account, :invalid - return - end + def account_is_provider_and_user_of_provider + unless account.is_a?(Account) && account.provider? + errors.add :account, :invalid + return + end - # if we have a user-id and we can't find the user for this provider we fail to generate the token - unless user_id.blank? - if account.managed_users.find_by_id(user_id).nil? - errors.add :user_id, :invalid - end - return + # if we have a user-id and we can't find the user for this provider we fail to generate the token + unless user_id.blank? + if account.managed_users.find_by_id(user_id).nil? + errors.add :user_id, :invalid end + return + end - if account.managed_users.find_by_username(username).nil? - errors.add :username, :invalid - end + if account.managed_users.find_by_username(username).nil? + errors.add :username, :invalid end + end end diff --git a/app/models/topic.rb b/app/models/topic.rb index bc3057c672..702337d40e 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -61,7 +61,6 @@ def self.with_post_by(user) end attr_accessor :body, :quiz, :quiz_id, :first_name, :last_name, :email, :human, :markup_type, :anonymous_user - attr_accessible :markup_type, :title, :body, :quiz, :quiz_id, :first_name, :last_name, :email, :tag_list, :anonymous_user, :tag_list attr_readonly :posts_count, :hits diff --git a/app/models/topic_category.rb b/app/models/topic_category.rb index 5df6d7693e..429fe1f4e6 100644 --- a/app/models/topic_category.rb +++ b/app/models/topic_category.rb @@ -3,8 +3,6 @@ class TopicCategory < ApplicationRecord #TODO: Rails 4 changes to has_many …, -> { order(…) } has_many :topics, -> { sticky_first.last_updated_first }, :foreign_key => :category_id - attr_protected :forum_id, :tenant_id - validates :name, presence: true, length: {maximum: 255} validates :name, uniqueness: { scope: :forum_id, case_sensitive: true } diff --git a/app/models/usage_limit.rb b/app/models/usage_limit.rb index 7adc72e2aa..d6c3fd6f04 100644 --- a/app/models/usage_limit.rb +++ b/app/models/usage_limit.rb @@ -1,6 +1,5 @@ class UsageLimit < ApplicationRecord include Symbolize - attr_accessible :period, :value, :metric, :plan include Backend::ModelExtensions::UsageLimit diff --git a/app/models/user_topic.rb b/app/models/user_topic.rb index 4eec433390..a59a19fc7c 100644 --- a/app/models/user_topic.rb +++ b/app/models/user_topic.rb @@ -3,8 +3,6 @@ class UserTopic < ApplicationRecord belongs_to :user belongs_to :topic - attr_protected :user_id, :topic_id, :tenant_id - validates :user, presence: true validates :topic, presence: true diff --git a/app/models/web_hook.rb b/app/models/web_hook.rb index dd373b5bc8..1a975939e4 100644 --- a/app/models/web_hook.rb +++ b/app/models/web_hook.rb @@ -3,8 +3,6 @@ class WebHook < ApplicationRecord alias provider account - attr_protected :account_id, :tenant_id - validates :account_id, presence: true validates :url, format: { :with => URI::DEFAULT_PARSER.make_regexp(%w[http https]), :if => :active }, length: { maximum: 255 } diff --git a/app/services/policies_config_params.rb b/app/services/policies_config_params.rb index 26b32afa99..27caf43e44 100644 --- a/app/services/policies_config_params.rb +++ b/app/services/policies_config_params.rb @@ -12,7 +12,7 @@ def call return policies_config_params if json_param? [policies_config_params].flatten.map do |policies_config| - policies_config.try(:permit!) + policies_config.try(:permit, :name, :version, :enabled, :removable, :id, :humanName, configuration: {}) end end diff --git a/app/views/provider/admin/cms/redirects/index.html.erb b/app/views/provider/admin/cms/redirects/index.html.erb index 8e32257606..7b6b5fe5da 100644 --- a/app/views/provider/admin/cms/redirects/index.html.erb +++ b/app/views/provider/admin/cms/redirects/index.html.erb @@ -20,8 +20,8 @@
<% @redirects.each do |redirect| %>