togglepro / pundit-resources

Integrate Pundit policies with jsonapi-resources
MIT License
46 stars 38 forks source link

Raise more helpful error when policy scope does not match resource scope #37

Open vestedpr-dev opened 5 years ago

vestedpr-dev commented 5 years ago

Thank you for PunditResources, we are using it heavily and it mostly works great!

When using PunditResources, one of the dangers is that the resource scope, does not match the policy scope. JSONAPI::Resources is quite a complicated beast but, what seems to happen is that, it gets the list of resources to be serialized from one place, but when it actually retrieves the resources to be serialized into the response, then it calls through the Pundit Scope (e.g. the index). (Anyone who knows, please correct me with a more sophisticated account of what JSONAPI::Resources is doing!)

When this problem occurs it throws an obscure exception:

NoMethodError (undefined method `_type' for NilClass:Class):

jsonapi-resources (0.9.10) lib/jsonapi/resource_serializer.rb:514:in `add_resource'
jsonapi-resources (0.9.10) lib/jsonapi/resource_serializer.rb:349:in `block (2 levels) in cached_relationships_hash'
jsonapi-resources (0.9.10) lib/jsonapi/resource_serializer.rb:348:in `each'
jsonapi-resources (0.9.10) lib/jsonapi/resource_serializer.rb:348:in `block in cached_relationships_hash'
jsonapi-resources (0.9.10) lib/jsonapi/resource_serializer.rb:328:in `each'
jsonapi-resources (0.9.10) lib/jsonapi/resource_serializer.rb:328:in `cached_relationships_hash'
jsonapi-resources (0.9.10) lib/jsonapi/resource_serializer.rb:152:in `object_hash'
jsonapi-resources (0.9.10) lib/jsonapi/resource_serializer.rb:521:in `add_resource'

This exception is so unrelated to the actual cause of the problem, it took me a long time to understand what was wrong. The root of this exception, I believe, is that the Pundit policy scope, when viewed by a particular user, does not match the underlying rails association.

Anyways I created this monkeypatch for our codebase so that in the future it will be more clear what's happening:

config/initializers/monkeypatch_jsonapi_resources_add_pundit_resources_exception.rb

module JSONAPI
  class ResourceSerializer

    def add_resource(source, include_directives, primary = false)

      # Begin modification
      begin
        type = source.is_a?(JSONAPI::CachedResourceFragment) ? source.type : source.class._type
      rescue NoMethodError
        raise "PunditResources error: authorization scope does not match association scope!"
      end
      # End modification

      id = source.id

      @included_objects[type] ||= {}
      existing = @included_objects[type][id]

      if existing.nil?
        obj_hash = object_hash(source, include_directives)
        @included_objects[type][id] = {
          primary: primary,
          object_hash: obj_hash,
          includes: Set.new(include_directives[:include_related].keys)
        }
      else
        include_related = Set.new(include_directives[:include_related].keys)
        unless existing[:includes].superset?(include_related)
          obj_hash = object_hash(source, include_directives)
          @included_objects[type][id][:object_hash].deep_merge!(obj_hash)
          @included_objects[type][id][:includes].add(include_related)
          @included_objects[type][id][:primary] = existing[:primary] | primary
        end
      end
    end
  end
end
vestedpr-dev commented 5 years ago

Related: "Fix for has_one relationships where related record does not exist" https://github.com/cerebris/jsonapi-resources/pull/987

Note also, this exception is when you have caching enabled for the resource. Without caching you still get an exception, of a different (less confusing but, still confusing) form:

KeyError (key not found: 2329):

jsonapi-resources (0.9.10) lib/jsonapi/resource.rb:1354:in `fetch'
jsonapi-resources (0.9.10) lib/jsonapi/resource.rb:1354:in `block (3 levels) in preload_included_fragments'
jsonapi-resources (0.9.10) lib/jsonapi/resource.rb:1349:in `each'
jsonapi-resources (0.9.10) lib/jsonapi/resource.rb:1349:in `each_with_index'
jsonapi-resources (0.9.10) lib/jsonapi/resource.rb:1349:in `block (2 levels) in preload_included_fragments'
jsonapi-resources (0.9.10) lib/jsonapi/resource.rb:1347:in `each'
jsonapi-resources (0.9.10) lib/jsonapi/resource.rb:1347:in `block in preload_included_fragments'
jsonapi-resources (0.9.10) lib/jsonapi/resource.rb:1259:in `each'