From 60526a52912be977f55b7165bcdce4bbac190927 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Mon, 6 Nov 2017 18:50:38 +0100
Subject: [PATCH] Fix TRIGGER checks for MySQL

This ensures we can check if the user has TRIGGER permissions without
querying restricted tables. Thanks to Steve Norman
(https://gitlab.com/stevenorman) for helping out with this merge
request.

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/38372
---
 .../unreleased/fix-mysql-grant-check.yml      |  5 ++++
 lib/gitlab/database/grant.rb                  | 30 ++++++++++++-------
 spec/lib/gitlab/database/grant_spec.rb        | 22 ++++----------
 3 files changed, 29 insertions(+), 28 deletions(-)
 create mode 100644 changelogs/unreleased/fix-mysql-grant-check.yml

diff --git a/changelogs/unreleased/fix-mysql-grant-check.yml b/changelogs/unreleased/fix-mysql-grant-check.yml
new file mode 100644
index 00000000000..a1c1aa67d79
--- /dev/null
+++ b/changelogs/unreleased/fix-mysql-grant-check.yml
@@ -0,0 +1,5 @@
+---
+title: Fix TRIGGER checks for MySQL
+merge_request:
+author:
+type: fixed
diff --git a/lib/gitlab/database/grant.rb b/lib/gitlab/database/grant.rb
index aee3981e79a..9f76967fc77 100644
--- a/lib/gitlab/database/grant.rb
+++ b/lib/gitlab/database/grant.rb
@@ -6,28 +6,36 @@ module Gitlab
         if Database.postgresql?
           'information_schema.role_table_grants'
         else
-          'mysql.user'
+          'information_schema.schema_privileges'
         end
 
-      def self.scope_to_current_user
-        if Database.postgresql?
-          where('grantee = user')
-        else
-          where("CONCAT(User, '@', Host) = current_user()")
-        end
-      end
-
       # Returns true if the current user can create and execute triggers on the
       # given table.
       def self.create_and_execute_trigger?(table)
         priv =
           if Database.postgresql?
             where(privilege_type: 'TRIGGER', table_name: table)
+              .where('grantee = user')
           else
-            where(Trigger_priv: 'Y')
+            queries = [
+              Grant.select(1)
+                .from('information_schema.user_privileges')
+                .where("PRIVILEGE_TYPE = 'SUPER'")
+                .where("GRANTEE = CONCAT('\\'', REPLACE(CURRENT_USER(), '@', '\\'@\\''), '\\'')"),
+
+              Grant.select(1)
+                .from('information_schema.schema_privileges')
+                .where("PRIVILEGE_TYPE = 'TRIGGER'")
+                .where('TABLE_SCHEMA = ?', Gitlab::Database.database_name)
+                .where("GRANTEE = CONCAT('\\'', REPLACE(CURRENT_USER(), '@', '\\'@\\''), '\\'')")
+            ]
+
+            union = SQL::Union.new(queries).to_sql
+
+            Grant.from("(#{union}) privs")
           end
 
-        priv.scope_to_current_user.any?
+        priv.any?
       end
     end
   end
diff --git a/spec/lib/gitlab/database/grant_spec.rb b/spec/lib/gitlab/database/grant_spec.rb
index 651da3e8476..5ebf3f399b6 100644
--- a/spec/lib/gitlab/database/grant_spec.rb
+++ b/spec/lib/gitlab/database/grant_spec.rb
@@ -1,16 +1,6 @@
 require 'spec_helper'
 
 describe Gitlab::Database::Grant do
-  describe '.scope_to_current_user' do
-    it 'scopes the relation to the current user' do
-      user = Gitlab::Database.username
-      column = Gitlab::Database.postgresql? ? :grantee : :User
-      names = described_class.scope_to_current_user.pluck(column).uniq
-
-      expect(names).to eq([user])
-    end
-  end
-
   describe '.create_and_execute_trigger' do
     it 'returns true when the user can create and execute a trigger' do
       # We assume the DB/user is set up correctly so that triggers can be
@@ -18,13 +8,11 @@ describe Gitlab::Database::Grant do
       expect(described_class.create_and_execute_trigger?('users')).to eq(true)
     end
 
-    it 'returns false when the user can not create and/or execute a trigger' do
-      allow(described_class).to receive(:scope_to_current_user)
-        .and_return(described_class.none)
-
-      result = described_class.create_and_execute_trigger?('kittens')
-
-      expect(result).to eq(false)
+    it 'returns false when the user can not create and/or execute a trigger', :postgresql do
+      # In case of MySQL the user may have SUPER permissions, making it
+      # impossible to have `false` returned when running tests; hence we only
+      # run these tests on PostgreSQL.
+      expect(described_class.create_and_execute_trigger?('foo')).to eq(false)
     end
   end
 end