raghav-reglobe commented on code in PR #67792:
URL: https://github.com/apache/doris/pull/67792#discussion_r4011503368


##########
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:
   Thanks, all three are right. Pushed as one follow-up commit on top:
   
   - **Every authenticator.** The lock is now enforced at the common finish in 
`AuthenticatorManager` (after whichever authenticator accepted the credential), 
plus on the LDAP branch of `Auth.checkPlainPassword`, which the Arrow Flight 
path uses. One shared predicate, `Auth.checkAccountLocked` → 
`PasswordPolicyManager.checkAccountLocked`, checks the administrative lock 
alone, so LDAP / integration / plugin logins to a locked Doris account get the 
same 3118 refusal; an identity without a Doris policy (an LDAP-only user) is 
untouched, and sessions already authenticated still are not terminated. 
`AuthenticatorManagerTest.testLockedAccountIsRefusedAfterAnyAuthenticatorAccepted`
 covers the finish step (refused with 3118, nothing applied to the context; an 
unlocked account finishes as before).
   - **Combined options.** Split into two independent checks as you suggested, 
so `ALTER USER ... FAILED_LOGIN_ATTEMPTS 3 PASSWORD_LOCK_TIME 60 SECOND 
ACCOUNT_LOCK` now hits the existing one-operation error and nothing is applied; 
same for `ACCOUNT_UNLOCK`. 
`AccountLockTest.testLockCombinedWithPolicyOptionsIsRejectedNotHalfApplied` 
asserts the policy values stay `DISABLED` and the account stays unlocked; the 
regression suite has the statement too.
   - **root.** `ALTER USER root ACCOUNT_LOCK` is refused with "Can not lock 
root user", next to the existing root-only check in `AlterUserInfo.validate`. 
Unit and regression cases added.



-- 
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]

Reply via email to