Table level denies, update callsites, fix bugs

3 files changed, 91 insertions, 22 deletions

diff --git a/mysql-test/suite/deny/dml.result b/mysql-test/suite/deny/dml.result
index 8df22d8abcc..c8c8beea3de 100644
--- a/mysql-test/suite/deny/dml.result
+++ b/mysql-test/suite/deny/dml.result
@@ -47,6 +47,10 @@ connect  con1,localhost,foo,,;
 # Test not allowed to delete / update because of lack of select
 # rights.
 #
+show grants;
+Grants for foo@%
+GRANT USAGE ON *.* TO `foo`@`%`
+GRANT SELECT (a), INSERT (a), UPDATE (a), DELETE ON `deny_db`.`t1` TO `foo`@`%`
 update deny_db.t1 set a=a*10 where a = 1;
 ERROR 42000: SELECT command denied to user 'foo'@'localhost' for column 'a' in table 't1'
 delete from deny_db.t1 where a = 20;
@@ -101,14 +105,9 @@ disconnect con1;
 # Test masking database & global level grants.
 #
 connection default;
-grant select on *.* to foo;
-grant insert on *.* to foo;
-grant update on *.* to foo;
-grant delete on *.* to foo;
-grant select on deny_db.* to foo;
-grant insert on deny_db.* to foo;
-grant update on deny_db.* to foo;
-grant delete on deny_db.* to foo;
+grant select, insert, update, delete on *.* to foo;
+grant select, insert, update, delete on deny_db.* to foo;
+grant select, insert, update, delete on deny_db.t1 to foo;
 deny select, insert on deny_db.* to foo;
 connect  con1,localhost,foo,,;
 insert into deny_db.t1 (a,b) values (1,10), (2, 20), (3, 30);
@@ -139,6 +138,38 @@ ERROR 42000: UPDATE command denied to user 'foo'@'localhost' for table 't1'
 delete from deny_db.t1 where a = 20;
 ERROR 42000: DELETE command denied to user 'foo'@'localhost' for table 't1'
 disconnect con1;
+#
+# Test revoking all denies.
+#
+connection default;
+revoke deny select, insert, update, delete on deny_db.* from foo;
+truncate deny_db.t1;
+connect  con1,localhost,foo,,;
+insert into deny_db.t1 (a,b) values (1,10), (2, 20), (3, 30);
+update deny_db.t1 set a=a*10;
+select * from deny_db.t1;
+a	b	secret
+10	10	NULL
+20	20	NULL
+30	30	NULL
+delete from deny_db.t1 where a = 20;
+disconnect con1;
+connection default;
+#
+# Test masking via table level denies
+#
+deny select on deny_db.t1 to foo;
+connect  con1,localhost,foo,,;
+insert into deny_db.t1 (a,b) values (1,10), (2, 20), (3, 30);
+update deny_db.t1 set a=a*10;
+ERROR 42000: SELECT command denied to user 'foo'@'localhost' for column 'a' in table 't1'
+update deny_db.t1 set a=a*10 where a = 10;
+ERROR 42000: SELECT command denied to user 'foo'@'localhost' for column 'a' in table 't1'
+delete from deny_db.t1 where a = 20;
+ERROR 42000: SELECT command denied to user 'foo'@'localhost' for column 'a' in table 't1'
+select * from deny_db.t1;
+ERROR 42000: SELECT command denied to user 'foo'@'localhost' for table 't1'
+disconnect con1;
 connection default;
 drop database deny_db;
 drop user foo;
diff --git a/mysql-test/suite/deny/dml.test b/mysql-test/suite/deny/dml.test
index 44af3552d23..edf43c04b8f 100644
--- a/mysql-test/suite/deny/dml.test
+++ b/mysql-test/suite/deny/dml.test
@@ -59,6 +59,7 @@ deny select on deny_db.* to foo;
 --echo # rights.
 --echo #
 
+show grants;
 --error ER_COLUMNACCESS_DENIED_ERROR
 update deny_db.t1 set a=a*10 where a = 1;
 
@@ -131,14 +132,9 @@ disconnect con1;
 --echo # Test masking database & global level grants.
 --echo #
 connection default;
-grant select on *.* to foo;
-grant insert on *.* to foo;
-grant update on *.* to foo;
-grant delete on *.* to foo;
-grant select on deny_db.* to foo;
-grant insert on deny_db.* to foo;
-grant update on deny_db.* to foo;
-grant delete on deny_db.* to foo;
+grant select, insert, update, delete on *.* to foo;
+grant select, insert, update, delete on deny_db.* to foo;
+grant select, insert, update, delete on deny_db.t1 to foo;
 deny select, insert on deny_db.* to foo;
 
 --connect (con1,localhost,foo,,)
@@ -181,6 +177,40 @@ update deny_db.t1 set a=a*10;
 delete from deny_db.t1 where a = 20;
 disconnect con1;
 
+--echo #
+--echo # Test revoking all denies.
+--echo #
+connection default;
+revoke deny select, insert, update, delete on deny_db.* from foo;
+truncate deny_db.t1;
+
+--connect (con1,localhost,foo,,)
+insert into deny_db.t1 (a,b) values (1,10), (2, 20), (3, 30);
+update deny_db.t1 set a=a*10;
+select * from deny_db.t1;
+delete from deny_db.t1 where a = 20;
+disconnect con1;
+
+connection default;
+
+--echo #
+--echo # Test masking via table level denies
+--echo #
+
+deny select on deny_db.t1 to foo;
+
+--connect (con1,localhost,foo,,)
+insert into deny_db.t1 (a,b) values (1,10), (2, 20), (3, 30);
+--error ER_COLUMNACCESS_DENIED_ERROR
+update deny_db.t1 set a=a*10;
+--error ER_COLUMNACCESS_DENIED_ERROR
+update deny_db.t1 set a=a*10 where a = 10;
+--error ER_COLUMNACCESS_DENIED_ERROR
+delete from deny_db.t1 where a = 20;
+--error ER_TABLEACCESS_DENIED_ERROR
+select * from deny_db.t1;
+disconnect con1;
+
 connection default;
 drop database deny_db;
 drop user foo;
diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
index 5afa23bcd58..ae3cfd5ddce 100644
--- a/sql/sql_acl.cc
+++ b/sql/sql_acl.cc
@@ -9698,6 +9698,18 @@ bool check_grant(THD *thd, privilege_t want_access, TABLE_LIST *tables,
                                                        t_ref->get_table_name());
 
     want_access&= ~(sctx->master_access & ~deny_mask);
+
+    /*
+       Make sure that whatever privileges were inherited from the global
+       level, that they are masked here properly.
+
+       TODO(cvicentiu) -> It would be better if check_grant didn't rely
+       on check_access being called with &table->grant.privilege as a
+       parameter, so we can avoid this hard-coupling between the 2 function
+       calls.
+    */
+    t_ref->grant.privilege&= ~deny_mask;
+
     if (!want_access)
       continue;                                 // ok
 
@@ -9720,12 +9732,8 @@ bool check_grant(THD *thd, privilege_t want_access, TABLE_LIST *tables,
       continue;
     }
 
-    // TODO(cvicentiu) table level denies need to be checked here.
-    // This can be achieved by computing the effective deny_mask for t_ref.
-    if (!(~(t_ref->grant.privilege & ~deny_mask) & want_access))
-    {
+    if (!(~t_ref->grant.privilege & want_access))
       continue;
-    }
 
     if (is_temporary_table(t_ref))
     {
@@ -9888,7 +9896,7 @@ bool check_grant_column(const Security_context *sctx,
   if (!want_access)
     DBUG_RETURN(0);				// Already checked
 
-  privilege_t deny_mask= acl_get_effective_deny_mask(sctx, db_name);
+  privilege_t deny_mask= acl_get_effective_deny_mask(sctx, db_name, table_name);
   if (want_access & deny_mask)
     goto err;