diff --git a/app/models/claim.rb b/app/models/claim.rb index 10e38dd469..f6c8f7928d 100644 --- a/app/models/claim.rb +++ b/app/models/claim.rb @@ -214,6 +214,28 @@ class Claim < ApplicationRecord .where("further_education_payments_eligibilities.verification = '{}'") end + scope :with_award_amounts, -> do + joins( + <<~SQL + JOIN ( + #{ + Policies::POLICIES.map do |policy| + " + SELECT + id, + #{policy.award_amount_column} AS award_amount, + '#{policy::Eligibility}' AS eligibility_type + FROM #{policy::Eligibility.table_name} + " + end.join(" UNION ALL ") + } + ) AS eligibilities + ON claims.eligibility_id = eligibilities.id + AND claims.eligibility_type = eligibilities.eligibility_type + SQL + ) + end + def onelogin_idv_full_name "#{onelogin_idv_first_name} #{onelogin_idv_last_name}" end diff --git a/app/models/payroll_run.rb b/app/models/payroll_run.rb index 946a745f79..2940a77433 100644 --- a/app/models/payroll_run.rb +++ b/app/models/payroll_run.rb @@ -84,48 +84,34 @@ def payments_count @payments_count ||= payments.count end + class LineItem < Struct.new(:id, :award_amount, keyword_init: true); end + def line_items(policy, filter: :all) - eligibilities_cte = "WITH eligibilities AS(" - eligibilities_cte += Policies::POLICIES.map do |policy| - <<~SQL - SELECT - id, - #{policy.award_amount_column} AS award_amount, - '#{policy::Eligibility}' AS eligibility_type - FROM #{policy::Eligibility.table_name} - SQL - end.join(" UNION ALL ") - eligibilities_cte += ")" - - sql = <<~SQL - #{eligibilities_cte} - SELECT - /* A topup is always paid in different payment/payroll_run than the main claim was */ + scope = Claim + .select( + " + DISTINCT(claims.id), COALESCE(topups.award_amount, eligibilities.award_amount) AS award_amount - FROM payments - JOIN claim_payments ON claim_payments.payment_id = payments.id - JOIN claims ON claims.id = claim_payments.claim_id - JOIN eligibilities - ON claims.eligibility_id = eligibilities.id - AND claims.eligibility_type = eligibilities.eligibility_type - LEFT JOIN topups ON topups.claim_id = claims.id - WHERE payments.payroll_run_id = '#{id}' - SQL - - unless policy == :all - sql += "\nAND claims.eligibility_type = 'Policies::#{policy}::Eligibility'" - end + " + ) + .with_award_amounts + .left_joins(:topups) + .joins(payments: :payroll_run) + .where(payroll_runs: {id: id}) + + scope = scope.by_policy(policy) unless policy == :all case filter - when :all - sql when :claims - sql += "\nAND topups.id IS NULL" + scope = scope.where(topups: {id: nil}) when :topups - sql += "\nAND topups.id IS NOT NULL" + scope = scope.where.not(topups: {id: nil}) end - ActiveRecord::Base.connection.execute(sql).map(&OpenStruct.method(:new)) + # Claim delegates it's award amount to eligibility, so we want to return + # a non claim object ensuring the award amount is from the topup if there + # is one + ActiveRecord::Base.connection.execute(scope.to_sql).map(&LineItem.method(:new)) end def ensure_no_payroll_run_this_month diff --git a/config/brakeman.ignore b/config/brakeman.ignore index 1e49f435ac..a5b10709f4 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -30,7 +30,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/models/claim/search.rb", - "line": 45, + "line": 35, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "Claim.submitted.where(\"LOWER(#{attribute}) = LOWER(?)\", search_term)", "render_path": null, @@ -115,6 +115,29 @@ ], "note": "" }, + { + "warning_type": "SQL Injection", + "warning_code": 0, + "fingerprint": "cddc34b96e07d19510d574438a1ce61b69d9c1f28c0d4e5eee631d17a9b38ba6", + "check_name": "SQL", + "message": "Possible SQL injection", + "file": "app/models/claim.rb", + "line": 222, + "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", + "code": "joins(\"JOIN (\\n #{Policies::POLICIES.map do\n \"\\n SELECT\\n id,\\n #{policy.award_amount_column} AS award_amount,\\n '#{policy::Eligibility}' AS eligibility_type\\n FROM #{policy::Eligibility.table_name}\\n \"\n end.join(\" UNION ALL \")}\\n) AS eligibilities\\nON claims.eligibility_id = eligibilities.id\\nAND claims.eligibility_type = eligibilities.eligibility_type\\n\")", + "render_path": null, + "location": { + "type": "method", + "class": "Claim", + "method": "with_award_amounts" + }, + "user_input": "Policies::POLICIES.map do\n \"\\n SELECT\\n id,\\n #{policy.award_amount_column} AS award_amount,\\n '#{policy::Eligibility}' AS eligibility_type\\n FROM #{policy::Eligibility.table_name}\\n \"\n end.join(\" UNION ALL \")", + "confidence": "Medium", + "cwe_id": [ + 89 + ], + "note": "Non of the columns used in generating the join are user provided" + }, { "warning_type": "SQL Injection", "warning_code": 0, @@ -122,7 +145,7 @@ "check_name": "SQL", "message": "Possible SQL injection", "file": "app/models/claim/matching_attribute_finder.rb", - "line": 54, + "line": 34, "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "code": "Claim.where(\"LOWER(#{\"CONCAT(#{attributes.join(\",\")})\"}) = LOWER(?)\", values_for_attributes(source_claim, attributes).join)", "render_path": null, @@ -139,6 +162,6 @@ "note": "" } ], - "updated": "2024-06-20 16:06:05 +0100", - "brakeman_version": "6.1.2" + "updated": "2024-10-14 12:08:54 +0100", + "brakeman_version": "6.2.1" }