raghav-reglobe commented on code in PR #66115:
URL: https://github.com/apache/doris/pull/66115#discussion_r4409545700
##########
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:
Fixed in 0f8b49234ac7: a binding transition (into, out of or between
integrations) now discards the retained secondary instead of carrying it
(`UserManager.setPassword(..., discardSecondary)` from
`Auth.setPasswordInternal`, derived from the account's state plus the journal's
binding so replay matches), and `RETAIN CURRENT PASSWORD` is refused into and
out of a binding — the current password of a bound account is a generated
placeholder. Tests: `testBindingTransitionDiscardsRetainedPassword`,
`testRetainIsRefusedAcrossBindingTransitions`,
`testBindingTransitionReplayDiscardsRetainedPassword`.
##########
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:
Fixed in 0f8b49234ac7: `refreshAuthenticationBinding` copies both slots
through the new `Password.copyOf` (the periodic resolver copy uses it too), so
the retained password authenticates immediately after RETAIN, ACCOUNT_UNLOCK
and REQUIRE changes on a domain account.
`testDomainRetainAuthenticatesBothSlotsBeforeTheResolverTick` asserts the
immediate path without a periodic refresh; the existing resolver test keeps
covering the periodic one.
--
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]
