Would extra protection on forms be a good idea?

Currently with the Authenticity Token, we have a really easy way to validate that a form was submitted from our site, vs from a third party, and can reject it easily. Using Strong Parameters, we can protect attributes from end-user assignment. I’m interested rather, in protecting against users manipulating the action url before submitting a form.

Example where this would be useful:

resources :products do
  resources :reviews

When I hit the #create in the ReviewsController, it would be kinda nice if I could safely assume that the review is being created for the exact product that the form action originally specified. That way, if I only render that form for people who have bought that product, I know they are allowed to review that product without having to check anything manually. I think to do this, you’d have to implement it almost exactly the same way Authenticity Tokens are.

Would this be useful in general? I think it’d be kinda nice to have form actions be restricted from tampering by default, and it could even cut down on database calls to check if people are authorized to do certain things, if you already checked auth before rendering the form in the first place.

From a neurosis perspective, go ahead. From a security perspective, it sounds over-engineered. Think about what security really does-- it gatekeeps people from accessing things they would have an incentive to profit from .

In your scenario, one user might have access to leave a review for a thing and another user might have access to leave a review for a different thing. But to do that, use cancancan or Pundit to define access control.

The idea that someone might ‘visit’ a page and be allowed to leave a review seems fleeting to me– think like hacker, not like an amateur who knows how to View In Source and/or use Postman. If restricting access to thing A doesn’t give you any material benefit from restricting access to thing B, then why worry about it?

Most hackers hack for financial gain. Pretty much all of the others you don’t need to worry about. (Then again, I’ve seen lots of times where financial gain was hidden within an exploit that the dev didn’t see.)

Now, if you want to makes sure that Customer has purchase Thing A before s/he can leave a review for Thing A— that makes total sense to me. But you can do that with access control rules in CanCanCan or Pundit.

I wouldn’t go reinventing access tokens for other purposes just becuase.

Just my 2¢

I think good UI should make it hard for users to be in the wrong place, even if it’s not a risk. If the server isn’t going to allow something, I’d rather stop users from winding up somewhere that looks like it will be allowed, rather than wait until something is submitted. Other than that, I’m not sure what you mean exactly. In my proposed scenario, the only way to send a valid post request would be from visiting the page with the form, and the only reason I should ever render the form is if they are authorized to see it. It’s quite normal to redirect someone if they somehow wind up making a request to see a form they should not be allowed to submit - even if you do have authorization in front of the create/edit/delete actions. So if I’m going to have that happen, I may as well take advantage of the fact that I checked authorization early.

That brings me back to the point I made here:

In order to do a very normal thing - check if someone is authorized to submit a form before rendering it for them in the first place - you end up having to check authorization twice. This wouldn’t replace CanCanCan or Pundit - but it could reduce the amount of work they have to do, and also the amount of code I have to write. It could make it so that basically all POST requests are essentially pre-validated because the cookie already tells me it’s allowed. So where before I had to think about authorization in the context of what users can see and what options they had presented to them as well as the context of receiving requests and determining if they are allowed to do what they are attempting - this way I would instead only have to care about what I send to them. All non-idempotent requests could be considered authorized by default because my token system would have rejected them otherwise (and without extra db queries). The only security concern I could think of would be having stale info about permissions, but token expiration/invalidation could address that I think.

One simple way is to scope your product finder to the current user, so in your reviews controller:

def create
  # page will 404 if the user tries to review a product they haven't purchased:
  product = current_user.products.find(params[:product_id])

  @review = product.reviews.build(reviews_params)
  if @review.save...

Another less straight forward option is to do some trickery with signed global ids because users can’t fake a signed id.

Use the sgid of the product in a hidden field in your form. It could be generated like product_sgid = product.to_sgid(for:"#{current_user.id}:#{product.id}")

Then in your controller:

def create
  product = GlobalID::Locator.locate_signed(review_params[:product_sgid], for:  "#{current_user.id}:#{params[:product_id]}")

Certainly everyone on your team would prefer you use the first version unless you have a really good reason to use signed a signed global id.

it’s an interesting idea, this pre-validation in the cookie. I kind of like it even if I think it sounds a little over-engineered. But you’d have to consider what if a state had changed since you pre-validated the cookie, how would you handle that (like somethign else changed that made it so the user wasn’t allowed to do that anymore)