CalvinKirs commented on PR #67792: URL: https://github.com/apache/doris/pull/67792#issuecomment-5676714423
Thanks, the follow-up looks good. Ran AccountLockTest / AuthenticatorManagerTest / AlterUserCommandTest locally, all green. Two leftovers: - `authenticate()` still returns early on `certDecision.shouldSkipPasswordVerification()`, before `finishSuccessfulAuthentication`, so a cert-only login never sees the lock. That early return predates this PR, but now that the comment says "after ANY authenticator" it should cover this one too — a `checkAccountLocked` call there is enough. - `Auth.checkPlainPassword` checks `currentUser.get(size - 1)`; every caller takes `get(0)`. Let's use `get(0)`. And the PR body is a bit behind the code now: the Design section still says enforcement is password-only, an older FE replaying LOCK_ACCOUNT logs an ERROR and stays unlocked (not "without failing"), and the release note should mention that `ACCOUNT_UNLOCK` combined with policy options now errors instead of half-applying. -- 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]
