Skip to content

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

Open
nadaelsayed11 wants to merge 1 commit into
MariaDB:10.11from
nadaelsayed11:10.11
Open

MDEV-37214: CREATE OR REPLACE UESR must not drop user if auth plugin is not loaded#5191
nadaelsayed11 wants to merge 1 commit into
MariaDB:10.11from
nadaelsayed11:10.11

Conversation

@nadaelsayed11
Copy link
Copy Markdown
Contributor

validated the plugin in "CREATE OR REPLACE UESR" before dropping the user.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses a bug (MDEV-37214) where executing CREATE OR REPLACE USER with an invalid authentication plugin could drop an existing user without writing to the binary log, leading to master/replica divergence. The changes introduce a pre-validation check for authentication plugins in sql/sql_acl.cc and add corresponding test cases. The review feedback identifies a critical issue: if a single statement contains multiple users (some valid and some invalid), the valid users will still be dropped before the statement fails, resulting in partial execution and replica divergence. The reviewer recommends pre-validating all users in a separate loop before performing any destructive operations and adding a test case to cover this multi-user scenario.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread sql/sql_acl.cc
Comment on lines +11209 to +11228
/*
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;
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;
Copy link
Copy Markdown

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.

Comment on lines +48 to +54
# 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;
Copy link
Copy Markdown

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;

@parasol-aser
Copy link
Copy Markdown

sql/sql_acl.cc:11209 in mysql_create_user()

The new per-account missing-plugin skip preserves the account on the primary, but if a later account in the same CREATE OR REPLACE USER statement succeeds, the original multi-account statement can still be binlogged. A replica with different auth-plugin availability can apply the skipped account change before failing replication with different errors, leaving account state divergent.

Fix: Prevalidate explicit auth plugins for the whole statement before any account mutation/binlog decision, or binlog only the account mutations that were actually applied; add a multi-account binlog/RPL regression for this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants