From 0cdc3451fd30003e654f40eba5d0fa14dbf6f5c9 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Fri, 31 Jan 2020 16:42:22 +0500 Subject: [PATCH 1/9] Add InvoiceDate field to Directo model See #1416 --- app/models/directo.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/app/models/directo.rb b/app/models/directo.rb index 5f73a3f16..789db64b2 100644 --- a/app/models/directo.rb +++ b/app/models/directo.rb @@ -22,14 +22,16 @@ class Directo < ApplicationRecord counter += 1 num = invoice.number + paid_at = invoice.account_activity.bank_transaction&.paid_at&.strftime("%Y-%m-%d") mappers[num] = invoice xml.invoice( - "SalesAgent" => Setting.directo_sales_agent, - "Number" => num, - "InvoiceDate" => invoice.issue_date.strftime("%Y-%m-%d"), - "PaymentTerm" => Setting.directo_receipt_payment_term, - "Currency" => invoice.currency, - "CustomerCode"=> invoice.buyer.accounting_customer_code + "SalesAgent" => Setting.directo_sales_agent, + "Number" => num, + "InvoiceDate" => invoice.issue_date.strftime("%Y-%m-%d"), + 'TransactionDate' => paid_at, + "PaymentTerm" => Setting.directo_receipt_payment_term, + "Currency" => invoice.currency, + "CustomerCode"=> invoice.buyer.accounting_customer_code ){ xml.line( "ProductID" => Setting.directo_receipt_product_name, From 0cf2ff92a3142afbbee9a2b707b4d850da2df670 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20Erik=20=C3=95unapuu?= Date: Fri, 31 Jan 2020 12:56:58 +0200 Subject: [PATCH 2/9] Bind specific invoice when paid via Banklink / EveryPay Fix styling issues Move redundant logic to shared base --- .../registrar/payments_controller.rb | 2 ++ app/models/bank_transaction.rb | 1 + app/models/payment_orders/bank_link.rb | 13 +++++----- app/models/payment_orders/base.rb | 26 +++++++++++++++++++ app/models/payment_orders/every_pay.rb | 14 +++++----- test/models/payment_orders/bank_link_test.rb | 4 ++- test/models/payment_orders/every_pay_test.rb | 4 ++- 7 files changed, 50 insertions(+), 14 deletions(-) diff --git a/app/controllers/registrar/payments_controller.rb b/app/controllers/registrar/payments_controller.rb index 5be0d6562..8df9bb046 100644 --- a/app/controllers/registrar/payments_controller.rb +++ b/app/controllers/registrar/payments_controller.rb @@ -27,6 +27,8 @@ class Registrar opts = { response: params } @payment = ::PaymentOrders.create_with_type(params[:bank], invoice, opts) if @payment.valid_response_from_intermediary? && @payment.settled_payment? + Rails.logger.info("User paid invoice ##{invoice.number} successfully") + @payment.complete_transaction if invoice.paid? diff --git a/app/models/bank_transaction.rb b/app/models/bank_transaction.rb index d2f8bb66b..cb9aebc5e 100644 --- a/app/models/bank_transaction.rb +++ b/app/models/bank_transaction.rb @@ -48,6 +48,7 @@ class BankTransaction < ApplicationRecord end invoice = Invoice.find_by(number: invoice_no) + @registrar = invoice.buyer unless invoice errors.add(:base, I18n.t('invoice_was_not_found')) diff --git a/app/models/payment_orders/bank_link.rb b/app/models/payment_orders/bank_link.rb index e568da0df..1e4c2785a 100644 --- a/app/models/payment_orders/bank_link.rb +++ b/app/models/payment_orders/bank_link.rb @@ -51,11 +51,7 @@ module PaymentOrders def complete_transaction return unless valid_successful_transaction? - transaction = BankTransaction.find_by( - description: invoice.order, - currency: invoice.currency, - iban: invoice.seller_iban - ) + transaction = compose_or_find_transaction transaction.sum = response['VK_AMOUNT'] transaction.bank_reference = response['VK_T_NO'] @@ -65,7 +61,12 @@ module PaymentOrders transaction.paid_at = Time.parse(response["VK_T_DATETIME"]) transaction.save! - transaction.autobind_invoice + transaction.bind_invoice(invoice.number) + if transaction.errors.empty? + Rails.logger.info("Invoice ##{invoice.number} was marked as paid") + else + Rails.logger.error("Failed to bind invoice ##{invoice.number}") + end end def settled_payment? diff --git a/app/models/payment_orders/base.rb b/app/models/payment_orders/base.rb index cf0293025..772f33ba5 100644 --- a/app/models/payment_orders/base.rb +++ b/app/models/payment_orders/base.rb @@ -26,6 +26,32 @@ module PaymentOrders transaction.save! end + def compose_or_find_transaction + transaction = BankTransaction.find_by(base_transaction_params) + + # Transaction already autobinded (possibly) invalid invoice + if transaction.binded? + Rails.logger.info("Transaction #{transaction.id} is already binded") + Rails.logger.info('Creating new BankTransaction record.') + + transaction = new_base_transaction + end + + transaction + end + + def new_base_transaction + BankTransaction.new(base_transaction_params) + end + + def base_transaction_params + { + description: invoice.order, + currency: invoice.currency, + iban: invoice.seller_iban, + } + end + def form_url ENV["payments_#{type}_url"] end diff --git a/app/models/payment_orders/every_pay.rb b/app/models/payment_orders/every_pay.rb index b4ddcdf29..a866ba972 100644 --- a/app/models/payment_orders/every_pay.rb +++ b/app/models/payment_orders/every_pay.rb @@ -20,6 +20,7 @@ module PaymentOrders def valid_response_from_intermediary? return false unless response + valid_hmac? && valid_amount? && valid_account? end @@ -30,18 +31,19 @@ module PaymentOrders def complete_transaction return unless valid_response_from_intermediary? && settled_payment? - transaction = BankTransaction.find_by( - description: invoice.order, - currency: invoice.currency, - iban: invoice.seller_iban - ) + transaction = compose_or_find_transaction transaction.sum = response[:amount] transaction.paid_at = Date.strptime(response[:timestamp], '%s') transaction.buyer_name = response[:cc_holder_name] transaction.save! - transaction.autobind_invoice + transaction.bind_invoice(invoice.number) + if transaction.errors.empty? + Rails.logger.info("Invoice ##{invoice.number} marked as paid") + else + Rails.logger.error("Failed to bind invoice ##{invoice.number}") + end end private diff --git a/test/models/payment_orders/bank_link_test.rb b/test/models/payment_orders/bank_link_test.rb index f1069819c..002f488b9 100644 --- a/test/models/payment_orders/bank_link_test.rb +++ b/test/models/payment_orders/bank_link_test.rb @@ -114,7 +114,9 @@ class BankLinkTest < ActiveSupport::TestCase mock_transaction.expect(:paid_at= , Date.parse('2018-04-01 00:30:00 +0300'), [Time.parse('2018-04-01T00:30:00+0300')]) mock_transaction.expect(:buyer_name=, 'John Doe', ['John Doe']) mock_transaction.expect(:save!, true) - mock_transaction.expect(:autobind_invoice, AccountActivity.new) + mock_transaction.expect(:binded?, false) + mock_transaction.expect(:bind_invoice, AccountActivity.new, [1]) + mock_transaction.expect(:errors, []) BankTransaction.stub(:find_by, mock_transaction) do @completed_bank_link.complete_transaction diff --git a/test/models/payment_orders/every_pay_test.rb b/test/models/payment_orders/every_pay_test.rb index 202efc1b7..81e077b23 100644 --- a/test/models/payment_orders/every_pay_test.rb +++ b/test/models/payment_orders/every_pay_test.rb @@ -72,7 +72,9 @@ class EveryPayTest < ActiveSupport::TestCase mock_transaction.expect(:paid_at= , Date.strptime('1524136727', '%s'), [Date.strptime('1524136727', '%s')]) mock_transaction.expect(:buyer_name=, 'John Doe', ['John Doe']) mock_transaction.expect(:save!, true) - mock_transaction.expect(:autobind_invoice, AccountActivity.new) + mock_transaction.expect(:binded?, false) + mock_transaction.expect(:bind_invoice, AccountActivity.new, [1]) + mock_transaction.expect(:errors, []) BankTransaction.stub(:find_by, mock_transaction) do @every_pay.complete_transaction From 4985c8f42bea5bfa7283aed8c00f428489ec9db6 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Fri, 31 Jan 2020 17:28:22 +0500 Subject: [PATCH 3/9] Add test to check if transaction date included in request --- test/models/directo_test.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 test/models/directo_test.rb diff --git a/test/models/directo_test.rb b/test/models/directo_test.rb new file mode 100644 index 000000000..9dbbf64d4 --- /dev/null +++ b/test/models/directo_test.rb @@ -0,0 +1,20 @@ +require 'test_helper' + +class DirectoTest < ActiveSupport::TestCase + setup do + @invoice = invoices(:one) + end + + def test_xml_is_include_transaction_date + @invoice.update(total: @invoice.account_activity.bank_transaction.sum) + @invoice.account_activity.bank_transaction.update(paid_at: Time.zone.now) + + stub_request(:post, ENV['directo_invoice_url']).with do |request| + request.body.include? 'TransactionDate' + end + + assert_nothing_raised do + Directo.send_receipts + end + end +end From a190f4ecca8b905dd2f8b976d16ff088a82b45bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20V=C3=B5hmar?= Date: Fri, 31 Jan 2020 15:14:14 +0200 Subject: [PATCH 4/9] Update CHANGELOG.md [ci skip] --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 701678596..820cd7c72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +31.01.2020 +* Instant payments marks specific invoice as paid [#1500](https://github.com/internetee/registry/issues/1500) + 29.01.2020 * Fixed the invoice binding bug where process failed if registrar tried to load a sum that they have used before [#1496](https://github.com/internetee/registry/issues/1496) From 05d1cc3cb9ec1df26b7dcdf5e1885edbf1184072 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20V=C3=B5hmar?= Date: Fri, 31 Jan 2020 17:05:55 +0200 Subject: [PATCH 5/9] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 820cd7c72..14a0bba2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ 31.01.2020 * Instant payments marks specific invoice as paid [#1500](https://github.com/internetee/registry/issues/1500) +* Sending invoice payment date to accounting [#1416](https://github.com/internetee/registry/issues/1416) 29.01.2020 * Fixed the invoice binding bug where process failed if registrar tried to load a sum that they have used before [#1496](https://github.com/internetee/registry/issues/1496) From 89cdfe20fc0af4fd7226a2ee7d1e0b88db830ba3 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Mon, 3 Feb 2020 13:47:33 +0500 Subject: [PATCH 6/9] Pump SimpleIDN gem version, add check/test to block punicode IDN domains Closes #1142 --- Gemfile | 2 +- Gemfile.lock | 4 ++-- app/models/dns/domain_name.rb | 3 ++- test/fixtures/blocked_domains.yml | 2 ++ test/models/dns/domain_name_test.rb | 3 +++ 5 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index 26294b04e..b3882e792 100644 --- a/Gemfile +++ b/Gemfile @@ -37,7 +37,7 @@ gem 'grape' # registry specfic gem 'isikukood' # for EE-id validation -gem 'simpleidn', '0.0.7' # For punycode +gem 'simpleidn', '0.0.9' # For punycode gem 'money-rails' gem 'data_migrate' gem 'whenever', '0.9.4', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 4dd4bd57c..23aa90a51 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -382,7 +382,7 @@ GEM json (>= 1.8, < 3) simplecov-html (~> 0.10.0) simplecov-html (0.10.2) - simpleidn (0.0.7) + simpleidn (0.0.9) sinatra (2.0.7) mustermann (~> 1.0) rack (~> 2.0) @@ -491,7 +491,7 @@ DEPENDENCIES select2-rails (= 3.5.9.3) selectize-rails (= 0.12.1) simplecov - simpleidn (= 0.0.7) + simpleidn (= 0.0.9) uglifier validates_email_format_of (= 1.6.3) webdrivers diff --git a/app/models/dns/domain_name.rb b/app/models/dns/domain_name.rb index d2ca9fa50..e4dd24fa5 100644 --- a/app/models/dns/domain_name.rb +++ b/app/models/dns/domain_name.rb @@ -60,7 +60,8 @@ module DNS end def blocked? - BlockedDomain.where(name: name).any? + BlockedDomain.where(name: name).any? || + BlockedDomain.where(name: SimpleIDN.to_unicode(name)).any? end def reserved? diff --git a/test/fixtures/blocked_domains.yml b/test/fixtures/blocked_domains.yml index 4bf0d0299..52c9beec2 100644 --- a/test/fixtures/blocked_domains.yml +++ b/test/fixtures/blocked_domains.yml @@ -1,2 +1,4 @@ one: name: blocked.test +idn: + name: blockedäöüõ.test diff --git a/test/models/dns/domain_name_test.rb b/test/models/dns/domain_name_test.rb index 5d0dd5386..bd83076bc 100644 --- a/test/models/dns/domain_name_test.rb +++ b/test/models/dns/domain_name_test.rb @@ -131,7 +131,10 @@ class DNS::DomainNameTest < ActiveSupport::TestCase def test_blocked assert_equal 'blocked.test', blocked_domains(:one).name + assert_equal 'blockedäöüõ.test', blocked_domains(:idn).name assert DNS::DomainName.new('blocked.test').blocked? + assert DNS::DomainName.new('blockedäöüõ.test').blocked? + assert DNS::DomainName.new(SimpleIDN.to_ascii('blockedäöüõ.test')).blocked? assert_not DNS::DomainName.new('nonblocked .test').blocked? end From 5d56998e21e382fb8f7b76bf49d5ed5f63cdf78e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20V=C3=B5hmar?= Date: Tue, 4 Feb 2020 15:53:57 +0200 Subject: [PATCH 7/9] Update CHANGELOG.md [ci skip] --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14a0bba2b..8a14ba1c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +04.02.2020 +* SimpleIDN gem update to 0.0.9 [#1508](https://github.com/internetee/registry/pull/1508) + 31.01.2020 * Instant payments marks specific invoice as paid [#1500](https://github.com/internetee/registry/issues/1500) * Sending invoice payment date to accounting [#1416](https://github.com/internetee/registry/issues/1416) From 934033cfbcffadff2bcc0d54e166e311c7b9a008 Mon Sep 17 00:00:00 2001 From: Alex Sherman Date: Tue, 4 Feb 2020 18:34:50 +0500 Subject: [PATCH 8/9] Add test & validation to not to register blocked IDN domains via EPP Fixes https://github.com/internetee/registry/issues/1142#issuecomment-581889350 --- app/validators/domain_name_validator.rb | 4 ++- .../epp/domain/create/base_test.rb | 30 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/app/validators/domain_name_validator.rb b/app/validators/domain_name_validator.rb index 0d5638b37..2652c44d6 100644 --- a/app/validators/domain_name_validator.rb +++ b/app/validators/domain_name_validator.rb @@ -33,7 +33,9 @@ class DomainNameValidator < ActiveModel::EachValidator def validate_blocked(value) return true unless value - return false if BlockedDomain.where(name: value).count.positive? + return false if BlockedDomain.where(name: value).any? + return false if BlockedDomain.where(name: SimpleIDN.to_unicode(value)).any? + DNS::Zone.where(origin: value).count.zero? end end diff --git a/test/integration/epp/domain/create/base_test.rb b/test/integration/epp/domain/create/base_test.rb index ff8da3696..ffd56ffc5 100644 --- a/test/integration/epp/domain/create/base_test.rb +++ b/test/integration/epp/domain/create/base_test.rb @@ -144,6 +144,36 @@ class EppDomainCreateBaseTest < EppTestCase assert_epp_response :data_management_policy_violation end + def test_blocked_punicode_domain_cannot_be_registered + blocked_domain = 'blockedäöüõ.test' + assert BlockedDomain.find_by(name: blocked_domain) + + request_xml = <<-XML + + + + + + #{SimpleIDN.to_ascii('blockedäöüõ.test')} + #{contacts(:john).code} + + + + + #{'test' * 2000} + + + + + XML + + assert_no_difference 'Domain.count' do + post epp_create_path, params: { frame: request_xml }, + headers: { 'HTTP_COOKIE' => 'session=api_bestnames' } + end + assert_epp_response :data_management_policy_violation + end + def test_reserved_domain_cannot_be_registered_with_wrong_registration_code request_xml = <<-XML From 687e72d6d5cd09d11bf51b7557a4bec14aace158 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20V=C3=B5hmar?= Date: Tue, 4 Feb 2020 17:42:59 +0200 Subject: [PATCH 9/9] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a14ba1c1..c0670abc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ 04.02.2020 +* Fixed bug that allowed bypassing blocked domain validation using punycode [#1142](https://github.com/internetee/registry/issues/1142) * SimpleIDN gem update to 0.0.9 [#1508](https://github.com/internetee/registry/pull/1508) 31.01.2020