Merge pull request #4355 from 3scale/THREESCALE-11522-fix-validate-readonly-fields

THREESCALE-11522: Backport fix for CVE-2024-12125 [2.15]

Author: jlledom

app/models/fields_definition.rb

@@ -24,6 +24,7 @@ def self.push_target(klass)
   scope :by_target, ->(class_name) { where(['target = ?', class_name])}
   scope :by_name, ->(name) { where(['name = ?', name])}
   scope :required, -> { where({ :required => true })}
+  scope :read_only, ->(read_only = true) { where(read_only: read_only) }
 
   def self.editable_by(user)
     select{ |fd| fd.editable_by?(user) }

features/developer_portal/admin/applications/extra_fields.feature

@@ -63,6 +63,28 @@ Feature: Developer portal application extra fields
       | Plate number |
       | Stealth      |
 
+  Scenario: Read-only extra fields are ignored if received
+    Given the following application:
+      | Buyer | Name       | Product |
+      | Jane  | Jane's App | The API |
+    And the application has the following extra fields:
+      | Description | It's a car |
+      | Engine      | 120        |
+      | Wheels      | 4          |
+      | Color       | White      |
+    When the buyer logs in
+    And the provider has the field "plate_number" for applications as editable
+    And they go to application "Jane's App" dev portal edit page
+    And the provider has the field "plate_number" for applications as read only
+    And the form is submitted with:
+      | Description  | It's a car |
+      | Engine       | 120        |
+      | Wheels       | 4          |
+      | Color        | White      |
+      | Plate number | edit me    |
+    Then they should see the flash message "Application was successfully updated"
+    And they should not see "Plate number"
+
   Scenario: Extra fields validation
     When they go to the dev portal new application page
     And the form is submitted with:

features/old/accounts/buyers/account_fields.feature

@@ -46,6 +46,18 @@ Feature: Buyer side, account fields
        And I press "Update"
      Then I should see "The account information was updated."
 
+  Scenario: Read-only extra fields are ignored on update
+    Given I log in as "bob" on foo.3scale.localhost
+    And provider "foo.3scale.localhost" has the field "non_editable" for accounts as editable
+    Then I go to the account edit page
+    And provider "foo.3scale.localhost" has the field "non_editable" for accounts as read only
+    And the form is submitted with:
+      | Required field | value  |
+      | False field    |        |
+      | Choices field  |      3 |
+      | Non editable   | edited |
+    Then I should see "The account information was updated."
+    Then I should not see "Non editable"
 
   Scenario: Viewing account with extra fields
     #TODO ugly table change this

features/step_definitions/provider_steps.rb

@@ -61,6 +61,12 @@ def import_simple_layout(provider)
   a.save!
 end
 
+Given "{provider} has the field {string} for {field_definition_target} as {read_only_status}" do |provider, name, target, read_only|
+  a = provider.fields_definitions.by_target(target).find { |fd| fd.name == name }
+  a.read_only = read_only
+  a.save!
+end
+
 Given "{provider} allows to change account plan {change_plan_permission}" do |provider, plan_permission|
   provider.set_change_account_plan_permission! plan_permission
 end

features/support/parameter_types.rb

@@ -469,3 +469,14 @@ def find_metric(metrics, name)
   regexp: /(.*)/,
   transformer: ->(selector) { selector_for(selector) }
 )
+
+ParameterType(
+  name: 'read_only_status',
+  regexp: /(editable|read only)/,
+  transformer: ->(value) do
+    {
+      'editable' => false,
+      'read only' => true,
+    }[value]
+  end
+)

lib/developer_portal/app/controllers/developer_portal/accounts/invitee_signups_controller.rb

@@ -119,7 +119,7 @@ def check_invitation!
   end
 
   def build_user
-    @user = @invitation.make_user(params[:user] || {})
+    @user = @invitation.make_user(filter_readonly_params(params[:user], User))
   end
 
   def invitation_token

lib/developer_portal/app/controllers/developer_portal/admin/account/personal_details_controller.rb

@@ -46,7 +46,7 @@ def deny_unless_can_update
   end
 
   def user_params
-    params.require(:user).permit([:current_password] +
+    filter_readonly_params(params.require(:user), User).permit([:current_password] +
                                    resource.special_fields +
                                    resource.defined_fields.map(&:name))
   end

lib/developer_portal/app/controllers/developer_portal/admin/account/users_controller.rb

@@ -53,7 +53,7 @@ def collection
 
   def update_resource(user, attributes)
     attributes.each do |attrs|
-      user.attributes = attrs
+      user.attributes = filter_readonly_params(attrs, User)
       user.role = attrs[:role] if can? :update_role, user
     end
     user.save

lib/developer_portal/app/controllers/developer_portal/admin/accounts_controller.rb

@@ -35,7 +35,7 @@ def update
   private
 
   def account_params
-    params.require(:account)
+    filter_readonly_params(params.require(:account), Account)
   end
 
   def find_countries

lib/developer_portal/app/controllers/developer_portal/admin/applications_controller.rb

@@ -172,11 +172,12 @@ def accepted_application_params
     # cinstance[*] naming is present for legacy reasons
     application_attributes = params[:application] || params[:cinstance]
     return {} unless application_attributes
+
     permitted_params = fields_definitions + %i[plan_id redirect_url]
     application_attributes.permit(*permitted_params)
   end
 
   def fields_definitions
-    FieldsDefinition.by_provider(site_account).by_target('Cinstance').pluck(:name)
+    FieldsDefinition.by_provider(site_account).by_target('Cinstance').read_only(false).pluck(:name)
   end
 end