secrets/ssh: Return the allow_empty_principals field in read api (#28901

)

* secrets/ssh: Return the allow_empty_principals field in read api

 - Return the new field in the read response api and add a test case
   that will catch these errors in the future of adding a field to
   the ssh role and not returning it in the read api response

* Add cl

builtin/logical/ssh/backend_test.go

@@ -15,6 +15,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/go-secure-stdlib/strutil"
 	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/builtin/credential/userpass"
 	"github.com/hashicorp/vault/helper/testhelpers/corehelpers"
@@ -24,6 +25,7 @@ import (
 	"github.com/hashicorp/vault/sdk/logical"
 	"github.com/hashicorp/vault/vault"
 	"github.com/mitchellh/mapstructure"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/crypto/ssh"
 )
@@ -212,6 +214,101 @@ func testSSH(user, host string, auth ssh.AuthMethod, command string) error {
 	return nil
 }
 
+// TestBackend_ReadRolesReturnsAllFields validates we did not forget to return a newly added
+// field from the ssh role in the read API.
+func TestBackend_ReadRolesReturnsAllFields(t *testing.T) {
+	t.Parallel()
+
+	config := logical.TestBackendConfig()
+	config.StorageView = &logical.InmemStorage{}
+
+	b, err := Backend(config)
+	require.NoError(t, err, "failed creating backend")
+	err = b.Setup(context.Background(), config)
+	require.NoError(t, err, "failed setting up backend")
+
+	tests := []struct {
+		name          string
+		data          map[string]interface{}
+		ignoredFields []string
+	}{
+		{
+			name: "otp",
+			data: map[string]interface{}{
+				"key_type":     "otp",
+				"default_user": "ubuntu",
+			},
+			ignoredFields: []string{
+				"allow_host_certificates", "allow_subdomains",
+				"default_user_template", "allowed_extensions", "allowed_user_key_lengths",
+				"allow_empty_principals", "ttl", "allowed_domains_template",
+				"default_extensions_template", "default_critical_options",
+				"allow_bare_domains", "allowed_domains", "allowed_critical_options",
+				"allow_user_certificates", "allow_user_key_ids", "algorithm_signer",
+				"not_before_duration", "max_ttl", "default_extensions", "allowed_users_template", "key_id_format",
+			},
+		},
+		{
+			name: "ca",
+			data: map[string]interface{}{
+				"key_type":                "ca",
+				"algorithm_signer":        "rsa-sha2-256",
+				"allow_user_certificates": true,
+			},
+			ignoredFields: []string{"port", "cidr_list", "exclude_cidr_list"},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			var fieldsToIgnore []string
+			fieldsToIgnore = append(fieldsToIgnore, tc.ignoredFields...)
+			// These fields apply to all use cases, role_version is never returned and
+			// allowed_user_key_types_lengths is an internal value for the allowed_user_key_lengths field
+			fieldsToIgnore = append(fieldsToIgnore, "role_version")
+			fieldsToIgnore = append(fieldsToIgnore, "allowed_user_key_types_lengths")
+
+			roleName := fmt.Sprintf("roles/role-%s", tc.name)
+			req := &logical.Request{
+				Operation: logical.UpdateOperation,
+				Path:      roleName,
+				Storage:   config.StorageView,
+				Data:      tc.data,
+			}
+			resp, err := b.HandleRequest(context.Background(), req)
+			if err != nil || (resp != nil && resp.IsError()) || resp != nil {
+				t.Fatalf("failed to create role %s: resp:%#v err:%s", roleName, resp, err)
+			}
+
+			req = &logical.Request{
+				Operation: logical.ReadOperation,
+				Path:      roleName,
+				Storage:   config.StorageView,
+			}
+			resp, err = b.HandleRequest(context.Background(), req)
+			if err != nil || (resp != nil && resp.IsError()) || resp == nil {
+				t.Fatalf("failed to read role %s: resp:%#v err:%s", roleName, resp, err)
+			}
+
+			roleMap := map[string]interface{}{}
+			err = mapstructure.Decode(sshRole{}, &roleMap)
+			require.NoError(t, err, "failed getting all fields in ssh role")
+
+			// Identify missing fields from OTP response
+			var missingFields []string
+			for fieldName := range roleMap {
+				if _, ok := resp.Data[fieldName]; !ok {
+					if strutil.StrListContains(fieldsToIgnore, fieldName) {
+						continue
+					}
+					missingFields = append(missingFields, fieldName)
+				}
+			}
+			assert.Empty(t, missingFields, "response was missing fields: %s", missingFields)
+		})
+	}
+}
+
 func TestBackend_AllowedUsers(t *testing.T) {
 	config := logical.TestBackendConfig()
 	config.StorageView = &logical.InmemStorage{}

builtin/logical/ssh/path_roles.go

@@ -698,6 +698,7 @@ func (b *backend) parseRole(role *sshRole) (map[string]interface{}, error) {
 			"allowed_user_key_lengths":    role.AllowedUserKeyTypesLengths,
 			"algorithm_signer":            role.AlgorithmSigner,
 			"not_before_duration":         int64(role.NotBeforeDuration.Seconds()),
+			"allow_empty_principals":      role.AllowEmptyPrincipals,
 		}
 	case KeyTypeDynamic:
 		return nil, fmt.Errorf("dynamic key type roles are no longer supported")

changelog/28901.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+secrets/ssh: Return the flag `allow_empty_principals` in the read role api when key_type is "ca"
+```