Add initial JoinTokens UI #43766

avatus · 2024-07-02T20:32:38Z

This PR is part of the new join tokens feature. This will make the join tokens UI available on /web/tokens but not enable it in the navigation. You will have to manually view it. We will add it to the navigation just before the backport to the release branch. I did it this way so we can start PRing the larger chunks while we work on the "create token" forms. So obviously, the "create token" button doesn't exist on here. this PR is for viewing/editting tokens

avatus · 2024-07-02T22:27:34Z

lib/web/join_tokens_test.go

 		})
 	}
 }

+func filterContentField(tokens []ui.JoinToken) []ui.JoinToken {


I added this method to avoid adding a bunch to the tests just to get the yaml content correct

would using cmpopts.IgnoreFields work?

strideynet

Looks good from the backend perspective.

strideynet · 2024-07-03T11:04:06Z

lib/web/apiserver.go

@@ -769,6 +769,7 @@ func (h *Handler) bindDefaultEndpoints() {
 	h.GET("/webapi/auth/export", h.authExportPublic)

 	// join token handlers
+	h.POST("/webapi/token/yaml", h.WithAuth(h.upsertTokenContent))


If this is an upsert, would PUT be more accurate method?

Yeah you're right. I'll update

avatus · 2024-07-03T16:37:23Z

web/packages/teleport/src/services/api/api.ts

+    });
+  },
+
+  // TODO (avatus) add abort signal to this


added this TODO because adding the abort signal to our put method here would change a LOT of spots and I don't want it to muddy up this PR

You can make it optional

ryanclark · 2024-07-04T09:35:30Z

web/packages/teleport/src/services/joinToken/makeJoinToken.ts

+    isStatic,
+    safeName,
+    method,
+    roles: roles?.sort() || [],


Are these strings? I think we should sort by a.localeCompare(b)

ryanclark · 2024-07-04T09:36:56Z

web/packages/teleport/src/JoinTokens/JoinTokens.tsx

+          <Text bold as="span">
+            {` ${token.safeName}`}
+          </Text>
+          . This will not remove any resource that used this join token to join


Should this be "resources"?

I think we should reword "join token to join" too

Hmm, I think this copy should change quite a bit. I'm not sure it's 100% accurate anymore because it also depends on token type (I believe this is only true with join method type token)

I'll revisit this entire blurb

ryanclark · 2024-07-04T09:38:18Z

web/packages/teleport/src/JoinTokens/JoinTokens.tsx

+      <Cell align="right">
+        <HoverTooltip
+          justifyContentProps={{ justifyContent: 'end' }}
+          tipContent="You cannot configure or delete static tokens via the web UI. Static tokens should be removed from your Teleport configuration file."


I think we should change this to say "You can only configure or delete static tokens through tctl`, that way we convey they same thing but give the user the info they need to continue on

Static tokens are in the actual teleport yaml config and not via tctl. They can only be removed by removing from the file. Also, this was the copy suggested during ux meeting

Ah, I had no idea where they were managed - fair enough

ryanclark · 2024-07-04T09:39:31Z

web/packages/teleport/src/JoinTokens/JoinTokens.tsx

+      open={true}
+    >
+      <DialogHeader>
+        <DialogTitle>Delete Join Token?</DialogTitle>


It's a common UI trend to get the user to type "delete" or "delete me" to confirm they want to delete something. Should we use that here? cc @roraback

Chatted with kenny and we decided not to implement this. Great idea tho, thank you

kimlisa

just some minor stuff but otherwise lgtm.

another thing to think about is, if mfa authn failed, it shows a blank page, can we somehow propagate the error and render an error?

kimlisa · 2024-07-09T17:24:55Z

lib/web/join_tokens_test.go

 		})
 	}
 }

+func filterContentField(tokens []ui.JoinToken) []ui.JoinToken {


would using cmpopts.IgnoreFields work?

kimlisa · 2024-07-09T17:47:39Z

web/packages/teleport/src/JoinTokens/JoinTokens.story.tsx

+
+initialize();
+
+export const Loaded = () => (


add a failed story?

kimlisa · 2024-07-09T18:00:58Z

web/packages/teleport/src/JoinTokens/JoinTokens.tsx

+        {deleteTokenAttempt.status === 'error' && (
+          <Alert kind="danger">{deleteTokenAttempt.statusText}</Alert>
+        )}


what about the other failure modes? like fetching and upserting

Thnks, i forgot to add alerts for those!

I added the alert for fetching, but the error for upserting is shown in the yaml editor. i will update this when we add the forms

This PR is part of the new join tokens feature. This will make the join tokens UI available on /web/tokens but not enable it in the navigation. You will have to manually view it. We will add it to the navigation just before the backport to the release branch. I did it this way so we can start PRing the larger chunks while we work on the "create token" forms. So obviously, the "create token" button doesn't exist on here. this is for viewing/editting for now.

avatus added the no-changelog Indicates that a PR does not require a changelog entry label Jul 2, 2024

avatus requested review from ryanclark, kimlisa and rudream July 2, 2024 20:32

github-actions bot requested review from gzdunek and ravicious July 2, 2024 20:33

github-actions bot added size/lg ui labels Jul 2, 2024

avatus removed request for ravicious and gzdunek July 2, 2024 21:03

avatus commented Jul 2, 2024

View reviewed changes

strideynet approved these changes Jul 3, 2024

View reviewed changes

avatus commented Jul 3, 2024

View reviewed changes

avatus force-pushed the avatus/join branch from 8d07b25 to 16376aa Compare July 3, 2024 18:34

ryanclark reviewed Jul 4, 2024

View reviewed changes

avatus requested a review from ryanclark July 8, 2024 17:05

ryanclark approved these changes Jul 9, 2024

View reviewed changes

public-teleport-github-review-bot bot removed request for kimlisa and rudream July 9, 2024 11:44

avatus force-pushed the avatus/join branch 2 times, most recently from 8c90fde to 27c2171 Compare July 9, 2024 17:31

kimlisa approved these changes Jul 9, 2024

View reviewed changes

avatus force-pushed the avatus/join branch from a4a363f to 5e8097d Compare July 9, 2024 22:57

avatus force-pushed the avatus/join branch from 5e8097d to 8b4fc4c Compare July 9, 2024 22:58

avatus added this pull request to the merge queue Jul 10, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 10, 2024

avatus added this pull request to the merge queue Jul 10, 2024

Merged via the queue into master with commit d47bc96 Jul 10, 2024
42 checks passed

avatus deleted the avatus/join branch July 10, 2024 04:42

avatus mentioned this pull request Jul 26, 2024

[16] Join Tokens UI #44701

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial JoinTokens UI #43766

Add initial JoinTokens UI #43766

avatus commented Jul 2, 2024 •

edited

Loading

avatus Jul 2, 2024

kimlisa Jul 9, 2024

strideynet left a comment

strideynet Jul 3, 2024

avatus Jul 3, 2024

avatus Jul 3, 2024

ryanclark Jul 4, 2024

ryanclark Jul 4, 2024

ryanclark Jul 4, 2024

ryanclark Jul 4, 2024

avatus Jul 4, 2024

ryanclark Jul 4, 2024

avatus Jul 4, 2024 •

edited

Loading

ryanclark Jul 4, 2024

ryanclark Jul 4, 2024

avatus Jul 8, 2024

kimlisa left a comment

kimlisa Jul 9, 2024

kimlisa Jul 9, 2024

kimlisa Jul 9, 2024

avatus Jul 9, 2024

avatus Jul 9, 2024


		initialize();

		export const Loaded = () => (

Add initial JoinTokens UI #43766

Add initial JoinTokens UI #43766

Conversation

avatus commented Jul 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

strideynet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avatus Jul 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kimlisa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avatus commented Jul 2, 2024 •

edited

Loading

avatus Jul 4, 2024 •

edited

Loading