Survey/Question/Answer validation with fields_for?

[Scott Goci]

Imagine we had a basic survey application (Rails 4.2 with Devise) similar to the following:

class User < ActiveRecord::Base

  # Assume devise user here

  has_many :user_surveys

  has_many :surveys, through: :user_surveys

  has_many :responses

end

class UserSurvey < ActiveRecord::Base

  belongs_to :user

  belongs_to :survey

end

class Survey < ActiveRecord::Base

  has_many :questions

  has_many :user_surveys

  has_many :users, through: :user_surveys

  accepts_nested_attributes_for :questions

 # name: string

end

class Question < ActiveRecord::Base

  belongs_to :survey

  has_many :responses

  accepts_nested_attributes_for :responses

 # response_text: string

end

Class Response < ActiveRecord::Base

  belongs_to :question

  belongs_to :user

  # response_text: string

end

The idea being, an admin can create a survey with questions, and then assign that survey to users through the join model UserSurvey.

Now imagine a basic form that we give the user for their survey that looks like this:

<h1><%= @survey.name %> Answers</h1>

<%= form_for(@survey) do |f| %>

  <h3><%= current_user.name %></h3>

  <table>

    <thead>

      <tr>

        <td>Questions</td>

        <td>Response</td>

      </tr>

    </thead>

    <tbody>

      <% @questions.each do |question| -%>

      <tr>

        <td><%= question.question_text %></td>

        <td>

        <%= f.fields_for :questions, question do |q| -%>

          <%= q.fields_for :responses, question.responses.find_or_initialize_by(user: current_user.id) do |r| -%>

            <%= r.text_area :response_text %>

            <%= r.hidden_field :user_id, current_user.id %>

          <% end -%>

        <% end -%>

        </td>

      </tr>

      <% end -%>

    </tbody>

  </table>

  <div class="actions">

    <%= f.submit %>

  </div>

<% end -%>

My main question revolves around the following line (under fields_for responses):

<%= r.hidden_field :user_id, current_user.id %>

I see it suggested other places (eg: here on stackoverflow, or this tutorial here ) to put a hidden user_id field, but this feels incredibly wrong to me -- if a user is malicious, they could edit the form and modify the hidden user_id field to modify another participants answer.

Now, the one solution I did think was that on the controller side I could mess with the params, IE assume the submitted parameters look as follows

"survey"=>{"questions_attributes"=>{"0"=>{"responses_attributes"=>{"0"=>{"response_text"=>"one"}}, "id"=>"7"}, "1"=>{"responses_attributes"=>{"0"=>{"response_text"=>"two"}}, "id"=>"8"}}}

I could do something like:

def update

  @survey = current_user.surveys.find(params[:id])

  @questions = @survey.questions

  params[:survey][:question_attributes].each do |key, attribs|

    question = @questions.find(attribs[:id])

    response = question.responses.where(:user_id => current_user.id).first_or_initialize

    response.response_text = attribs["responses_attributes"]["0"]["response_text"]

    response.save!

  end

end

This way, I not only check to make sure the survey is attached to the user, but also make sure the questions they are submitting are attached to that specific survey, and the responses are not only tied to the correct question, but also back to the correct user. This feels messy though, and I feel I'm majorly overthinking this.

Can anyone give me a sanity check as to what  I'm doing wrong? 

[Colin Law]

On 30 July 2015 at 21:44, Scott Goci <sco...@gmail.com> wrote:
> ...

> My main question revolves around the following line (under fields_for
> responses):
>
> <%= r.hidden_field :user_id, current_user.id %>
>
>
> I see it suggested other places (eg: here on stackoverflow, or this tutorial
> here ) to put a hidden user_id field, but this feels incredibly wrong to me
> -- if a user is malicious, they could edit the form and modify the hidden
> user_id field to modify another participants answer.

In the controller check that the id from the form matches that for the
current logged in user. In fact not sure why you need it in the form
at all, as it should just be that for the current user. Possibly I am
missing something in your question.

Colin

[Scott Goci]

At the end of my email, I do just that -- you notice that in the params I suggest, there is no user_id present, and instead I break apart the params and inject the user_id more manually, but breaking out the params seems messy to validate against, so I'm wondering if there is a more "cleaner" method of doing it.

[Colin Law]

On 30 July 2015 at 23:06, Scott Goci <sco...@gmail.com> wrote:
> At the end of my email, I do just that -- you notice that in the params I
> suggest, there is no user_id present, and instead I break apart the params
> and inject the user_id more manually, but breaking out the params seems
> messy to validate against, so I'm wondering if there is a more "cleaner"
> method of doing it.

That is not quite what I was suggesting, your question indicated you
were happy with the hidden field concept, apart from the fact that
someone might inject a false id. My suggestion was to use the hidden
field but then to verify it against current_user in the controller.
So just one test to insert in the controller.

Colin

[Fernando Kakimoto]

Does it make sense to not have this hidden_field in the form but instead add this info inside the controller while saving the object? For instance: 

Response.new(response_params)

def response_params

  params.require(:response).permit(:response_text).merge(user_id: current_user.id)

end

Colin

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-ta...@googlegroups.com.
To post to this group, send email to rubyonra...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/CAL%3D0gLumg6ZRnRutgEa-f6KFbT3oZ4B%3Da9fLkgZEsp_x8EuotQ%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Fernando Kakimoto
http://www.twitter.com/nandokakimoto

[Scott Goci]

That definitely makes more sense in my mind fernando, but if its a deeply nested object, can you do that? As the params would probably look something like:

params.require(:survey).permit(:question_attributes => [:response_attributes => [:response_text].merge(user_id: current_user.id)])

[Fernando Kakimoto]

If the logic to create your object is more complex than a few lines of code, you could extract it to a separate class. I used to create adapter classes which build model objects from the request parameters. That worked well for me.

To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/f0f8f28d-dd95-4d14-9090-68ba3fd14423%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.