CalvinKirs commented on code in PR #67792:
URL: https://github.com/apache/doris/pull/67792#discussion_r4002993118
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AlterUserInfo.java:
##########
@@ -116,7 +116,7 @@ public void validate() throws UserException {
}
passwordOptions.analyze();
if (passwordOptions.getAccountUnlocked() ==
PasswordPolicy.FailedLoginPolicy.LOCK_ACCOUNT) {
- throw new AnalysisException("Not support lock account now");
+ ops.add(AlterUserOpType.LOCK_ACCOUNT);
Review Comment:
`ACCOUNT_LOCK` combined with other password options silently drops the
others.
```sql
ALTER USER 'u'@'%' FAILED_LOGIN_ATTEMPTS 3 PASSWORD_LOCK_TIME 60 SECOND
ACCOUNT_LOCK;
```
After this, `account_locked = true` but `num_failed_login` and
`password_lock_seconds` are still `DISABLED` (verified with a local FE test).
The `if / else if` chain stops at `LOCK_ACCOUNT`, so `SET_PASSWORD_POLICY` is
never added and the `ops.size() != 1` check passes. Before this PR the
statement at least failed with "Not support lock account now"; now it
half-succeeds with no error. `CREATE USER` with the same options applies all of
them, so the two commands disagree.
Suggest splitting the lock/unlock check and the policy check into two
independent `if`s, so the combination hits the existing "one operation at a
time" error.
--
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]
