MDEV-13655: Set role does not properly grant privileges.

When granting a role to another role, DB privileges get propagated. If the grantee had no previous DB privileges, an extra ACL_DB entry is created to house those "indirectly received" privileges. If, afterwards, DB privileges are granted to the grantee directly, we must make sure to not create a duplicate ACL_DB entry.

Author: cvicentiu

mysql-test/suite/roles/set_role-13655.result

@@ -0,0 +1,50 @@
+#
+# MDEV-13655: SET ROLE does not properly grant privileges.
+#
+# We must test that if aditional db privileges get granted to a role
+# which previously inherited privileges from another granted role
+# keep the internal memory structures intact.
+#
+create role simple;
+#
+# First we create an entry with privileges for databases for the simple role.
+#
+grant select, insert, update, delete, lock tables, execute on t.* to simple;
+create role admin;
+#
+# Now we grant the simple role to admin. This means that db privileges
+# should propagate to admin.
+#
+grant simple to admin;
+show grants for admin;
+Grants for admin
+GRANT simple TO 'admin'
+GRANT USAGE ON *.* TO 'admin'
+GRANT USAGE ON *.* TO 'simple'
+GRANT SELECT, INSERT, UPDATE, DELETE, LOCK TABLES, EXECUTE ON `t`.* TO 'simple'
+#
+# Finally, we give the admin all the available privileges for the db.
+#
+grant all on t.* to admin;
+#
+# Create a user to test out the new roles;
+#
+create user foo;
+grant admin to foo;
+create database t;
+ERROR 42000: Access denied for user 'foo'@'%' to database 't'
+set role admin;
+show grants;
+Grants for foo@%
+GRANT admin TO 'foo'@'%'
+GRANT USAGE ON *.* TO 'foo'@'%'
+GRANT simple TO 'admin'
+GRANT USAGE ON *.* TO 'admin'
+GRANT ALL PRIVILEGES ON `t`.* TO 'admin'
+GRANT USAGE ON *.* TO 'simple'
+GRANT SELECT, INSERT, UPDATE, DELETE, LOCK TABLES, EXECUTE ON `t`.* TO 'simple'
+create database t;
+drop database t;
+drop role simple;
+drop role admin;
+drop user foo;

mysql-test/suite/roles/set_role-13655.test

@@ -0,0 +1,49 @@
+source include/not_embedded.inc;
+
+--echo #
+--echo # MDEV-13655: SET ROLE does not properly grant privileges.
+--echo #
+--echo # We must test that if aditional db privileges get granted to a role
+--echo # which previously inherited privileges from another granted role
+--echo # keep the internal memory structures intact.
+--echo #
+
+create role simple;
+
+--echo #
+--echo # First we create an entry with privileges for databases for the simple role.
+--echo #
+grant select, insert, update, delete, lock tables, execute on t.* to simple;
+create role admin;
+
+--echo #
+--echo # Now we grant the simple role to admin. This means that db privileges
+--echo # should propagate to admin.
+--echo #
+grant simple to admin;
+show grants for admin;
+
+--echo #
+--echo # Finally, we give the admin all the available privileges for the db.
+--echo #
+grant all on t.* to admin;
+
+--echo #
+--echo # Create a user to test out the new roles;
+--echo #
+create user foo;
+grant admin to foo;
+
+connect (foo,localhost,foo,,,,,);
+--error ER_DBACCESS_DENIED_ERROR
+create database t;
+set role admin;
+show grants;
+create database t;
+drop database t;
+
+connection default;
+
+drop role simple;
+drop role admin;
+drop user foo;

sql/sql_acl.cc

@@ -2129,37 +2129,42 @@ static void acl_insert_user(const char *user, const char *host,
 }
 
 
-static void acl_update_db(const char *user, const char *host, const char *db,
+static bool acl_update_db(const char *user, const char *host, const char *db,
  ulong privileges)
 {
  mysql_mutex_assert_owner(&acl_cache->lock);
 
+ bool updated= false;
+
  for (uint i=0 ; i < acl_dbs.elements ; i++)
  {
  ACL_DB *acl_db=dynamic_element(&acl_dbs,i,ACL_DB*);
  if ((!acl_db->user && !user[0]) ||
-(acl_db->user &&
-!strcmp(user,acl_db->user)))
+ (acl_db->user &&
+ !strcmp(user,acl_db->user)))
  {
  if ((!acl_db->host.hostname && !host[0]) ||
- (acl_db->host.hostname &&
- !strcmp(host, acl_db->host.hostname)))
+  (acl_db->host.hostname &&
+  !strcmp(host, acl_db->host.hostname)))
  {
-if ((!acl_db->db && !db[0]) ||
- (acl_db->db && !strcmp(db,acl_db->db)))
+ if ((!acl_db->db && !db[0]) ||
+  (acl_db->db && !strcmp(db,acl_db->db)))
 
-{
- if (privileges)
+ {
+  if (privileges)
  {
  acl_db->access= privileges;
  acl_db->initial_access= acl_db->access;
  }
- else
- delete_dynamic_element(&acl_dbs,i);
-}
+ else
+ delete_dynamic_element(&acl_dbs,i);
+ updated= true;
+ }
  }
  }
  }
+
+ return updated;
 }
 
 
@@ -3428,9 +3433,21 @@ static int replace_db_table(TABLE *table, const char *db,
  acl_cache->clear(1);// Clear privilege cache
  if (old_row_exists)
  acl_update_db(combo.user.str,combo.host.str,db,rights);
- else
- if (rights)
- acl_insert_db(combo.user.str,combo.host.str,db,rights);
+ else if (rights)
+ {
+ /*
+ If we did not have an already existing row, for users, we must always
+ insert an ACL_DB entry. For roles however, it is possible that one was
+ already created when DB privileges were propagated from other granted
+ roles onto the current role. For this case, first try to update the
+ existing entry, otherwise insert a new one.
+ */
+ if (!combo.is_role() ||
+ !acl_update_db(combo.user.str, combo.host.str, db, rights))
+ {
+ acl_insert_db(combo.user.str,combo.host.str,db,rights);
+ }
+ }
  DBUG_RETURN(0);
 
  /* This could only happen if the grant tables got corrupted */

sql/structs.h

@@ -191,7 +191,7 @@ typedef int *(*update_var)(THD *, struct st_mysql_show_var *);
 
 typedef structst_lex_user {
  LEX_STRING user, host, password, plugin, auth;
- bool is_role() { return user.str[0] && !host.str[0]; }
+ bool is_role() const { return user.str[0] && !host.str[0]; }
  void set_lex_string(LEX_STRING *l, char *buf)
  {
  if (is_role())