From 31b1d890c16f5d0e0859f2e5421eeae2e36ca4d9 Mon Sep 17 00:00:00 2001 From: Damon Earp Date: Wed, 10 Feb 2021 13:04:46 -0600 Subject: [PATCH 01/19] updated the alias to use the display name instead of uid --- path_login.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/path_login.go b/path_login.go index 213d3ddb..769d61b1 100644 --- a/path_login.go +++ b/path_login.go @@ -107,9 +107,10 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d return nil, logical.ErrPermissionDenied } + displayName := fmt.Sprintf("%s-%s", serviceAccount.namespace(), serviceAccount.name()) auth := &logical.Auth{ Alias: &logical.Alias{ - Name: serviceAccount.uid(), + Name: displayName, Metadata: map[string]string{ "service_account_uid": serviceAccount.uid(), "service_account_name": serviceAccount.name(), @@ -127,7 +128,7 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d "service_account_secret_name": serviceAccount.SecretName, "role": roleName, }, - DisplayName: fmt.Sprintf("%s-%s", serviceAccount.namespace(), serviceAccount.name()), + DisplayName: displayName, } role.PopulateTokenAuth(auth) From 06ceff5be41c57a1e234acad154df609264b7647 Mon Sep 17 00:00:00 2001 From: Damon Earp Date: Thu, 11 Feb 2021 12:18:36 -0600 Subject: [PATCH 02/19] added HumanReadableAlias to role options --- path_login.go | 10 +++++++--- path_role.go | 3 +++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/path_login.go b/path_login.go index 769d61b1..7b857fbd 100644 --- a/path_login.go +++ b/path_login.go @@ -107,10 +107,14 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d return nil, logical.ErrPermissionDenied } - displayName := fmt.Sprintf("%s-%s", serviceAccount.namespace(), serviceAccount.name()) + var name = serviceAccount.uid() + if role.HumanReadableAlias { + name = fmt.Sprintf("%s/%s", serviceAccount.namespace(), serviceAccount.name()) + } + auth := &logical.Auth{ Alias: &logical.Alias{ - Name: displayName, + Name: name, Metadata: map[string]string{ "service_account_uid": serviceAccount.uid(), "service_account_name": serviceAccount.name(), @@ -128,7 +132,7 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d "service_account_secret_name": serviceAccount.SecretName, "role": roleName, }, - DisplayName: displayName, + DisplayName: fmt.Sprintf("%s-%s", serviceAccount.namespace(), serviceAccount.name()), } role.PopulateTokenAuth(auth) diff --git a/path_role.go b/path_role.go index 6506b1fe..841d779b 100644 --- a/path_role.go +++ b/path_role.go @@ -332,6 +332,9 @@ type roleStorageEntry struct { // Audience is an optional jwt claim to verify Audience string `json:"audience" mapstructure:"audience" structs: "audience"` + // use the service accounts 'namespace/name' instead of uid for the alias name + HumanReadableAlias bool `json:"human_readable_alias" mapstructure:"human_readable_alias" structs:"human_readable_alias"` + // Deprecated by TokenParams Policies []string `json:"policies" structs:"policies" mapstructure:"policies"` NumUses int `json:"num_uses" mapstructure:"num_uses" structs:"num_uses"` From b2157774508affdff7905ef12b2b54bb22977112 Mon Sep 17 00:00:00 2001 From: Damon Earp Date: Thu, 11 Feb 2021 13:52:59 -0600 Subject: [PATCH 03/19] udpated all the other path_role functions to properly display the option --- path_role.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/path_role.go b/path_role.go index 841d779b..b28fbc9b 100644 --- a/path_role.go +++ b/path_role.go @@ -49,6 +49,10 @@ are allowed.`, Type: framework.TypeString, Description: "Optional Audience claim to verify in the jwt.", }, + "human_readable_alias" { + Type: framework.TypeBool, + Description: "Use Kubernetes Namepsace/ServiceAccount instead of Service Accounts UID for Alias name" + }, "policies": { Type: framework.TypeCommaStringSlice, Description: tokenutil.DeprecationText("token_policies"), @@ -152,6 +156,8 @@ func (b *kubeAuthBackend) pathRoleRead(ctx context.Context, req *logical.Request d["audience"] = role.Audience } + d["human_readable_alias"] = role.HumanReadableAlias + role.PopulateTokenData(d) if len(role.Policies) > 0 { @@ -302,6 +308,10 @@ func (b *kubeAuthBackend) pathRoleCreateUpdate(ctx context.Context, req *logical role.Audience = audience.(string) } + if humanReadableAlias, ok := data.GetOk("human_readable_alias"); ok { + role.HumanReadableAlias = humanReadableAlias.(bool) + } + // Store the entry. entry, err := logical.StorageEntryJSON("role/"+strings.ToLower(roleName), role) if err != nil { From adb50d760dc50638f072c1946b4da81319746013 Mon Sep 17 00:00:00 2001 From: Damon Earp Date: Thu, 11 Feb 2021 13:59:40 -0600 Subject: [PATCH 04/19] fixed syntax errors --- path_role.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/path_role.go b/path_role.go index b28fbc9b..ae4c1343 100644 --- a/path_role.go +++ b/path_role.go @@ -49,9 +49,9 @@ are allowed.`, Type: framework.TypeString, Description: "Optional Audience claim to verify in the jwt.", }, - "human_readable_alias" { + "human_readable_alias": { Type: framework.TypeBool, - Description: "Use Kubernetes Namepsace/ServiceAccount instead of Service Accounts UID for Alias name" + Description: "Use Kubernetes Namepsace/ServiceAccount instead of Service Accounts UID for Alias name", }, "policies": { Type: framework.TypeCommaStringSlice, From 0659d61c32292ab55f4e7199a48039decd2d0f6a Mon Sep 17 00:00:00 2001 From: Damon Earp Date: Thu, 11 Feb 2021 14:43:31 -0600 Subject: [PATCH 05/19] updated the option description --- path_role.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/path_role.go b/path_role.go index ae4c1343..2448ad6d 100644 --- a/path_role.go +++ b/path_role.go @@ -51,7 +51,7 @@ are allowed.`, }, "human_readable_alias": { Type: framework.TypeBool, - Description: "Use Kubernetes Namepsace/ServiceAccount instead of Service Accounts UID for Alias name", + Description: `Use "Kubernete's Namespace/Service Account Name" instead of the UID for the alias name.`, }, "policies": { Type: framework.TypeCommaStringSlice, From ce66adc92ebf254d5f6b675347b9fd15eab46469 Mon Sep 17 00:00:00 2001 From: Damon Earp Date: Thu, 11 Feb 2021 14:58:16 -0600 Subject: [PATCH 06/19] fixed indentation problem --- path_login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/path_login.go b/path_login.go index 7b857fbd..0d7ba805 100644 --- a/path_login.go +++ b/path_login.go @@ -109,7 +109,7 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d var name = serviceAccount.uid() if role.HumanReadableAlias { - name = fmt.Sprintf("%s/%s", serviceAccount.namespace(), serviceAccount.name()) + name = fmt.Sprintf("%s/%s", serviceAccount.namespace(), serviceAccount.name()) } auth := &logical.Auth{ From 6157db0bf5607ab858b17367e01cd307238748f5 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Mon, 30 Aug 2021 09:35:22 -0400 Subject: [PATCH 07/19] fix: path_role.go formatting. --- path_role.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/path_role.go b/path_role.go index 2448ad6d..54ace20d 100644 --- a/path_role.go +++ b/path_role.go @@ -50,7 +50,7 @@ are allowed.`, Description: "Optional Audience claim to verify in the jwt.", }, "human_readable_alias": { - Type: framework.TypeBool, + Type: framework.TypeBool, Description: `Use "Kubernete's Namespace/Service Account Name" instead of the UID for the alias name.`, }, "policies": { From 60041670fcc905e906c4bbd03096b5b7aa253107 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Mon, 30 Aug 2021 10:05:48 -0400 Subject: [PATCH 08/19] fix: update path role tests --- path_role_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/path_role_test.go b/path_role_test.go index b54e3774..389c2960 100644 --- a/path_role_test.go +++ b/path_role_test.go @@ -201,6 +201,7 @@ func TestPath_Read(t *testing.T) { "token_type": logical.TokenTypeDefault.String(), "token_explicit_max_ttl": int64(0), "token_no_default_policy": false, + "human_readable_alias": false, } req := &logical.Request{ From c5d25e2905daecabf3ff89f215612a9d16621d4b Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Thu, 2 Sep 2021 11:48:29 -0400 Subject: [PATCH 09/19] fix: add support for configuring alias name source --- backend.go | 44 ++++++++++++++++++++++++++++++++++++++++++-- path_login.go | 22 ++++++++++++++++------ path_role.go | 20 ++++++++++++-------- path_role_test.go | 3 ++- 4 files changed, 72 insertions(+), 17 deletions(-) diff --git a/backend.go b/backend.go index 8787184e..72dae48e 100644 --- a/backend.go +++ b/backend.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "sort" "strings" "sync" @@ -12,10 +13,20 @@ import ( ) const ( - configPath string = "config" - rolePrefix string = "role/" + configPath = "config" + rolePrefix = "role/" + + aliasNameSourceSAToken = "sa_token" + aliasNameSourceSAPath = "sa_path" + aliasNameSourceDefault = aliasNameSourceSAToken ) +// map alias name source to its description +var aliasNameSourceMap = map[string]string{ + aliasNameSourceSAToken: "format: ", + aliasNameSourceSAPath: "format: /", +} + // kubeAuthBackend implements logical.Backend type kubeAuthBackend struct { *framework.Backend @@ -128,10 +139,39 @@ func (b *kubeAuthBackend) role(ctx context.Context, s logical.Storage, name stri if len(role.TokenBoundCIDRs) == 0 && len(role.BoundCIDRs) > 0 { role.TokenBoundCIDRs = role.BoundCIDRs } + if role.AliasNameSource == "" { + role.AliasNameSource = aliasNameSourceDefault + } return role, nil } +func validateAliasNameSource(source string) error { + sources := make([]string, len(aliasNameSourceMap)) + i := 0 + for s := range aliasNameSourceMap { + if s == source { + return nil + } + sources[i] = s + i++ + } + sort.Strings(sources) + return fmt.Errorf(`invalid alias_name_source, must be one of: %s`, strings.Join(sources, ", ")) +} + +func getAliasNameSourceDesc() string { + var desc []string + for s, d := range aliasNameSourceMap { + if s == aliasNameSourceDefault { + d = d + " [default]" + } + desc = append(desc, fmt.Sprintf("%q (%s)", s, d)) + } + sort.Strings(desc) + return strings.Join(desc, ", ") +} + var backendHelp string = ` The Kubernetes Auth Backend allows authentication for Kubernetes service accounts. ` diff --git a/path_login.go b/path_login.go index 0d7ba805..d2dfe9ff 100644 --- a/path_login.go +++ b/path_login.go @@ -95,11 +95,26 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d return nil, errors.New("could not load backend configuration") } + if err := validateAliasNameSource(role.AliasNameSource); err != nil { + b.Logger().Error(err.Error()) + return nil, err + } + serviceAccount, err := b.parseAndValidateJWT(jwtStr, role, config) if err != nil { return nil, err } + var aliasName string + switch role.AliasNameSource { + case aliasNameSourceSAToken: + aliasName = serviceAccount.UID + case aliasNameSourceSAPath: + aliasName = fmt.Sprintf("%s/%s", serviceAccount.Namespace, serviceAccount.Name) + default: + return nil, fmt.Errorf("unknown alias_name_source %q", role.AliasNameSource) + } + // look up the JWT token in the kubernetes API err = serviceAccount.lookup(ctx, jwtStr, b.reviewFactory(config)) if err != nil { @@ -107,14 +122,9 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d return nil, logical.ErrPermissionDenied } - var name = serviceAccount.uid() - if role.HumanReadableAlias { - name = fmt.Sprintf("%s/%s", serviceAccount.namespace(), serviceAccount.name()) - } - auth := &logical.Auth{ Alias: &logical.Alias{ - Name: name, + Name: aliasName, Metadata: map[string]string{ "service_account_uid": serviceAccount.uid(), "service_account_name": serviceAccount.name(), diff --git a/path_role.go b/path_role.go index 54ace20d..451f2f63 100644 --- a/path_role.go +++ b/path_role.go @@ -49,9 +49,10 @@ are allowed.`, Type: framework.TypeString, Description: "Optional Audience claim to verify in the jwt.", }, - "human_readable_alias": { - Type: framework.TypeBool, - Description: `Use "Kubernete's Namespace/Service Account Name" instead of the UID for the alias name.`, + "alias_name_source": { + Type: framework.TypeString, + Description: fmt.Sprintf(`Source to use when deriving the Alias' name, valid choices: %s`, getAliasNameSourceDesc()), + Default: aliasNameSourceDefault, }, "policies": { Type: framework.TypeCommaStringSlice, @@ -156,7 +157,7 @@ func (b *kubeAuthBackend) pathRoleRead(ctx context.Context, req *logical.Request d["audience"] = role.Audience } - d["human_readable_alias"] = role.HumanReadableAlias + d["alias_name_source"] = role.AliasNameSource role.PopulateTokenData(d) @@ -308,8 +309,11 @@ func (b *kubeAuthBackend) pathRoleCreateUpdate(ctx context.Context, req *logical role.Audience = audience.(string) } - if humanReadableAlias, ok := data.GetOk("human_readable_alias"); ok { - role.HumanReadableAlias = humanReadableAlias.(bool) + if source, ok := data.GetOk("alias_name_source"); ok { + if err := validateAliasNameSource(source.(string)); err != nil { + return logical.ErrorResponse(err.Error()), nil + } + role.AliasNameSource = source.(string) } // Store the entry. @@ -342,8 +346,8 @@ type roleStorageEntry struct { // Audience is an optional jwt claim to verify Audience string `json:"audience" mapstructure:"audience" structs: "audience"` - // use the service accounts 'namespace/name' instead of uid for the alias name - HumanReadableAlias bool `json:"human_readable_alias" mapstructure:"human_readable_alias" structs:"human_readable_alias"` + // AliasNameSource used when deriving the Alias' name. + AliasNameSource string `json:"alias_name_source" mapstructure:"alias_name_source" structs:"alias_name_source"` // Deprecated by TokenParams Policies []string `json:"policies" structs:"policies" mapstructure:"policies"` diff --git a/path_role_test.go b/path_role_test.go index 389c2960..2191489f 100644 --- a/path_role_test.go +++ b/path_role_test.go @@ -64,6 +64,7 @@ func TestPath_Create(t *testing.T) { MaxTTL: 5 * time.Second, NumUses: 12, BoundCIDRs: nil, + AliasNameSource: aliasNameSourceDefault, } req := &logical.Request{ @@ -201,7 +202,7 @@ func TestPath_Read(t *testing.T) { "token_type": logical.TokenTypeDefault.String(), "token_explicit_max_ttl": int64(0), "token_no_default_policy": false, - "human_readable_alias": false, + "alias_name_source": aliasNameSourceDefault, } req := &logical.Request{ From 21f8d7604b24b681cdd4cff02022f5f8d1957d2a Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Thu, 2 Sep 2021 13:38:55 -0400 Subject: [PATCH 10/19] fix: extend tests --- path_login_test.go | 79 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 56 insertions(+), 23 deletions(-) diff --git a/path_login_test.go b/path_login_test.go index 97478187..7082d97c 100644 --- a/path_login_test.go +++ b/path_login_test.go @@ -31,18 +31,28 @@ var ( testNoPEMs = []string{testECCert, testRSACert} ) -func setupBackend(t *testing.T, pems []string, saName string, saNamespace string) (logical.Backend, logical.Storage) { - b, storage := getBackend(t) +type testBackendConfig struct { + pems []string + saName string + saNamespace string + aliasNameSource string +} + +func defaultTestBackendConfig() *testBackendConfig { + return &testBackendConfig{ + pems: testDefaultPEMs, + saName: testName, + saNamespace: testNamespace, + aliasNameSource: aliasNameSourceDefault, + } +} - // pems := []string{testECCert, testRSACert, testMinikubePubKey} - // pems := []string{testECCert, testRSACert} - // if noPEMs { - // pems = []string{} - // } +func setupBackend(t *testing.T, config *testBackendConfig) (logical.Backend, logical.Storage) { + b, storage := getBackend(t) // test no certificate data := map[string]interface{}{ - "pem_keys": pems, + "pem_keys": config.pems, "kubernetes_host": "host", "kubernetes_ca_cert": testCACert, } @@ -60,13 +70,14 @@ func setupBackend(t *testing.T, pems []string, saName string, saNamespace string } data = map[string]interface{}{ - "bound_service_account_names": saName, - "bound_service_account_namespaces": saNamespace, + "bound_service_account_names": config.saName, + "bound_service_account_namespaces": config.saNamespace, "policies": "test", "period": "3s", "ttl": "1s", "num_uses": 12, "max_ttl": "5s", + "alias_name_source": config.aliasNameSource, } req = &logical.Request{ @@ -86,7 +97,7 @@ func setupBackend(t *testing.T, pems []string, saName string, saNamespace string } func TestLogin(t *testing.T) { - b, storage := setupBackend(t, testDefaultPEMs, testName, testNamespace) + b, storage := setupBackend(t, defaultTestBackendConfig()) // Test bad inputs data := map[string]interface{}{ @@ -217,7 +228,9 @@ func TestLogin(t *testing.T) { } // test successful login for globbed name - b, storage = setupBackend(t, testDefaultPEMs, testGlobbedName, testNamespace) + config := defaultTestBackendConfig() + config.saName = testGlobbedName + b, storage = setupBackend(t, config) data = map[string]interface{}{ "role": "plugin-test", @@ -240,7 +253,9 @@ func TestLogin(t *testing.T) { } // test successful login for globbed namespace - b, storage = setupBackend(t, testDefaultPEMs, testName, testGlobbedNamespace) + config = defaultTestBackendConfig() + config.saNamespace = testGlobbedNamespace + b, storage = setupBackend(t, config) data = map[string]interface{}{ "role": "plugin-test", @@ -264,7 +279,7 @@ func TestLogin(t *testing.T) { } func TestLogin_ContextError(t *testing.T) { - b, storage := setupBackend(t, testDefaultPEMs, testName, testNamespace) + b, storage := setupBackend(t, defaultTestBackendConfig()) data := map[string]interface{}{ "role": "plugin-test", @@ -291,7 +306,9 @@ func TestLogin_ContextError(t *testing.T) { } func TestLogin_ECDSA_PEM(t *testing.T) { - b, storage := setupBackend(t, testNoPEMs, testName, testNamespace) + config := defaultTestBackendConfig() + config.pems = testNoPEMs + b, storage := setupBackend(t, config) // test no certificate data := map[string]interface{}{ @@ -335,7 +352,9 @@ func TestLogin_ECDSA_PEM(t *testing.T) { } func TestLogin_NoPEMs(t *testing.T) { - b, storage := setupBackend(t, testNoPEMs, testName, testNamespace) + config := defaultTestBackendConfig() + config.pems = testNoPEMs + b, storage := setupBackend(t, config) // test bad jwt service account data := map[string]interface{}{ @@ -383,7 +402,10 @@ func TestLogin_NoPEMs(t *testing.T) { } func TestLoginSvcAcctAndNamespaceSplats(t *testing.T) { - b, storage := setupBackend(t, testDefaultPEMs, "*", "*") + config := defaultTestBackendConfig() + config.saName = "*" + config.saNamespace = "*" + b, storage := setupBackend(t, config) // Test bad inputs data := map[string]interface{}{ @@ -514,7 +536,9 @@ func TestLoginSvcAcctAndNamespaceSplats(t *testing.T) { } // test successful login for globbed name - b, storage = setupBackend(t, testDefaultPEMs, testGlobbedName, testNamespace) + config = defaultTestBackendConfig() + config.saName = testGlobbedName + b, storage = setupBackend(t, config) data = map[string]interface{}{ "role": "plugin-test", @@ -537,7 +561,9 @@ func TestLoginSvcAcctAndNamespaceSplats(t *testing.T) { } // test successful login for globbed namespace - b, storage = setupBackend(t, testDefaultPEMs, testName, testGlobbedNamespace) + config = defaultTestBackendConfig() + config.saNamespace = testGlobbedNamespace + b, storage = setupBackend(t, config) data = map[string]interface{}{ "role": "plugin-test", @@ -561,7 +587,7 @@ func TestLoginSvcAcctAndNamespaceSplats(t *testing.T) { } func TestAliasLookAhead(t *testing.T) { - b, storage := setupBackend(t, testDefaultPEMs, testName, testNamespace) + b, storage := setupBackend(t, defaultTestBackendConfig()) // Test bad inputs data := map[string]interface{}{ @@ -589,7 +615,9 @@ func TestAliasLookAhead(t *testing.T) { } func TestLoginIssValidation(t *testing.T) { - b, storage := setupBackend(t, testNoPEMs, testName, testNamespace) + config := defaultTestBackendConfig() + config.pems = testNoPEMs + b, storage := setupBackend(t, config) // test iss validation enabled with default "kubernetes/serviceaccount" issuer data := map[string]interface{}{ @@ -768,7 +796,9 @@ Pk9Yf9rIf374m5XP1U8q79dBhLSIuaojsvOT39UUcPJROSD1FqYLued0rXiooIii -----END PUBLIC KEY-----` func TestLoginProjectedToken(t *testing.T) { - b, storage := setupBackend(t, append(testDefaultPEMs, testMinikubePubKey), testName, testNamespace) + config := defaultTestBackendConfig() + config.pems = append(testDefaultPEMs, testMinikubePubKey) + b, storage := setupBackend(t, config) // update backend to accept "default" bound account name data := map[string]interface{}{ @@ -879,7 +909,10 @@ func TestLoginProjectedToken(t *testing.T) { } func TestAliasLookAheadProjectedToken(t *testing.T) { - b, storage := setupBackend(t, append(testDefaultPEMs, testMinikubePubKey), "default", testNamespace) + config := defaultTestBackendConfig() + config.pems = append(testDefaultPEMs, testMinikubePubKey) + config.saName = "default" + b, storage := setupBackend(t, config) data := map[string]interface{}{ "jwt": jwtProjectedData, From 4ecb7abf483096a5ab1e2d5d86fad18c17fcf5bf Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Fri, 3 Sep 2021 09:03:45 -0400 Subject: [PATCH 11/19] fix: refactor validation of alias name source --- backend.go | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/backend.go b/backend.go index 72dae48e..13da5e6c 100644 --- a/backend.go +++ b/backend.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "sort" "strings" "sync" @@ -21,11 +20,15 @@ const ( aliasNameSourceDefault = aliasNameSourceSAToken ) -// map alias name source to its description -var aliasNameSourceMap = map[string]string{ - aliasNameSourceSAToken: "format: ", - aliasNameSourceSAPath: "format: /", -} +var ( + aliasNameSources = []string{aliasNameSourceSAToken, aliasNameSourceSAPath} + // map alias name source to its description + aliasNameSourceMap = map[string]string{ + aliasNameSourceSAToken: "format: ", + aliasNameSourceSAPath: "format: /", + } + errInvalidAliasNameSource = fmt.Errorf(`invalid alias_name_source, must be one of: %s`, strings.Join(aliasNameSources, ", ")) +) // kubeAuthBackend implements logical.Backend type kubeAuthBackend struct { @@ -147,28 +150,23 @@ func (b *kubeAuthBackend) role(ctx context.Context, s logical.Storage, name stri } func validateAliasNameSource(source string) error { - sources := make([]string, len(aliasNameSourceMap)) - i := 0 for s := range aliasNameSourceMap { if s == source { return nil } - sources[i] = s - i++ } - sort.Strings(sources) - return fmt.Errorf(`invalid alias_name_source, must be one of: %s`, strings.Join(sources, ", ")) + return errInvalidAliasNameSource } func getAliasNameSourceDesc() string { - var desc []string - for s, d := range aliasNameSourceMap { + var desc = make([]string, len(aliasNameSources)) + for i, s := range aliasNameSources { + d := aliasNameSourceMap[s] if s == aliasNameSourceDefault { d = d + " [default]" } - desc = append(desc, fmt.Sprintf("%q (%s)", s, d)) + desc[i] = fmt.Sprintf("%q (%s)", s, d) } - sort.Strings(desc) return strings.Join(desc, ", ") } From 23ce29fe0a96069475e173c17039b39ab8b7e18c Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Fri, 3 Sep 2021 09:14:19 -0400 Subject: [PATCH 12/19] fix: use serviceAccount.uid() --- path_login.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/path_login.go b/path_login.go index d2dfe9ff..859992e5 100644 --- a/path_login.go +++ b/path_login.go @@ -108,7 +108,7 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d var aliasName string switch role.AliasNameSource { case aliasNameSourceSAToken: - aliasName = serviceAccount.UID + aliasName = serviceAccount.uid() case aliasNameSourceSAPath: aliasName = fmt.Sprintf("%s/%s", serviceAccount.Namespace, serviceAccount.Name) default: From 40ecafed3570dcfd269833c7834264757cbed356 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Fri, 3 Sep 2021 15:36:39 -0400 Subject: [PATCH 13/19] fix: refactor alias_name_source validation on auth --- backend.go | 7 +++---- path_login.go | 27 ++++++++++++++------------- path_role.go | 8 ++++++-- path_role_test.go | 1 + 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/backend.go b/backend.go index 13da5e6c..a85a1f4d 100644 --- a/backend.go +++ b/backend.go @@ -15,6 +15,8 @@ const ( configPath = "config" rolePrefix = "role/" + // aliasNameSourceUnset provides backwards compatibility with preexisting roles. + aliasNameSourceUnset = "" aliasNameSourceSAToken = "sa_token" aliasNameSourceSAPath = "sa_path" aliasNameSourceDefault = aliasNameSourceSAToken @@ -142,15 +144,12 @@ func (b *kubeAuthBackend) role(ctx context.Context, s logical.Storage, name stri if len(role.TokenBoundCIDRs) == 0 && len(role.BoundCIDRs) > 0 { role.TokenBoundCIDRs = role.BoundCIDRs } - if role.AliasNameSource == "" { - role.AliasNameSource = aliasNameSourceDefault - } return role, nil } func validateAliasNameSource(source string) error { - for s := range aliasNameSourceMap { + for _, s := range aliasNameSources { if s == source { return nil } diff --git a/path_login.go b/path_login.go index 859992e5..0f6c81e5 100644 --- a/path_login.go +++ b/path_login.go @@ -95,24 +95,14 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d return nil, errors.New("could not load backend configuration") } - if err := validateAliasNameSource(role.AliasNameSource); err != nil { - b.Logger().Error(err.Error()) - return nil, err - } - serviceAccount, err := b.parseAndValidateJWT(jwtStr, role, config) if err != nil { return nil, err } - var aliasName string - switch role.AliasNameSource { - case aliasNameSourceSAToken: - aliasName = serviceAccount.uid() - case aliasNameSourceSAPath: - aliasName = fmt.Sprintf("%s/%s", serviceAccount.Namespace, serviceAccount.Name) - default: - return nil, fmt.Errorf("unknown alias_name_source %q", role.AliasNameSource) + aliasName, err := b.getAliasName(role, serviceAccount) + if err != nil { + return nil, err } // look up the JWT token in the kubernetes API @@ -152,6 +142,17 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d }, nil } +func (b *kubeAuthBackend) getAliasName(role *roleStorageEntry, serviceAccount *serviceAccount) (string, error) { + switch role.AliasNameSource { + case aliasNameSourceSAToken, aliasNameSourceUnset: + return serviceAccount.uid(), nil + case aliasNameSourceSAPath: + return fmt.Sprintf("%s/%s", serviceAccount.Namespace, serviceAccount.Name), nil + default: + return "", fmt.Errorf("unknown alias_name_source %q", role.AliasNameSource) + } +} + // aliasLookahead returns the alias object with the SA UID from the JWT // Claims. func (b *kubeAuthBackend) aliasLookahead(_ context.Context, _ *logical.Request, data *framework.FieldData) (*logical.Response, error) { diff --git a/path_role.go b/path_role.go index 451f2f63..98ebdd27 100644 --- a/path_role.go +++ b/path_role.go @@ -157,8 +157,6 @@ func (b *kubeAuthBackend) pathRoleRead(ctx context.Context, req *logical.Request d["audience"] = role.Audience } - d["alias_name_source"] = role.AliasNameSource - role.PopulateTokenData(d) if len(role.Policies) > 0 { @@ -180,6 +178,8 @@ func (b *kubeAuthBackend) pathRoleRead(ctx context.Context, req *logical.Request d["num_uses"] = role.NumUses } + d["alias_name_source"] = role.AliasNameSource + return &logical.Response{ Data: d, }, nil @@ -314,6 +314,10 @@ func (b *kubeAuthBackend) pathRoleCreateUpdate(ctx context.Context, req *logical return logical.ErrorResponse(err.Error()), nil } role.AliasNameSource = source.(string) + } else if role.AliasNameSource == aliasNameSourceUnset { + if s, ok := data.Schema["alias_name_source"]; ok { + role.AliasNameSource = s.Default.(string) + } } // Store the entry. diff --git a/path_role_test.go b/path_role_test.go index 2191489f..c52e4db4 100644 --- a/path_role_test.go +++ b/path_role_test.go @@ -45,6 +45,7 @@ func TestPath_Create(t *testing.T) { "ttl": "1s", "num_uses": 12, "max_ttl": "5s", + "alias_name_source": aliasNameSourceDefault, } expected := &roleStorageEntry{ From b423a93a2f7347795273aedbaef384987a3b7150 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Tue, 7 Sep 2021 10:43:24 -0400 Subject: [PATCH 14/19] fix: aliaslookahead:ssupport alternate alias name. --- path_login.go | 51 ++++++++++++++++++++++++++++++++++------------ path_login_test.go | 8 +++++--- path_role.go | 4 +--- 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/path_login.go b/path_login.go index 0f6c81e5..78ede8aa 100644 --- a/path_login.go +++ b/path_login.go @@ -55,14 +55,14 @@ func pathLogin(b *kubeAuthBackend) *framework.Path { // pathLogin is used to authenticate to this backend func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - roleName := data.Get("role").(string) - if len(roleName) == 0 { - return logical.ErrorResponse("missing role"), nil + roleName, resp := b.getFieldValueStr(data, "role") + if resp != nil { + return resp, nil } - jwtStr := data.Get("jwt").(string) - if len(jwtStr) == 0 { - return logical.ErrorResponse("missing jwt"), nil + jwtStr, resp := b.getFieldValueStr(data, "jwt") + if resp != nil { + return resp, nil } b.l.RLock() @@ -142,6 +142,14 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d }, nil } +func (b *kubeAuthBackend) getFieldValueStr(data *framework.FieldData, param string) (string, *logical.Response) { + val := data.Get(param).(string) + if len(val) == 0 { + return "", logical.ErrorResponse("missing %s", param) + } + return val, nil +} + func (b *kubeAuthBackend) getAliasName(role *roleStorageEntry, serviceAccount *serviceAccount) (string, error) { switch role.AliasNameSource { case aliasNameSourceSAToken, aliasNameSourceUnset: @@ -155,10 +163,23 @@ func (b *kubeAuthBackend) getAliasName(role *roleStorageEntry, serviceAccount *s // aliasLookahead returns the alias object with the SA UID from the JWT // Claims. -func (b *kubeAuthBackend) aliasLookahead(_ context.Context, _ *logical.Request, data *framework.FieldData) (*logical.Response, error) { - jwtStr := data.Get("jwt").(string) - if len(jwtStr) == 0 { - return logical.ErrorResponse("missing jwt"), nil +func (b *kubeAuthBackend) aliasLookahead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { + roleName, resp := b.getFieldValueStr(data, "role") + if resp != nil { + return resp, nil + } + + jwtStr, resp := b.getFieldValueStr(data, "jwt") + if resp != nil { + return resp, nil + } + + role, err := b.role(ctx, req.Storage, roleName) + if err != nil { + return nil, err + } + if role == nil { + return logical.ErrorResponse(fmt.Sprintf("invalid role name \"%s\"", roleName)), nil } // Parse into JWT @@ -174,15 +195,19 @@ func (b *kubeAuthBackend) aliasLookahead(_ context.Context, _ *logical.Request, return nil, err } - saUID := sa.uid() - if saUID == "" { + if sa.uid() == "" { return nil, errors.New("could not parse UID from claims") } + aliasName, err := b.getAliasName(role, sa) + if err != nil { + return nil, err + } + return &logical.Response{ Auth: &logical.Auth{ Alias: &logical.Alias{ - Name: saUID, + Name: aliasName, }, }, }, nil diff --git a/path_login_test.go b/path_login_test.go index 7082d97c..e2d45560 100644 --- a/path_login_test.go +++ b/path_login_test.go @@ -591,7 +591,8 @@ func TestAliasLookAhead(t *testing.T) { // Test bad inputs data := map[string]interface{}{ - "jwt": jwtData, + "jwt": jwtData, + "role": "plugin-test", } req := &logical.Request{ @@ -915,7 +916,8 @@ func TestAliasLookAheadProjectedToken(t *testing.T) { b, storage := setupBackend(t, config) data := map[string]interface{}{ - "jwt": jwtProjectedData, + "jwt": jwtProjectedData, + "role": "plugin-test", } req := &logical.Request{ @@ -933,7 +935,7 @@ func TestAliasLookAheadProjectedToken(t *testing.T) { t.Fatalf("err:%s resp:%#v\n", err, resp) } - if resp.Auth.Alias.Name != "77c81ad7-1bea-4d94-9ca5-f5d7f3632331" { + if resp.Auth.Alias.Name != testProjectedUID { t.Fatalf("Unexpected UID: %s", resp.Auth.Alias.Name) } } diff --git a/path_role.go b/path_role.go index 98ebdd27..7407f9b5 100644 --- a/path_role.go +++ b/path_role.go @@ -315,9 +315,7 @@ func (b *kubeAuthBackend) pathRoleCreateUpdate(ctx context.Context, req *logical } role.AliasNameSource = source.(string) } else if role.AliasNameSource == aliasNameSourceUnset { - if s, ok := data.Schema["alias_name_source"]; ok { - role.AliasNameSource = s.Default.(string) - } + role.AliasNameSource = data.Get("alias_name_source").(string) } // Store the entry. From b7738b1ec2d62cbde11ba6377baa9a3b853722a0 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Tue, 7 Sep 2021 10:45:15 -0400 Subject: [PATCH 15/19] fix: move sa.UID validation to sa.uid() --- path_login.go | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/path_login.go b/path_login.go index 78ede8aa..ebc6f1ed 100644 --- a/path_login.go +++ b/path_login.go @@ -112,11 +112,15 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d return nil, logical.ErrPermissionDenied } + uid, err := serviceAccount.uid() + if err != nil { + return nil, err + } auth := &logical.Auth{ Alias: &logical.Alias{ Name: aliasName, Metadata: map[string]string{ - "service_account_uid": serviceAccount.uid(), + "service_account_uid": uid, "service_account_name": serviceAccount.name(), "service_account_namespace": serviceAccount.namespace(), "service_account_secret_name": serviceAccount.SecretName, @@ -126,7 +130,7 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d "role": roleName, }, Metadata: map[string]string{ - "service_account_uid": serviceAccount.uid(), + "service_account_uid": uid, "service_account_name": serviceAccount.name(), "service_account_namespace": serviceAccount.namespace(), "service_account_secret_name": serviceAccount.SecretName, @@ -153,7 +157,11 @@ func (b *kubeAuthBackend) getFieldValueStr(data *framework.FieldData, param stri func (b *kubeAuthBackend) getAliasName(role *roleStorageEntry, serviceAccount *serviceAccount) (string, error) { switch role.AliasNameSource { case aliasNameSourceSAToken, aliasNameSourceUnset: - return serviceAccount.uid(), nil + uid, err := serviceAccount.uid() + if err != nil { + return "", err + } + return uid, nil case aliasNameSourceSAPath: return fmt.Sprintf("%s/%s", serviceAccount.Namespace, serviceAccount.Name), nil default: @@ -195,10 +203,6 @@ func (b *kubeAuthBackend) aliasLookahead(ctx context.Context, req *logical.Reque return nil, err } - if sa.uid() == "" { - return nil, errors.New("could not parse UID from claims") - } - aliasName, err := b.getAliasName(role, sa) if err != nil { return nil, err @@ -357,11 +361,17 @@ type serviceAccount struct { // uid returns the UID for the service account, preferring the projected service // account value if found -func (s *serviceAccount) uid() string { +// return an error when the UID is empty. +func (s *serviceAccount) uid() (string, error) { + uid := s.UID if s.Kubernetes != nil && s.Kubernetes.ServiceAccount != nil { - return s.Kubernetes.ServiceAccount.UID + uid = s.Kubernetes.ServiceAccount.UID + } + + if uid == "" { + return "", errors.New("could not parse UID from claims") } - return s.UID + return uid, nil } // name returns the name for the service account, preferring the projected @@ -407,7 +417,11 @@ func (s *serviceAccount) lookup(ctx context.Context, jwtStr string, tr tokenRevi if s.name() != r.Name { return errors.New("JWT names did not match") } - if s.uid() != r.UID { + uid, err := s.uid() + if err != nil { + return err + } + if uid != r.UID { return errors.New("JWT UIDs did not match") } if s.namespace() != r.Namespace { From 57851c038bb85abff7176b5fc2e5234c86903eea Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Tue, 7 Sep 2021 14:55:36 -0400 Subject: [PATCH 16/19] fix: extend aliasLookahead tests. --- path_login_test.go | 105 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 84 insertions(+), 21 deletions(-) diff --git a/path_login_test.go b/path_login_test.go index e2d45560..9e268cd2 100644 --- a/path_login_test.go +++ b/path_login_test.go @@ -587,31 +587,94 @@ func TestLoginSvcAcctAndNamespaceSplats(t *testing.T) { } func TestAliasLookAhead(t *testing.T) { - b, storage := setupBackend(t, defaultTestBackendConfig()) - - // Test bad inputs - data := map[string]interface{}{ - "jwt": jwtData, - "role": "plugin-test", - } - - req := &logical.Request{ - Operation: logical.AliasLookaheadOperation, - Path: "login", - Storage: storage, - Data: data, - Connection: &logical.Connection{ - RemoteAddr: "127.0.0.1", + testCases := map[string]struct { + role string + jwt string + config *testBackendConfig + expectedAliasName string + wantErr error + }{ + "default": { + role: "plugin-test", + jwt: jwtData, + config: defaultTestBackendConfig(), + expectedAliasName: testUID, + }, + "no_role": { + jwt: jwtData, + config: defaultTestBackendConfig(), + wantErr: errors.New("missing role"), + }, + "no_jwt": { + role: "plugin-test", + config: defaultTestBackendConfig(), + wantErr: errors.New("missing jwt"), + }, + "sa_token": { + role: "plugin-test", + jwt: jwtData, + config: &testBackendConfig{ + pems: testDefaultPEMs, + saName: testName, + saNamespace: testNamespace, + aliasNameSource: aliasNameSourceSAToken, + }, + expectedAliasName: testUID, + }, + "sa_path": { + role: "plugin-test", + jwt: jwtData, + config: &testBackendConfig{ + pems: testDefaultPEMs, + saName: testName, + saNamespace: testNamespace, + aliasNameSource: aliasNameSourceSAPath, + }, + expectedAliasName: fmt.Sprintf("%s/%s", testNamespace, testName), }, } - resp, err := b.HandleRequest(context.Background(), req) - if err != nil || (resp != nil && resp.IsError()) { - t.Fatalf("err:%s resp:%#v\n", err, resp) - } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + b, storage := setupBackend(t, tc.config) - if resp.Auth.Alias.Name != testUID { - t.Fatalf("Unexpected UID: %s", resp.Auth.Alias.Name) + req := &logical.Request{ + Operation: logical.AliasLookaheadOperation, + Path: "login", + Storage: storage, + Data: map[string]interface{}{ + "jwt": tc.jwt, + "role": tc.role, + }, + Connection: &logical.Connection{ + RemoteAddr: "127.0.0.1", + }, + } + + resp, err := b.HandleRequest(context.Background(), req) + if tc.wantErr != nil { + var actual error + if err != nil { + actual = err + } else if resp != nil && resp.IsError() { + actual = resp.Error() + } else { + t.Fatalf("no error found") + } + + if tc.wantErr.Error() != actual.Error() { + t.Fatalf("expected err %q, actual %q", tc.wantErr, actual) + } + } else { + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + if resp.Auth.Alias.Name != tc.expectedAliasName { + t.Fatalf("expected Alias.Name %s, actual %s", tc.expectedAliasName, resp.Auth.Alias.Name) + } + } + }) } } From a3e230d9820150e5d75e8f4747afd54b23e9a4d4 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Fri, 10 Sep 2021 10:07:43 -0400 Subject: [PATCH 17/19] fix: test alias_name_source in TestPath_Create() --- path_login_test.go | 2 +- path_role_test.go | 283 ++++++++++++++++++++++++--------------------- 2 files changed, 152 insertions(+), 133 deletions(-) diff --git a/path_login_test.go b/path_login_test.go index 9e268cd2..a9eb5e2f 100644 --- a/path_login_test.go +++ b/path_login_test.go @@ -659,7 +659,7 @@ func TestAliasLookAhead(t *testing.T) { } else if resp != nil && resp.IsError() { actual = resp.Error() } else { - t.Fatalf("no error found") + t.Fatalf("expected error") } if tc.wantErr.Error() != actual.Error() { diff --git a/path_role_test.go b/path_role_test.go index c52e4db4..d67a01e3 100644 --- a/path_role_test.go +++ b/path_role_test.go @@ -2,6 +2,8 @@ package kubeauth import ( "context" + "errors" + "fmt" "testing" "time" @@ -35,141 +37,158 @@ func getBackend(t *testing.T) (logical.Backend, logical.Storage) { } func TestPath_Create(t *testing.T) { - b, storage := getBackend(t) - - data := map[string]interface{}{ - "bound_service_account_names": "name", - "bound_service_account_namespaces": "namespace", - "policies": "test", - "period": "3s", - "ttl": "1s", - "num_uses": 12, - "max_ttl": "5s", - "alias_name_source": aliasNameSourceDefault, - } - - expected := &roleStorageEntry{ - TokenParams: tokenutil.TokenParams{ - TokenPolicies: []string{"test"}, - TokenPeriod: 3 * time.Second, - TokenTTL: 1 * time.Second, - TokenMaxTTL: 5 * time.Second, - TokenNumUses: 12, - TokenBoundCIDRs: nil, + testCases := map[string]struct { + data map[string]interface{} + expected *roleStorageEntry + wantErr error + }{ + "default": { + data: map[string]interface{}{ + "bound_service_account_names": "name", + "bound_service_account_namespaces": "namespace", + "policies": "test", + "period": "3s", + "ttl": "1s", + "num_uses": 12, + "max_ttl": "5s", + "alias_name_source": aliasNameSourceDefault, + }, + expected: &roleStorageEntry{ + TokenParams: tokenutil.TokenParams{ + TokenPolicies: []string{"test"}, + TokenPeriod: 3 * time.Second, + TokenTTL: 1 * time.Second, + TokenMaxTTL: 5 * time.Second, + TokenNumUses: 12, + TokenBoundCIDRs: nil, + }, + Policies: []string{"test"}, + Period: 3 * time.Second, + ServiceAccountNames: []string{"name"}, + ServiceAccountNamespaces: []string{"namespace"}, + TTL: 1 * time.Second, + MaxTTL: 5 * time.Second, + NumUses: 12, + BoundCIDRs: nil, + AliasNameSource: aliasNameSourceDefault, + }, + }, + "alias_name_source_sa_path": { + data: map[string]interface{}{ + "bound_service_account_names": "name", + "bound_service_account_namespaces": "namespace", + "policies": "test", + "period": "3s", + "ttl": "1s", + "num_uses": 12, + "max_ttl": "5s", + "alias_name_source": aliasNameSourceSAPath, + }, + expected: &roleStorageEntry{ + TokenParams: tokenutil.TokenParams{ + TokenPolicies: []string{"test"}, + TokenPeriod: 3 * time.Second, + TokenTTL: 1 * time.Second, + TokenMaxTTL: 5 * time.Second, + TokenNumUses: 12, + TokenBoundCIDRs: nil, + }, + Policies: []string{"test"}, + Period: 3 * time.Second, + ServiceAccountNames: []string{"name"}, + ServiceAccountNamespaces: []string{"namespace"}, + TTL: 1 * time.Second, + MaxTTL: 5 * time.Second, + NumUses: 12, + BoundCIDRs: nil, + AliasNameSource: aliasNameSourceSAPath, + }, + }, + "invalid_alias_name_source": { + data: map[string]interface{}{ + "bound_service_account_names": "name", + "bound_service_account_namespaces": "namespace", + "policies": "test", + "period": "3s", + "ttl": "1s", + "num_uses": 12, + "max_ttl": "5s", + "alias_name_source": "_invalid_", + }, + wantErr: errInvalidAliasNameSource, + }, + "no_service_account_names": { + data: map[string]interface{}{ + "policies": "test", + }, + wantErr: errors.New(`"bound_service_account_names" can not be empty`), + }, + "no_service_account_namespaces": { + data: map[string]interface{}{ + "bound_service_account_names": "name", + "policies": "test", + }, + wantErr: errors.New(`"bound_service_account_namespaces" can not be empty`), + }, + "mixed_splat_values_names": { + data: map[string]interface{}{ + "bound_service_account_names": "*, test", + "bound_service_account_namespaces": "*", + "policies": "test", + }, + wantErr: errors.New(`can not mix "*" with values`), + }, + "mixed_splat_values_namespaces": { + data: map[string]interface{}{ + "bound_service_account_names": "*, test", + "bound_service_account_namespaces": "*", + "policies": "test", + }, + wantErr: errors.New(`can not mix "*" with values`), }, - Policies: []string{"test"}, - Period: 3 * time.Second, - ServiceAccountNames: []string{"name"}, - ServiceAccountNamespaces: []string{"namespace"}, - TTL: 1 * time.Second, - MaxTTL: 5 * time.Second, - NumUses: 12, - BoundCIDRs: nil, - AliasNameSource: aliasNameSourceDefault, - } - - req := &logical.Request{ - Operation: logical.CreateOperation, - Path: "role/plugin-test", - Storage: storage, - Data: data, - } - - resp, err := b.HandleRequest(context.Background(), req) - if err != nil || (resp != nil && resp.IsError()) { - t.Fatalf("err:%s resp:%#v\n", err, resp) - } - actual, err := b.(*kubeAuthBackend).role(context.Background(), storage, "plugin-test") - if err != nil { - t.Fatal(err) - } - - if diff := deep.Equal(expected, actual); diff != nil { - t.Fatal(diff) - } - - // Test no service account info - data = map[string]interface{}{ - "policies": "test", - } - - req = &logical.Request{ - Operation: logical.CreateOperation, - Path: "role/test2", - Storage: storage, - Data: data, - } - - resp, err = b.HandleRequest(context.Background(), req) - if resp != nil && !resp.IsError() { - t.Fatalf("expected error") - } - if resp.Error().Error() != "\"bound_service_account_names\" can not be empty" { - t.Fatalf("unexpected err: %v", resp) - } - - // Test no service account info - data = map[string]interface{}{ - "bound_service_account_names": "name", - "policies": "test", - } - - req = &logical.Request{ - Operation: logical.CreateOperation, - Path: "role/test2", - Storage: storage, - Data: data, - } - - resp, err = b.HandleRequest(context.Background(), req) - if resp != nil && !resp.IsError() { - t.Fatalf("expected error") - } - if resp.Error().Error() != "\"bound_service_account_namespaces\" can not be empty" { - t.Fatalf("unexpected err: %v", resp) - } - - // Test mixed "*" and values - data = map[string]interface{}{ - "bound_service_account_names": "*, test", - "bound_service_account_namespaces": "*", - "policies": "test", - } - - req = &logical.Request{ - Operation: logical.CreateOperation, - Path: "role/test2", - Storage: storage, - Data: data, - } - - resp, err = b.HandleRequest(context.Background(), req) - if resp == nil || !resp.IsError() { - t.Fatalf("expected error") - } - if resp.Error().Error() != "can not mix \"*\" with values" { - t.Fatalf("unexpected err: %v", resp) - } - - data = map[string]interface{}{ - "bound_service_account_names": "*", - "bound_service_account_namespaces": "*, test", - "policies": "test", - } - - req = &logical.Request{ - Operation: logical.CreateOperation, - Path: "role/test2", - Storage: storage, - Data: data, } - resp, err = b.HandleRequest(context.Background(), req) - if resp == nil || !resp.IsError() { - t.Fatalf("expected error") - } - if resp.Error().Error() != "can not mix \"*\" with values" { - t.Fatalf("unexpected err: %v", resp) + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + b, storage := getBackend(t) + path := fmt.Sprintf("role/%s", name) + req := &logical.Request{ + Operation: logical.CreateOperation, + Path: path, + Storage: storage, + Data: tc.data, + } + + resp, err := b.HandleRequest(context.Background(), req) + + if tc.wantErr != nil { + var actual error + if err != nil { + actual = err + } else if resp != nil && resp.IsError() { + actual = resp.Error() + } else { + t.Fatalf("expected error") + } + + if tc.wantErr.Error() != actual.Error() { + t.Fatalf("expected err %q, actual %q", tc.wantErr, actual) + } + } else { + if tc.wantErr == nil && (err != nil || (resp != nil && resp.IsError())) { + t.Fatalf("err:%s resp:%#v\n", err, resp) + } + + actual, err := b.(*kubeAuthBackend).role(context.Background(), storage, name) + if err != nil { + t.Fatal(err) + } + + if diff := deep.Equal(tc.expected, actual); diff != nil { + t.Fatal(diff) + } + } + }) } } From 25d184220d8080fba882b1481db4b406771baf0a Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Mon, 13 Sep 2021 09:49:44 -0400 Subject: [PATCH 18/19] fix: use double-quoted string format --- path_login.go | 4 ++-- path_login_test.go | 8 ++++---- path_role.go | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/path_login.go b/path_login.go index ebc6f1ed..a1eaecea 100644 --- a/path_login.go +++ b/path_login.go @@ -73,7 +73,7 @@ func (b *kubeAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d return nil, err } if role == nil { - return logical.ErrorResponse(fmt.Sprintf("invalid role name \"%s\"", roleName)), nil + return logical.ErrorResponse(fmt.Sprintf("invalid role name %q", roleName)), nil } // Check for a CIDR match. @@ -187,7 +187,7 @@ func (b *kubeAuthBackend) aliasLookahead(ctx context.Context, req *logical.Reque return nil, err } if role == nil { - return logical.ErrorResponse(fmt.Sprintf("invalid role name \"%s\"", roleName)), nil + return logical.ErrorResponse(fmt.Sprintf("invalid role name %q", roleName)), nil } // Parse into JWT diff --git a/path_login_test.go b/path_login_test.go index a9eb5e2f..f049aca0 100644 --- a/path_login_test.go +++ b/path_login_test.go @@ -154,7 +154,7 @@ func TestLogin(t *testing.T) { if resp == nil || !resp.IsError() { t.Fatal("expected error") } - if resp.Error().Error() != "invalid role name \"plugin-test-bad\"" { + if resp.Error().Error() != `invalid role name "plugin-test-bad"` { t.Fatalf("unexpected error: %s", resp.Error()) } @@ -462,7 +462,7 @@ func TestLoginSvcAcctAndNamespaceSplats(t *testing.T) { if resp == nil || !resp.IsError() { t.Fatal("expected error") } - if resp.Error().Error() != "invalid role name \"plugin-test-bad\"" { + if resp.Error().Error() != `invalid role name "plugin-test-bad"` { t.Fatalf("unexpected error: %s", resp.Error()) } @@ -799,7 +799,7 @@ func TestLoginIssValidation(t *testing.T) { if err == nil { t.Fatal("expected error") } - if err.Error() != "claim \"iss\" is invalid" { + if err.Error() != `claim "iss" is invalid` { t.Fatalf("unexpected error: %s", err) } @@ -887,7 +887,7 @@ func TestLoginProjectedToken(t *testing.T) { t.Fatalf("err:%s resp:%#v\n", err, resp) } - var roleNameError = fmt.Errorf("invalid role name \"%s\"", "plugin-test-x") + var roleNameError = fmt.Errorf("invalid role name %q", "plugin-test-x") testCases := map[string]struct { role string diff --git a/path_role.go b/path_role.go index 7407f9b5..9d5cca18 100644 --- a/path_role.go +++ b/path_role.go @@ -283,11 +283,11 @@ func (b *kubeAuthBackend) pathRoleCreateUpdate(ctx context.Context, req *logical } // Verify names was not empty if len(role.ServiceAccountNames) == 0 { - return logical.ErrorResponse("\"bound_service_account_names\" can not be empty"), nil + return logical.ErrorResponse("%q can not be empty", "bound_service_account_names"), nil } // Verify * was not set with other data if len(role.ServiceAccountNames) > 1 && strutil.StrListContains(role.ServiceAccountNames, "*") { - return logical.ErrorResponse("can not mix \"*\" with values"), nil + return logical.ErrorResponse("can not mix %q with values", "*"), nil } if namespaces, ok := data.GetOk("bound_service_account_namespaces"); ok { @@ -297,11 +297,11 @@ func (b *kubeAuthBackend) pathRoleCreateUpdate(ctx context.Context, req *logical } // Verify namespaces is not empty if len(role.ServiceAccountNamespaces) == 0 { - return logical.ErrorResponse("\"bound_service_account_namespaces\" can not be empty"), nil + return logical.ErrorResponse("%q can not be empty", "bound_service_account_namespaces"), nil } // Verify * was not set with other data if len(role.ServiceAccountNamespaces) > 1 && strutil.StrListContains(role.ServiceAccountNamespaces, "*") { - return logical.ErrorResponse("can not mix \"*\" with values"), nil + return logical.ErrorResponse("can not mix %q with values", "*"), nil } // optional audience field From 9eb89bd209fe72c4a4d05615eba02b1c9824f176 Mon Sep 17 00:00:00 2001 From: Ben Ash Date: Mon, 13 Sep 2021 10:17:48 -0400 Subject: [PATCH 19/19] fix: drop dynamic alias_name_source description --- backend.go | 20 ++------------------ path_role.go | 11 ++++++++--- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/backend.go b/backend.go index a85a1f4d..b8c64bcf 100644 --- a/backend.go +++ b/backend.go @@ -23,12 +23,8 @@ const ( ) var ( - aliasNameSources = []string{aliasNameSourceSAToken, aliasNameSourceSAPath} - // map alias name source to its description - aliasNameSourceMap = map[string]string{ - aliasNameSourceSAToken: "format: ", - aliasNameSourceSAPath: "format: /", - } + // when adding new alias name sources make sure to update the corresponding FieldSchema description in path_role.go + aliasNameSources = []string{aliasNameSourceSAToken, aliasNameSourceSAPath} errInvalidAliasNameSource = fmt.Errorf(`invalid alias_name_source, must be one of: %s`, strings.Join(aliasNameSources, ", ")) ) @@ -157,18 +153,6 @@ func validateAliasNameSource(source string) error { return errInvalidAliasNameSource } -func getAliasNameSourceDesc() string { - var desc = make([]string, len(aliasNameSources)) - for i, s := range aliasNameSources { - d := aliasNameSourceMap[s] - if s == aliasNameSourceDefault { - d = d + " [default]" - } - desc[i] = fmt.Sprintf("%q (%s)", s, d) - } - return strings.Join(desc, ", ") -} - var backendHelp string = ` The Kubernetes Auth Backend allows authentication for Kubernetes service accounts. ` diff --git a/path_role.go b/path_role.go index 9d5cca18..7f3b3ed5 100644 --- a/path_role.go +++ b/path_role.go @@ -50,9 +50,14 @@ are allowed.`, Description: "Optional Audience claim to verify in the jwt.", }, "alias_name_source": { - Type: framework.TypeString, - Description: fmt.Sprintf(`Source to use when deriving the Alias' name, valid choices: %s`, getAliasNameSourceDesc()), - Default: aliasNameSourceDefault, + Type: framework.TypeString, + Description: fmt.Sprintf(`Source to use when deriving the Alias name. +valid choices: + %q : e.g. 474b11b5-0f20-4f9d-8ca5-65715ab325e0 (most secure choice) + %q : / e.g. vault/vault-agent +default: %q +`, aliasNameSourceSAToken, aliasNameSourceSAPath, aliasNameSourceDefault), + Default: aliasNameSourceDefault, }, "policies": { Type: framework.TypeCommaStringSlice,