diff --git a/.rubocop.yml b/.rubocop.yml index 9171c586b9c6216a43633b46a26560a5e7424203..66db1f36d72f3f9a99b522f1b4bf0f2fe0074720 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -723,6 +723,12 @@ API/GrapeArrayMissingCoerce: - 'lib/**/api/**/*.rb' - 'ee/**/api/**/*.rb' +API/ClassLevelAllowAccessWithScope: + Enabled: true + Include: + - 'lib/**/api/**/*.rb' + - 'ee/lib/**/api/**/*.rb' + Cop/SidekiqOptionsQueue: Enabled: true Exclude: diff --git a/.rubocop_todo/api/class_level_allow_access_with_scope.yml b/.rubocop_todo/api/class_level_allow_access_with_scope.yml new file mode 100644 index 0000000000000000000000000000000000000000..9ebd0fb8c0cd41e882fc4139a7cc1f356d12453d --- /dev/null +++ b/.rubocop_todo/api/class_level_allow_access_with_scope.yml @@ -0,0 +1,3 @@ +--- +API/ClassLevelAllowAccessWithScope: + Details: grace period diff --git a/rubocop/cop/api/class_level_allow_access_with_scope.rb b/rubocop/cop/api/class_level_allow_access_with_scope.rb new file mode 100644 index 0000000000000000000000000000000000000000..ad045584e47f7fb85947691077d80458d195c2ea --- /dev/null +++ b/rubocop/cop/api/class_level_allow_access_with_scope.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require_relative '../../code_reuse_helpers' + +module RuboCop + module Cop + module API + class ClassLevelAllowAccessWithScope < RuboCop::Cop::Base + include CodeReuseHelpers + + # This cop checks that `allow_access_with_scope` is called only at the class level. + # This is because `allow_access_with_scope` aggregates scopes for each call in a class. + # Calling `allow_access_with_scope` within a `namespace` or an alias method such as + # `resource`, `resources`, `segment` or `group` may mislead developers to think the scope + # would be only allowed within given namespace which is not the case. + # + # @example + # + # # bad + # class MyClass < ::API::Base + # include APIGuard + # namespace 'my_namespace' do + # resource :my_resource do + # allow_access_with_scope :ai_workflows + # + # # good + # class MyClass < ::API::Base + # include APIGuard + # allow_access_with_scope :ai_workflows + # + MSG = '`allow_access_with_scope` should only be called on class-level and not within a namespace.' + + # In Grape::DSL::Routing::ClassMethods + # group, segment, resource, and resources are all aliased to namespace + BANNED_BLOCKS = %i[group namespace resource resources segment].freeze + + RESTRICT_ON_SEND = %i[allow_access_with_scope].freeze + def on_send(node) + return unless namespace?(node) + + add_offense(node) + end + + private + + def namespace?(node) + node.each_ancestor(:block).any? do |block_node| + BANNED_BLOCKS.include?(block_node.method_name) + end + end + end + end + end +end diff --git a/spec/rubocop/cop/api/class_level_allow_access_with_scope_spec.rb b/spec/rubocop/cop/api/class_level_allow_access_with_scope_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..30b124959c65c59b66ed5240087385697ad7918c --- /dev/null +++ b/spec/rubocop/cop/api/class_level_allow_access_with_scope_spec.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +require 'rubocop_spec_helper' +require_relative '../../../../rubocop/cop/api/class_level_allow_access_with_scope' + +RSpec.describe RuboCop::Cop::API::ClassLevelAllowAccessWithScope, feature_category: :shared do + let(:msg) { described_class::MSG } + + context "when there is no `allow_access_with_scope`" do + it "does not add an offense" do + expect_no_offenses(<<~CODE) + class MyClass < ::API::Base + include APIGuard + namespace 'my_namespace' do + + end + end + CODE + end + end + + context "when there is class level `allow_access_with_scope`" do + it "does not add an offense" do + expect_no_offenses(<<~CODE) + class MyClass < ::API::Base + include APIGuard + allow_access_with_scope :my_scope + namespace 'my_namespace' do + + end + end + CODE + end + end + + context "when there is `allow_access_with_scope` under namespace" do + it "adds an offense" do + expect_offense(<<~CODE) + class MyClass < ::API::Base + include APIGuard + namespace 'my_namespace' do + allow_access_with_scope :my_scope + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} + end + end + CODE + end + end + + context "when there is `allow_access_with_scope` under group" do + it "adds an offense" do + expect_offense(<<~CODE) + class MyClass < ::API::Base + include APIGuard + group 'my_namespace' do + allow_access_with_scope :my_scope + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} + end + end + CODE + end + end + + context "when there is `allow_access_with_scope` under resource" do + it "adds an offense" do + expect_offense(<<~CODE) + class MyClass < ::API::Base + include APIGuard + resource 'my_namespace' do + allow_access_with_scope :my_scope + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} + end + end + CODE + end + end + + context "when there is `allow_access_with_scope` under resources" do + it "adds an offense" do + expect_offense(<<~CODE) + class MyClass < ::API::Base + include APIGuard + resources 'my_namespace' do + allow_access_with_scope :my_scope + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} + end + end + CODE + end + end + + context "when there is `allow_access_with_scope` under segment" do + it "adds an offense" do + expect_offense(<<~CODE) + class MyClass < ::API::Base + include APIGuard + segment 'my_namespace' do + allow_access_with_scope :my_scope + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} + end + end + CODE + end + end + + context "when there are `allow_access_with_scope`s both class level and under namespace" do + it "adds an offense" do + expect_offense(<<~CODE) + class MyClass < ::API::Base + include APIGuard + allow_access_with_scope :my_scope + namespace 'my_namespace' do + allow_access_with_scope :my_scope + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg} + end + end + CODE + end + end +end