Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dotar de más seguridad la redirección al formulario del TPV #1

Open
guzmanthegood opened this issue Feb 1, 2016 · 1 comment
Open

Comments

@guzmanthegood
Copy link
Contributor

La instrucción de redirección parece poco segura. Quizás se pudiera pasar los parámetros de forma codificada con algún helper que codifique y decodifique los parámetros.

redirect_to redsys_form_path(amount: '20.35', order: '0001', language: '001')
@guzmanthegood guzmanthegood changed the title Dotar de más seguridad a la redirección al formulario del TPV Dotar de más seguridad la redirección al formulario del TPV Feb 1, 2016
Senen pushed a commit that referenced this issue Sep 6, 2017
generator updated for rails 5
@alexmorral
Copy link

Creo que es una manera muy insegura de pasar los parámetros. Si durante la redirección se cambian los parámetros se realiza la petición a redsys con parámetros incorrectos.

#  redsys-rails/app/controllers/redsys/tpv_controller.rb 
def form
      amount = BigDecimal.new(params[:amount] || '0')
      order = params[:order] || '0'
      language = params[:language]
      url_ok = params[:url_ok]
      url_ko = params[:url_ko]
      merchant_url = params[:merchant_url] || redsys_notification_url if defined?(redsys_notification_url)
      merchant_name = params[:merchant_name]
      product_description = params[:product_description]
      @tpv = Redsys::Tpv.new(amount, order, language, merchant_url, url_ok, url_ko, merchant_name, product_description)
end

Obtiene los parámetros directamente de la URL y los codifica si no lo he entendido mal.

Se me han ocurrido dos maneras de mejorar un poco la seguridad:

Método 1 (bastante fácil y obvio)

Antes de hacer el redirect_to, crear el objeto Transaction con los datos de la transacción.

transaction = Transaction.create!(amount: 10, order: 'XXX', status: :pending, etc)
redirect_to redsys_form_path(order: transaction.order, etc)

y luego en el método form, usar los datos del objeto transaction

#  redsys-rails/app/controllers/redsys/tpv_controller.rb 
def form
      transaction = Transaction.find_by(order: params[:order])
      @error = 'No existe la transacción o no está disponible' if transaction.nil? || !transaction.pending?
      # TODO: More checks (eg: transaction has an user and the user is logged in, etc)
      @tpv = Redsys::Tpv.new(transaction.amount, transaction.order, transaction.language, transaction.merchant_url, transaction.url_ok, transaction.url_ko, transaction.merchant_name, transaction.product_description)
end

Y en el fichero form.html.erb, controlar que si existe @error por ejemplo, no renderize el formulario ni lo envíe.

<!--  redsys-rails/app/views/redsys/tpv/form.html.erb  -->
<% if @error %>
<!-- Show error -->
<% else %>
<%= form_tag(Redsys::Tpv.tpv_url, id: 'redsys_tpv_form') do %>
    <%= hidden_field_tag 'Ds_SignatureVersion', Redsys::Tpv.signature_version %>
    <%= hidden_field_tag 'Ds_MerchantParameters', @tpv.merchant_params %>
    <%= hidden_field_tag 'Ds_Signature', @tpv.merchant_signature %>
<% end %>
<script type="text/javascript">
    document.getElementById("redsys_tpv_form").submit();
</script>
<h2> Redirecting..</h2>
<% end %>

Finalmente en el fichero notifications_controller habría que poner algo para saber que la transaction ha finalizado.

Método 2

Desde la view desde donde se realiza el pago (ej: botón "Pagar") se llama mediante AJAX a un método parecido al form, y que luego haga un render partial: del form. El AJAX hace un append del form que le llega al body y luego hace submit. De esta manera no hay redirección y no hay posibilidad de cambiar los parámetros.

#  redsys-rails/app/controllers/redsys/tpv_controller.rb 
def ajax_form
      # Get the info related to the payment to use on the transaction object (using params[:something])
      transaction = Transaction.create!(amount: 10, order: 'XXX', status: :pending, etc)
      language = '001'
      url_ok = 'XXX'
      url_ko = 'YYY'
      merchant_url = 'ZZZ' || redsys_notification_url if defined?(redsys_notification_url)
      merchant_name = 'PPP'
      product_description = 'HHH'
      @tpv = Redsys::Tpv.new(transaction.amount, transaction.order, language, merchant_url, url_ok, url_ko, merchant_name, product_description)
      render partial: 'form'
end
showLoading();
$.ajax({
      url: payment_form_url,
      method: "POST",
      data: payment_params, // Whatever you need to know what you want to pay
      error: function(jqXHR, textStatus, errorThrown) {
            console.log(errorThrown);
            hideLoading();
      },
      success: function(data, textStatus, jqXHR) {
            hideLoading();
            // console.log(data);
            form_container.html(data);
            $('#payment_form_id').submit();
      }
});

No he probado aún este método para mejorar la seguridad, pero creo que no me dejo nada.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants