diff --git a/Gemfile b/Gemfile index f19be3a..a83e02c 100644 --- a/Gemfile +++ b/Gemfile @@ -17,6 +17,8 @@ gem 'redis', '>= 4.0.1' gem 'sassc-rails' gem "sentry-ruby" gem "sentry-rails" +# Required when Sentry profiles_sample_rate is set (see config/initializers/sentry.rb) +gem 'stackprof' gem 'skylight' gem 'stimulus-rails' gem 'turbo-rails' @@ -40,6 +42,8 @@ group :development, :test do gem 'pry-rails' gem 'rails-controller-testing' gem 'rspec-rails' + gem "rubocop", "~> 1.86" + gem "rubocop-rails" gem 'simplecov' gem 'webdrivers' end diff --git a/Gemfile.lock b/Gemfile.lock index bfe47d8..34c0cb0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -92,6 +92,7 @@ GEM arbre (1.7.0) activesupport (>= 3.0.0) ruby2_keywords (>= 0.0.2) + ast (2.4.3) base64 (0.3.0) bcrypt (3.1.22) benchmark (0.5.0) @@ -187,6 +188,7 @@ GEM thor (>= 0.14, < 2.0) jsbundling-rails (1.3.1) railties (>= 6.0.0) + json (2.19.3) kaminari (1.2.2) activesupport (>= 4.1.0) kaminari-actionview (= 1.2.2) @@ -199,6 +201,7 @@ GEM activerecord kaminari-core (= 1.2.2) kaminari-core (1.2.2) + language_server-protocol (3.17.0.5) launchy (3.1.1) addressable (~> 2.8) childprocess (~> 5.0) @@ -210,6 +213,7 @@ GEM letter_opener (~> 1.9) railties (>= 6.1) rexml + lint_roller (1.1.0) logger (1.7.0) loofah (2.25.1) crass (~> 1.0.2) @@ -251,10 +255,15 @@ GEM racc (~> 1.4) orm_adapter (0.5.0) ostruct (0.6.3) + parallel (1.28.0) + parser (3.3.11.1) + ast (~> 2.4.1) + racc pg (1.5.9) pp (0.6.2) prettyprint prettyprint (0.2.0) + prism (1.9.0) pry (0.14.2) coderay (~> 1.1) method_source (~> 1.0) @@ -313,6 +322,7 @@ GEM thor (~> 1.0, >= 1.2.2) tsort (>= 0.2) zeitwerk (~> 2.6) + rainbow (3.1.1) rake (13.2.1) ransack (4.4.1) activerecord (>= 7.2) @@ -349,6 +359,27 @@ GEM rspec-mocks (~> 3.13) rspec-support (~> 3.13) rspec-support (3.13.2) + rubocop (1.86.0) + json (~> 2.3) + language_server-protocol (~> 3.17.0.2) + lint_roller (~> 1.1.0) + parallel (~> 1.10) + parser (>= 3.3.0.2) + rainbow (>= 2.2.2, < 4.0) + regexp_parser (>= 2.9.3, < 3.0) + rubocop-ast (>= 1.49.0, < 2.0) + ruby-progressbar (~> 1.7) + unicode-display_width (>= 2.4.0, < 4.0) + rubocop-ast (1.49.1) + parser (>= 3.3.7.2) + prism (~> 1.7) + rubocop-rails (2.34.3) + activesupport (>= 4.2.0) + lint_roller (~> 1.1) + rack (>= 1.1) + rubocop (>= 1.75.0, < 2.0) + rubocop-ast (>= 1.44.0, < 2.0) + ruby-progressbar (1.13.0) ruby2_keywords (0.0.5) rubyzip (2.4.1) sassc (2.4.0) @@ -385,6 +416,7 @@ GEM actionpack (>= 6.1) activesupport (>= 6.1) sprockets (>= 3.0.0) + stackprof (0.2.28) stimulus-rails (1.3.4) railties (>= 6.0.0) stringio (3.1.5) @@ -398,6 +430,9 @@ GEM tzinfo (2.0.6) concurrent-ruby (~> 1.0) unaccent (0.4.0) + unicode-display_width (3.2.0) + unicode-emoji (~> 4.1) + unicode-emoji (4.2.0) useragent (0.16.11) warden (1.2.9) rack (>= 2.0.9) @@ -452,11 +487,14 @@ DEPENDENCIES rails-controller-testing redis (>= 4.0.1) rspec-rails + rubocop (~> 1.86) + rubocop-rails sassc-rails sentry-rails sentry-ruby simplecov skylight + stackprof stimulus-rails turbo-rails tzinfo-data diff --git a/app/admin/dashboard.rb b/app/admin/dashboard.rb index 541913d..beda553 100644 --- a/app/admin/dashboard.rb +++ b/app/admin/dashboard.rb @@ -36,7 +36,8 @@ column do current_year_applications_scope = Application.active_conference_applications current_year_application_count = current_year_applications_scope.count - recent_applications = current_year_applications_scope.order(created_at: :desc).limit(25) + recent_applications = + current_year_applications_scope.order(created_at: :desc).limit(25) panel "Latest 25 of #{current_year_application_count} Applications for the #{ApplicationSetting.get_current_app_year} conference" do table_for recent_applications do @@ -69,10 +70,28 @@ column do panel "Recent Payments" do - table_for Payment.current_conference_payments.sort.reverse.first(10) do + recent_payments = + Payment.current_conference_payments + .order(created_at: :desc) + .limit(10) + .includes(:user) + .to_a + conf_year = ApplicationSetting.get_current_app_year + user_ids = recent_payments.map(&:user_id).uniq + current_app_by_user_id = + if user_ids.empty? + {} + else + Application.where(user_id: user_ids, conf_year: conf_year) + .order(:user_id, :id) + .group_by(&:user_id) + .transform_values(&:last) + end + table_for recent_payments do column("Name") do |a| - if a.user.current_conf_application.present? - link_to a.user.current_conf_application.display_name, admin_application_path(a.user.current_conf_application) + app = current_app_by_user_id[a.user_id] + if app.present? + link_to app.display_name, admin_application_path(app) else "#{a.user.email} ( - waiting for application to be submitted)" end @@ -128,7 +147,7 @@ panel "Waiting for responses from these #{ApplicationSetting.get_current_app_year} applicants (#{offered_count})" do applications = if offered_applications.respond_to?(:sort) - offered_applications.sort.reverse + offered_applications.includes(:user).sort.reverse else Array(offered_applications).select { |app| app.respond_to?(:user) } end diff --git a/app/admin/payment_gateway_callbacks.rb b/app/admin/payment_gateway_callbacks.rb new file mode 100644 index 0000000..7fddcab --- /dev/null +++ b/app/admin/payment_gateway_callbacks.rb @@ -0,0 +1,55 @@ +ActiveAdmin.register PaymentGatewayCallback do + menu parent: 'User Management', priority: 2.5, label: 'Gateway callbacks' + + actions :index, :show + + config.sort_order = 'created_at_desc' + + filter :transaction_id + filter :processing_status, as: :select, collection: PaymentGatewayCallback::PROCESSING_STATUSES + filter :event_type + filter :user + filter :payment + filter :created_at + + index do + actions + column :id do |callback| + link_to callback.id, admin_payment_gateway_callback_path(callback) + end + column :transaction_id + column :processing_status + column :event_type + column :user + column :payment + column :failure_reason do |callback| + truncate(callback.failure_reason.to_s, length: 80) + end + column :created_at + end + + show do + attributes_table do + row :id + row :transaction_id + row :processing_status + row :event_type + row :user do |callback| + if callback.user + link_to(callback.user.email, admin_user_path(callback.user)) + end + end + row :payment do |callback| + if callback.payment + link_to("Payment ##{callback.payment.id}", admin_payment_path(callback.payment)) + end + end + row :failure_reason + row :created_at + row :updated_at + row :payload do |callback| + pre JSON.pretty_generate(callback.payload.presence || {}) + end + end + end +end diff --git a/app/admin/payments.rb b/app/admin/payments.rb index e68dad4..cecc6ce 100644 --- a/app/admin/payments.rb +++ b/app/admin/payments.rb @@ -75,6 +75,26 @@ row :created_at row :updated_at end + + panel 'Gateway callbacks' do + callbacks = payment.payment_gateway_callbacks.order(created_at: :desc) + if callbacks.any? + table_for callbacks do + column :id do |callback| + link_to(callback.id, admin_payment_gateway_callback_path(callback)) + end + column :processing_status + column :transaction_id + column :failure_reason do |callback| + truncate(callback.failure_reason.to_s, length: 60) + end + column :created_at + end + else + para 'No gateway callback rows are linked to this payment. Callbacks are attached when a gateway receipt is successfully recorded for this row.' + end + end + active_admin_comments end diff --git a/app/controllers/payments_controller.rb b/app/controllers/payments_controller.rb index 0f5df1f..0b0c93a 100644 --- a/app/controllers/payments_controller.rb +++ b/app/controllers/payments_controller.rb @@ -1,173 +1,166 @@ - require 'digest' - require 'time' - - class PaymentsController < ApplicationController - MAX_PAYMENT_AMOUNT = 2000 - - protect_from_forgery with: :exception - skip_before_action :verify_authenticity_token, only: [:payment_receipt] - before_action :verify_payment_callback, only: [:payment_receipt] - before_action :authenticate_user!, except: [:delete_manual_payment] - before_action :current_user, only: %i[payment_receipt make_payment payment_show] - before_action :current_application, only: %i[payment_receipt payment_show] - before_action :authenticate_admin_user!, only: [:delete_manual_payment] - prepend_before_action :verify_authenticity_token, only: [:delete_manual_payment] - - def index - redirect_to root_url +require 'digest' +require 'time' + +class PaymentsController < ApplicationController + MAX_PAYMENT_AMOUNT = 2000 + + protect_from_forgery with: :exception + skip_before_action :verify_authenticity_token, only: [:payment_receipt] + before_action :verify_payment_callback, only: [:payment_receipt] + + before_action :authenticate_user!, except: %i[delete_manual_payment payment_receipt] + before_action :current_user, only: %i[make_payment payment_show] + before_action :current_application, only: %i[payment_show] + + before_action :authenticate_admin_user!, only: [:delete_manual_payment] + prepend_before_action :verify_authenticity_token, only: [:delete_manual_payment] + + def index + redirect_to root_url + end + + def payment_receipt + result = Payments::GatewayReceiptRecorder.new(callback_params: url_params).call + + case result.status + when :duplicate + redirect_to all_payments_path + when :recorded + redirect_to all_payments_path, notice: 'Your Payment Was Successfully Recorded' + when :forbidden + head :forbidden + else + head :unprocessable_entity end + end - def payment_receipt - if Payment.pluck(:transaction_id).include?(params['transactionId']) - redirect_to all_payments_path - else - Payment.create( - transaction_type: params['transactionType'], - transaction_status: params['transactionStatus'], - transaction_id: params['transactionId'], - total_amount: params['transactionTotalAmount'], - transaction_date: params['transactionDate'], - account_type: params['transactionAcountType'], - result_code: params['transactionResultCode'], - result_message: params['transactionResultMessage'], - user_account: params['orderNumber'], - payer_identity: @current_user.email, - timestamp: params['timestamp'], - transaction_hash: params['hash'], - user_id: current_user.id, - conf_year: ApplicationSetting.get_current_app_year - ) - @current_application.update(offer_status: "registration_accepted") - redirect_to all_payments_path, notice: "Your Payment Was Successfully Recorded" - end + def make_payment + amount = validated_payment_amount(params['amount']) + if amount.nil? + redirect_to all_payments_path, alert: 'Please enter a valid payment amount.' + return end - def make_payment - amount = validated_payment_amount(params['amount']) - if amount.nil? - redirect_to all_payments_path, alert: 'Please enter a valid payment amount.' - return - end + processed_url = generate_hash(current_user, amount) + redirect_to processed_url, allow_other_host: true + end + + def payment_show + redirect_to root_url unless user_has_payments?(current_user) + @users_current_payments = Payment.current_conference_payments.where(user_id: current_user.id) + @ttl_paid = Payment.current_conference_payments.where(user_id: current_user.id, transaction_status: '1').pluck(:total_amount).map(&:to_f).sum / 100 + @has_subscription = @current_application.subscription + @cost_subscription = @current_application.subscription_cost + @total_cost = @current_application.total_cost + @balance_due = @total_cost - @ttl_paid + @max_payment_amount = max_payment_amount_for(@balance_due) + end - processed_url = generate_hash(current_user, amount) - redirect_to processed_url, allow_other_host: true + def delete_manual_payment + @payment = Payment.find(params[:id]) + @payment.destroy + respond_to do |format| + format.html { redirect_to admin_payments_url, notice: 'Payment was successfully deleted.' } + format.json { head :no_content } end + end - def payment_show - redirect_to root_url unless user_has_payments?(current_user) - @users_current_payments = Payment.current_conference_payments.where(user_id: current_user ) - @ttl_paid = Payment.current_conference_payments.where(user_id: current_user, transaction_status: '1').pluck(:total_amount).map(&:to_f).sum / 100 - @has_subscription = @current_application.subscription - @cost_subscription = @current_application.subscription_cost - @total_cost = @current_application.total_cost - @balance_due = @total_cost - @ttl_paid - @max_payment_amount = max_payment_amount_for(@balance_due) + private + + def verify_payment_callback + unless params['hash'].present? && params['timestamp'].present? && params['transactionId'].present? && params['orderNumber'].present? + head :forbidden + return end + end - def delete_manual_payment - @payment = Payment.find(params[:id]) - @payment.destroy - respond_to do |format| - format.html { redirect_to admin_payments_url, notice: 'Payment was successfully deleted.' } - format.json { head :no_content } - end + def generate_hash(current_user, amount = 100) + user_account = current_user.email.partition('@').first + '-' + current_user.id.to_s + amount_to_be_payed = amount.to_i + if Rails.env.development? || Rails.env.staging? || Rails.application.credentials.NELNET_SERVICE[:SERVICE_SELECTOR] == 'QA' + key_to_use = 'test_key' + url_to_use = 'test_URL' + else + key_to_use = 'prod_key' + url_to_use = 'prod_URL' end - private - def verify_payment_callback - unless params['hash'].present? && params['timestamp'].present? && params['transactionId'].present? - head :forbidden - return - end - end + connection_hash = { + 'test_key' => Rails.application.credentials.NELNET_SERVICE[:DEVELOPMENT_KEY], + 'test_URL' => Rails.application.credentials.NELNET_SERVICE[:DEVELOPMENT_URL], + 'prod_key' => Rails.application.credentials.NELNET_SERVICE[:PRODUCTION_KEY], + 'prod_URL' => Rails.application.credentials.NELNET_SERVICE[:PRODUCTION_URL] + } + + redirect_url = connection_hash[url_to_use] + current_epoch_time = DateTime.now.strftime('%Q').to_i + initial_hash = { + 'orderNumber' => user_account, + 'orderType' => 'English Department Online', + 'orderDescription' => 'Bearriver Conference Fees', + 'amountDue' => amount_to_be_payed * 100, + 'redirectUrl' => redirect_url, + 'redirectUrlParameters' => 'transactionType,transactionStatus,transactionId,transactionTotalAmount,transactionDate,transactionAcountType,transactionResultCode,transactionResultMessage,orderNumber', + 'retriesAllowed' => 1, + 'timestamp' => current_epoch_time, + 'key' => connection_hash[key_to_use] + } + + # Sample Hash Creation + hash_to_be_encoded = initial_hash.values.map { |v| "#{v}" }.join('') + encoded_hash = Digest::SHA256.hexdigest hash_to_be_encoded + + # Final URL + url_for_payment = initial_hash.map { |k, v| "#{k}=#{v}&" unless k == 'key' }.join('') + connection_hash[url_to_use] + '?' + url_for_payment + 'hash=' + encoded_hash + end - def generate_hash(current_user, amount=100) - user_account = current_user.email.partition('@').first + '-' + current_user.id.to_s - amount_to_be_payed = amount.to_i - if Rails.env.development? || Rails.env.staging? || Rails.application.credentials.NELNET_SERVICE[:SERVICE_SELECTOR] == "QA" - key_to_use = 'test_key' - url_to_use = 'test_URL' - else - key_to_use = 'prod_key' - url_to_use = 'prod_URL' - end - - connection_hash = { - 'test_key' => Rails.application.credentials.NELNET_SERVICE[:DEVELOPMENT_KEY], - 'test_URL' => Rails.application.credentials.NELNET_SERVICE[:DEVELOPMENT_URL], - 'prod_key' => Rails.application.credentials.NELNET_SERVICE[:PRODUCTION_KEY], - 'prod_URL' => Rails.application.credentials.NELNET_SERVICE[:PRODUCTION_URL] - } - - redirect_url = connection_hash[url_to_use] - current_epoch_time = DateTime.now.strftime("%Q").to_i - initial_hash = { - 'orderNumber' => user_account, - 'orderType' => 'English Department Online', - 'orderDescription' => 'Bearriver Conference Fees', - 'amountDue' => amount_to_be_payed * 100, - 'redirectUrl' => redirect_url, - 'redirectUrlParameters' => 'transactionType,transactionStatus,transactionId,transactionTotalAmount,transactionDate,transactionAcountType,transactionResultCode,transactionResultMessage,orderNumber', - 'retriesAllowed' => 1, - 'timestamp' => current_epoch_time, - 'key' => connection_hash[key_to_use] - } - - # Sample Hash Creation - hash_to_be_encoded = initial_hash.values.map{|v| "#{v}"}.join('') - encoded_hash = Digest::SHA256.hexdigest hash_to_be_encoded - - # Final URL - url_for_payment = initial_hash.map{|k,v| "#{k}=#{v}&" unless k == 'key'}.join('') - final_url = connection_hash[url_to_use] + '?' + url_for_payment + 'hash=' + encoded_hash - end + def validated_payment_amount(raw_amount) + return nil if raw_amount.respond_to?(:blank?) ? raw_amount.blank? : raw_amount.nil? - def validated_payment_amount(raw_amount) - return nil if raw_amount.respond_to?(:blank?) ? raw_amount.blank? : raw_amount.nil? + amount = Integer(raw_amount, exception: false) + if amount.nil? + begin + amount = BigDecimal(raw_amount.to_s).to_i + rescue ArgumentError + return nil + end + end + return nil if amount <= 0 - amount = Integer(raw_amount, exception: false) - if amount.nil? - begin - amount = BigDecimal(raw_amount.to_s).to_i - rescue ArgumentError - return nil - end - end - return nil if amount <= 0 + balance_due = current_balance_due + return nil if balance_due <= 0 - balance_due = current_balance_due - return nil if balance_due <= 0 + max_amount = max_payment_amount_for(balance_due) + return nil if amount > max_amount - max_amount = max_payment_amount_for(balance_due) - return nil if amount > max_amount + amount + end - amount - end + def max_payment_amount_for(balance_due) + [balance_due.floor, MAX_PAYMENT_AMOUNT].min + end - def max_payment_amount_for(balance_due) - [balance_due.floor, MAX_PAYMENT_AMOUNT].min - end + def current_balance_due + current_application + return 0.0 if @current_application.nil? - def current_balance_due - current_application - return 0.0 if @current_application.nil? - - total_cost = begin - @current_application.total_cost - rescue StandardError => e - Rails.logger.error("Error computing total_cost for application #{@current_application.id}: #{e.class}: #{e.message}") if defined?(Rails) && Rails.respond_to?(:logger) - 0.0 - end - total_cost = total_cost.to_f - total_paid = Payment.current_conference_payments.where(user_id: current_user, transaction_status: '1').pluck(:total_amount).map(&:to_f).sum / 100 - total_cost - total_paid - end + total_cost = begin + @current_application.total_cost + rescue StandardError => e + Rails.logger.error("Error computing total_cost for application #{@current_application.id}: #{e.class}: #{e.message}") if defined?(Rails) && Rails.respond_to?(:logger) + 0.0 + end + total_cost = total_cost.to_f + total_paid = Payment.current_conference_payments.where(user_id: current_user.id, transaction_status: '1').pluck(:total_amount).map(&:to_f).sum / 100 + total_cost - total_paid + end - def url_params - params.permit(:amount, :transactionType, :transactionStatus, :transactionId, :transactionTotalAmount, :transactionDate, :transactionAcountType, :transactionResultCode, :transactionResultMessage, :orderNumber, :timestamp, :hash, :conf_year) - end + def url_params + params.permit(:amount, :transactionType, :transactionStatus, :transactionId, :transactionTotalAmount, :transactionDate, :transactionAcountType, :transactionResultCode, :transactionResultMessage, :orderNumber, :timestamp, :hash, :conf_year) + end - def current_application - @current_application = Application.active_conference_applications.find_by(user_id: current_user) - end + def current_application + @current_application = Application.active_conference_applications.find_by(user_id: current_user.id) end +end diff --git a/app/models/payment.rb b/app/models/payment.rb index 68699bd..4748d45 100644 --- a/app/models/payment.rb +++ b/app/models/payment.rb @@ -26,12 +26,13 @@ class Payment < ApplicationRecord validates :transaction_date, presence: true validates :account_type, presence: true, if: :manual_entry? belongs_to :user + has_many :payment_gateway_callbacks, dependent: :nullify, inverse_of: :payment validate :manual_payment_decimal validate :valid_transaction_date before_save :check_manual_amount def self.ransackable_associations(auth_object = nil) - ["user"] + ["user", "payment_gateway_callbacks"] end def self.ransackable_attributes(auth_object = nil) diff --git a/app/models/payment_gateway_callback.rb b/app/models/payment_gateway_callback.rb new file mode 100644 index 0000000..1d43b41 --- /dev/null +++ b/app/models/payment_gateway_callback.rb @@ -0,0 +1,36 @@ +# == Schema Information +# +# Table name: payment_gateway_callbacks +# +# id :bigint not null, primary key +# event_type :string default("receipt"), not null +# failure_reason :text +# payload :jsonb not null +# processing_status :string not null +# transaction_id :string +# created_at :datetime not null +# updated_at :datetime not null +# payment_id :bigint +# user_id :bigint +# +class PaymentGatewayCallback < ApplicationRecord + PROCESSING_STATUSES = %w[recorded duplicate rejected error].freeze + + belongs_to :payment, optional: true, inverse_of: :payment_gateway_callbacks + belongs_to :user, optional: true + + validates :processing_status, presence: true, inclusion: { in: PROCESSING_STATUSES } + validates :event_type, presence: true + validates :payload, presence: true + + def self.ransackable_associations(auth_object = nil) + %w[payment user] + end + + def self.ransackable_attributes(auth_object = nil) + %w[ + created_at event_type failure_reason id id_value payload payment_id + processing_status transaction_id updated_at user_id + ] + end +end diff --git a/app/services/payments/gateway_receipt_recorder.rb b/app/services/payments/gateway_receipt_recorder.rb new file mode 100644 index 0000000..ce28041 --- /dev/null +++ b/app/services/payments/gateway_receipt_recorder.rb @@ -0,0 +1,127 @@ +module Payments + class GatewayReceiptRecorder + # payment is only set for :recorded; other outcomes pass payment: nil explicitly for clarity. + Result = Struct.new(:status, :payment, keyword_init: true) + + # Matches generate_hash: "-" + # Greedy .+ anchors the final "-" segment as user id (supports hyphens in local part). + ORDER_NUMBER_PATTERN = /\A(.+)-(\d+)\z/ + + def initialize(callback_params:) + @callback_params = callback_params.to_h + end + + def call + callback_user = resolve_user_from_order_number + return callback_user if callback_user.is_a?(Result) + + if Payment.exists?(transaction_id: transaction_id) + return duplicate_result(callback_user) + end + + payment = Payment.new(payment_attributes(callback_user)) + begin + if payment.save + current_application_for(callback_user)&.update(offer_status: 'registration_accepted') + record_callback('recorded', callback_user, payment) + Result.new(status: :recorded, payment: payment) + elsif duplicate_transaction_id_failure?(payment) + duplicate_result(callback_user) + else + record_callback('error', callback_user, nil, payment.errors.full_messages.join(', ')) + Result.new(status: :error, payment: nil) + end + rescue ActiveRecord::RecordNotUnique + duplicate_result(callback_user) + end + end + + private + + attr_reader :callback_params + + def transaction_id + callback_params['transactionId'] + end + + def resolve_user_from_order_number + parsed = parse_order_number(callback_params['orderNumber']) + return reject_callback('invalid_order_number') if parsed.is_a?(Symbol) + + prefix, user_id = parsed + user = User.find_by(id: user_id) + return reject_callback('user_not_found') if user.nil? + + email_local = user.email.to_s.partition('@').first + return reject_callback('order_number_mismatch') unless email_local == prefix + + user + end + + # Returns :invalid_format, :invalid_user_id, or [email_local_prefix, user_id Integer] + def parse_order_number(raw) + order_number = raw.to_s.strip + match = order_number.match(ORDER_NUMBER_PATTERN) + return :invalid_format unless match + + prefix = match[1] + return :invalid_format if prefix.blank? + + user_id = Integer(match[2], 10) + return :invalid_user_id if user_id <= 0 + + [prefix, user_id] + rescue ArgumentError, TypeError + :invalid_user_id + end + + def current_application_for(user) + Application.active_conference_applications.find_by(user_id: user.id) + end + + def payment_attributes(user) + { + transaction_type: callback_params['transactionType'], + transaction_status: callback_params['transactionStatus'], + transaction_id: transaction_id, + total_amount: callback_params['transactionTotalAmount'], + transaction_date: callback_params['transactionDate'], + account_type: callback_params['transactionAcountType'], + result_code: callback_params['transactionResultCode'], + result_message: callback_params['transactionResultMessage'], + user_account: callback_params['orderNumber'], + payer_identity: user.email, + timestamp: callback_params['timestamp'], + transaction_hash: callback_params['hash'], + user_id: user.id, + conf_year: ApplicationSetting.get_current_app_year + } + end + + def reject_callback(reason) + record_callback('rejected', nil, nil, reason) + Result.new(status: :forbidden, payment: nil) + end + + def duplicate_result(callback_user) + record_callback('duplicate', callback_user) + Result.new(status: :duplicate, payment: nil) + end + + def duplicate_transaction_id_failure?(payment) + payment.errors.of_kind?(:transaction_id, :taken) + end + + def record_callback(status, user = nil, payment = nil, reason = nil) + PaymentGatewayCallback.create( + user: user, + payment: payment, + transaction_id: transaction_id, + processing_status: status, + event_type: 'receipt', + failure_reason: reason, + payload: callback_params + ) + end + end +end diff --git a/db/migrate/20260406114103_create_payment_gateway_callbacks.rb b/db/migrate/20260406114103_create_payment_gateway_callbacks.rb new file mode 100644 index 0000000..4d1561e --- /dev/null +++ b/db/migrate/20260406114103_create_payment_gateway_callbacks.rb @@ -0,0 +1,18 @@ +class CreatePaymentGatewayCallbacks < ActiveRecord::Migration[7.2] + def change + create_table :payment_gateway_callbacks do |t| + t.references :payment, foreign_key: true + t.references :user, foreign_key: true + t.string :transaction_id + t.string :processing_status, null: false + t.string :event_type, null: false, default: 'receipt' + t.text :failure_reason + t.jsonb :payload, null: false, default: {} + + t.timestamps + end + + add_index :payment_gateway_callbacks, :transaction_id + add_index :payment_gateway_callbacks, :processing_status + end +end diff --git a/db/migrate/20260406171925_add_unique_index_on_payments_transaction_id.rb b/db/migrate/20260406171925_add_unique_index_on_payments_transaction_id.rb new file mode 100644 index 0000000..5a9d6f8 --- /dev/null +++ b/db/migrate/20260406171925_add_unique_index_on_payments_transaction_id.rb @@ -0,0 +1,9 @@ +# Enforces transaction_id uniqueness at the DB level (concurrent callbacks). +# If this migration fails, resolve duplicate transaction_id values in payments first. +class AddUniqueIndexOnPaymentsTransactionId < ActiveRecord::Migration[7.2] + disable_ddl_transaction! + + def change + add_index :payments, :transaction_id, unique: true, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 924afa0..4707b16 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2026_03_31_160000) do +ActiveRecord::Schema[7.2].define(version: 2026_04_06_171925) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -171,6 +171,22 @@ t.boolean "active", default: true end + create_table "payment_gateway_callbacks", force: :cascade do |t| + t.bigint "payment_id" + t.bigint "user_id" + t.string "transaction_id" + t.string "processing_status", null: false + t.string "event_type", default: "receipt", null: false + t.text "failure_reason" + t.jsonb "payload", default: {}, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["payment_id"], name: "index_payment_gateway_callbacks_on_payment_id" + t.index ["processing_status"], name: "index_payment_gateway_callbacks_on_processing_status" + t.index ["transaction_id"], name: "index_payment_gateway_callbacks_on_transaction_id" + t.index ["user_id"], name: "index_payment_gateway_callbacks_on_user_id" + end + create_table "payments", force: :cascade do |t| t.string "transaction_type" t.string "transaction_status" @@ -188,6 +204,7 @@ t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false t.integer "conf_year" + t.index ["transaction_id"], name: "index_payments_on_transaction_id", unique: true t.index ["user_id"], name: "index_payments_on_user_id" end @@ -222,5 +239,7 @@ add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id" add_foreign_key "applications", "partner_registrations" add_foreign_key "applications", "users" + add_foreign_key "payment_gateway_callbacks", "payments" + add_foreign_key "payment_gateway_callbacks", "users" add_foreign_key "payments", "users" end diff --git a/spec/controllers/payments_controller_spec.rb b/spec/controllers/payments_controller_spec.rb index 29c46ea..4b41aea 100644 --- a/spec/controllers/payments_controller_spec.rb +++ b/spec/controllers/payments_controller_spec.rb @@ -47,7 +47,7 @@ transactionAcountType: 'registration', transactionResultCode: '0', transactionResultMessage: 'Approved', - orderNumber: 'testuser-123', + orderNumber: "#{user.email.partition('@').first}-#{user.id}", timestamp: Time.current.to_i.to_s, hash: 'valid_hash_here' } @@ -136,9 +136,9 @@ end context 'when user is not authenticated' do - it 'redirects to sign in page' do + it 'processes callback without requiring user sign in' do post :payment_receipt, params: valid_params - expect(response).to redirect_to(new_user_session_path) + expect(response).to redirect_to(all_payments_path) end end end @@ -333,7 +333,7 @@ transactionAcountType: 'registration', transactionResultCode: '0', transactionResultMessage: 'Approved', - orderNumber: 'testuser-123' + orderNumber: "#{user.email.partition('@').first}-#{user.id}" } expect(response).not_to have_http_status(:forbidden) end @@ -351,7 +351,7 @@ transactionAcountType: 'registration', transactionResultCode: '0', transactionResultMessage: 'Approved', - orderNumber: 'testuser-123' + orderNumber: "#{user.email.partition('@').first}-#{user.id}" } expect(response).to have_http_status(:forbidden) end @@ -369,7 +369,7 @@ transactionAcountType: 'registration', transactionResultCode: '0', transactionResultMessage: 'Approved', - orderNumber: 'testuser-123' + orderNumber: "#{user.email.partition('@').first}-#{user.id}" } expect(response).to have_http_status(:forbidden) end @@ -387,7 +387,25 @@ transactionAcountType: 'registration', transactionResultCode: '0', transactionResultMessage: 'Approved', - orderNumber: 'testuser-123' + orderNumber: "#{user.email.partition('@').first}-#{user.id}" + } + expect(response).to have_http_status(:forbidden) + end + end + + context 'with missing orderNumber' do + it 'returns forbidden status' do + post :payment_receipt, params: { + hash: 'valid_hash', + timestamp: '1234567890', + transactionId: 'test_123', + transactionType: 'Credit', + transactionStatus: '1', + transactionTotalAmount: '10000', + transactionDate: '202401011200', + transactionAcountType: 'registration', + transactionResultCode: '0', + transactionResultMessage: 'Approved' } expect(response).to have_http_status(:forbidden) end @@ -472,7 +490,7 @@ transactionAcountType: 'registration', transactionResultCode: '0', transactionResultMessage: 'Approved', - orderNumber: 'testuser-123', + orderNumber: "#{user.email.partition('@').first}-#{user.id}", timestamp: Time.current.to_i.to_s, hash: 'valid_hash_here' } diff --git a/spec/factories/payment_gateway_callbacks.rb b/spec/factories/payment_gateway_callbacks.rb new file mode 100644 index 0000000..75c098a --- /dev/null +++ b/spec/factories/payment_gateway_callbacks.rb @@ -0,0 +1,15 @@ +FactoryBot.define do + factory :payment_gateway_callback do + processing_status { 'recorded' } + event_type { 'receipt' } + transaction_id { "txn_#{SecureRandom.hex(4)}" } + payload do + { + 'transactionId' => transaction_id, + 'timestamp' => Time.current.to_i.to_s, + 'hash' => SecureRandom.hex(8), + 'orderNumber' => "user-#{create(:user).id}" + } + end + end +end diff --git a/spec/models/payment_gateway_callback_spec.rb b/spec/models/payment_gateway_callback_spec.rb new file mode 100644 index 0000000..5ef424c --- /dev/null +++ b/spec/models/payment_gateway_callback_spec.rb @@ -0,0 +1,16 @@ +require 'rails_helper' + +RSpec.describe PaymentGatewayCallback, type: :model do + describe 'validations' do + it 'is valid with valid attributes' do + callback = build(:payment_gateway_callback) + expect(callback).to be_valid + end + + it 'is invalid with unsupported processing_status' do + callback = build(:payment_gateway_callback, processing_status: 'unknown_status') + expect(callback).not_to be_valid + expect(callback.errors[:processing_status]).to include('is not included in the list') + end + end +end diff --git a/spec/models/payment_spec.rb b/spec/models/payment_spec.rb index 894345c..cce438f 100644 --- a/spec/models/payment_spec.rb +++ b/spec/models/payment_spec.rb @@ -93,7 +93,7 @@ describe 'class methods' do describe '.ransackable_associations' do it 'returns the correct associations' do - expect(Payment.ransackable_associations).to match_array(["user"]) + expect(Payment.ransackable_associations).to match_array(%w[user payment_gateway_callbacks]) end end diff --git a/spec/requests/payments_spec.rb b/spec/requests/payments_spec.rb index 1097f38..0309546 100644 --- a/spec/requests/payments_spec.rb +++ b/spec/requests/payments_spec.rb @@ -161,12 +161,11 @@ "transactionAcountType" => "registration", "transactionResultCode" => "0", "transactionResultMessage" => "Approved", - "orderNumber" => "user-1" + "orderNumber" => "#{user.email.partition('@').first}-#{user.id}" } end before do - sign_in user allow_any_instance_of(PaymentsController).to receive(:verify_payment_callback).and_return(true) mock_application = instance_double(Application) @@ -183,10 +182,33 @@ end it "does not create a new payment and redirects to all_payments_path" do - post payment_receipt_path, params: payment_params + expect { + post payment_receipt_path, params: payment_params + }.to change(PaymentGatewayCallback, :count).by(1) expect(response).to redirect_to(all_payments_path) + expect(PaymentGatewayCallback.order(:id).last.processing_status).to eq('duplicate') + end + end + + context "when callback user cannot be resolved from orderNumber" do + it "returns forbidden status" do + payment_params["orderNumber"] = "missing-user-999999" + expect { + post payment_receipt_path, params: payment_params + }.to change(PaymentGatewayCallback, :count).by(1) + expect(response).to have_http_status(:forbidden) + expect(PaymentGatewayCallback.order(:id).last.processing_status).to eq('rejected') end end + + it "creates a payment and records a callback audit row" do + expect { + post payment_receipt_path, params: payment_params + }.to change(Payment, :count).by(1).and change(PaymentGatewayCallback, :count).by(1) + + expect(response).to redirect_to(all_payments_path) + expect(PaymentGatewayCallback.order(:id).last.processing_status).to eq('recorded') + end end end diff --git a/spec/services/payments/gateway_receipt_recorder_spec.rb b/spec/services/payments/gateway_receipt_recorder_spec.rb new file mode 100644 index 0000000..09ac80e --- /dev/null +++ b/spec/services/payments/gateway_receipt_recorder_spec.rb @@ -0,0 +1,114 @@ +require 'rails_helper' + +RSpec.describe Payments::GatewayReceiptRecorder, type: :service do + let(:user) { create(:user) } + let(:application) { create(:application, user: user) } + let(:callback_params) do + { + 'transactionType' => 'Credit', + 'transactionStatus' => '1', + 'transactionId' => 'service_txn_123', + 'transactionTotalAmount' => '10000', + 'transactionDate' => Time.current.strftime('%m/%d/%Y'), + 'transactionAcountType' => 'registration', + 'transactionResultCode' => '0', + 'transactionResultMessage' => 'Approved', + 'orderNumber' => "#{user.email.partition('@').first}-#{user.id}", + 'timestamp' => Time.current.to_i.to_s, + 'hash' => 'valid_hash' + } + end + + before do + create(:application_setting, active_application: true, contest_year: Time.current.year) + allow(ApplicationSetting).to receive(:get_current_app_year).and_return(Time.current.year) + application + end + + describe '#call' do + it 'records a new payment and creates a recorded callback audit' do + result = described_class.new(callback_params: callback_params).call + + expect(result.status).to eq(:recorded) + expect(Payment.find_by(transaction_id: 'service_txn_123')).to be_present + + callback = PaymentGatewayCallback.order(:id).last + expect(callback.processing_status).to eq('recorded') + expect(callback.user_id).to eq(user.id) + expect(callback.payment_id).to be_present + expect(callback.transaction_id).to eq('service_txn_123') + end + + it 'returns duplicate and creates duplicate callback audit when transaction already exists' do + create(:payment, transaction_id: 'service_txn_123', user: user) + + result = described_class.new(callback_params: callback_params).call + + expect(result.status).to eq(:duplicate) + callback = PaymentGatewayCallback.order(:id).last + expect(callback.processing_status).to eq('duplicate') + expect(callback.user_id).to eq(user.id) + expect(callback.payment_id).to be_nil + end + + it 'returns duplicate when uniqueness validation fails (stale exists? check)' do + create(:payment, transaction_id: 'service_txn_123', user: user) + allow(Payment).to receive(:exists?).with(transaction_id: 'service_txn_123').and_return(false) + + result = described_class.new(callback_params: callback_params).call + + expect(result.status).to eq(:duplicate) + expect(PaymentGatewayCallback.order(:id).last.processing_status).to eq('duplicate') + end + + it 'returns duplicate when insert raises RecordNotUnique (concurrent callbacks)' do + allow_any_instance_of(Payment).to receive(:save).and_raise(ActiveRecord::RecordNotUnique.new('duplicate key')) + + result = described_class.new(callback_params: callback_params).call + + expect(result.status).to eq(:duplicate) + expect(PaymentGatewayCallback.order(:id).last.processing_status).to eq('duplicate') + end + + it 'returns forbidden and creates rejected callback audit when user cannot be resolved' do + invalid_params = callback_params.merge('orderNumber' => 'missing-user-999999') + + result = described_class.new(callback_params: invalid_params).call + + expect(result.status).to eq(:forbidden) + callback = PaymentGatewayCallback.order(:id).last + expect(callback.processing_status).to eq('rejected') + expect(callback.failure_reason).to eq('user_not_found') + expect(callback.user_id).to be_nil + end + + it 'returns forbidden when orderNumber does not match "-" format' do + invalid_params = callback_params.merge('orderNumber' => 'not-valid') + + result = described_class.new(callback_params: invalid_params).call + + expect(result.status).to eq(:forbidden) + expect(PaymentGatewayCallback.order(:id).last.failure_reason).to eq('invalid_order_number') + end + + it 'returns forbidden when user id segment is not a positive integer' do + invalid_params = callback_params.merge('orderNumber' => "#{user.email.partition('@').first}-0") + + result = described_class.new(callback_params: invalid_params).call + + expect(result.status).to eq(:forbidden) + expect(PaymentGatewayCallback.order(:id).last.failure_reason).to eq('invalid_order_number') + end + + it 'returns forbidden when email local part does not match orderNumber prefix' do + invalid_params = callback_params.merge('orderNumber' => "wrongprefix-#{user.id}") + + result = described_class.new(callback_params: invalid_params).call + + expect(result.status).to eq(:forbidden) + callback = PaymentGatewayCallback.order(:id).last + expect(callback.failure_reason).to eq('order_number_mismatch') + expect(callback.user_id).to be_nil + end + end +end