acl: make auth method default across all types (#15869)

This commit is contained in:
Piotr Kazmierczak 2023-01-26 14:17:11 +01:00 committed by GitHub
parent 5d33891910
commit f4d6efe69f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 61 additions and 36 deletions

View File

@ -48,7 +48,8 @@ Login Options:
has configured a default, this flag is optional.
-type
Type of the auth method to login to. Defaults to "OIDC".
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
@ -88,7 +89,7 @@ func (l *LoginCommand) Run(args []string) int {
flags := l.Meta.FlagSet(l.Name(), FlagSetClient)
flags.Usage = func() { l.Ui.Output(l.Help()) }
flags.StringVar(&l.authMethodName, "method", "", "")
flags.StringVar(&l.authMethodType, "type", "OIDC", "")
flags.StringVar(&l.authMethodType, "type", "", "")
flags.StringVar(&l.callbackAddr, "oidc-callback-addr", "localhost:4649", "")
flags.BoolVar(&l.json, "json", false, "")
flags.StringVar(&l.template, "t", "", "")
@ -112,9 +113,6 @@ func (l *LoginCommand) Run(args []string) int {
// means an empty type is only possible is the caller specifies this
// explicitly.
switch sanitizedMethodType {
case "":
l.Ui.Error("Please supply an authentication type")
return 1
case api.ACLAuthMethodTypeOIDC:
default:
l.Ui.Error(fmt.Sprintf("Unsupported authentication type %q", sanitizedMethodType))
@ -127,9 +125,10 @@ func (l *LoginCommand) Run(args []string) int {
return 1
}
// If the caller did not supply and auth method name, 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 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 == "" {
authMethodList, _, err := client.ACLAuthMethods().List(nil)
@ -141,11 +140,21 @@ 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
}
}
}
if l.authMethodName == "" {
l.Ui.Error("Must specify an auth method name, no default found")
if l.authMethodName == "" || l.authMethodType == "" {
l.Ui.Error("Must specify an auth method name and type, no default found")
return 1
}
}

View File

@ -5,6 +5,7 @@ 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"
@ -47,7 +48,27 @@ func TestLoginCommand_Run(t *testing.T) {
// 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, no default found")
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.ErrorWriter.Reset()

View File

@ -1824,13 +1824,13 @@ func (a *ACL) UpsertAuthMethods(
}
// Are we trying to upsert a default auth method? Check if there isn't
// a default one for that very type already.
// a default one already.
if authMethod.Default {
existingMethodsDefaultmethod, _ := stateSnapshot.GetDefaultACLAuthMethodByType(nil, authMethod.Type)
if existingMethodsDefaultmethod != nil && existingMethodsDefaultmethod.Name != authMethod.Name {
existingMethodsDefaultMethod, _ := stateSnapshot.GetDefaultACLAuthMethod(nil)
if existingMethodsDefaultMethod != nil && existingMethodsDefaultMethod.Name != authMethod.Name {
return structs.NewErrRPCCodedf(
http.StatusBadRequest,
"default method for type %s already exists: %v", authMethod.Type, existingMethodsDefaultmethod.Name,
"default method already exists: %v", existingMethodsDefaultMethod.Name,
)
}
}

View File

@ -65,11 +65,10 @@ func (s *StateStore) upsertACLAuthMethodTxn(index uint64, txn *txn, method *stru
// setting. We therefore need to check we are not trying to create a method
// with an existing name or a duplicate default for the same type.
if method.Default {
existingMethodsDefaultmethod, _ := s.GetDefaultACLAuthMethodByType(nil, method.Type)
if existingMethodsDefaultmethod != nil && existingMethodsDefaultmethod.Name != method.Name {
existingMethodsDefaultMethod, _ := s.GetDefaultACLAuthMethod(nil)
if existingMethodsDefaultMethod != nil && existingMethodsDefaultMethod.Name != method.Name {
return false, fmt.Errorf(
"default ACL auth method for type %s already exists: %v",
method.Type, existingMethodsDefaultmethod.Name,
"default ACL auth method already exists: %v", existingMethodsDefaultMethod.Name,
)
}
}
@ -186,10 +185,9 @@ func (s *StateStore) GetACLAuthMethodByName(ws memdb.WatchSet, authMethod string
return nil, nil
}
// GetDefaultACLAuthMethodByType returns a default ACL Auth Methods for a given
// auth type. Since we only want 1 default auth method per type, this function
// is used during upserts to facilitate that check.
func (s *StateStore) GetDefaultACLAuthMethodByType(ws memdb.WatchSet, methodType string) (*structs.ACLAuthMethod, error) {
// GetDefaultACLAuthMethod returns a default ACL Auth Method. This function is
// used during upserts to facilitate a check that there's only 1 default Auth Method.
func (s *StateStore) GetDefaultACLAuthMethod(ws memdb.WatchSet) (*structs.ACLAuthMethod, error) {
txn := s.db.ReadTxn()
// Walk the entire table to get all ACL auth methods.
@ -205,8 +203,8 @@ func (s *StateStore) GetDefaultACLAuthMethodByType(ws memdb.WatchSet, methodType
if !ok {
return true
}
// any non-default method or method of different type than desired gets filtered-out
return !method.Default || method.Type != methodType
// any non-default method gets filtered-out
return !method.Default
})
for raw := filter.Next(); raw != nil; raw = filter.Next() {

View File

@ -227,7 +227,7 @@ func TestStateStore_GetACLAuthMethodByName(t *testing.T) {
must.Equal(t, mockedACLAuthMethods[1], authMethod)
}
func TestStateStore_GetDefaultACLAuthMethodByType(t *testing.T) {
func TestStateStore_GetDefaultACLAuthMethod(t *testing.T) {
ci.Parallel(t)
testState := testStateStore(t)
@ -240,16 +240,12 @@ func TestStateStore_GetDefaultACLAuthMethodByType(t *testing.T) {
mockedACLAuthMethods := []*structs.ACLAuthMethod{am1, am2}
must.NoError(t, testState.UpsertACLAuthMethods(10, mockedACLAuthMethods))
// Get the default method for OIDC
// Get the default method
ws := memdb.NewWatchSet()
defaultOIDCMethod, err := testState.GetDefaultACLAuthMethodByType(ws, "OIDC")
defaultACLAuthMethod, err := testState.GetDefaultACLAuthMethod(ws)
must.NoError(t, err)
must.True(t, defaultOIDCMethod.Default)
must.Eq(t, am1, defaultOIDCMethod)
must.True(t, defaultACLAuthMethod.Default)
must.Eq(t, am1, defaultACLAuthMethod)
// Get the default method for jwt (should not return anything)
defaultJWTMethod, err := testState.GetDefaultACLAuthMethodByType(ws, "JWT")
must.NoError(t, err)
must.Nil(t, defaultJWTMethod)
}

View File

@ -28,8 +28,9 @@ 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. Defaults to, and currently
only supports, "OIDC".
- `-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