-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Implement temporary last step of Connect My Computer flow in Discover #34452
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
e6b7851
Rename applyUserTraits to reloadUser, remove return value
ravicious 433ed08
Reload user before each poll after timeout
ravicious 1fc2b8b
SetupConnect: Update agent meta before proceeding
ravicious 756b6dd
Add a stub of test connection step
ravicious 97b32bc
Add button for connecting to node
ravicious a723d37
Reload user before rendering TestConnection
ravicious e15b69b
Refetch node after refreshing session
ravicious 7a982b0
Disable back button for TestConnection step
ravicious 54d3ebb
Add extra expectation for userService.reloadUser
ravicious a1e678e
Remove unnecessary useCallback
ravicious File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,46 @@ 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: | ||
| // <current-username>`. | ||
| // | ||
| // 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); | ||
| // TODO(ravicious): Take these from the context rather than from props. | ||
| const { agentMeta, updateAgentMeta, nextStep } = props; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. non-blocker nit: discover work started by prop drilling (me), then ryan introduced context. so ideally, we should be getting these props by |
||
| const handleNextStep = () => { | ||
| 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(); | ||
| }; | ||
|
|
||
| useEffect(() => { | ||
| if (isPolling) { | ||
| const id = window.setTimeout(() => setShowHint(true), showHintTimeout); | ||
|
|
@@ -196,7 +229,7 @@ export function SetupConnect( | |
| {pollingStatus} | ||
|
|
||
| <ActionButtons | ||
| onProceed={props.nextStep} | ||
| onProceed={handleNextStep} | ||
| disableProceed={!node} | ||
| onPrev={props.prevStep} | ||
| /> | ||
|
|
@@ -225,6 +258,7 @@ export function SetupConnect( | |
| export const usePollForConnectMyComputerNode = (args: { | ||
| username: string; | ||
| clusterId: string; | ||
| reloadUser: boolean; | ||
| pingInterval: number; | ||
| }): { | ||
| node: Node | undefined; | ||
|
|
@@ -237,6 +271,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 +306,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 | ||
|
|
||
137 changes: 137 additions & 0 deletions
137
web/packages/teleport/src/Discover/ConnectMyComputer/TestConnection/TestConnection.story.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| /** | ||
| * 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 { 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 ( | ||
| <Provider> | ||
| <TestConnection {...agentStepProps} /> | ||
| </Provider> | ||
| ); | ||
| }; | ||
|
|
||
| Story.parameters = { | ||
| msw: { | ||
| handlers: [ | ||
| rest.post(cfg.api.webRenewTokenPath, (req, res, ctx) => | ||
| res(ctx.json({})) | ||
| ), | ||
| rest.get(cfg.api.nodesPath, (req, res, ctx) => | ||
| res(ctx.json({ items: [node] })) | ||
| ), | ||
| ], | ||
| }, | ||
| }; | ||
|
|
||
| export const ReloadUserProcessing = () => { | ||
| return ( | ||
| <Provider> | ||
| <TestConnection {...agentStepProps} /> | ||
| </Provider> | ||
| ); | ||
| }; | ||
|
|
||
| ReloadUserProcessing.parameters = { | ||
| msw: { | ||
| handlers: [ | ||
| rest.post(cfg.api.webRenewTokenPath, (req, res, ctx) => | ||
| res(ctx.delay('infinite')) | ||
| ), | ||
| ], | ||
| }, | ||
| }; | ||
|
|
||
| export const ReloadUserError = () => { | ||
| return ( | ||
| <Provider> | ||
| <TestConnection {...agentStepProps} /> | ||
| </Provider> | ||
| ); | ||
| }; | ||
|
|
||
| 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 ( | ||
| <MemoryRouter> | ||
| <UserContext.Provider | ||
| value={{ | ||
| preferences, | ||
| updatePreferences, | ||
| getClusterPinnedResources, | ||
| updateClusterPinnedResources, | ||
| }} | ||
| > | ||
| <ContextProvider ctx={ctx}>{children}</ContextProvider> | ||
| </UserContext.Provider> | ||
| </MemoryRouter> | ||
| ); | ||
| }; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.