Skip to content

feat: Add reset-password command #1380

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
May 12, 2022
Merged

feat: Add reset-password command #1380

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b080572
allow non-destructively checking if database needs to be migrated
dwahler May 10, 2022
18de0d2
feat: Add reset-password command
dwahler May 10, 2022
bfa835d
fix linter errors
dwahler May 10, 2022
907f457
clean up reset-password usage prompt
dwahler May 11, 2022
f6f2310
Add confirmation to reset-password command
dwahler May 12, 2022
cb36912
Ping database before checking migration, to improve error message
dwahler May 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add confirmation to reset-password command
  • Loading branch information
@dwahler
dwahler committed May 12, 2022
commit f6f2310d5418ef5944deafc15bf2e83bc9551344
11 changes: 11 additions & 0 deletions cli/resetpassword.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,17 @@ func resetPassword() *cobra.Command {
if err != nil {
return xerrors.Errorf("password prompt: %w", err)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a confirm step, similar to create first user?

coder/cli/login.go

Lines 121 to 133 in 9d94f4f

_, err = cliui.Prompt(cmd, cliui.PromptOptions{
Text: "Confirm " + cliui.Styles.Field.Render("password") + ":",
Secret: true,
Validate: func(s string) error {
if s != password {
return xerrors.Errorf("Passwords do not match")
}
return nil
},
})
if err != nil {
return xerrors.Errorf("confirm password prompt: %w", err)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question.

Personally, I think the value of a password confirmation step is in making it harder to accidentally lock yourself out by changing your own password to something with a typo in it. I understand why it's helpful in the "create first user" flow, since you can only do that once, but if you make a typo when resetting a password it's easy to just re-run the command and fix it, since you're assumed to have direct access to the DB.

On the other hand, I can see the argument for just doing the confirmation anyway, for consistency with "create first user" and with UNIX passwd.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I agree that it's a bit redundant since you can just change it again.

My thought was both for uniformity and that the repetition increases the chance of typing the correct password, which could help when the user hops onto e.g. the webui and tries to login. If they're very confident they typed in the right password on the cli, but can't login on the web, that could lead to a frustrating experience.

I think either approach is fine though so I'll leave it up to you. 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense. I added a confirmation prompt, but I deviated from the behavior of coder login slightly in that if the passwords don't match, we just exit instead of repeating the second prompt. This matches the behavior of the passwd command, and I think it makes more sense because if you make a typo, you probably don't know whether it's the first or second input that's correct.

confirmedPassword, err := cliui.Prompt(cmd, cliui.PromptOptions{
Text: "Confirm " + cliui.Styles.Field.Render("password") + ":",
Secret: true,
Validate: cliui.ValidateNotEmpty,
})
if err != nil {
return xerrors.Errorf("confirm password prompt: %w", err)
}
if password != confirmedPassword {
return xerrors.New("Passwords do not match")
}

hashedPassword, err := userpassword.Hash(password)
if err != nil {
Expand Down
24 changes: 15 additions & 9 deletions cli/resetpassword_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ func TestResetPassword(t *testing.T) {
t.SkipNow()
}

const email = "some@one.com"
const username = "example"
const oldPassword = "password"
const newPassword = "password2"

// start postgres and coder server processes

connectionURL, closeFunc, err := postgres.Open()
Expand All @@ -48,16 +53,16 @@ func TestResetPassword(t *testing.T) {
return true
}, 15*time.Second, 25*time.Millisecond)
_, err = client.CreateFirstUser(ctx, codersdk.CreateFirstUserRequest{
Email: "some@one.com",
Username: "example",
Password: "password",
Email: email,
Username: username,
Password: oldPassword,
OrganizationName: "example",
})
require.NoError(t, err)

// reset the password

resetCmd, cmdCfg := clitest.New(t, "reset-password", "--postgres-url", connectionURL, "example")
resetCmd, cmdCfg := clitest.New(t, "reset-password", "--postgres-url", connectionURL, username)
clitest.SetupConfig(t, client, cmdCfg)
cmdDone := make(chan struct{})
pty := ptytest.New(t)
Expand All @@ -73,7 +78,8 @@ func TestResetPassword(t *testing.T) {
output string
input string
}{
{"password", "password2"},
{"Enter new", newPassword},
{"Confirm", newPassword},
}
for _, match := range matches {
pty.ExpectMatch(match.output)
Expand All @@ -84,14 +90,14 @@ func TestResetPassword(t *testing.T) {
// now try logging in

_, err = client.LoginWithPassword(ctx, codersdk.LoginWithPasswordRequest{
Email: "some@one.com",
Password: "password",
Email: email,
Password: oldPassword,
})
require.Error(t, err)

_, err = client.LoginWithPassword(ctx, codersdk.LoginWithPasswordRequest{
Email: "some@one.com",
Password: "password2",
Email: email,
Password: newPassword,
})
require.NoError(t, err)

Expand Down