small medium large xlarge

Kalle_pragsmall
25 Jun 2017, 23:11
Bijan Hoomand (3 posts)

Hi,

In chapter 10, page 138, the author is talking about the threat of the Internet and the fact that ‘we also want to prevent access to other people’s carts’. He proposes removing :cart_id from line_items_controller.rb; however this does not prevent me to see other customers’ carts. I can still easily browse ‘http://localhost:3000/carts/2’ for example, where I am the owner of cart with ID 1.

In order to fix the problem, I changed carts_controller.rb to below:

class CartsController < ApplicationController include CurrentCart before_action :set_cart, only: [:show, :edit, :update, :destroy] rescue_from ActiveRecord::RecordNotFound, with: :invalid_cart

And commented the local set_cart method in carts_controller.rb:

private # Use callbacks to share common setup or constraints between actions. # def set_cart # @cart = Cart.find(params[:id]) # end

This way, no matter what cart_id you enter as GET parameters, you’d always get your own cart_id which is written in your session variable and you cannot ask to see another cart_id. (‘include CurrentCart’ is including the CurrentCart concern that the book talks about in earlier chapters).

Was wondering if anyone else agrees with me that the solution that the author proposes to prevent browsing others’ carts is not really working and if you agree with my workaround.

Generic-user-small
27 Jul 2017, 01:08
Michael LeSauvage (1 post)

Just came here to look for info on this exact topic!

I was headed down the same approach you took (though I had forgotten that we hadn’t included CurrentCart yet). I agree, not allowing a specification of which cart to delete removes any issues, including cart snooping.

I suspect there could be a reason for leaving the functionality like the author had it - if you want to be able to inspect carts on the back end, restricting access this way means that your own session can only see your own cart. But one way or another you’re going to have to deal with permissions at some point.

Once that update is made, you can actually change the button_to as follows:

<%= button_to 'Empty cart', nil, method: :delete,
                data: { confirm: 'Are you sure?' } %>

…as it doesn’t matter what cart URL you send along with the post/delete.

This approach does break the update and destroy controller tests and I’m having to dig into them to figure out why and how to fix them.

Edit: The destroy test may have been broken anyway. I noticed that the test on page 140 didn’t match my test, and I couldn’t find anyplace else that had updated it. When I matched the test in the book, it worked. Looking at it, it overrides the cart lookup in set_cart with a new value, retrieved from the cart_id.

It looks like what might not be working here is the @cart being set by set_cart. I’m not certain why that is. If it’s that sessions don’t work, why are we able to access the sessions variable?

Or maybe that’s it. Maybe session is now just a local variable, not accessible to the controllers.

However, with our change to the controller to use sessions, it also breaks the “should update cart” test, requiring a similar change to look up the correct cart for the redirect:

  test "should update cart" do
    patch cart_url(@cart), params: { cart: {  } }
    @cart = Cart.find(session[:cart_id])
    assert_redirected_to cart_url(@cart)
  end
You must be logged in to comment