Skip to content

(#4534) Add PROXY grant support to mysql_grant #934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions lib/puppet/provider/mysql_grant/mysql.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

def self.instances
instances = []
users.select{ |user| user =~ /.+@/ }.collect do |user|
users.collect do |user|
user_string = self.cmd_user(user)
query = "SHOW GRANTS FOR #{user_string};"
begin
Expand Down Expand Up @@ -46,7 +46,7 @@ def self.instances
end
end
# Same here, but to remove OPTION leaving just GRANT.
if rest.match(/WITH\sGRANT\sOPTION/)
if rest.match(/WITH\sGRANT\sOPTION/)
options = ['GRANT']
else
options = ['NONE']
Expand Down Expand Up @@ -80,7 +80,7 @@ def self.prefetch(resources)
def grant(user, table, privileges, options)
user_string = self.class.cmd_user(user)
priv_string = self.class.cmd_privs(privileges)
table_string = self.class.cmd_table(table)
table_string = privileges.include?('PROXY') ? self.class.cmd_user(table) : self.class.cmd_table(table)
query = "GRANT #{priv_string}"
query << " ON #{table_string}"
query << " TO #{user_string}"
Expand All @@ -102,14 +102,14 @@ def create

def revoke(user, table, revoke_privileges = ['ALL'])
user_string = self.class.cmd_user(user)
table_string = self.class.cmd_table(table)
table_string = revoke_privileges.include?('PROXY') ? self.class.cmd_user(table) : self.class.cmd_table(table)
priv_string = self.class.cmd_privs(revoke_privileges)
# revoke grant option needs to be a extra query, because
# "REVOKE ALL PRIVILEGES, GRANT OPTION [..]" is only valid mysql syntax
# if no ON clause is used.
# It hast to be executed before "REVOKE ALL [..]" since a GRANT has to
# exist to be executed successfully
if revoke_privileges.include? 'ALL'
if revoke_privileges.include? 'ALL' and !revoke_privileges.include?('PROXY')
query = "REVOKE GRANT OPTION ON #{table_string} FROM #{user_string}"
mysql([defaults_file, system_database, '-e', query].compact)
end
Expand All @@ -121,7 +121,11 @@ def destroy
# if the user was dropped, it'll have been removed from the user hash
# as the grants are alraedy removed by the DROP statement
if self.class.users.include? @property_hash[:user]
revoke(@property_hash[:user], @property_hash[:table])
if @property_hash[:privileges].include?('PROXY')
revoke(@property_hash[:user], @property_hash[:table], @property_hash[:privileges])
else
revoke(@property_hash[:user], @property_hash[:table])
end
end
@property_hash.clear

Expand Down
9 changes: 8 additions & 1 deletion lib/puppet/type/mysql_grant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def initialize(*args)

validate do
fail('privileges parameter is required.') if self[:ensure] == :present and self[:privileges].nil?
fail('PROXY must be the only privilege specified.') if Array(self[:privileges]).count > 1 and Array(self[:privileges]).include?('PROXY')
fail('table parameter is required.') if self[:ensure] == :present and self[:table].nil?
fail('user parameter is required.') if self[:ensure] == :present and self[:user].nil?
fail('name must match user and table parameters') if self[:name] != "#{self[:user]}/#{self[:table]}"
Expand All @@ -51,11 +52,17 @@ def initialize(*args)
newproperty(:table) do
desc 'Table to apply privileges to.'

validate do |value|
if Array(@resource[:privileges]).include?('PROXY') and !/^[0-9a-zA-Z$_]*@[\w%\.:\-\/]*$/.match(value)
raise(ArgumentError, '"table" for PROXY should be specified as proxy_user@proxy_host')
end
end

munge do |value|
value.delete("`")
end

newvalues(/.*\..*/,/@/)
newvalues(/.*\..*/,/^[0-9a-zA-Z$_]*@[\w%\.:\-\/]*$/)
end

newproperty(:user) do
Expand Down
100 changes: 99 additions & 1 deletion spec/acceptance/types/mysql_grant_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class { 'mysql::server':
describe 'missing privileges for user' do
it 'should fail' do
pp = <<-EOS
mysql_user { 'test1@tester':
mysql_user { 'test1@tester':
ensure => present,
}
mysql_grant { 'test1@tester/test.*':
Expand Down Expand Up @@ -443,6 +443,104 @@ class { 'mysql::server':
end
end

describe 'adding proxy privileges' do
it 'should work without errors' do
pp = <<-EOS
mysql_user { 'proxy1@tester':
ensure => present,
}
mysql_grant { 'proxy1@tester/proxy_user@proxy_host':
ensure => 'present',
table => 'proxy_user@proxy_host',
user => 'proxy1@tester',
privileges => ['PROXY'],
require => Mysql_user['proxy1@tester'],
}
EOS

apply_manifest(pp, :catch_failures => true)
end

it 'should find the user' do
shell("mysql -NBe \"SHOW GRANTS FOR proxy1@tester\"") do |r|
expect(r.stdout).to match(/GRANT PROXY ON 'proxy_user'@'proxy_host' TO 'proxy1'@'tester'/)
expect(r.stderr).to be_empty
end
end
end

describe 'removing proxy privileges' do
it 'should work without errors' do
pp = <<-EOS
mysql_user { 'proxy1@tester':
ensure => present,
}
mysql_grant { 'proxy1@tester/proxy_user@proxy_host':
ensure => 'absent',
table => 'proxy_user@proxy_host',
user => 'proxy1@tester',
privileges => ['PROXY'],
require => Mysql_user['proxy1@tester'],
}
EOS

apply_manifest(pp, :catch_failures => true)
end

it 'should find the user' do
shell("mysql -NBe \"SHOW GRANTS FOR proxy1@tester\"") do |r|
expect(r.stdout).to_not match(/GRANT PROXY ON 'proxy_user'@'proxy_host' TO 'proxy1'@'tester'/)
expect(r.stderr).to be_empty
end
end
end

describe 'adding proxy privileges with other privileges' do
it 'should fail' do
pp = <<-EOS
mysql_user { 'proxy2@tester':
ensure => present,
}
mysql_grant { 'proxy2@tester/proxy_user@proxy_host':
ensure => 'present',
table => 'proxy_user@proxy_host',
user => 'proxy2@tester',
privileges => ['PROXY', 'SELECT'],
require => Mysql_user['proxy2@tester'],
}
EOS

expect(apply_manifest(pp, :expect_failures => true).stderr).to match(/PROXY must be the only privilege specified/)
end

it 'should not find the user' do
expect(shell("mysql -NBe \"SHOW GRANTS FOR proxy2@tester\"", { :acceptable_exit_codes => 1}).stderr).to match(/There is no such grant defined for user 'proxy2' on host 'tester'/)
end
end

describe 'adding proxy privileges with invalid proxy user' do
it 'should fail' do
pp = <<-EOS
mysql_user { 'proxy3@tester':
ensure => present,
}
mysql_grant { 'proxy3@tester/invalid_proxy_user':
ensure => 'present',
table => 'invalid_proxy_user',
user => 'proxy3@tester',
privileges => ['PROXY'],
require => Mysql_user['proxy3@tester'],
}
EOS

expect(apply_manifest(pp, :expect_failures => true).stderr).to match(/"table" for PROXY should be specified as proxy_user@proxy_host/)
end

it 'should not find the user' do
expect(shell("mysql -NBe \"SHOW GRANTS FOR proxy3@tester\"", { :acceptable_exit_codes => 1}).stderr).to match(/There is no such grant defined for user 'proxy3' on host 'tester'/)
end
end

describe 'grants with skip-name-resolve specified' do
it 'setup mysql::server' do
pp = <<-EOS
Expand Down
28 changes: 17 additions & 11 deletions spec/unit/puppet/type/mysql_grant_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,38 +3,44 @@
describe Puppet::Type.type(:mysql_grant) do

before :each do
@user = Puppet::Type.type(:mysql_grant).new(:name => 'foo@localhost/*.*', :privileges => ['ALL', 'PROXY'], :table => ['*.*','@'], :user => 'foo@localhost')
@user = Puppet::Type.type(:mysql_grant).new(:name => 'foo@localhost/*.*', :privileges => ['ALL'], :table => ['*.*'], :user => 'foo@localhost')
end

it 'should accept a grant name' do
expect(@user[:name]).to eq('foo@localhost/*.*')
end

it 'should accept ALL privileges' do
@user[:privileges] = 'ALL'
expect(@user[:privileges]).to eq(['ALL'])
end

it 'should accept PROXY privilege' do
@user[:privileges] = 'PROXY'
@user[:table] = 'proxy_user@proxy_host'
expect(@user[:privileges]).to eq(['PROXY'])
end

it 'should accept a table' do
@user[:table] = '*.*'
expect(@user[:table]).to eq('*.*')
end

it 'should accept @ for table' do
@user[:table] = '@'
expect(@user[:table]).to eq('@')
end


it 'should accept proxy user for table' do
@user[:table] = 'proxy_user@proxy_host'
expect(@user[:table]).to eq('proxy_user@proxy_host')
end

it 'should accept a user' do
@user[:user] = 'foo@localhost'
expect(@user[:user]).to eq('foo@localhost')
end

it 'should require a name' do
expect {
Puppet::Type.type(:mysql_grant).new({})
Expand All @@ -43,29 +49,29 @@

it 'should require the name to match the user and table' do
expect {
Puppet::Type.type(:mysql_grant).new(:name => 'foo', :privileges => ['ALL', 'PROXY'], :table => ['*.*','@'], :user => 'foo@localhost')
Puppet::Type.type(:mysql_grant).new(:name => 'foo', :privileges => ['ALL'], :table => ['*.*'], :user => 'foo@localhost')
}.to raise_error /name must match user and table parameters/
end

describe 'it should munge privileges' do

it 'to just ALL' do
@user = Puppet::Type.type(:mysql_grant).new(
:name => 'foo@localhost/*.*', :table => ['*.*','@'], :user => 'foo@localhost',
:privileges => ['ALL', 'PROXY'] )
:name => 'foo@localhost/*.*', :table => ['*.*'], :user => 'foo@localhost',
:privileges => ['ALL'] )
expect(@user[:privileges]).to eq(['ALL'])
end

it 'to upcase and ordered' do
@user = Puppet::Type.type(:mysql_grant).new(
:name => 'foo@localhost/*.*', :table => ['*.*','@'], :user => 'foo@localhost',
:name => 'foo@localhost/*.*', :table => ['*.*'], :user => 'foo@localhost',
:privileges => ['select', 'Insert'] )
expect(@user[:privileges]).to eq(['INSERT', 'SELECT'])
end

it 'ordered including column privileges' do
@user = Puppet::Type.type(:mysql_grant).new(
:name => 'foo@localhost/*.*', :table => ['*.*','@'], :user => 'foo@localhost',
:name => 'foo@localhost/*.*', :table => ['*.*'], :user => 'foo@localhost',
:privileges => ['SELECT(Host,Address)', 'Insert'] )
expect(@user[:privileges]).to eq(['INSERT', 'SELECT (Address, Host)'])
end
Expand Down