From da131fe5521191050996a73258c5689d1be4cb5e Mon Sep 17 00:00:00 2001 From: Rajat Bajaj Date: Tue, 13 Aug 2024 11:15:38 +0530 Subject: [PATCH] Added new login flow. Respect domain flag on auth0 login command. (#1038) * Added use case to ensure if domain if passed in cli, it's populated to device login flow Signed-off-by: Rajat Bajaj * Updated login flow, improved validations, added relevant comments. * minor update to flow * Added test cases for new login flow * Updated auth_test GetDeviceCode func call * Convert if-else clause to switch case * Fixed linitng issues * Bump golang.org/x/net from 0.26.0 to 0.27.0 (#1035) Bumps [golang.org/x/net](https://github.com/golang/net) from 0.26.0 to 0.27.0. - [Commits](https://github.com/golang/net/compare/v0.26.0...v0.27.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump github.com/auth0/go-auth0 from 1.7.0 to 1.8.0 (#1036) Bumps [github.com/auth0/go-auth0](https://github.com/auth0/go-auth0) from 1.7.0 to 1.8.0. - [Release notes](https://github.com/auth0/go-auth0/releases) - [Changelog](https://github.com/auth0/go-auth0/blob/main/CHANGELOG.md) - [Commits](https://github.com/auth0/go-auth0/compare/v1.7.0...v1.8.0) --- updated-dependencies: - dependency-name: github.com/auth0/go-auth0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump rexml from 3.2.8 to 3.3.2 in /docs (#1041) Bumps [rexml](https://github.com/ruby/rexml) from 3.2.8 to 3.3.2. - [Release notes](https://github.com/ruby/rexml/releases) - [Changelog](https://github.com/ruby/rexml/blob/master/NEWS.md) - [Commits](https://github.com/ruby/rexml/compare/v3.2.8...v3.3.2) --- updated-dependencies: - dependency-name: rexml dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump github.com/lestrrat-go/jwx from 1.2.29 to 1.2.30 (#1042) Bumps [github.com/lestrrat-go/jwx](https://github.com/lestrrat-go/jwx) from 1.2.29 to 1.2.30. - [Release notes](https://github.com/lestrrat-go/jwx/releases) - [Changelog](https://github.com/lestrrat-go/jwx/blob/v1.2.30/Changes) - [Commits](https://github.com/lestrrat-go/jwx/compare/v1.2.29...v1.2.30) --- updated-dependencies: - dependency-name: github.com/lestrrat-go/jwx dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Update codeowner file with new GitHub team name (#1039) * Converted sensitive value to env vars * Bump github.com/schollz/progressbar/v3 from 3.14.4 to 3.14.5 (#1043) Bumps [github.com/schollz/progressbar/v3](https://github.com/schollz/progressbar) from 3.14.4 to 3.14.5. - [Release notes](https://github.com/schollz/progressbar/releases) - [Commits](https://github.com/schollz/progressbar/compare/v3.14.4...v3.14.5) --- updated-dependencies: - dependency-name: github.com/schollz/progressbar/v3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Removed a positive case * Bump rexml from 3.3.2 to 3.3.3 in /docs (#1044) Bumps [rexml](https://github.com/ruby/rexml) from 3.3.2 to 3.3.3. - [Release notes](https://github.com/ruby/rexml/releases) - [Changelog](https://github.com/ruby/rexml/blob/master/NEWS.md) - [Commits](https://github.com/ruby/rexml/compare/v3.3.2...v3.3.3) --- updated-dependencies: - dependency-name: rexml dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump github.com/hashicorp/hc-install from 0.7.0 to 0.8.0 (#1045) Bumps [github.com/hashicorp/hc-install](https://github.com/hashicorp/hc-install) from 0.7.0 to 0.8.0. - [Release notes](https://github.com/hashicorp/hc-install/releases) - [Commits](https://github.com/hashicorp/hc-install/compare/v0.7.0...v0.8.0) --- updated-dependencies: - dependency-name: github.com/hashicorp/hc-install dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump golang.org/x/net from 0.26.0 to 0.27.0 (#1035) Bumps [golang.org/x/net](https://github.com/golang/net) from 0.26.0 to 0.27.0. - [Commits](https://github.com/golang/net/compare/v0.26.0...v0.27.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump golang.org/x/net from 0.26.0 to 0.27.0 (#1035) Bumps [golang.org/x/net](https://github.com/golang/net) from 0.26.0 to 0.27.0. - [Commits](https://github.com/golang/net/compare/v0.26.0...v0.27.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump golang.org/x/net from 0.26.0 to 0.27.0 (#1035) Bumps [golang.org/x/net](https://github.com/golang/net) from 0.26.0 to 0.27.0. - [Commits](https://github.com/golang/net/compare/v0.26.0...v0.27.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * rebased branch with main --------- Signed-off-by: Rajat Bajaj Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: stevenwong-okta --- internal/auth/auth.go | 4 +- internal/auth/auth_test.go | 4 +- internal/cli/cli.go | 8 +-- internal/cli/flags.go | 2 +- internal/cli/login.go | 107 +++++++++++++++++++++++++++++++------ internal/cli/login_test.go | 55 +++++++++++++++++++ 6 files changed, 152 insertions(+), 28 deletions(-) create mode 100644 internal/cli/login_test.go diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 2122e78b1..5e354133d 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -144,13 +144,13 @@ var RequiredScopes = []string{ // GetDeviceCode kicks-off the device authentication flow by requesting // a device code from Auth0. The returned state contains the // URI for the next step of the flow. -func GetDeviceCode(ctx context.Context, httpClient *http.Client, additionalScopes []string) (State, error) { +func GetDeviceCode(ctx context.Context, httpClient *http.Client, additionalScopes []string, domain string) (State, error) { a := credentials data := url.Values{ "client_id": []string{a.ClientID}, "scope": []string{strings.Join(append(RequiredScopes, additionalScopes...), " ")}, - "audience": []string{a.Audience}, + "audience": []string{domain}, } request, err := http.NewRequestWithContext( diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index ba63540f8..f4bd1fff2 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -132,7 +132,7 @@ func TestGetDeviceCode(t *testing.T) { u := url.URL{Scheme: "https", Host: parsedURL.Host, Path: "/oauth/device/code"} credentials.DeviceCodeEndpoint = u.String() - state, err := GetDeviceCode(context.Background(), ts.Client(), []string{}) + state, err := GetDeviceCode(context.Background(), ts.Client(), []string{}, "") assert.NoError(t, err) assert.Equal(t, "device-code-here", state.DeviceCode) @@ -180,7 +180,7 @@ func TestGetDeviceCode(t *testing.T) { u := url.URL{Scheme: "https", Host: parsedURL.Host, Path: "/oauth/device/code"} credentials.DeviceCodeEndpoint = u.String() - _, err = GetDeviceCode(context.Background(), ts.Client(), []string{}) + _, err = GetDeviceCode(context.Background(), ts.Client(), []string{}, "") assert.EqualError(t, err, testCase.expect) }) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index b374a4555..eb9dfc9aa 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -73,7 +73,7 @@ func (c *cli) setupWithAuthentication(ctx context.Context) error { switch err { case config.ErrTokenMissingRequiredScopes: c.renderer.Warnf("Required scopes have changed. Please log in to re-authorize the CLI.\n") - tenant, err = RunLoginAsUser(ctx, c, tenant.GetExtraRequestedScopes()) + tenant, err = RunLoginAsUser(ctx, c, tenant.GetExtraRequestedScopes(), "") if err != nil { return err } @@ -95,7 +95,7 @@ func (c *cli) setupWithAuthentication(ctx context.Context) error { c.renderer.Warnf("Failed to renew access token: %s", err) c.renderer.Warnf("Please log in to re-authorize the CLI.\n") - tenant, err = RunLoginAsUser(ctx, c, tenant.GetExtraRequestedScopes()) + tenant, err = RunLoginAsUser(ctx, c, tenant.GetExtraRequestedScopes(), "") if err != nil { return err } @@ -136,10 +136,6 @@ func canPrompt(cmd *cobra.Command) bool { return iostream.IsInputTerminal() && iostream.IsOutputTerminal() && !noInput } -func shouldPrompt(cmd *cobra.Command, flag *Flag) bool { - return canPrompt(cmd) && !flag.IsSet(cmd) -} - func shouldPromptWhenNoLocalFlagsSet(cmd *cobra.Command) bool { localFlagIsSet := false cmd.LocalFlags().VisitAll(func(f *pflag.Flag) { diff --git a/internal/cli/flags.go b/internal/cli/flags.go index acea7d85f..b81e32b29 100644 --- a/internal/cli/flags.go +++ b/internal/cli/flags.go @@ -287,7 +287,7 @@ func shouldAsk(cmd *cobra.Command, f *Flag, isUpdate bool) bool { return shouldPromptWhenNoLocalFlagsSet(cmd) } - return shouldPrompt(cmd, f) + return canPrompt(cmd) && !f.IsSet(cmd) } func markFlagRequired(cmd *cobra.Command, f *Flag, isUpdate bool) error { diff --git a/internal/cli/login.go b/internal/cli/login.go index 527371e69..e1824ab4c 100644 --- a/internal/cli/login.go +++ b/internal/cli/login.go @@ -57,10 +57,6 @@ type LoginInputs struct { AdditionalScopes []string } -func (i *LoginInputs) isLoggingInAsAMachine() bool { - return i.ClientID != "" || i.ClientSecret != "" || i.Domain != "" -} - func (i *LoginInputs) isLoggingInWithAdditionalScopes() bool { return len(i.AdditionalScopes) > 0 } @@ -82,12 +78,65 @@ func loginCmd(cli *cli) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { var selectedLoginType string const loginAsUser, loginAsMachine = "As a user", "As a machine" + shouldLoginAsUser, shouldLoginAsMachine := false, false + + /* + Based on the initial inputs we'd like to determine if + it's a machine login or a user login + If we successfully determine it, we don't need to prompt the user. + + The --no-input flag add strict restriction that we shall not take any further input after + initial command. + Hence, the flow diverges into two based on no-input flag's value. + */ + switch { + case cli.noInput: + switch { + case inputs.Domain != "" && inputs.ClientSecret != "" && inputs.ClientID != "": + // If all three fields are passed, machine login flag is set to true. + shouldLoginAsMachine = true + case inputs.Domain != "" && inputs.ClientSecret == "" && inputs.ClientID == "": + /* + The domain flag is common between Machine and User Login. + If domain is passed without client-id and client-secret, + it can be evaluated that it is a user login flow. + */ + shouldLoginAsUser = true + case inputs.Domain != "" || inputs.ClientSecret != "" || inputs.ClientID != "": + /* + At this point, if AT LEAST one of the three flags are passed but not ALL three, + we return an error since it's a no-input flow and it will need all three params + for successful machine flow. + Note that we already determined it's not a user login flow in the condition above. + */ + return fmt.Errorf("flags client-id, client-secret and domain are required together") + default: + /* + If no flags are passed along with --no-input, it is defaulted to user login flow. + */ + shouldLoginAsUser = true + } + default: + if inputs.ClientSecret != "" || inputs.ClientID != "" { + /* + If all three params are passed, we evaluate it as a Machine Login Flow. + Else required params are prompted for. + */ + shouldLoginAsMachine = true + } + } - // We want to prompt if we don't pass the following flags: - // --no-input, --scopes, --client-id, --client-secret, --domain. - // Because then the prompt is unnecessary as we know the login type. - shouldPrompt := !inputs.isLoggingInAsAMachine() && !cli.noInput && !inputs.isLoggingInWithAdditionalScopes() - if shouldPrompt { + // If additional scopes are passed we mark shouldLoginAsUser flag to be true. + if inputs.isLoggingInWithAdditionalScopes() { + shouldLoginAsUser = true + } + + /* + If we are unable to determine if it's a user login or a machine login + based on all the evaluation above, we go on to prompt the user and + determine if it's LoginAsUser or LoginAsMachine + */ + if !shouldLoginAsUser && !shouldLoginAsMachine { cli.renderer.Output( fmt.Sprintf( "%s\n\n%s\n%s\n\n%s\n%s\n%s\n%s\n\n", @@ -107,7 +156,7 @@ func loginCmd(cli *cli) *cobra.Command { "Authenticating as a user is recommended if performing ad-hoc operations or working locally.", "Alternatively, authenticating as a machine is recommended for automated workflows (ex:CI).", ) - input := prompt.SelectInput("", label, help, []string{loginAsUser, loginAsMachine}, loginAsUser, shouldPrompt) + input := prompt.SelectInput("", label, help, []string{loginAsUser, loginAsMachine}, loginAsUser, true) if err := prompt.AskOne(input, &selectedLoginType); err != nil { return handleInputError(err) } @@ -115,10 +164,8 @@ func loginCmd(cli *cli) *cobra.Command { ctx := cmd.Context() - // Allows to skip to user login if either the --no-input or --scopes flag is passed. - shouldLoginAsUser := (cli.noInput && !inputs.isLoggingInAsAMachine()) || inputs.isLoggingInWithAdditionalScopes() || selectedLoginType == loginAsUser - if shouldLoginAsUser { - if _, err := RunLoginAsUser(ctx, cli, inputs.AdditionalScopes); err != nil { + if shouldLoginAsUser || selectedLoginType == loginAsUser { + if _, err := RunLoginAsUser(ctx, cli, inputs.AdditionalScopes, inputs.Domain); err != nil { return fmt.Errorf("failed to start the authentication process: %w", err) } } else { @@ -143,8 +190,8 @@ func loginCmd(cli *cli) *cobra.Command { loginClientID.RegisterString(cmd, &inputs.ClientID, "") loginClientSecret.RegisterString(cmd, &inputs.ClientSecret, "") loginAdditionalScopes.RegisterStringSlice(cmd, &inputs.AdditionalScopes, []string{}) - cmd.MarkFlagsRequiredTogether("client-id", "client-secret", "domain") cmd.MarkFlagsMutuallyExclusive("client-id", "scopes") + cmd.MarkFlagsMutuallyExclusive("client-secret", "scopes") cmd.SetHelpFunc(func(cmd *cobra.Command, args []string) { _ = cmd.Flags().MarkHidden("tenant") @@ -154,10 +201,36 @@ func loginCmd(cli *cli) *cobra.Command { return cmd } +func ensureAuth0URL(input string) (string, error) { + if input == "" { + return "https://*.auth0.com/api/v2/", nil + } + input = strings.TrimPrefix(input, "http://") + input = strings.TrimPrefix(input, "https://") + input = strings.TrimSuffix(input, "/api/v2") + + // Check if the input ends with auth0.com . + if !strings.HasSuffix(input, "auth0.com") { + return "", fmt.Errorf("not a valid auth0.com domain") + } + + // Extract the domain part without any path. + domainParts := strings.Split(input, "/") + domain := domainParts[0] + + // Return the formatted URL. + return fmt.Sprintf("https://%s/api/v2/", domain), nil +} + // RunLoginAsUser runs the login flow guiding the user through the process // by showing the login instructions, opening the browser. -func RunLoginAsUser(ctx context.Context, cli *cli, additionalScopes []string) (config.Tenant, error) { - state, err := auth.GetDeviceCode(ctx, http.DefaultClient, additionalScopes) +func RunLoginAsUser(ctx context.Context, cli *cli, additionalScopes []string, domain string) (config.Tenant, error) { + domain, err := ensureAuth0URL(domain) + if err != nil { + return config.Tenant{}, err + } + + state, err := auth.GetDeviceCode(ctx, http.DefaultClient, additionalScopes, domain) if err != nil { return config.Tenant{}, fmt.Errorf("failed to get the device code: %w", err) } diff --git a/internal/cli/login_test.go b/internal/cli/login_test.go new file mode 100644 index 000000000..fbae2c24b --- /dev/null +++ b/internal/cli/login_test.go @@ -0,0 +1,55 @@ +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestLoginCommand(t *testing.T) { + t.Run("Negative Test: it returns an error since client-id, client-secret and domain must be passed together", func(t *testing.T) { + cli := &cli{} + cli.noInput = true + cmd := loginCmd(cli) + cmd.SetArgs([]string{"--client-id", "t3dbMFeTokYBguVu1Ty88gqntUXELSn9"}) + err := cmd.Execute() + + assert.EqualError(t, err, "flags client-id, client-secret and domain are required together") + }) + + t.Run("Negative Test: it returns an error since client-id, client-secret and domain must be passed together", func(t *testing.T) { + cli := &cli{} + cli.noInput = true + cmd := loginCmd(cli) + cmd.SetArgs([]string{"--client-secret", "3OAzE7j2HTnGOPeCRFX3Hg-0sipaEnodzQK8xpwsRiTkqdjjwEFT04rgCjfslianfs"}) + err := cmd.Execute() + assert.EqualError(t, err, "flags client-id, client-secret and domain are required together") + }) + + t.Run("Negative Test: it returns an error since client-id, client-secret and domain must be passed together", func(t *testing.T) { + cli := &cli{} + cli.noInput = true + cmd := loginCmd(cli) + cmd.SetArgs([]string{"--client-id", "t3dbMFeTokYBguVu1Ty88gqntUXELSn9", "--client-secret", "3OAzE7j2HTnGOPeCRFX3Hg-0sipaEnodzQK8xpkqdjjwEFT0EFT04rgCp4PZL4Z"}) + err := cmd.Execute() + assert.EqualError(t, err, "flags client-id, client-secret and domain are required together") + }) + + t.Run("Negative Test: it returns an error since client-id, client-secret and domain must be passed together", func(t *testing.T) { + cli := &cli{} + cli.noInput = true + cmd := loginCmd(cli) + cmd.SetArgs([]string{"--client-id", "t3dbMFeTokYBguVu1Ty88gqntUXELSn9", "--domain", "duedares.us.auth0.com"}) + err := cmd.Execute() + assert.EqualError(t, err, "flags client-id, client-secret and domain are required together") + }) + + t.Run("Negative Test: it returns an error since client-id, client-secret and domain must be passed together", func(t *testing.T) { + cli := &cli{} + cli.noInput = true + cmd := loginCmd(cli) + cmd.SetArgs([]string{"--client-secret", "3OAzE7j2HTnGOPeCRFX3Hg-0sipaEnodzQK8xpkqdjjwEFT0EFT04rgCp4PZL4Z", "--domain", "duedares.us.auth0.com"}) + err := cmd.Execute() + assert.EqualError(t, err, "flags client-id, client-secret and domain are required together") + }) +}