From 14770044f1ab7990c2bd9da18436a94d4a676511 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 13 Jan 2025 17:55:41 +0100 Subject: [PATCH] Add basic support for target port in gateways in Connect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Update type for targetSubresourceName on DocumentGateway The way DocumentsService.createGatewayDocument is implemented means that the targetSubresourceName property is always present, but it can be undefined. * Use "local port" instead of "port" in DocumentGatewapApp * Rewrite gateway FieldInputs to use styled components * Update comments in protos * useGateway: Stabilize useAsync functions of ports * Add padding to menu label if it's first child * Add support for required prop to Input and FieldInput * Add UI for changing target port * ActionButtons: Show ports of multi-port apps when VNet is not supported Now that we have support for the target port in Connect's gateways, we can show the ports and then open a gateway for that specific port on click. * Add RWMutex to gateways * Clear app gateway cert on target port change * Remove gateways/app.LocalProxyURL It was used only in tests and it made sense only for web apps anyway. * TestTCP: Close connections when test ends * Create context with timeout in testGatewayCertRenewal …instead of in each function that uses it. * Add tests for changing the target port of a TCP gateway * Parallelize app gateway tests within MFA/non-MFA groups * Make testGatewayConnection take ctx as first arg This will be needed in tests that check target port validation. * Validate target port of app gateways * Increase timeouts in app gateway tests --- .../go/teleport/lib/teleterm/v1/gateway.pb.go | 5 +- .../ts/teleport/lib/teleterm/v1/gateway_pb.ts | 5 +- integration/appaccess/appaccess_test.go | 1 + integration/appaccess/pack.go | 56 ++++ integration/proxy/proxy_helpers.go | 53 +++- integration/proxy/proxy_test.go | 30 ++- integration/proxy/teleterm_test.go | 254 +++++++++++++++--- .../apiserver/handler/handler_gateways.go | 2 +- lib/teleterm/clusters/cluster_apps.go | 31 ++- lib/teleterm/clusters/cluster_gateways.go | 31 ++- lib/teleterm/daemon/daemon.go | 24 +- lib/teleterm/gateway/app.go | 11 - lib/teleterm/gateway/app_middleware.go | 6 +- lib/teleterm/gateway/base.go | 28 +- lib/teleterm/gateway/config.go | 5 + lib/teleterm/gateway/interfaces.go | 5 +- lib/teleterm/gateway/kube.go | 2 + proto/teleport/lib/teleterm/v1/gateway.proto | 5 +- web/packages/design/src/Input/Input.tsx | 3 + web/packages/design/src/Menu/Menu.story.tsx | 12 + web/packages/design/src/Menu/MenuItem.tsx | 30 ++- web/packages/design/src/keyframes.ts | 4 + .../components/FieldInput/FieldInput.tsx | 5 +- .../teleterm/src/services/tshd/testHelpers.ts | 2 +- .../src/ui/DocumentCluster/ActionButtons.tsx | 97 ++++--- .../src/ui/DocumentGateway/useGateway.ts | 63 +++-- .../src/ui/DocumentGatewayApp/AppGateway.tsx | 208 +++++++++++--- .../DocumentGatewayApp.story.tsx | 47 +++- .../DocumentGatewayApp/DocumentGatewayApp.tsx | 13 +- .../src/ui/TabHost/useTabShortcuts.test.tsx | 2 + .../src/ui/components/FieldInputs.tsx | 37 +-- .../src/ui/components/OfflineGateway.tsx | 4 +- .../documentsService/connectToApp.test.ts | 28 +- .../documentsService/connectToApp.ts | 20 +- .../documentsService/documentsService.test.ts | 2 + .../documentsService/types.ts | 6 +- 36 files changed, 904 insertions(+), 233 deletions(-) diff --git a/gen/proto/go/teleport/lib/teleterm/v1/gateway.pb.go b/gen/proto/go/teleport/lib/teleterm/v1/gateway.pb.go index 7f5f4f8332bd0..fcfb0d077a532 100644 --- a/gen/proto/go/teleport/lib/teleterm/v1/gateway.pb.go +++ b/gen/proto/go/teleport/lib/teleterm/v1/gateway.pb.go @@ -62,10 +62,11 @@ type Gateway struct { LocalAddress string `protobuf:"bytes,5,opt,name=local_address,json=localAddress,proto3" json:"local_address,omitempty"` // local_port is the gateway address on localhost LocalPort string `protobuf:"bytes,6,opt,name=local_port,json=localPort,proto3" json:"local_port,omitempty"` - // protocol is the gateway protocol + // protocol is the protocol used by the gateway. For databases, it matches the type of the + // database that the gateway targets. For apps, it's either "HTTP" or "TCP". Protocol string `protobuf:"bytes,7,opt,name=protocol,proto3" json:"protocol,omitempty"` // target_subresource_name points at a subresource of the remote resource, for example a - // database name on a database server. + // database name on a database server or a target port of a multi-port TCP app. TargetSubresourceName string `protobuf:"bytes,9,opt,name=target_subresource_name,json=targetSubresourceName,proto3" json:"target_subresource_name,omitempty"` // gateway_cli_client represents a command that the user can execute to connect to the resource // through the gateway. diff --git a/gen/proto/ts/teleport/lib/teleterm/v1/gateway_pb.ts b/gen/proto/ts/teleport/lib/teleterm/v1/gateway_pb.ts index f6523f7cc2210..194cc93867671 100644 --- a/gen/proto/ts/teleport/lib/teleterm/v1/gateway_pb.ts +++ b/gen/proto/ts/teleport/lib/teleterm/v1/gateway_pb.ts @@ -80,14 +80,15 @@ export interface Gateway { */ localPort: string; /** - * protocol is the gateway protocol + * protocol is the protocol used by the gateway. For databases, it matches the type of the + * database that the gateway targets. For apps, it's either "HTTP" or "TCP". * * @generated from protobuf field: string protocol = 7; */ protocol: string; /** * target_subresource_name points at a subresource of the remote resource, for example a - * database name on a database server. + * database name on a database server or a target port of a multi-port TCP app. * * @generated from protobuf field: string target_subresource_name = 9; */ diff --git a/integration/appaccess/appaccess_test.go b/integration/appaccess/appaccess_test.go index dffd5f8aa1912..8bb73e091754b 100644 --- a/integration/appaccess/appaccess_test.go +++ b/integration/appaccess/appaccess_test.go @@ -831,6 +831,7 @@ func TestTCP(t *testing.T) { conn, err := net.Dial("tcp", localProxyAddress) require.NoError(t, err) + defer conn.Close() buf := make([]byte, 1024) n, err := conn.Read(buf) diff --git a/integration/appaccess/pack.go b/integration/appaccess/pack.go index 609a60adca036..a3e19634c79e0 100644 --- a/integration/appaccess/pack.go +++ b/integration/appaccess/pack.go @@ -185,6 +185,34 @@ func (p *Pack) RootAppPublicAddr() string { return p.rootAppPublicAddr } +func (p *Pack) RootTCPAppName() string { + return p.rootTCPAppName +} + +func (p *Pack) RootTCPMessage() string { + return p.rootTCPMessage +} + +func (p *Pack) RootTCPMultiPortAppName() string { + return p.rootTCPMultiPortAppName +} + +func (p *Pack) RootTCPMultiPortAppPortAlpha() int { + return p.rootTCPMultiPortAppPortAlpha +} + +func (p *Pack) RootTCPMultiPortMessageAlpha() string { + return p.rootTCPMultiPortMessageAlpha +} + +func (p *Pack) RootTCPMultiPortAppPortBeta() int { + return p.rootTCPMultiPortAppPortBeta +} + +func (p *Pack) RootTCPMultiPortMessageBeta() string { + return p.rootTCPMultiPortMessageBeta +} + func (p *Pack) RootAuthServer() *auth.Server { return p.rootCluster.Process.GetAuthServer() } @@ -201,6 +229,34 @@ func (p *Pack) LeafAppPublicAddr() string { return p.leafAppPublicAddr } +func (p *Pack) LeafTCPAppName() string { + return p.leafTCPAppName +} + +func (p *Pack) LeafTCPMessage() string { + return p.leafTCPMessage +} + +func (p *Pack) LeafTCPMultiPortAppName() string { + return p.leafTCPMultiPortAppName +} + +func (p *Pack) LeafTCPMultiPortAppPortAlpha() int { + return p.leafTCPMultiPortAppPortAlpha +} + +func (p *Pack) LeafTCPMultiPortMessageAlpha() string { + return p.leafTCPMultiPortMessageAlpha +} + +func (p *Pack) LeafTCPMultiPortAppPortBeta() int { + return p.leafTCPMultiPortAppPortBeta +} + +func (p *Pack) LeafTCPMultiPortMessageBeta() string { + return p.leafTCPMultiPortMessageBeta +} + func (p *Pack) LeafAuthServer() *auth.Server { return p.leafCluster.Process.GetAuthServer() } diff --git a/integration/proxy/proxy_helpers.go b/integration/proxy/proxy_helpers.go index 9de514caf73e1..e5e717410e8b9 100644 --- a/integration/proxy/proxy_helpers.go +++ b/integration/proxy/proxy_helpers.go @@ -28,6 +28,7 @@ import ( "net/http" "net/url" "path/filepath" + "strconv" "strings" "testing" "time" @@ -684,7 +685,7 @@ func mustFindKubePod(t *testing.T, tc *client.TeleportClient) { require.Equal(t, types.KindKubePod, response.Resources[0].Kind) } -func mustConnectDatabaseGateway(t *testing.T, _ *daemon.Service, gw gateway.Gateway) { +func mustConnectDatabaseGateway(ctx context.Context, t *testing.T, _ *daemon.Service, gw gateway.Gateway) { t.Helper() dbGateway, err := gateway.AsDatabase(gw) @@ -705,15 +706,15 @@ func mustConnectDatabaseGateway(t *testing.T, _ *daemon.Service, gw gateway.Gate require.NoError(t, client.Close()) } -// mustConnectAppGateway verifies that the gateway acts as an unauthenticated proxy that forwards -// requests to the app behind it. -func mustConnectAppGateway(t *testing.T, _ *daemon.Service, gw gateway.Gateway) { +// mustConnectWebAppGateway verifies that the gateway acts as an unauthenticated proxy that forwards +// requests to the web app behind it. +func mustConnectWebAppGateway(ctx context.Context, t *testing.T, _ *daemon.Service, gw gateway.Gateway) { t.Helper() - appGw, err := gateway.AsApp(gw) - require.NoError(t, err) + gatewayAddress := net.JoinHostPort(gw.LocalAddress(), gw.LocalPort()) + gatewayURL := fmt.Sprintf("http://%s", gatewayAddress) - req, err := http.NewRequest(http.MethodGet, appGw.LocalProxyURL(), nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, gatewayURL, nil) require.NoError(t, err) client := &http.Client{} @@ -724,6 +725,44 @@ func mustConnectAppGateway(t *testing.T, _ *daemon.Service, gw gateway.Gateway) require.Equal(t, http.StatusOK, resp.StatusCode) } +func makeMustConnectMultiPortTCPAppGateway(wantMessage string, otherTargetPort int, otherWantMessage string) testGatewayConnectionFunc { + return func(ctx context.Context, t *testing.T, d *daemon.Service, gw gateway.Gateway) { + t.Helper() + + gwURI := gw.URI().String() + originalTargetPort := gw.TargetSubresourceName() + makeMustConnectTCPAppGateway(wantMessage)(ctx, t, d, gw) + + _, err := d.SetGatewayTargetSubresourceName(ctx, gwURI, strconv.Itoa(otherTargetPort)) + require.NoError(t, err) + makeMustConnectTCPAppGateway(otherWantMessage)(ctx, t, d, gw) + + // Restore the original port, so that the next time the test calls this function after certs + // expire, wantMessage is going to match the port that the gateway points to. + _, err = d.SetGatewayTargetSubresourceName(ctx, gwURI, originalTargetPort) + require.NoError(t, err) + makeMustConnectTCPAppGateway(wantMessage)(ctx, t, d, gw) + } +} + +func makeMustConnectTCPAppGateway(wantMessage string) testGatewayConnectionFunc { + return func(ctx context.Context, t *testing.T, _ *daemon.Service, gw gateway.Gateway) { + t.Helper() + + gatewayAddress := net.JoinHostPort(gw.LocalAddress(), gw.LocalPort()) + conn, err := net.Dial("tcp", gatewayAddress) + require.NoError(t, err) + defer conn.Close() + + buf := make([]byte, 1024) + n, err := conn.Read(buf) + require.NoError(t, err) + + resp := strings.TrimSpace(string(buf[:n])) + require.Equal(t, wantMessage, resp) + } +} + func kubeClientForLocalProxy(t *testing.T, kubeconfigPath, teleportCluster, kubeCluster string) *kubernetes.Clientset { t.Helper() diff --git a/integration/proxy/proxy_test.go b/integration/proxy/proxy_test.go index 0dcf986d70109..7935c5aff06cb 100644 --- a/integration/proxy/proxy_test.go +++ b/integration/proxy/proxy_test.go @@ -54,6 +54,7 @@ import ( "github.com/gravitational/teleport/lib" "github.com/gravitational/teleport/lib/auth/testauthority" libclient "github.com/gravitational/teleport/lib/client" + "github.com/gravitational/teleport/lib/client/mfa" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/multiplexer" @@ -1315,18 +1316,29 @@ func TestALPNSNIProxyAppAccess(t *testing.T) { }) t.Run("teleterm app gateways cert renewal", func(t *testing.T) { - user, _ := pack.CreateUser(t) - tc := pack.MakeTeleportClient(t, user.GetName()) - - // test without per session MFA. - testTeletermAppGateway(t, pack, tc) + t.Run("without per-session MFA", func(t *testing.T) { + makeTC := func(t *testing.T) (*libclient.TeleportClient, mfa.WebauthnLoginFunc) { + user, _ := pack.CreateUser(t) + tc := pack.MakeTeleportClient(t, user.GetName()) + return tc, nil + } + testTeletermAppGateway(t, pack, makeTC) + testTeletermAppGatewayTargetPortValidation(t, pack, makeTC) + }) - t.Run("per session MFA", func(t *testing.T) { - // They update user's authentication to Webauthn so they must run after tests which do not use MFA. + t.Run("per-session MFA", func(t *testing.T) { + // They update clusters authentication to Webauthn so they must run after tests which do not use MFA. requireSessionMFAAuthPref(ctx, t, pack.RootAuthServer(), "127.0.0.1") requireSessionMFAAuthPref(ctx, t, pack.LeafAuthServer(), "127.0.0.1") - tc.WebauthnLogin = setupUserMFA(ctx, t, pack.RootAuthServer(), user.GetName(), "127.0.0.1") - testTeletermAppGateway(t, pack, tc) + makeTCAndWebauthnLogin := func(t *testing.T) (*libclient.TeleportClient, mfa.WebauthnLoginFunc) { + // Create a separate user for each tests to enable parallel tests that use per-session MFA. + // See the comment for webauthnLogin in setupUserMFA for more details. + user, _ := pack.CreateUser(t) + tc := pack.MakeTeleportClient(t, user.GetName()) + webauthnLogin := setupUserMFA(ctx, t, pack.RootAuthServer(), user.GetName(), "127.0.0.1") + return tc, webauthnLogin + } + testTeletermAppGateway(t, pack, makeTCAndWebauthnLogin) }) }) } diff --git a/integration/proxy/teleterm_test.go b/integration/proxy/teleterm_test.go index 67feeda87944c..18b0efd4884c7 100644 --- a/integration/proxy/teleterm_test.go +++ b/integration/proxy/teleterm_test.go @@ -19,9 +19,11 @@ package proxy import ( + "cmp" "context" "errors" "net" + "strconv" "sync" "sync/atomic" "testing" @@ -50,9 +52,9 @@ import ( "github.com/gravitational/teleport/lib/auth/mocku2f" wancli "github.com/gravitational/teleport/lib/auth/webauthncli" wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" - "github.com/gravitational/teleport/lib/client" libclient "github.com/gravitational/teleport/lib/client" "github.com/gravitational/teleport/lib/client/clientcache" + "github.com/gravitational/teleport/lib/client/mfa" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/service" "github.com/gravitational/teleport/lib/service/servicecfg" @@ -168,8 +170,8 @@ func testDBGatewayCertRenewal(ctx context.Context, t *testing.T, params dbGatewa TargetURI: params.databaseURI.String(), TargetUser: params.pack.Root.User.GetName(), }, - testGatewayConnectionFunc: mustConnectDatabaseGateway, - webauthnLogin: params.webauthnLogin, + testGatewayConnection: mustConnectDatabaseGateway, + webauthnLogin: params.webauthnLogin, generateAndSetupUserCreds: func(t *testing.T, tc *libclient.TeleportClient, ttl time.Duration) { creds, err := helpers.GenerateUserCreds(helpers.UserCredsRequest{ Process: params.pack.Root.Cluster.Process, @@ -184,7 +186,7 @@ func testDBGatewayCertRenewal(ctx context.Context, t *testing.T, params dbGatewa ) } -type testGatewayConnectionFunc func(*testing.T, *daemon.Service, gateway.Gateway) +type testGatewayConnectionFunc func(context.Context, *testing.T, *daemon.Service, gateway.Gateway) type generateAndSetupUserCredsFunc func(t *testing.T, tc *libclient.TeleportClient, ttl time.Duration) @@ -192,14 +194,19 @@ type gatewayCertRenewalParams struct { tc *libclient.TeleportClient albAddr string createGatewayParams daemon.CreateGatewayParams - testGatewayConnectionFunc testGatewayConnectionFunc + testGatewayConnection testGatewayConnectionFunc webauthnLogin libclient.WebauthnLoginFunc generateAndSetupUserCreds generateAndSetupUserCredsFunc + wantPromptMFACallCount int } func testGatewayCertRenewal(ctx context.Context, t *testing.T, params gatewayCertRenewalParams) { t.Helper() + // The test can potentially hang forever if something is wrong with the MFA prompt, add a timeout. + ctx, cancel := context.WithTimeout(ctx, time.Minute) + t.Cleanup(cancel) + tc := params.tc // Save the profile yaml file to disk as test helpers like helpers.NewClientWithCreds don't do @@ -273,7 +280,7 @@ func testGatewayCertRenewal(ctx context.Context, t *testing.T, params gatewayCer gateway, err := daemonService.CreateGateway(ctx, params.createGatewayParams) require.NoError(t, err, trace.DebugReport(err)) - params.testGatewayConnectionFunc(t, daemonService, gateway) + params.testGatewayConnection(ctx, t, daemonService, gateway) // Advance the fake clock to simulate the db cert expiry inside the middleware. fakeClock.Advance(time.Hour * 48) @@ -286,16 +293,17 @@ func testGatewayCertRenewal(ctx context.Context, t *testing.T, params gatewayCer // and then it will attempt to reissue the user cert using an expired user cert. // The mocked tshdEventsClient will issue a valid user cert, save it to disk, and the middleware // will let the connection through. - params.testGatewayConnectionFunc(t, daemonService, gateway) + params.testGatewayConnection(ctx, t, daemonService, gateway) require.Equal(t, uint32(1), tshdEventsService.reloginCallCount.Load(), "Unexpected number of calls to TSHDEventsClient.Relogin") require.Equal(t, uint32(0), tshdEventsService.sendNotificationCallCount.Load(), "Unexpected number of calls to TSHDEventsClient.SendNotification") if params.webauthnLogin != nil { - // There are two calls, one to issue the certs when creating the gateway and then another to - // reissue them after relogin. - require.Equal(t, uint32(2), tshdEventsService.promptMFACallCount.Load(), + // By default, there are two calls, one to issue the certs when creating the gateway and then + // another to reissue them after relogin. + wantCallCount := cmp.Or(params.wantPromptMFACallCount, 2) + require.Equal(t, uint32(wantCallCount), tshdEventsService.promptMFACallCount.Load(), "Unexpected number of calls to TSHDEventsClient.PromptMFA") } } @@ -474,9 +482,6 @@ func TestTeletermKubeGateway(t *testing.T) { t.Run("root with per-session MFA", func(t *testing.T) { profileName := mustGetProfileName(t, suite.root.Web) kubeURI := uri.NewClusterURI(profileName).AppendKube(kubeClusterName) - // The test can potentially hang forever if something is wrong with the MFA prompt, add a timeout. - ctx, cancel := context.WithTimeout(ctx, 10*time.Second) - t.Cleanup(cancel) testKubeGatewayCertRenewal(ctx, t, kubeGatewayCertRenewalParams{ suite: suite, kubeURI: kubeURI, @@ -486,9 +491,6 @@ func TestTeletermKubeGateway(t *testing.T) { t.Run("leaf with per-session MFA", func(t *testing.T) { profileName := mustGetProfileName(t, suite.root.Web) kubeURI := uri.NewClusterURI(profileName).AppendLeafCluster(suite.leaf.Secrets.SiteName).AppendKube(kubeClusterName) - // The test can potentially hang forever if something is wrong with the MFA prompt, add a timeout. - ctx, cancel := context.WithTimeout(ctx, 10*time.Second) - t.Cleanup(cancel) testKubeGatewayCertRenewal(ctx, t, kubeGatewayCertRenewalParams{ suite: suite, kubeURI: kubeURI, @@ -523,7 +525,7 @@ func testKubeGatewayCertRenewal(ctx context.Context, t *testing.T, params kubeGa }) require.NoError(t, err) - testKubeConnection := func(t *testing.T, daemonService *daemon.Service, gw gateway.Gateway) { + testKubeConnection := func(ctx context.Context, t *testing.T, daemonService *daemon.Service, gw gateway.Gateway) { t.Helper() clientOnce.Do(func() { @@ -548,8 +550,8 @@ func testKubeGatewayCertRenewal(ctx context.Context, t *testing.T, params kubeGa createGatewayParams: daemon.CreateGatewayParams{ TargetURI: params.kubeURI.String(), }, - testGatewayConnectionFunc: testKubeConnection, - webauthnLogin: params.webauthnLogin, + testGatewayConnection: testKubeConnection, + webauthnLogin: params.webauthnLogin, generateAndSetupUserCreds: func(t *testing.T, tc *libclient.TeleportClient, ttl time.Duration) { creds, err := helpers.GenerateUserCreds(helpers.UserCredsRequest{ Process: params.suite.root.Process, @@ -614,6 +616,10 @@ func setupUserMFA(ctx context.Context, t *testing.T, authServer *auth.Server, us }) require.NoError(t, err) + // webauthnLogin is not safe for concurrent use, partly due to the implementation of device, but + // mostly because Teleport itself doesn't allow for more than one in-flight MFA challenge. This is + // an arbitrary limitation which in theory we could change. But for now, parallel tests that use + // webauthnLogin must use a separate user for each test and not trigger parallel MFA prompts. webauthnLogin := func(ctx context.Context, origin string, assertion *wantypes.CredentialAssertion, prompt wancli.LoginPrompt, opts *wancli.LoginOpts) (*proto.MFAAuthenticateResponse, string, error) { car, err := device.SignAssertion(origin, assertion) if err != nil { @@ -676,34 +682,210 @@ func requireSessionMFARole(ctx context.Context, t *testing.T, authServer *auth.S require.NoError(t, err) } -func testTeletermAppGateway(t *testing.T, pack *appaccess.Pack, tc *client.TeleportClient) { +type makeTCAndWebauthnLoginFunc func(t *testing.T) (*libclient.TeleportClient, mfa.WebauthnLoginFunc) + +func testTeletermAppGateway(t *testing.T, pack *appaccess.Pack, makeTCAndWebauthnLogin makeTCAndWebauthnLoginFunc) { ctx := context.Background() t.Run("root cluster", func(t *testing.T) { - profileName := mustGetProfileName(t, pack.RootWebAddr()) - appURI := uri.NewClusterURI(profileName).AppendApp(pack.RootAppName()) + t.Parallel() - // The test can potentially hang forever if something is wrong with the MFA prompt, add a timeout. - ctx, cancel := context.WithTimeout(ctx, 10*time.Second) - t.Cleanup(cancel) - testAppGatewayCertRenewal(ctx, t, pack, tc, appURI) + t.Run("web app", func(t *testing.T) { + t.Parallel() + + profileName := mustGetProfileName(t, pack.RootWebAddr()) + appURI := uri.NewClusterURI(profileName).AppendApp(pack.RootAppName()) + + testAppGatewayCertRenewal(ctx, t, pack, makeTCAndWebauthnLogin, appURI) + }) + + t.Run("TCP app", func(t *testing.T) { + t.Parallel() + + profileName := mustGetProfileName(t, pack.RootWebAddr()) + appURI := uri.NewClusterURI(profileName).AppendApp(pack.RootTCPAppName()) + + tc, webauthnLogin := makeTCAndWebauthnLogin(t) + + testGatewayCertRenewal( + ctx, + t, + gatewayCertRenewalParams{ + tc: tc, + createGatewayParams: daemon.CreateGatewayParams{TargetURI: appURI.String()}, + testGatewayConnection: makeMustConnectTCPAppGateway(pack.RootTCPMessage()), + generateAndSetupUserCreds: pack.GenerateAndSetupUserCreds, + webauthnLogin: webauthnLogin, + }, + ) + }) + + t.Run("multi-port TCP app", func(t *testing.T) { + t.Parallel() + profileName := mustGetProfileName(t, pack.RootWebAddr()) + appURI := uri.NewClusterURI(profileName).AppendApp(pack.RootTCPMultiPortAppName()) + + tc, webauthnLogin := makeTCAndWebauthnLogin(t) + + testGatewayCertRenewal( + ctx, + t, + gatewayCertRenewalParams{ + tc: tc, + createGatewayParams: daemon.CreateGatewayParams{ + TargetURI: appURI.String(), + TargetSubresourceName: strconv.Itoa(pack.RootTCPMultiPortAppPortAlpha()), + }, + testGatewayConnection: makeMustConnectMultiPortTCPAppGateway( + pack.RootTCPMultiPortMessageAlpha(), pack.RootTCPMultiPortAppPortBeta(), pack.RootTCPMultiPortMessageBeta(), + ), + generateAndSetupUserCreds: pack.GenerateAndSetupUserCreds, + webauthnLogin: webauthnLogin, + // First MFA prompt is made when creating the gateway. Then makeMustConnectMultiPortTCPAppGateway + // changes the target port twice, which means two more prompts. + // + // Then testGatewayCertRenewal expires the certs and calls + // makeMustConnectMultiPortTCPAppGateway. The first connection refreshes the expired cert, + // then the function changes the target port twice again, resulting in two more prompts. + wantPromptMFACallCount: 3 + 3, + }, + ) + }) }) t.Run("leaf cluster", func(t *testing.T) { - profileName := mustGetProfileName(t, pack.RootWebAddr()) - appURI := uri.NewClusterURI(profileName). - AppendLeafCluster(pack.LeafAppClusterName()). - AppendApp(pack.LeafAppName()) + t.Parallel() + + t.Run("web app", func(t *testing.T) { + t.Parallel() + + profileName := mustGetProfileName(t, pack.RootWebAddr()) + appURI := uri.NewClusterURI(profileName). + AppendLeafCluster(pack.LeafAppClusterName()). + AppendApp(pack.LeafAppName()) + + testAppGatewayCertRenewal(ctx, t, pack, makeTCAndWebauthnLogin, appURI) + }) + + t.Run("TCP app", func(t *testing.T) { + t.Parallel() + + profileName := mustGetProfileName(t, pack.RootWebAddr()) + appURI := uri.NewClusterURI(profileName).AppendLeafCluster(pack.LeafAppClusterName()).AppendApp(pack.LeafTCPAppName()) - // The test can potentially hang forever if something is wrong with the MFA prompt, add a timeout. - ctx, cancel := context.WithTimeout(ctx, 10*time.Second) + tc, webauthnLogin := makeTCAndWebauthnLogin(t) + + testGatewayCertRenewal( + ctx, + t, + gatewayCertRenewalParams{ + tc: tc, + createGatewayParams: daemon.CreateGatewayParams{TargetURI: appURI.String()}, + testGatewayConnection: makeMustConnectTCPAppGateway(pack.LeafTCPMessage()), + generateAndSetupUserCreds: pack.GenerateAndSetupUserCreds, + webauthnLogin: webauthnLogin, + }, + ) + }) + + t.Run("multi-port TCP app", func(t *testing.T) { + t.Parallel() + + profileName := mustGetProfileName(t, pack.RootWebAddr()) + appURI := uri.NewClusterURI(profileName).AppendLeafCluster(pack.LeafAppClusterName()).AppendApp(pack.LeafTCPMultiPortAppName()) + + tc, webauthnLogin := makeTCAndWebauthnLogin(t) + + testGatewayCertRenewal( + ctx, + t, + gatewayCertRenewalParams{ + tc: tc, + createGatewayParams: daemon.CreateGatewayParams{ + TargetURI: appURI.String(), + TargetSubresourceName: strconv.Itoa(pack.LeafTCPMultiPortAppPortAlpha()), + }, + testGatewayConnection: makeMustConnectMultiPortTCPAppGateway( + pack.LeafTCPMultiPortMessageAlpha(), pack.LeafTCPMultiPortAppPortBeta(), pack.LeafTCPMultiPortMessageBeta(), + ), + generateAndSetupUserCreds: pack.GenerateAndSetupUserCreds, + webauthnLogin: webauthnLogin, + // First MFA prompt is made when creating the gateway. Then makeMustConnectMultiPortTCPAppGateway + // changes the target port twice, which means two more prompts. + // + // Then testGatewayCertRenewal expires the certs and calls + // makeMustConnectMultiPortTCPAppGateway. The first connection refreshes the expired cert, + // then the function changes the target port twice again, resulting in two more prompts. + wantPromptMFACallCount: 3 + 3, + }, + ) + }) + }) +} + +func testTeletermAppGatewayTargetPortValidation(t *testing.T, pack *appaccess.Pack, makeTCAndWebauthnLogin makeTCAndWebauthnLoginFunc) { + t.Run("target port validation", func(t *testing.T) { + t.Parallel() + + tc, _ := makeTCAndWebauthnLogin(t) + err := tc.SaveProfile(false /* makeCurrent */) + require.NoError(t, err) + + storage, err := clusters.NewStorage(clusters.Config{ + Dir: tc.KeysDir, + InsecureSkipVerify: tc.InsecureSkipVerify, + HardwareKeyPromptConstructor: func(rootClusterURI uri.ResourceURI) keys.HardwareKeyPrompt { + return nil + }, + }) + require.NoError(t, err) + daemonService, err := daemon.New(daemon.Config{ + Storage: storage, + CreateTshdEventsClientCredsFunc: func() (grpc.DialOption, error) { + return grpc.WithTransportCredentials(insecure.NewCredentials()), nil + }, + CreateClientCacheFunc: func(newClient clientcache.NewClientFunc) (daemon.ClientCache, error) { + return clientcache.NewNoCache(newClient), nil + }, + KubeconfigsDir: t.TempDir(), + AgentsDir: t.TempDir(), + }) + require.NoError(t, err) + t.Cleanup(func() { + daemonService.Stop() + }) + + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) t.Cleanup(cancel) - testAppGatewayCertRenewal(ctx, t, pack, tc, appURI) + + // Here the test setup ends and actual test code starts. + profileName := mustGetProfileName(t, pack.RootWebAddr()) + appURI := uri.NewClusterURI(profileName).AppendApp(pack.RootTCPMultiPortAppName()) + + _, err = daemonService.CreateGateway(ctx, daemon.CreateGatewayParams{ + TargetURI: appURI.String(), + // 42 shouldn't be handed out to a non-root user when creating a listener on port 0, so it's + // unlikely that 42 is going to end up in the app spec. + TargetSubresourceName: "42", + }) + require.True(t, trace.IsBadParameter(err), "Expected BadParameter, got %v", err) + require.ErrorContains(t, err, "not included in target ports") + + gateway, err := daemonService.CreateGateway(ctx, daemon.CreateGatewayParams{ + TargetURI: appURI.String(), + TargetSubresourceName: strconv.Itoa(pack.RootTCPMultiPortAppPortAlpha()), + }) + require.NoError(t, err) + + _, err = daemonService.SetGatewayTargetSubresourceName(ctx, gateway.URI().String(), "42") + require.True(t, trace.IsBadParameter(err), "Expected BadParameter, got %v", err) + require.ErrorContains(t, err, "not included in target ports") }) } -func testAppGatewayCertRenewal(ctx context.Context, t *testing.T, pack *appaccess.Pack, tc *libclient.TeleportClient, appURI uri.ResourceURI) { +func testAppGatewayCertRenewal(ctx context.Context, t *testing.T, pack *appaccess.Pack, makeTCAndWebauthnLogin makeTCAndWebauthnLoginFunc, appURI uri.ResourceURI) { t.Helper() + tc, webauthnLogin := makeTCAndWebauthnLogin(t) testGatewayCertRenewal( ctx, @@ -713,9 +895,9 @@ func testAppGatewayCertRenewal(ctx context.Context, t *testing.T, pack *appacces createGatewayParams: daemon.CreateGatewayParams{ TargetURI: appURI.String(), }, - testGatewayConnectionFunc: mustConnectAppGateway, + testGatewayConnection: mustConnectWebAppGateway, generateAndSetupUserCreds: pack.GenerateAndSetupUserCreds, - webauthnLogin: tc.WebauthnLogin, + webauthnLogin: webauthnLogin, }, ) } diff --git a/lib/teleterm/apiserver/handler/handler_gateways.go b/lib/teleterm/apiserver/handler/handler_gateways.go index 5a303e8e45c78..dbb0de52c9363 100644 --- a/lib/teleterm/apiserver/handler/handler_gateways.go +++ b/lib/teleterm/apiserver/handler/handler_gateways.go @@ -119,7 +119,7 @@ func makeGatewayCLICommand(cmds cmd.Cmds) *api.GatewayCLICommand { // // In Connect this is used to update the db name of a db connection along with the CLI command. func (s *Handler) SetGatewayTargetSubresourceName(ctx context.Context, req *api.SetGatewayTargetSubresourceNameRequest) (*api.Gateway, error) { - gateway, err := s.DaemonService.SetGatewayTargetSubresourceName(req.GatewayUri, req.TargetSubresourceName) + gateway, err := s.DaemonService.SetGatewayTargetSubresourceName(ctx, req.GatewayUri, req.TargetSubresourceName) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/teleterm/clusters/cluster_apps.go b/lib/teleterm/clusters/cluster_apps.go index 5b92788cb15b4..cfdecb8a62f66 100644 --- a/lib/teleterm/clusters/cluster_apps.go +++ b/lib/teleterm/clusters/cluster_apps.go @@ -25,6 +25,7 @@ import ( apiclient "github.com/gravitational/teleport/api/client" "github.com/gravitational/teleport/api/client/proto" + apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/client" @@ -55,11 +56,11 @@ type SAMLIdPServiceProvider struct { Provider types.SAMLIdPServiceProvider } -func (c *Cluster) getApp(ctx context.Context, authClient authclient.ClientI, appName string) (types.Application, error) { +func GetApp(ctx context.Context, authClient authclient.ClientI, appName string) (types.Application, error) { var app types.Application err := AddMetadataToRetryableError(ctx, func() error { apps, err := apiclient.GetAllResources[types.AppServer](ctx, authClient, &proto.ListResourcesRequest{ - Namespace: c.clusterClient.Namespace, + Namespace: apidefaults.Namespace, ResourceType: types.KindAppServer, PredicateExpression: fmt.Sprintf(`name == "%s"`, appName), }) @@ -143,3 +144,29 @@ func (c *Cluster) GetAWSRoles(app types.Application) aws.Roles { } return aws.Roles{} } + +// ValidateTargetPort parses rawTargetPort to uint32 and checks if it's included in TCP ports of app. +// It also returns an error if app doesn't have any TCP ports defined. +func ValidateTargetPort(app types.Application, rawTargetPort string) (uint32, error) { + if rawTargetPort == "" { + return 0, nil + } + + targetPort, err := parseTargetPort(rawTargetPort) + if err != nil { + return 0, trace.Wrap(err) + } + + tcpPorts := app.GetTCPPorts() + if len(tcpPorts) == 0 { + return 0, trace.BadParameter("cannot specify target port %d because app %s does not provide access to multiple ports", + targetPort, app.GetName()) + } + + if !tcpPorts.Contains(int(targetPort)) { + return 0, trace.BadParameter("port %d is not included in target ports of app %s", + targetPort, app.GetName()) + } + + return targetPort, nil +} diff --git a/lib/teleterm/clusters/cluster_gateways.go b/lib/teleterm/clusters/cluster_gateways.go index 590fa27611f21..5a08464919e11 100644 --- a/lib/teleterm/clusters/cluster_gateways.go +++ b/lib/teleterm/clusters/cluster_gateways.go @@ -21,6 +21,7 @@ package clusters import ( "context" "crypto/tls" + "strconv" "github.com/gravitational/trace" @@ -160,7 +161,7 @@ func (c *Cluster) createKubeGateway(ctx context.Context, params CreateGatewayPar func (c *Cluster) createAppGateway(ctx context.Context, params CreateGatewayParams) (gateway.Gateway, error) { appName := params.TargetURI.GetAppName() - app, err := c.getApp(ctx, params.ClusterClient.AuthClient, appName) + app, err := GetApp(ctx, params.ClusterClient.AuthClient, appName) if err != nil { return nil, trace.Wrap(err) } @@ -170,6 +171,13 @@ func (c *Cluster) createAppGateway(ctx context.Context, params CreateGatewayPara ClusterName: c.clusterClient.SiteName, URI: app.GetURI(), } + if params.TargetSubresourceName != "" { + targetPort, err := ValidateTargetPort(app, params.TargetSubresourceName) + if err != nil { + return nil, trace.Wrap(err) + } + routeToApp.TargetPort = targetPort + } var cert tls.Certificate if err := AddMetadataToRetryableError(ctx, func() error { @@ -182,6 +190,7 @@ func (c *Cluster) createAppGateway(ctx context.Context, params CreateGatewayPara gw, err := gateway.New(gateway.Config{ LocalPort: params.LocalPort, TargetURI: params.TargetURI, + TargetSubresourceName: params.TargetSubresourceName, TargetName: appName, Cert: cert, Protocol: app.GetProtocol(), @@ -195,6 +204,9 @@ func (c *Cluster) createAppGateway(ctx context.Context, params CreateGatewayPara RootClusterCACertPoolFunc: c.clusterClient.RootClusterCACertPool, ClusterName: c.Name, Username: c.status.Username, + // For multi-port TCP apps, the target port is stored in the target subresource name. Whenever + // that field is updated, the local proxy needs to generate a new cert which includes that port. + ClearCertsOnTargetSubresourceNameChange: true, }) return gw, trace.Wrap(err) } @@ -214,7 +226,7 @@ func (c *Cluster) ReissueGatewayCerts(ctx context.Context, clusterClient *client return cert, trace.Wrap(err) case g.TargetURI().IsApp(): appName := g.TargetURI().GetAppName() - app, err := c.getApp(ctx, clusterClient.AuthClient, appName) + app, err := GetApp(ctx, clusterClient.AuthClient, appName) if err != nil { return tls.Certificate{}, trace.Wrap(err) } @@ -224,6 +236,13 @@ func (c *Cluster) ReissueGatewayCerts(ctx context.Context, clusterClient *client ClusterName: c.clusterClient.SiteName, URI: app.GetURI(), } + if g.TargetSubresourceName() != "" { + targetPort, err := parseTargetPort(g.TargetSubresourceName()) + if err != nil { + return tls.Certificate{}, trace.BadParameter(err.Error()) + } + routeToApp.TargetPort = targetPort + } // The cert is returned from this function and finally set on LocalProxy by the middleware. cert, err := c.ReissueAppCert(ctx, clusterClient, routeToApp) @@ -232,3 +251,11 @@ func (c *Cluster) ReissueGatewayCerts(ctx context.Context, clusterClient *client return tls.Certificate{}, trace.NotImplemented("ReissueGatewayCerts does not support this gateway kind %v", g.TargetURI().String()) } } + +func parseTargetPort(rawTargetPort string) (uint32, error) { + targetPort, err := strconv.ParseUint(rawTargetPort, 10, 32) + if err != nil { + return 0, trace.BadParameter(err.Error()) + } + return uint32(targetPort), nil +} diff --git a/lib/teleterm/daemon/daemon.go b/lib/teleterm/daemon/daemon.go index 13f12f4dfa253..3848c43caa2a4 100644 --- a/lib/teleterm/daemon/daemon.go +++ b/lib/teleterm/daemon/daemon.go @@ -511,7 +511,7 @@ func (s *Service) GetGatewayCLICommand(ctx context.Context, gateway gateway.Gate // SetGatewayTargetSubresourceName updates the TargetSubresourceName field of a gateway stored in // s.gateways. -func (s *Service) SetGatewayTargetSubresourceName(gatewayURI, targetSubresourceName string) (gateway.Gateway, error) { +func (s *Service) SetGatewayTargetSubresourceName(ctx context.Context, gatewayURI, targetSubresourceName string) (gateway.Gateway, error) { s.mu.Lock() defer s.mu.Unlock() @@ -520,6 +520,28 @@ func (s *Service) SetGatewayTargetSubresourceName(gatewayURI, targetSubresourceN return nil, trace.Wrap(err) } + targetURI := gateway.TargetURI() + switch { + case targetURI.IsApp(): + clusterClient, err := s.GetCachedClient(ctx, targetURI) + if err != nil { + return nil, trace.Wrap(err) + } + + var app types.Application + if err := clusters.AddMetadataToRetryableError(ctx, func() error { + var err error + app, err = clusters.GetApp(ctx, clusterClient.CurrentCluster(), targetURI.GetAppName()) + return trace.Wrap(err) + }); err != nil { + return nil, trace.Wrap(err) + } + + if _, err := clusters.ValidateTargetPort(app, targetSubresourceName); err != nil { + return nil, trace.Wrap(err) + } + } + gateway.SetTargetSubresourceName(targetSubresourceName) return gateway, nil diff --git a/lib/teleterm/gateway/app.go b/lib/teleterm/gateway/app.go index 603d640a05a9c..248f48fe873a1 100644 --- a/lib/teleterm/gateway/app.go +++ b/lib/teleterm/gateway/app.go @@ -19,8 +19,6 @@ package gateway import ( "context" "crypto/tls" - "net/url" - "strings" "github.com/gravitational/trace" @@ -33,15 +31,6 @@ type app struct { *base } -// LocalProxyURL returns the URL of the local proxy. -func (a *app) LocalProxyURL() string { - proxyURL := url.URL{ - Scheme: strings.ToLower(a.Protocol()), - Host: a.LocalAddress() + ":" + a.LocalPort(), - } - return proxyURL.String() -} - func makeAppGateway(cfg Config) (Gateway, error) { base, err := newBase(cfg) if err != nil { diff --git a/lib/teleterm/gateway/app_middleware.go b/lib/teleterm/gateway/app_middleware.go index 2da5946018147..89a695640f446 100644 --- a/lib/teleterm/gateway/app_middleware.go +++ b/lib/teleterm/gateway/app_middleware.go @@ -43,12 +43,12 @@ func (m *appMiddleware) OnNewConnection(ctx context.Context, lp *alpn.LocalProxy return nil } - // Return early and don't fire onExpiredCert if certs are invalid but not due to expiry. - if !errors.As(err, &x509.CertificateInvalidError{}) { + // Return early and don't fire onExpiredCert if certs are invalid but not due to expiry or removal. + if !errors.As(err, &x509.CertificateInvalidError{}) && !trace.IsNotFound(err) { return trace.Wrap(err) } - m.log.WithError(err).Debug("Gateway certificates have expired") + m.log.WithError(err).Debug("Gateway certificates have expired or been removed") cert, err := m.onExpiredCert(ctx) if err != nil { diff --git a/lib/teleterm/gateway/base.go b/lib/teleterm/gateway/base.go index e0a9a33cc4d86..57657aec42424 100644 --- a/lib/teleterm/gateway/base.go +++ b/lib/teleterm/gateway/base.go @@ -20,9 +20,11 @@ package gateway import ( "context" + "crypto/tls" "fmt" "net" "strconv" + "sync" "github.com/gravitational/trace" "github.com/sirupsen/logrus" @@ -89,6 +91,9 @@ func newBase(cfg Config) (*base, error) { // Close terminates gateway connection. Fails if called on an already closed gateway. func (b *base) Close() error { + b.mu.Lock() + defer b.mu.Unlock() + b.closeCancel() var errs []error @@ -158,17 +163,29 @@ func (b *base) TargetUser() string { } func (b *base) TargetSubresourceName() string { + b.mu.RLock() + defer b.mu.RUnlock() + return b.cfg.TargetSubresourceName } func (b *base) SetTargetSubresourceName(value string) { + b.mu.Lock() + defer b.mu.Unlock() b.cfg.TargetSubresourceName = value + + if b.cfg.ClearCertsOnTargetSubresourceNameChange { + b.Log().Info("Clearing cert") + b.localProxy.SetCert(tls.Certificate{}) + } } func (b *base) Log() *logrus.Entry { return b.cfg.Log } +// LocalAddress returns the local host in the net package terms (localhost or 127.0.0.1, depending +// on the platform). func (b *base) LocalAddress() string { return b.cfg.LocalAddress } @@ -187,15 +204,13 @@ func (b *base) LocalPortInt() int { } func (b *base) cloneConfig() Config { + b.mu.RLock() + defer b.mu.RUnlock() + return *b.cfg } -// Gateway describes local proxy that creates a gateway to the remote Teleport resource. -// -// Gateway is not safe for concurrent use in itself. However, all access to gateways is gated by -// daemon.Service which obtains a lock for any operation pertaining to gateways. -// -// In the future if Gateway becomes more complex it might be worthwhile to add an RWMutex to it. +// Gateway is a local proxy to a remote Teleport resource. type base struct { cfg *Config localProxy *alpn.LocalProxy @@ -206,6 +221,7 @@ type base struct { // that the local proxy is now closed and to release any resources. closeContext context.Context closeCancel context.CancelFunc + mu sync.RWMutex } type TCPPortAllocator interface { diff --git a/lib/teleterm/gateway/config.go b/lib/teleterm/gateway/config.go index c870df9075728..978cbf58d5ae3 100644 --- a/lib/teleterm/gateway/config.go +++ b/lib/teleterm/gateway/config.go @@ -91,6 +91,11 @@ type Config struct { RootClusterCACertPoolFunc alpnproxy.GetClusterCACertPoolFunc // KubeconfigsDir is the directory containing kubeconfigs for kube gateways. KubeconfigsDir string + // ClearCertsOnTargetSubresourceNameChange is useful in situations where TargetSubresourceName is + // used to generate a cert. In that case, after TargetSubresourceName is changed, the gateway will + // clear the cert from the local proxy and the middleware is going to request a new cert on the + // next connection. + ClearCertsOnTargetSubresourceNameChange bool } // OnExpiredCertFunc is the type of a function that is called when a new downstream connection is diff --git a/lib/teleterm/gateway/interfaces.go b/lib/teleterm/gateway/interfaces.go index 4cedf02e7ffd7..b0bbefcb4b1a9 100644 --- a/lib/teleterm/gateway/interfaces.go +++ b/lib/teleterm/gateway/interfaces.go @@ -42,6 +42,8 @@ type Gateway interface { TargetSubresourceName() string SetTargetSubresourceName(value string) Log() *logrus.Entry + // LocalAddress returns the local host in the net package terms (localhost or 127.0.0.1, depending + // on the platform). LocalAddress() string LocalPort() string LocalPortInt() int @@ -94,7 +96,4 @@ type Kube interface { // App defines an app gateway. type App interface { Gateway - - // LocalProxyURL returns the URL of the local proxy. - LocalProxyURL() string } diff --git a/lib/teleterm/gateway/kube.go b/lib/teleterm/gateway/kube.go index 17a3b4a55241d..d03bdd08bd09b 100644 --- a/lib/teleterm/gateway/kube.go +++ b/lib/teleterm/gateway/kube.go @@ -185,6 +185,8 @@ func (k *kube) makeForwardProxyForKube() error { } func (k *kube) writeKubeconfig(key *keys.PrivateKey, cas map[string]tls.Certificate) error { + k.base.mu.RLock() + defer k.base.mu.RUnlock() ca, ok := cas[k.cfg.ClusterName] if !ok { return trace.BadParameter("CA for teleport cluster %q is missing", k.cfg.ClusterName) diff --git a/proto/teleport/lib/teleterm/v1/gateway.proto b/proto/teleport/lib/teleterm/v1/gateway.proto index 7661a6bf31f4a..4399fcc307e26 100644 --- a/proto/teleport/lib/teleterm/v1/gateway.proto +++ b/proto/teleport/lib/teleterm/v1/gateway.proto @@ -43,12 +43,13 @@ message Gateway { string local_address = 5; // local_port is the gateway address on localhost string local_port = 6; - // protocol is the gateway protocol + // protocol is the protocol used by the gateway. For databases, it matches the type of the + // database that the gateway targets. For apps, it's either "HTTP" or "TCP". string protocol = 7; reserved 8; reserved "cli_command"; // target_subresource_name points at a subresource of the remote resource, for example a - // database name on a database server. + // database name on a database server or a target port of a multi-port TCP app. string target_subresource_name = 9; // gateway_cli_client represents a command that the user can execute to connect to the resource // through the gateway. diff --git a/web/packages/design/src/Input/Input.tsx b/web/packages/design/src/Input/Input.tsx index 3cf50e9d1009b..fe3b7feca968c 100644 --- a/web/packages/design/src/Input/Input.tsx +++ b/web/packages/design/src/Input/Input.tsx @@ -70,6 +70,7 @@ interface InputProps extends ColorProps, SpaceProps, WidthProps, HeightProps { inputMode?: InputMode; spellCheck?: boolean; style?: React.CSSProperties; + required?: boolean; 'aria-invalid'?: HTMLAttributes<'input'>['aria-invalid']; 'aria-describedby'?: HTMLAttributes<'input'>['aria-describedby']; @@ -170,6 +171,7 @@ const Input = forwardRef((props, ref) => { inputMode, spellCheck, style, + required, 'aria-invalid': ariaInvalid, 'aria-describedby': ariaDescribedBy, @@ -222,6 +224,7 @@ const Input = forwardRef((props, ref) => { inputMode, spellCheck, style, + required, 'aria-invalid': ariaInvalid, 'aria-describedby': ariaDescribedBy, diff --git a/web/packages/design/src/Menu/Menu.story.tsx b/web/packages/design/src/Menu/Menu.story.tsx index c7b0726ea414b..c3ba4ae802762 100644 --- a/web/packages/design/src/Menu/Menu.story.tsx +++ b/web/packages/design/src/Menu/Menu.story.tsx @@ -107,6 +107,18 @@ export const MenuItems = () => ( Amet nisi tempor + +

Label as first child

+ + Tempus ut libero + Lorem ipsum + Dolor sit amet + + Leo vitae arcu + Donec volutpat + Mauris sit + + ); diff --git a/web/packages/design/src/Menu/MenuItem.tsx b/web/packages/design/src/Menu/MenuItem.tsx index 5ccbae227c835..a9b06373bd787 100644 --- a/web/packages/design/src/Menu/MenuItem.tsx +++ b/web/packages/design/src/Menu/MenuItem.tsx @@ -71,37 +71,39 @@ const MenuItemBase = styled(Flex)` ${fromThemeBase} `; -export const MenuItemSectionLabel = styled(MenuItemBase).attrs({ - px: 2, +export const MenuItemSectionSeparator = styled.hr.attrs({ onClick: event => { // Make sure that clicks on this element don't trigger onClick set on MenuList. event.stopPropagation(); }, })` - font-weight: bold; - min-height: 16px; + background: ${props => props.theme.colors.interactive.tonal.neutral[1]}; + height: 1px; + border: 0; + font-size: 0; `; -export const MenuItemSectionSeparator = styled.hr.attrs({ +export const MenuItemSectionLabel = styled(MenuItemBase).attrs({ + px: 2, onClick: event => { // Make sure that clicks on this element don't trigger onClick set on MenuList. event.stopPropagation(); }, })` - background: ${props => props.theme.colors.interactive.tonal.neutral[1]}; - height: 1px; - border: 0; - font-size: 0; + font-weight: bold; + min-height: 16px; - // Add padding to the label for extra visual space, but only when it follows a separator. - // If a separator follows a MenuItem, there's already enough visual space, so no extra space is - // needed. The hover state of MenuItem highlights everything right from the separator start to the - // end of MenuItem. + // Add padding to the label for extra visual space, but only when it follows a separator or is the + // first child. + // + // If a separator follows a MenuItem, there's already enough visual space between MenuItem and + // separator, so no extra space is needed. The hover state of MenuItem highlights everything right + // from the separator start to the end of MenuItem. // // Padding is used instead of margin here on purpose, so that there's no empty transparent space // between Separator and Label – otherwise clicking on that space would count as a click on // MenuList and not trigger onClick set on Separator or Label. - & + ${MenuItemSectionLabel} { + ${MenuItemSectionSeparator} + &, &:first-child { padding-top: ${props => props.theme.space[1]}px; } `; diff --git a/web/packages/design/src/keyframes.ts b/web/packages/design/src/keyframes.ts index c49799db9f67f..a3a7bf96f7245 100644 --- a/web/packages/design/src/keyframes.ts +++ b/web/packages/design/src/keyframes.ts @@ -46,3 +46,7 @@ export const blink = keyframes` opacity: 100%; } `; + +export const disappear = keyframes` +to { opacity: 0; } +`; diff --git a/web/packages/shared/components/FieldInput/FieldInput.tsx b/web/packages/shared/components/FieldInput/FieldInput.tsx index 2ac28f54e810c..2f3a3eb012550 100644 --- a/web/packages/shared/components/FieldInput/FieldInput.tsx +++ b/web/packages/shared/components/FieldInput/FieldInput.tsx @@ -59,6 +59,7 @@ const FieldInput = forwardRef( toolTipContent = null, disabled = false, markAsError = false, + required = false, ...styles }, ref @@ -94,6 +95,7 @@ const FieldInput = forwardRef( size={size} aria-invalid={hasError || markAsError} aria-describedby={helperTextId} + required={required} /> ); @@ -219,7 +221,7 @@ export type FieldInputProps = BoxProps & { id?: string; name?: string; value?: string; - label?: string; + label?: React.ReactNode; helperText?: React.ReactNode; icon?: React.ComponentType; size?: InputSize; @@ -245,4 +247,5 @@ export type FieldInputProps = BoxProps & { // input box as error color before validator // runs (which marks it as error) markAsError?: boolean; + required?: boolean; }; diff --git a/web/packages/teleterm/src/services/tshd/testHelpers.ts b/web/packages/teleterm/src/services/tshd/testHelpers.ts index b19fc95725192..8cb15ec3e3701 100644 --- a/web/packages/teleterm/src/services/tshd/testHelpers.ts +++ b/web/packages/teleterm/src/services/tshd/testHelpers.ts @@ -290,7 +290,7 @@ export const makeAppGateway = ( targetUri: appUri, localAddress: 'localhost', localPort: '1337', - targetSubresourceName: 'bar', + targetSubresourceName: undefined, gatewayCliCommand: { path: '', preview: 'curl http://localhost:1337', diff --git a/web/packages/teleterm/src/ui/DocumentCluster/ActionButtons.tsx b/web/packages/teleterm/src/ui/DocumentCluster/ActionButtons.tsx index c16dd1d5fa779..39147660e843e 100644 --- a/web/packages/teleterm/src/ui/DocumentCluster/ActionButtons.tsx +++ b/web/packages/teleterm/src/ui/DocumentCluster/ActionButtons.tsx @@ -23,7 +23,7 @@ import { MenuItemSectionLabel, MenuItemSectionSeparator, } from 'design/Menu/MenuItem'; -import { App } from 'gen-proto-ts/teleport/lib/teleterm/v1/app_pb'; +import { App, PortRange } from 'gen-proto-ts/teleport/lib/teleterm/v1/app_pb'; import { Cluster } from 'gen-proto-ts/teleport/lib/teleterm/v1/cluster_pb'; import { Database } from 'gen-proto-ts/teleport/lib/teleterm/v1/database_pb'; import { Kube } from 'gen-proto-ts/teleport/lib/teleterm/v1/kube_pb'; @@ -125,8 +125,11 @@ export function ConnectAppActionButton(props: { app: App }): React.JSX.Element { connectToAppWithVnet(appContext, launchVnet, props.app, targetPort); } - function setUpGateway(): void { - setUpAppGateway(appContext, props.app, { origin: 'resource_table' }); + function setUpGateway(targetPort?: number): void { + setUpAppGateway(appContext, props.app, { + telemetry: { origin: 'resource_table' }, + targetPort, + }); } const rootCluster = appContext.clustersService.findCluster( @@ -229,7 +232,7 @@ function AppButton(props: { cluster: Cluster; rootCluster: Cluster; connectWithVnet(targetPort?: number): void; - setUpGateway(): void; + setUpGateway(targetPort?: number): void; onLaunchUrl(): void; isVnetSupported: boolean; }) { @@ -285,37 +288,15 @@ function AppButton(props: { target="_blank" title="Launch the app in the browser" > - Set up connection + props.setUpGateway()}> + Set up connection + ); } // TCP app with VNet. if (props.isVnetSupported) { - let $targetPorts: JSX.Element; - if (props.app.tcpPorts.length) { - $targetPorts = ( - <> - - Available target ports - {props.app.tcpPorts.map((portRange, index) => ( - props.connectWithVnet(portRange.port)} - > - {formatPortRange(portRange)} - - ))} - - ); - } - return ( props.connectWithVnet()} > - Connect without VNet - {$targetPorts} + props.setUpGateway()}> + Connect without VNet + + {!!props.app.tcpPorts.length && ( + <> + + props.connectWithVnet(port)} + /> + + )} + + ); + } + + // Multi-port TCP app without VNet. + if (props.app.tcpPorts.length) { + return ( + props.setUpGateway()} + > + props.setUpGateway(port)} + /> ); } - // TCP app without VNet. + // Single-port TCP app without VNet. return ( props.setUpGateway()} textTransform="none" > Connect @@ -341,6 +349,29 @@ function AppButton(props: { ); } +const AvailableTargetPorts = (props: { + tcpPorts: PortRange[]; + onItemClick: (portRangePort: number) => void; +}) => ( + <> + Available target ports + {props.tcpPorts.map((portRange, index) => ( + props.onItemClick(portRange.port)} + > + {formatPortRange(portRange)} + + ))} + +); + export function AccessRequestButton(props: { isResourceAdded: boolean; requestStarted: boolean; diff --git a/web/packages/teleterm/src/ui/DocumentGateway/useGateway.ts b/web/packages/teleterm/src/ui/DocumentGateway/useGateway.ts index 1c08bae058742..743667ebfa662 100644 --- a/web/packages/teleterm/src/ui/DocumentGateway/useGateway.ts +++ b/web/packages/teleterm/src/ui/DocumentGateway/useGateway.ts @@ -30,6 +30,7 @@ import { retryWithRelogin } from 'teleterm/ui/utils'; export function useGateway(doc: DocumentGateway) { const ctx = useAppContext(); + const { clustersService } = ctx; const { documentsService } = useWorkspaceContext(); // The port to show as default in the input field in case creating a gateway fails. // This is typically the case if someone reopens the app and the port of the gateway is already @@ -51,7 +52,7 @@ export function useGateway(doc: DocumentGateway) { try { gw = await retryWithRelogin(ctx, doc.targetUri, () => - ctx.clustersService.createGateway({ + clustersService.createGateway({ targetUri: doc.targetUri, localPort: port, targetUser: doc.targetUser, @@ -92,34 +93,52 @@ export function useGateway(doc: DocumentGateway) { }); const [disconnectAttempt, disconnect] = useAsync(async () => { - await ctx.clustersService.removeGateway(doc.gatewayUri); + await clustersService.removeGateway(doc.gatewayUri); documentsService.close(doc.uri); }); const [changeTargetSubresourceNameAttempt, changeTargetSubresourceName] = - useAsync(async (name: string) => { - const updatedGateway = - await ctx.clustersService.setGatewayTargetSubresourceName( - doc.gatewayUri, - name - ); + useAsync( + useCallback( + (name: string) => + retryWithRelogin(ctx, doc.targetUri, async () => { + const updatedGateway = + await clustersService.setGatewayTargetSubresourceName( + doc.gatewayUri, + name + ); - documentsService.update(doc.uri, { - targetSubresourceName: updatedGateway.targetSubresourceName, - }); - }); - - const [changePortAttempt, changePort] = useAsync(async (port: string) => { - const updatedGateway = await ctx.clustersService.setGatewayLocalPort( - doc.gatewayUri, - port + documentsService.update(doc.uri, { + targetSubresourceName: updatedGateway.targetSubresourceName, + }); + }), + [ + clustersService, + documentsService, + doc.uri, + doc.gatewayUri, + ctx, + doc.targetUri, + ] + ) ); - documentsService.update(doc.uri, { - targetSubresourceName: updatedGateway.targetSubresourceName, - port: updatedGateway.localPort, - }); - }); + const [changePortAttempt, changePort] = useAsync( + useCallback( + async (port: string) => { + const updatedGateway = await clustersService.setGatewayLocalPort( + doc.gatewayUri, + port + ); + + documentsService.update(doc.uri, { + targetSubresourceName: updatedGateway.targetSubresourceName, + port: updatedGateway.localPort, + }); + }, + [clustersService, documentsService, doc.uri, doc.gatewayUri] + ) + ); useEffect( function createGatewayOnMount() { diff --git a/web/packages/teleterm/src/ui/DocumentGatewayApp/AppGateway.tsx b/web/packages/teleterm/src/ui/DocumentGatewayApp/AppGateway.tsx index 1c2981ce9f42b..bd31e84d80035 100644 --- a/web/packages/teleterm/src/ui/DocumentGatewayApp/AppGateway.tsx +++ b/web/packages/teleterm/src/ui/DocumentGatewayApp/AppGateway.tsx @@ -16,18 +16,27 @@ * along with this program. If not, see . */ -import { useMemo, useRef } from 'react'; +import { + ChangeEvent, + ChangeEventHandler, + PropsWithChildren, + useEffect, + useMemo, + useState, +} from 'react'; +import styled from 'styled-components'; import { Alert, - Box, ButtonSecondary, + disappear, Flex, H1, - Indicator, Link, + rotate360, Text, } from 'design'; +import { Check, Spinner } from 'design/Icon'; import { Gateway } from 'gen-proto-ts/teleport/lib/teleterm/v1/gateway_pb'; import { TextSelectCopy } from 'shared/components/TextSelectCopy'; import Validation from 'shared/components/Validation'; @@ -39,68 +48,110 @@ import { PortFieldInput } from '../components/FieldInputs'; export function AppGateway(props: { gateway: Gateway; disconnectAttempt: Attempt; - changePort(port: string): void; - changePortAttempt: Attempt; + changeLocalPort(port: string): void; + changeLocalPortAttempt: Attempt; + changeTargetPort(port: string): void; + changeTargetPortAttempt: Attempt; disconnect(): void; }) { const { gateway } = props; - const formRef = useRef(); - const { changePort } = props; - const handleChangePort = useMemo(() => { - return debounce((value: string) => { - if (formRef.current.reportValidity()) { - changePort(value); - } - }, 1000); - }, [changePort]); + const { + changeLocalPort, + changeLocalPortAttempt, + changeTargetPort, + changeTargetPortAttempt, + disconnectAttempt, + } = props; + // It must be possible to update local port while target port is invalid, hence why + // useDebouncedPortChangeHandler checks the validity of only one input at a time. Otherwise the UI + // would lose updates to the local port while the target port was invalid. + const handleLocalPortChange = useDebouncedPortChangeHandler(changeLocalPort); + const handleTargetPortChange = + useDebouncedPortChangeHandler(changeTargetPort); let address = `${gateway.localAddress}:${gateway.localPort}`; if (gateway.protocol === 'HTTP') { address = `http://${address}`; } + // AppGateway doesn't have access to the app resource itself, so it has to decide whether the + // app is multi-port or not in some other way. + // For multi-port apps, DocumentGateway comes with targetSubresourceName prefilled to the first + // port number found in TCP ports. Single-port apps have this field empty. + // So, if targetSubresourceName is present, then the app must be multi-port. In this case, the + // user is free to change it and can never provide an empty targetSubresourceName. + // When the app is not multi-port, targetSubresourceName is empty and the user cannot change it. + const isMultiPort = + gateway.protocol === 'TCP' && gateway.targetSubresourceName; + return ( - - + +

App Connection

Close Connection - {props.disconnectAttempt.status === 'error' && ( - + {disconnectAttempt.status === 'error' && ( + Could not close the connection )} - + + } defaultValue={gateway.localPort} - onChange={e => handleChangePort(e.target.value)} - mb={2} + onChange={handleLocalPortChange} + mb={0} /> + {isMultiPort && ( + + } + required + defaultValue={gateway.targetSubresourceName} + onChange={handleTargetPortChange} + mb={0} + /> + )} - {props.changePortAttempt.status === 'processing' && ( - - )} - Access the app at: - +
+ Access the app at: + +
- {props.changePortAttempt.status === 'error' && ( - - Could not change the port number + {changeLocalPortAttempt.status === 'error' && ( + + Could not change the local port + + )} + + {changeTargetPortAttempt.status === 'error' && ( + + Could not change the target port )} @@ -115,6 +166,89 @@ export function AppGateway(props: { {' '} for more details. - + ); } + +const LabelWithAttemptStatus = (props: { + text: string; + attempt: Attempt; +}) => ( + + {props.text} + {props.attempt.status === 'processing' && ( + + )} + {props.attempt.status === 'success' && ( + // CSS animations are repeated whenever the parent goes from `display: none` to something + // else. As a result, we need to unmount the animated check so that the animation is not + // repeated when the user switches to this tab. + // https://www.w3.org/TR/css-animations-1/#example-4e34d7ba + + + + )} + +); + +/** + * useDebouncedPortChangeHandler returns a debounced change handler that calls the change function + * only if the input from which the event originated is valid. + */ +const useDebouncedPortChangeHandler = ( + changeFunc: (port: string) => void +): ChangeEventHandler => + useMemo( + () => + debounce((event: ChangeEvent) => { + if (event.target.reportValidity()) { + changeFunc(event.target.value); + } + }, 1000), + [changeFunc] + ); + +const AnimatedSpinner = styled(Spinner)` + animation: ${rotate360} 1.5s infinite linear; + // The spinner needs to be positioned absolutely so that the fact that it's spinning + // doesn't affect the size of the parent. + position: absolute; + right: 0; + top: 0; +`; + +const disappearanceDelayMs = 1000; +const disappearanceDurationMs = 200; + +const DisappearingCheck = styled(Check)` + opacity: 1; + animation: ${disappear}; + animation-delay: ${disappearanceDelayMs}ms; + animation-duration: ${disappearanceDurationMs}ms; + animation-fill-mode: forwards; +`; + +const UnmountAfter = ({ + timeoutMs, + children, +}: PropsWithChildren<{ timeoutMs: number }>) => { + const [isMounted, setIsMounted] = useState(true); + + useEffect(() => { + const timeout = setTimeout(() => { + setIsMounted(false); + }, timeoutMs); + + return () => { + clearTimeout(timeout); + }; + }, [timeoutMs]); + + return isMounted ? children : null; +}; diff --git a/web/packages/teleterm/src/ui/DocumentGatewayApp/DocumentGatewayApp.story.tsx b/web/packages/teleterm/src/ui/DocumentGatewayApp/DocumentGatewayApp.story.tsx index c0b4ec802b28e..936f1c8a399b1 100644 --- a/web/packages/teleterm/src/ui/DocumentGatewayApp/DocumentGatewayApp.story.tsx +++ b/web/packages/teleterm/src/ui/DocumentGatewayApp/DocumentGatewayApp.story.tsx @@ -30,9 +30,10 @@ import { MockWorkspaceContextProvider } from 'teleterm/ui/fixtures/MockWorkspace import * as types from 'teleterm/ui/services/workspacesService'; type StoryProps = { - appType: 'web' | 'tcp'; + appType: 'web' | 'tcp' | 'tcp-multi-port'; online: boolean; - changePort: 'succeed' | 'throw-error'; + changeLocalPort: 'succeed' | 'throw-error'; + changeTargetPort: 'succeed' | 'throw-error'; disconnect: 'succeed' | 'throw-error'; }; @@ -42,9 +43,14 @@ const meta: Meta = { argTypes: { appType: { control: { type: 'radio' }, - options: ['web', 'tcp'], + options: ['web', 'tcp', 'tcp-multi-port'], }, - changePort: { + changeLocalPort: { + if: { arg: 'online' }, + control: { type: 'radio' }, + options: ['succeed', 'throw-error'], + }, + changeTargetPort: { if: { arg: 'online' }, control: { type: 'radio' }, options: ['succeed', 'throw-error'], @@ -58,7 +64,8 @@ const meta: Meta = { args: { appType: 'web', online: true, - changePort: 'succeed', + changeLocalPort: 'succeed', + changeTargetPort: 'succeed', disconnect: 'succeed', }, }; @@ -70,6 +77,10 @@ export function Story(props: StoryProps) { if (props.appType === 'tcp') { gateway.protocol = 'TCP'; } + if (props.appType === 'tcp-multi-port') { + gateway.protocol = 'TCP'; + gateway.targetSubresourceName = '4242'; + } const documentGateway: types.DocumentGateway = { kind: 'doc.gateway', targetUri: '/clusters/bar/apps/quux', @@ -80,10 +91,14 @@ export function Story(props: StoryProps) { targetUser: '', status: '', targetName: 'quux', + targetSubresourceName: undefined, }; if (!props.online) { documentGateway.gatewayUri = undefined; } + if (props.appType === 'tcp-multi-port') { + documentGateway.targetSubresourceName = '4242'; + } const appContext = new MockAppContext(); appContext.workspacesService.setState(draftState => { @@ -105,8 +120,26 @@ export function Story(props: StoryProps) { wait(1000).then( () => new MockedUnaryCall( - { ...gateway, localPort }, - props.changePort === 'throw-error' + { + ...appContext.clustersService.findGateway(gateway.uri), + localPort, + }, + props.changeLocalPort === 'throw-error' + ? new Error('something went wrong') + : undefined + ) + ); + appContext.tshd.setGatewayTargetSubresourceName = ({ + targetSubresourceName, + }) => + wait(1000).then( + () => + new MockedUnaryCall( + { + ...appContext.clustersService.findGateway(gateway.uri), + targetSubresourceName, + }, + props.changeTargetPort === 'throw-error' ? new Error('something went wrong') : undefined ) diff --git a/web/packages/teleterm/src/ui/DocumentGatewayApp/DocumentGatewayApp.tsx b/web/packages/teleterm/src/ui/DocumentGatewayApp/DocumentGatewayApp.tsx index ba70a7dfbdbe3..24db9f673be64 100644 --- a/web/packages/teleterm/src/ui/DocumentGatewayApp/DocumentGatewayApp.tsx +++ b/web/packages/teleterm/src/ui/DocumentGatewayApp/DocumentGatewayApp.tsx @@ -29,13 +29,15 @@ export function DocumentGatewayApp(props: { const { doc } = props; const { gateway, - changePort, - changePortAttempt, + changePort: changeLocalPort, + changePortAttempt: changeLocalPortAttempt, connected, connectAttempt, disconnect, disconnectAttempt, reconnect, + changeTargetSubresourceName: changeTargetPort, + changeTargetSubresourceNameAttempt: changeTargetPortAttempt, } = useGateway(doc); return ( @@ -47,14 +49,17 @@ export function DocumentGatewayApp(props: { targetName={doc.targetName} gatewayPort={{ isSupported: true, defaultPort: doc.port }} reconnect={reconnect} + portFieldLabel="Local Port (optional)" /> ) : ( )} diff --git a/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.test.tsx b/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.test.tsx index ce65290c2eb1f..b8fe467178b54 100644 --- a/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.test.tsx +++ b/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.test.tsx @@ -55,6 +55,7 @@ function getMockDocuments(): Document[] { targetUri: '/clusters/bar/dbs/foobar', targetName: 'foobar', targetUser: 'foo', + targetSubresourceName: undefined, origin: 'resource_table', status: '', }, @@ -66,6 +67,7 @@ function getMockDocuments(): Document[] { targetUri: '/clusters/bar/dbs/foobar', targetName: 'foobar', targetUser: 'bar', + targetSubresourceName: undefined, origin: 'resource_table', status: '', }, diff --git a/web/packages/teleterm/src/ui/components/FieldInputs.tsx b/web/packages/teleterm/src/ui/components/FieldInputs.tsx index 21086d8f9bb23..7e7d57e4ec40f 100644 --- a/web/packages/teleterm/src/ui/components/FieldInputs.tsx +++ b/web/packages/teleterm/src/ui/components/FieldInputs.tsx @@ -16,23 +16,26 @@ * along with this program. If not, see . */ -import { forwardRef } from 'react'; +import styled from 'styled-components'; -import FieldInput, { FieldInputProps } from 'shared/components/FieldInput'; +import FieldInput from 'shared/components/FieldInput'; -export const ConfigFieldInput = forwardRef( - (props, ref) => -); +export const ConfigFieldInput = styled(FieldInput).attrs({ size: 'small' })` + input { + &:invalid, + &:invalid:hover { + border-color: ${props => + props.theme.colors.interactive.solid.danger.default}; + } + } +`; -export const PortFieldInput = forwardRef( - (props, ref) => ( - - ) -); +export const PortFieldInput = styled(ConfigFieldInput).attrs({ + type: 'number', + min: 1, + max: 65535, + // Without a min width, the stepper controls end up being to close to a long port number such + // as 65535. minWidth instead of width allows the field to grow with the label, so that e.g. + // a custom label of "Local Port (optional)" is displayed on a single line. + minWidth: '110px', +})``; diff --git a/web/packages/teleterm/src/ui/components/OfflineGateway.tsx b/web/packages/teleterm/src/ui/components/OfflineGateway.tsx index 500a85951ba9a..2dbc1027565ee 100644 --- a/web/packages/teleterm/src/ui/components/OfflineGateway.tsx +++ b/web/packages/teleterm/src/ui/components/OfflineGateway.tsx @@ -36,7 +36,9 @@ export function OfflineGateway(props: { targetName: string; /** Gateway kind displayed in the UI, for example, 'database'. */ gatewayKind: string; + portFieldLabel?: string; }) { + const portFieldLabel = props.portFieldLabel || 'Port (optional)'; const defaultPort = props.gatewayPort.isSupported ? props.gatewayPort.defaultPort : undefined; @@ -88,7 +90,7 @@ export function OfflineGateway(props: { {props.gatewayPort.isSupported && ( { describe('setUpAppGateway', () => { test.each([ { - name: 'creates tunnel for a tcp app', + name: 'creates tunnel for a single-port TCP app', app: makeApp({ endpointUri: 'tcp://localhost:3000', }), }, + { + name: 'creates tunnel for a multi-port TCP app', + app: makeApp({ + endpointUri: 'tcp://localhost', + tcpPorts: [{ port: 1234, endPort: 0 }], + }), + expectedTargetSubresourceName: '1234', + }, + { + name: 'creates tunnel for a multi-port TCP app with a preselected target port', + app: makeApp({ + endpointUri: 'tcp://localhost', + tcpPorts: [{ port: 1234, endPort: 0 }], + }), + targetPort: 1234, + }, { name: 'creates tunnel for a web app', app: makeApp({ endpointUri: 'http://localhost:3000', }), }, - ])('$name', async ({ app }) => { + ])('$name', async ({ app, targetPort, expectedTargetSubresourceName }) => { const appContext = new MockAppContext(); setTestCluster(appContext); - await setUpAppGateway(appContext, app, { origin: 'resource_table' }); + await setUpAppGateway(appContext, app, { + telemetry: { origin: 'resource_table' }, + targetPort, + }); const documents = appContext.workspacesService .getActiveWorkspaceDocumentService() .getGatewayDocuments(); @@ -147,7 +166,8 @@ describe('setUpAppGateway', () => { port: undefined, status: '', targetName: 'foo', - targetSubresourceName: undefined, + targetSubresourceName: + expectedTargetSubresourceName || targetPort?.toString() || undefined, targetUri: '/clusters/teleport-local/apps/foo', targetUser: '', title: 'foo', diff --git a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToApp.ts b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToApp.ts index 93aee047a7341..2711bae403b0d 100644 --- a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToApp.ts +++ b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/connectToApp.ts @@ -115,13 +115,21 @@ export async function connectToApp( return; } - await setUpAppGateway(ctx, target, telemetry); + await setUpAppGateway(ctx, target, { telemetry }); } export async function setUpAppGateway( ctx: IAppContext, target: App, - telemetry: { origin: DocumentOrigin } + options: { + telemetry: { origin: DocumentOrigin }; + /** + * targetPort allows the caller to preselect the target port for the gateway. Works only with + * multi-port TCP apps. If it's not specified and the app is multi-port, the first port from + * it's TCP ports is used instead. + */ + targetPort?: number; + } ) { const rootClusterUri = routing.ensureRootClusterUri(target.uri); @@ -129,16 +137,20 @@ export async function setUpAppGateway( ctx.workspacesService.getWorkspaceDocumentService(rootClusterUri); const doc = documentsService.createGatewayDocument({ targetUri: target.uri, - origin: telemetry.origin, + origin: options.telemetry.origin, targetName: routing.parseAppUri(target.uri).params.appId, targetUser: '', + targetSubresourceName: + target.tcpPorts.length > 0 + ? (options.targetPort || target.tcpPorts[0].port).toString() + : undefined, }); const connectionToReuse = ctx.connectionTracker.findConnectionByDocument(doc); if (connectionToReuse) { await ctx.connectionTracker.activateItem(connectionToReuse.id, { - origin: telemetry.origin, + origin: options.telemetry.origin, }); } else { await ctx.workspacesService.setActiveWorkspace(rootClusterUri); diff --git a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/documentsService.test.ts b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/documentsService.test.ts index b50989a4273ff..96d1f3129ea24 100644 --- a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/documentsService.test.ts +++ b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/documentsService.test.ts @@ -79,6 +79,7 @@ describe('document should be added', () => { targetUri: '/clusters/bar/dbs/quux', targetName: 'quux', targetUser: 'foo', + targetSubresourceName: undefined, origin: 'resource_table', status: '', }; @@ -155,6 +156,7 @@ test('only gateway documents should be returned', () => { targetUri: '/clusters/bar/dbs/quux', targetName: 'quux', targetUser: 'foo', + targetSubresourceName: undefined, origin: 'resource_table', status: '', }; diff --git a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/types.ts b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/types.ts index 970fb09d22ba7..e975d8e268ae7 100644 --- a/web/packages/teleterm/src/ui/services/workspacesService/documentsService/types.ts +++ b/web/packages/teleterm/src/ui/services/workspacesService/documentsService/types.ts @@ -109,7 +109,11 @@ export interface DocumentGateway extends DocumentBase { targetUri: uri.DatabaseUri | uri.AppUri; targetUser: string; targetName: string; - targetSubresourceName?: string; + /** + * targetSubresourceName contains database name for db gateways and target port for TCP app + * gateways. + */ + targetSubresourceName: string | undefined; port?: string; origin: DocumentOrigin; }