From b2a615c3c60dd3315ecf33d5fdb61061c9d4003e Mon Sep 17 00:00:00 2001
From: Luke Duncalfe <lduncalfe@eml.cc>
Date: Fri, 21 Jun 2019 17:09:02 +1200
Subject: [PATCH] Sanity check for GraphQL authorized?

Raise an exception if a developer calls any of the GraphQL authorization
methods and a `authorize :permission` is missing from a mutation class.

Previously `authorized?` would return `true` in this situation, which
although technically is accurate is not what a developer is intending.
---
 .../graphql/authorize/authorize_resource.rb   |  6 +++
 .../authorize/authorize_resource_spec.rb      | 45 +++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/lib/gitlab/graphql/authorize/authorize_resource.rb b/lib/gitlab/graphql/authorize/authorize_resource.rb
index b367a97105c..c619464f69f 100644
--- a/lib/gitlab/graphql/authorize/authorize_resource.rb
+++ b/lib/gitlab/graphql/authorize/authorize_resource.rb
@@ -48,6 +48,12 @@ module Gitlab
         end
 
         def authorized?(object)
+          # Sanity check. We don't want to accidentally allow a developer to authorize
+          # without first adding permissions to authorize against
+          if self.class.required_permissions.empty?
+            raise Gitlab::Graphql::Errors::ArgumentError, "#{self.class.name} has no authorizations"
+          end
+
           self.class.required_permissions.all? do |ability|
             # The actions could be performed across multiple objects. In which
             # case the current user is common, and we could benefit from the
diff --git a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb
index 13cf52fd795..f9c4c619330 100644
--- a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb
+++ b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb
@@ -101,6 +101,51 @@ describe Gitlab::Graphql::Authorize::AuthorizeResource do
     end
   end
 
+  context 'when the class does not define authorize' do
+    let(:fake_class) do
+      Class.new do
+        include Gitlab::Graphql::Authorize::AuthorizeResource
+
+        attr_reader :user, :found_object
+
+        def initialize(user, found_object)
+          @user, @found_object = user, found_object
+        end
+
+        def find_object(*_args)
+          found_object
+        end
+
+        def current_user
+          user
+        end
+
+        def self.name
+          'TestClass'
+        end
+      end
+    end
+    let(:error) { /#{fake_class.name} has no authorizations/ }
+
+    describe '#authorized_find' do
+      it 'raises a comprehensive error message' do
+        expect { loading_resource.authorized_find }.to raise_error(error)
+      end
+    end
+
+    describe '#authorized_find!' do
+      it 'raises a comprehensive error message' do
+        expect { loading_resource.authorized_find! }.to raise_error(error)
+      end
+    end
+
+    describe '#authorized?' do
+      it 'raises a comprehensive error message' do
+        expect { loading_resource.authorized?(project) }.to raise_error(error)
+      end
+    end
+  end
+
   describe '#authorize' do
     it 'adds permissions from subclasses to those of superclasses when used on classes' do
       base_class = Class.new do