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 07543f8bdf
commit 0a2b425eb5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
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"
"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 <IP>:<PORT> 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,32 +100,20 @@ 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
)
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 {
@ -139,22 +123,33 @@ func (l *LoginCommand) Run(args []string) int {
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
}
defaultMethod = authMethod
}
}
if l.authMethodName == "" || l.authMethodType == "" {
l.Ui.Error("Must specify an auth method name and type, no default found")
// 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
}
} 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 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
}

View File

@ -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()

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
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 `<IP>:<PORT>` 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