Skip to content
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

[STORY-642] new api endpoint to handle password reset #1071

Merged
Merged
Changes from 1 commit
Commits
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
Next Next commit
Database user password update uses new API endpoint for generating pa…
…ssword.

This is dependent on the merging / release of the equivalent branch in go-scalingo library.
sc-zenokerr committed Jul 4, 2024
commit db2e8ecd5d56e3b8b19121caf987a528b02f2456
2 changes: 1 addition & 1 deletion cmd/databases.go
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@
"github.com/urfave/cli/v2"

"github.com/Scalingo/cli/db"
dbUsers "github.com/Scalingo/cli/db/users"

Check failure on line 13 in cmd/databases.go

GitHub Actions / golangci-lint on a PR or from a tag

could not import github.com/Scalingo/cli/db/users (-: # github.com/Scalingo/cli/db/users
"github.com/Scalingo/cli/detect"
"github.com/Scalingo/cli/utils"
"github.com/Scalingo/go-scalingo/v7"
@@ -272,7 +272,7 @@

username := c.Args().First()

err := dbUsers.UpdateUser(c.Context, currentApp, addonName, username)
err := dbUsers.UpdateUserPassword(c.Context, currentApp, addonName, username)
if err != nil {
errorQuit(c.Context, err)
}
37 changes: 21 additions & 16 deletions db/users/update.go → db/users/update_password.go
Original file line number Diff line number Diff line change
@@ -8,10 +8,9 @@
"github.com/Scalingo/cli/io"
"github.com/Scalingo/go-scalingo/v7"
"github.com/Scalingo/go-utils/errors/v2"
"github.com/Scalingo/gopassword"
)

func UpdateUser(ctx context.Context, app, addonUUID, username string) error {
func UpdateUserPassword(ctx context.Context, app, addonUUID, username string) error {
isSupported, err := doesDatabaseHandleUserManagement(ctx, app, addonUUID)
if err != nil {
return errors.Wrap(ctx, err, "get user management information")
@@ -60,25 +59,31 @@
isPasswordGenerated := false
if password == "" {
isPasswordGenerated = true
password = gopassword.Generate(64)
confirmedPassword = password
}

userUpdateParam := scalingo.DatabaseUpdateUserParam{
DatabaseID: addonUUID,
Password: password,
PasswordConfirmation: confirmedPassword,
}
databaseUsers, err := c.DatabaseUpdateUser(ctx, app, addonUUID, username, userUpdateParam)
if err != nil {
return errors.Wrap(ctx, err, "update password of the given database user")
}
var databaseUser scalingo.DatabaseUser

// We have two different API calls here to avoid breaking backwards compatibility of the CLI
if !isPasswordGenerated {
userUpdateParam := scalingo.DatabaseUpdateUserParam{
DatabaseID: addonUUID,
Password: password,
PasswordConfirmation: confirmedPassword,
}
databaseUser, err = c.DatabaseUpdateUser(ctx, app, addonUUID, username, userUpdateParam)
if err != nil {
return errors.Wrap(ctx, err, "update password of the given database user")
}
} else {
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: I like returning the function as soon as possible. In this case, you could return here and the else is useless. There is less indentation which makes the code easier to read IMO

Suggested change
}
} else {
}
fmt.Printf("User \"%s\" password updated.\n", databaseUser.Name)
return nil
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I actually misread the original flow. Your suggestion is absolutely correct. Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you ask product about the CLI backward compatibility?

Yes. My question was answered by benjamin in the comments in the [STORY-642] in notion.

It should be (remain) possible to set a user-supplied password using the CLI.

databaseUser, err = c.DatabaseUserPasswordReset(ctx, app, addonUUID, username)

Check failure on line 78 in db/users/update_password.go

GitHub Actions / golangci-lint on a PR or from a tag

c.DatabaseUserPasswordReset undefined (type *scalingo.Client has no field or method DatabaseUserPasswordReset) (typecheck)

Check failure on line 78 in db/users/update_password.go

GitHub Actions / golangci-lint on a PR or from a tag

c.DatabaseUserPasswordReset undefined (type *scalingo.Client has no field or method DatabaseUserPasswordReset)) (typecheck)

Check failure on line 78 in db/users/update_password.go

GitHub Actions / Unit Tests

c.DatabaseUserPasswordReset undefined (type *scalingo.Client has no field or method DatabaseUserPasswordReset)
if err != nil {
return errors.Wrap(ctx, err, "reset the password of the given database user")
}

if isPasswordGenerated {
fmt.Printf("User \"%s\" updated with password \"%s\".\n", databaseUsers.Name, password)
fmt.Printf("User \"%s\" updated with password \"%s\".\n", databaseUser.Name, databaseUser.Password)
return nil
}

fmt.Printf("User \"%s\" password updated.\n", databaseUsers.Name)
fmt.Printf("User \"%s\" password updated.\n", databaseUser.Name)
return nil
}