From 0a2b425eb505b1092b2335d95b915f3ce47feb3c Mon Sep 17 00:00:00 2001 From: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com> Date: Fri, 17 Mar 2023 19:14:28 +0100 Subject: [PATCH] cli: nomad login command should not require a -type flag and should respect default auth method (#16504) nomad login command does not need to know ACL Auth Method's type, since all method names are unique. Co-authored-by: James Rasell --- .changelog/16504.txt | 7 ++ command/login.go | 95 ++++++++++++------------- command/login_test.go | 34 ++------- website/content/docs/commands/login.mdx | 6 +- 4 files changed, 59 insertions(+), 83 deletions(-) create mode 100644 .changelog/16504.txt diff --git a/.changelog/16504.txt b/.changelog/16504.txt new file mode 100644 index 000000000..478ffdf5c --- /dev/null +++ b/.changelog/16504.txt @@ -0,0 +1,7 @@ +```release-note:breaking-change +cli: nomad login no longer requires -type flag, since auth method names are globally unique. +``` + +```release-note:bug +cli: nomad login no longer ignores default auth method if they are present. +``` diff --git a/command/login.go b/command/login.go index 6d623dbd8..4a880d54d 100644 --- a/command/login.go +++ b/command/login.go @@ -8,10 +8,11 @@ import ( "strings" "github.com/hashicorp/cap/util" - "github.com/hashicorp/nomad/api" - "github.com/hashicorp/nomad/lib/auth/oidc" "github.com/mitchellh/cli" "github.com/posener/complete" + + "github.com/hashicorp/nomad/api" + "github.com/hashicorp/nomad/lib/auth/oidc" ) // Ensure LoginCommand satisfies the cli.Command interface. @@ -21,7 +22,7 @@ var _ cli.Command = &LoginCommand{} type LoginCommand struct { Meta - authMethodType string + authMethodType string // deprecated in 1.5.2, left for backwards compat authMethodName string callbackAddr string @@ -47,10 +48,6 @@ Login Options: The name of the ACL auth method to login to. If the cluster administrator has configured a default, this flag is optional. - -type - Type of the auth method to login to. If the cluster administrator has - configured a default, this flag is optional. - -oidc-callback-addr The address to use for the local OIDC callback server. This should be given in the form of : and defaults to "localhost:4649". @@ -73,7 +70,6 @@ func (l *LoginCommand) AutocompleteFlags() complete.Flags { return mergeAutocompleteFlags(l.Meta.AutocompleteFlags(FlagSetClient), complete.Flags{ "-method": complete.PredictAnything, - "-type": complete.PredictSet("OIDC"), "-oidc-callback-addr": complete.PredictAnything, "-json": complete.PredictNothing, "-t": complete.PredictAnything, @@ -104,57 +100,56 @@ func (l *LoginCommand) Run(args []string) int { return 1 } - // Auth method types are particular with their naming, so ensure we forgive - // any case mistakes here from the user. - l.authMethodType = strings.ToUpper(l.authMethodType) - - // Ensure we sanitize the method type so we do not pedantically return an - // error when the caller uses "oidc" rather than "OIDC". The flag default - // means an empty type is only possible is the caller specifies this - // explicitly. - switch l.authMethodType { - case api.ACLAuthMethodTypeOIDC: - default: - l.Ui.Error(fmt.Sprintf("Unsupported authentication type %q", l.authMethodType)) - return 1 - } - client, err := l.Meta.Client() if err != nil { l.Ui.Error(fmt.Sprintf("Error initializing client: %s", err)) return 1 } - // If the caller did not supply an auth method name or type, attempt to - // lookup the default. This ensures a nice UX as clusters are expected to - // only have one method, and this avoids having to type the name during - // each login. - if l.authMethodName == "" { + var ( + defaultMethod *api.ACLAuthMethodListStub + methodType string + ) - authMethodList, _, err := client.ACLAuthMethods().List(nil) - if err != nil { - l.Ui.Error(fmt.Sprintf("Error listing ACL auth methods: %s", err)) + if l.authMethodType != "" { + l.Ui.Warn("warning: '-type' flag has been deprecated for nomad login command and will be ignored.") + } + + authMethodList, _, err := client.ACLAuthMethods().List(nil) + if err != nil { + l.Ui.Error(fmt.Sprintf("Error listing ACL auth methods: %s", err)) + return 1 + } + + for _, authMethod := range authMethodList { + if authMethod.Default { + defaultMethod = authMethod + } + } + + // If there is a default method available, and the caller did not pass method + // name, fill it in. In case there is no default method, error and quit. + if l.authMethodName == "" { + if defaultMethod != nil { + l.authMethodName = defaultMethod.Name + methodType = defaultMethod.Type + } else { + l.Ui.Error("Must specify an auth method name, no default found") return 1 } - - for _, authMethod := range authMethodList { - if authMethod.Default { - l.authMethodName = authMethod.Name - if l.authMethodType == "" { - l.authMethodType = authMethod.Type - } - if l.authMethodType != authMethod.Type { - l.Ui.Error(fmt.Sprintf( - "Specified type: %s does not match the type of the default method: %s", - l.authMethodType, authMethod.Type, - )) - return 1 - } + } else { + // Find the method by name in the state store and get its type + for _, method := range authMethodList { + if method.Name == l.authMethodName { + methodType = method.Type } } - if l.authMethodName == "" || l.authMethodType == "" { - l.Ui.Error("Must specify an auth method name and type, no default found") + if methodType == "" { + l.Ui.Error(fmt.Sprintf( + "Error: method %s not found in the state store. ", + l.authMethodName, + )) return 1 } } @@ -164,11 +159,11 @@ func (l *LoginCommand) Run(args []string) int { // reusable and generic handling of errors and outputs. var authFn func(context.Context, *api.Client) (*api.ACLToken, error) - switch l.authMethodType { + switch methodType { case api.ACLAuthMethodTypeOIDC: authFn = l.loginOIDC default: - l.Ui.Error(fmt.Sprintf("Unsupported authentication type %q", l.authMethodType)) + l.Ui.Error(fmt.Sprintf("Unsupported authentication type %q", methodType)) return 1 } @@ -191,7 +186,7 @@ func (l *LoginCommand) Run(args []string) int { return 0 } - l.Ui.Output(fmt.Sprintf("Successfully logged in via %s and %s\n", l.authMethodType, l.authMethodName)) + l.Ui.Output(fmt.Sprintf("Successfully logged in via %s and %s\n", methodType, l.authMethodName)) outputACLToken(l.Ui, token) return 0 } diff --git a/command/login_test.go b/command/login_test.go index a7348a55f..9c8966917 100644 --- a/command/login_test.go +++ b/command/login_test.go @@ -5,7 +5,6 @@ import ( "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/command/agent" - "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" "github.com/mitchellh/cli" "github.com/shoenig/test/must" @@ -38,37 +37,16 @@ func TestLoginCommand_Run(t *testing.T) { ui.OutputWriter.Reset() ui.ErrorWriter.Reset() - // Attempt to call it with an unsupported method type. - must.Eq(t, 1, cmd.Run([]string{"-address=" + agentURL, "-type=SAML"})) - must.StrContains(t, ui.ErrorWriter.String(), `Unsupported authentication type "SAML"`) + // Attempt to run the command without specifying a method name, when there's no default available + must.Eq(t, 1, cmd.Run([]string{"-address=" + agentURL})) + must.StrContains(t, ui.ErrorWriter.String(), "Must specify an auth method name, no default found") ui.OutputWriter.Reset() ui.ErrorWriter.Reset() - // Use a valid method type but with incorrect casing so we can ensure this - // is handled. - must.Eq(t, 1, cmd.Run([]string{"-address=" + agentURL, "-type=oIdC"})) - must.StrContains(t, ui.ErrorWriter.String(), "Must specify an auth method name and type, no default found") - - ui.OutputWriter.Reset() - ui.ErrorWriter.Reset() - - // Store a default auth method - state := srv.Agent.Server().State() - method := &structs.ACLAuthMethod{ - Name: "test-auth-method", - Default: true, - Type: "JWT", - Config: &structs.ACLAuthMethodConfig{ - OIDCDiscoveryURL: "http://example.com", - }, - } - method.SetHash() - must.NoError(t, state.UpsertACLAuthMethods(1000, []*structs.ACLAuthMethod{method})) - - // Specify an incorrect type of default method - must.Eq(t, 1, cmd.Run([]string{"-address=" + agentURL, "-type=OIDC"})) - must.StrContains(t, ui.ErrorWriter.String(), "Specified type: OIDC does not match the type of the default method: JWT") + // Attempt to login using a non-existing method + must.Eq(t, 1, cmd.Run([]string{"-address=" + agentURL, "-method", "there-is-no-such-method"})) + must.StrContains(t, ui.ErrorWriter.String(), "Error: method there-is-no-such-method not found in the state store. ") ui.OutputWriter.Reset() ui.ErrorWriter.Reset() diff --git a/website/content/docs/commands/login.mdx b/website/content/docs/commands/login.mdx index 5f83c5a8c..1dd2bf164 100644 --- a/website/content/docs/commands/login.mdx +++ b/website/content/docs/commands/login.mdx @@ -28,10 +28,6 @@ requested auth method for a newly minted Nomad ACL token. - `-method`: The name of the ACL auth method to log in via. If the cluster administrator has configured a default, this flag is optional. -- `-type`: Type of the auth method to log in via. If the cluster administrator - has configured a default, this flag is optional. Currently only supports - "OIDC". - - `-oidc-callback-addr`: The address to use for the local OIDC callback server. This should be given in the form of `:` and defaults to `localhost:4649`. @@ -45,7 +41,7 @@ requested auth method for a newly minted Nomad ACL token. Login using an OIDC provider: ```shell-session -$ nomad login -type=OIDC -method=auth0 +$ nomad login -method=auth0 Successfully logged in via OIDC and auth0 Accessor ID = 68123fee-1e8b-7ecc-5b34-505ecd2dcb80