raghav-reglobe commented on PR #66115: URL: https://github.com/apache/doris/pull/66115#issuecomment-5108555787
Thanks @CalvinKirs — agreed on all counts. The "park your own password in the secondary slot" scenario is real, it survives later rotations exactly as you describe, and MySQL's manual is unambiguous that the clauses are privileged even on one's own account. I'd talked myself into "self SET PASSWORD is unprivileged, so RETAIN on it follows" — but RETAIN is not a password change, it's a persistence grant for a *second* credential, and that deserves its own gate. Updated the PR (head `1a5136ab05`): - `SetPassVarOp.validate()` now falls through to the global GRANT check when `RETAIN CURRENT PASSWORD` is present, including for one's own account. A plain self `SET PASSWORD` (no clause) stays unprivileged — unchanged behavior. `ALTER USER` was already covered by `AlterUserInfo.validate()` as you noted. - Took your inline fix too: the password change now builds the full `Password` (new primary + carried secondary) and swaps it in as one reference assignment, and `matchUserPassword` compares against a single snapshot — no window where a client on the old password gets rejected mid-rotation, no torn read across the two slots. - The PR description's "Deliberate differences" section is rewritten — it no longer claims self-service needs no privilege, and the semantics table now includes the MySQL privilege rule. - Tests: unprivileged self-RETAIN is rejected (and with GRANT it works), executed through the parsed command rather than direct `Auth` calls. On the finer-grained privilege: I agree it doesn't belong in this PR, but I do have the concrete delegation case you mentioned — low-privilege service accounts (BI tools and similar machine identities) that should rotate their own credential with an overlap window but must never hold GRANT, since GRANT is account-management power. Filed as #66198: an `APPLICATION_PASSWORD_ADMIN`-equivalent `Privilege` bit accepted (alongside GRANT/ADMIN) for own-account clauses only, plus the "who currently holds a secondary password" introspection you pointed out is missing — MySQL surfaces it in `mysql.user.User_attributes`, and the two belong together. Would you be open to that direction? While addressing this I also fixed the journal-compat issue the automated review flagged: `DISCARD OLD PASSWORD` no longer journals as a new `AlterUserOpType` on `OP_ALTER_USER` (an older FE fails replay on the unknown enum). It rides `OP_SET_PASSWORD` via `PrivInfo` with a discard flag and `passwd` = the unchanged primary, so a pre-feature binary replays it as a same-password set — a no-op. Downgrade after first use is safe now; the description's upgrade section is updated accordingly. -- 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]
