diff --git a/lib/jsonapi/authorization/default_pundit_authorizer.rb b/lib/jsonapi/authorization/default_pundit_authorizer.rb index a1974189..a2502e3c 100644 --- a/lib/jsonapi/authorization/default_pundit_authorizer.rb +++ b/lib/jsonapi/authorization/default_pundit_authorizer.rb @@ -237,10 +237,15 @@ def include_has_one_resource(_source_record, related_record) private - def authorize_relationship_operation(source_record, relationship_method, *args) + def authorize_relationship_operation( + source_record, + relationship_method, + related_record_or_records = nil + ) policy = ::Pundit.policy(user, source_record) if policy.respond_to?(relationship_method) - unless policy.public_send(relationship_method, *args) + args = [relationship_method, related_record_or_records].reject(&:nil?) + unless policy.public_send(*args) raise ::Pundit::NotAuthorizedError, query: relationship_method, record: source_record, @@ -248,6 +253,11 @@ def authorize_relationship_operation(source_record, relationship_method, *args) end else ::Pundit.authorize(user, source_record, 'update?') + if related_record_or_records + Array(related_record_or_records).each do |related_record| + ::Pundit.authorize(user, related_record, 'update?') + end + end end end diff --git a/spec/jsonapi/authorization/default_pundit_authorizer_spec.rb b/spec/jsonapi/authorization/default_pundit_authorizer_spec.rb index f0e9a63d..1ddbaacc 100644 --- a/spec/jsonapi/authorization/default_pundit_authorizer_spec.rb +++ b/spec/jsonapi/authorization/default_pundit_authorizer_spec.rb @@ -7,6 +7,38 @@ let(:source_record) { Article.new } let(:authorizer) { described_class.new({}) } + shared_examples_for :update_singular_fallback do |related_record_method| + context 'authorized for update? on related record' do + before { stub_policy_actions(send(related_record_method), update?: true) } + + it { is_expected.not_to raise_error } + end + + context 'unauthorized for update? on related record' do + before { stub_policy_actions(send(related_record_method), update?: false) } + + it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } + end + end + + shared_examples_for :update_multiple_fallback do |related_records_method| + context 'authorized for update? on all related records' do + before do + send(related_records_method).each { |r| stub_policy_actions(r, update?: true) } + end + + it { is_expected.not_to raise_error } + end + + context 'unauthorized for update? on any related records' do + before do + stub_policy_actions(send(related_records_method).first, update?: false) + end + + it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } + end + end + describe '#find' do subject(:method_call) do -> { authorizer.find(source_record) } @@ -197,7 +229,7 @@ context 'where replace_? is undefined' do context 'authorized for update? on source record' do before { stub_policy_actions(source_record, update?: true) } - it { is_expected.not_to raise_error } + include_examples :update_singular_fallback, :related_record end context 'unauthorized for update? on source record' do @@ -302,7 +334,7 @@ context 'where replace_? is undefined' do context 'authorized for update? on source record' do before { stub_policy_actions(source_record, update?: true) } - it { is_expected.not_to raise_error } + include_examples :update_multiple_fallback, :related_records end context 'unauthorized for update? on source record' do @@ -348,20 +380,14 @@ it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } end - context 'authorized for create? where create_with_? is undefined' do - context 'authorized for update? on related record' do - before do - stub_policy_actions(source_class, create?: true) - stub_policy_actions(related_record, update?: true) - end - it { is_expected.not_to raise_error } + context 'where create_with_? is undefined' do + context 'authorized for create? on source class' do + before { stub_policy_actions(source_class, create?: true) } + include_examples :update_singular_fallback, :related_record end - context 'unauthorized for update? on related record' do - before do - stub_policy_actions(source_class, create?: true) - stub_policy_actions(related_record, update?: false) - end + context 'unauthorized for create? on source class' do + before { stub_policy_actions(source_class, create?: false) } it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } end end @@ -415,20 +441,14 @@ it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } end - context 'authorized for create? where create_with_? is undefined' do - context 'authorized for update? on related records' do - before do - stub_policy_actions(source_class, create?: true) - related_records.each { |r| stub_policy_actions(r, update?: true) } - end - it { is_expected.not_to raise_error } + context 'where create_with_? is undefined' do + context 'authorized for create? on source class' do + before { stub_policy_actions(source_class, create?: true) } + include_examples :update_multiple_fallback, :related_records end - context 'unauthorized for update? on any related records' do - before do - stub_policy_actions(source_class, create?: true) - stub_policy_actions(related_records.first, update?: false) - end + context 'unauthorized for create? on source class' do + before { stub_policy_actions(source_class, create?: false) } it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } end end @@ -477,21 +497,14 @@ it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } end - context 'where replace_? not defined' do - # CommentPolicy does not define #replace_article?, so #update? should determine authorization - let(:source_record) { comments(:comment_1) } - let(:related_records) { Article.new } - subject(:method_call) do - -> { authorizer.replace_to_one_relationship(source_record, related_record, :article) } - end - - context 'authorized for update? on record' do - before { allow_action(source_record, 'update?') } - it { is_expected.not_to raise_error } + context 'where replace_? is undefined' do + context 'authorized for update? on source record' do + before { stub_policy_actions(source_record, update?: true) } + include_examples :update_singular_fallback, :related_record end - context 'unauthorized for update? on record' do - before { disallow_action(source_record, 'update?') } + context 'unauthorized for update? on source record' do + before { stub_policy_actions(source_record, update?: false) } it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } end end @@ -524,19 +537,13 @@ end context 'where add_to_? not defined' do - # ArticlePolicy does not define #add_to_tags?, so #update? should determine authorization - let(:related_records) { Array.new(3) { Tag.new } } - subject(:method_call) do - -> { authorizer.create_to_many_relationship(source_record, related_records, :tags) } - end - context 'authorized for update? on record' do - before { allow_action(source_record, 'update?') } - it { is_expected.not_to raise_error } + before { stub_policy_actions(source_record, update?: true) } + include_examples :update_multiple_fallback, :related_records end context 'unauthorized for update? on record' do - before { disallow_action(source_record, 'update?') } + before { stub_policy_actions(source_record, update?: false) } it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } end end @@ -570,19 +577,13 @@ end context 'where replace_? not defined' do - # ArticlePolicy does not define #replace_tags?, so #update? should determine authorization - let(:new_tags) { Array.new(3) { Tag.new } } - subject(:method_call) do - -> { authorizer.replace_to_many_relationship(article, new_tags, :tags) } - end - context 'authorized for update? on record' do - before { allow_action(article, 'update?') } - it { is_expected.not_to raise_error } + before { stub_policy_actions(article, update?: true) } + include_examples :update_multiple_fallback, :new_comments end context 'unauthorized for update? on record' do - before { disallow_action(article, 'update?') } + before { stub_policy_actions(article, update?: false) } it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } end end @@ -616,19 +617,13 @@ end context 'where remove_from_? not defined' do - # ArticlePolicy does not define #remove_from_tags?, so #update? should determine authorization - let(:tags_to_remove) { article.tags.limit(2) } - subject(:method_call) do - -> { authorizer.create_to_many_relationship(article, tags_to_remove, :tags) } - end - context 'authorized for update? on article' do - before { allow_action(article, 'update?') } - it { is_expected.not_to raise_error } + before { stub_policy_actions(article, update?: true) } + include_examples :update_multiple_fallback, :comments_to_remove end context 'unauthorized for update? on article' do - before { disallow_action(article, 'update?') } + before { stub_policy_actions(article, update?: false) } it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } end end @@ -660,19 +655,13 @@ end context 'where remove_? not defined' do - # CommentPolicy does not define #remove_article?, so #update? should determine authorization - let(:source_record) { comments(:comment_1) } - subject(:method_call) do - -> { authorizer.remove_to_one_relationship(source_record, :article) } - end - context 'authorized for update? on record' do - before { allow_action(source_record, 'update?') } + before { stub_policy_actions(source_record, update?: true) } it { is_expected.not_to raise_error } end context 'unauthorized for update? on record' do - before { disallow_action(source_record, 'update?') } + before { stub_policy_actions(source_record, update?: false) } it { is_expected.to raise_error(::Pundit::NotAuthorizedError) } end end