From 0453d7dd82699f878e30bc6f4ea4531e70be5055 Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Wed, 31 Oct 2012 13:16:25 -0600 Subject: [PATCH 1/3] Failing test case. --- bin/pt-show-grants | 11 ++++++++-- t/pt-show-grants/basics.t | 25 ++++++++++++++++++---- t/pt-show-grants/samples/column-grants.sql | 18 ++++++++++++++++ t/pt-show-grants/samples/column-grants.txt | 1 + 4 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 t/pt-show-grants/samples/column-grants.sql create mode 100644 t/pt-show-grants/samples/column-grants.txt diff --git a/bin/pt-show-grants b/bin/pt-show-grants index 1e5dc110..4876b5b7 100755 --- a/bin/pt-show-grants +++ b/bin/pt-show-grants @@ -1636,10 +1636,16 @@ sub _d { # ########################################################################### package pt_show_grants; +use strict; +use warnings FATAL => 'all'; use English qw(-no_match_vars); - use constant PTDEBUG => $ENV{PTDEBUG} || 0; +use Data::Dumper; +$Data::Dumper::Quotekeys = 0; +$Data::Dumper::Indent = 1; +$Data::Dumper::Sortkeys = 1; + sub main { @ARGV = @_; # set global ARGV for this package @@ -1757,6 +1763,7 @@ sub main { PTDEBUG && _d($EVAL_ERROR); $exit_status = 1; } + PTDEBUG && _d('Grants:', Dumper(\@grants)); next unless @grants; if ( $o->get('separate') ) { # List each grant separately. @@ -1894,7 +1901,7 @@ pt-show-grants - Canonicalize and print MySQL grants so you can effectively repl =head1 SYNOPSIS -Usage: pt-show-grants [OPTION...] [DSN] +Usage: pt-show-grants [OPTIONS] [DSN] pt-show-grants shows grants (user privileges) from a MySQL server. diff --git a/t/pt-show-grants/basics.t b/t/pt-show-grants/basics.t index 351a07ce..86ab4281 100644 --- a/t/pt-show-grants/basics.t +++ b/t/pt-show-grants/basics.t @@ -22,9 +22,6 @@ my $dbh = $sb->get_dbh_for('master'); if ( !$dbh ) { plan skip_all => 'Cannot connect to sandbox master'; } -else { - plan tests => 11; -} $sb->wipe_clean($dbh); @@ -94,8 +91,28 @@ like( 'No output when all users skipped' ); +# ############################################################################# +# pt-show-grant doesn't support column-level grants +# https://bugs.launchpad.net/percona-toolkit/+bug/866075 +# ############################################################################# +$sb->load_file('master', 't/pt-show-grants/samples/column-grants.sql'); +diag(`/tmp/12345/use -u root -e "GRANT SELECT(DateCreated, PckPrice, PaymentStat, SANumber) ON test.t TO 'sally'\@'%'"`); +diag(`/tmp/12345/use -u root -e "GRANT SELECT(city_id), INSERT(city) ON sakila.city TO 'sally'\@'%'"`); + +ok( + no_diff( + sub { pt_show_grants::main('-F', $cnf, qw(--only sally)) }, + "t/pt-show-grants/samples/column-grants.txt", + stderr => 1, + ), + "Column-level grants (bug 866075)" +); + +diag(`/tmp/12345/use -u root -e "DROP USER 'sally'\@'%'"`); + # ############################################################################# # Done. # ############################################################################# +$sb->wipe_clean($dbh); ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); -exit; +done_testing; diff --git a/t/pt-show-grants/samples/column-grants.sql b/t/pt-show-grants/samples/column-grants.sql new file mode 100644 index 00000000..b805c298 --- /dev/null +++ b/t/pt-show-grants/samples/column-grants.sql @@ -0,0 +1,18 @@ +drop database if exists test; +create database test; +use test; + +CREATE TABLE t ( + `SOrNum` mediumint(9) unsigned NOT NULL auto_increment, + `SPNum` mediumint(9) unsigned NOT NULL, + `DateCreated` timestamp NOT NULL default CURRENT_TIMESTAMP, + `DateRelease` timestamp NOT NULL default '0000-00-00 00:00:00', + `ActualReleasedDate` timestamp NULL default NULL, + `PckPrice` decimal(10,2) NOT NULL default '0.00', + `Status` varchar(20) NOT NULL, + `PaymentStat` varchar(20) NOT NULL default 'Unpaid', + `CusCode` int(9) unsigned NOT NULL, + `SANumber` mediumint(9) unsigned NOT NULL default '0', + `SpecialInstruction` varchar(500) default NULL, + PRIMARY KEY (`SOrNum`) +) ENGINE=InnoDB; diff --git a/t/pt-show-grants/samples/column-grants.txt b/t/pt-show-grants/samples/column-grants.txt new file mode 100644 index 00000000..4ba28051 --- /dev/null +++ b/t/pt-show-grants/samples/column-grants.txt @@ -0,0 +1 @@ +-- From 0fd9239500faa87f1aa7ada8c3afee3e52360c7b Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Wed, 31 Oct 2012 14:20:43 -0600 Subject: [PATCH 2/3] Implement split_grants() instead of split(',', $grants) to handle spliting grants with column-level privs. --- bin/pt-show-grants | 46 +++++++++++++++++-- t/pt-show-grants/basics.t | 22 ++++++++- .../samples/column-grants-separate-revoke.txt | 10 ++++ .../samples/column-grants-separate.txt | 5 ++ t/pt-show-grants/samples/column-grants.txt | 5 +- 5 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 t/pt-show-grants/samples/column-grants-separate-revoke.txt create mode 100644 t/pt-show-grants/samples/column-grants-separate.txt diff --git a/bin/pt-show-grants b/bin/pt-show-grants index 4876b5b7..99f90d53 100755 --- a/bin/pt-show-grants +++ b/bin/pt-show-grants @@ -1766,11 +1766,14 @@ sub main { PTDEBUG && _d('Grants:', Dumper(\@grants)); next unless @grants; - if ( $o->get('separate') ) { # List each grant separately. + if ( $o->get('separate') ) { + # List each grant separately. @grants = map { my ( $grants, $on_what ) = $_ =~ m/GRANT (.*?) ON (.*)$/; - map { "GRANT $_ ON $on_what" } split(', ', $grants); + map { "GRANT $_ ON $on_what" } split_grants($grants); } @grants; + PTDEBUG && _d('Grants separated:', Dumper(\@grants)); + my $count; # If the row with IDENTIFIED BY has multiple grants, this will # create many such rows; strip it from all but the first. @@ -1782,12 +1785,15 @@ sub main { } $_; } @grants; + PTDEBUG && _d('Grants separated:', Dumper(\@grants)); } - else { # Sort the actual grants lexically within each row for consistency. + else { + # Sort the actual grants lexically within each row for consistency. @grants = map { - $_ =~ s/GRANT (.*?) ON (`|\*)/"GRANT " . join(', ', sort(split(', ', $1))) . " ON $2"/e; + $_ =~ s/GRANT (.*?) ON (`|\*)/"GRANT " . join(', ', sort(split_grants($1))) . " ON $2"/e; $_; } @grants; + PTDEBUG && _d('Grants grouped:', Dumper(\@grants)); } # Sort the grant rows for consistency too, but the one with the password @@ -1795,6 +1801,7 @@ sub main { @grants = sort { $b =~ m/IDENTIFIED BY/ <=> $a =~ m/IDENTIFIED BY/ || $a cmp $b } @grants; + PTDEBUG && _d('Grants sorted:', Dumper(\@grants)); # Print REVOKE statements. if ( $o->get('revoke') ) { @@ -1809,7 +1816,7 @@ sub main { my @result; if ( $o->get('separate') ) { @result = map { "REVOKE $_ ON $on_what FROM $user" } - split(', ', $grants); + split_grants($grants); } else { @result = "REVOKE $grants ON $on_what FROM $user"; @@ -1874,6 +1881,35 @@ sub parse_user { return ( $user, $host ); } +sub split_grants { + my ($grants) = @_; + return unless $grants; + my @grants; + if ( $grants =~ m/(?:INSERT|SELECT|UPDATE) \(/ ) { + PTDEBUG && _d('Splitting grants on keywords:', $grants); + @grants = map { + my $grant = $_; + $grant =~ s/^\s+//; + $grant =~ s/,\s*$//; + $grant; + } $grants =~ m/ + \G # Start matching after the previous match + \s? # Space after previous match's separating comma + (?: # Either match... + (?: (?:INSERT|SELECT|UPDATE)\s\(.+?\) ) # a column grant + | (?: [A-Z\s]+ ) # or a table grant + ) + ,? # Separted from the next grant, if any, by a comma + /xg; + } + else { + PTDEBUG && _d('Splitting grants on comma:', $grants); + @grants = split(', ', $grants); + } + PTDEBUG && _d('Grants split:', Dumper(\@grants)); + return @grants; +} + sub _d { my ($package, undef, $line) = caller 0; @_ = map { (my $temp = $_) =~ s/\n/\n# /g; $temp; } diff --git a/t/pt-show-grants/basics.t b/t/pt-show-grants/basics.t index 86ab4281..5c739c7e 100644 --- a/t/pt-show-grants/basics.t +++ b/t/pt-show-grants/basics.t @@ -101,13 +101,33 @@ diag(`/tmp/12345/use -u root -e "GRANT SELECT(city_id), INSERT(city) ON sakila.c ok( no_diff( - sub { pt_show_grants::main('-F', $cnf, qw(--only sally)) }, + sub { pt_show_grants::main('-F', $cnf, qw(--only sally --no-header)) }, "t/pt-show-grants/samples/column-grants.txt", stderr => 1, ), "Column-level grants (bug 866075)" ); +ok( + no_diff( + sub { pt_show_grants::main('-F', $cnf, qw(--only sally --no-header), + qw(--separate)) }, + "t/pt-show-grants/samples/column-grants-separate.txt", + stderr => 1, + ), + "Column-level grants --separate (bug 866075)" +); + +ok( + no_diff( + sub { pt_show_grants::main('-F', $cnf, qw(--only sally --no-header), + qw(--separate --revoke)) }, + "t/pt-show-grants/samples/column-grants-separate-revoke.txt", + stderr => 1, + ), + "Column-level grants --separate --revoke (bug 866075)" +); + diag(`/tmp/12345/use -u root -e "DROP USER 'sally'\@'%'"`); # ############################################################################# diff --git a/t/pt-show-grants/samples/column-grants-separate-revoke.txt b/t/pt-show-grants/samples/column-grants-separate-revoke.txt new file mode 100644 index 00000000..157350be --- /dev/null +++ b/t/pt-show-grants/samples/column-grants-separate-revoke.txt @@ -0,0 +1,10 @@ +-- Revoke statements for 'sally'@'%' +REVOKE INSERT (city) ON `sakila`.`city` FROM 'sally'@'%'; +REVOKE SELECT (SANumber, DateCreated, PaymentStat, PckPrice) ON `test`.`t` FROM 'sally'@'%'; +REVOKE SELECT (city_id) ON `sakila`.`city` FROM 'sally'@'%'; +REVOKE USAGE ON *.* FROM 'sally'@'%'; +-- Grants for 'sally'@'%' +GRANT INSERT (city) ON `sakila`.`city` TO 'sally'@'%'; +GRANT SELECT (SANumber, DateCreated, PaymentStat, PckPrice) ON `test`.`t` TO 'sally'@'%'; +GRANT SELECT (city_id) ON `sakila`.`city` TO 'sally'@'%'; +GRANT USAGE ON *.* TO 'sally'@'%'; diff --git a/t/pt-show-grants/samples/column-grants-separate.txt b/t/pt-show-grants/samples/column-grants-separate.txt new file mode 100644 index 00000000..9b6361d9 --- /dev/null +++ b/t/pt-show-grants/samples/column-grants-separate.txt @@ -0,0 +1,5 @@ +-- Grants for 'sally'@'%' +GRANT INSERT (city) ON `sakila`.`city` TO 'sally'@'%'; +GRANT SELECT (SANumber, DateCreated, PaymentStat, PckPrice) ON `test`.`t` TO 'sally'@'%'; +GRANT SELECT (city_id) ON `sakila`.`city` TO 'sally'@'%'; +GRANT USAGE ON *.* TO 'sally'@'%'; diff --git a/t/pt-show-grants/samples/column-grants.txt b/t/pt-show-grants/samples/column-grants.txt index 4ba28051..54feb4a5 100644 --- a/t/pt-show-grants/samples/column-grants.txt +++ b/t/pt-show-grants/samples/column-grants.txt @@ -1 +1,4 @@ --- +-- Grants for 'sally'@'%' +GRANT INSERT (city), SELECT (city_id) ON `sakila`.`city` TO 'sally'@'%'; +GRANT SELECT (SANumber, DateCreated, PaymentStat, PckPrice) ON `test`.`t` TO 'sally'@'%'; +GRANT USAGE ON *.* TO 'sally'@'%'; From a5a23665feea30e672fcc63d530c3d95dbd65bd0 Mon Sep 17 00:00:00 2001 From: Brian Fraser Date: Thu, 1 Nov 2012 06:15:54 -0300 Subject: [PATCH 3/3] Simplified split_grants() and added a test for mixed table & column grants on the same table --- bin/pt-show-grants | 20 ++++++++----------- t/pt-show-grants/basics.t | 11 ++++++++++ .../samples/column-grants-combined.txt | 4 ++++ 3 files changed, 23 insertions(+), 12 deletions(-) create mode 100644 t/pt-show-grants/samples/column-grants-combined.txt diff --git a/bin/pt-show-grants b/bin/pt-show-grants index 99f90d53..d73a6157 100755 --- a/bin/pt-show-grants +++ b/bin/pt-show-grants @@ -1887,19 +1887,15 @@ sub split_grants { my @grants; if ( $grants =~ m/(?:INSERT|SELECT|UPDATE) \(/ ) { PTDEBUG && _d('Splitting grants on keywords:', $grants); - @grants = map { - my $grant = $_; - $grant =~ s/^\s+//; - $grant =~ s/,\s*$//; - $grant; - } $grants =~ m/ - \G # Start matching after the previous match - \s? # Space after previous match's separating comma - (?: # Either match... - (?: (?:INSERT|SELECT|UPDATE)\s\(.+?\) ) # a column grant - | (?: [A-Z\s]+ ) # or a table grant + # TODO: the following .+? might break (e.g. on `annoying)column`). + # Remember to update this whenever we switch to using + # a common SQL regex module + @grants = $grants =~ m/ + ( + (?:INSERT|SELECT|UPDATE)\s\(.+?\) # a column grants + | [A-Z\s]+ ) - ,? # Separted from the next grant, if any, by a comma + (?:,\s)? # Separted from the next grant, if any, by a comma /xg; } else { diff --git a/t/pt-show-grants/basics.t b/t/pt-show-grants/basics.t index 5c739c7e..d526ffea 100644 --- a/t/pt-show-grants/basics.t +++ b/t/pt-show-grants/basics.t @@ -128,6 +128,17 @@ ok( "Column-level grants --separate --revoke (bug 866075)" ); +diag(`/tmp/12345/use -u root -e "GRANT SELECT ON sakila.city TO 'sally'\@'%'"`); + +ok( + no_diff( + sub { pt_show_grants::main('-F', $cnf, qw(--only sally --no-header)) }, + "t/pt-show-grants/samples/column-grants-combined.txt", + stderr => 1, + ), + "Column-level grants combined with table-level grants on the same table (bug 866075)" +); + diag(`/tmp/12345/use -u root -e "DROP USER 'sally'\@'%'"`); # ############################################################################# diff --git a/t/pt-show-grants/samples/column-grants-combined.txt b/t/pt-show-grants/samples/column-grants-combined.txt new file mode 100644 index 00000000..fd679980 --- /dev/null +++ b/t/pt-show-grants/samples/column-grants-combined.txt @@ -0,0 +1,4 @@ +-- Grants for 'sally'@'%' +GRANT INSERT (city), SELECT, SELECT (city_id) ON `sakila`.`city` TO 'sally'@'%'; +GRANT SELECT (SANumber, DateCreated, PaymentStat, PckPrice) ON `test`.`t` TO 'sally'@'%'; +GRANT USAGE ON *.* TO 'sally'@'%';