From 43f73dbb35b031b396d87ab718f5b776a68c5ee9 Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Wed, 8 Mar 2017 11:33:39 -0300 Subject: [PATCH 01/18] Update remove_to_one_relationship Update both the authorizer and the default pundit policy to follow same pattern as the other relationship endpoints (sending the related record as an argument to the policy method). --- lib/jsonapi/authorization/authorizing_processor.rb | 10 +++++++++- lib/jsonapi/authorization/default_pundit_authorizer.rb | 4 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/jsonapi/authorization/authorizing_processor.rb b/lib/jsonapi/authorization/authorizing_processor.rb index 7bedefb2..dfb830f0 100644 --- a/lib/jsonapi/authorization/authorizing_processor.rb +++ b/lib/jsonapi/authorization/authorizing_processor.rb @@ -210,8 +210,16 @@ def authorize_remove_to_one_relationship params[:resource_id], context: context )._model - + relationship_type = params[:relationship_type].to_sym + relationship_resource = @resource_klass + ._relationship(relationship_type) + .resource_klass + .find_by_key( + params[:associated_key], + context: context + ) + related_record = related_resource._model unless related_resource.nil? authorizer.remove_to_one_relationship(source_record, relationship_type) end diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index 05863aa1..94f99b2f 100644 --- a/lib/jsonapi/authorization/default_pundit_authorizer.rb +++ b/lib/jsonapi/authorization/default_pundit_authorizer.rb @@ -191,9 +191,9 @@ def remove_to_many_relationship(source_record, related_record, relationship_type # # * +source_record+ - The record whose relationship is modified # * +relationship_type+ - The relationship type - def remove_to_one_relationship(source_record, relationship_type) + def remove_to_one_relationship(source_record, related_record, relationship_type) relationship_method = "remove_#{relationship_type}?" - authorize_relationship_operation(source_record, relationship_method) + authorize_relationship_operation(source_record, relationship_method, related_record) end # Any request including ?include=other-resources From 933547ff8520fd3845e644801735c9ad78ad3a76 Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Wed, 8 Mar 2017 11:34:54 -0300 Subject: [PATCH 02/18] Add related_models_with_context method helper --- .../authorization/authorizing_processor.rb | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/lib/jsonapi/authorization/authorizing_processor.rb b/lib/jsonapi/authorization/authorizing_processor.rb index dfb830f0..c936be8f 100644 --- a/lib/jsonapi/authorization/authorizing_processor.rb +++ b/lib/jsonapi/authorization/authorizing_processor.rb @@ -269,6 +269,41 @@ def related_models end end + def related_models_with_context + data = params[:data] + return { relationship: nil, relation_name: nil, records: nil } if data.nil? + + [:to_one, :to_many].flat_map do |rel_type| + data[rel_type].flat_map do |assoc_name, assoc_value| + related_models = + case assoc_value + when Hash # polymorphic relationship + resource_class = @resource_klass.resource_for(assoc_value[:type].to_s) + resource_class.find_by_key(assoc_value[:id], context: context)._model + when Array + resource_class = @resource_klass.resource_for(assoc_name.to_s) + resource_class.find_by_keys(assoc_value, context: context).map(&:_model) + else + resource_class = resource_class_for_relationship(assoc_name) + primary_key = resource_class._primary_key.to_sym + resource_class._model_class.find_by(primary_key => assoc_value) + end + old_records = if params[:resource_id] + @resource_klass.find_by_key( + params[:resource_id], + context: context + )._model.send(assoc_name) + end + { + relationship: @resource_klass._relationships[assoc_name], + relation_name: assoc_name, + records: related_models, + old_records: old_records + } + end + end + end + def authorize_model_includes(source_record) if params[:include_directives] params[:include_directives].model_includes.each do |include_item| From f0d300a08157e379b9a5ecea8d1891f0de78b5e8 Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Wed, 8 Mar 2017 11:38:27 -0300 Subject: [PATCH 03/18] User related_models_with_context for authorize_create_resource and authorize_replace_fields --- lib/jsonapi/authorization/authorizing_processor.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/jsonapi/authorization/authorizing_processor.rb b/lib/jsonapi/authorization/authorizing_processor.rb index c936be8f..c35dd570 100644 --- a/lib/jsonapi/authorization/authorizing_processor.rb +++ b/lib/jsonapi/authorization/authorizing_processor.rb @@ -103,18 +103,18 @@ def authorize_show_related_resources end def authorize_replace_fields + field_data = params[:data] source_record = @resource_klass.find_by_key( params[:resource_id], context: context )._model - - authorizer.replace_fields(source_record, related_models) + authorizer.replace_fields(source_record, related_models_with_context) end def authorize_create_resource - source_class = @resource_klass._model_class - - authorizer.create_resource(source_class, related_models) + field_data = params[:data] + source_class = resource_klass._model_class + authorizer.create_resource(source_class, related_models_with_context) end def authorize_remove_resource From d79acfc194be82b9e04908ecb36ae512401bd008 Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Wed, 8 Mar 2017 11:45:55 -0300 Subject: [PATCH 04/18] Add relationship policy checks to create_resource --- .../default_pundit_authorizer.rb | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index 94f99b2f..fc57d03b 100644 --- a/lib/jsonapi/authorization/default_pundit_authorizer.rb +++ b/lib/jsonapi/authorization/default_pundit_authorizer.rb @@ -104,14 +104,28 @@ def replace_fields(source_record, new_related_records) # ==== Parameters # # * +source_class+ - The class of the record to be created - # * +related_records+ - An array of records to be associated to the new - # record. This will contain the records specified in the - # "relationships" key in the request - def create_resource(source_class, related_records) + # * +related_records_with_context+ - A hash with the relationship, + # relation name, and records to be associated with the new record. This + # will contain the records specified in the "relationships" key in the request + def create_resource(source_class, related_records_with_context) ::Pundit.authorize(user, source_class, 'create?') - related_records.each do |record| - ::Pundit.authorize(user, record, 'update?') + policy = ::Pundit.policy!(user, source_class) + related_records_with_context.each do |data| + records = data[:records] + add_method_name = relationship_method(data, prefix: 'add') + allowed = if policy.respond_to?(add_method_name) + policy.public_send(add_method_name, records) + else + policy.update? + end + + unless allowed + raise ::Pundit::NotAuthorizedError, + query: add_method_name + record: records, + policy: policy + end end end From 45b8e7ea3961211039d66e4fb625acb344a13d2c Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Wed, 8 Mar 2017 11:50:17 -0300 Subject: [PATCH 05/18] Add relationship policy checking to replace_fields --- .../default_pundit_authorizer.rb | 51 +++++++++++++++---- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index fc57d03b..bd4fe6a4 100644 --- a/lib/jsonapi/authorization/default_pundit_authorizer.rb +++ b/lib/jsonapi/authorization/default_pundit_authorizer.rb @@ -86,16 +86,49 @@ def show_related_resources(source_record) # ==== Parameters # # * +source_record+ - The record to be modified - # * +new_related_records+ - An array of records to be associated to the - # +source_record+. This will contain the records specified in the - # "relationships" key in the request - #-- - # TODO: Should probably take old records as well - def replace_fields(source_record, new_related_records) + # * +related_records_with_context+ - A has with the relationship object, + # the relationship name, an Array of new related records, and an Array + # of old related records that are to be replaced. + def replace_fields(source_record, related_records_with_context) ::Pundit.authorize(user, source_record, 'update?') - new_related_records.each do |record| - ::Pundit.authorize(user, record, 'update?') + policy = ::Pundit.policy!(user, source_record) + related_records_with_context.each do |data| + relationship = data[:relationship] + relation_name = data[:relation_name] + new_records = data[:records] + old_records = data[:old_records] + remove_method_name = relationship_method(data, prefix: 'remove') + add_method_name = relationship_method(data, prefix: 'add') + + if old_records + allowed_to_remove= + if policy.respond_to?(remove_method_name) + policy.public_send(remove_method_name, old_records) + else + policy.update? + end + unless allowed_to_remove + raise ::Pundit::NotAuthorizedError, + query: remove_method_name, + record: old_records, + policy: policy + end + end + + allowed_to_add = + if policy.respond_to?(add_method_name) + policy.public_send(add_method_name, new_records) + else + policy.update? + end + + unless allowed_to_add + raise ::Pundit::NotAuthorizedError, + query: add_method_name, + record: new_records, + policy: policy + end end end @@ -122,7 +155,7 @@ def create_resource(source_class, related_records_with_context) unless allowed raise ::Pundit::NotAuthorizedError, - query: add_method_name + query: add_method_name, record: records, policy: policy end From 44779eff4427ab9bc66a4252f5467ad9c27440e1 Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Wed, 8 Mar 2017 11:59:06 -0300 Subject: [PATCH 06/18] Add relationship_method helper --- .../authorization/default_pundit_authorizer.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index bd4fe6a4..9fad5d6d 100644 --- a/lib/jsonapi/authorization/default_pundit_authorizer.rb +++ b/lib/jsonapi/authorization/default_pundit_authorizer.rb @@ -292,6 +292,23 @@ def authorize_relationship_operation(source_record, relationship_method, *args) ::Pundit.authorize(user, source_record, 'update?') end end + + def relationship_method(data, **options) + relationship = data[:relationship] + assoc_name = data[:relation_name] + prefix = options[:prefix] + + case relationship + when ->(relationship) { relationship.polymorphic } + polymorphic_type = data[:records].class.name.downcase + "#{prefix}_#{relationship.class_name.downcase}_#{polymorphic_type}?" + when ->(relationship) { relationship.class == JSONAPI::Relationship::ToOne } + "#{prefix}_#{assoc_name}?" + else + prefix_preposition = prefix == 'add' ? 'to' : 'from' + "#{prefix}_#{prefix_preposition}_#{assoc_name}?" + end + end end end end From 78c84679d0fd2b61dd33488902008eb1e36fee1f Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Wed, 8 Mar 2017 12:10:52 -0300 Subject: [PATCH 07/18] Minor styling changes --- .../authorization/default_pundit_authorizer.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index 9fad5d6d..06db83c7 100644 --- a/lib/jsonapi/authorization/default_pundit_authorizer.rb +++ b/lib/jsonapi/authorization/default_pundit_authorizer.rb @@ -110,9 +110,9 @@ def replace_fields(source_record, related_records_with_context) end unless allowed_to_remove raise ::Pundit::NotAuthorizedError, - query: remove_method_name, - record: old_records, - policy: policy + query: remove_method_name, + record: old_records, + policy: policy end end @@ -125,9 +125,9 @@ def replace_fields(source_record, related_records_with_context) unless allowed_to_add raise ::Pundit::NotAuthorizedError, - query: add_method_name, - record: new_records, - policy: policy + query: add_method_name, + record: new_records, + policy: policy end end end @@ -155,9 +155,9 @@ def create_resource(source_class, related_records_with_context) unless allowed raise ::Pundit::NotAuthorizedError, - query: add_method_name, - record: records, - policy: policy + query: add_method_name, + record: records, + policy: policy end end end From 34a06efd8920049a27ce9ff0b90c9b38f1ecad19 Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Wed, 8 Mar 2017 12:38:43 -0300 Subject: [PATCH 08/18] Add missing param and fix typo --- lib/jsonapi/authorization/authorizing_processor.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/jsonapi/authorization/authorizing_processor.rb b/lib/jsonapi/authorization/authorizing_processor.rb index c35dd570..98dfd994 100644 --- a/lib/jsonapi/authorization/authorizing_processor.rb +++ b/lib/jsonapi/authorization/authorizing_processor.rb @@ -216,12 +216,12 @@ def authorize_remove_to_one_relationship ._relationship(relationship_type) .resource_klass .find_by_key( - params[:associated_key], + params[:resource_id], context: context ) - related_record = related_resource._model unless related_resource.nil? + related_record = relationship_resource._model unless relationship_resource.nil? - authorizer.remove_to_one_relationship(source_record, relationship_type) + authorizer.remove_to_one_relationship(source_record, related_record, relationship_type) end private From 49d3390b203cb912e2653202552fbd4bf28fb118 Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Thu, 9 Mar 2017 10:25:26 -0300 Subject: [PATCH 09/18] Change back remove_to_one_relationship methods --- lib/jsonapi/authorization/authorizing_processor.rb | 12 ++---------- .../authorization/default_pundit_authorizer.rb | 4 ++-- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/lib/jsonapi/authorization/authorizing_processor.rb b/lib/jsonapi/authorization/authorizing_processor.rb index 98dfd994..25c1fd01 100644 --- a/lib/jsonapi/authorization/authorizing_processor.rb +++ b/lib/jsonapi/authorization/authorizing_processor.rb @@ -210,18 +210,10 @@ def authorize_remove_to_one_relationship params[:resource_id], context: context )._model - + relationship_type = params[:relationship_type].to_sym - relationship_resource = @resource_klass - ._relationship(relationship_type) - .resource_klass - .find_by_key( - params[:resource_id], - context: context - ) - related_record = relationship_resource._model unless relationship_resource.nil? - authorizer.remove_to_one_relationship(source_record, related_record, relationship_type) + authorizer.remove_to_one_relationship(source_record, relationship_type) end private diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index 06db83c7..3167d260 100644 --- a/lib/jsonapi/authorization/default_pundit_authorizer.rb +++ b/lib/jsonapi/authorization/default_pundit_authorizer.rb @@ -238,9 +238,9 @@ def remove_to_many_relationship(source_record, related_record, relationship_type # # * +source_record+ - The record whose relationship is modified # * +relationship_type+ - The relationship type - def remove_to_one_relationship(source_record, related_record, relationship_type) + def remove_to_one_relationship(source_record, relationship_type) relationship_method = "remove_#{relationship_type}?" - authorize_relationship_operation(source_record, relationship_method, related_record) + authorize_relationship_operation(source_record, relationship_method) end # Any request including ?include=other-resources From 2d8222c7a758172b21ce8de413d226c3f33f9a61 Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Fri, 10 Mar 2017 13:05:58 -0300 Subject: [PATCH 10/18] Remove old records from helper, use authorize_association_operation helper, refactorings. --- .../authorization/authorizing_processor.rb | 7 -- .../default_pundit_authorizer.rb | 71 +++---------------- 2 files changed, 11 insertions(+), 67 deletions(-) diff --git a/lib/jsonapi/authorization/authorizing_processor.rb b/lib/jsonapi/authorization/authorizing_processor.rb index 25c1fd01..c131c592 100644 --- a/lib/jsonapi/authorization/authorizing_processor.rb +++ b/lib/jsonapi/authorization/authorizing_processor.rb @@ -280,17 +280,10 @@ def related_models_with_context primary_key = resource_class._primary_key.to_sym resource_class._model_class.find_by(primary_key => assoc_value) end - old_records = if params[:resource_id] - @resource_klass.find_by_key( - params[:resource_id], - context: context - )._model.send(assoc_name) - end { relationship: @resource_klass._relationships[assoc_name], relation_name: assoc_name, records: related_models, - old_records: old_records } end end diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index 3167d260..9b80a961 100644 --- a/lib/jsonapi/authorization/default_pundit_authorizer.rb +++ b/lib/jsonapi/authorization/default_pundit_authorizer.rb @@ -87,48 +87,14 @@ def show_related_resources(source_record) # # * +source_record+ - The record to be modified # * +related_records_with_context+ - A has with the relationship object, - # the relationship name, an Array of new related records, and an Array - # of old related records that are to be replaced. + # the relationship name, an Array of new related records. def replace_fields(source_record, related_records_with_context) ::Pundit.authorize(user, source_record, 'update?') - - policy = ::Pundit.policy!(user, source_record) related_records_with_context.each do |data| relationship = data[:relationship] relation_name = data[:relation_name] - new_records = data[:records] - old_records = data[:old_records] - remove_method_name = relationship_method(data, prefix: 'remove') - add_method_name = relationship_method(data, prefix: 'add') - - if old_records - allowed_to_remove= - if policy.respond_to?(remove_method_name) - policy.public_send(remove_method_name, old_records) - else - policy.update? - end - unless allowed_to_remove - raise ::Pundit::NotAuthorizedError, - query: remove_method_name, - record: old_records, - policy: policy - end - end - - allowed_to_add = - if policy.respond_to?(add_method_name) - policy.public_send(add_method_name, new_records) - else - policy.update? - end - - unless allowed_to_add - raise ::Pundit::NotAuthorizedError, - query: add_method_name, - record: new_records, - policy: policy - end + records = data[:records] + authorize_relationship_operation(source_record, relationship_method(data), records) end end @@ -142,23 +108,10 @@ def replace_fields(source_record, related_records_with_context) # will contain the records specified in the "relationships" key in the request def create_resource(source_class, related_records_with_context) ::Pundit.authorize(user, source_class, 'create?') - - policy = ::Pundit.policy!(user, source_class) related_records_with_context.each do |data| records = data[:records] - add_method_name = relationship_method(data, prefix: 'add') - allowed = if policy.respond_to?(add_method_name) - policy.public_send(add_method_name, records) - else - policy.update? - end - - unless allowed - raise ::Pundit::NotAuthorizedError, - query: add_method_name, - record: records, - policy: policy - end + add_method_name = relationship_method(data) + authorize_relationship_operation(source_class, add_method_name, records) end end @@ -293,20 +246,18 @@ def authorize_relationship_operation(source_record, relationship_method, *args) end end - def relationship_method(data, **options) + def relationship_method(data) relationship = data[:relationship] assoc_name = data[:relation_name] - prefix = options[:prefix] case relationship - when ->(relationship) { relationship.polymorphic } + when relationship.polymorphic polymorphic_type = data[:records].class.name.downcase - "#{prefix}_#{relationship.class_name.downcase}_#{polymorphic_type}?" - when ->(relationship) { relationship.class == JSONAPI::Relationship::ToOne } - "#{prefix}_#{assoc_name}?" + "#add_#{relationship.class_name.downcase}_#{polymorphic_type}?" + when relationship.class == JSONAPI::Relationship::ToOne + "add_#{assoc_name}?" else - prefix_preposition = prefix == 'add' ? 'to' : 'from' - "#{prefix}_#{prefix_preposition}_#{assoc_name}?" + "add_to_#{assoc_name}?" end end end From 00a179df5086b0e62d029fb62d406eef0b419688 Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Mon, 13 Mar 2017 10:15:16 -0300 Subject: [PATCH 11/18] Fix up case statement --- lib/jsonapi/authorization/default_pundit_authorizer.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index 9b80a961..25faef02 100644 --- a/lib/jsonapi/authorization/default_pundit_authorizer.rb +++ b/lib/jsonapi/authorization/default_pundit_authorizer.rb @@ -251,10 +251,10 @@ def relationship_method(data) assoc_name = data[:relation_name] case relationship - when relationship.polymorphic + when ->(relationship) { relationship.polymorphic } polymorphic_type = data[:records].class.name.downcase "#add_#{relationship.class_name.downcase}_#{polymorphic_type}?" - when relationship.class == JSONAPI::Relationship::ToOne + when JSONAPI::Relationship::ToOne "add_#{assoc_name}?" else "add_to_#{assoc_name}?" From 5a83560c6731e4fdbf673552f9eeb79e835a1c22 Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Mon, 13 Mar 2017 11:06:48 -0300 Subject: [PATCH 12/18] Properly handle old record removal validation in replace_fields --- .../default_pundit_authorizer.rb | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index 25faef02..65e448bb 100644 --- a/lib/jsonapi/authorization/default_pundit_authorizer.rb +++ b/lib/jsonapi/authorization/default_pundit_authorizer.rb @@ -93,7 +93,25 @@ def replace_fields(source_record, related_records_with_context) related_records_with_context.each do |data| relationship = data[:relationship] relation_name = data[:relation_name] - records = data[:records] + records = data[:records] + + # Authorize removing old records + case relationship + when JSONAPI::Relationship::ToMany + old_records = source_record.send relation_name + authorize_relationship_operation( + source_record, + relationship_method(data, prefix: 'remove'), + old_records + ) + else + authorize_relationship_operation( + source_record, + relationship_method(data, prefix: 'remove') + ) + end + + # Authorize adding new records authorize_relationship_operation(source_record, relationship_method(data), records) end end @@ -246,18 +264,20 @@ def authorize_relationship_operation(source_record, relationship_method, *args) end end - def relationship_method(data) + def relationship_method(data, **options) relationship = data[:relationship] assoc_name = data[:relation_name] + prefix = options[:prefix] || 'add' case relationship when ->(relationship) { relationship.polymorphic } polymorphic_type = data[:records].class.name.downcase - "#add_#{relationship.class_name.downcase}_#{polymorphic_type}?" + "#{prefix}_#{relationship.class_name.downcase}_#{polymorphic_type}?" when JSONAPI::Relationship::ToOne - "add_#{assoc_name}?" + "#{prefix}_#{assoc_name}?" else - "add_to_#{assoc_name}?" + prefix_preposition = prefix == 'add' ? 'to' : 'from' + "#{prefix}_#{prefix_preposition}_#{assoc_name}?" end end end From 09b26e496e57d6dcf2a8538a1507bc7e34f8e993 Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Mon, 13 Mar 2017 11:51:49 -0300 Subject: [PATCH 13/18] Fix rubocop complaints --- lib/jsonapi/authorization/authorizing_processor.rb | 4 +--- lib/jsonapi/authorization/default_pundit_authorizer.rb | 10 +++++----- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/jsonapi/authorization/authorizing_processor.rb b/lib/jsonapi/authorization/authorizing_processor.rb index c131c592..46d9a9eb 100644 --- a/lib/jsonapi/authorization/authorizing_processor.rb +++ b/lib/jsonapi/authorization/authorizing_processor.rb @@ -103,7 +103,6 @@ def authorize_show_related_resources end def authorize_replace_fields - field_data = params[:data] source_record = @resource_klass.find_by_key( params[:resource_id], context: context @@ -112,7 +111,6 @@ def authorize_replace_fields end def authorize_create_resource - field_data = params[:data] source_class = resource_klass._model_class authorizer.create_resource(source_class, related_models_with_context) end @@ -283,7 +281,7 @@ def related_models_with_context { relationship: @resource_klass._relationships[assoc_name], relation_name: assoc_name, - records: related_models, + records: related_models } end end diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index 65e448bb..eb3993ac 100644 --- a/lib/jsonapi/authorization/default_pundit_authorizer.rb +++ b/lib/jsonapi/authorization/default_pundit_authorizer.rb @@ -95,13 +95,13 @@ def replace_fields(source_record, related_records_with_context) relation_name = data[:relation_name] records = data[:records] - # Authorize removing old records + # Authorize removing old records case relationship when JSONAPI::Relationship::ToMany old_records = source_record.send relation_name authorize_relationship_operation( - source_record, - relationship_method(data, prefix: 'remove'), + source_record, + relationship_method(data, prefix: 'remove'), old_records ) else @@ -121,7 +121,7 @@ def replace_fields(source_record, related_records_with_context) # ==== Parameters # # * +source_class+ - The class of the record to be created - # * +related_records_with_context+ - A hash with the relationship, + # * +related_records_with_context+ - A hash with the relationship, # relation name, and records to be associated with the new record. This # will contain the records specified in the "relationships" key in the request def create_resource(source_class, related_records_with_context) @@ -270,7 +270,7 @@ def relationship_method(data, **options) prefix = options[:prefix] || 'add' case relationship - when ->(relationship) { relationship.polymorphic } + when ->(rel) { rel.polymorphic } polymorphic_type = data[:records].class.name.downcase "#{prefix}_#{relationship.class_name.downcase}_#{polymorphic_type}?" when JSONAPI::Relationship::ToOne From 061d16515973bfd9f8884ab9e3c5217465da0898 Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Mon, 13 Mar 2017 15:36:33 -0300 Subject: [PATCH 14/18] Properly format polymorphic type --- lib/jsonapi/authorization/default_pundit_authorizer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index eb3993ac..37b546dc 100644 --- a/lib/jsonapi/authorization/default_pundit_authorizer.rb +++ b/lib/jsonapi/authorization/default_pundit_authorizer.rb @@ -271,7 +271,7 @@ def relationship_method(data, **options) case relationship when ->(rel) { rel.polymorphic } - polymorphic_type = data[:records].class.name.downcase + polymorphic_type = data[:records].class.name.underscore "#{prefix}_#{relationship.class_name.downcase}_#{polymorphic_type}?" when JSONAPI::Relationship::ToOne "#{prefix}_#{assoc_name}?" From b09fbb3d77d1a1956362672937b9a2bce209a2e4 Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Mon, 3 Apr 2017 22:10:17 -0300 Subject: [PATCH 15/18] Refactor `create_resources` and `replace_fields` - Utilizes `authorize_replace_to_many_relationship` and `authorize_replace_to_one_relationship` helpers. - Removes helpers no longer used after refactor. --- .../authorization/authorizing_processor.rb | 5 +- .../default_pundit_authorizer.rb | 56 +++++-------------- 2 files changed, 16 insertions(+), 45 deletions(-) diff --git a/lib/jsonapi/authorization/authorizing_processor.rb b/lib/jsonapi/authorization/authorizing_processor.rb index 46d9a9eb..bb721918 100644 --- a/lib/jsonapi/authorization/authorizing_processor.rb +++ b/lib/jsonapi/authorization/authorizing_processor.rb @@ -271,15 +271,16 @@ def related_models_with_context resource_class = @resource_klass.resource_for(assoc_value[:type].to_s) resource_class.find_by_key(assoc_value[:id], context: context)._model when Array - resource_class = @resource_klass.resource_for(assoc_name.to_s) + resource_class = resource_class_for_relationship(assoc_name) resource_class.find_by_keys(assoc_value, context: context).map(&:_model) else resource_class = resource_class_for_relationship(assoc_name) primary_key = resource_class._primary_key.to_sym resource_class._model_class.find_by(primary_key => assoc_value) end + { - relationship: @resource_klass._relationships[assoc_name], + relation_type: rel_type, relation_name: assoc_name, records: related_models } diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index 37b546dc..e3c268a9 100644 --- a/lib/jsonapi/authorization/default_pundit_authorizer.rb +++ b/lib/jsonapi/authorization/default_pundit_authorizer.rb @@ -90,30 +90,7 @@ def show_related_resources(source_record) # the relationship name, an Array of new related records. def replace_fields(source_record, related_records_with_context) ::Pundit.authorize(user, source_record, 'update?') - related_records_with_context.each do |data| - relationship = data[:relationship] - relation_name = data[:relation_name] - records = data[:records] - - # Authorize removing old records - case relationship - when JSONAPI::Relationship::ToMany - old_records = source_record.send relation_name - authorize_relationship_operation( - source_record, - relationship_method(data, prefix: 'remove'), - old_records - ) - else - authorize_relationship_operation( - source_record, - relationship_method(data, prefix: 'remove') - ) - end - - # Authorize adding new records - authorize_relationship_operation(source_record, relationship_method(data), records) - end + authorize_related_records(source_record, related_records_with_context) end # POST /resources @@ -126,11 +103,7 @@ def replace_fields(source_record, related_records_with_context) # will contain the records specified in the "relationships" key in the request def create_resource(source_class, related_records_with_context) ::Pundit.authorize(user, source_class, 'create?') - related_records_with_context.each do |data| - records = data[:records] - add_method_name = relationship_method(data) - authorize_relationship_operation(source_class, add_method_name, records) - end + authorize_related_records(source_class, related_records_with_context) end # DELETE /resources/:id @@ -264,20 +237,17 @@ def authorize_relationship_operation(source_record, relationship_method, *args) end end - def relationship_method(data, **options) - relationship = data[:relationship] - assoc_name = data[:relation_name] - prefix = options[:prefix] || 'add' - - case relationship - when ->(rel) { rel.polymorphic } - polymorphic_type = data[:records].class.name.underscore - "#{prefix}_#{relationship.class_name.downcase}_#{polymorphic_type}?" - when JSONAPI::Relationship::ToOne - "#{prefix}_#{assoc_name}?" - else - prefix_preposition = prefix == 'add' ? 'to' : 'from' - "#{prefix}_#{prefix_preposition}_#{assoc_name}?" + def authorize_related_records(source_record, related_records_with_context) + related_records_with_context.each do |data| + relation_type = data[:relation_type] + relation_name = data[:relation_name] + records = data[:records] + case relation_type + when :to_many + replace_to_many_relationship(source_class, records, relation_name) + when :to_one + replace_to_one_relationship(source_record, records, relation_name) + end end end end From 4a95aa26fa947e4c5a0bda36078ccd9b06a59fe0 Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Tue, 4 Apr 2017 21:25:05 -0300 Subject: [PATCH 16/18] Rollback create_resource --- lib/jsonapi/authorization/authorizing_processor.rb | 2 +- .../authorization/default_pundit_authorizer.rb | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/jsonapi/authorization/authorizing_processor.rb b/lib/jsonapi/authorization/authorizing_processor.rb index bb721918..738b7fcf 100644 --- a/lib/jsonapi/authorization/authorizing_processor.rb +++ b/lib/jsonapi/authorization/authorizing_processor.rb @@ -112,7 +112,7 @@ def authorize_replace_fields def authorize_create_resource source_class = resource_klass._model_class - authorizer.create_resource(source_class, related_models_with_context) + authorizer.create_resource(source_class, related_models) end def authorize_remove_resource diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index e3c268a9..c1e1e95f 100644 --- a/lib/jsonapi/authorization/default_pundit_authorizer.rb +++ b/lib/jsonapi/authorization/default_pundit_authorizer.rb @@ -98,12 +98,15 @@ def replace_fields(source_record, related_records_with_context) # ==== Parameters # # * +source_class+ - The class of the record to be created - # * +related_records_with_context+ - A hash with the relationship, - # relation name, and records to be associated with the new record. This - # will contain the records specified in the "relationships" key in the request - def create_resource(source_class, related_records_with_context) + # * +related_records+ - An array of records to be associated to the new + # record. This will contain the records specified in the + # "relationships" key in the request + def create_resource(source_class, related_records) ::Pundit.authorize(user, source_class, 'create?') - authorize_related_records(source_class, related_records_with_context) + + related_records.each do |record| + ::Pundit.authorize(user, record, 'update?') + end end # DELETE /resources/:id From 082d9ebe6d1ecd1ac5cbc0043f0cef110b07429e Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Tue, 4 Apr 2017 22:48:17 -0300 Subject: [PATCH 17/18] Add spec coverage for #replace_fields --- .../default_pundit_authorizer.rb | 4 +-- .../default_pundit_authorizer_spec.rb | 35 ++++++++----------- spec/requests/tricky_operations_spec.rb | 11 ++++-- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index c1e1e95f..d2c6baba 100644 --- a/lib/jsonapi/authorization/default_pundit_authorizer.rb +++ b/lib/jsonapi/authorization/default_pundit_authorizer.rb @@ -86,7 +86,7 @@ def show_related_resources(source_record) # ==== Parameters # # * +source_record+ - The record to be modified - # * +related_records_with_context+ - A has with the relationship object, + # * +related_records_with_context+ - A hash with the association type, # the relationship name, an Array of new related records. def replace_fields(source_record, related_records_with_context) ::Pundit.authorize(user, source_record, 'update?') @@ -247,7 +247,7 @@ def authorize_related_records(source_record, related_records_with_context) records = data[:records] case relation_type when :to_many - replace_to_many_relationship(source_class, records, relation_name) + replace_to_many_relationship(source_record, records, relation_name) when :to_one replace_to_one_relationship(source_record, records, relation_name) end diff --git a/spec/jsonapi/authorization/default_pundit_authorizer_spec.rb b/spec/jsonapi/authorization/default_pundit_authorizer_spec.rb index 2b99494d..5c6c640e 100644 --- a/spec/jsonapi/authorization/default_pundit_authorizer_spec.rb +++ b/spec/jsonapi/authorization/default_pundit_authorizer_spec.rb @@ -161,8 +161,13 @@ describe '#replace_fields' do let(:related_records) { Array.new(3) { Comment.new } } + let(:related_records_with_context) do + Array.new(1) { + Hash[:relation_name, :comments, :relation_type, :to_many, :records, related_records] + } + end subject(:method_call) do - -> { authorizer.replace_fields(source_record, related_records) } + -> { authorizer.replace_fields(source_record, related_records_with_context) } end context 'authorized for update? on source record' do @@ -173,18 +178,13 @@ it { is_expected.not_to raise_error } end - context 'authorized for update? on all of the related records' do - before { related_records.each { |r| allow_action(r, 'update?') } } - it { is_expected.not_to raise_error } + context 'authorized for replace_comments? on source record' do + before { stub_policy_actions(source_record, replace_comments?: true, update?: true) } + it { is_expected.not_to raise_error(::Pundit::NotAuthorizedError) } end - context 'unauthorized for update? on any of the related records' do - let(:related_records) { [Comment.new(id: 1), Comment.new(id: 2)] } - before do - allow_action(related_records.first, 'update?') - disallow_action(related_records.last, 'update?') - end - + context 'unauthorized for replace_comments? on source record' do + before { stub_policy_actions(source_record, replace_comments?: false, update?: true) } it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } end end @@ -197,18 +197,13 @@ it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } end - context 'authorized for update? on all of the related records' do - before { related_records.each { |r| allow_action(r, 'update?') } } + context 'authorized for replace_comments? on source record' do + before { stub_policy_actions(source_record, replace_comments?: true, update?: false) } it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } end - context 'unauthorized for update? on any of the related records' do - let(:related_records) { [Comment.new(id: 1), Comment.new(id: 2)] } - before do - allow_action(related_records.first, 'update?') - disallow_action(related_records.last, 'update?') - end - + context 'unauthorized for replace_comments? on source record' do + before { stub_policy_actions(source_record, replace_comments?: false, update?: false) } it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } end end diff --git a/spec/requests/tricky_operations_spec.rb b/spec/requests/tricky_operations_spec.rb index 19bc353f..3b346062 100644 --- a/spec/requests/tricky_operations_spec.rb +++ b/spec/requests/tricky_operations_spec.rb @@ -92,6 +92,11 @@ let!(:new_comments) do Array.new(2) { Comment.create } end + let(:related_records_with_context) do + Array.new(1) { + Hash[:relation_name, :comments, :relation_type, :to_many, :records, new_comments] + } + end let(:policy_scope) { Article.where(id: article.id) } let(:comments_policy_scope) { Comment.all } before do @@ -120,13 +125,13 @@ context 'authorized for replace_fields on article and all new records' do context 'not limited by Comments policy scope' do - before { allow_operation('replace_fields', article, new_comments) } + before { allow_operation('replace_fields', article, related_records_with_context) } it { is_expected.to be_successful } end context 'limited by Comments policy scope' do let(:comments_policy_scope) { Comment.where("id NOT IN (?)", new_comments.map(&:id)) } - before { allow_operation('replace_fields', article, new_comments) } + before { allow_operation('replace_fields', article, related_records_with_context) } it do pending 'DISCUSS: Should this error out somehow?' @@ -136,7 +141,7 @@ end context 'unauthorized for replace_fields on article and all new records' do - before { disallow_operation('replace_fields', article, new_comments) } + before { disallow_operation('replace_fields', article, related_records_with_context) } it { is_expected.to be_forbidden } end From 41d641cba56af6f90e13807e0a7fc5e332ba49ab Mon Sep 17 00:00:00 2001 From: Greg Fisher Date: Wed, 5 Apr 2017 07:42:20 -0300 Subject: [PATCH 18/18] Switch to hash and array literal in spec, use resource.find_by_key --- lib/jsonapi/authorization/authorizing_processor.rb | 3 +-- .../authorization/default_pundit_authorizer_spec.rb | 8 +++++--- spec/requests/tricky_operations_spec.rb | 8 +++++--- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/lib/jsonapi/authorization/authorizing_processor.rb b/lib/jsonapi/authorization/authorizing_processor.rb index 738b7fcf..acaaa324 100644 --- a/lib/jsonapi/authorization/authorizing_processor.rb +++ b/lib/jsonapi/authorization/authorizing_processor.rb @@ -275,8 +275,7 @@ def related_models_with_context resource_class.find_by_keys(assoc_value, context: context).map(&:_model) else resource_class = resource_class_for_relationship(assoc_name) - primary_key = resource_class._primary_key.to_sym - resource_class._model_class.find_by(primary_key => assoc_value) + resource_class.find_by_key(assoc_value[:id], context: context)._model end { diff --git a/spec/jsonapi/authorization/default_pundit_authorizer_spec.rb b/spec/jsonapi/authorization/default_pundit_authorizer_spec.rb index 5c6c640e..63599ab5 100644 --- a/spec/jsonapi/authorization/default_pundit_authorizer_spec.rb +++ b/spec/jsonapi/authorization/default_pundit_authorizer_spec.rb @@ -162,9 +162,11 @@ describe '#replace_fields' do let(:related_records) { Array.new(3) { Comment.new } } let(:related_records_with_context) do - Array.new(1) { - Hash[:relation_name, :comments, :relation_type, :to_many, :records, related_records] - } + [{ + relation_name: :comments, + relation_type: :to_many, + records: related_records + }] end subject(:method_call) do -> { authorizer.replace_fields(source_record, related_records_with_context) } diff --git a/spec/requests/tricky_operations_spec.rb b/spec/requests/tricky_operations_spec.rb index 3b346062..9d6dd876 100644 --- a/spec/requests/tricky_operations_spec.rb +++ b/spec/requests/tricky_operations_spec.rb @@ -93,9 +93,11 @@ Array.new(2) { Comment.create } end let(:related_records_with_context) do - Array.new(1) { - Hash[:relation_name, :comments, :relation_type, :to_many, :records, new_comments] - } + [{ + relation_name: :comments, + relation_type: :to_many, + records: new_comments + }] end let(:policy_scope) { Article.where(id: article.id) } let(:comments_policy_scope) { Comment.all }