Skip to content

Commit

Permalink
VAULT-2285 adding capability to accept comma separated entries for au… (
Browse files Browse the repository at this point in the history 
#12126)

* VAULT-2285 adding capability to accept comma separated entries for auth enable/tune

* Adding changelog

* Adding logic to detect invalid input parameter for auth enable config

* Updating tune.mdx

* Updating secret enable/tune for comma separated parameters

* Adding further parameter checks for auth/secret tests
Fixing changelog
using builtin type for a switch statement
Fixing a possible panic scenario

* Changing a function name, using deep.Equal instead of what reflect package provides

* Fixing auth/secret enable/tune mdx files

* One more mdx file fix

* Only when users provide a single comma separated string in a curl command, split the entries by commas

* Fixing API docs for auth/mount enable/tune for comma separated entries

* updating docs, removing an unnecessary switch case
  • Loading branch information
@hghaf099
hghaf099 committed Aug 9, 2021
1 parent 01e0aff commit dd294fc
Show file tree
Hide file tree
Showing 14 changed files with 265 additions and 54 deletions.
3 changes: 3 additions & 0 deletions changelog/12126.txt
View file
@@ -0,0 +1,3 @@
```release-note:bug
cli/api: Providing consistency for the use of comma separated parameters in auth/secret enable/tune
```
19 changes: 19 additions & 0 deletions command/auth_enable_test.go
View file
Expand Up @@ -5,6 +5,7 @@ import (
"strings"
"testing"

"github.com/go-test/deep"
"github.com/hashicorp/vault/helper/builtinplugins"
"github.com/hashicorp/vault/sdk/helper/consts"
"github.com/mitchellh/cli"
Expand Down Expand Up @@ -86,6 +87,12 @@ func TestAuthEnableCommand_Run(t *testing.T) {
code := cmd.Run([]string{
"-path", "auth_integration/",
"-description", "The best kind of test",
"-audit-non-hmac-request-keys", "foo,bar",
"-audit-non-hmac-response-keys", "foo,bar",
"-passthrough-request-headers", "authorization,authentication",
"-passthrough-request-headers", "www-authentication",
"-allowed-response-headers", "authorization",
"-listing-visibility", "unauth",
"userpass",
})
if exp := 0; code != exp {
Expand Down Expand Up @@ -113,6 +120,18 @@ func TestAuthEnableCommand_Run(t *testing.T) {
if exp := "The best kind of test"; authInfo.Description != exp {
t.Errorf("expected %q to be %q", authInfo.Description, exp)
}
if diff := deep.Equal([]string{"authorization,authentication", "www-authentication"}, authInfo.Config.PassthroughRequestHeaders); len(diff) > 0 {
t.Errorf("Failed to find expected values in PassthroughRequestHeaders. Difference is: %v", diff)
}
if diff := deep.Equal([]string{"authorization"}, authInfo.Config.AllowedResponseHeaders); len(diff) > 0 {
t.Errorf("Failed to find expected values in AllowedResponseHeaders. Difference is: %v", diff)
}
if diff := deep.Equal([]string{"foo,bar"}, authInfo.Config.AuditNonHMACRequestKeys); len(diff) > 0 {
t.Errorf("Failed to find expected values in AuditNonHMACRequestKeys. Difference is: %v", diff)
}
if diff := deep.Equal([]string{"foo,bar"}, authInfo.Config.AuditNonHMACResponseKeys); len(diff) > 0 {
t.Errorf("Failed to find expected values in AuditNonHMACResponseKeys. Difference is: %v", diff)
}
})

t.Run("communication_failure", func(t *testing.T) {
Expand Down
42 changes: 33 additions & 9 deletions command/auth_tune.go
View file
Expand Up @@ -20,15 +20,17 @@ var (
type AuthTuneCommand struct {
*BaseCommand

flagAuditNonHMACRequestKeys []string
flagAuditNonHMACResponseKeys []string
flagDefaultLeaseTTL time.Duration
flagDescription string
flagListingVisibility string
flagMaxLeaseTTL time.Duration
flagOptions map[string]string
flagTokenType string
flagVersion int
flagAuditNonHMACRequestKeys []string
flagAuditNonHMACResponseKeys []string
flagDefaultLeaseTTL time.Duration
flagDescription string
flagListingVisibility string
flagMaxLeaseTTL time.Duration
flagPassthroughRequestHeaders []string
flagAllowedResponseHeaders []string
flagOptions map[string]string
flagTokenType string
flagVersion int
}

func (c *AuthTuneCommand) Synopsis() string {
Expand Down Expand Up @@ -107,6 +109,20 @@ func (c *AuthTuneCommand) Flags() *FlagSets {
"or a previously configured value for the auth method.",
})

f.StringSliceVar(&StringSliceVar{
Name: flagNamePassthroughRequestHeaders,
Target: &c.flagPassthroughRequestHeaders,
Usage: "Comma-separated string or list of request header values that " +
"will be sent to the plugin",
})

f.StringSliceVar(&StringSliceVar{
Name: flagNameAllowedResponseHeaders,
Target: &c.flagAllowedResponseHeaders,
Usage: "Comma-separated string or list of response header values that " +
"plugins will be allowed to set",
})

f.StringMapVar(&StringMapVar{
Name: "options",
Target: &c.flagOptions,
Expand Down Expand Up @@ -194,6 +210,14 @@ func (c *AuthTuneCommand) Run(args []string) int {
mountConfigInput.ListingVisibility = c.flagListingVisibility
}

if fl.Name == flagNamePassthroughRequestHeaders {
mountConfigInput.PassthroughRequestHeaders = c.flagPassthroughRequestHeaders
}

if fl.Name == flagNameAllowedResponseHeaders {
mountConfigInput.AllowedResponseHeaders = c.flagAllowedResponseHeaders
}

if fl.Name == flagNameTokenType {
mountConfigInput.TokenType = c.flagTokenType
}
Expand Down
16 changes: 16 additions & 0 deletions command/auth_tune_test.go
View file
Expand Up @@ -4,6 +4,7 @@ import (
"strings"
"testing"

"github.com/go-test/deep"
"github.com/hashicorp/vault/api"
"github.com/mitchellh/cli"
)
Expand Down Expand Up @@ -92,6 +93,9 @@ func TestAuthTuneCommand_Run(t *testing.T) {
"-max-lease-ttl", "1h",
"-audit-non-hmac-request-keys", "foo,bar",
"-audit-non-hmac-response-keys", "foo,bar",
"-passthrough-request-headers", "authorization",
"-passthrough-request-headers", "www-authentication",
"-allowed-response-headers", "authorization,www-authentication",
"-listing-visibility", "unauth",
"my-auth/",
})
Expand Down Expand Up @@ -126,6 +130,18 @@ func TestAuthTuneCommand_Run(t *testing.T) {
if exp := 3600; mountInfo.Config.MaxLeaseTTL != exp {
t.Errorf("expected %d to be %d", mountInfo.Config.MaxLeaseTTL, exp)
}
if diff := deep.Equal([]string{"authorization", "www-authentication"}, mountInfo.Config.PassthroughRequestHeaders); len(diff) > 0 {
t.Errorf("Failed to find expected values in PassthroughRequestHeaders. Difference is: %v", diff)
}
if diff := deep.Equal([]string{"authorization,www-authentication"}, mountInfo.Config.AllowedResponseHeaders); len(diff) > 0 {
t.Errorf("Failed to find expected values in AllowedResponseHeaders. Difference is: %v", diff)
}
if diff := deep.Equal([]string{"foo,bar"}, mountInfo.Config.AuditNonHMACRequestKeys); len(diff) > 0 {
t.Errorf("Failed to find expected values in AuditNonHMACRequestKeys. Difference is: %v", diff)
}
if diff := deep.Equal([]string{"foo,bar"}, mountInfo.Config.AuditNonHMACResponseKeys); len(diff) > 0 {
t.Errorf("Failed to find expected values in AuditNonHMACResponseKeys. Difference is: %v", diff)
}
})

t.Run("flags_description", func(t *testing.T) {
Expand Down
19 changes: 19 additions & 0 deletions command/secrets_enable_test.go
View file
Expand Up @@ -5,6 +5,7 @@ import (
"strings"
"testing"

"github.com/go-test/deep"
"github.com/hashicorp/vault/helper/builtinplugins"
"github.com/hashicorp/vault/sdk/helper/consts"
"github.com/mitchellh/cli"
Expand Down Expand Up @@ -107,6 +108,11 @@ func TestSecretsEnableCommand_Run(t *testing.T) {
"-description", "The best kind of test",
"-default-lease-ttl", "30m",
"-max-lease-ttl", "1h",
"-audit-non-hmac-request-keys", "foo,bar",
"-audit-non-hmac-response-keys", "foo,bar",
"-passthrough-request-headers", "authorization,authentication",
"-passthrough-request-headers", "www-authentication",
"-allowed-response-headers", "authorization",
"-force-no-cache",
"pki",
})
Expand Down Expand Up @@ -144,6 +150,19 @@ func TestSecretsEnableCommand_Run(t *testing.T) {
if exp := true; mountInfo.Config.ForceNoCache != exp {
t.Errorf("expected %t to be %t", mountInfo.Config.ForceNoCache, exp)
}
if diff := deep.Equal([]string{"authorization,authentication", "www-authentication"}, mountInfo.Config.PassthroughRequestHeaders); len(diff) > 0 {
t.Errorf("Failed to find expected values in PassthroughRequestHeaders. Difference is: %v", diff)
}
if diff := deep.Equal([]string{"authorization"}, mountInfo.Config.AllowedResponseHeaders); len(diff) > 0 {
t.Errorf("Failed to find expected values in AllowedResponseHeaders. Difference is: %v", diff)
}
if diff := deep.Equal([]string{"foo,bar"}, mountInfo.Config.AuditNonHMACRequestKeys); len(diff) > 0 {
t.Errorf("Failed to find expected values in AuditNonHMACRequestKeys. Difference is: %v", diff)
}
if diff := deep.Equal([]string{"foo,bar"}, mountInfo.Config.AuditNonHMACResponseKeys); len(diff) > 0 {
t.Errorf("Failed to find expected values in AuditNonHMACResponseKeys. Difference is: %v", diff)
}

})

t.Run("communication_failure", func(t *testing.T) {
Expand Down
40 changes: 32 additions & 8 deletions command/secrets_tune.go
View file
Expand Up @@ -20,14 +20,16 @@ var (
type SecretsTuneCommand struct {
*BaseCommand

flagAuditNonHMACRequestKeys []string
flagAuditNonHMACResponseKeys []string
flagDefaultLeaseTTL time.Duration
flagDescription string
flagListingVisibility string
flagMaxLeaseTTL time.Duration
flagOptions map[string]string
flagVersion int
flagAuditNonHMACRequestKeys []string
flagAuditNonHMACResponseKeys []string
flagDefaultLeaseTTL time.Duration
flagDescription string
flagListingVisibility string
flagMaxLeaseTTL time.Duration
flagPassthroughRequestHeaders []string
flagAllowedResponseHeaders []string
flagOptions map[string]string
flagVersion int
}

func (c *SecretsTuneCommand) Synopsis() string {
Expand Down Expand Up @@ -106,6 +108,20 @@ func (c *SecretsTuneCommand) Flags() *FlagSets {
"TTL, or a previously configured value for the secrets engine.",
})

f.StringSliceVar(&StringSliceVar{
Name: flagNamePassthroughRequestHeaders,
Target: &c.flagPassthroughRequestHeaders,
Usage: "Comma-separated string or list of request header values that " +
"will be sent to the plugin",
})

f.StringSliceVar(&StringSliceVar{
Name: flagNameAllowedResponseHeaders,
Target: &c.flagAllowedResponseHeaders,
Usage: "Comma-separated string or list of response header values that " +
"plugins will be allowed to set",
})

f.StringMapVar(&StringMapVar{
Name: "options",
Target: &c.flagOptions,
Expand Down Expand Up @@ -189,6 +205,14 @@ func (c *SecretsTuneCommand) Run(args []string) int {
if fl.Name == flagNameListingVisibility {
mountConfigInput.ListingVisibility = c.flagListingVisibility
}

if fl.Name == flagNamePassthroughRequestHeaders {
mountConfigInput.PassthroughRequestHeaders = c.flagPassthroughRequestHeaders
}

if fl.Name == flagNameAllowedResponseHeaders {
mountConfigInput.AllowedResponseHeaders = c.flagAllowedResponseHeaders
}
})

if err := client.Sys().TuneMount(mountPath, mountConfigInput); err != nil {
Expand Down
16 changes: 16 additions & 0 deletions command/secrets_tune_test.go
View file
Expand Up @@ -4,6 +4,7 @@ import (
"strings"
"testing"

"github.com/go-test/deep"
"github.com/hashicorp/vault/api"
"github.com/mitchellh/cli"
)
Expand Down Expand Up @@ -166,6 +167,9 @@ func TestSecretsTuneCommand_Run(t *testing.T) {
"-max-lease-ttl", "1h",
"-audit-non-hmac-request-keys", "foo,bar",
"-audit-non-hmac-response-keys", "foo,bar",
"-passthrough-request-headers", "authorization",
"-passthrough-request-headers", "www-authentication",
"-allowed-response-headers", "authorization,www-authentication",
"-listing-visibility", "unauth",
"mount_tune_integration/",
})
Expand Down Expand Up @@ -200,6 +204,18 @@ func TestSecretsTuneCommand_Run(t *testing.T) {
if exp := 3600; mountInfo.Config.MaxLeaseTTL != exp {
t.Errorf("expected %d to be %d", mountInfo.Config.MaxLeaseTTL, exp)
}
if diff := deep.Equal([]string{"authorization", "www-authentication"}, mountInfo.Config.PassthroughRequestHeaders); len(diff) > 0 {
t.Errorf("Failed to find expected values for PassthroughRequestHeaders. Difference is: %v", diff)
}
if diff := deep.Equal([]string{"authorization,www-authentication"}, mountInfo.Config.AllowedResponseHeaders); len(diff) > 0 {
t.Errorf("Failed to find expected values in AllowedResponseHeaders. Difference is: %v", diff)
}
if diff := deep.Equal([]string{"foo,bar"}, mountInfo.Config.AuditNonHMACRequestKeys); len(diff) > 0 {
t.Errorf("Failed to find expected values in AuditNonHMACRequestKeys. Difference is: %v", diff)
}
if diff := deep.Equal([]string{"foo,bar"}, mountInfo.Config.AuditNonHMACResponseKeys); len(diff) > 0 {
t.Errorf("Failed to find expected values in AuditNonHMACResponseKeys. Difference is: %v", diff)
}
})

t.Run("flags_description", func(t *testing.T) {
Expand Down
40 changes: 39 additions & 1 deletion vault/logical_system.go
View file
Expand Up @@ -898,6 +898,13 @@ func (b *SystemBackend) handleMount(ctx context.Context, req *logical.Request, d
var apiConfig APIMountConfig

configMap := data.Get("config").(map[string]interface{})
// Augmenting configMap for some config options to treat them as comma separated entries
err := expandStringValsWithCommas(configMap)
if err != nil {
return logical.ErrorResponse(
"unable to parse given auth config information"),
logical.ErrInvalidRequest
}
if configMap != nil && len(configMap) != 0 {
err := mapstructure.Decode(configMap, &apiConfig)
if err != nil {
Expand Down Expand Up @@ -1564,7 +1571,6 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string,

if rawVal, ok := data.GetOk("allowed_response_headers"); ok {
headers := rawVal.([]string)

oldVal := mountEntry.Config.AllowedResponseHeaders
mountEntry.Config.AllowedResponseHeaders = headers

Expand Down Expand Up @@ -1869,6 +1875,31 @@ func (b *SystemBackend) handleAuthTable(ctx context.Context, req *logical.Reques
return resp, nil
}

func expandStringValsWithCommas(configMap map[string]interface{}) error {
configParamNameSlice := []string{
"audit_non_hmac_request_keys",
"audit_non_hmac_response_keys",
"passthrough_request_headers",
"allowed_response_headers",
}
for _, paramName := range configParamNameSlice {
if raw, ok := configMap[paramName]; ok {
switch t := raw.(type) {
case string:
// To be consistent with auth tune, and in cases where a single comma separated strings
// is provided in the curl command, we split the entries by the commas.
rawNew := raw.(string)
res, err := parseutil.ParseCommaStringSlice(rawNew)
if err != nil {
return fmt.Errorf("invalid input parameter %v of type %v", paramName, t)
}
configMap[paramName] = res
}
}
}
return nil
}

// handleEnableAuth is used to enable a new credential backend
func (b *SystemBackend) handleEnableAuth(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
repState := b.Core.ReplicationState()
Expand All @@ -1895,6 +1926,13 @@ func (b *SystemBackend) handleEnableAuth(ctx context.Context, req *logical.Reque
var apiConfig APIMountConfig

configMap := data.Get("config").(map[string]interface{})
// Augmenting configMap for some config options to treat them as comma separated entries
err := expandStringValsWithCommas(configMap)
if err != nil {
return logical.ErrorResponse(
"unable to parse given auth config information"),
logical.ErrInvalidRequest
}
if configMap != nil && len(configMap) != 0 {
err := mapstructure.Decode(configMap, &apiConfig)
if err != nil {
Expand Down

0 comments on commit dd294fc

Please sign in to comment.