From 58446f2133ce30468be42b4b63794419a0a5801c Mon Sep 17 00:00:00 2001
From: Nicolas Blanc <sinfomicien@gmail.com>
Date: Thu, 11 Apr 2019 23:27:40 +0200
Subject: [PATCH] Correct GRANTS management bugs #236 & #243

---
 CHANGELOG.md                                  |  2 ++
 resources/user.rb                             | 28 +++++++++++--------
 test/cookbooks/test/recipes/user_database.rb  | 11 ++++++--
 .../resources/controls/user_spec.rb           |  1 +
 4 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 88a8e682..7f3ec137 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
 - Write a new documentation following sous-chefs.org guidelines
 - undefined method `ext_conf_dir` when using mariadb 2.0.0 ([#225](https://github.com/sous-chefs/mariadb/issues/225)) 
 - Rename property `apt_repository` to `apt_repository_uri` in repository resource ([#245](https://github.com/sous-chefs/mariadb/issues/245))
+- Grant action should not require and modify the password ([#236](https://github.com/sous-chefs/mariadb/issues/236))
+- Grant fails if symbol privilege contains an underscore ([#243](https://github.com/sous-chefs/mariadb/issues/243))
 
 ### Removed
 
diff --git a/resources/user.rb b/resources/user.rb
index 7a695192..be41b6ba 100644
--- a/resources/user.rb
+++ b/resources/user.rb
@@ -36,7 +36,7 @@
   if current_resource.nil?
     converge_by "Creating user '#{new_resource.username}'@'#{new_resource.host}'" do
       create_sql = "CREATE USER '#{new_resource.username}'@'#{new_resource.host}'"
-      if new_resource.password
+      unless new_resource.password.nil?
         create_sql << ' IDENTIFIED BY '
         create_sql << if new_resource.password.is_a?(HashedPassword)
                         " PASSWORD '#{new_resource.password}'"
@@ -194,7 +194,7 @@ def desired_privs
     desired_privs
   end
 
-  def revokify_key(key)
+  def clean_grant_name(key)
     return '' if key.nil?
 
     # Some keys need to be translated as outlined by the table found here:
@@ -245,15 +245,19 @@ def revokify_key(key)
 
   # Repair
   if incorrect_privs
+    privileges_to_set = new_resource.privileges.map { |key| clean_grant_name(key) }
     converge_by "Granting privs for '#{new_resource.username}'@'#{new_resource.host}'" do
-      repair_sql = "GRANT #{new_resource.privileges.join(',')}"
+      repair_sql = "GRANT #{privileges_to_set.join(',')}"
       repair_sql << " ON #{db_name}.#{tbl_name}"
-      repair_sql << " TO '#{new_resource.username}'@'#{new_resource.host}' IDENTIFIED BY"
-      repair_sql << if new_resource.password.is_a?(HashedPassword)
-                      " PASSWORD '#{new_resource.password}'"
-                    else
-                      " '#{new_resource.password}'"
-                    end
+      repair_sql << " TO '#{new_resource.username}'@'#{new_resource.host}'"
+      unless new_resource.password.nil?
+        repair_sql << ' IDENTIFIED BY'
+        repair_sql << if new_resource.password.is_a?(HashedPassword)
+                        " PASSWORD '#{new_resource.password}'"
+                      else
+                        " '#{new_resource.password}'"
+                      end
+      end
       repair_sql << ' REQUIRE SSL' if new_resource.require_ssl
       repair_sql << ' REQUIRE X509' if new_resource.require_x509
       repair_sql << ' WITH GRANT OPTION' if new_resource.grant_option
@@ -263,9 +267,9 @@ def revokify_key(key)
       run_query(repair_sql)
       run_query('FLUSH PRIVILEGES')
     end
-  else
+  elsif !password_up_to_date && !new_resource.password.nil?
     # The grants are correct, but perhaps the password needs updating?
-    update_user_password unless password_up_to_date
+    update_user_password
   end
 end
 
@@ -286,7 +290,7 @@ def revokify_key(key)
     desired_privs.each do |p|
       key = p.to_s.capitalize.tr(' ', '_').gsub('Replication_', 'Repl_').gsub('Create_temporary_tables', 'Create_tmp_table').gsub('Show_databases', 'Show_db')
       key = "#{key}_priv"
-      privs_to_revoke << revokify_key(p) if r[key] != 'N'
+      privs_to_revoke << clean_grant_name(p) if r[key] != 'N'
     end
   end
 
diff --git a/test/cookbooks/test/recipes/user_database.rb b/test/cookbooks/test/recipes/user_database.rb
index a832c433..5010ae12 100644
--- a/test/cookbooks/test/recipes/user_database.rb
+++ b/test/cookbooks/test/recipes/user_database.rb
@@ -119,15 +119,22 @@
 end
 
 mariadb_user 'fozzie' do
-  database_name 'databass'
   password 'wokkawokka'
   host 'mars'
-  privileges [:select, :update, :insert]
+  privileges [:usage]
   require_ssl true
   ctrl_password 'gsql'
   action :grant
 end
 
+mariadb_user 'fozzie' do
+  database_name 'databass'
+  host 'mars'
+  privileges [:select, :update, :insert, :show_view]
+  ctrl_password 'gsql'
+  action :grant
+end
+
 hash2 = hashed_password('*F798E7C0681068BAE3242AA2297D2360DBBDA62B'); # 'zokkazokka'
 
 mariadb_user 'moozie' do
diff --git a/test/integration/resources/controls/user_spec.rb b/test/integration/resources/controls/user_spec.rb
index 08449a42..489abd75 100644
--- a/test/integration/resources/controls/user_spec.rb
+++ b/test/integration/resources/controls/user_spec.rb
@@ -11,6 +11,7 @@
 
   describe sql.query("show grants for 'fozzie'@'mars'") do
     its(:stdout) { should include '*EF112B3D562CB63EA3275593C10501B59C4A390D' }
+    its(:stdout) { should include 'SHOW VIEW' }
   end
 
   describe sql.query('show grants for  \'moozie\'@\'127.0.0.1\'') do