From d6ba6d1b02341ef3a158549fb443e58a8c2a4af0 Mon Sep 17 00:00:00 2001
From: James Fleming <jflemingprod@gmail.com>
Date: Sat, 6 Jul 2013 23:12:41 +0200
Subject: [PATCH] Use strong params in photos_controller

Add specs to check mass-assignment gotchas in PhotosController.
---
 app/controllers/photos_controller.rb       |  8 ++++++--
 spec/controllers/photos_controller_spec.rb | 23 +++++++++++++++++++++-
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/app/controllers/photos_controller.rb b/app/controllers/photos_controller.rb
index 2d7fb936f..c70a4a920 100644
--- a/app/controllers/photos_controller.rb
+++ b/app/controllers/photos_controller.rb
@@ -41,7 +41,7 @@ class PhotosController < ApplicationController
   def create
     rescuing_photo_errors do
       if remotipart_submitted?
-        @photo = current_user.build_post(:photo, params[:photo])
+        @photo = current_user.build_post(:photo, photo_params)
         if @photo.save
           respond_to do |format|
             format.json { render :json => {"success" => true, "data" => @photo.as_api_response(:backbone)} }
@@ -114,7 +114,7 @@ class PhotosController < ApplicationController
   def update
     photo = current_user.photos.where(:id => params[:id]).first
     if photo
-      if current_user.update_post( photo, params[:photo] )
+      if current_user.update_post( photo, photo_params )
         flash.now[:notice] = I18n.t 'photos.update.notice'
         respond_to do |format|
           format.js{ render :json => photo, :status => 200 }
@@ -133,6 +133,10 @@ class PhotosController < ApplicationController
 
   private
 
+  def photo_params
+    params.require(:photo).permit(:public, :text, :pending, :user_file, :image_url, :aspect_ids, :set_profile_photo)
+  end
+
   def file_handler(params)
     # For XHR file uploads, request.params[:qqfile] will be the path to the temporary file
     # For regular form uploads (such as those made by Opera), request.params[:qqfile] will be an UploadedFile which can be returned unaltered.
diff --git a/spec/controllers/photos_controller_spec.rb b/spec/controllers/photos_controller_spec.rb
index af7e076dd..99635571e 100644
--- a/spec/controllers/photos_controller_spec.rb
+++ b/spec/controllers/photos_controller_spec.rb
@@ -54,6 +54,20 @@ describe PhotosController do
       }.should change(Photo, :count).by(1)
     end
 
+    it "doesn't allow mass assignment of person" do
+      new_user = FactoryGirl.create(:user)
+      @params[:photo][:author] = new_user
+      post :create, @params
+      Photo.last.author.should == alice.person
+    end
+
+    it "doesn't allow mass assignment of person_id" do
+      new_user = FactoryGirl.create(:user)
+      @params[:photo][:author_id] = new_user.id
+      post :create, @params
+      Photo.last.author.should == alice.person
+    end
+
     it 'can set the photo as the profile photo' do
       old_url = alice.person.profile.image_url
       @params[:photo][:set_profile_photo] = true
@@ -137,7 +151,14 @@ describe PhotosController do
       @alices_photo.reload.text.should == "now with lasers!"
     end
 
-    it "doesn't overwrite random attributes" do
+    it "doesn't allow mass assignment of person" do
+      new_user = FactoryGirl.create(:user)
+      params = { :text => "now with lasers!", :author => new_user }
+      put :update, :id => @alices_photo.id, :photo => params
+      @alices_photo.reload.author.should == alice.person
+    end
+
+    it "doesn't allow mass assignment of person_id" do
       new_user = FactoryGirl.create(:user)
       params = { :text => "now with lasers!", :author_id => new_user.id }
       put :update, :id => @alices_photo.id, :photo => params