CalvinKirs commented on code in PR #66115:
URL: https://github.com/apache/doris/pull/66115#discussion_r3955198083
##########
fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java:
##########
@@ -1051,21 +1074,72 @@ public void setPasswordInternal(UserIdentity userIdent,
byte[] password, UserIde
ErrorReport.reportDdlException(ErrorCode.ERR_CREDENTIALS_CONTRADICT_TO_HISTORY,
userIdent.getQualifiedUser(), userIdent.getHost());
}
+ if (retainCurrentPassword) {
+ // 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).
+ User user = userManager.getUserByUserIdentity(userIdent);
+ 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,
retainCurrentPassword);
if (password != null) {
// save password to password history
passwdPolicyManager.updatePassword(userIdent, password);
}
if (!isReplay) {
- PrivInfo info = new PrivInfo(userIdent, null, password, null,
null);
+ PrivInfo info = new PrivInfo(userIdent, null, password, null,
null, retainCurrentPassword, false);
+ Env.getCurrentEnv().getEditLog().logSetPassword(info);
+ }
+ } finally {
+ writeUnlock();
+ }
+ LOG.info("finished to set password for {}. is replay: {}, retain
current: {}",
+ userIdent, isReplay, retainCurrentPassword);
+ }
+
+ /**
+ * MySQL-compatible "ALTER USER ... DISCARD OLD PASSWORD": drop the
+ * retained secondary password.
+ *
+ * <p>Journaled via OP_SET_PASSWORD/PrivInfo with the discard flag and
+ * passwd = the account's CURRENT primary password, NOT via a new
+ * operation value on OP_ALTER_USER: an FE binary without this feature
+ * deserializes an unknown AlterUserOpType enum name as null and fails
+ * replay, whereas it replays this entry as a plain set-password to the
+ * value the account already holds — a harmless no-op. That keeps the
+ * journal readable by pre-feature binaries.
+ */
+ private void discardOldPasswordInternal(UserIdentity userIdent, boolean
isReplay) throws DdlException {
+ writeLock();
+ try {
+ User user = userManager.getUserByUserIdentity(userIdent);
+ if (user == null) {
+ throw new DdlException("user " + userIdent + " does not
exist");
+ }
+ userManager.discardOldPassword(userIdent, true);
+ if (!isReplay) {
+ byte[] currentPrimary = user.getPassword() == null ||
user.getPassword().getPassword() == null
+ ? new byte[0] : user.getPassword().getPassword();
+ PrivInfo info = new PrivInfo(userIdent, null, currentPrimary,
null, null, false, true);
Review Comment:
One remaining detail: on an older FE, replaying DISCARD as SET_PASSWORD
preserves the primary password value, but still appends password history and
resets expiration. The password-policy state therefore changes.
The rest looks good to me. Please also add regression tests for
dual-password rotation and discard. Sorry for the late review!
--
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]
