Merge pull request #347 from scouttyg/security-overhaul

Complete security overhaul on the application due to possible attack vec...

.gitignore

@@ -25,6 +25,7 @@ DS_Store
 .rvmrc
 .ruby-version
 .ruby-gemset
+.secret
 
 /public/assets
 

Gemfile

@@ -1,7 +1,7 @@
 source 'https://rubygems.org'
 
 # Core gems
-gem 'rails', '3.2.16'
+gem 'rails', '3.2.17'
 
 # Database adapters
 gem 'pg'

Gemfile.lock

@@ -1,31 +1,31 @@
 GEM
   remote: https://rubygems.org/
   specs:
-    actionmailer (3.2.16)
-      actionpack (= 3.2.16)
+    actionmailer (3.2.17)
+      actionpack (= 3.2.17)
       mail (~> 2.5.4)
-    actionpack (3.2.16)
-      activemodel (= 3.2.16)
-      activesupport (= 3.2.16)
+    actionpack (3.2.17)
+      activemodel (= 3.2.17)
+      activesupport (= 3.2.17)
       builder (~> 3.0.0)
       erubis (~> 2.7.0)
       journey (~> 1.0.4)
       rack (~> 1.4.5)
       rack-cache (~> 1.2)
       rack-test (~> 0.6.1)
       sprockets (~> 2.2.1)
-    activemodel (3.2.16)
-      activesupport (= 3.2.16)
+    activemodel (3.2.17)
+      activesupport (= 3.2.17)
       builder (~> 3.0.0)
-    activerecord (3.2.16)
-      activemodel (= 3.2.16)
-      activesupport (= 3.2.16)
+    activerecord (3.2.17)
+      activemodel (= 3.2.17)
+      activesupport (= 3.2.17)
       arel (~> 3.0.2)
       tzinfo (~> 0.3.29)
-    activeresource (3.2.16)
-      activemodel (= 3.2.16)
-      activesupport (= 3.2.16)
-    activesupport (3.2.16)
+    activeresource (3.2.17)
+      activemodel (= 3.2.17)
+      activesupport (= 3.2.17)
+    activesupport (3.2.17)
       i18n (~> 0.6, >= 0.6.4)
       multi_json (~> 1.0)
     addressable (2.3.6)
@@ -211,22 +211,22 @@ GEM
       rack
     rack-test (0.6.2)
       rack (>= 1.0)
-    rails (3.2.16)
-      actionmailer (= 3.2.16)
-      actionpack (= 3.2.16)
-      activerecord (= 3.2.16)
-      activeresource (= 3.2.16)
-      activesupport (= 3.2.16)
+    rails (3.2.17)
+      actionmailer (= 3.2.17)
+      actionpack (= 3.2.17)
+      activerecord (= 3.2.17)
+      activeresource (= 3.2.17)
+      activesupport (= 3.2.17)
       bundler (~> 1.0)
-      railties (= 3.2.16)
-    railties (3.2.16)
-      actionpack (= 3.2.16)
-      activesupport (= 3.2.16)
+      railties (= 3.2.17)
+    railties (3.2.17)
+      actionpack (= 3.2.17)
+      activesupport (= 3.2.17)
       rack-ssl (~> 1.3.2)
       rake (>= 0.8.7)
       rdoc (~> 3.4)
       thor (>= 0.14.6, < 2.0)
-    rake (10.2.2)
+    rake (10.3.1)
     rb-fsevent (0.9.4)
     rb-inotify (0.9.3)
       ffi (>= 0.5.0)
@@ -333,7 +333,7 @@ DEPENDENCIES
   poltergeist
   pry-rails
   quiet_assets
-  rails (= 3.2.16)
+  rails (= 3.2.17)
   remotipart
   rspec-rails
   sass (= 3.2.13)

app/controllers/channels_controller.rb

@@ -2,7 +2,6 @@ class ChannelsController < ApplicationController
   before_filter :authenticate_user!
   before_filter :find_channel_by_name, :only => :show
   load_and_authorize_resource
-  before_filter :set_channel_owner, only: :create
 
   def index
     # NOTE Eager loading doesn't respect limit
@@ -27,6 +26,8 @@ def index
   end
 
   def create
+    @channel = Channel.new(params[:channel])
+    @channel.user_id = current_user.id
     respond_to do |format|
       if @channel.save
         format.json { render :json => @channel, :status => :created }
@@ -63,8 +64,4 @@ def destroy
   def find_channel_by_name
     @channel = Channel.where("LOWER(name) = ?", params['id'].downcase).first
   end
-
-  def set_channel_owner
-    @channel.user = current_user
-  end
 end

app/models/activity.rb

@@ -1,6 +1,7 @@
 class Activity < ActiveRecord::Base
   belongs_to :user
   belongs_to :channel
+  attr_accessible :content, :action
 
   paginates_per Kandan::Config.options[:per_page]
 

app/models/attachment.rb

@@ -2,6 +2,8 @@ class Attachment < ActiveRecord::Base
   belongs_to :channel
   belongs_to :user
 
+  attr_accessible :file
+
   if ENV['S3_BUCKET']
     has_attached_file(:file, {
       :storage         => :s3,
@@ -19,8 +21,6 @@ class Attachment < ActiveRecord::Base
     has_attached_file :file
     do_not_validate_attachment_file_type :file
   end
-
-  attr_accessible :file
 
   def url
     file.to_s

app/models/attachment_observer.rb

@@ -2,11 +2,11 @@ class AttachmentObserver < ActiveRecord::Observer
 
   def after_save(attachment)
     activity = Activity.new(
-      :channel_id => attachment.channel_id,
-      :user_id    => attachment.user_id,
       :action     => "upload",
       :content    => attachment.url
       )
+    activity.channel_id = attachment.channel_id
+    activity.user_id    = attachment.user_id
     activity.save
   end
 

app/models/channel.rb

@@ -2,6 +2,7 @@ class Channel < ActiveRecord::Base
   has_many :activities, :dependent => :destroy
   has_many :attachments, :dependent => :destroy
   belongs_to :user
+  attr_accessible :name
 
   validates :name, :presence => { :message => "Room name cannot be blank"}, :uniqueness => { :message => "Room name is already taken" }
   validates :user, :presence => { :message => "Room must belong to a user"}
@@ -22,11 +23,17 @@ def primary
     end
 
     def user_connect(user)
-      activity = Channel.primary.activities.create!(:user_id => user.id, :action => "connect")
+      activity = Channel.primary.activities.new
+      activity.user_id = user.id
+      activity.action = "connect"
+      activity.save!
     end
 
     def user_disconnect(user)
-      activity = Channel.primary.activities.create!(:user_id => user.id, :action => "disconnect")
+      activity = Channel.primary.activities.new
+      activity.user_id = user.id
+      activity.action = "disconnect"
+      activity.save!
     end
   end
 end

app/models/user.rb

@@ -23,7 +23,7 @@ class User < ActiveRecord::Base
   devise devise *Kandan.devise_modules
 
   # Setup accessible (or protected) attributes for your model
-  attr_accessible :id, :username, :email, :password, :password_confirmation, :remember_me, :first_name, :last_name, :locale, :gravatar_hash, :registration_status, :avatar_url
+  attr_accessible :username, :email, :password, :password_confirmation, :remember_me, :first_name, :last_name, :locale, :gravatar_hash, :avatar_url
 
   def full_name
     "#{self.first_name.to_s} #{self.last_name.to_s}".titlecase

config/application.rb

@@ -61,7 +61,8 @@ class Application < Rails::Application
     # 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
+    config.active_record.mass_assignment_sanitizer = :strict
 
     # Enable the asset pipeline
     config.assets.enabled = true

config/initializers/devise.rb

@@ -6,7 +6,7 @@
   # note that it will be overwritten if you use your own mailer class with default "from" parameter.
   config.mailer_sender = "no-reply@kandan.com"
 
-  config.secret_key = '2a9ce1b0e9c5637175f692e9bcf57100fda5eb903a9540679f44f19ec7835eae95851c0f5854f6b0eadaae6b2f55b1a8ba6e304a5c738b9f4b9e4f37462d1eb0'
+  #config.secret_key = '2a9ce1b0e9c5637175f692e9bcf57100fda5eb903a9540679f44f19ec7835eae95851c0f5854f6b0eadaae6b2f55b1a8ba6e304a5c738b9f4b9e4f37462d1eb0'
   # Configure the class responsible to send e-mails.
   # config.mailer = "Devise::Mailer"
 

config/initializers/secret_token.rb

@@ -1,7 +1,26 @@
+require 'securerandom'
 # Be sure to restart your server when you modify this file.
 
 # Your secret key for verifying the integrity of signed cookies.
 # If you change this key, all old signed cookies will become invalid!
 # Make sure the secret is at least 30 characters and all random,
 # no regular words or you'll be exposed to dictionary attacks.
-Kandan::Application.config.secret_token = 'bb04f17da6af7d0c441a12973ce4594dc562fdf035987fa39d1034dffe2708ed1791dee4e09bd3ebd06769699bd063821fa40881724e2d3ebbb57cfe56f32c12'
+
+def find_secure_token
+  token_file = Rails.root.join('.secret')
+  if ENV.key?('SECRET_KEY_BASE')
+    ENV['SECRET_KEY_BASE']
+  elsif File.exist? token_file
+    # Use the existing token.
+    File.read(token_file).chomp
+  else
+    # Generate a new token of 64 random hexadecimal characters and store it in token_file.
+    token = SecureRandom.hex(64)
+    File.write(token_file, token)
+    token
+  end
+end
+
+Kandan::Application.config.secret_token  = find_secure_token
+Kandan::Application.config.secret_key_base = find_secure_token
+

spec/factories/attachment_factory.rb

@@ -0,0 +1,11 @@
+FactoryGirl.define do
+  sequence :file_name do |n|
+    "file#{n}.jpg"
+  end
+
+  factory :attachment do
+    file { generate(:file_name) }
+    user
+    channel
+  end
+end

spec/models/attachment_observer_spec.rb

@@ -2,7 +2,11 @@
 
 describe AttachmentObserver do
   it "should create an activity on save" do
+
     Activity.any_instance.should_receive(:save)
-    Attachment.new.save
+    a = Attachment.new
+    c = FactoryGirl.create(:channel)
+    a.stub(:channel).and_return(c)
+    a.save
   end
 end