github-actions[bot] commented on code in PR #66115:
URL: https://github.com/apache/doris/pull/66115#discussion_r4375441387
##########
fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java:
##########
@@ -1210,8 +1226,25 @@ private void setPasswordInternal(UserIdentity userIdent,
byte[] password, UserId
ErrorReport.reportDdlException(ErrorCode.ERR_CREDENTIALS_CONTRADICT_TO_HISTORY,
userIdent.getQualifiedUser(), userIdent.getHost());
}
+ if (retainCurrent) {
+ // MySQL-compatible RETAIN CURRENT PASSWORD constraint:
+ // "If you specify RETAIN CURRENT PASSWORD for an account
+ // that has an empty primary password, the statement
+ // fails." Checked here (not only at analysis) so every
+ // caller is covered; skipped on replay (the journal
+ // already passed this check). NB an EMPTY NEW password is
+ // NOT an error — it empties the secondary as well (MySQL
+ // semantics, handled in UserManager.setPassword).
+ if (user == null || user.getPassword() == null
+ || user.getPassword().getPassword() == null
+ || user.getPassword().getPassword().length == 0) {
+ throw new DdlException(
+ "Current password cannot be retained for user
" + userIdent
+ + " because it does not exist or is
empty");
+ }
+ }
}
- userManager.setPassword(userIdent, password, errOnNonExist);
+ userManager.setPassword(userIdent, password, errOnNonExist,
retainCurrent);
Review Comment:
[P2] Preserve the secondary in the immediate domain refresh
For an already resolved domain account, retaining `p1` while setting `p2`
builds both slots here, but the same method then calls
`refreshAuthenticationBinding` at lines 1258-1259. That helper still executes
`user.setPassword(domain.getPassword().getPassword())`
(`UserManager.java:523`), replacing every resolved-IP entry with a primary-only
Password. Clients using `p1` are rejected immediately after the successful
RETAIN until the next resolver tick; ACCOUNT_UNLOCK and REQUIRE changes can
erase the working secondary through the same helper. The earlier domain thread
covered the periodic rebuild, which now repairs this separate immediate-copy
failure. Copy both slots in `refreshAuthenticationBinding` too, and test login
immediately after RETAIN/unlock/TLS changes without forcing a periodic refresh
first; the new domain unit test currently forces that refresh before checking
either password.
##########
fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserManager.java:
##########
@@ -370,14 +438,77 @@ public Map<String, List<User>> getNameToUsers() {
}
public void setPassword(UserIdentity userIdentity, byte[] password,
boolean errOnNonExist) throws DdlException {
+ setPassword(userIdentity, password, errOnNonExist, false);
+ }
+
+ /**
+ * Set the user's password, with MySQL-compatible dual password semantics:
+ * with {@code retainCurrent} ("RETAIN CURRENT PASSWORD") the previous
+ * primary password becomes the secondary password and remains valid for
+ * authentication; without it an existing secondary password remains
+ * UNCHANGED (MySQL: "If an account has a secondary password and you
+ * change its primary password without specifying RETAIN CURRENT PASSWORD,
+ * the secondary password remains unchanged."). Setting an EMPTY password
+ * empties the secondary password as well, even with retain (also MySQL).
+ */
+ public void setPassword(UserIdentity userIdentity, byte[] password,
boolean errOnNonExist,
+ boolean retainCurrent) throws DdlException {
User user = getUserByUserIdentity(userIdentity);
if (user == null) {
if (errOnNonExist) {
throw new DdlException("user " + userIdentity + " does not
exist");
}
return;
}
- user.setPassword(password);
+ Password oldPassword = user.getPassword();
+ byte[] carried;
+ if (password == null || password.length == 0) {
+ // an empty new password empties the secondary as well, even with
+ // RETAIN CURRENT PASSWORD (MySQL semantics)
+ carried = null;
+ } else if (retainCurrent) {
+ carried = oldPassword == null ? null : oldPassword.getPassword();
+ } else {
+ carried = oldPassword == null ? null :
oldPassword.getSecondaryPassword();
Review Comment:
[P1] Clear retained credentials when authentication bindings change
With a valid `bound` integration and a local account initially using `p1`,
execute `ALTER USER u IDENTIFIED BY 'p2' RETAIN CURRENT PASSWORD`, then `ALTER
USER u IDENTIFIED WITH AUTHENTICATION INTEGRATION bound AS 'external'`, then
`ALTER USER u IDENTIFIED BY 'p3'`. Both binding transitions reach this
no-retain branch, so `p1` survives beside the generated bound password and is
carried again when the binding is cleared. The final native account
unexpectedly accepts `p1` as well as `p3`; the bound-user guard suppresses it
only during the external-auth period. Clear local secondary state when the
authentication binding changes, reject RETAIN of a bound placeholder, and cover
bind/unbind plus replay starting with a retained credential. This also follows
the [MySQL dual-password
rule](https://dev.mysql.com/doc/refman/8.0/en/password-management.html) that
changing authentication plugins discards the secondary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
