Turn on mass assignment protection

Require any attribute that is going to be mass assigned to be
whitelisted, and whitelist those attributes which need it

app/controllers/trace_controller.rb

@@ -153,7 +153,7 @@ class TraceController < ApplicationController
         @trace.errors.add(:gpx_file, "can't be blank")
       end
     else
-      @trace = Trace.new(:visibility => default_visibility)
+      @trace = Trace.new({:visibility => default_visibility}, :without_protection => true)
     end
 
     @title = t 'trace.create.upload_trace'
@@ -386,7 +386,7 @@ private
       :inserted => true,
       :user => @user,
       :timestamp => Time.now.getutc
-    })
+    }, :without_protection => true)
 
     Trace.transaction do
       begin

app/controllers/user_roles_controller.rb

@@ -11,7 +11,9 @@ class UserRolesController < ApplicationController
   around_filter :setup_nonce
 
   def grant
-    @this_user.roles.create(:role => @role, :granter_id => @user.id)
+    @this_user.roles.create({
+      :role => @role, :granter_id => @user.id
+    }, :without_protection => true)
     redirect_to :controller => 'user', :action => 'view', :display_name => @this_user.display_name
   end
 

app/models/client_application.rb

@@ -13,6 +13,11 @@ class ClientApplication < ActiveRecord::Base
   validates_format_of :support_url, :with => /\Ahttp(s?):\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/i, :allow_blank=>true
   validates_format_of :callback_url, :with => /\A[a-z][a-z0-9.+-]*:\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/i, :allow_blank=>true
 
+  attr_accessible :name, :url, :support_url, :callback_url,
+                  :allow_read_prefs, :allow_write_prefs,
+                  :allow_write_diary, :allow_write_api,
+                  :allow_read_gpx, :allow_write_gpx
+
   before_validation :generate_keys, :on => :create
 
   attr_accessor :token_callback_url
@@ -54,7 +59,7 @@ class ClientApplication < ActiveRecord::Base
     permissions.each do |p|
       params[p] = true
     end
-    RequestToken.create(params)
+    RequestToken.create(params, :without_protection => true)
   end
 
   def access_token_for_user(user)
@@ -65,7 +70,7 @@ class ClientApplication < ActiveRecord::Base
         params[p] = true
       end
 
-      token = access_tokens.create(params)
+      token = access_tokens.create(params, :without_protection => true)
     end
     
     token

app/models/diary_comment.rb

@@ -5,6 +5,8 @@ class DiaryComment < ActiveRecord::Base
   validates_presence_of :body
   validates_associated :diary_entry
 
+  attr_accessible :body
+
   def digest
     md5 = Digest::MD5.new
     md5 << diary_entry_id.to_s

app/models/diary_entry.rb

@@ -23,4 +23,6 @@ class DiaryEntry < ActiveRecord::Base
   validates_numericality_of :longitude, :allow_nil => true,
                             :greater_than_or_equal_to => -180, :less_than_or_equal_to => 180
   validates_associated :language
+
+  attr_accessible :title, :body, :language_code, :latitude, :longitude
 end

app/models/message.rb

@@ -9,6 +9,8 @@ class Message < ActiveRecord::Base
   validates_inclusion_of :message_read, :in => [ true, false ]
   validates_as_utf8 :title
 
+  attr_accessible :title, :body
+
   def digest
     md5 = Digest::MD5.new
     md5 << from_user_id.to_s

app/models/oauth_nonce.rb

@@ -3,7 +3,9 @@
 class OauthNonce < ActiveRecord::Base
   validates_presence_of :nonce, :timestamp
   validates_uniqueness_of :nonce, :scope => :timestamp
-  
+
+  attr_accessible :nonce, :timestamp
+
   # Remembers a nonce and it's associated timestamp. It returns false if it has already been used
   def self.remember(nonce, timestamp)
     oauth_nonce = OauthNonce.create(:nonce => nonce, :timestamp => timestamp)

app/models/request_token.rb

@@ -21,7 +21,7 @@ class RequestToken < OauthToken
         params[p] = read_attribute(p)
       }
 
-      access_token = AccessToken.create(params)
+      access_token = AccessToken.create(params, :without_protection => true)
       invalidate!
       access_token
     end

app/models/tracetag.rb

@@ -4,5 +4,7 @@ class Tracetag < ActiveRecord::Base
   validates_format_of :tag, :with => /^[^\/;.,?]*$/
   validates_length_of :tag, :within => 1..255
 
+  attr_accessible :tag
+
   belongs_to :trace, :foreign_key => 'gpx_id'
 end

app/models/user.rb

@@ -41,6 +41,9 @@ class User < ActiveRecord::Base
   validates_numericality_of :home_zoom, :only_integer => true, :allow_nil => true
   validates_inclusion_of :preferred_editor, :in => Editors::ALL_EDITORS, :allow_nil => true
 
+  attr_accessible :display_name, :email, :email_confirmation, :openid_url,
+                  :pass_crypt, :pass_crypt_confirmation, :consider_pd
+
   after_initialize :set_creation_time
   before_save :encrypt_password
 

app/models/user_block.rb

@@ -18,9 +18,11 @@ class UserBlock < ActiveRecord::Base
   # revokes the block, allowing the user to use the API again. the argument
   # is the user object who is revoking the ban.
   def revoke!(revoker)
-    update_attributes({ :ends_at => Time.now.getutc(),
+    update_attributes({
-                        :revoker_id => revoker.id,
+      :ends_at => Time.now.getutc(),
-                        :needs_view => false })
+      :revoker_id => revoker.id,
+      :needs_view => false
+    }, :without_protection => true)
   end
 
   private

app/models/user_token.rb

@@ -1,6 +1,8 @@
 class UserToken < ActiveRecord::Base
   belongs_to :user
 
+  attr_accessible :referer
+
   after_initialize :set_defaults
 
 private

config/application.rb

@@ -62,7 +62,7 @@ module OpenStreetMap
     # This will create an empty whitelist of attributes available for mass-assignment for all models
     # in your app. As such, your models will need to explicitly whitelist or blacklist accessible
     # parameters by using an attr_accessible or attr_protected declaration.
-    # config.active_record.whitelist_attributes = true
+    config.active_record.whitelist_attributes = true
 
     # Enable the asset pipeline
     config.assets.enabled = true

script/deliver-message

@@ -26,10 +26,13 @@ else
   body = mail
 end
 
-message = Message.new(:sender => from, :recipient => to,
+message = Message.new({
-                      :sent_on => mail.date.new_offset(0),
+  :sender => from,
-                      :title => mail.subject.sub(/\[OpenStreetMap\] */, ""),
+  :recipient => to,
-                      :body => body.decoded)
+  :sent_on => mail.date.new_offset(0),
+  :title => mail.subject.sub(/\[OpenStreetMap\] */, ""),
+  :body => body.decoded
+}, :without_protection => true)
 message.save!
 
 Notifier.message_notification(message).deliver

test/integration/user_blocks_test.rb

@@ -17,10 +17,12 @@ class UserBlocksTest < ActionController::IntegrationTest
     assert_response :success
 
     # now block the user
-    UserBlock.create(:user_id => blocked_user.id,
+    UserBlock.create({
-                     :creator_id => users(:moderator_user).id,
+      :user_id => blocked_user.id,
-                     :reason => "testing",
+      :creator_id => users(:moderator_user).id,
-                     :ends_at => Time.now.getutc + 5.minutes)
+      :reason => "testing",
+      :ends_at => Time.now.getutc + 5.minutes
+    }, :without_protection => true)
     get "/api/#{API_VERSION}/user/details", nil, auth_header(blocked_user.display_name, "test")
     assert_response :forbidden
   end
@@ -29,10 +31,12 @@ class UserBlocksTest < ActionController::IntegrationTest
     blocked_user = users(:public_user)
     moderator = users(:moderator_user)
 
-    block = UserBlock.create(:user_id => blocked_user.id,
+    block = UserBlock.create({
-                             :creator_id => moderator.id,
+      :user_id => blocked_user.id,
-                             :reason => "testing",
+      :creator_id => moderator.id,
-                             :ends_at => Time.now.getutc + 5.minutes)
+      :reason => "testing",
+      :ends_at => Time.now.getutc + 5.minutes
+    }, :without_protection => true)
     get "/api/#{API_VERSION}/user/details", nil, auth_header(blocked_user.display_name, "test")
     assert_response :forbidden
 

test/unit/diary_entry_test.rb

@@ -25,8 +25,8 @@ class DiaryEntryTest < ActiveSupport::TestCase
   end
   
   def diary_entry_valid(attrs, result = true)
-    entry = DiaryEntry.new(diary_entries(:normal_user_entry_1).attributes)
+    entry = DiaryEntry.new(diary_entries(:normal_user_entry_1).attributes, :without_protection => true)
-    entry.attributes = attrs
+    entry.assign_attributes(attrs, :without_protection => true)
     assert_equal result, entry.valid?, "Expected #{attrs.inspect} to be #{result}"
   end  
 end

test/unit/node_test.rb

@@ -77,11 +77,13 @@ class NodeTest < ActiveSupport::TestCase
   
   # Check that you can create a node and store it
   def test_create
-    node_template = Node.new(:latitude => 12.3456,
+    node_template = Node.new({
-                             :longitude => 65.4321,
+      :latitude => 12.3456,
-                             :changeset_id => changesets(:normal_user_first_change).id,
+      :longitude => 65.4321,
-                             :visible => 1, 
+      :changeset_id => changesets(:normal_user_first_change).id,
-                             :version => 1)
+      :visible => 1, 
+      :version => 1
+    }, :without_protection => true)
     assert node_template.create_with_history(users(:normal_user))
 
     node = Node.find(node_template.id)

test/unit/oauth_token_test.rb

@@ -15,7 +15,9 @@ class OauthTokenTest < ActiveSupport::TestCase
   ##
   # check that an authorized token is authorised and can be invalidated
   def test_token_authorisation
-    tok = RequestToken.create :client_application => client_applications(:oauth_web_app)
+    tok = RequestToken.create({
+      :client_application => client_applications(:oauth_web_app)
+    }, :without_protection => true)
     assert_equal false, tok.authorized?, "Token should be created unauthorised."
     tok.authorize!(users(:public_user))
     assert_equal true, tok.authorized?, "Token should now be authorised."

test/unit/user_test.rb

@@ -18,23 +18,27 @@ class UserTest < ActiveSupport::TestCase
   end
   
   def test_unique_email
-    new_user = User.new(:email => users(:normal_user).email,
+    new_user = User.new({
+      :email => users(:normal_user).email,
       :status => "active", 
       :pass_crypt => Digest::MD5.hexdigest('test'),
       :display_name => "new user",
       :data_public => 1,
-      :description => "desc")
+      :description => "desc"
+    }, :without_protection => true)
     assert !new_user.save
     assert new_user.errors[:email].include?("has already been taken")
   end
   
   def test_unique_display_name
-    new_user = User.new(:email => "tester@openstreetmap.org",
+    new_user = User.new({
+      :email => "tester@openstreetmap.org",
       :status => "pending",
       :pass_crypt => Digest::MD5.hexdigest('test'),
       :display_name => users(:normal_user).display_name, 
       :data_public => 1,
-      :description => "desc")
+      :description => "desc"
+    }, :without_protection => true)
     assert !new_user.save
     assert new_user.errors[:display_name].include?("has already been taken")
   end