Return role info for each role on pathRoleList (#3532)

* Return role info for each role on pathRoleList

* Change roles -> key_info, only return key_type

* Do not initialize result map in parseRole, refactor ListResponseWithInfo

* Add role list test

builtin/logical/ssh/backend_test.go

@@ -271,6 +271,31 @@ func TestSSHBackend_Lookup(t *testing.T) {
 	})
 }
 
+func TestSSHBackend_RoleList(t *testing.T) {
+	testOTPRoleData := map[string]interface{}{
+		"key_type":     testOTPKeyType,
+		"default_user": testUserName,
+		"cidr_list":    testCIDRList,
+	}
+	resp1 := map[string]interface{}{}
+	resp2 := map[string]interface{}{
+		"keys": []string{testOTPRoleName},
+		"key_info": map[string]interface{}{
+			testOTPRoleName: map[string]interface{}{
+				"key_type": testOTPKeyType,
+			},
+		},
+	}
+	logicaltest.Test(t, logicaltest.TestCase{
+		Factory: testingFactory,
+		Steps: []logicaltest.TestStep{
+			testRoleList(t, resp1),
+			testRoleWrite(t, testOTPRoleName, testOTPRoleData),
+			testRoleList(t, resp2),
+		},
+	})
+}
+
 func TestSSHBackend_DynamicKeyCreate(t *testing.T) {
 	testDynamicRoleData := map[string]interface{}{
 		"key_type":     testDynamicKeyType,
@@ -962,6 +987,25 @@ func testRoleWrite(t *testing.T, name string, data map[string]interface{}) logic
 	}
 }
 
+func testRoleList(t *testing.T, expected map[string]interface{}) logicaltest.TestStep {
+	return logicaltest.TestStep{
+		Operation: logical.ListOperation,
+		Path:      "roles",
+		Check: func(resp *logical.Response) error {
+			if resp == nil {
+				return fmt.Errorf("nil response")
+			}
+			if resp.Data == nil {
+				return fmt.Errorf("nil data")
+			}
+			if !reflect.DeepEqual(resp.Data, expected) {
+				return fmt.Errorf("Invalid response:\nactual:%#v\nexpected is %#v", resp.Data, expected)
+			}
+			return nil
+		},
+	}
+}
+
 func testRoleRead(t *testing.T, roleName string, expected map[string]interface{}) logicaltest.TestStep {
 	return logicaltest.TestStep{
 		Operation: logical.ReadOperation,

builtin/logical/ssh/path_roles.go

@@ -488,37 +488,23 @@ func (b *backend) getRole(s logical.Storage, n string) (*sshRole, error) {
 	return &result, nil
 }
 
-func (b *backend) pathRoleList(req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
+// parseRole converts a sshRole object into its map[string]interface representation,
-	entries, err := req.Storage.List("roles/")
+// with appropriate values for each KeyType. If the KeyType is invalid, it will retun
-	if err != nil {
+// an error.
-		return nil, err
+func (b *backend) parseRole(role *sshRole) (map[string]interface{}, error) {
-	}
+	var result map[string]interface{}
 
-	return logical.ListResponse(entries), nil
+	switch role.KeyType {
-}
+	case KeyTypeOTP:
-
+		result = map[string]interface{}{
-func (b *backend) pathRoleRead(req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
-	role, err := b.getRole(req.Storage, d.Get("role").(string))
-	if err != nil {
-		return nil, err
-	}
-	if role == nil {
-		return nil, nil
-	}
-
-	// Return information should be based on the key type of the role
-	if role.KeyType == KeyTypeOTP {
-		return &logical.Response{
-			Data: map[string]interface{}{
 			"default_user":      role.DefaultUser,
 			"cidr_list":         role.CIDRList,
 			"exclude_cidr_list": role.ExcludeCIDRList,
 			"key_type":          role.KeyType,
 			"port":              role.Port,
 			"allowed_users":     role.AllowedUsers,
-			},
+		}
-		}, nil
+	case KeyTypeCA:
-	} else if role.KeyType == KeyTypeCA {
 		ttl, err := parseutil.ParseDurationSecond(role.TTL)
 		if err != nil {
 			return nil, err
@@ -528,8 +514,7 @@ func (b *backend) pathRoleRead(req *logical.Request, d *framework.FieldData) (*l
 			return nil, err
 		}
 
-		return &logical.Response{
+		result = map[string]interface{}{
-			Data: map[string]interface{}{
 			"allowed_users":            role.AllowedUsers,
 			"allowed_domains":          role.AllowedDomains,
 			"default_user":             role.DefaultUser,
@@ -546,11 +531,9 @@ func (b *backend) pathRoleRead(req *logical.Request, d *framework.FieldData) (*l
 			"key_type":                 role.KeyType,
 			"default_critical_options": role.DefaultCriticalOptions,
 			"default_extensions":       role.DefaultExtensions,
-			},
+		}
-		}, nil
+	case KeyTypeDynamic:
-	} else {
+		result = map[string]interface{}{
-		return &logical.Response{
-			Data: map[string]interface{}{
 			"key":               role.KeyName,
 			"admin_user":        role.AdminUser,
 			"default_user":      role.DefaultUser,
@@ -566,9 +549,73 @@ func (b *backend) pathRoleRead(req *logical.Request, d *framework.FieldData) (*l
 			// being used to install the key. If there is some problem,
 			// the script can be modified and configured by clients.
 			"install_script": role.InstallScript,
-			},
-		}, nil
 		}
+	default:
+		return nil, fmt.Errorf("invalid key type: %v", role.KeyType)
+	}
+
+	return result, nil
+}
+
+func (b *backend) pathRoleList(req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
+	entries, err := req.Storage.List("roles/")
+	if err != nil {
+		return nil, err
+	}
+
+	keyInfo := map[string]interface{}{}
+	for _, entry := range entries {
+		role, err := b.getRole(req.Storage, entry)
+		if err != nil {
+			// On error, log warning and continue
+			if b.Logger().IsWarn() {
+				b.Logger().Warn("ssh: error getting role info", "role", entry, "error", err)
+			}
+			continue
+		}
+		if role == nil {
+			// On empty role, log warning and continue
+			if b.Logger().IsWarn() {
+				b.Logger().Warn("ssh: no role info found", "role", entry)
+			}
+			continue
+		}
+
+		roleInfo, err := b.parseRole(role)
+		if err != nil {
+			if b.Logger().IsWarn() {
+				b.Logger().Warn("ssh: error parsing role info", "role", entry, "error", err)
+			}
+			continue
+		}
+
+		if keyType, ok := roleInfo["key_type"]; ok {
+			keyInfo[entry] = map[string]interface{}{
+				"key_type": keyType,
+			}
+		}
+	}
+
+	return logical.ListResponseWithInfo(entries, keyInfo), nil
+}
+
+func (b *backend) pathRoleRead(req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
+	role, err := b.getRole(req.Storage, d.Get("role").(string))
+	if err != nil {
+		return nil, err
+	}
+	if role == nil {
+		return nil, nil
+	}
+
+	roleInfo, err := b.parseRole(role)
+	if err != nil {
+		return nil, err
+	}
+
+	return &logical.Response{
+		Data: roleInfo,
+	}, nil
 }
 
 func (b *backend) pathRoleDelete(req *logical.Request, d *framework.FieldData) (*logical.Response, error) {

logical/response.go

@@ -110,3 +110,23 @@ func ListResponse(keys []string) *Response {
 	}
 	return resp
 }
+
+// ListResponseWithInfo is used to format a response to a list operation and
+// return the keys as well as a map with corresponding key info.
+func ListResponseWithInfo(keys []string, keyInfo map[string]interface{}) *Response {
+	resp := ListResponse(keys)
+
+	keyInfoData := make(map[string]interface{})
+	for _, key := range keys {
+		val, ok := keyInfo[key]
+		if ok {
+			keyInfoData[key] = val
+		}
+	}
+
+	if len(keyInfoData) > 0 {
+		resp.Data["key_info"] = keyInfoData
+	}
+
+	return resp
+}