Merge pull request #1271 from neo4jrb/fix-active-controller-params-7.1.x

Backporting #1245 (Unpermitted parameters) to 7.2.x

Gemfile

@@ -13,9 +13,9 @@ gemspec
 gem 'listen', '< 3.1'
 
 if RUBY_VERSION.to_f < 2.2
-  gem 'activemodel', '~> 4'
-  gem 'activesupport', '~> 4'
-  gem 'railties', '~> 4'
+  gem 'activemodel', '~> 4.2'
+  gem 'activesupport', '~> 4.2'
+  gem 'railties', '~> 4.2'
 end
 
 group 'test' do
@@ -25,7 +25,6 @@ group 'test' do
     gem 'overcommit', '< 0.35.0'
   else
     gem 'overcommit'
-    gem 'activesupport', '>= 4.2'
   end
   gem 'codecov', require: false
   gem 'simplecov', require: false

lib/neo4j.rb

@@ -40,6 +40,7 @@
 require 'neo4j/shared/initialize'
 require 'neo4j/shared/query_factory'
 require 'neo4j/shared/cypher'
+require 'neo4j/shared/permitted_attributes'
 require 'neo4j/shared'
 
 require 'neo4j/active_rel/callbacks'

lib/neo4j/active_node.rb

@@ -44,10 +44,11 @@ module ActiveNode
     include Neo4j::ActiveNode::Scope
     include Neo4j::ActiveNode::Dependent
     include Neo4j::ActiveNode::Enum
+    include Neo4j::Shared::PermittedAttributes
 
     def initialize(args = nil)
-      symbol_args = args.is_a?(Hash) ? args.symbolize_keys : args
-      super(symbol_args)
+      args = sanitize_input_parameters(args)
+      super(args)
     end
 
     def neo4j_obj

lib/neo4j/active_rel.rb

@@ -18,13 +18,14 @@ module ActiveRel
     include Neo4j::ActiveRel::Query
     include Neo4j::ActiveRel::Types
     include Neo4j::Shared::Enum
+    include Neo4j::Shared::PermittedAttributes
 
     class FrozenRelError < Neo4j::Error; end
 
     def initialize(from_node = nil, to_node = nil, args = nil)
       load_nodes(node_or_nil(from_node), node_or_nil(to_node))
       resolved_args = hash_or_nil(from_node, args)
-      symbol_args = resolved_args.is_a?(Hash) ? resolved_args.symbolize_keys : resolved_args
+      symbol_args = sanitize_input_parameters(resolved_args)
       super(symbol_args)
     end
 
@@ -60,26 +61,7 @@ def node_or_nil(node)
     end
 
     def hash_or_nil(node_or_hash, hash_or_nil)
-      node_or_hash.is_a?(Hash) ? node_or_hash : hash_or_nil
-    end
-
-    module ClassMethods
-      [:create, :create!].each do |meth|
-        define_method(meth) do |from_node_or_args = nil, to_node = nil, args = nil|
-          return super(from_node_or_args) if from_node_or_args.is_a?(Hash)
-          args_hash = args || {}
-          args_with_node!(:from_node, from_node_or_args, args_hash)
-          args_with_node!(:to_node, to_node, args_hash)
-          super(args_hash)
-        end
-      end
-
-      private
-
-      def args_with_node!(key, node, args)
-        args[key] = node if node.is_a?(Neo4j::ActiveNode)
-        args
-      end
+      hash_or_parameter?(node_or_hash) ? node_or_hash : hash_or_nil
     end
   end
 end

lib/neo4j/active_rel/persistence.rb

@@ -55,26 +55,13 @@ def create_model
     module ClassMethods
       # Creates a new relationship between objects
       # @param [Hash] props the properties the new relationship should have
-      def create(props = {})
-        relationship_props = extract_association_attributes!(props) || {}
-        new(props).tap do |obj|
-          relationship_props.each do |prop, value|
-            obj.send("#{prop}=", value)
-          end
-          obj.save
-        end
+      def create(*args)
+        new(*args).tap(&:save)
       end
 
       # Same as #create, but raises an error if there is a problem during save.
       def create!(*args)
-        props = args[0] || {}
-        relationship_props = extract_association_attributes!(props) || {}
-        new(props).tap do |obj|
-          relationship_props.each do |prop, value|
-            obj.send("#{prop}=", value)
-          end
-          obj.save!
-        end
+        new(*args).tap(&:save!)
       end
 
       def create_method

lib/neo4j/shared/permitted_attributes.rb

@@ -0,0 +1,28 @@
+module Neo4j::Shared
+  module PermittedAttributes
+    extend ActiveSupport::Concern
+    include ActiveModel::ForbiddenAttributesProtection
+
+    def process_attributes(attributes)
+      attributes = sanitize_input_parameters(attributes)
+      super(attributes)
+    end
+
+    def attributes=(attributes)
+      attributes = sanitize_input_parameters(attributes)
+      super(attributes)
+    end
+
+    protected
+
+    # Check if an argument is a string or an ActionController::Parameters
+    def hash_or_parameter?(args)
+      args.is_a?(Hash) || args.respond_to?(:to_unsafe_h)
+    end
+
+    def sanitize_input_parameters(attributes)
+      attributes = sanitize_for_mass_assignment(attributes)
+      attributes.respond_to?(:symbolize_keys) ? attributes.symbolize_keys : attributes
+    end
+  end
+end

spec/e2e/active_model_spec.rb

@@ -864,4 +864,13 @@ def self.named_jim
       it { is_expected.to eq :exact }
     end
   end
+
+  context 'with `ActionController::Parameters`' do
+    let(:params) { action_controller_params('prop_with_default' => 'something else') }
+    let(:create_params) { params }
+    let(:klass) { IceLolly }
+    let(:subject) { klass.new }
+
+    it_should_behave_like 'handles permitted parameters'
+  end
 end

spec/e2e/active_rel_spec.rb

@@ -593,4 +593,13 @@ class ActiveRelSpecTypesInheritedRelClass < ActiveRelSpecTypesAutomaticRelType
       end
     end
   end
+
+  context 'with `ActionController::Parameters`' do
+    let(:params) { action_controller_params('score' => 7) }
+    let(:create_params) { params.merge(from_node: from_node, to_node: to_node) }
+    let(:klass) { MyRelClass }
+    subject { klass.new(from_node: from_node, to_node: to_node) }
+
+    it_should_behave_like 'handles permitted parameters'
+  end
 end

spec/e2e/enum_spec.rb

@@ -179,4 +179,22 @@
       end.to raise_error(Neo4j::Shared::Enum::ConflictingEnumMethodError)
     end
   end
+
+  context 'when using `ActionController::Parameters`' do
+    let(:params) { action_controller_params('type' => 'image').permit! }
+    it 'assigns enums correctly when instancing a new class' do
+      using_action_controller do
+        file = StoredFile.new(params)
+        expect(file.type).to eq('image')
+      end
+    end
+
+    it 'assigns enums correctly when assigning to `attributes`' do
+      using_action_controller do
+        file = StoredFile.new
+        file.attributes = params
+        expect(file.type).to eq('image')
+      end
+    end
+  end
 end

spec/shared_examples/forbidden_attributes_shared_examples.rb

@@ -0,0 +1,63 @@
+shared_examples 'handles permitted parameters' do
+  describe '#new' do
+    it 'assigns permitted params' do
+      using_action_controller do
+        params.permit!
+        expect(klass.new(create_params).attributes).to include(params.to_h)
+      end
+    end
+
+    it 'fails on unpermitted parameters' do
+      using_action_controller do
+        expect { klass.new(create_params) }.to raise_error ActiveModel::ForbiddenAttributesError
+      end
+    end
+  end
+
+  describe '#create' do
+    it 'assigns permitted params' do
+      using_action_controller do
+        params.permit!
+        expect(klass.create(create_params).attributes).to include(params.to_h)
+      end
+    end
+
+    it 'fails on unpermitted parameters' do
+      using_action_controller do
+        expect { klass.create(create_params) }.to raise_error ActiveModel::ForbiddenAttributesError
+      end
+    end
+  end
+
+  describe '#attributes=' do
+    it 'assigns permitted params' do
+      using_action_controller do
+        params.permit!
+        subject.attributes = params
+        expect(subject.attributes).to include(params.to_h)
+      end
+    end
+
+    it 'fails on unpermitted parameters' do
+      using_action_controller do
+        expect { subject.attributes = params }.to raise_error ActiveModel::ForbiddenAttributesError
+      end
+    end
+  end
+
+  describe '#update' do
+    it 'assigns permitted params' do
+      using_action_controller do
+        params.permit!
+        subject.update(params)
+        expect(subject.attributes).to include(params.to_h)
+      end
+    end
+
+    it 'fails on unpermitted parameters' do
+      using_action_controller do
+        expect { klass.new.update(params) }.to raise_error ActiveModel::ForbiddenAttributesError
+      end
+    end
+  end
+end

spec/spec_helper.rb

@@ -115,6 +115,47 @@ def log_queries!
     end
   end
 
+  def action_controller_params(args)
+    ActionController::Parameters.new(args)
+  end
+
+  def handle_child_output(read, write)
+    read.close
+    begin
+      rest = yield
+      write.puts [Marshal.dump(rest)].pack('m')
+    rescue StandardError => e
+      write.puts [Marshal.dump(e)].pack('m')
+    end
+    exit!
+  end
+
+  def do_in_child(&block)
+    read, write = IO.pipe
+    pid = fork do
+      handle_child_output(read, write, &block)
+    end
+    write.close
+    result = Marshal.load(read.read.unpack('m').first)
+    Process.wait2(pid)
+
+    fail result if result.class < Exception
+    result
+  end
+
+  # A trick to load action_controller without requiring in all specs. Not working in JRuby.
+  def using_action_controller
+    if RUBY_PLATFORM == 'java'
+      require 'action_controller'
+      yield
+    else
+      do_in_child do
+        require 'action_controller'
+        yield
+      end
+    end
+  end
+
   class_methods do
     def let_config(var_name)
       before do