Show related resources doesn't check related resource policy

[Matthijsy]

Hi,

I just found out that when accessing the relationships of a model (for example /users/1/addresses) with a has_many relationship only the model#show? policy is checked. I would expect that also the relationship policy is checked (in my example addresses#show?). What is the reason that this doesn't happen? Is this an error or is this intended?

In my user addresses example I cannot forbid a user to get addresses of all the other users without forbidding all users to see another user. This is not what I want. How can I make this happen?

[valscion]

When you're accessing something plural, like in your case "addresses", jsnonapi-authorization should call addresses#index? and also make the returned addresses records go through your policy scope.

Could you tell me a bit more about what you see happening?

[Matthijsy]

I have a user which has many addresses, I defined all the policy methods of addresses as false (for debugging). When I do a request to /addresses I get 403. However when I do /users/1/addresses I get the list of addresses. Do I need to include some mixin? I have included JSONAPI::Authorization::PunditScopedResource

When looking in the authorizer class I see some difference between /resource/:id/relationship and /resource?include=relationship. Is this maybe cause by that? Because when i do /user?include=addresses I do get a 403.

Reference:
https://github.com/venuu/jsonapi-authorization/blob/master/lib/jsonapi/authorization/default_pundit_authorizer.rb#L80
https://github.com/venuu/jsonapi-authorization/blob/master/lib/jsonapi/authorization/default_pundit_authorizer.rb#L226

[valscion]

Hmm yeah, PunditScopedResource should be enough to make it go through your policy scope:

jsonapi-authorization/lib/jsonapi/authorization/pundit_scoped_resource.rb

Lines 22 to 24 in 8176e89

	when JSONAPI::Relationship::ToMany
	user_context = JSONAPI::Authorization.configuration.user_context(context)
	::Pundit.policy_scope!(user_context, record_or_records)

Have you perhaps overridden the definition for records_for in your UserResource or somewhere else?

[valscion]

The specs here:

jsonapi-authorization/spec/requests/related_resources_operations_spec.rb

Lines 21 to 54 in 8176e89

	describe 'GET /articles/:id/comments' do
	subject(:last_response) { get("/articles/#{article.external_id}/comments") }
	let(:article) { articles(:article_with_comments) }
	let(:policy_scope) { Article.all }
	let(:comments_on_article) { article.comments }
	let(:comments_policy_scope) { comments_on_article.limit(1) }
	before do
	allow_any_instance_of(CommentPolicy::Scope).to receive(:resolve).and_return(comments_policy_scope)
	end
	context 'unauthorized for show_related_resources' do
	before { disallow_operation('show_related_resources', source_record: article) }
	it { is_expected.to be_forbidden }
	end
	context 'authorized for show_related_resources' do
	before { allow_operation('show_related_resources', source_record: article) }
	it { is_expected.to be_ok }
	# If this happens in real life, it's mostly a bug. We want to document the
	# behaviour in that case anyway, as it might be surprising.
	context 'limited by policy scope' do
	let(:policy_scope) { Article.where.not(id: article.id) }
	it { is_expected.to be_not_found }
	end
	it 'displays only comments allowed by CommentPolicy::Scope' do
	expect(json_data.length).to eq(1)
	expect(json_data.first["id"]).to eq(comments_policy_scope.first.id.to_s)
	end
	end
	end

Should say that jsonapi-authorization does apply the policy scope:

jsonapi-authorization/spec/requests/related_resources_operations_spec.rb

Lines 27 to 31 in 8176e89

	let(:comments_policy_scope) { comments_on_article.limit(1) }
	before do
	allow_any_instance_of(CommentPolicy::Scope).to receive(:resolve).and_return(comments_policy_scope)
	end

jsonapi-authorization/spec/requests/related_resources_operations_spec.rb

Lines 49 to 52 in 8176e89

	it 'displays only comments allowed by CommentPolicy::Scope' do
	expect(json_data.length).to eq(1)
	expect(json_data.first["id"]).to eq(comments_policy_scope.first.id.to_s)
	end

As otherwise the spec would've returned 3 comments instead of only 1:

jsonapi-authorization/spec/fixtures/comments.yml

Lines 1 to 13 in 8176e89

	---
	comment_1:
	article: article_with_comments
	author: user_with_comments
	reviewing_user: reviewer
	comment_2:
	article: article_with_comments
	author: user_with_comments
	reviewing_user: reviewer
	comment_3:
	article: article_with_comments
	author: user_with_comments
	reviewing_user: reviewer

[Matthijsy]

Now I see why it is not working. It is not calling AddressPolicy#index? but it's calling AddressPolicy::Scope#resolve. So I need to create a scope resolve method for the relationship. Is this the inteded behaviour?

[valscion]

Hmm... when viewing related resources, we do not call RelatedRecordPolicy#index? at all here:

jsonapi-authorization/lib/jsonapi/authorization/authorizing_processor.rb

Lines 103 to 110 in 8176e89

	def authorize_show_related_resources
	source_record = params[:source_klass].find_by_key(
	params[:source_id],
	context: context
	)._model
	authorizer.show_related_resources(source_record: source_record)
	end

That might be a bug. I wonder if we have the information in that place to figure out which related resource class we're trying to look at? The method should pass related_record_class: keyword argument to the authorizer if possible, and then that should be used to call:

def show_related_resources(source_record:, related_record_class:)
  ::Pundit.authorize(user, source_record, 'show?')
  ::Pundit.authorize(user, related_record_class, 'index?')
end

[valscion]

Now I see why it is not working. It is not calling AddressPolicy#index? but it's calling AddressPolicy::Scope#resolve. So I need to create a scope resolve method for the relationship. Is this the inteded behaviour?

Yes, you should create a scope resolve method for the relationship. I think it should've errored out if you didn't have one defined? Or maybe you have some base class that defines an "all results are OK" scope resolver?

[Matthijsy]

Yes we have an base Scope which returns all the records. It would be great if for related resources it will call the related record policy index? as well (as you mentioned 2 comments above). For now I will work with the Scope.resolve

[valscion]

Yeah sounds like a plan 😊