From ce1a060338229c2ff3ec9b5b5f529204b2287e1b Mon Sep 17 00:00:00 2001 From: Michael Christenson II Date: Mon, 4 Dec 2023 11:04:51 -0500 Subject: [PATCH] Updates for code review Ensure valid id before deletion process Normalize messages for deletion Remove unnecessary path escapes Continue on read failure instead of attempting delete Inform user of failed id on batch deletion Only document at most two ids likely --- internal/cli/actions.go | 7 ++++--- internal/cli/apis.go | 18 ++++++++++-------- internal/cli/apps.go | 19 ++++++++++--------- internal/cli/custom_domains.go | 19 ++++++++++--------- internal/cli/log_streams.go | 20 +++++++++++--------- internal/cli/organizations.go | 18 ++++++++++-------- internal/cli/roles.go | 18 ++++++++++-------- internal/cli/rules.go | 18 ++++++++++-------- internal/cli/users.go | 18 ++++++++++-------- internal/cli/users_blocks.go | 9 +++++---- 10 files changed, 90 insertions(+), 74 deletions(-) diff --git a/internal/cli/actions.go b/internal/cli/actions.go index e4bbea73e..8da2bc8bf 100644 --- a/internal/cli/actions.go +++ b/internal/cli/actions.go @@ -361,7 +361,6 @@ func deleteActionCmd(cli *cli) *cobra.Command { cmd := &cobra.Command{ Use: "delete", Aliases: []string{"rm"}, - Args: cobra.MinimumNArgs(0), Short: "Delete an action", Long: "Delete an action.\n\n" + "To delete interactively, use `auth0 actions delete` with no arguments.\n\n" + @@ -391,8 +390,10 @@ func deleteActionCmd(cli *cli) *cobra.Command { return ansi.Spinner("Deleting action", func() error { var errs []error for _, id := range ids { - if err := cli.api.Action.Delete(cmd.Context(), id); err != nil { - errs = append(errs, err) + if id != "" { + if err := cli.api.Action.Delete(cmd.Context(), id); err != nil { + errs = append(errs, err) + } } } return errors.Join(errs...) diff --git a/internal/cli/apis.go b/internal/cli/apis.go index 1b0c42aca..d7aa66a5a 100644 --- a/internal/cli/apis.go +++ b/internal/cli/apis.go @@ -419,7 +419,6 @@ func deleteAPICmd(cli *cli) *cobra.Command { cmd := &cobra.Command{ Use: "delete", Aliases: []string{"rm"}, - Args: cobra.MinimumNArgs(0), Short: "Delete an API", Long: "Delete an API.\n\n" + "To delete interactively, use `auth0 apis delete` with no arguments.\n\n" + @@ -446,15 +445,18 @@ func deleteAPICmd(cli *cli) *cobra.Command { } } - return ansi.Spinner("Deleting API", func() error { + return ansi.Spinner("Deleting API(s)", func() error { var errs []error for _, id := range ids { - if _, err := cli.api.ResourceServer.Read(cmd.Context(), url.PathEscape(id)); err != nil { - errs = append(errs, fmt.Errorf("Unable to read API for deletion: %w", err)) - } - - if err := cli.api.ResourceServer.Delete(cmd.Context(), url.PathEscape(id)); err != nil { - errs = append(errs, fmt.Errorf("Unable to delete API: %w", err)) + if id != "" { + if _, err := cli.api.ResourceServer.Read(cmd.Context(), url.PathEscape(id)); err != nil { + errs = append(errs, fmt.Errorf("Unable to delete API (%s): %w", id, err)) + continue + } + + if err := cli.api.ResourceServer.Delete(cmd.Context(), url.PathEscape(id)); err != nil { + errs = append(errs, fmt.Errorf("Unable to delete API (%s): %w", id, err)) + } } } return errors.Join(errs...) diff --git a/internal/cli/apps.go b/internal/cli/apps.go index be7efac8b..f900ff1d2 100644 --- a/internal/cli/apps.go +++ b/internal/cli/apps.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "net/url" "strings" "github.com/auth0/go-auth0/management" @@ -306,7 +305,6 @@ func deleteAppCmd(cli *cli) *cobra.Command { cmd := &cobra.Command{ Use: "delete", Aliases: []string{"rm"}, - Args: cobra.MinimumNArgs(0), Short: "Delete an application", Long: "Delete an application.\n\n" + "To delete interactively, use `auth0 apps delete` with no arguments.\n\n" + @@ -335,15 +333,18 @@ func deleteAppCmd(cli *cli) *cobra.Command { } } - return ansi.Spinner("Deleting Application", func() error { + return ansi.Spinner("Deleting Application(s)", func() error { var errs []error for _, id := range ids { - if _, err := cli.api.Client.Read(cmd.Context(), url.PathEscape(id)); err != nil { - errs = append(errs, fmt.Errorf("Unable to read application for deletion: %w", err)) - } - - if err := cli.api.Client.Delete(cmd.Context(), url.PathEscape(id)); err != nil { - errs = append(errs, fmt.Errorf("Unable to delete application: %w", err)) + if id != "" { + if _, err := cli.api.Client.Read(cmd.Context(), id); err != nil { + errs = append(errs, fmt.Errorf("Unable to delete application (%s): %w", id, err)) + continue + } + + if err := cli.api.Client.Delete(cmd.Context(), id); err != nil { + errs = append(errs, fmt.Errorf("Unable to delete application (%s): %w", id, err)) + } } } return errors.Join(errs...) diff --git a/internal/cli/custom_domains.go b/internal/cli/custom_domains.go index f71d5d7b4..e36e234cc 100644 --- a/internal/cli/custom_domains.go +++ b/internal/cli/custom_domains.go @@ -327,7 +327,6 @@ func deleteCustomDomainCmd(cli *cli) *cobra.Command { cmd := &cobra.Command{ Use: "delete", Aliases: []string{"rm"}, - Args: cobra.MinimumNArgs(0), Short: "Delete a custom domain", Long: "Delete a custom domain.\n\n" + "To delete interactively, use `auth0 domains delete` with no arguments.\n\n" + @@ -337,8 +336,8 @@ func deleteCustomDomainCmd(cli *cli) *cobra.Command { auth0 domains rm auth0 domains delete auth0 domains delete --force - auth0 domains delete - auth0 domains delete --force`, + auth0 domains delete + auth0 domains delete --force`, RunE: func(cmd *cobra.Command, args []string) error { ids := make([]string, len(args)) if len(args) == 0 { @@ -359,12 +358,14 @@ func deleteCustomDomainCmd(cli *cli) *cobra.Command { return ansi.Spinner("Deleting custom domain", func() error { var errs []error for _, id := range ids { - if _, err := cli.api.CustomDomain.Read(cmd.Context(), url.PathEscape(id)); err != nil { - return fmt.Errorf("Unable to read custom domain for deletion: %w", err) - } - - if err := cli.api.CustomDomain.Delete(cmd.Context(), url.PathEscape(id)); err != nil { - return fmt.Errorf("Unable to delete custom domain: %w", err) + if id != "" { + if _, err := cli.api.CustomDomain.Read(cmd.Context(), url.PathEscape(id)); err != nil { + return fmt.Errorf("Unable to delete custom domain (%s): %w", id, err) + } + + if err := cli.api.CustomDomain.Delete(cmd.Context(), url.PathEscape(id)); err != nil { + return fmt.Errorf("Unable to delete custom domain (%s): %w", id, err) + } } } diff --git a/internal/cli/log_streams.go b/internal/cli/log_streams.go index cc5e6cec5..d33e92247 100644 --- a/internal/cli/log_streams.go +++ b/internal/cli/log_streams.go @@ -171,7 +171,6 @@ func deleteLogStreamCmd(cli *cli) *cobra.Command { cmd := &cobra.Command{ Use: "delete", Aliases: []string{"rm"}, - Args: cobra.MinimumNArgs(0), Short: "Delete a log stream", Long: "Delete a log stream.\n\n" + "To delete interactively, use `auth0 logs streams delete` with no arguments.\n\n" + @@ -181,8 +180,8 @@ func deleteLogStreamCmd(cli *cli) *cobra.Command { auth0 logs streams rm auth0 logs streams delete auth0 logs streams delete --force - auth0 logs streams delete - auth0 logs streams delete --force`, + auth0 logs streams delete + auth0 logs streams delete --force`, RunE: func(cmd *cobra.Command, args []string) error { ids := make([]string, len(args)) if len(args) == 0 { @@ -200,14 +199,17 @@ func deleteLogStreamCmd(cli *cli) *cobra.Command { } } - return ansi.Spinner("Deleting Log Stream", func() error { + return ansi.Spinner("Deleting Log Stream(s)", func() error { var errs []error for _, id := range ids { - if _, err := cli.api.LogStream.Read(cmd.Context(), id); err != nil { - errs = append(errs, fmt.Errorf("Unable to read log stream for deletion: %w", err)) - } - if err := cli.api.LogStream.Delete(cmd.Context(), id); err != nil { - errs = append(errs, fmt.Errorf("Unable to delete log stream: %w", err)) + if id != "" { + if _, err := cli.api.LogStream.Read(cmd.Context(), id); err != nil { + errs = append(errs, fmt.Errorf("Unable to delete log stream (%s): %w", id, err)) + continue + } + if err := cli.api.LogStream.Delete(cmd.Context(), id); err != nil { + errs = append(errs, fmt.Errorf("Unable to delete log stream (%s): %w", id, err)) + } } } diff --git a/internal/cli/organizations.go b/internal/cli/organizations.go index 03fcd171a..b39e7e5e2 100644 --- a/internal/cli/organizations.go +++ b/internal/cli/organizations.go @@ -411,7 +411,6 @@ func deleteOrganizationCmd(cli *cli) *cobra.Command { cmd := &cobra.Command{ Use: "delete", Aliases: []string{"rm"}, - Args: cobra.MinimumNArgs(0), Short: "Delete an organization", Long: "Delete an organization.\n\n" + "To delete interactively, use `auth0 orgs delete` with no arguments.\n\n" + @@ -440,15 +439,18 @@ func deleteOrganizationCmd(cli *cli) *cobra.Command { } } - return ansi.Spinner("Deleting organization", func() error { + return ansi.Spinner("Deleting organization(s)", func() error { var errs []error for _, id := range ids { - if _, err := cli.api.Organization.Read(cmd.Context(), url.PathEscape(id)); err != nil { - errs = append(errs, fmt.Errorf("Unable to read organization for deletion: %w", err)) - } - - if err := cli.api.Organization.Delete(cmd.Context(), url.PathEscape(id)); err != nil { - errs = append(errs, fmt.Errorf("Unable to delete organization: %w", err)) + if id != "" { + if _, err := cli.api.Organization.Read(cmd.Context(), id); err != nil { + errs = append(errs, fmt.Errorf("Unable to delete organization (%s): %w", id, err)) + continue + } + + if err := cli.api.Organization.Delete(cmd.Context(), id); err != nil { + errs = append(errs, fmt.Errorf("Unable to delete organization (%s): %w", id, err)) + } } } return errors.Join(errs...) diff --git a/internal/cli/roles.go b/internal/cli/roles.go index 10c972f5b..e63091e55 100644 --- a/internal/cli/roles.go +++ b/internal/cli/roles.go @@ -285,7 +285,6 @@ func deleteRoleCmd(cli *cli) *cobra.Command { cmd := &cobra.Command{ Use: "delete", Aliases: []string{"rm"}, - Args: cobra.MinimumNArgs(0), Short: "Delete a role", Long: "Delete a role.\n\n" + "To delete interactively, use `auth0 roles delete`.\n\n" + @@ -313,15 +312,18 @@ func deleteRoleCmd(cli *cli) *cobra.Command { } } - return ansi.Spinner("Deleting Role", func() error { + return ansi.Spinner("Deleting Role(s)", func() error { var errs []error for _, id := range ids { - if _, err := cli.api.Role.Read(cmd.Context(), id); err != nil { - errs = append(errs, fmt.Errorf("Unable to read role for deletion: %w", err)) - } - - if err := cli.api.Role.Delete(cmd.Context(), id); err != nil { - errs = append(errs, fmt.Errorf("Unable to delete role: %w", err)) + if id != "" { + if _, err := cli.api.Role.Read(cmd.Context(), id); err != nil { + errs = append(errs, fmt.Errorf("Unable to delete role (%s): %w", id, err)) + continue + } + + if err := cli.api.Role.Delete(cmd.Context(), id); err != nil { + errs = append(errs, fmt.Errorf("Unable to delete role (%s): %w", id, err)) + } } } return errors.Join(errs...) diff --git a/internal/cli/rules.go b/internal/cli/rules.go index 3af86aca4..5a317d2cb 100644 --- a/internal/cli/rules.go +++ b/internal/cli/rules.go @@ -252,7 +252,6 @@ func deleteRuleCmd(cli *cli) *cobra.Command { cmd := &cobra.Command{ Use: "delete", Aliases: []string{"rm"}, - Args: cobra.MinimumNArgs(0), Short: "Delete a rule", Long: rulesDeprecationDocumentationText + "Delete a rule.\n\n" + "To delete interactively, use `auth0 rules delete` with no arguments.\n\n" + @@ -280,15 +279,18 @@ func deleteRuleCmd(cli *cli) *cobra.Command { } } - return ansi.Spinner("Deleting Rule", func() error { + return ansi.Spinner("Deleting Rule(s)", func() error { var errs []error for _, id := range ids { - if _, err := cli.api.Rule.Read(cmd.Context(), id); err != nil { - errs = append(errs, fmt.Errorf("Unable to read rule for deletion: %w", err)) - } - - if err := cli.api.Rule.Delete(cmd.Context(), id); err != nil { - errs = append(errs, fmt.Errorf("Unable to delete rule: %w", err)) + if id != "" { + if _, err := cli.api.Rule.Read(cmd.Context(), id); err != nil { + errs = append(errs, fmt.Errorf("Unable to delete rule (%s): %w", id, err)) + continue + } + + if err := cli.api.Rule.Delete(cmd.Context(), id); err != nil { + errs = append(errs, fmt.Errorf("Unable to delete rule (%s): %w", id, err)) + } } } return errors.Join(errs...) diff --git a/internal/cli/users.go b/internal/cli/users.go index 8ef64c6e1..2f458a2be 100644 --- a/internal/cli/users.go +++ b/internal/cli/users.go @@ -360,7 +360,6 @@ func deleteUserCmd(cli *cli) *cobra.Command { cmd := &cobra.Command{ Use: "delete", Aliases: []string{"rm"}, - Args: cobra.MinimumNArgs(0), Short: "Delete a user", Long: "Delete a user.\n\n" + "To delete interactively, use `auth0 users delete` with no arguments.\n\n" + @@ -389,15 +388,18 @@ func deleteUserCmd(cli *cli) *cobra.Command { } } - return ansi.Spinner("Deleting user", func() error { + return ansi.Spinner("Deleting user(s)", func() error { var errs []error for _, id := range ids { - if _, err := cli.api.User.Read(cmd.Context(), id); err != nil { - errs = append(errs, fmt.Errorf("Unable to read user for deletion: %w", err)) - } - - if err := cli.api.User.Delete(cmd.Context(), id); err != nil { - errs = append(errs, fmt.Errorf("Unable to delete user: %w", err)) + if id != "" { + if _, err := cli.api.User.Read(cmd.Context(), id); err != nil { + errs = append(errs, fmt.Errorf("Unable to delete user (%s): %w", id, err)) + continue + } + + if err := cli.api.User.Delete(cmd.Context(), id); err != nil { + errs = append(errs, fmt.Errorf("Unable to delete user (%s): %w", id, err)) + } } } return errors.Join(errs...) diff --git a/internal/cli/users_blocks.go b/internal/cli/users_blocks.go index e83aab07d..223452812 100644 --- a/internal/cli/users_blocks.go +++ b/internal/cli/users_blocks.go @@ -67,7 +67,6 @@ func listUserBlocksCmd(cli *cli) *cobra.Command { func deleteUserBlocksCmd(cli *cli) *cobra.Command { cmd := &cobra.Command{ Use: "unblock", - Args: cobra.MinimumNArgs(0), Short: "Remove brute-force protection blocks for a given user", Long: "Remove brute-force protection blocks for a given user.", Example: ` auth0 users blocks unblock @@ -85,11 +84,13 @@ func deleteUserBlocksCmd(cli *cli) *cobra.Command { ids = append(ids, args...) } - return ansi.Spinner("Unblocking user...", func() error { + return ansi.Spinner("Unblocking user(s)...", func() error { var errs []error for _, id := range ids { - if err := cli.api.User.Unblock(cmd.Context(), id); err != nil { - errs = append(errs, fmt.Errorf("failed to unblock user with ID %s: %w", id, err)) + if id != "" { + if err := cli.api.User.Unblock(cmd.Context(), id); err != nil { + errs = append(errs, fmt.Errorf("failed to unblock user with ID %s: %w", id, err)) + } } } return errors.Join(errs...)