-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cli: ensure user set --password can also modify the password #28197
Conversation
@mberhault @bdarnell should we backport this to 2.0? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 1 stale)
pkg/sql/create_user.go
Outdated
} | ||
|
||
n.run.rowsAffected, err = params.extendedEvalCtx.ExecCfg.InternalExecutor.Exec( | ||
params.ctx, | ||
opName, | ||
params.p.txn, | ||
"insert into system.users values ($1, $2, $3)", | ||
`upsert into system.users(username, "hashedPassword", "isRole") values ($1, $2, $3)`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this isn't right, you can't use the CREATE USER
command to modify a user, that's what the ALTER USER
command is for.
Until now the flag `--password` could only set the password if the user did not exist previously (ie. during user creation). This patch fixes the feature so that `--password` can be used to change an existing user's password. Release note (bug fix): `cockroach user set --password` can now change the password of existing users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 1 stale)
pkg/sql/create_user.go, line 125 at r1 (raw file):
Previously, mberhault (marc) wrote…
this isn't right, you can't use the
CREATE USER
command to modify a user, that's what theALTER USER
command is for.
Oh I see.
Thanks for catching this. Fixed. PTAL.
LGTM. Thanks for that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👎 Rejected by code reviews |
bors r+ |
28197: cli: ensure user set --password can also modify the password r=knz a=knz Fixes #27882. Until now the flag `--password` could only set the password if the user did not exist previously (ie. during user creation). This patch fixes the feature so that `--password` can be used to change an existing user's password. Release note (bug fix): `cockroach user set --password` can now change the password of existing users. Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
Build succeeded |
Fixes #27882.
Until now the flag
--password
could only set the password if theuser did not exist previously (ie. during user creation). This patch
fixes the feature so that
--password
can be used to change anexisting user's password.
Release note (bug fix):
cockroach user set --password
can now changethe password of existing users.