diff --git a/app/controllers/epp/domains_controller.rb b/app/controllers/epp/domains_controller.rb index 82eae2105..5fe1df879 100644 --- a/app/controllers/epp/domains_controller.rb +++ b/app/controllers/epp/domains_controller.rb @@ -127,45 +127,46 @@ class Epp::DomainsController < EppController def validate_create @prefix = 'create > create >' - requires('name', 'ns', 'registrant', 'ns > hostAttr') + requires 'name', 'ns', 'registrant', 'ns > hostAttr' @prefix = 'extension > create >' mutually_exclusive 'keyData', 'dsData' @prefix = nil - requires('extension > extdata > legalDocument') + requires 'extension > extdata > legalDocument' end def validate_renew @prefix = 'renew > renew >' - requires('name', 'curExpDate', 'period') + requires 'name', 'curExpDate', 'period' end def validate_update if element_count('update > chg > registrant') > 0 - requires('extension > extdata > legalDocument') + requires 'extension > extdata > legalDocument' end @prefix = 'update > update >' - requires('name') + requires 'name' end ## TRANSFER def validate_transfer - @prefix = 'transfer > transfer >' - requires('name') + requires 'transfer > transfer' - op = params[:parsed_frame].css('transfer').first[:op] - return if %w(approve query reject).include?(op) - epp_errors << { code: '2306', msg: I18n.t('errors.messages.attribute_op_is_invalid') } + @prefix = 'transfer > transfer >' + requires 'name' + + @prefix = nil + requires_attribute 'transfer', 'op', values: %(approve, query, reject) end ## DELETE def validate_delete - requires('extension > extdata > legalDocument') + requires 'extension > extdata > legalDocument' @prefix = 'delete > delete >' - requires('name') + requires 'name' end def domain_create_params diff --git a/app/controllers/epp_controller.rb b/app/controllers/epp_controller.rb index 8981dcd8f..cf18f211d 100644 --- a/app/controllers/epp_controller.rb +++ b/app/controllers/epp_controller.rb @@ -62,20 +62,56 @@ class EppController < ApplicationController handle_errors and return if epp_errors.any? end + # let's follow grape's validations: https://github.com/intridea/grape/#parameter-validation-and-coercion + + # Example usage: + # + # requires 'transfer' + # + # Adds error to epp_errors if element is missing or blank + # Returns last element of selectors if it exists + # + # TODO: Add possibility to pass validations / options in the method + def requires(*selectors) + el, missing = nil, nil selectors.each do |selector| full_selector = [@prefix, selector].join(' ') el = params[:parsed_frame].css(full_selector).first + + missing = el.nil? || (el.text.blank? && el.children.none?) + epp_errors << { code: '2003', msg: I18n.t('errors.messages.required_parameter_missing', key: el.try(:name) || selector) - } if el.nil? || el.text.blank? + } if missing end - epp_errors.empty? + missing ? false : el # return last selector if it was present + end + + # Example usage: + # + # requires_attribute 'transfer', 'op', values: %(approve, query, reject) + # + # Adds error to epp_errors if element or attribute is missing or attribute attribute is not one + # of the values + + def requires_attribute(element_selector, attribute_selector, options) + element = requires(element_selector) + return unless element + + attribute = element[attribute_selector] + + values = options.delete(:values) + return if attribute && values.include?(attribute) + + epp_errors << { + code: '2306', + msg: I18n.t('attribute_is_invalid', attribute: attribute_selector) + } end - # let's follow grape's validations: https://github.com/intridea/grape/#parameter-validation-and-coercion def exactly_one_of(*selectors) return if element_count(*selectors) == 1 diff --git a/config/locales/en.yml b/config/locales/en.yml index b14579991..a6e4c18ed 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -269,7 +269,6 @@ en: invalid_type: 'PostalInfo type is invalid' unimplemented_command: 'Unimplemented command' domain_exists_but_belongs_to_other_registrar: 'Domain exists but belongs to other registrar' - attribute_op_is_invalid: 'Attribute op is invalid' code: 'Code' @@ -501,3 +500,4 @@ en: unknown_expiry_relative_pattern: 'Expiry relative must be compatible to ISO 8601' unknown_expiry_absolute_pattern: 'Expiry absolute must be compatible to ISO 8601' mutally_exclusive_params: 'Mutually exclusive parameters: %{params}' + attribute_is_invalid: 'Attribute is invalid: %{attribute}' diff --git a/spec/epp/domain_spec.rb b/spec/epp/domain_spec.rb index 355c06f09..5fe84c283 100644 --- a/spec/epp/domain_spec.rb +++ b/spec/epp/domain_spec.rb @@ -944,7 +944,7 @@ describe 'EPP Domain', epp: true do it 'returns an error for incorrect op attribute' do response = epp_plain_request(domain_transfer_xml({}, 'bla'), :xml) response[:result_code].should == '2306' - response[:msg].should == 'Attribute op is invalid' + response[:msg].should == 'Attribute is invalid: op' end it 'creates new pw after successful transfer' do