From e4dde85a9a1b3f159f5e33274e97e04547665701 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 27 Jun 2022 11:19:39 +0200 Subject: [PATCH] Connect: Fetch correct role set when listing db users (#13790) * Move fetchRoleSet from tsh/db to lib/services as FetchAllClusterRoles. * Connect: Fetch correct role set when listing db users * Add tests for FetchAllClusterRoles --- lib/services/role.go | 32 +++++++ lib/services/role_test.go | 104 +++++++++++++++++++++ lib/teleterm/clusters/cluster.go | 2 +- lib/teleterm/clusters/cluster_databases.go | 27 +++++- lib/teleterm/clusters/config.go | 2 +- tool/tsh/db.go | 32 +------ 6 files changed, 162 insertions(+), 37 deletions(-) diff --git a/lib/services/role.go b/lib/services/role.go index d64b9803b079c..291f3ea34cf03 100644 --- a/lib/services/role.go +++ b/lib/services/role.go @@ -735,6 +735,38 @@ func FetchRoles(roleNames []string, access RoleGetter, traits map[string][]strin return NewRoleSet(roles...), nil } +// CurrentUserRoleGetter limits the interface of auth.ClientI to methods needed by FetchClusterRoles. +type CurrentUserRoleGetter interface { + GetCurrentUser(context.Context) (types.User, error) + RoleGetter +} + +// FetchAllClusterRoles fetches all roles available to the user on the specified cluster. +func FetchAllClusterRoles(ctx context.Context, access CurrentUserRoleGetter, defaultRoles []string, defaultTraits wrappers.Traits) (RoleSet, error) { + roles := defaultRoles + traits := defaultTraits + + // Typically, auth.ClientI is passed as currentUserRoleGetter. Older versions of the auth client + // may not implement GetCurrentUser() so we fail gracefully and use default roles and traits instead. + user, err := access.GetCurrentUser(ctx) + if err == nil { + roles = user.GetRoles() + traits = user.GetTraits() + } else { + log.Debugf("Failed to fetch current user information: %v.", err) + } + + // get the role definition for all roles of user. + // this may only fail if the role which we are looking for does not exist, or we don't have access to it. + // example scenario when this may happen: + // 1. we have set of roles [foo bar] from profile. + // 2. the cluster is remote and maps the [foo, bar] roles to single role [guest] + // 3. the remote cluster doesn't implement GetCurrentUser(), so we have no way to learn of [guest]. + // 4. FetchRoles([foo bar], ..., ...) fails as [foo bar] does not exist on remote cluster. + roleSet, err := FetchRoles(roles, access, traits) + return roleSet, trace.Wrap(err) +} + // ExtractRolesFromCert extracts roles from certificate metadata extensions. func ExtractRolesFromCert(cert *ssh.Certificate) ([]string, error) { data, ok := cert.Extensions[teleport.CertExtensionTeleportRoles] diff --git a/lib/services/role_test.go b/lib/services/role_test.go index ba453edb239a1..ffa36e8f2e292 100644 --- a/lib/services/role_test.go +++ b/lib/services/role_test.go @@ -18,6 +18,7 @@ package services import ( "bytes" + "context" "encoding/json" "fmt" "strconv" @@ -4937,3 +4938,106 @@ func TestHostUsers_CanCreateHostUser(t *testing.T) { }) } } + +type mockCurrentUserRoleGetter struct { + currentUser types.User + nameToRole map[string]types.Role +} + +func (m mockCurrentUserRoleGetter) GetCurrentUser(ctx context.Context) (types.User, error) { + if m.currentUser != nil { + return m.currentUser, nil + } + return nil, trace.NotFound("currentUser not set") +} + +func (m mockCurrentUserRoleGetter) GetRole(ctx context.Context, name string) (types.Role, error) { + if role, ok := m.nameToRole[name]; ok { + return role, nil + } + return nil, trace.NotFound("role not found: %v", name) +} + +type mockCurrentUser struct { + types.User + roles []string + traits wrappers.Traits +} + +func (u mockCurrentUser) GetRoles() []string { + return u.roles +} + +func (u mockCurrentUser) GetTraits() map[string][]string { + return u.traits +} + +func TestFetchAllClusterRoles_PrefersRolesAndTraitsFromCurrentUser(t *testing.T) { + defaultRoles := []string{"access", "editor"} + defaultTraits := map[string][]string{ + "logins": {"defaultTraitLogin"}, + } + + user := mockCurrentUser{ + roles: []string{"dev", "admin"}, + traits: map[string][]string{ + "logins": {"currentUserTraitLogin"}, + }, + } + + devRole := newRole(func(r *types.RoleV5) { + r.Metadata.Name = "dev" + r.Spec.Allow.Logins = []string{"{{internal.logins}}"} + }) + adminRole := newRole(func(r *types.RoleV5) { + r.Metadata.Name = "admin" + }) + + currentUserRoleGetter := mockCurrentUserRoleGetter{ + nameToRole: map[string]types.Role{ + "dev": &devRole, + "admin": &adminRole, + }, + currentUser: user, + } + + roleSet, err := FetchAllClusterRoles(context.Background(), currentUserRoleGetter, + defaultRoles, defaultTraits) + + require.NoError(t, err) + + require.Contains(t, roleSet, &devRole, "devRole not found in roleSet") + require.Contains(t, roleSet, &adminRole, "adminRole not found in roleSet") + require.Equal(t, []string{"currentUserTraitLogin"}, roleSet[0].GetLogins(types.Allow)) +} + +func TestFetchAllClusterRoles_UsesDefaultRolesAndTraitsIfCurrentUserIsUnavailable(t *testing.T) { + defaultRoles := []string{"access", "editor"} + defaultTraits := map[string][]string{ + "logins": {"defaultTraitLogin"}, + } + + accessRole := newRole(func(r *types.RoleV5) { + r.Metadata.Name = "access" + r.Spec.Allow.Logins = []string{"{{internal.logins}}"} + }) + editorRole := newRole(func(r *types.RoleV5) { + r.Metadata.Name = "editor" + }) + + currentUserRoleGetter := mockCurrentUserRoleGetter{ + nameToRole: map[string]types.Role{ + "access": &accessRole, + "editor": &editorRole, + }, + } + + roleSet, err := FetchAllClusterRoles(context.Background(), currentUserRoleGetter, + defaultRoles, defaultTraits) + + require.NoError(t, err) + + require.Contains(t, roleSet, &accessRole, "accessRole not found in roleSet") + require.Contains(t, roleSet, &editorRole, "editorRole not found in roleSet") + require.Equal(t, []string{"defaultTraitLogin"}, roleSet[0].GetLogins(types.Allow)) +} diff --git a/lib/teleterm/clusters/cluster.go b/lib/teleterm/clusters/cluster.go index 7514bba0056e0..43cb76373e4df 100644 --- a/lib/teleterm/clusters/cluster.go +++ b/lib/teleterm/clusters/cluster.go @@ -39,7 +39,7 @@ type Cluster struct { Name string // Log is a component logger - Log logrus.FieldLogger + Log *logrus.Entry // dir is the directory where cluster certificates are stored dir string // Status is the cluster status diff --git a/lib/teleterm/clusters/cluster_databases.go b/lib/teleterm/clusters/cluster_databases.go index 95678717faab7..351bff2001ca7 100644 --- a/lib/teleterm/clusters/cluster_databases.go +++ b/lib/teleterm/clusters/cluster_databases.go @@ -22,6 +22,7 @@ import ( "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/client" dbprofile "github.com/gravitational/teleport/lib/client/db" libdefaults "github.com/gravitational/teleport/lib/defaults" @@ -152,17 +153,33 @@ func (c *Cluster) ReissueDBCerts(ctx context.Context, user, dbName string, db ty // GetAllowedDatabaseUsers returns allowed users for the given database based on the role set. func (c *Cluster) GetAllowedDatabaseUsers(ctx context.Context, dbURI string) ([]string, error) { - var roleSet services.RoleSet - var err error + var authClient auth.ClientI - err = addMetadataToRetryableError(ctx, func() error { - roleSet, err = services.FetchRoles(c.status.Roles, c.clusterClient, c.status.Traits) - return err + err := addMetadataToRetryableError(ctx, func() error { + proxyClient, err := c.clusterClient.ConnectToProxy(ctx) + if err != nil { + return trace.Wrap(err) + } + defer proxyClient.Close() + + authClient, err = proxyClient.ConnectToCluster(ctx, c.clusterClient.SiteName) + if err != nil { + return trace.Wrap(err) + } + + return nil }) if err != nil { return nil, trace.Wrap(err) } + defer authClient.Close() + + roleSet, err := services.FetchAllClusterRoles(ctx, authClient, c.status.Roles, c.status.Traits) + if err != nil { + return nil, trace.Wrap(err) + } + db, err := c.GetDatabase(ctx, dbURI) if err != nil { return nil, trace.Wrap(err) diff --git a/lib/teleterm/clusters/config.go b/lib/teleterm/clusters/config.go index 743c28e7c068c..c3a627e0f182f 100644 --- a/lib/teleterm/clusters/config.go +++ b/lib/teleterm/clusters/config.go @@ -32,7 +32,7 @@ type Config struct { // InsecureSkipVerify is an option to skip TLS cert check InsecureSkipVerify bool // Log is a component logger - Log logrus.FieldLogger + Log *logrus.Entry } // CheckAndSetDefaults checks the configuration for its validity and sets default values if needed diff --git a/tool/tsh/db.go b/tool/tsh/db.go index ff239c4642b61..137d3f55a1ba0 100644 --- a/tool/tsh/db.go +++ b/tool/tsh/db.go @@ -17,7 +17,6 @@ limitations under the License. package main import ( - "context" "encoding/base64" "fmt" "net" @@ -28,7 +27,6 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/types" - "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/client" dbprofile "github.com/gravitational/teleport/lib/client/db" "github.com/gravitational/teleport/lib/client/db/dbcmd" @@ -80,7 +78,7 @@ func onListDatabases(cf *CLIConf) error { return trace.Wrap(err) } - roleSet, err := fetchRoleSet(cf.Context, cluster, profile) + roleSet, err := services.FetchAllClusterRoles(cf.Context, cluster, profile.Roles, profile.Traits) if err != nil { log.Debugf("Failed to fetch user roles: %v.", err) } @@ -143,7 +141,7 @@ func listDatabasesAllClusters(cf *CLIConf) error { continue } - roleSet, err := fetchRoleSet(cf.Context, cluster, profile) + roleSet, err := services.FetchAllClusterRoles(cf.Context, cluster, profile.Roles, profile.Traits) if err != nil { log.Debugf("Failed to fetch user roles: %v.", err) } @@ -815,32 +813,6 @@ func isMFADatabaseAccessRequired(cf *CLIConf, tc *client.TeleportClient, databas return mfaResp.GetRequired(), nil } -// fetchRoleSet fetches a user's roles for a specified cluster. -func fetchRoleSet(ctx context.Context, cluster auth.ClientI, profile *client.ProfileStatus) (services.RoleSet, error) { - // get roles and traits. default to the set from profile, try to get up-to-date version from server point of view. - roles := profile.Roles - traits := profile.Traits - - // GetCurrentUser() may not be implemented, fail gracefully. - user, err := cluster.GetCurrentUser(ctx) - if err == nil { - roles = user.GetRoles() - traits = user.GetTraits() - } else { - log.Debugf("Failed to fetch current user information: %v.", err) - } - - // get the role definition for all roles of user. - // this may only fail if the role which we are looking for does not exist, or we don't have access to it. - // example scenario when this may happen: - // 1. we have set of roles [foo bar] from profile. - // 2. the cluster is remote and maps the [foo, bar] roles to single role [guest] - // 3. the remote cluster doesn't implement GetCurrentUser(), so we have no way to learn of [guest]. - // 4. services.FetchRoles([foo bar], ..., ...) fails as [foo bar] does not exist on remote cluster. - roleSet, err := services.FetchRoles(roles, cluster, traits) - return roleSet, trace.Wrap(err) -} - // pickActiveDatabase returns the database the current profile is logged into. // // If logged into multiple databases, returns an error unless one specified