MDEV-14443: SHOW command denies take effect for dropped users

3 files changed, 208 insertions, 10 deletions

diff --git a/mysql-test/suite/deny/show_generic.result b/mysql-test/suite/deny/show_generic.result
new file mode 100644
index 00000000000..261170a9b27
--- /dev/null
+++ b/mysql-test/suite/deny/show_generic.result
@@ -0,0 +1,93 @@
+#
+# This test checks that denies take part in SHOW commands properly.
+#
+create user foo;
+create database some_db;
+create table some_db.t1 (id int);
+connect  con1,localhost,foo,,;
+show tables from some_db;
+ERROR 42000: Access denied for user 'foo'@'%' to database 'some_db'
+disconnect con1;
+connection default;
+grant insert on some_db.t1 to foo;
+connect  con1,localhost,foo,,;
+show tables from some_db;
+Tables_in_some_db
+t1
+disconnect con1;
+connection default;
+deny insert on some_db.* to foo;
+connect  con1,localhost,foo,,;
+show databases;
+Database
+information_schema
+test
+show tables from some_db;
+ERROR 42000: Access denied for user 'foo'@'%' to database 'some_db'
+disconnect con1;
+connection default;
+#
+# Test that dropping a user does not give access to connections that
+# previously had elements denied.
+#
+# We will use two users, foo which will receive denies, bar which will
+# not. This will showcase the difference in behaviour.
+#
+create user bar;
+#
+# This global grant is cached in sctx->master_access on connection.
+#
+grant select on *.* to foo;
+grant select on *.* to bar;
+connect  con1,localhost,foo,,;
+show tables from some_db;
+Tables_in_some_db
+t1
+disconnect con1;
+connection default;
+#
+# Here we add a mask for some_db, it should no longer be visible to foo.
+#
+deny select on some_db.* to foo;
+connect  con1,localhost,foo,,;
+use some_db;
+ERROR 42000: Access denied for user 'foo'@'%' to database 'some_db'
+show tables from some_db;
+ERROR 42000: Access denied for user 'foo'@'%' to database 'some_db'
+#
+# bar user has access to the database (no denies present)
+#
+connect  con2,localhost,bar,,;
+use some_db;
+show tables from some_db;
+Tables_in_some_db
+t1
+#
+# Do not disconnect foo & bar, but drop the users.
+#
+connection default;
+drop user foo;
+drop user bar;
+#
+# Our current running connection, because it featured denies previously
+# now doesn't have access.
+#
+connection con1;
+use some_db;
+ERROR 42000: Access denied for user 'foo'@'%' to database 'some_db'
+show tables from some_db;
+ERROR 42000: Access denied for user 'foo'@'%' to database 'some_db'
+disconnect con1;
+#
+# However, bar does not have any denies active. In this case we can keep
+# the previous MariaDB behaviour of using the "cache" within
+# sctx->master_access to grant access.
+#
+connection con2;
+use some_db;
+show tables from some_db;
+Tables_in_some_db
+t1
+disconnect con2;
+connection default;
+drop database some_db;
diff --git a/mysql-test/suite/deny/show_generic.test b/mysql-test/suite/deny/show_generic.test
new file mode 100644
index 00000000000..8ca36a9bab8
--- /dev/null
+++ b/mysql-test/suite/deny/show_generic.test
@@ -0,0 +1,102 @@
+--source include/not_embedded.inc
+
+--echo #
+--echo # This test checks that denies take part in SHOW commands properly.
+--echo #
+create user foo;
+
+create database some_db;
+create table some_db.t1 (id int);
+
+
+--connect (con1,localhost,foo,,)
+--error ER_DBACCESS_DENIED_ERROR
+show tables from some_db;
+disconnect con1;
+connection default;
+
+grant insert on some_db.t1 to foo;
+--connect (con1,localhost,foo,,)
+show tables from some_db;
+disconnect con1;
+
+connection default;
+deny insert on some_db.* to foo;
+
+--connect (con1,localhost,foo,,)
+show databases;
+--error ER_DBACCESS_DENIED_ERROR
+show tables from some_db;
+disconnect con1;
+
+connection default;
+
+--echo #
+--echo # Test that dropping a user does not give access to connections that
+--echo # previously had elements denied.
+--echo #
+--echo # We will use two users, foo which will receive denies, bar which will
+--echo # not. This will showcase the difference in behaviour.
+--echo #
+create user bar;
+
+--echo #
+--echo # This global grant is cached in sctx->master_access on connection.
+--echo #
+grant select on *.* to foo;
+grant select on *.* to bar;
+
+--connect (con1,localhost,foo,,)
+show tables from some_db;
+disconnect con1;
+
+connection default;
+--echo #
+--echo # Here we add a mask for some_db, it should no longer be visible to foo.
+--echo #
+deny select on some_db.* to foo;
+
+--connect (con1,localhost,foo,,)
+--error ER_DBACCESS_DENIED_ERROR
+use some_db;
+--error ER_DBACCESS_DENIED_ERROR
+show tables from some_db;
+
+--echo #
+--echo # bar user has access to the database (no denies present)
+--echo #
+--connect (con2,localhost,bar,,)
+use some_db;
+show tables from some_db;
+
+--echo #
+--echo # Do not disconnect foo & bar, but drop the users.
+--echo #
+
+connection default;
+drop user foo;
+drop user bar;
+
+--echo #
+--echo # Our current running connection, because it featured denies previously
+--echo # now doesn't have access.
+--echo #
+connection con1;
+--error ER_DBACCESS_DENIED_ERROR
+use some_db;
+--error ER_DBACCESS_DENIED_ERROR
+show tables from some_db;
+disconnect con1;
+
+--echo #
+--echo # However, bar does not have any denies active. In this case we can keep
+--echo # the previous MariaDB behaviour of using the "cache" within
+--echo # sctx->master_access to grant access.
+--echo #
+connection con2;
+use some_db;
+show tables from some_db;
+disconnect con2;
+
+connection default;
+drop database some_db;
diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
index e61dd7db887..189c727be3c 100644
--- a/sql/sql_acl.cc
+++ b/sql/sql_acl.cc
@@ -3364,7 +3364,6 @@ check_access(THD *thd, privilege_t want_access,
     */
     const ACL_internal_schema_access *access;
     access= get_cached_schema_access(grant_internal_info, db);
-    /* TODO(cvicentiu) this needs to handle denies. */
     if (access)
     {
       switch (access->check(want_access, save_priv))
@@ -3451,7 +3450,6 @@ check_access(THD *thd, privilege_t want_access,
     Save the union of User-table and the intersection between Db-table and
     Host-table privileges, masked with the deny mask,
     with the already saved internal privileges.
-    TODO(cvicentiu) denies for internal privileges?
   */
   masked_access|= db_access & ~deny_mask;
   *save_priv|= masked_access;
@@ -3620,7 +3618,6 @@ static bool check_show_access(THD *thd, TABLE_LIST *table)
 
     privilege_t deny_mask= acl_get_effective_deny_mask(thd->security_ctx,
                                                        dst_db_name);
-    //TODO(cvicentiu) compute correct deny mask...
     if (!cur_access && check_grant_db(thd->security_ctx, dst_db_name.str,
                                       deny_mask))
     {
@@ -4802,16 +4799,22 @@ privilege_t acl_get_effective_deny_mask(const Security_context *ctx,
 
   mysql_mutex_lock(&acl_cache->lock);
   ACL_USER *acl_user= find_user_exact(ctx->priv_host, ctx->priv_user);
-  DBUG_ASSERT(acl_user); //TODO(cvicentiu) check this?
   if (!acl_user)
   {
-    // TODO(cvicentiu)
-    // Somehow the user was lost...
-    // This can probably happen when one connection exists and then
-    // the user is dropped, then the function is invoked.
-    DBUG_ASSERT(0);
+    /*
+       We can't find the user associated with the current security context.
+
+       This can happen when a user is dropped while a connection still exists
+       authenticated as that user.
+
+       There are two possibilities here: block all access or don't block any
+       access. Historical MariaDB behaviour meant that global & database
+       level grants are cached when the connection is made, preserving access.
+       However we do not cache denies in sctx, thus to ensure we do not leak any
+       data unintentionally, we assume the user has every privilege denied!
+    */
     mysql_mutex_unlock(&acl_cache->lock);
-    return NO_ACL;
+    return ALL_KNOWN_ACL;
   }
 
   result= acl_user->denies->get_global();