From ac2d08212b20bae20c85ea225ec3fb0052a9e1af Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Tue, 18 Jun 2019 16:18:26 +0200 Subject: [PATCH] Add a cop to ensure we authorize GraphQL types --- rubocop/cop/graphql/authorize_types.rb | 61 +++++++++++++++++ rubocop/rubocop.rb | 1 + .../cop/graphql/authorize_types_spec.rb | 66 +++++++++++++++++++ 3 files changed, 128 insertions(+) create mode 100644 rubocop/cop/graphql/authorize_types.rb create mode 100644 spec/rubocop/cop/graphql/authorize_types_spec.rb diff --git a/rubocop/cop/graphql/authorize_types.rb b/rubocop/cop/graphql/authorize_types.rb new file mode 100644 index 00000000000..93fe80c3edf --- /dev/null +++ b/rubocop/cop/graphql/authorize_types.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require_relative '../../spec_helpers' + +module RuboCop + module Cop + module Graphql + class AuthorizeTypes < RuboCop::Cop::Cop + include SpecHelpers + + MSG = 'Add an `authorize :ability` call to the type: '\ + 'https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#type-authorization' + + TYPES_DIR = 'app/graphql/types' + + # We want to exclude our own basetypes and scalars + WHITELISTED_TYPES = %w[BaseEnum BaseScalar BasePermissionType MutationType + QueryType GraphQL::Schema BaseUnion].freeze + + def_node_search :authorize?, <<~PATTERN + (send nil? :authorize ...) + PATTERN + + def on_class(node) + return unless in_type?(node) + return if whitelisted?(class_constant(node)) + return if whitelisted?(superclass_constant(node)) + + add_offense(node, location: :expression) unless authorize?(node) + end + + private + + def in_type?(node) + return if in_spec?(node) + + path = node.location.expression.source_buffer.name + + path.include?(TYPES_DIR) + end + + def whitelisted?(class_node) + return false unless class_node&.const_name + + WHITELISTED_TYPES.any? { |whitelisted| class_node.const_name.include?(whitelisted) } + end + + def class_constant(node) + node.descendants.first + end + + def superclass_constant(class_node) + # First one is the class name itself, second is it's superclass + _class_constant, *others = class_node.descendants + + others.find { |node| node.const_type? && node&.const_name != 'Types' } + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index e2a19978839..27c63d92ae5 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -43,3 +43,4 @@ require_relative 'cop/code_reuse/serializer' require_relative 'cop/code_reuse/active_record' require_relative 'cop/group_public_or_visible_to_user' require_relative 'cop/inject_enterprise_edition_module' +require_relative 'cop/graphql/authorize_types' diff --git a/spec/rubocop/cop/graphql/authorize_types_spec.rb b/spec/rubocop/cop/graphql/authorize_types_spec.rb new file mode 100644 index 00000000000..eae3e176d64 --- /dev/null +++ b/spec/rubocop/cop/graphql/authorize_types_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/graphql/authorize_types' + +describe RuboCop::Cop::Graphql::AuthorizeTypes do + include RuboCop::RSpec::ExpectOffense + include CopHelper + + subject(:cop) { described_class.new } + + context 'when in a type folder' do + before do + allow(cop).to receive(:in_type?).and_return(true) + end + + it 'adds an offense when there is no authorize call' do + inspect_source(<<~TYPE) + module Types + class AType < BaseObject + field :a_thing + field :another_thing + end + end + TYPE + + expect(cop.offenses.size).to eq 1 + end + + it 'does not add an offense for classes that have an authorize call' do + expect_no_offenses(<<~TYPE.strip) + module Types + class AType < BaseObject + graphql_name 'ATypeName' + + authorize :an_ability, :second_ability + + field :a_thing + end + end + TYPE + end + + it 'does not add an offense for classes that only have an authorize call' do + expect_no_offenses(<<~TYPE.strip) + module Types + class AType < SuperClassWithFields + authorize :an_ability + end + end + TYPE + end + + it 'does not add an offense for base types' do + expect_no_offenses(<<~TYPE) + module Types + class AType < BaseEnum + field :a_thing + end + end + TYPE + end + end +end