From e6b7851667550189e752f270fe4e4dfb00f6d60a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Thu, 9 Nov 2023 19:11:03 +0100 Subject: [PATCH 01/10] Rename applyUserTraits to reloadUser, remove return value --- .../Discover/Shared/SetupAccess/useUserTraits.test.tsx | 8 ++++---- .../src/Discover/Shared/SetupAccess/useUserTraits.ts | 2 +- web/packages/teleport/src/services/user/user.ts | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/web/packages/teleport/src/Discover/Shared/SetupAccess/useUserTraits.test.tsx b/web/packages/teleport/src/Discover/Shared/SetupAccess/useUserTraits.test.tsx index 6d17bf7900272..733d009c437f9 100644 --- a/web/packages/teleport/src/Discover/Shared/SetupAccess/useUserTraits.test.tsx +++ b/web/packages/teleport/src/Discover/Shared/SetupAccess/useUserTraits.test.tsx @@ -40,7 +40,7 @@ describe('onProceed correctly deduplicates, removes static traits, updates meta, const ctx = createTeleportContext(); jest.spyOn(ctx.userService, 'fetchUser').mockResolvedValue(getMockUser()); jest.spyOn(ctx.userService, 'updateUser').mockResolvedValue(null); - jest.spyOn(ctx.userService, 'applyUserTraits').mockResolvedValue(null); + jest.spyOn(ctx.userService, 'reloadUser').mockResolvedValue(null); jest .spyOn(userEventService, 'captureDiscoverEvent') .mockResolvedValue(null as never); // return value does not matter but required by ts @@ -120,7 +120,7 @@ describe('onProceed correctly deduplicates, removes static traits, updates meta, }); await waitFor(() => { - expect(ctx.userService.applyUserTraits).toHaveBeenCalledTimes(1); + expect(ctx.userService.reloadUser).toHaveBeenCalledTimes(1); }); // Test that we are updating the user with the correct traits. @@ -199,7 +199,7 @@ describe('onProceed correctly deduplicates, removes static traits, updates meta, }); await waitFor(() => { - expect(ctx.userService.applyUserTraits).toHaveBeenCalledTimes(1); + expect(ctx.userService.reloadUser).toHaveBeenCalledTimes(1); }); // Test that we are updating the user with the correct traits. @@ -270,7 +270,7 @@ describe('onProceed correctly deduplicates, removes static traits, updates meta, }); await waitFor(() => { - expect(ctx.userService.applyUserTraits).toHaveBeenCalledTimes(1); + expect(ctx.userService.reloadUser).toHaveBeenCalledTimes(1); }); // Test that we are updating the user with the correct traits. diff --git a/web/packages/teleport/src/Discover/Shared/SetupAccess/useUserTraits.ts b/web/packages/teleport/src/Discover/Shared/SetupAccess/useUserTraits.ts index 523f606a8936d..81702e8e03d15 100644 --- a/web/packages/teleport/src/Discover/Shared/SetupAccess/useUserTraits.ts +++ b/web/packages/teleport/src/Discover/Shared/SetupAccess/useUserTraits.ts @@ -248,7 +248,7 @@ export function useUserTraits(props: AgentStepProps) { throw error; }); - await ctx.userService.applyUserTraits().catch((error: Error) => { + await ctx.userService.reloadUser().catch((error: Error) => { emitErrorEvent(`error applying new user traits: ${error.message}`); throw error; }); diff --git a/web/packages/teleport/src/services/user/user.ts b/web/packages/teleport/src/services/user/user.ts index c3e9fad0c5065..30c32db8a3b1c 100644 --- a/web/packages/teleport/src/services/user/user.ts +++ b/web/packages/teleport/src/services/user/user.ts @@ -68,8 +68,8 @@ const service = { return api.delete(cfg.getUserWithUsernameUrl(name)); }, - applyUserTraits() { - return session.renewSession({ reloadUser: true }); + async reloadUser() { + await session.renewSession({ reloadUser: true }); }, checkUserHasAccessToRegisteredResource(): Promise { From 433ed0888e118f2cecad5326bc2f5b1d4e3c0c96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Fri, 10 Nov 2023 10:39:47 +0100 Subject: [PATCH 02/10] Reload user before each poll after timeout --- .../SetupConnect/SetupConnect.story.tsx | 7 +- .../SetupConnect/SetupConnect.test.ts | 65 ++++++++++++++++++- .../SetupConnect/SetupConnect.tsx | 32 ++++++++- .../teleport/src/services/user/user.ts | 4 +- .../src/services/websession/websession.ts | 8 +-- 5 files changed, 103 insertions(+), 13 deletions(-) diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.story.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.story.tsx index 8a9eab3dd228a..d177545fe3415 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.story.tsx +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.story.tsx @@ -120,7 +120,12 @@ export const HintTimeout = () => ( HintTimeout.parameters = { msw: { - handlers: [noNodesHandler], + handlers: [ + noNodesHandler, + rest.post(cfg.api.webRenewTokenPath, (req, res, ctx) => + res(ctx.json({})) + ), + ], }, }; diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.test.ts b/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.test.ts index c7184774d2851..785e7bddb9d4e 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.test.ts +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.test.ts @@ -17,13 +17,12 @@ import { renderHook } from '@testing-library/react-hooks'; import * as useTeleport from 'teleport/useTeleport'; -import NodeService from 'teleport/services/nodes/nodes'; +import NodeService, { Node } from 'teleport/services/nodes'; +import UserService from 'teleport/services/user'; import TeleportContext from 'teleport/teleportContext'; import { nodes } from 'teleport/Nodes/fixtures'; -import { Node } from 'teleport/services/nodes'; - import { usePollForConnectMyComputerNode } from './SetupConnect'; beforeEach(() => { @@ -66,6 +65,7 @@ describe('usePollForConnectMyComputerNode', () => { username: 'alice', clusterId: 'foo', pingInterval: 1, + reloadUser: false, }) ); @@ -78,4 +78,63 @@ describe('usePollForConnectMyComputerNode', () => { expect(result.current.node).toEqual(expectedNode); expect(result.current.isPolling).toBe(false); }); + + it('reloads user before each poll if reloadUser is true', async () => { + const expectedNode = nodes[0]; + let hasReloadedUser = false; + + const nodeService = { + fetchNodes: jest.fn(), + } as Partial as NodeService; + + jest.mocked(nodeService).fetchNodes.mockImplementation(async () => { + if (hasReloadedUser) { + return { agents: [expectedNode] }; + } else { + return { agents: [] }; + } + }); + + const userService = { + reloadUser: jest.fn(), + } as Partial as typeof UserService; + + jest.mocked(userService).reloadUser.mockImplementation(async () => { + hasReloadedUser = true; + }); + + jest + .spyOn(useTeleport, 'default') + .mockReturnValue({ nodeService, userService } as TeleportContext); + + const { result, rerender, waitFor, waitForValueToChange } = renderHook( + usePollForConnectMyComputerNode, + { + initialProps: { + reloadUser: false, + username: 'alice', + clusterId: 'foo', + pingInterval: 1, + }, + } + ); + expect(result.error).toBeUndefined(); + await waitFor(() => { + expect(nodeService.fetchNodes).toHaveBeenCalled(); + }); + + rerender({ + reloadUser: true, + username: 'alice', + clusterId: 'foo', + pingInterval: 1, + }); + expect(result.error).toBeUndefined(); + + await waitForValueToChange(() => result.current.node, { interval: 3 }); + expect(userService.reloadUser).toHaveBeenCalled(); + + expect(result.current.node).toEqual(expectedNode); + expect(result.current.isPolling).toBe(false); + }); }); diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.tsx index 01c7882c8053b..359b1165f6bc4 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.tsx +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.tsx @@ -53,6 +53,7 @@ export function SetupConnect( ) { const pingInterval = props.pingInterval || 1000 * 3; // 3 seconds const showHintTimeout = props.showHintTimeout || 1000 * 60 * 5; // 5 minutes + const ctx = useTeleport(); const clusterId = ctx.storeUser.getClusterId(); const { cluster, username } = ctx.storeUser.state; @@ -63,14 +64,28 @@ export function SetupConnect( username, path: Path.ConnectMyComputer, }); + const [showHint, setShowHint] = useState(false); const { node, isPolling } = usePollForConnectMyComputerNode({ username, clusterId, - pingInterval, + // If reloadUser is set to true, the polling callback takes longer to finish so let's increase + // the polling interval as well. + pingInterval: showHint ? pingInterval * 2 : pingInterval, + // Completing the Connect My Computer setup in Connect causes the user to gain a new role. That + // role grants access to nodes labeled with `teleport.dev/connect-my-computer/owner: + // `. + // + // In certain cases, that role might be the only role which grants the user the visibility of + // the Connect My Computer node. For example, if the user doesn't have a role like the built-in + // access which gives blanket access to all nodes, the user won't be able to see the node until + // they have the Connect My Computer role in their cert. + // + // As such, if we don't reload the cert during polling, it might never see the node. So let's + // flip it to true after a timeout. + reloadUser: showHint, }); - const [showHint, setShowHint] = useState(false); useEffect(() => { if (isPolling) { const id = window.setTimeout(() => setShowHint(true), showHintTimeout); @@ -225,6 +240,7 @@ export function SetupConnect( export const usePollForConnectMyComputerNode = (args: { username: string; clusterId: string; + reloadUser: boolean; pingInterval: number; }): { node: Node | undefined; @@ -237,6 +253,10 @@ export const usePollForConnectMyComputerNode = (args: { const node = usePoll( useCallback( async signal => { + if (args.reloadUser) { + await ctx.userService.reloadUser(signal); + } + const request = { query: `labels["${constants.ConnectMyComputerNodeOwnerLabel}"] == "${args.username}"`, // An arbitrary limit where we bank on the fact that no one is going to have 50 Connect My @@ -268,7 +288,13 @@ export const usePollForConnectMyComputerNode = (args: { return node; } }, - [ctx.nodeService, args.clusterId, args.username] + [ + ctx.nodeService, + ctx.userService, + args.clusterId, + args.username, + args.reloadUser, + ] ), isPolling, args.pingInterval diff --git a/web/packages/teleport/src/services/user/user.ts b/web/packages/teleport/src/services/user/user.ts index 30c32db8a3b1c..b76a664946380 100644 --- a/web/packages/teleport/src/services/user/user.ts +++ b/web/packages/teleport/src/services/user/user.ts @@ -68,8 +68,8 @@ const service = { return api.delete(cfg.getUserWithUsernameUrl(name)); }, - async reloadUser() { - await session.renewSession({ reloadUser: true }); + async reloadUser(signal?: AbortSignal) { + await session.renewSession({ reloadUser: true }, signal); }, checkUserHasAccessToRegisteredResource(): Promise { diff --git a/web/packages/teleport/src/services/websession/websession.ts b/web/packages/teleport/src/services/websession/websession.ts index 569033663ef7d..965276ce6603f 100644 --- a/web/packages/teleport/src/services/websession/websession.ts +++ b/web/packages/teleport/src/services/websession/websession.ts @@ -71,8 +71,8 @@ const session = { // renewSession renews session and returns the // absolute time the new session expires. - renewSession(req: RenewSessionRequest): Promise { - return this._renewToken(req).then(token => token.sessionExpires); + renewSession(req: RenewSessionRequest, signal?: AbortSignal): Promise { + return this._renewToken(req, signal).then(token => token.sessionExpires); }, /** @@ -133,10 +133,10 @@ const session = { return this._timeLeft() < RENEW_TOKEN_TIME; }, - _renewToken(req: RenewSessionRequest = {}) { + _renewToken(req: RenewSessionRequest = {}, signal?: AbortSignal) { this._setAndBroadcastIsRenewing(true); return api - .post(cfg.getRenewTokenUrl(), req) + .post(cfg.getRenewTokenUrl(), req, signal) .then(json => { const token = makeBearerToken(json); localStorage.setBearerToken(token); From 1fc2b8b0e299596120b62c58f8c749351d67fd3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Fri, 10 Nov 2023 11:48:00 +0100 Subject: [PATCH 03/10] SetupConnect: Update agent meta before proceeding --- .../SetupConnect/SetupConnect.story.tsx | 1 + .../SetupConnect/SetupConnect.tsx | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.story.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.story.tsx index d177545fe3415..928242fc741d8 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.story.tsx +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.story.tsx @@ -37,6 +37,7 @@ const oneDay = 1000 * 60 * 60 * 24; const setupConnectProps = { prevStep: () => {}, nextStep: () => {}, + updateAgentMeta: () => {}, // Set high default intervals and timeouts so that stories don't poll for no reason. pingInterval: oneDay, showHintTimeout: oneDay, diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.tsx index 359b1165f6bc4..2a042ef3d86ef 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.tsx +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.tsx @@ -86,6 +86,23 @@ export function SetupConnect( reloadUser: showHint, }); + const { agentMeta, updateAgentMeta, nextStep } = props; + const handleNextStep = useCallback(() => { + if (!node) { + return; + } + + updateAgentMeta({ + ...agentMeta, + // Node is an oddity in that the hostname is the more + // user identifiable resource name and what user expects + // as the resource name. + resourceName: node.hostname, + node, + }); + nextStep(); + }, [node, updateAgentMeta, agentMeta, nextStep]); + useEffect(() => { if (isPolling) { const id = window.setTimeout(() => setShowHint(true), showHintTimeout); @@ -211,7 +228,7 @@ export function SetupConnect( {pollingStatus} From 756b6dd01c8a5d3985b0c979e9c2dbcc4b0abdf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Fri, 10 Nov 2023 11:48:47 +0100 Subject: [PATCH 04/10] Add a stub of test connection step --- .../TestConnection/TestConnection.story.tsx | 37 +++++++++++++ .../TestConnection/TestConnection.tsx | 52 +++++++++++++++++++ .../ConnectMyComputer/TestConnection/index.ts | 17 ++++++ .../ConnectMyComputer/{index.tsx => index.ts} | 18 ++++--- 4 files changed, 118 insertions(+), 6 deletions(-) create mode 100644 web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx create mode 100644 web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx create mode 100644 web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/index.ts rename web/packages/teleport/src/Discover/ConnectMyComputer/{index.tsx => index.ts} (73%) diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx new file mode 100644 index 0000000000000..c0cdfab3d3e83 --- /dev/null +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx @@ -0,0 +1,37 @@ +/** + * 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. + */ + +import React from 'react'; + +import { nodes } from 'teleport/Nodes/fixtures'; + +import { TestConnection } from './TestConnection'; + +export default { + title: 'Teleport/Discover/ConnectMyComputer/TestConnection', +}; + +const node = nodes[0]; + +export const Story = () => { + return ( + {}} + nextStep={() => {}} + agentMeta={{ resourceName: node.hostname, node, agentMatcherLabels: [] }} + /> + ); +}; diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx new file mode 100644 index 0000000000000..a4b6deae9ff1c --- /dev/null +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx @@ -0,0 +1,52 @@ +/** + * 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. + */ + +import React from 'react'; + +import { ButtonSecondary, Flex, Text } from 'design'; + +import { ActionButtons, StyledBox, Header } from 'teleport/Discover/Shared'; + +import { NodeMeta } from '../../useDiscover'; + +import type { AgentStepProps } from '../../types'; + +export const TestConnection = (props: AgentStepProps) => { + const meta = props.agentMeta as NodeMeta; + + return ( + +
+
Start a Session
+
+ + + Step 1: Connect to Your Computer + + Optionally verify that you can connect to “{meta.resourceName} + ” by starting a session. + + Start a Session + + + + + ); +}; diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/index.ts b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/index.ts new file mode 100644 index 0000000000000..6852c35b30fc0 --- /dev/null +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/index.ts @@ -0,0 +1,17 @@ +/** + * 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. + */ + +export { TestConnection } from './TestConnection'; diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/index.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/index.ts similarity index 73% rename from web/packages/teleport/src/Discover/ConnectMyComputer/index.tsx rename to web/packages/teleport/src/Discover/ConnectMyComputer/index.ts index 74c79ad382ff9..2c043f7bd6bd9 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/index.tsx +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/index.ts @@ -14,15 +14,14 @@ * limitations under the License. */ -import React from 'react'; - import { ResourceViewConfig } from 'teleport/Discover/flow'; -import { ResourceKind } from 'teleport/Discover/Shared'; +import { Finished, ResourceKind } from 'teleport/Discover/Shared'; import { DiscoverEvent } from 'teleport/services/userEvent'; import { ResourceSpec } from '../SelectResource'; import { SetupConnect } from './SetupConnect'; +import { TestConnection } from './TestConnection'; export const ConnectMyComputerResource: ResourceViewConfig = { kind: ResourceKind.ConnectMyComputer, @@ -33,11 +32,18 @@ export const ConnectMyComputerResource: ResourceViewConfig = { eventName: DiscoverEvent.DeployService, }, { - title: 'Test Connection', - component: () =>
WIP
, + // TODO(ravicious): Rename this to "Test Connection" after implementing the connection test. + title: 'Start a Session', + component: TestConnection, eventName: DiscoverEvent.TestConnection, // TODO(ravicious): Manually emit success event on test connection success. - // manuallyEmitSuccessEvent: true, + manuallyEmitSuccessEvent: true, + }, + { + title: 'Finished', + component: Finished, + hide: true, + eventName: DiscoverEvent.Completed, }, ], }; From 97b32bcbcdd3263bdf36590c585325b811b91919 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Fri, 10 Nov 2023 12:35:30 +0100 Subject: [PATCH 05/10] Add button for connecting to node --- .../TestConnection/TestConnection.story.tsx | 17 ++++++++++++----- .../TestConnection/TestConnection.tsx | 5 +++-- .../UnifiedResources/ResourceActionButton.tsx | 10 ++++++++-- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx index c0cdfab3d3e83..e9b2400726911 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx @@ -15,6 +15,7 @@ */ import React from 'react'; +import { MemoryRouter } from 'react-router'; import { nodes } from 'teleport/Nodes/fixtures'; @@ -28,10 +29,16 @@ const node = nodes[0]; export const Story = () => { return ( - {}} - nextStep={() => {}} - agentMeta={{ resourceName: node.hostname, node, agentMatcherLabels: [] }} - /> + + {}} + nextStep={() => {}} + agentMeta={{ + resourceName: node.hostname, + node, + agentMatcherLabels: [], + }} + /> + ); }; diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx index a4b6deae9ff1c..94c6e1eea9960 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx @@ -16,9 +16,10 @@ import React from 'react'; -import { ButtonSecondary, Flex, Text } from 'design'; +import { Flex, Text } from 'design'; import { ActionButtons, StyledBox, Header } from 'teleport/Discover/Shared'; +import { NodeConnect } from 'teleport/UnifiedResources/ResourceActionButton'; import { NodeMeta } from '../../useDiscover'; @@ -39,7 +40,7 @@ export const TestConnection = (props: AgentStepProps) => { Optionally verify that you can connect to “{meta.resourceName} ” by starting a session. - Start a Session + { } }; -const NodeConnect = ({ node }: { node: Node }) => { +export const NodeConnect = ({ + node, + textTransform, +}: { + node: Node; + textTransform?: string; +}) => { const { clusterId } = useStickyClusterId(); const startSshSession = (login: string, serverId: string) => { const url = cfg.getSshConnectRoute({ @@ -78,7 +84,7 @@ const NodeConnect = ({ node }: { node: Node }) => { return ( Date: Fri, 10 Nov 2023 17:07:44 +0100 Subject: [PATCH 06/10] Reload user before rendering TestConnection --- .../TestConnection/TestConnection.story.tsx | 106 ++++++++++++++++-- .../TestConnection/TestConnection.tsx | 63 ++++++++++- 2 files changed, 156 insertions(+), 13 deletions(-) diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx index e9b2400726911..0b3e0784c5d3c 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx @@ -16,29 +16,119 @@ import React from 'react'; import { MemoryRouter } from 'react-router'; +import { initialize, mswLoader } from 'msw-storybook-addon'; +import { rest } from 'msw'; import { nodes } from 'teleport/Nodes/fixtures'; +import { ContextProvider } from 'teleport'; +import cfg from 'teleport/config'; +import { UserContext } from 'teleport/User/UserContext'; +import { createTeleportContext } from 'teleport/mocks/contexts'; +import { makeDefaultUserPreferences } from 'teleport/services/userPreferences/userPreferences'; import { TestConnection } from './TestConnection'; export default { title: 'Teleport/Discover/ConnectMyComputer/TestConnection', + loaders: [mswLoader], }; +initialize(); + const node = nodes[0]; +const agentStepProps = { + prevStep: () => {}, + nextStep: () => {}, + agentMeta: { resourceName: node.hostname, node, agentMatcherLabels: [] }, +}; + export const Story = () => { + return ( + + + + ); +}; + +Story.parameters = { + msw: { + handlers: [ + rest.post(cfg.api.webRenewTokenPath, (req, res, ctx) => + res(ctx.json({})) + ), + ], + }, +}; + +export const ReloadUserProcessing = () => { + return ( + + + + ); +}; + +ReloadUserProcessing.parameters = { + msw: { + handlers: [ + rest.post(cfg.api.webRenewTokenPath, (req, res, ctx) => + res(ctx.delay('infinite')) + ), + ], + }, +}; + +export const ReloadUserError = () => { + return ( + + + + ); +}; + +ReloadUserError.parameters = { + msw: { + handlers: [ + // The first handler returns an error immediately. Subsequent requests return after a delay so + // that we can show a spinner after clicking on "Retry". + rest.post(cfg.api.webRenewTokenPath, (req, res, ctx) => + res.once( + ctx.status(500), + ctx.json({ message: 'Could not renew session' }) + ) + ), + rest.post(cfg.api.webRenewTokenPath, (req, res, ctx) => + res( + ctx.delay(1000), + ctx.status(500), + ctx.json({ message: 'Could not renew session' }) + ) + ), + ], + }, +}; + +const Provider = ({ children }) => { + const ctx = createTeleportContext(); + + const preferences = makeDefaultUserPreferences(); + const updatePreferences = () => Promise.resolve(); + const getClusterPinnedResources = () => Promise.resolve([]); + const updateClusterPinnedResources = () => Promise.resolve(); + return ( - {}} - nextStep={() => {}} - agentMeta={{ - resourceName: node.hostname, - node, - agentMatcherLabels: [], + + > + {children} + ); }; diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx index 94c6e1eea9960..f6e3b9177dcac 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx @@ -14,11 +14,19 @@ * limitations under the License. */ -import React from 'react'; +import React, { useEffect, useCallback, useRef } from 'react'; -import { Flex, Text } from 'design'; +import { ButtonPrimary, Flex, Indicator, Text } from 'design'; +import * as Icons from 'design/Icon'; +import { useAsync } from 'shared/hooks/useAsync'; -import { ActionButtons, StyledBox, Header } from 'teleport/Discover/Shared'; +import useTeleport from 'teleport/useTeleport'; +import { + ActionButtons, + StyledBox, + Header, + TextIcon, +} from 'teleport/Discover/Shared'; import { NodeConnect } from 'teleport/UnifiedResources/ResourceActionButton'; import { NodeMeta } from '../../useDiscover'; @@ -26,6 +34,28 @@ import { NodeMeta } from '../../useDiscover'; import type { AgentStepProps } from '../../types'; export const TestConnection = (props: AgentStepProps) => { + const { userService } = useTeleport(); + const abortController = useRef(); + const [reloadUserAttempt, reloadUser] = useAsync( + useCallback( + (signal: AbortSignal) => userService.reloadUser(signal), + [userService] + ) + ); + + // When the user sets up Connect My Computer in Teleport Connect, a new role gets added to the + // user. Because of that, we need to reload the current session so that the user is able to + // connect to the new node, without having to log in to the cluster again. + useEffect(() => { + abortController.current = new AbortController(); + + reloadUser(abortController.current.signal); + + return () => { + abortController.current.abort(); + }; + }, []); + const meta = props.agentMeta as NodeMeta; return ( @@ -37,14 +67,37 @@ export const TestConnection = (props: AgentStepProps) => { Step 1: Connect to Your Computer - Optionally verify that you can connect to “{meta.resourceName} + Optionally verify that you can connect to “ + {meta.resourceName} ” by starting a session. - + {reloadUserAttempt.status === '' || + (reloadUserAttempt.status === 'processing' && )} + + {reloadUserAttempt.status === 'error' && ( + <> + + + Encountered Error: {reloadUserAttempt.statusText} + + + reloadUser(abortController.current.signal)} + > + Retry + + + )} + + {reloadUserAttempt.status === 'success' && ( + + )} From e15b69b151d13721c98e18f405612db1061c01a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Fri, 10 Nov 2023 17:47:56 +0100 Subject: [PATCH 07/10] Refetch node after refreshing session This makes sure that the list of logins in the node is up to date. --- .../TestConnection/TestConnection.story.tsx | 3 + .../TestConnection/TestConnection.tsx | 62 +++++++++++++------ 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx index 0b3e0784c5d3c..c3d0adfe0fd7e 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx @@ -57,6 +57,9 @@ Story.parameters = { rest.post(cfg.api.webRenewTokenPath, (req, res, ctx) => res(ctx.json({})) ), + rest.get(cfg.api.nodesPath, (req, res, ctx) => + res(ctx.json({ items: [node] })) + ), ], }, }; diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx index f6e3b9177dcac..1c7988152f6da 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx @@ -34,30 +34,53 @@ import { NodeMeta } from '../../useDiscover'; import type { AgentStepProps } from '../../types'; export const TestConnection = (props: AgentStepProps) => { - const { userService } = useTeleport(); + const { userService, nodeService, storeUser } = useTeleport(); + const clusterId = storeUser.getClusterId(); + const meta = props.agentMeta as NodeMeta; + const abortController = useRef(); - const [reloadUserAttempt, reloadUser] = useAsync( + // When the user sets up Connect My Computer in Teleport Connect, a new role gets added to the + // user. Because of that, we need to reload the current session so that the user is able to + // connect to the new node, without having to log in to the cluster again. + // + // We also need to refetch the node so that it includes any new logins. + const [refetchNodeAttempt, refetchNode] = useAsync( useCallback( - (signal: AbortSignal) => userService.reloadUser(signal), - [userService] + async (signal: AbortSignal) => { + await userService.reloadUser(signal); + + const response = await nodeService.fetchNodes( + clusterId, + { search: meta.node.id, limit: 1 }, + signal + ); + + if (response.agents.length === 0) { + throw new Error('Could not find the Connect My Computer node'); + } + + if (response.agents.length > 1) { + throw new Error( + 'Found multiple nodes matching the ID of the Connect My Computer node' + ); + } + + return response.agents[0]; + }, + [userService, nodeService, clusterId, meta.node.id] ) ); - // When the user sets up Connect My Computer in Teleport Connect, a new role gets added to the - // user. Because of that, we need to reload the current session so that the user is able to - // connect to the new node, without having to log in to the cluster again. useEffect(() => { abortController.current = new AbortController(); - reloadUser(abortController.current.signal); + refetchNode(abortController.current.signal); return () => { abortController.current.abort(); }; }, []); - const meta = props.agentMeta as NodeMeta; - return (
@@ -71,33 +94,36 @@ export const TestConnection = (props: AgentStepProps) => { {meta.resourceName} ” by starting a session. - {reloadUserAttempt.status === '' || - (reloadUserAttempt.status === 'processing' && )} + {refetchNodeAttempt.status === '' || + (refetchNodeAttempt.status === 'processing' && )} - {reloadUserAttempt.status === 'error' && ( + {refetchNodeAttempt.status === 'error' && ( <> - Encountered Error: {reloadUserAttempt.statusText} + Encountered Error: {refetchNodeAttempt.statusText} reloadUser(abortController.current.signal)} + onClick={() => refetchNode(abortController.current.signal)} > Retry )} - {reloadUserAttempt.status === 'success' && ( - + {refetchNodeAttempt.status === 'success' && ( + )} From 7a982b0e6572808064da375603fb2c89187ca45e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 13 Nov 2023 11:52:58 +0100 Subject: [PATCH 08/10] Disable back button for TestConnection step --- .../ConnectMyComputer/TestConnection/TestConnection.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx index 1c7988152f6da..5be03c6df6f38 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.tsx @@ -125,7 +125,9 @@ export const TestConnection = (props: AgentStepProps) => { onProceed={props.nextStep} disableProceed={refetchNodeAttempt.status !== 'success'} lastStep={true} - onPrev={props.prevStep} + // onPrev is not passed on purpose to disable the back button. The flow would go back to + // polling which wouldn't make sense as the user has already connected their computer so the + // step would poll forever, unless the user removed the agent and configured it again. /> ); From 54d3ebb42358a22518ad260df341bdfab85d94b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 13 Nov 2023 12:04:49 +0100 Subject: [PATCH 09/10] Add extra expectation for userService.reloadUser --- .../Discover/ConnectMyComputer/SetupConnect/SetupConnect.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.test.ts b/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.test.ts index 785e7bddb9d4e..18e9783624bdd 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.test.ts +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.test.ts @@ -122,6 +122,7 @@ describe('usePollForConnectMyComputerNode', () => { await waitFor(() => { expect(nodeService.fetchNodes).toHaveBeenCalled(); }); + expect(userService.reloadUser).not.toHaveBeenCalled(); rerender({ reloadUser: true, From a1e678ecf791ca20ec839bdc06b8d6f051a62a07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 13 Nov 2023 12:05:48 +0100 Subject: [PATCH 10/10] Remove unnecessary useCallback --- .../Discover/ConnectMyComputer/SetupConnect/SetupConnect.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.tsx b/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.tsx index 2a042ef3d86ef..83f45c9ecf770 100644 --- a/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.tsx +++ b/web/packages/teleport/src/Discover/ConnectMyComputer/SetupConnect/SetupConnect.tsx @@ -86,8 +86,9 @@ export function SetupConnect( reloadUser: showHint, }); + // TODO(ravicious): Take these from the context rather than from props. const { agentMeta, updateAgentMeta, nextStep } = props; - const handleNextStep = useCallback(() => { + const handleNextStep = () => { if (!node) { return; } @@ -101,7 +102,7 @@ export function SetupConnect( node, }); nextStep(); - }, [node, updateAgentMeta, agentMeta, nextStep]); + }; useEffect(() => { if (isPolling) {