From 512b254820e50d9e9f9a4d5290b84cfd20d691da Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Fri, 3 Nov 2017 17:12:03 -0400 Subject: [PATCH] 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 | 44 +++++++ builtin/logical/ssh/path_roles.go | 173 ++++++++++++++++++---------- logical/response.go | 20 ++++ 3 files changed, 174 insertions(+), 63 deletions(-) diff --git a/builtin/logical/ssh/backend_test.go b/builtin/logical/ssh/backend_test.go index 139d24aca..654f7bd28 100644 --- a/builtin/logical/ssh/backend_test.go +++ b/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, diff --git a/builtin/logical/ssh/path_roles.go b/builtin/logical/ssh/path_roles.go index de2f1d5ff..ec2d53449 100644 --- a/builtin/logical/ssh/path_roles.go +++ b/builtin/logical/ssh/path_roles.go @@ -488,13 +488,115 @@ func (b *backend) getRole(s logical.Storage, n string) (*sshRole, error) { return &result, nil } +// parseRole converts a sshRole object into its map[string]interface representation, +// with appropriate values for each KeyType. If the KeyType is invalid, it will retun +// an error. +func (b *backend) parseRole(role *sshRole) (map[string]interface{}, error) { + var result map[string]interface{} + + switch role.KeyType { + case KeyTypeOTP: + result = 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, + } + case KeyTypeCA: + ttl, err := parseutil.ParseDurationSecond(role.TTL) + if err != nil { + return nil, err + } + maxTTL, err := parseutil.ParseDurationSecond(role.MaxTTL) + if err != nil { + return nil, err + } + + result = map[string]interface{}{ + "allowed_users": role.AllowedUsers, + "allowed_domains": role.AllowedDomains, + "default_user": role.DefaultUser, + "ttl": int64(ttl.Seconds()), + "max_ttl": int64(maxTTL.Seconds()), + "allowed_critical_options": role.AllowedCriticalOptions, + "allowed_extensions": role.AllowedExtensions, + "allow_user_certificates": role.AllowUserCertificates, + "allow_host_certificates": role.AllowHostCertificates, + "allow_bare_domains": role.AllowBareDomains, + "allow_subdomains": role.AllowSubdomains, + "allow_user_key_ids": role.AllowUserKeyIDs, + "key_id_format": role.KeyIDFormat, + "key_type": role.KeyType, + "default_critical_options": role.DefaultCriticalOptions, + "default_extensions": role.DefaultExtensions, + } + case KeyTypeDynamic: + result = map[string]interface{}{ + "key": role.KeyName, + "admin_user": role.AdminUser, + "default_user": role.DefaultUser, + "cidr_list": role.CIDRList, + "exclude_cidr_list": role.ExcludeCIDRList, + "port": role.Port, + "key_type": role.KeyType, + "key_bits": role.KeyBits, + "allowed_users": role.AllowedUsers, + "key_option_specs": role.KeyOptionSpecs, + // Returning install script will make the output look messy. + // But this is one way for clients to see the script that is + // being used to install the key. If there is some problem, + // the script can be modified and configured by clients. + "install_script": role.InstallScript, + } + 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 } - return logical.ListResponse(entries), nil + 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) { @@ -506,69 +608,14 @@ func (b *backend) pathRoleRead(req *logical.Request, d *framework.FieldData) (*l 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 - } else if role.KeyType == KeyTypeCA { - ttl, err := parseutil.ParseDurationSecond(role.TTL) - if err != nil { - return nil, err - } - maxTTL, err := parseutil.ParseDurationSecond(role.MaxTTL) - if err != nil { - return nil, err - } - - return &logical.Response{ - Data: map[string]interface{}{ - "allowed_users": role.AllowedUsers, - "allowed_domains": role.AllowedDomains, - "default_user": role.DefaultUser, - "ttl": int64(ttl.Seconds()), - "max_ttl": int64(maxTTL.Seconds()), - "allowed_critical_options": role.AllowedCriticalOptions, - "allowed_extensions": role.AllowedExtensions, - "allow_user_certificates": role.AllowUserCertificates, - "allow_host_certificates": role.AllowHostCertificates, - "allow_bare_domains": role.AllowBareDomains, - "allow_subdomains": role.AllowSubdomains, - "allow_user_key_ids": role.AllowUserKeyIDs, - "key_id_format": role.KeyIDFormat, - "key_type": role.KeyType, - "default_critical_options": role.DefaultCriticalOptions, - "default_extensions": role.DefaultExtensions, - }, - }, nil - } else { - return &logical.Response{ - Data: map[string]interface{}{ - "key": role.KeyName, - "admin_user": role.AdminUser, - "default_user": role.DefaultUser, - "cidr_list": role.CIDRList, - "exclude_cidr_list": role.ExcludeCIDRList, - "port": role.Port, - "key_type": role.KeyType, - "key_bits": role.KeyBits, - "allowed_users": role.AllowedUsers, - "key_option_specs": role.KeyOptionSpecs, - // Returning install script will make the output look messy. - // But this is one way for clients to see the script that is - // 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 + 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) { diff --git a/logical/response.go b/logical/response.go index 6ee452b68..ab92fd541 100644 --- a/logical/response.go +++ b/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 +}