Update genUsername to cap STS usernames at 32 chars (#12185)

* update genUsername to cap STS usernames at 64 chars

* add changelog

* refactor tests into t.Run block

* patch: remove warningExpected bool and include expected string

* patch: revert sts to cap at 32 chars and add assume_role case in genUsername

* update changelog

* update genUsername to return error if username generated exceeds length limits

* update changelog

* add conditional default username template to provide custom STS usernames

* update changelog

* include test for failing STS length case

* update comments for more clarity

builtin/logical/aws/path_config_root.go

@@ -8,7 +8,8 @@ import (
 	"github.com/hashicorp/vault/sdk/logical"
 )
 
-const defaultUserNameTemplate = `{{ printf "vault-%s-%s-%s" (printf "%s-%s" (.DisplayName) (.PolicyName) | truncate 42) (unix_time) (random 20) | truncate 64 }}`
+// A single default template that supports both the different credential types (IAM/STS) that are capped at differing length limits (64 chars/32 chars respectively)
+const defaultUserNameTemplate = `{{ if (eq .Type "STS") }}{{ printf "vault-%s-%s"  (unix_time) (random 20) | truncate 32 }}{{ else }}{{ printf "vault-%s-%s-%s" (printf "%s-%s" (.DisplayName) (.PolicyName) | truncate 42) (unix_time) (random 20) | truncate 64 }}{{ end }}`
 
 func pathConfigRoot(b *backend) *framework.Path {
 	return &framework.Path{

builtin/logical/aws/secret_access_keys.go

@@ -20,7 +20,6 @@ import (
 const (
 	secretAccessKeyType = "access_keys"
 	storageKey          = "config/root"
-	defaultSTSTemplate  = `{{ printf "vault-%d-%d" (unix_time) (random 20) | truncate 32 }}`
 )
 
 func secretAccessKeys(b *backend) *framework.Secret {
@@ -47,42 +46,45 @@ func secretAccessKeys(b *backend) *framework.Secret {
 	}
 }
 
-func genUsername(displayName, policyName, userType, usernameTemplate string) (ret string, warning string, err error) {
+func genUsername(displayName, policyName, userType, usernameTemplate string) (ret string, err error) {
 	switch userType {
-	case "iam_user":
+	case "iam_user", "assume_role":
-		// IAM users are capped at 64 chars; this leaves, after the beginning and
+		// IAM users are capped at 64 chars
-		// end added below, 42 chars to play with.
 		up, err := template.NewTemplate(template.Template(usernameTemplate))
 		if err != nil {
-			return "", "", fmt.Errorf("unable to initialize username template: %w", err)
+			return "", fmt.Errorf("unable to initialize username template: %w", err)
 		}
 
 		um := UsernameMetadata{
+			Type:        "IAM",
 			DisplayName: normalizeDisplayName(displayName),
 			PolicyName:  normalizeDisplayName(policyName),
 		}
 
 		ret, err = up.Generate(um)
 		if err != nil {
-			return "", "", fmt.Errorf("failed to generate username: %w", err)
+			return "", fmt.Errorf("failed to generate username: %w", err)
 		}
-		// To prevent template from exceeding IAM length limits
+		// To prevent a custom template from exceeding IAM length limits
 		if len(ret) > 64 {
-			ret = ret[0:64]
+			return "", fmt.Errorf("the username generated by the template exceeds the IAM username length limits of 64 chars")
-			warning = "the calling token display name/IAM policy name were truncated to 64 characters to fit within IAM username length limits"
 		}
 	case "sts":
-		// Capped at 32 chars, which leaves only a couple of characters to play
-		// with, so don't insert display name or policy name at all
 		up, err := template.NewTemplate(template.Template(usernameTemplate))
 		if err != nil {
-			return "", "", fmt.Errorf("unable to initialize username template: %w", err)
+			return "", fmt.Errorf("unable to initialize username template: %w", err)
 		}
 
-		um := UsernameMetadata{}
+		um := UsernameMetadata{
+			Type: "STS",
+		}
 		ret, err = up.Generate(um)
 		if err != nil {
-			return "", "", fmt.Errorf("failed to generate username: %w", err)
+			return "", fmt.Errorf("failed to generate username: %w", err)
+		}
+		// To prevent a custom template from exceeding STS length limits
+		if len(ret) > 32 {
+			return "", fmt.Errorf("the username generated by the template exceeds the STS username length limits of 32 chars")
 		}
 	}
 	return
@@ -112,7 +114,18 @@ func (b *backend) getFederationToken(ctx context.Context, s logical.Storage,
 		return logical.ErrorResponse(err.Error()), nil
 	}
 
-	username, usernameWarning, usernameError := genUsername(displayName, policyName, "sts", defaultSTSTemplate)
+	config, err := readConfig(ctx, s)
+	if err != nil {
+		return nil, fmt.Errorf("unable to read configuration: %w", err)
+	}
+
+	// Set as defaultUsernameTemplate if not provided
+	usernameTemplate := config.UsernameTemplate
+	if usernameTemplate == "" {
+		usernameTemplate = defaultUserNameTemplate
+	}
+
+	username, usernameError := genUsername(displayName, policyName, "sts", usernameTemplate)
 	// Send a 400 to Framework.OperationFunc Handler
 	if usernameError != nil {
 		return nil, usernameError
@@ -158,10 +171,6 @@ func (b *backend) getFederationToken(ctx context.Context, s logical.Storage,
 	// STS are purposefully short-lived and aren't renewable
 	resp.Secret.Renewable = false
 
-	if usernameWarning != "" {
-		resp.AddWarning(usernameWarning)
-	}
-
 	return resp, nil
 }
 
@@ -202,10 +211,9 @@ func (b *backend) assumeRole(ctx context.Context, s logical.Storage,
 		usernameTemplate = defaultUserNameTemplate
 	}
 
-	roleSessionNameWarning := ""
 	var roleSessionNameError error
 	if roleSessionName == "" {
-		roleSessionName, roleSessionNameWarning, roleSessionNameError = genUsername(displayName, roleName, "iam_user", usernameTemplate)
+		roleSessionName, roleSessionNameError = genUsername(displayName, roleName, "assume_role", usernameTemplate)
 		// Send a 400 to Framework.OperationFunc Handler
 		if roleSessionNameError != nil {
 			return nil, roleSessionNameError
@@ -247,10 +255,6 @@ func (b *backend) assumeRole(ctx context.Context, s logical.Storage,
 	// STS are purposefully short-lived and aren't renewable
 	resp.Secret.Renewable = false
 
-	if roleSessionNameWarning != "" {
-		resp.AddWarning(roleSessionNameWarning)
-	}
-
 	return resp, nil
 }
 
@@ -291,7 +295,7 @@ func (b *backend) secretAccessKeysCreate(
 		usernameTemplate = defaultUserNameTemplate
 	}
 
-	username, usernameWarning, usernameError := genUsername(displayName, policyName, "iam_user", usernameTemplate)
+	username, usernameError := genUsername(displayName, policyName, "iam_user", usernameTemplate)
 	// Send a 400 to Framework.OperationFunc Handler
 	if usernameError != nil {
 		return nil, usernameError
@@ -419,10 +423,6 @@ func (b *backend) secretAccessKeysCreate(
 	resp.Secret.TTL = lease.Lease
 	resp.Secret.MaxTTL = lease.LeaseMax
 
-	if usernameWarning != "" {
-		resp.AddWarning(usernameWarning)
-	}
-
 	return resp, nil
 }
 
@@ -506,6 +506,7 @@ func convertPolicyARNs(policyARNs []string) []*sts.PolicyDescriptorType {
 }
 
 type UsernameMetadata struct {
+	Type        string
 	DisplayName string
 	PolicyName  string
 }

builtin/logical/aws/secret_access_keys_test.go

@@ -47,53 +47,70 @@ func TestNormalizeDisplayName_NormNotRequired(t *testing.T) {
 }
 
 func TestGenUsername(t *testing.T) {
-
+	type testCase struct {
-	testUsername, warning, err := genUsername("name1", "policy1", "iam_user", `{{ printf "vault-%s-%s-%s-%s" (.DisplayName) (.PolicyName) (unix_time) (random 20) | truncate 64 }}`)
+		name             string
-	if err != nil {
+		policy           string
-		t.Fatalf(
+		userType         string
-			"expected no err; got %s",
+		UsernameTemplate string
-			err,
+		expectedError    string
-		)
+		expectedRegex    string
+		expectedLength   int
 	}
 
-	expectedUsernameRegex := `^vault-name1-policy1-[0-9]+-[a-zA-Z0-9]+`
+	tests := map[string]testCase{
-	require.Regexp(t, expectedUsernameRegex, testUsername)
+		"Truncated to 64. No warnings expected": {
-	// IAM usernames are capped at 64 characters
+			name:             "name1",
-	if len(testUsername) > 64 {
+			policy:           "policy1",
-		t.Fatalf(
+			userType:         "iam_user",
-			"expected IAM username to be of length 64, got %d",
+			UsernameTemplate: defaultUserNameTemplate,
-			len(testUsername),
+			expectedError:    "",
-		)
+			expectedRegex:    `^vault-name1-policy1-[0-9]+-[a-zA-Z0-9]+`,
+			expectedLength:   64,
+		},
+		"Truncated to 32. No warnings expected": {
+			name:             "name1",
+			policy:           "policy1",
+			userType:         "sts",
+			UsernameTemplate: defaultUserNameTemplate,
+			expectedError:    "",
+			expectedRegex:    `^vault-[0-9]+-[a-zA-Z0-9]+`,
+			expectedLength:   32,
+		},
+		"Too long. Error expected — IAM": {
+			name:             "this---is---a---very---long---name",
+			policy:           "long------policy------name",
+			userType:         "assume_role",
+			UsernameTemplate: `{{ if (eq .Type "IAM") }}{{ printf "%s-%s-%s-%s" (.DisplayName) (.PolicyName) (unix_time) (random 20) }}{{ end }}`,
+			expectedError:    "the username generated by the template exceeds the IAM username length limits of 64 chars",
+			expectedRegex:    "",
+			expectedLength:   64,
+		},
+		"Too long. Error expected — STS": {
+			name:             "this---is---a---very---long---name",
+			policy:           "long------policy------name",
+			userType:         "sts",
+			UsernameTemplate: `{{ if (eq .Type "STS") }}{{ printf "%s-%s-%s-%s" (.DisplayName) (.PolicyName) (unix_time) (random 20) }}{{ end }}`,
+			expectedError:    "the username generated by the template exceeds the STS username length limits of 32 chars",
+			expectedRegex:    "",
+			expectedLength:   32,
+		},
 	}
 
-	testUsername, warning, err = genUsername(
+	for testDescription, testCase := range tests {
-		"this---is---a---very---long---name",
+		t.Run(testDescription, func(t *testing.T) {
-		"long------policy------name",
+			testUsername, err := genUsername(testCase.name, testCase.policy, testCase.userType, testCase.UsernameTemplate)
-		"iam_user",
+			if err != nil && !strings.Contains(err.Error(), testCase.expectedError) {
-		`{{ printf "%s-%s-%s-%s" (.DisplayName) (.PolicyName) (unix_time) (random 20) }}`,
+				t.Fatalf("expected an error %s; instead received %s", testCase.expectedError, err)
-	)
+			}
 
-	if warning == "" || !strings.Contains(warning, "calling token display name/IAM policy name were truncated to 64 characters") {
+			if err == nil {
-		t.Fatalf("expected a truncate warning; received empty string")
+				require.Regexp(t, testCase.expectedRegex, testUsername)
-	}
-	if len(testUsername) != 64 {
-		t.Fatalf("expected a username cap at 64 chars; got length: %d", len(testUsername))
-	}
 
-	testUsername, warning, err = genUsername("name1", "policy1", "sts", defaultSTSTemplate)
+				if len(testUsername) > testCase.expectedLength {
-	if strings.Contains(testUsername, "name1") || strings.Contains(testUsername, "policy1") {
+					t.Fatalf("expected username to be of length %d, got %d", testCase.expectedLength, len(testUsername))
-		t.Fatalf(
+				}
-			"expected sts username to not contain display name or policy name; got %s",
+			}
-			testUsername,
+		})
-		)
-	}
-	// STS usernames are capped at 64 characters
-	if len(testUsername) > 32 {
-		t.Fatalf(
-			"expected sts username to be under 32 chars; got %s of length %d",
-			testUsername,
-			len(testUsername),
-		)
 	}
 }
 

changelog/12185.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+secrets/aws: Add conditional template that allows custom usernames for both STS and IAM cases
+```