diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..3d117f1 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,4 @@ +## 0.2.0 + +- Added `report_only` option to `firewalled_belongs_to` to log violations and + not raise diff --git a/activerecord-firewall.gemspec b/activerecord-firewall.gemspec index 85ac579..7199919 100644 --- a/activerecord-firewall.gemspec +++ b/activerecord-firewall.gemspec @@ -16,7 +16,7 @@ Gem::Specification.new do |s| s.files = Dir["{app,config,db,lib}/**/*", "MIT-LICENSE", "Rakefile", "README.md"] - s.add_dependency "rails", "~> 5.1.5" + s.add_dependency "rails", "~> 5.1.0" s.add_development_dependency "sqlite3" end diff --git a/dev.yml b/dev.yml index fb4b5fa..3d866e3 100644 --- a/dev.yml +++ b/dev.yml @@ -20,4 +20,4 @@ commands: syntax: argument: file optional: args... - run: bin/testunit + run: bin/test diff --git a/lib/activerecord/firewall/firewalled_belongs_to.rb b/lib/activerecord/firewall/firewalled_belongs_to.rb index f45763e..303923a 100644 --- a/lib/activerecord/firewall/firewalled_belongs_to.rb +++ b/lib/activerecord/firewall/firewalled_belongs_to.rb @@ -3,13 +3,17 @@ module ActiveRecord module FirewalledBelongsTo - def firewalled_belongs_to(foreign_key_type, *args) + def firewalled_belongs_to(foreign_key_type, *args, report_only: false, **options) key_column_name = "#{foreign_key_type.to_s}_id" - belongs_to foreign_key_type + belongs_to foreign_key_type, *args, **options attribute key_column_name, - FirewalledIDType.new(self, foreign_key_type, ActiveRecord::Firewall.current_name.constantize) + FirewalledIDType.new( + self, + foreign_key_type, + ActiveRecord::Firewall.current_name.constantize, + report_only: report_only) after_find do |record| # This explicitly loads the foreign key and diff --git a/lib/activerecord/firewall/firewalled_id_type.rb b/lib/activerecord/firewall/firewalled_id_type.rb index 0b4c151..2629ffc 100644 --- a/lib/activerecord/firewall/firewalled_id_type.rb +++ b/lib/activerecord/firewall/firewalled_id_type.rb @@ -2,10 +2,11 @@ module ActiveRecord class FirewalledIDType < ActiveRecord::Type::BigInteger class FirewalledAccess < ActiveRecord::RecordNotFound; end - def initialize(model, protected_type, source) + def initialize(model, protected_type, source, report_only: false) super() @model = model @protected_type = protected_type.to_sym + @report_only = report_only @source = source end @@ -33,7 +34,11 @@ def check_attribute!(id) #{id} was accessed from #{humanized_protected_type} #{current_id} END - raise FirewalledAccess, message + if @report_only + Rails.logger.info "[activerecord-firewall] #{message}" + else + raise FirewalledAccess, message + end end end end diff --git a/lib/activerecord/firewall/version.rb b/lib/activerecord/firewall/version.rb index 018bfb0..638bbe8 100644 --- a/lib/activerecord/firewall/version.rb +++ b/lib/activerecord/firewall/version.rb @@ -1,5 +1,5 @@ module Activerecord module Firewall - VERSION = '0.1.0' + VERSION = '0.2.0' end end diff --git a/test/dummy/app/controllers/blog_post_controller.rb b/test/dummy/app/controllers/blog_post_controller.rb index 31003c1..2c8e89d 100644 --- a/test/dummy/app/controllers/blog_post_controller.rb +++ b/test/dummy/app/controllers/blog_post_controller.rb @@ -6,4 +6,12 @@ def show render html: "

Blog post #{@blog.title} (#{@blog.id}, #{@blog.user.id}) is allowed for user #{Current.user.name} (#{Current.user.id})

" end + + def image + Current.user = User.find_by_id(params[:user_id]) + + @image = Image.find_by_id(params[:image_id]) + + render html: "

Blog post #{@image.alt} (#{@image.id}, #{@image.user.id}) is allowed for user #{Current.user.name} (#{Current.user.id})

" + end end diff --git a/test/dummy/app/models/image.rb b/test/dummy/app/models/image.rb new file mode 100644 index 0000000..817d0b3 --- /dev/null +++ b/test/dummy/app/models/image.rb @@ -0,0 +1,3 @@ +class Image < ApplicationRecord + firewalled_belongs_to :user, report_only: true +end diff --git a/test/dummy/config/routes.rb b/test/dummy/config/routes.rb index ff5f08a..76187b3 100644 --- a/test/dummy/config/routes.rb +++ b/test/dummy/config/routes.rb @@ -1,5 +1,6 @@ Rails.application.routes.draw do # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html - get '/:user_id/:blog_post_id', to: "blog_post#show" + get '/blogs/:user_id/:blog_post_id', to: "blog_post#show" + get '/images/:user_id/:image_id', to: "blog_post#image" end diff --git a/test/dummy/db/migrate/20180312161755_create_images.rb b/test/dummy/db/migrate/20180312161755_create_images.rb new file mode 100644 index 0000000..917eef8 --- /dev/null +++ b/test/dummy/db/migrate/20180312161755_create_images.rb @@ -0,0 +1,11 @@ +class CreateImages < ActiveRecord::Migration[5.1] + def change + create_table :images do |t| + t.string :alt + t.string :url + t.belongs_to :user, foreign_key: true + + t.timestamps + end + end +end diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb index 74ab74a..f63e71f 100644 --- a/test/dummy/db/schema.rb +++ b/test/dummy/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180307230944) do +ActiveRecord::Schema.define(version: 20180312161755) do create_table "blog_posts", force: :cascade do |t| t.integer "user_id" @@ -21,6 +21,15 @@ t.index ["user_id"], name: "index_blog_posts_on_user_id" end + create_table "images", force: :cascade do |t| + t.string "alt" + t.string "url" + t.integer "user_id" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["user_id"], name: "index_images_on_user_id" + end + create_table "users", force: :cascade do |t| t.string "name" t.string "description" diff --git a/test/dummy/test/models/image_test.rb b/test/dummy/test/models/image_test.rb new file mode 100644 index 0000000..2316cb1 --- /dev/null +++ b/test/dummy/test/models/image_test.rb @@ -0,0 +1,35 @@ +require 'test_helper' + +class ImageTest < ActiveSupport::TestCase + setup do + @goodbob = users(:goodbob) + @evilbob = users(:evilbob) + + @goodbobs_image = images(:goodbobs_image) + @evilbobs_image = images(:evilbobs_image) + + @log_msg = "[activerecord-firewall] Image from User #{@goodbob.id} was accessed from User #{@evilbob.id}" + + Current.user = @evilbob + end + + teardown do + Current.reset + end + + test "image belonging to evil bob is accessible by evil bob with no log messages" do + assert_nothing_raised do + assert_not_logged(@log_msg) do + Image.where(user: @evilbob).first + end + end + end + + test "image belonging to good bob is accessible by evil bob with no log messages" do + assert_nothing_raised do + assert_logged(@log_msg) do + Image.where(user: @goodbob).first + end + end + end +end diff --git a/test/fixtures/images.yml b/test/fixtures/images.yml new file mode 100644 index 0000000..8eff89f --- /dev/null +++ b/test/fixtures/images.yml @@ -0,0 +1,11 @@ +# Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html + +goodbobs_image: + alt: This is alt text for good bob's image + url: https://example.com/goodbob.png + user: goodbob + +evilbobs_image: + alt: This is alt text for evil bob's image + url: https://example.com/evilbob.png + user: evilbob diff --git a/test/test_helper.rb b/test/test_helper.rb index 0459073..a81e265 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -14,7 +14,36 @@ class ActiveSupport::TestCase # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order. fixtures :all + def assert_logged(message) + old_logger = Rails.logger + log = StringIO.new + Rails.logger = Logger.new(log) + + begin + yield + + log.rewind + assert_match message, log.read + ensure + Rails.logger = old_logger + end + end + # Add more helper methods to be used by all tests here... + def assert_not_logged(message) + old_logger = Rails.logger + log = StringIO.new + Rails.logger = Logger.new(log) + + begin + yield + + log.rewind + assert_no_match message, log.read + ensure + Rails.logger = old_logger + end + end end # Load fixtures from the engine