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 <jrasell@users.noreply.github.com>
This commit is contained in:
Piotr Kazmierczak
2023-03-17 19:14:28 +01:00
committed by GitHub
parent ed498f8ddb
commit b95b105288
4 changed files with 59 additions and 83 deletions

7
.changelog/16504.txt Normal file
View File

@@ -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.
```

View File

@@ -8,10 +8,11 @@ import (
"strings" "strings"
"github.com/hashicorp/cap/util" "github.com/hashicorp/cap/util"
"github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/lib/auth/oidc"
"github.com/mitchellh/cli" "github.com/mitchellh/cli"
"github.com/posener/complete" "github.com/posener/complete"
"github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/lib/auth/oidc"
) )
// Ensure LoginCommand satisfies the cli.Command interface. // Ensure LoginCommand satisfies the cli.Command interface.
@@ -21,7 +22,7 @@ var _ cli.Command = &LoginCommand{}
type LoginCommand struct { type LoginCommand struct {
Meta Meta
authMethodType string authMethodType string // deprecated in 1.5.2, left for backwards compat
authMethodName string authMethodName string
callbackAddr string callbackAddr string
@@ -47,10 +48,6 @@ Login Options:
The name of the ACL auth method to login to. If the cluster administrator The name of the ACL auth method to login to. If the cluster administrator
has configured a default, this flag is optional. 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 -oidc-callback-addr
The address to use for the local OIDC callback server. This should be given The address to use for the local OIDC callback server. This should be given
in the form of <IP>:<PORT> and defaults to "localhost:4649". in the form of <IP>:<PORT> and defaults to "localhost:4649".
@@ -73,7 +70,6 @@ func (l *LoginCommand) AutocompleteFlags() complete.Flags {
return mergeAutocompleteFlags(l.Meta.AutocompleteFlags(FlagSetClient), return mergeAutocompleteFlags(l.Meta.AutocompleteFlags(FlagSetClient),
complete.Flags{ complete.Flags{
"-method": complete.PredictAnything, "-method": complete.PredictAnything,
"-type": complete.PredictSet("OIDC"),
"-oidc-callback-addr": complete.PredictAnything, "-oidc-callback-addr": complete.PredictAnything,
"-json": complete.PredictNothing, "-json": complete.PredictNothing,
"-t": complete.PredictAnything, "-t": complete.PredictAnything,
@@ -104,57 +100,56 @@ func (l *LoginCommand) Run(args []string) int {
return 1 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() client, err := l.Meta.Client()
if err != nil { if err != nil {
l.Ui.Error(fmt.Sprintf("Error initializing client: %s", err)) l.Ui.Error(fmt.Sprintf("Error initializing client: %s", err))
return 1 return 1
} }
// If the caller did not supply an auth method name or type, attempt to var (
// lookup the default. This ensures a nice UX as clusters are expected to defaultMethod *api.ACLAuthMethodListStub
// only have one method, and this avoids having to type the name during methodType string
// each login. )
if l.authMethodName == "" {
authMethodList, _, err := client.ACLAuthMethods().List(nil) if l.authMethodType != "" {
if err != nil { l.Ui.Warn("warning: '-type' flag has been deprecated for nomad login command and will be ignored.")
l.Ui.Error(fmt.Sprintf("Error listing ACL auth methods: %s", err)) }
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 return 1
} }
} else {
for _, authMethod := range authMethodList { // Find the method by name in the state store and get its type
if authMethod.Default { for _, method := range authMethodList {
l.authMethodName = authMethod.Name if method.Name == l.authMethodName {
if l.authMethodType == "" { methodType = method.Type
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
}
} }
} }
if l.authMethodName == "" || l.authMethodType == "" { if methodType == "" {
l.Ui.Error("Must specify an auth method name and type, no default found") l.Ui.Error(fmt.Sprintf(
"Error: method %s not found in the state store. ",
l.authMethodName,
))
return 1 return 1
} }
} }
@@ -164,11 +159,11 @@ func (l *LoginCommand) Run(args []string) int {
// reusable and generic handling of errors and outputs. // reusable and generic handling of errors and outputs.
var authFn func(context.Context, *api.Client) (*api.ACLToken, error) var authFn func(context.Context, *api.Client) (*api.ACLToken, error)
switch l.authMethodType { switch methodType {
case api.ACLAuthMethodTypeOIDC: case api.ACLAuthMethodTypeOIDC:
authFn = l.loginOIDC authFn = l.loginOIDC
default: default:
l.Ui.Error(fmt.Sprintf("Unsupported authentication type %q", l.authMethodType)) l.Ui.Error(fmt.Sprintf("Unsupported authentication type %q", methodType))
return 1 return 1
} }
@@ -191,7 +186,7 @@ func (l *LoginCommand) Run(args []string) int {
return 0 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) outputACLToken(l.Ui, token)
return 0 return 0
} }

View File

@@ -5,7 +5,6 @@ import (
"github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/ci"
"github.com/hashicorp/nomad/command/agent" "github.com/hashicorp/nomad/command/agent"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/hashicorp/nomad/testutil" "github.com/hashicorp/nomad/testutil"
"github.com/mitchellh/cli" "github.com/mitchellh/cli"
"github.com/shoenig/test/must" "github.com/shoenig/test/must"
@@ -38,37 +37,16 @@ func TestLoginCommand_Run(t *testing.T) {
ui.OutputWriter.Reset() ui.OutputWriter.Reset()
ui.ErrorWriter.Reset() ui.ErrorWriter.Reset()
// Attempt to call it with an unsupported method type. // 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, "-type=SAML"})) must.Eq(t, 1, cmd.Run([]string{"-address=" + agentURL}))
must.StrContains(t, ui.ErrorWriter.String(), `Unsupported authentication type "SAML"`) must.StrContains(t, ui.ErrorWriter.String(), "Must specify an auth method name, no default found")
ui.OutputWriter.Reset() ui.OutputWriter.Reset()
ui.ErrorWriter.Reset() ui.ErrorWriter.Reset()
// Use a valid method type but with incorrect casing so we can ensure this // Attempt to login using a non-existing method
// is handled. must.Eq(t, 1, cmd.Run([]string{"-address=" + agentURL, "-method", "there-is-no-such-method"}))
must.Eq(t, 1, cmd.Run([]string{"-address=" + agentURL, "-type=oIdC"})) must.StrContains(t, ui.ErrorWriter.String(), "Error: method there-is-no-such-method not found in the state store. ")
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")
ui.OutputWriter.Reset() ui.OutputWriter.Reset()
ui.ErrorWriter.Reset() ui.ErrorWriter.Reset()

View File

@@ -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 - `-method`: The name of the ACL auth method to log in via. If the cluster
administrator has configured a default, this flag is optional. 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. - `-oidc-callback-addr`: The address to use for the local OIDC callback server.
This should be given in the form of `<IP>:<PORT>` and defaults to This should be given in the form of `<IP>:<PORT>` and defaults to
`localhost:4649`. `localhost:4649`.
@@ -45,7 +41,7 @@ requested auth method for a newly minted Nomad ACL token.
Login using an OIDC provider: Login using an OIDC provider:
```shell-session ```shell-session
$ nomad login -type=OIDC -method=auth0 $ nomad login -method=auth0
Successfully logged in via OIDC and auth0 Successfully logged in via OIDC and auth0
Accessor ID = 68123fee-1e8b-7ecc-5b34-505ecd2dcb80 Accessor ID = 68123fee-1e8b-7ecc-5b34-505ecd2dcb80