Added GRANT ROLE TO ... and REVOKE ROLE FROM ... functionality.

TODO:

Privilege checks are not done upon executing the command.

mysql-test/r/acl_roles_show_grants.result

@@ -31,8 +31,8 @@ select * from information_schema.applicable_roles;
 GRANTEE	ROLE_NAME	IS_GRANTABLE
 select * from information_schema.applicable_roles;
 GRANTEE	ROLE_NAME	IS_GRANTABLE
-test_user@localhost	test_role1	YES
 test_role1	test_role2	YES
+test_user@localhost	test_role1	YES
 test_user@localhost	test_role2	YES
 show grants;
 Grants for test_user@localhost
@@ -45,8 +45,8 @@ test_user@localhost	NULL
 set role test_role1;
 select * from information_schema.enabled_roles;
 ROLE_NAME
-test_role2
 test_role1
+test_role2
 select current_user(), current_role();
 current_user()	current_role()
 test_user@localhost	test_role1

mysql-test/t/acl_roles_show_grants.test

@@ -24,21 +24,25 @@ select user, host from mysql.db;
 grant select on mysql.* to test_role2;
 flush privileges;
 
+--sorted_result
 select * from information_schema.applicable_roles;
 
 change_user 'test_user';
 
+--sorted_result
 select * from information_schema.applicable_roles;
 
 --sorted_result
 show grants;
 select current_user(), current_role();
 set role test_role1;
+--sorted_result
 select * from information_schema.enabled_roles;
 select current_user(), current_role();
 --sorted_result
 show grants;
 set role none;
+--sorted_result
 select * from information_schema.enabled_roles;
 select current_user(), current_role();
 --sorted_result
@@ -60,6 +64,7 @@ show grants for CURRENT_ROLE;
 show grants for CURRENT_ROLE();
 
 set role test_role2;
+--sorted_result
 select * from information_schema.enabled_roles;
 select current_user(), current_role();
 --sorted_result

scripts/mysql_system_tables.sql

@@ -41,8 +41,7 @@ CREATE TABLE IF NOT EXISTS user (   Host char(60) binary DEFAULT '' NOT NULL, Us
 -- Remember for later if user table already existed
 set @had_user_table= @@warning_count != 0;
 
-CREATE TABLE IF NOT EXISTS roles_mapping (HostFk char(60) binary DEFAULT '' NOT NULL, UserFk char(16) binary DEFAULT '' NOT NULL, RoleFk char(16) binary DEFAULT '' NOT NULL);
+CREATE TABLE IF NOT EXISTS roles_mapping (HostFk char(60) binary DEFAULT '' NOT NULL, UserFk char(16) binary DEFAULT '' NOT NULL, RoleFk char(16) binary DEFAULT '' NOT NULL, unique index (HostFk, UserFk, RoleFk));
-
 
 CREATE TABLE IF NOT EXISTS func (  name char(64) binary DEFAULT '' NOT NULL, ret tinyint(1) DEFAULT '0' NOT NULL, dl char(128) DEFAULT '' NOT NULL, type enum ('function','aggregate') COLLATE utf8_general_ci NOT NULL, PRIMARY KEY (name) ) engine=MyISAM CHARACTER SET utf8 COLLATE utf8_bin   comment='User defined functions';
 

sql/sql_acl.cc

@@ -2741,6 +2741,62 @@ int add_role_user_mapping(ROLE_GRANT_PAIR *mapping)
   return result;
 }
 
+int remove_role_user_mapping(ROLE_GRANT_PAIR *mapping)
+{
+  ACL_USER_BASE *user= find_user_no_anon((mapping->u_hname) ? mapping->u_hname: "",
+                                    (mapping->u_uname) ? mapping->u_uname: "",
+                                    TRUE);
+  ACL_ROLE *role= find_acl_role(mapping->r_uname ? mapping->r_uname: "");
+
+
+  int result= 0;
+  uint idx_user, idx_role;
+  bool deleted_role= FALSE, deleted_user= FALSE;
+
+  if (user == NULL || role == NULL)
+  {
+    /* There still exists the possibility that the user is actually a role */
+    if (user == NULL && role && (!mapping->u_hname || !mapping->u_hname[0])
+        && /* in this case the grantee is a role */
+        ((user= find_acl_role(mapping->u_uname ? mapping->u_uname: ""))))
+    {
+      result= 1;
+    }
+    else
+    {
+      DBUG_PRINT("warning", ("Invalid remove_role_user_mapping '%s'@'%s' %s %p %p",
+                             mapping->u_uname, mapping->u_hname,
+                             mapping->r_uname, user, role));
+
+      return -1;
+    }
+  }
+
+  /* scan both arrays to find and delete both links */
+  for (idx_user=0; idx_user < user->role_grants.elements; idx_user++)
+  {
+    if (role == *dynamic_element(&user->role_grants, idx_user, ACL_ROLE**))
+    {
+      delete_dynamic_element(&user->role_grants, idx_user);
+      deleted_user= TRUE;
+    }
+  }
+
+  for (idx_role=0; idx_role < role->parent_grantee.elements; idx_role++)
+  {
+    if (user == *dynamic_element(&role->parent_grantee, idx_role,
+                                 ACL_USER_BASE**))
+    {
+      delete_dynamic_element(&role->parent_grantee, idx_role);
+      deleted_role= TRUE;
+    }
+  }
+
+  /* we should always get to delete from both arrays */
+  DBUG_ASSERT(deleted_role && deleted_user);
+  return result;
+}
+
 
 /*
   Rebuild the role grants every time the acl_users is modified
@@ -3695,6 +3751,60 @@ abort:
   DBUG_RETURN(-1);
 }
 
+static int
+replace_roles_mapping_table(TABLE *table, ROLE_GRANT_PAIR *pair,
+                            bool revoke_grant)
+{
+  DBUG_ENTER("replace_roles_mapping_table");
+
+  uchar row_key[MAX_KEY_LENGTH];
+  int error;
+  table->use_all_columns();
+  table->field[0]->store(pair->u_hname, strlen(pair->u_hname),
+                         system_charset_info);
+  table->field[1]->store(pair->u_uname, strlen(pair->u_uname),
+                         system_charset_info);
+  table->field[2]->store(pair->r_uname, strlen(pair->r_uname),
+                         system_charset_info);
+  key_copy(row_key, table->record[0], table->key_info,
+           table->key_info->key_length);
+
+  if (table->file->ha_index_read_idx_map(table->record[0], 0, row_key,
+                                         HA_WHOLE_KEY, HA_READ_KEY_EXACT))
+  {
+    /* No match */
+    if (revoke_grant)
+    {
+      DBUG_RETURN(1);
+    }
+  }
+  if (revoke_grant)
+  {
+    if ((error= table->file->ha_delete_row(table->record[0])))
+    {
+      DBUG_PRINT("info", ("error deleting row '%s' '%s' '%s'",
+                          pair->u_hname, pair->u_uname, pair->r_uname));
+      goto table_error;
+    }
+  }
+  else
+  {
+    if ((error= table->file->ha_write_row(table->record[0])))
+    {
+      DBUG_PRINT("info", ("error inserting row '%s' '%s' '%s'",
+                          pair->u_hname, pair->u_uname, pair->r_uname));
+      goto table_error;
+    }
+  }
+
+  /* all ok */
+  DBUG_RETURN(0);
+
+table_error:
+  DBUG_PRINT("info", ("table error"));
+  table->file->print_error(error, MYF(0));
+  DBUG_RETURN(1);
+}
 
 static void
 acl_update_proxy_user(ACL_PROXY_USER *new_value, bool is_revoke)
@@ -5183,7 +5293,7 @@ static void append_user(String *str, const char *u, const char *h,
 }
 
 
-bool mysql_grant_role(THD *thd, List <LEX_USER> &list)
+bool mysql_grant_role(THD *thd, List <LEX_USER> &list, bool revoke)
 {
   DBUG_ENTER("mysql_grant_role");
   /*
@@ -5215,16 +5325,29 @@ bool mysql_grant_role(THD *thd, List <LEX_USER> &list)
     rolename= granted_role->user.str;
   }
 
+  TABLE_LIST tables;
+  tables.init_one_table(C_STRING_WITH_LEN("mysql"),
+                        C_STRING_WITH_LEN("roles_mapping"),
+                        "roles_mapping", TL_WRITE);
+
   mysql_rwlock_wrlock(&LOCK_grant);
   mysql_mutex_lock(&acl_cache->lock);
   if (!(role= find_acl_role(rolename)))
   {
-    my_error(ER_INVALID_ROLE, MYF(0), rolename);
     mysql_mutex_unlock(&acl_cache->lock);
     mysql_rwlock_unlock(&LOCK_grant);
+    my_error(ER_INVALID_ROLE, MYF(0), rolename);
     DBUG_RETURN(TRUE);
   }
 
+  if (open_and_lock_tables(thd, &tables, FALSE, MYSQL_LOCK_IGNORE_TIMEOUT))
+  {                                             // Should never happen
+    mysql_mutex_unlock(&acl_cache->lock);
+    mysql_rwlock_unlock(&LOCK_grant);
+    my_error(ER_NO_SUCH_TABLE, MYF(0), "mysql", "roles_mapping");
+    DBUG_RETURN(TRUE);                          /* purecov: deadcode */
+  }
+
   while ((user= user_list++))
   {
     role_as_user= NULL;
@@ -5244,6 +5367,7 @@ bool mysql_grant_role(THD *thd, List <LEX_USER> &list)
         result= 1;
         continue;
       }
+
       /* can not grant current_role to current_role */
       if (granted_role->user.str == current_role.str)
       {
@@ -5264,31 +5388,69 @@ bool mysql_grant_role(THD *thd, List <LEX_USER> &list)
     ROLE_GRANT_PAIR *mapping= (ROLE_GRANT_PAIR *)
                                 alloc_root(&mem, sizeof(ROLE_GRANT_PAIR));
 
-    /* TODO write into roles_mapping table */
     init_role_grant_pair(&mem, mapping,
                          username, hostname, rolename);
-    int res= add_role_user_mapping(mapping);
+
-    if (res == -1)
+    if (!revoke)
     {
-      append_user(&wrong_users, username, hostname, role_as_user != NULL);
+      int res= add_role_user_mapping(mapping);
+      /* role or user does not exist*/
+      if (res == -1)
+      {
+        append_user(&wrong_users, username, hostname, role_as_user != NULL);
+        result= 1;
+        continue;
+      }
+
+      /*
+        Check if this grant would cause a cycle. It only needs to be run
+        if we're granting a role to a role
+       */
+      if (role_as_user &&
+          traverse_role_graph(role, NULL, NULL, NULL, role_explore_detect_cycle,
+                              NULL) == 2)
+      {
+        append_user(&wrong_users, username, hostname, TRUE);
+        result= 1;
+        /* need to rollback the mapping added previously */
+        remove_role_user_mapping(mapping);
+        continue;
+      }
+    }
+    else
+    {
+      /* revoke a role grant */
+      int res= remove_role_user_mapping(mapping);
+      if (res == -1)
+      {
+        append_user(&wrong_users, username, hostname, role_as_user != NULL);
+        result= 1;
+        continue;
+      }
+    }
+
+    /* write into the roles_mapping table */
+    if (replace_roles_mapping_table(tables.table, mapping, revoke))
+    {
+      append_user(&wrong_users, username, hostname, TRUE);
       result= 1;
+      if (!revoke)
+      {
+        /* need to rollback the mapping added previously */
+        remove_role_user_mapping(mapping);
+      }
+      else
+      {
+        /* need to rollback the mapping deleted previously */
+        add_role_user_mapping(mapping);
+      }
       continue;
     }
 
     /*
-       Check if this grant would cause a cycle. It only needs to be run
+       Only need to propagate grants when granting/revoking a role to/from
-       if we're granting a role to a role
+       a role
-     */
+    */
-    if (role_as_user &&
-        traverse_role_graph(role, NULL, NULL, NULL, role_explore_detect_cycle,
-                            NULL) == 2)
-    {
-      append_user(&wrong_users, username, hostname, TRUE);
-      result= 1;
-      continue;
-    }
-
-    /* only need to propagate grants when granting a role to a role */
     if (role_as_user)
     {
       acl_update_role_entry(role_as_user, role_as_user->initial_role_access);
@@ -5302,7 +5464,6 @@ bool mysql_grant_role(THD *thd, List <LEX_USER> &list)
              rolename,
              wrong_users.c_ptr_safe());
 
-
   DBUG_RETURN(result);
 }
 

sql/sql_acl.h

@@ -204,7 +204,7 @@ int check_change_password(THD *thd, const char *host, const char *user,
 bool change_password(THD *thd, const char *host, const char *user,
                      char *password);
 
-bool mysql_grant_role(THD *thd, List<LEX_USER> &user_list);
+bool mysql_grant_role(THD *thd, List<LEX_USER> &user_list, bool revoke);
 bool mysql_grant(THD *thd, const char *db, List <LEX_USER> &user_list,
                  ulong rights, bool revoke, bool is_proxy);
 int mysql_table_grant(THD *thd, TABLE_LIST *table, List <LEX_USER> &user_list,

sql/sql_parse.cc

@@ -3894,7 +3894,9 @@ end_with_restore_list:
 
     if (thd->security_ctx->user)              // If not replication
     {
-      if (!(res= mysql_grant_role(thd, lex->users_list)))
+      if (!(res= mysql_grant_role(thd, lex->users_list,
+                                  lex->sql_command == SQLCOM_GRANT_ROLE ? 0 : 1
+                                  )))
         my_ok(thd);
     }
     else

sql/sql_yacc.yy

@@ -14251,7 +14251,9 @@ revoke_command:
           {
             LEX *lex= Lex;
             lex->sql_command= SQLCOM_REVOKE_ROLE;
-            lex->type= 0;
+            /* The first role is the one that is revoked */
+            if (Lex->users_list.push_front($1))
+              MYSQL_YYABORT;
           }
 
         ;