From 9a0951e3428e01a7eb1574e58bd2507c264253ce Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Thu, 9 Feb 2023 12:55:24 +0100 Subject: [PATCH 1/2] Connect: return logged in user in `ListRootClusters` (#20809) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add integration/teleterm_test with tests for get* and list* clusters Co-authored-by: Rafał Cieślak * Move `testAddingRootCluster to integration/teleterm_test * Add `ClusterWithDetails` type instead of enriching existing `Cluster` with optional properties * Remove unnecessary condition * Add license * Bunch of renames * Do more in mustLogin * Remove watcher --------- Co-authored-by: Rafał Cieślak (cherry picked from commit 7ed92c1c18bae9bd5ee413156df0f0426653aca4) --- integration/teleterm_test.go | 229 ++++++++++++++++++ lib/teleterm/api/proto/v1/cluster.proto | 10 +- .../api/protogen/golang/v1/cluster.pb.go | 13 +- .../apiserver/handler/handler_clusters.go | 46 ++-- lib/teleterm/clusters/cluster.go | 36 +-- lib/teleterm/daemon/daemon.go | 10 +- 6 files changed, 290 insertions(+), 54 deletions(-) create mode 100644 integration/teleterm_test.go diff --git a/integration/teleterm_test.go b/integration/teleterm_test.go new file mode 100644 index 0000000000000..64c28682cb7ed --- /dev/null +++ b/integration/teleterm_test.go @@ -0,0 +1,229 @@ +// Copyright 2023 Gravitational, Inc +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package integration + +import ( + "context" + "fmt" + "net" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/gravitational/teleport/api/types" + api "github.com/gravitational/teleport/gen/proto/go/teleport/lib/teleterm/v1" + dbhelpers "github.com/gravitational/teleport/integration/db" + "github.com/gravitational/teleport/integration/helpers" + "github.com/gravitational/teleport/lib/client" + "github.com/gravitational/teleport/lib/service" + "github.com/gravitational/teleport/lib/teleterm/api/uri" + "github.com/gravitational/teleport/lib/teleterm/apiserver/handler" + "github.com/gravitational/teleport/lib/teleterm/clusters" + "github.com/gravitational/teleport/lib/teleterm/daemon" +) + +func TestTeleterm(t *testing.T) { + pack := dbhelpers.SetupDatabaseTest(t, + dbhelpers.WithListenerSetupDatabaseTest(helpers.SingleProxyPortSetup), + dbhelpers.WithLeafConfig(func(config *service.Config) { + config.Auth.NetworkingConfig.SetProxyListenerMode(types.ProxyListenerMode_Multiplex) + }), + dbhelpers.WithRootConfig(func(config *service.Config) { + config.Auth.NetworkingConfig.SetProxyListenerMode(types.ProxyListenerMode_Multiplex) + }), + ) + pack.WaitForLeaf(t) + + creds, err := helpers.GenerateUserCreds(helpers.UserCredsRequest{ + Process: pack.Root.Cluster.Process, + Username: pack.Root.User.GetName(), + }) + require.NoError(t, err) + + t.Run("adding root cluster", func(t *testing.T) { + t.Parallel() + + testAddingRootCluster(t, pack, creds) + }) + + t.Run("ListRootClusters returns logged in user", func(t *testing.T) { + t.Parallel() + + testListRootClustersReturnsLoggedInUser(t, pack, creds) + }) + t.Run("GetCluster returns properties from auth server", func(t *testing.T) { + t.Parallel() + + testGetClusterReturnsPropertiesFromAuthServer(t, pack) + }) +} + +func testAddingRootCluster(t *testing.T, pack *dbhelpers.DatabasePack, creds *helpers.UserCreds) { + storage, err := clusters.NewStorage(clusters.Config{ + Dir: t.TempDir(), + InsecureSkipVerify: true, + }) + require.NoError(t, err) + + daemonService, err := daemon.New(daemon.Config{ + Storage: storage, + }) + require.NoError(t, err) + t.Cleanup(func() { + daemonService.Stop() + }) + + addedCluster, err := daemonService.AddCluster(context.Background(), pack.Root.Cluster.Web) + require.NoError(t, err) + + clusters, err := daemonService.ListRootClusters(context.Background()) + require.NoError(t, err) + + clusterURIs := make([]uri.ResourceURI, 0, len(clusters)) + for _, cluster := range clusters { + clusterURIs = append(clusterURIs, cluster.URI) + } + require.ElementsMatch(t, clusterURIs, []uri.ResourceURI{addedCluster.URI}) +} + +func testListRootClustersReturnsLoggedInUser(t *testing.T, pack *dbhelpers.DatabasePack, creds *helpers.UserCreds) { + tc := mustLogin(t, pack.Root.User.GetName(), pack, creds) + + storage, err := clusters.NewStorage(clusters.Config{ + Dir: tc.KeysDir, + InsecureSkipVerify: tc.InsecureSkipVerify, + }) + require.NoError(t, err) + + daemonService, err := daemon.New(daemon.Config{ + Storage: storage, + }) + require.NoError(t, err) + t.Cleanup(func() { + daemonService.Stop() + }) + + handler, err := handler.New( + handler.Config{ + DaemonService: daemonService, + }, + ) + require.NoError(t, err) + + response, err := handler.ListRootClusters(context.Background(), &api.ListClustersRequest{}) + require.NoError(t, err) + + require.Equal(t, 1, len(response.Clusters)) + require.Equal(t, pack.Root.User.GetName(), response.Clusters[0].LoggedInUser.Name) +} + +func testGetClusterReturnsPropertiesFromAuthServer(t *testing.T, pack *dbhelpers.DatabasePack) { + authServer := pack.Root.Cluster.Process.GetAuthServer() + + // Use random names to not collide with other tests. + uuid := uuid.NewString() + suggestedReviewer := "suggested-reviewer" + requestableRoleName := fmt.Sprintf("%s-%s", "requested-role", uuid) + userName := fmt.Sprintf("%s-%s", "user", uuid) + roleName := fmt.Sprintf("%s-%s", "get-cluster-role", uuid) + + requestableRole, err := types.NewRole(requestableRoleName, types.RoleSpecV6{}) + require.NoError(t, err) + + // Create user role with ability to request role + userRole, err := types.NewRole(roleName, types.RoleSpecV6{ + Options: types.RoleOptions{}, + Allow: types.RoleConditions{ + Logins: []string{ + userName, + }, + NodeLabels: types.Labels{types.Wildcard: []string{types.Wildcard}}, + Request: &types.AccessRequestConditions{ + Roles: []string{requestableRoleName}, + SuggestedReviewers: []string{suggestedReviewer}, + }, + }, + }) + require.NoError(t, err) + + // add role that user can request + err = authServer.UpsertRole(context.Background(), requestableRole) + require.NoError(t, err) + + // add role that allows to request "requestableRole" + err = authServer.UpsertRole(context.Background(), userRole) + require.NoError(t, err) + + user, err := types.NewUser(userName) + user.AddRole(userRole.GetName()) + require.NoError(t, err) + + err = authServer.UpsertUser(user) + require.NoError(t, err) + + creds, err := helpers.GenerateUserCreds(helpers.UserCredsRequest{ + Process: pack.Root.Cluster.Process, + Username: userName, + }) + require.NoError(t, err) + + tc := mustLogin(t, userName, pack, creds) + + storage, err := clusters.NewStorage(clusters.Config{ + Dir: tc.KeysDir, + InsecureSkipVerify: tc.InsecureSkipVerify, + }) + require.NoError(t, err) + + daemonService, err := daemon.New(daemon.Config{ + Storage: storage, + }) + require.NoError(t, err) + t.Cleanup(func() { + daemonService.Stop() + }) + + handler, err := handler.New( + handler.Config{ + DaemonService: daemonService, + }, + ) + require.NoError(t, err) + + rootClusterName, _, err := net.SplitHostPort(pack.Root.Cluster.Web) + require.NoError(t, err) + + response, err := handler.GetCluster(context.Background(), &api.GetClusterRequest{ + ClusterUri: uri.NewClusterURI(rootClusterName).String(), + }) + require.NoError(t, err) + + require.Equal(t, userName, response.LoggedInUser.Name) + require.ElementsMatch(t, []string{requestableRoleName}, response.LoggedInUser.RequestableRoles) + require.ElementsMatch(t, []string{suggestedReviewer}, response.LoggedInUser.SuggestedReviewers) +} + +func mustLogin(t *testing.T, userName string, pack *dbhelpers.DatabasePack, creds *helpers.UserCreds) *client.TeleportClient { + tc, err := pack.Root.Cluster.NewClientWithCreds(helpers.ClientConfig{ + Login: userName, + Cluster: pack.Root.Cluster.Secrets.SiteName, + }, *creds) + require.NoError(t, err) + // The profile on disk created by NewClientWithCreds doesn't have WebProxyAddr set. + tc.WebProxyAddr = pack.Root.Cluster.Web + tc.SaveProfile(false /* makeCurrent */) + return tc +} diff --git a/lib/teleterm/api/proto/v1/cluster.proto b/lib/teleterm/api/proto/v1/cluster.proto index 66a9e9770cccc..43954ce662944 100644 --- a/lib/teleterm/api/proto/v1/cluster.proto +++ b/lib/teleterm/api/proto/v1/cluster.proto @@ -33,9 +33,8 @@ message Cluster { bool leaf = 5; // User is the cluster access control list of the logged-in user LoggedInUser logged_in_user = 7; - // features describes the auth servers features - // Only present in situations where detailed - // information is queried from the auth server. + // features describes the auth servers features. + // Only present when detailed information is queried from the auth server. Features features = 8; } @@ -51,8 +50,11 @@ message LoggedInUser { ACL acl = 4; // active_requests is an array of request-id strings of active requests repeated string active_requests = 5; - + // suggested_reviewers for the given user. + // Only present when detailed information is queried from the auth server. repeated string suggested_reviewers = 6; + // requestable_roles for the given user. + // Only present when detailed information is queried from the auth server. repeated string requestable_roles = 7; } diff --git a/lib/teleterm/api/protogen/golang/v1/cluster.pb.go b/lib/teleterm/api/protogen/golang/v1/cluster.pb.go index 73ebef2c477c9..080337f35eebf 100644 --- a/lib/teleterm/api/protogen/golang/v1/cluster.pb.go +++ b/lib/teleterm/api/protogen/golang/v1/cluster.pb.go @@ -58,9 +58,8 @@ type Cluster struct { Leaf bool `protobuf:"varint,5,opt,name=leaf,proto3" json:"leaf,omitempty"` // User is the cluster access control list of the logged-in user LoggedInUser *LoggedInUser `protobuf:"bytes,7,opt,name=logged_in_user,json=loggedInUser,proto3" json:"logged_in_user,omitempty"` - // features describes the auth servers features - // Only present in situations where detailed - // information is queried from the auth server. + // features describes the auth servers features. + // Only present when detailed information is queried from the auth server. Features *Features `protobuf:"bytes,8,opt,name=features,proto3" json:"features,omitempty"` } @@ -160,9 +159,13 @@ type LoggedInUser struct { // acl is the user acl Acl *ACL `protobuf:"bytes,4,opt,name=acl,proto3" json:"acl,omitempty"` // active_requests is an array of request-id strings of active requests - ActiveRequests []string `protobuf:"bytes,5,rep,name=active_requests,json=activeRequests,proto3" json:"active_requests,omitempty"` + ActiveRequests []string `protobuf:"bytes,5,rep,name=active_requests,json=activeRequests,proto3" json:"active_requests,omitempty"` + // suggested_reviewers for the given user. + // Only present when detailed information is queried from the auth server. SuggestedReviewers []string `protobuf:"bytes,6,rep,name=suggested_reviewers,json=suggestedReviewers,proto3" json:"suggested_reviewers,omitempty"` - RequestableRoles []string `protobuf:"bytes,7,rep,name=requestable_roles,json=requestableRoles,proto3" json:"requestable_roles,omitempty"` + // requestable_roles for the given user. + // Only present when detailed information is queried from the auth server. + RequestableRoles []string `protobuf:"bytes,7,rep,name=requestable_roles,json=requestableRoles,proto3" json:"requestable_roles,omitempty"` } func (x *LoggedInUser) Reset() { diff --git a/lib/teleterm/apiserver/handler/handler_clusters.go b/lib/teleterm/apiserver/handler/handler_clusters.go index 8018ce770f7e5..7b18daad33ed0 100644 --- a/lib/teleterm/apiserver/handler/handler_clusters.go +++ b/lib/teleterm/apiserver/handler/handler_clusters.go @@ -76,43 +76,43 @@ func (s *Handler) RemoveCluster(ctx context.Context, req *api.RemoveClusterReque // GetCluster returns a cluster func (s *Handler) GetCluster(ctx context.Context, req *api.GetClusterRequest) (*api.Cluster, error) { - cluster, err := s.DaemonService.ResolveFullCluster(ctx, req.ClusterUri) + cluster, err := s.DaemonService.ResolveClusterWithDetails(ctx, req.ClusterUri) if err != nil { return nil, trace.Wrap(err) } - return newAPIRootCluster(cluster), nil + return newAPIRootClusterWithDetails(cluster), nil } func newAPIRootCluster(cluster *clusters.Cluster) *api.Cluster { - apiCluster := &api.Cluster{ - Uri: cluster.URI.String(), - Name: cluster.Name, - ProxyHost: cluster.GetProxyHost(), - Connected: cluster.Connected(), - LoggedInUser: newAPILoggedInUser(cluster.LoggedInUser), - } + loggedInUser := cluster.GetLoggedInUser() - // Only include features in the api response if they - // exist on the supplied cluster - if cluster.Features != nil { - apiCluster.Features = &api.Features{ - AdvancedAccessWorkflows: cluster.Features.GetAdvancedAccessWorkflows(), - } + apiCluster := &api.Cluster{ + Uri: cluster.URI.String(), + Name: cluster.Name, + ProxyHost: cluster.GetProxyHost(), + Connected: cluster.Connected(), + LoggedInUser: &api.LoggedInUser{ + Name: loggedInUser.Name, + SshLogins: loggedInUser.SSHLogins, + Roles: loggedInUser.Roles, + ActiveRequests: loggedInUser.ActiveRequests, + }, } return apiCluster } -func newAPILoggedInUser(user clusters.LoggedInUser) *api.LoggedInUser { - return &api.LoggedInUser{ - Name: user.Name, - SshLogins: user.SSHLogins, - Roles: user.Roles, - ActiveRequests: user.ActiveRequests, - SuggestedReviewers: user.SuggestedReviewers, - RequestableRoles: user.RequestableRoles, +func newAPIRootClusterWithDetails(cluster *clusters.ClusterWithDetails) *api.Cluster { + apiCluster := newAPIRootCluster(cluster.Cluster) + + apiCluster.Features = &api.Features{ + AdvancedAccessWorkflows: cluster.Features.GetAdvancedAccessWorkflows(), } + apiCluster.LoggedInUser.RequestableRoles = cluster.RequestableRoles + apiCluster.LoggedInUser.SuggestedReviewers = cluster.SuggestedReviewers + + return apiCluster } func newAPILeafCluster(leaf clusters.LeafCluster) *api.Cluster { diff --git a/lib/teleterm/clusters/cluster.go b/lib/teleterm/clusters/cluster.go index 745f6217c544e..6da1c3d61298e 100644 --- a/lib/teleterm/clusters/cluster.go +++ b/lib/teleterm/clusters/cluster.go @@ -51,11 +51,16 @@ type Cluster struct { clusterClient *client.TeleportClient // clock is a clock for time-related operations clock clockwork.Clock +} + +type ClusterWithDetails struct { + *Cluster // Auth server features - // only present where the auth client can be queried - // and set with GetClusterFeatures - Features *proto.Features - LoggedInUser LoggedInUser + Features *proto.Features + // SuggestedReviewers for the given user. + SuggestedReviewers []string + // RequestableRoles for the given user. + RequestableRoles []string } // Connected indicates if connection to the cluster can be established @@ -63,10 +68,10 @@ func (c *Cluster) Connected() bool { return c.status.Name != "" && !c.status.IsExpired(c.clock) } -// EnrichWithDetails will make a network request to the auth server and add details to the -// current Cluster that cannot be found on the disk only, including details about the LoggedInUser +// GetWithDetails makes requests to the auth server to return details of the current +// Cluster that cannot be found on the disk only, including details about the user // and enabled enterprise features. This method requires a valid cert. -func (c *Cluster) EnrichWithDetails(ctx context.Context) (*Cluster, error) { +func (c *Cluster) GetWithDetails(ctx context.Context) (*ClusterWithDetails, error) { var ( pingResponse proto.PingResponse caps *types.AccessCapabilities @@ -104,14 +109,14 @@ func (c *Cluster) EnrichWithDetails(ctx context.Context) (*Cluster, error) { return nil, trace.Wrap(err) } - user := c.GetLoggedInUser() - user.SuggestedReviewers = caps.SuggestedReviewers - user.RequestableRoles = caps.RequestableRoles - c.LoggedInUser = user - - c.Features = pingResponse.ServerFeatures + withDetails := &ClusterWithDetails{ + Cluster: c, + SuggestedReviewers: caps.SuggestedReviewers, + RequestableRoles: caps.RequestableRoles, + Features: pingResponse.ServerFeatures, + } - return c, nil + return withDetails, nil } // GetRoles returns currently logged-in user roles @@ -214,9 +219,6 @@ type LoggedInUser struct { Roles []string // ActiveRequests is the user active requests ActiveRequests []string - - SuggestedReviewers []string - RequestableRoles []string } // addMetadataToRetryableError is Connect's equivalent of client.RetryWithRelogin. By adding the diff --git a/lib/teleterm/daemon/daemon.go b/lib/teleterm/daemon/daemon.go index 331039baf7cbc..fdad3346d5012 100644 --- a/lib/teleterm/daemon/daemon.go +++ b/lib/teleterm/daemon/daemon.go @@ -126,20 +126,20 @@ func (s *Service) ResolveCluster(uri string) (*clusters.Cluster, error) { return cluster, nil } -// ResolveFullCluster returns full cluster information. It makes a request to the auth server and includes -// details about the cluster and logged in user -func (s *Service) ResolveFullCluster(ctx context.Context, uri string) (*clusters.Cluster, error) { +// ResolveClusterWithDetails returns fully detailed cluster information. It makes requests to the auth server and includes +// details about the cluster and logged in user. +func (s *Service) ResolveClusterWithDetails(ctx context.Context, uri string) (*clusters.ClusterWithDetails, error) { cluster, err := s.ResolveCluster(uri) if err != nil { return nil, trace.Wrap(err) } - cluster, err = cluster.EnrichWithDetails(ctx) + withDetails, err := cluster.GetWithDetails(ctx) if err != nil { return nil, trace.Wrap(err) } - return cluster, nil + return withDetails, nil } // ClusterLogout logs a user out from the cluster From 2012f2cff4443351a1dfd80ada90e926a4759335 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Wed, 15 Feb 2023 16:04:52 +0100 Subject: [PATCH 2/2] Adjust test to v11 --- integration/teleterm_test.go | 40 +++--------------------------------- 1 file changed, 3 insertions(+), 37 deletions(-) diff --git a/integration/teleterm_test.go b/integration/teleterm_test.go index 64c28682cb7ed..01e705e0b19db 100644 --- a/integration/teleterm_test.go +++ b/integration/teleterm_test.go @@ -53,12 +53,6 @@ func TestTeleterm(t *testing.T) { }) require.NoError(t, err) - t.Run("adding root cluster", func(t *testing.T) { - t.Parallel() - - testAddingRootCluster(t, pack, creds) - }) - t.Run("ListRootClusters returns logged in user", func(t *testing.T) { t.Parallel() @@ -71,34 +65,6 @@ func TestTeleterm(t *testing.T) { }) } -func testAddingRootCluster(t *testing.T, pack *dbhelpers.DatabasePack, creds *helpers.UserCreds) { - storage, err := clusters.NewStorage(clusters.Config{ - Dir: t.TempDir(), - InsecureSkipVerify: true, - }) - require.NoError(t, err) - - daemonService, err := daemon.New(daemon.Config{ - Storage: storage, - }) - require.NoError(t, err) - t.Cleanup(func() { - daemonService.Stop() - }) - - addedCluster, err := daemonService.AddCluster(context.Background(), pack.Root.Cluster.Web) - require.NoError(t, err) - - clusters, err := daemonService.ListRootClusters(context.Background()) - require.NoError(t, err) - - clusterURIs := make([]uri.ResourceURI, 0, len(clusters)) - for _, cluster := range clusters { - clusterURIs = append(clusterURIs, cluster.URI) - } - require.ElementsMatch(t, clusterURIs, []uri.ResourceURI{addedCluster.URI}) -} - func testListRootClustersReturnsLoggedInUser(t *testing.T, pack *dbhelpers.DatabasePack, creds *helpers.UserCreds) { tc := mustLogin(t, pack.Root.User.GetName(), pack, creds) @@ -140,11 +106,11 @@ func testGetClusterReturnsPropertiesFromAuthServer(t *testing.T, pack *dbhelpers userName := fmt.Sprintf("%s-%s", "user", uuid) roleName := fmt.Sprintf("%s-%s", "get-cluster-role", uuid) - requestableRole, err := types.NewRole(requestableRoleName, types.RoleSpecV6{}) + requestableRole, err := types.NewRole(requestableRoleName, types.RoleSpecV5{}) require.NoError(t, err) // Create user role with ability to request role - userRole, err := types.NewRole(roleName, types.RoleSpecV6{ + userRole, err := types.NewRole(roleName, types.RoleSpecV5{ Options: types.RoleOptions{}, Allow: types.RoleConditions{ Logins: []string{ @@ -224,6 +190,6 @@ func mustLogin(t *testing.T, userName string, pack *dbhelpers.DatabasePack, cred require.NoError(t, err) // The profile on disk created by NewClientWithCreds doesn't have WebProxyAddr set. tc.WebProxyAddr = pack.Root.Cluster.Web - tc.SaveProfile(false /* makeCurrent */) + tc.SaveProfile(tc.KeysDir, false /* makeCurrent */) return tc }