Hi, Oleksandr,

commit 5154e224a97 Author: Oleksandr Byelkin <sanja@mariadb.com> Date: Mon Dec 13 16:15:21 2021 +0100

diff --git a/scripts/mysql_system_tables.sql b/scripts/mysql_system_tables.sql index fd2f1c95dda..a1721913b2c 100644 --- a/scripts/mysql_system_tables.sql +++ b/scripts/mysql_system_tables.sql @@ -47,6 +47,10 @@ INSERT IGNORE INTO global_priv SELECT * FROM tmp_user_sys WHERE 0 <> @need_sys_u DROP TABLE tmp_user_sys;

+-- This special "role" needed for GRAND ... TO PUBLIC +INSERT IGNORE INTO global_priv (Host,User,Priv) VALUES ('', 'PUBLIC', concat('{"access":0,"version_id":',regexp_replace(regexp_replace(@@version, '\\b\\d\\b', '0\\0'), '\\D', ''),',"is_role":true}'));

1. why here and not in mysql_system_tables_data.sql ? 2. why this version magic? mysql_system_tables_data.sql doesn't do it for other accounts 3. why do you do it at all? I thought the server can create this role as needed

+ + CREATE DEFINER='mariadb.sys'@'localhost' SQL SECURITY DEFINER VIEW IF NOT EXISTS user AS SELECT Host, User, @@ -95,7 +99,9 @@ CREATE DEFINER='mariadb.sys'@'localhost' SQL SECURITY DEFINER VIEW IF NOT EXISTS ELT(IFNULL(JSON_VALUE(Priv, '$.is_role'), 0) + 1, 'N', 'Y') AS is_role, IFNULL(JSON_VALUE(Priv, '$.default_role'), '') AS default_role, CAST(IFNULL(JSON_VALUE(Priv, '$.max_statement_time'), 0.0) AS DECIMAL(12,6)) AS max_statement_time - FROM global_priv; + FROM global_priv +-- Do not show special role for GRANT TO PUBLIC + WHERE not (Host = "" and User = "PUBLIC");

Why? I don't think it's wrong to show a new PUBLIC role.

-- Remember for later if user table already existed set @had_user_table= @@warning_count != 0; diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 601cae2e945..aa9bd91ec52 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -316,6 +318,13 @@ static bool show_table_and_column_privileges(THD *, const char *, const char *, static int show_routine_grants(THD *, const char *, const char *, const Sp_handler *sph, char *, int);

+static ACL_ROLE *acl_public= NULL; + +inline privilege_t public_access() +{ + return (acl_public ? acl_public->access : NO_ACL); +}

Why not to have acl_public always not null? e.g. define it as static ACL_ROLE acl_public; so it's not even a pointer. And don't store it in the dynamic array. Or make it like static ACL_ROLE acl_public_dummy, *acl_public= &acl_public_dummy; then you can keep he real role in the dynamic array, but still acl_public will never be null. but in the standard PUBLIC is not a role at all, so may be it'd be simpler not to store it in the dynamic array

+ class Grant_tables; class User_table; class Proxies_priv_table; @@ -3148,7 +3183,7 @@ bool acl_getroot(Security_context *sctx, const char *user, const char *host, } else // Role, not User { - ACL_ROLE *acl_role= find_acl_role(user); + ACL_ROLE *acl_role= find_acl_role(user, false);

are there tests for that?

if (acl_role) { res= 0; @@ -3161,6 +3196,26 @@ bool acl_getroot(Security_context *sctx, const char *user, const char *host, } }

+ /* + PUBLIC magic: + + Note: for usual user privileges of 3 component merged together: + 1) user privileges + 2) set role privileges + 3) public privileges + But for this routine (used in Security_context::change_security_context) + only 2 component merged: + 1) user OR role privileges we are switching to

because if we're switching to a user, there can be no set role? I feel this comment is more confusing, then helping, really, I'd suggest to remove it completely

+ 2) public privileges + */ + if (acl_public) + { + if (ACL_DB *acl_db= acl_db_find(db, public_name.str, "", "", FALSE)) + sctx->db_access|= acl_db->access; + + sctx->master_access|= acl_public->access; + } + mysql_mutex_unlock(&acl_cache->lock); DBUG_RETURN(res); } @@ -3320,20 +3375,22 @@ int acl_setrole(THD *thd, const char *rolename, privilege_t access) /* merge the privileges */ Security_context *sctx= thd->security_ctx; sctx->master_access= access; - if (thd->db.str) - sctx->db_access= acl_get(sctx->host, sctx->ip, sctx->user, thd->db.str, FALSE); - if (!strcasecmp(rolename, "NONE")) { thd->security_ctx->priv_role[0]= 0; } else { - if (thd->db.str) - sctx->db_access|= acl_get("", "", rolename, thd->db.str, FALSE); /* mark the current role */ strmake_buf(thd->security_ctx->priv_role, rolename); } + if (thd->db.str) + sctx->db_access= acl_get_all3(sctx, thd->db.str, FALSE); + + // PUBLIC magic + if (acl_public) + sctx->master_access|= acl_public->access;

why not to do it in check_user_can_set_role()? Like - *access = acl_user->access | role->access; + *access = acl_user->access | role->access | acl_public->access;

+ return 0; }

@@ -3347,9 +3404,13 @@ static uchar* check_get_key(ACL_USER *buff, size_t *length,

static void acl_update_role(const char *rolename, const privilege_t privileges) { - ACL_ROLE *role= find_acl_role(rolename); + ACL_ROLE *role= find_acl_role(rolename, true); if (role) + { role->initial_role_access= role->access= privileges; + if (strcasecmp(rolename, public_name.str) == 0) + acl_public= role;

why? I mean, in what case can acl_public != role here?

+ } }

@@ -3638,6 +3703,23 @@ privilege_t acl_get(const char *host, const char *ip, DBUG_RETURN(db_access & host_access); }

+/* + Check if there is access for the host/user, role, public on the database +*/ + +privilege_t acl_get_all3(Security_context *sctx, const char *db, + bool db_is_patern) +{ + privilege_t access= acl_get(sctx->host, sctx->ip, + sctx->priv_user, db, db_is_patern); + if (sctx->priv_role[0]) + access|= acl_get("", "", sctx->priv_role, db, db_is_patern); + if (acl_public) + access|= acl_get("", "", public_name.str, db, db_is_patern);

how can this be optimized, considering that in most cases and for most db names there will be no grants to role and public? A couple of ideas: maintain counters total_number_of_db_grants_to_roles ad total_number_of_db_grants_to_public. This is rather crude. keep grants to users, to roles, and to public in separate caches. or in different dynamic arrays (won't help much in a default setup) keep propery per ACL_DB or shared between all ACL_DB's of one DB? that'd be rather tricky to maintain

+ return access; +} + + /* Check if there are any possible matching entries for this host

@@ -3759,7 +3841,7 @@ static bool add_role_user_mapping(const char *uname, const char *hname, const char *rname) { ACL_USER_BASE *grantee= find_acl_user_base(uname, hname); - ACL_ROLE *role= find_acl_role(rname); + ACL_ROLE *role= find_acl_role(rname, true);

why?

if (grantee == NULL || role == NULL) return 1; @@ -4247,7 +4329,7 @@ bool is_acl_user(const char *host, const char *user) if (*host) // User res= find_user_exact(host, user) != NULL; else // Role - res= find_acl_role(user) != NULL; + res= find_acl_role(user, false) != NULL;

this disallows DEFINER PUBLIC, right? I couldn't find a test for that.

mysql_mutex_unlock(&acl_cache->lock); return res; @@ -7537,11 +7620,11 @@ bool mysql_grant_role(THD *thd, List <LEX_USER> &list, bool revoke) if (user->host.str) hostname= user->host; else - if ((role_as_user= find_acl_role(user->user.str))) + if ((role_as_user= find_acl_role(user->user.str, false)))

Why is it false here? It looks like it disallows GRANT role TO PUBLIC

hostname= empty_clex_str; else { - if (is_invalid_role_name(username.str)) + if (check_role_name(username.str, false) == ROLE_NAME_INVALID) { append_user(thd, &wrong_users, &username, &empty_clex_str); result= 1; @@ -8280,19 +8355,14 @@ bool check_grant(THD *thd, privilege_t want_access, TABLE_LIST *tables, if (any_combination_will_do) continue;

- t_ref->grant.grant_table_user= grant_table; // Remember for column test - t_ref->grant.grant_table_role= grant_table_role; - t_ref->grant.version= grant_version; - t_ref->grant.privilege|= grant_table ? grant_table->privs : NO_ACL; - t_ref->grant.privilege|= grant_table_role ? grant_table_role->privs : NO_ACL; + t_ref->grant.privilege|= t_ref->grant.aggregate_privs(); t_ref->grant.want_privilege= ((want_access & COL_ACLS) & ~t_ref->grant.privilege);

if (!(~t_ref->grant.privilege & want_access)) continue;

- if ((want_access&= ~((grant_table ? grant_table->cols : NO_ACL) | - (grant_table_role ? grant_table_role->cols : NO_ACL) | - t_ref->grant.privilege))) + if ((want_access&= ~(t_ref->grant.aggregate_cols() | + t_ref->grant.privilege))) { goto err; // impossible }

1. why is this if() here, if it's impossible? 2. I'm a bit worried that you set t_ref->grant.version early when it's still possible to goto err. Old code was setting the version very late after all checks have succeeded.

@@ -8335,6 +8405,49 @@ static void check_grant_column_int(GRANT_TABLE *grant_table, const char *name, } }

+inline privilege_t GRANT_INFO::aggregate_privs() +{ + return (grant_table_user ? grant_table_user->privs : NO_ACL) | + (grant_table_role ? grant_table_role->privs : NO_ACL) | + (grant_public ? grant_public->privs : NO_ACL); +} + +inline privilege_t GRANT_INFO::aggregate_cols() +{ + return (grant_table_user ? grant_table_user->cols : NO_ACL) | + (grant_table_role ? grant_table_role->cols : NO_ACL) | + (grant_public ? grant_public->cols : NO_ACL); +} + +void GRANT_INFO::refresh(const Security_context *sctx, + const char *db, const char *table) +{ + if (version != grant_version) + read(sctx, db, table); +} + +void GRANT_INFO::read(const Security_context *sctx, + const char *db, const char *table) +{ +#ifdef EMBEDDED_LIBRARY + grant_table_user= grant_table_role= grant_public= NULL; +#else + grant_table_user= + table_hash_search(sctx->host, sctx->ip, db, + sctx->priv_user, + table, FALSE); /* purecov: inspected */

you don't need to preserve purecov comments when moving the code around

+ grant_table_role= + sctx->priv_role[0] ? table_hash_search("", NULL, db, + sctx->priv_role, + table, TRUE) : NULL; + grant_public= + acl_public ? table_hash_search("", NULL, db, + public_name.str, + table, TRUE) : NULL; +#endif + version= grant_version; /* purecov: inspected */ +} + /* Check column rights in given security context

@@ -9533,6 +9626,13 @@ static bool show_global_privileges(THD *thd, ACL_USER_BASE *acl_entry, want_access= ((ACL_ROLE *)acl_entry)->initial_role_access; else want_access= acl_entry->access; + + // suppress "GRANT USAGE ON *.* TO `PUBLIC`"

why? I agree that it's pointless, but disabling just it is kind of difficult to explain.

+ if (!(want_access & ~GRANT_ACL) && + acl_entry->user.length == public_name.length && + strcasecmp(acl_entry->user.str, public_name.str) == 0) + return FALSE; + if (test_all_bits(want_access, (GLOBAL_ACLS & ~ GRANT_ACL))) global.append(STRING_WITH_LEN("ALL PRIVILEGES")); else if (!(want_access & ~GRANT_ACL)) @@ -11015,10 +11116,15 @@ bool mysql_drop_user(THD *thd, List <LEX_USER> &list, bool handle_as_role) { int rc; user_name= get_current_user(thd, tmp_user_name, false); - if (!user_name) + if (!user_name || (handle_as_role && + (strcasecmp(user_name->user.str, + public_name.str) == 0)))

better add this condition to the following if(), not to this one. Like: - if (handle_as_role != user_name->is_role()) + if (handle_as_role != user_name->is_role() || + (handle_as_role && check_role_name(...) == ROLE_NAME_INVALID))

{ thd->clear_error(); - append_str(&wrong_users, STRING_WITH_LEN("CURRENT_ROLE")); + if (!user_name) + append_str(&wrong_users, STRING_WITH_LEN("CURRENT_ROLE")); + else + append_str(&wrong_users, public_name.str, public_name.length); result= TRUE; continue; } diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 6290a47ee30..143e647f74c 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -5304,7 +5300,7 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond) &thd->col_access, NULL, 0, 1) || (!thd->col_access && check_grant_db(thd, db_name->str))) || sctx->master_access & (DB_ACLS | SHOW_DB_ACL) || - acl_get(sctx->host, sctx->ip, sctx->priv_user, db_name->str, 0)) + acl_get_all3(sctx, db_name->str, 0))

Was it a bug? That one couldn't see db's if privileges were granted via roles?

#endif { Dynamic_array<LEX_CSTRING*> table_names(PSI_INSTRUMENT_MEM); diff --git a/mysql-test/main/public_basic.test b/mysql-test/main/public_basic.test new file mode 100644 index 00000000000..3f5993dfe97 --- /dev/null +++ b/mysql-test/main/public_basic.test @@ -0,0 +1,93 @@ +SHOW GRANTS FOR PUBLIC; + +--echo # it is not PUBLIC but an user +--echo # (this should work as it allowed for roles for example) +create user PUBLIC; +create user PUBLIC@localhost; +GRANT SELECT on test.* to PUBLIC@localhost; +drop user PUBLIC@localhost; +drop user PUBLIC; + +--echo # preinstalled PUBLIC +GRANT SELECT on test.* to PUBLIC; +GRANT SELECT on mysql.db to PUBLIC; +--replace_regex /"version_id"\:[0-9]+/"version_id":VERSION/ +select * from mysql.global_priv where user="PUBLIC" ; + +SHOW GRANTS FOR PUBLIC; + +GRANT UPDATE on test.* to PUBLIC; +GRANT UPDATE on mysql.db to PUBLIC; + +SHOW GRANTS FOR PUBLIC; + +REVOKE SELECT on test.* from PUBLIC; +REVOKE SELECT on mysql.db from PUBLIC; + +SHOW GRANTS FOR PUBLIC; + +REVOKE UPDATE on test.* from PUBLIC; +REVOKE UPDATE on mysql.db from PUBLIC; + +--error ER_NONEXISTING_GRANT +REVOKE UPDATE on test.* from PUBLIC; +--error ER_NONEXISTING_TABLE_GRANT +REVOKE UPDATE on mysql.db from PUBLIC; + +SHOW GRANTS FOR PUBLIC; + +--echo # automaticly added PUBLIC +delete from mysql.global_priv where user="PUBLIC"; +flush privileges; +select * from mysql.global_priv where user="PUBLIC" ; +GRANT SELECT on test.* to PUBLIC; +GRANT SELECT on mysql.db to PUBLIC; +--replace_regex /"version_id"\:[0-9]+/"version_id":VERSION/ +select * from mysql.global_priv where user="PUBLIC" ; + +SHOW GRANTS FOR PUBLIC; + +GRANT UPDATE on test.* to PUBLIC; +GRANT UPDATE on mysql.db to PUBLIC; + +SHOW GRANTS FOR PUBLIC; + +REVOKE SELECT on test.* from PUBLIC; +REVOKE SELECT on mysql.db from PUBLIC; + +SHOW GRANTS FOR PUBLIC; + +REVOKE UPDATE on test.* from PUBLIC; +REVOKE UPDATE on mysql.db from PUBLIC; + +SHOW GRANTS FOR PUBLIC; + +--error ER_INVALID_ROLE +GRANT XXXXXX TO CURRENT_USER; +--echo # following should fail with the same error as above +--error ER_INVALID_ROLE +GRANT PUBLIC TO CURRENT_USER; + +--error ER_INVALID_ROLE +REVOKE XXXXXX FROM CURRENT_USER; +--echo # following should fail with the same error as above +--error ER_INVALID_ROLE +REVOKE PUBLIC FROM CURRENT_USER; +--error ER_CANNOT_USER + +drop role XXXXXX; +--echo # following should fail with the same error as above +--error ER_CANNOT_USER +drop role PUBLIC; + +--error ER_INVALID_ROLE +SET ROLE XXXXXX; +--echo # following should fail with the same error as above +--error ER_INVALID_ROLE +SET ROLE PUBLIC; + +--error ER_INVALID_ROLE +SET DEFAULT ROLE XXXXXX; +--echo # following should fail with the same error as above +--error ER_INVALID_ROLE +SET DEFAULT ROLE PUBLIC;

This is good, but one can update global_priv table directly and put the default_role=PUBLIC there. Please, add a test for that. and also a couple of tests for `public` (in backticks)

diff --git a/mysql-test/main/public_privileges.test b/mysql-test/main/public_privileges.test new file mode 100644 index 00000000000..a542376f05c --- /dev/null +++ b/mysql-test/main/public_privileges.test @@ -0,0 +1,293 @@ +--echo # +--echo # Test DB/TABLE/COLUMN privileges in queries +--echo # + +SHOW GRANTS FOR PUBLIC; + +create user testuser; +create database testdb1; +use testdb1; +create table t1 (a int, b int); +insert into t1 values (1,2); +create database testdb2; +use testdb2; +create table t2 (a int, b int); +insert into t2 values (1,2); +create table t3 (a int, b int); +insert into t3 values (1,2); + +connect (testuser,localhost,testuser,,); +connection testuser;

"connect testuser" also switches to this connection, the following "connection testuser" is redundant

+--error ER_TABLEACCESS_DENIED_ERROR +select * from testdb1.t1; +--error ER_TABLEACCESS_DENIED_ERROR +select * from testdb2.t2; +--error ER_TABLEACCESS_DENIED_ERROR +select b from testdb2.t3; +--error ER_TABLEACCESS_DENIED_ERROR +select a from testdb2.t3; + +connection default; + +GRANT SELECT ON testdb1.* to PUBLIC; +GRANT SELECT ON testdb2.t2 to PUBLIC; +GRANT SELECT (b) ON testdb2.t3 to PUBLIC; + +disconnect testuser; +connect (testuser,localhost,testuser,,); +connection testuser; +select * from testdb1.t1; +select * from testdb2.t2; +select b from testdb2.t3; +--error ER_COLUMNACCESS_DENIED_ERROR +select a from testdb2.t3; + +connection default; + +--replace_column 1 # 3 # 6 # 7 # +SHOW PROCESSLIST;

why?

+ +connection default; + +use test; +disconnect testuser; +REVOKE SELECT ON testdb1.* from PUBLIC; +REVOKE SELECT ON testdb2.t2 from PUBLIC; +REVOKE SELECT (b) ON testdb2.t3 from PUBLIC;

+drop user testuser; +drop database testdb1; +drop database testdb2; + +--echo # +--echo # test global process list privilege and EXECUTE db level +--echo # + +create user testuser; +create database testdb; +use testdb; +create procedure p1 () select 1; + +connect (testuser,localhost,testuser,,); +connection testuser; + +--replace_column 1 # 3 # 6 # 7 # +SHOW PROCESSLIST; +--error ER_PROCACCESS_DENIED_ERROR +call testdb.p1(); + +connection default; + +GRANT PROCESS ON *.* to PUBLIC; +GRANT EXECUTE ON testdb.* to PUBLIC; + +disconnect testuser; +connect (testuser,localhost,testuser,,); +connection testuser; + +--replace_column 1 # 3 # 6 # 7 # +SHOW PROCESSLIST; +call testdb.p1(); + +connection default; + +--replace_column 1 # 3 # 6 # 7 # +SHOW PROCESSLIST; + +connection default; + +use test; +disconnect testuser; +REVOKE PROCESS ON *.* from PUBLIC; +REVOKE EXECUTE ON testdb.* from PUBLIC; +drop user testuser; +drop database testdb; + +--echo # +--echo # test DB privilege to allow USE statement +--echo # + +create user testuser; +create database testdb; + +connect (testuser,localhost,testuser,,); +connection testuser; + +--error ER_DBACCESS_DENIED_ERROR +use testdb; + +connection default; + +GRANT LOCK TABLES ON testdb.* to PUBLIC; + +disconnect testuser; +connect (testuser,localhost,testuser,,); +connection testuser; + +use testdb; + +connection default; + +use test; +disconnect testuser; +REVOKE LOCK TABLES ON testdb.* from PUBLIC; +drop user testuser; +drop database testdb; + + +--echo # +--echo # test DB privilege to allow USE statement (as above) +--echo # test current db privileges +--echo # + +create user testuser; +create database testdb; +use testdb; +create table t1 (a int); +insert into t1 values (1); +GRANT LOCK TABLES ON testdb.* to PUBLIC; + +connect (testuser,localhost,testuser,,); +connection testuser; + +use testdb; +--error ER_TABLEACCESS_DENIED_ERROR +update t1 set a=a+1; + +connection default; + +GRANT UPDATE,SELECT ON testdb.* to PUBLIC; + +disconnect testuser; +connect (testuser,localhost,testuser,,); +connection testuser; + +use testdb; +update t1 set a=a+1; + +connection default; +select * from testdb.t1; + +use test; +disconnect testuser; +REVOKE LOCK TABLES ON testdb.* from PUBLIC; +REVOKE UPDATE,SELECT ON testdb.* from PUBLIC; +drop user testuser; +drop database testdb; + + +--echo # +--echo # test DB privilege to allow USE statement (as above) +--echo # test table/column privileges in current DB +--echo # + +create user testuser; +create database testdb; +use testdb; +create table t1 (a int); +insert into t1 values (1); +create table t2 (a int, b int); +insert into t2 values (1,2); +GRANT LOCK TABLES ON testdb.* to PUBLIC; + +connect (testuser,localhost,testuser,,); +connection testuser; + +use testdb; +--error ER_TABLEACCESS_DENIED_ERROR +delete from t1; +--error ER_TABLEACCESS_DENIED_ERROR +select b from t2; +--error ER_TABLEACCESS_DENIED_ERROR +select a from t2; + +connection default; + +GRANT DELETE ON testdb.t1 to PUBLIC; +GRANT SELECT (a) ON testdb.t2 to PUBLIC; + +disconnect testuser; +connect (testuser,localhost,testuser,,); +connection testuser; + +use testdb; +delete from t1; +select a from t2; +--error ER_COLUMNACCESS_DENIED_ERROR +select b from t2; + +connection default; +select * from testdb.t1; + +use test; +disconnect testuser; +REVOKE LOCK TABLES ON testdb.* from PUBLIC; +REVOKE DELETE ON testdb.t1 from PUBLIC; +REVOKE SELECT (a) ON testdb.t2 from PUBLIC; +drop user testuser; +drop database testdb; + +--echo # +--echo # test function privilege +--echo # + +create user testuser; +create database testdb; +use testdb; +create function f1() returns int return 2; + +connect (testuser,localhost,testuser,,); +connection testuser; + +--error ER_PROCACCESS_DENIED_ERROR +alter function testdb.f1 comment "A stupid function"; +--error ER_PROCACCESS_DENIED_ERROR +select testdb.f1(); + +connection default; + +GRANT ALTER ROUTINE ON testdb.* to PUBLIC; + +disconnect testuser; +connect (testuser,localhost,testuser,,); +connection testuser; + +alter function testdb.f1 comment "A stupid function"; +--error ER_PROCACCESS_DENIED_ERROR +select testdb.f1(); + +connection default; + +use test; +disconnect testuser; +REVOKE ALTER ROUTINE ON testdb.* from PUBLIC; +drop function testdb.f1; +drop user testuser; +drop database testdb; + +--echo # +--echo # bug with automatically added PUBLIC role +--echo # + +--echo # automaticly added PUBLIC +delete from mysql.global_priv where user="PUBLIC"; +flush privileges; +GRANT SELECT on test.* to PUBLIC; + +REVOKE SELECT on test.* from PUBLIC; + +create user testuser; +create database testdb1; +use testdb1; +create table t1 (a int, b int); +insert into t1 values (1,2); + +connect (testuser,localhost,testuser,,); +connection testuser; +--error ER_TABLEACCESS_DENIED_ERROR +select * from testdb1.t1; + +connection default; + +disconnect testuser; +drop user testuser; +drop database testdb1; diff --git a/mysql-test/suite/roles/none_public.test b/mysql-test/suite/roles/none_public.test index a0ec2315cfc..d0ba62d226b 100644 --- a/mysql-test/suite/roles/none_public.test +++ b/mysql-test/suite/roles/none_public.test @@ -19,8 +19,9 @@ grant select on *.* to none; grant public to role1; --error ER_INVALID_ROLE grant role1 to public; ^^^

may be `revoke all privileges, grant option` ? looks like a good place to have it tested this shouldn't fail

---error ER_INVALID_ROLE -grant select on *.* to public; +# PUBLIC is legal role +#--error ER_INVALID_ROLE +#grant select on *.* to public;

you can just remove it, commenting out isn't better

--error ER_INVALID_ROLE grant role1 to current_role; @@ -40,16 +41,18 @@ revoke select on *.* from public;

--error ER_INVALID_ROLE show grants for none; ---error ER_INVALID_ROLE -show grants for public; +# PUBLIC is legal role +#--error ER_INVALID_ROLE +#show grants for public;

--error ER_INVALID_ROLE create definer=none view test.v1 as select 1; ---error ER_INVALID_ROLE -create definer=public view test.v1 as select 1; +# PUBLIC is legal role +#--error ER_INVALID_ROLE +#create definer=public view test.v1 as select 1;

eh, no, PUBLIC isn't a valid definer, as far as I understand.

drop role role1;

-insert mysql.global_priv values ('', 'none', '{"is_role":true}'), ('', 'public', '{"is_role":true}'); +insert mysql.global_priv values ('', 'none', '{"is_role":true}'); flush privileges; -delete from mysql.global_priv where host=''; +delete from mysql.global_priv where host='' and user='none'; diff --git a/mysql-test/main/sp_notembedded.result b/mysql-test/main/sp_notembedded.result index e03361598a6..239f8acb7f3 100644 --- a/mysql-test/main/sp_notembedded.result +++ b/mysql-test/main/sp_notembedded.result @@ -345,7 +345,6 @@ show grants; Grants for foo1@localhost GRANT USAGE ON *.* TO `foo1`@`localhost` IDENTIFIED BY PASSWORD '-F3A2A51A9B0F2BE2468926B4132313728C250DBF' GRANT CREATE ROUTINE ON `test`.* TO `foo1`@`localhost` -GRANT EXECUTE, ALTER ROUTINE ON PROCEDURE `test`.`spfoo` TO `foo1`@`localhost`

Why did this line disappear?

connection default; disconnect foo; drop procedure spfoo;

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org