DmitryTsepelev / rubocop-graphql

Rubocop extension for enforcing graphql-ruby best practices
MIT License
221 stars 50 forks source link

Check a variety of base classes in the NotAuthorizedNodeType check #143

Closed patch0 closed 1 year ago

patch0 commented 1 year ago

Closes #142

If I add Types::BaseUserType to SafeBaseClasses I'd expect all of following to pass the cop:

  1. Absolute reference
module Types
  class UserType < ::Types::BaseUserType
    implements GraphQL::Types::Relay::Node

    field :uuid, ID, null: false
  end
end
  1. Relative reference to the "root" namespace
module Types
  class UserType < Types::BaseUserType
    implements GraphQL::Types::Relay::Node

    field :uuid, ID, null: false
  end
end
  1. Relative reference within the module
module Types
  class UserType < BaseUserType
    implements GraphQL::Types::Relay::Node

    field :uuid, ID, null: false
  end
end

The only one that currently passes the cop is case 2.

These are all valid ruby, but could lead to confusion/frustration when trying to sort out GraphQL authorization. I'd propose checking a list of possible parent classes in this instance.

In case 1 we can just check Types::BaseUserType is our SafeBaseClasses list.

In case 2 we need to check if either Types::BaseUserType or Types::Types::BaseUserType is in our list.

Finally in case 3 we need to check if either BaseUserType or Types::BaseUserType is in our list.

I don't think we can see from the AST which other classes have been defined in the code, so checking for the two possible permutations is required for this cop to be robust.

DmitryTsepelev commented 1 year ago

Hey hey, makes a lot of sense, thank you! Could you please at these rubocop offences and I'll merge it?

patch0 commented 1 year ago

Hey hey, makes a lot of sense, thank you! Could you please at these rubocop offences and I'll merge it?

Thanks! All done :)