Skip to content

MDEV-37214: CREATE OR REPLACE UESR must not drop user if auth plugin is not loaded #5191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nadaelsayed11 wants to merge 1 commit into
base: 10.11
Choose a base branch
from
+36 −0
Open

MDEV-37214: CREATE OR REPLACE UESR must not drop user if auth plugin is not loaded #5191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions mysql-test/main/create_drop_user.result
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,10 @@ Warnings:
Note 1974 Can't drop user 'u1'@'%'; it doesn't exist
DROP USER u2;
ERROR HY000: Operation DROP USER failed for 'u2'@'%'
CREATE USER u3@localhost;
CREATE OR REPLACE USER u3@localhost IDENTIFIED VIA no_such_plugin;
ERROR HY000: Plugin 'no_such_plugin' is not loaded
SELECT user FROM mysql.user WHERE user='u3';
User
u3
DROP USER u3@localhost;
8 changes: 8 additions & 0 deletions mysql-test/main/create_drop_user.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,11 @@ DROP USER IF EXISTS u1, u2;

--error ER_CANNOT_USER
DROP USER u2;

# MDEV-37214: CREATE OR REPLACE USER with invalid plugin must not drop
# the existing user and must not leave master/replica in diverged state.
CREATE USER u3@localhost;
--error ER_PLUGIN_IS_NOT_LOADED
CREATE OR REPLACE USER u3@localhost IDENTIFIED VIA no_such_plugin;
SELECT user FROM mysql.user WHERE user='u3';
DROP USER u3@localhost;
Comment on lines +48 to +54
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Jun 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To prevent future regressions and verify that multi-user statements do not suffer from partial execution or master/replica divergence, please add a test case covering the scenario where a valid user and an invalid user are specified in the same CREATE OR REPLACE USER statement.

For example:

CREATE USER u1@localhost IDENTIFIED BY 'old_password';
CREATE USER u2@localhost;

--error ER_PLUGIN_IS_NOT_LOADED
CREATE OR REPLACE USER u1@localhost IDENTIFIED BY 'new_password', u2@localhost IDENTIFIED VIA no_such_plugin;

--echo # Verify that u1 was NOT modified because the statement failed
--echo # (its password should still be 'old_password')
--echo # ... query mysql.user or global_priv to verify ...

DROP USER u1@localhost, u2@localhost;

Copy link
Copy Markdown
Member

@gkodinov gkodinov Jun 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please describe what is this testing exactly? I do not see replication related testing done in this file.

Please consider adding a test that either checks the full scenario:

  • do the command on the master
  • verify that the binlog has the right contents on the master
  • observe that the slave didn't stop with an error when replicating it.
  • Conditionally include multiple accounts to the command as is suggested.

21 changes: 21 additions & 0 deletions sql/sql_acl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11206,6 +11206,27 @@ bool mysql_create_user(THD *thd, List <LEX_USER> &list, bool handle_as_role)
{
if (thd->lex->create_info.or_replace())
{
/*
MDEV-37214: pre-validate auth plugins before the destructive DROP so
that a missing plugin does not silently drop the user without a binlog
event, causing master/replica divergence.
*/
bool plugin_error= false;
Copy link
Copy Markdown
Member

@gkodinov gkodinov Jun 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this is a valiant effort, I doubt this could be the only reason why a CREATE OR REPLACE USER can fail.
It could probably fail for a number of other reasons, like .e.g. SQL_MODE flag NO_AUTO_CREATE_USER etc.
And, it can fail due to a transient condition: e.g. the plugin could become unavailable after it being checked by your code above.

As Brandon mentioned this is a complex problem.

First of all I'd like to see an explanation on why there's a doubling of the binlog entry in this case. If there still is doubling, that is.

Secondly, you need to reach an agreement with the other MariaDB developers on what would the proper fix be in this case.

FWIW, in MySQL, a similar issue class was solved (atomic ACL) by:

  • running a check pass to ensure the statement is not in some kind of violation (what you've tried to do here)
  • run all table updates on an ACID compliant storage engine (InnoDB) in explicit transactional mode and making sure there's a single commit or rollback at the end.
  • Making sure that in-memory structures are also reset post- rollback while holding a high level ACL lock to ensure atomicity.

A different approach could be taken here. This is why it's best to discuss it with the reviewer(s) first, as I've suggested.

I would at least make a utility function called "pre-check" and stuff all the checks you're doing here into it.

I'd also explore the other ACL statements. Maybe ALTER has a similar issue?

for (USER_AUTH *auth= user_name->auth; auth; auth= auth->next)
{
if (auth->plugin.length &&
!plugin_is_ready(&auth->plugin, MYSQL_AUTHENTICATION_PLUGIN))
{
my_error(ER_PLUGIN_IS_NOT_LOADED, MYF(0), auth->plugin.str);
append_user(thd, &wrong_users, user_name);
result= true;
plugin_error= true;
break;
}
}
if (plugin_error)
continue;
Comment on lines +11209 to +11228
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Jun 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There is a potential master/replica divergence issue when CREATE OR REPLACE USER is called with multiple users, where some have valid plugins and others have invalid plugins.

The Issue

The current validation is performed inside the main loop that iterates over each user in the list. If a query like the following is executed:

CREATE OR REPLACE USER u1 IDENTIFIED VIA valid_plugin, u2 IDENTIFIED VIA invalid_plugin;
  1. u1 is processed first. Since its plugin is valid, it is dropped and recreated on the master.
  2. u2 is processed next. Since its plugin is invalid, the loop sets result = true and continues.
  3. The function eventually returns true (failure), so the statement fails and is not written to the binary log.

As a result, u1 is modified on the master but not on the replica, leading to master/replica divergence.

Recommended Solution

To prevent partial execution and master/replica divergence, we should pre-validate the authentication plugins for all users in the list in a separate loop at the very beginning of mysql_create_user, before any destructive operations (like dropping users) are performed. If any user in the list has an invalid plugin, we should fail the entire statement immediately.


// Drop the existing user
if (handle_grant_data(thd, tables, 1, user_name, NULL) <= 0)
{
Expand Down