Hi, Vicențiu! On Jun 25, noreply@launchpad.net wrote:

------------------------------------------------------------ revno: 3766 committer: Vicențiu Ciorbaru <cvicentiu@gmail.com> branch nick: trunk timestamp: Tue 2013-06-25 23:52:30 +0300 message: Added separation between roles and users in the mysql.user table modified: sql/sql_acl.cc

=== modified file 'sql/sql_acl.cc' --- sql/sql_acl.cc 2013-06-23 12:25:04 +0000 +++ sql/sql_acl.cc 2013-06-25 20:52:30 +0000 @@ -542,7 +542,16 @@ #endif /* HAVE_OPENSSL && !EMBEDDED_LIBRARY */ #define NORMAL_HANDSHAKE_SIZE 6

-static DYNAMIC_ARRAY acl_hosts, acl_users, acl_dbs, acl_proxy_users; +static DYNAMIC_ARRAY acl_hosts, acl_users, acl_dbs, acl_proxy_users, acl_roles; +/* XXX + ***** Potential optimization ***** + role_grants could potentially be a HASH with keys as usernames and values + as a DYNAMIC_ARRAY of pointers to granted roles + XXX + ***** Implementation choice for now ***** + An array representing mappings between acl_users and acl_roles; +*/

Agree about a hash. Hosts, users and dbs are stored in an array, because they support wildcards in MariaDB, not only exact matches. And very specific sort order, from most specific to least specific. But we only need exact matches for role names, so you can put all roles in a hash (see HASH or Hash_set).

+static DYNAMIC_ARRAY role_grants;

Same. Do you need an array here? Is the order of entries important? I don't think it is. In fact, you probably may store role<->user relationships directly in the ACL_USER structure. For a user - list of granted roles. For a role - list of users it is granted to. Then it should be easy to check if a given user can SET a given role.

static MEM_ROOT mem, memex; static bool initialized=0; static bool allow_all_hosts=1; @@ -876,11 +891,20 @@ while (!(read_record_info.read_record(&read_record_info))) { ACL_USER user; + bool is_role= FALSE; bzero(&user, sizeof(user)); update_hostname(&user.host, get_field(&mem, table->field[0])); user.user= get_field(&mem, table->field[1]); - DBUG_PRINT("info", ("user_entry %s", user.user)); - if (check_no_resolve && hostname_requires_resolving(user.host.hostname)) + + /* If the user entry is a role, skip password and hostname checks + A user can not log in with a role so some checks are not necessary + */ + if (!user.host.hostname) { + is_role= TRUE;

I'm sorry, but it wouldn't do. Apparently it is possible to create a user with the empty host name: MariaDB [test]> create user foo@''; Query OK, 0 rows affected (0.00 sec) MariaDB [test]> select user,host from mysql.user; +------+-----------+ | user | host | +------+-----------+ | foo | | Looking at compare_hostname() function, one can see that empty hostname means "any hostname is ok", it's a wildcard. It means, you cannot use empty hostnames to distinguish between roles and users. Which is a pity, I'd also prefer to use an empty hostname for that. You can use a NULL hostname (you'd need ALTER TABLE to make the column nullable) or a separate column, like, Is_role ENUM('N','Y').

+ } + + if (!is_role && check_no_resolve && + hostname_requires_resolving(user.host.hostname))

Regards, Sergei