Skip to content

fix: 🐛 improve password confirmation prompt (fixes #1390) #1418

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

fix: 🐛 improve password confirmation prompt (fixes #1390) #1418

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1bd78ae
fix: 🐛 improve password confirmation prompt (#1390)
srizzling Mar 24, 2025
51dae7b
Merge branch 'master' into fix-ambigous-errors
RossRichardson Apr 23, 2025
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
28 changes: 16 additions & 12 deletions cmd/saml2aws/commands/configure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,23 @@ func Configure(configFlags *flags.CommonFlags) error {
return errors.Wrap(err, "failed to input configuration")
}

if credentials.SupportsStorage() && idpAccountPassword == "" {
password := prompter.Password("Password")
if password != "" {
if confirmPassword := prompter.Password("Confirm"); confirmPassword == password {
idpAccountPassword = password
} else {
log.Println("Passwords did not match")
os.Exit(1)
}
} else {
log.Println("No password supplied")
}
if !credentials.SupportsStorage() || idpAccountPassword != "" {
return nil
}

password := prompter.Password("Enter password")
if password == "" {
log.Println("No password supplied")
return nil
}

repeat := prompter.Password("Re-enter password")
if repeat != password {
log.Println("Confirmed password does not match")
os.Exit(1)
}
Comment on lines +57 to +61
Copy link
Contributor

@pwmcintyre pwmcintyre Mar 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

is this a good idea? 💭

Suggested change
repeat := prompter.Password("Re-enter password")
if repeat != password {
log.Println("Confirmed password does not match")
os.Exit(1)
}
for {
if password == prompter.Password("Re-enter password") {
break
}
log.Println("Confirmed password does not match")
}

Copy link
Contributor

@pwmcintyre pwmcintyre Mar 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

spicy opinion: password repeaters are annoying

Suggested change
repeat := prompter.Password("Re-enter password")
if repeat != password {
log.Println("Confirmed password does not match")
os.Exit(1)
}

Copy link

Choose a reason for hiding this comment

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

Agree with repeat. For sign up: maybe? for logins: NO. let's delete?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its one time during configure operation.. so I guess if a user gets a password wrong.. we are just assuming they will just do it again.

Seems okay to me.


idpAccountPassword = password
}

if credentials.SupportsStorage() {
Expand Down
Loading