feat(join-tokens): [44794] Add Join Token form for GitHub by nicholasmarais1158 · Pull Request #54308 · gravitational/teleport

nicholasmarais1158 · 2025-04-25T11:15:23Z

Overview

It's currently not possible to create a new Join Token for GitHub via the Join Tokens page (/web/tokens) - it can however, be done by creating a Bot linked to GitHub. This change adds a form for creating and editing GitHub join tokens.

Issue: #44794

Depends on: #54374

changelog: Create and edit GitHub join tokens from the Join Tokens page

Changes

Add form for create/edit
Handle unsupported fields during edit - this prevents any custom/unsupported config being silently overwritten when editing a token.
Fix spacing on side panel - remove double spacing on right of forms
Fix/silence a few lint issues

Demo

Screen.Recording.2025-04-28.at.17.49.23.mov

ryanclark · 2025-04-25T13:01:21Z

+export const checkIAMYAMLData = (data: unknown) => {
+  const keys = collectKeys(data);
+  return (
+    !keys || new Set(keys).isSubsetOf(new Set(['.aws_account', '.aws_arn']))


I think here and line 267 should match how you're doing it elsewhere, i.e. the static set is assigned to a variable - makes it more readable

return !keys || new Set(keys).isSubsetOf(supportedFields)

Done in c2d0292.

ryanclark · 2025-04-25T13:02:49Z

+      <Text fontWeight={700} mt={5}>
+        GHES configuration
+      </Text>
+
+      <Text fontWeight="regular" mb={2}>
+        Additional settings for configuring GitHub Enterprise Server.
+      </Text>
+
+      <FieldInput
+        label="Server host"
+        placeholder="github.example.com"
+        value={github.server_host}
+        onChange={e =>
+          onUpdateState({
+            ...tokenState,
+            github: {
+              ...tokenState.github,
+              server_host: e.target.value,
+            },
+          })
+        }
+        readonly={readonly}
+      />
+
+      <FieldInput
+        label="Slug"
+        placeholder="octo-enterprise"
+        value={github.enterprise_slug}
+        onChange={e =>
+          onUpdateState({
+            ...tokenState,
+            github: {
+              ...tokenState.github,
+              enterprise_slug: e.target.value,
+            },
+          })
+        }
+        readonly={readonly}
+      />
+
+      <FieldInput
+        label="Static JWKS"
+        placeholder='{"keys":[--snip--]}'
+        toolTipContent="JSON Web Key Set used to verify the token issued by GitHub Actions"
+        value={github.static_jwks}
+        onChange={e =>
+          onUpdateState({
+            ...tokenState,
+            github: {
+              ...tokenState.github,
+              static_jwks: e.target.value,
+            },
+          })
+        }
+        readonly={readonly}
+      />


Thoughts on making this a collapsable section, collapsed by default? I imagine for most cases these won't be used

Done in e1f827b.

Reused a component from Roles. Not very pretty, but I didn't want to include extracting a reusable component in this PR.

zmb3 · 2025-04-25T17:08:47Z

Ideally the GHES Configuration would not show up in OSS builds because that's an enterprise only feature.

zmb3 · 2025-04-29T11:13:06Z

+            readonly={readonly}
+          />
+
+          <FieldInput


Are we asking for this information twice by requiring a "fully qualified" repository in the previous step?

Yes, potentially. Either a repo (inc owner) or just the owner is required. The owner is redundant if a repo is provided.

We could consolidate both into a single field and infer which to populate based on the presence of a slash. On the other hand, matching the two fields from the yaml representation seems suited to this non-day-one use case.

Which approach would you suggest?

Personally, I lean towards reflecting the underlying YAML here, otherwise we'll have to find a way to reflect cases where both the owner and repo are set in the underlying YAML as a single field.

avatus · 2025-04-29T16:45:17Z

+export const collectKeys = (value: unknown, prefix: string = '') => {
+  if (typeof value !== 'object' || value === null) {
+    return prefix ? [prefix] : null;
+  }
+


this seems to be somewhat "generic" of a utility. if its only really used from the JoinTokens directory, then id just export it from JoinTokens tbh. if you wanna use it everywhere, you could add it somewhere in the shared package. either way, i wont die on the hill just giving some info.

Moved to shared/utils in 521b629.

avatus · 2025-04-29T16:47:04Z

+export const checkIamYamlData = (data: unknown) => {
+  const keys = collectKeys(data);
+  return !keys || new Set(keys).isSubsetOf(supportedFieldsIam);
+};
+


similar to my comment about collectKeys, this is exported but only used in UpsertJoinTokenDialog so lets just throw it in there and not export it

Moved to UpsertJoinTokenDialog in 521b629.

avatus · 2025-04-29T16:47:32Z

            placeholder="arn:aws:iam::account-id:role/*"
            value={rule.aws_arn}
            onChange={e => setTokenRulesField(index, 'aws_arn', e.target.value)}
+            readonly={readonly}


when would readonly be true for IAM forms?

When it's data includes configuration other than .aws_account and .aws_arn. In which case editing the fields is prevented for all join methods and the submit button is disabled.

Does that behaviour feel ok?

yeah sounds good!

avatus · 2025-04-29T16:49:21Z

+            placeholder="gravitational/teleport"
+            toolTipContent="Fully qualified name of a GitHub repository (i.e. including the owner)"


does this placeholder match the suggestion from the tooltip? if so, it seems a bit off. when i think of "fully qualified name" to me it includes some sort of host/domain type stuff.

I grabbed the copy from the resource reference; https://goteleport.com/docs/reference/machine-id/github-actions/

Would something this fit better?

The full name of a GitHub repository, prefixed by the owning user or organisation (e.g. gravitational/teleport)

I think keeping it in line with the resource reference is fine. thanks for the context

…marais/feat/44794-github-join-tokens

# Conflicts: # lib/web/join_tokens.go # lib/web/join_tokens_test.go

backport-bot-workflows · 2025-05-01T15:16:54Z

@nicholasmarais1158 See the table below for backport results.

Branch	Result
branch/v16	Failed
branch/v17	Failed

* Fix/silence lint issues * Fix spacing on side panel * Return github-specific config from `GET /webapi/tokens` * Add github form for create/edit * Handle unsupported fields during edit * Add js docs for check functions * Add js docs for github check function * Fix casing in test mocks * Fix missing `readonly` prop * Remove unused import * Retain metadata (inc expiry) on token edit * Remove mutual-exclusivity on repo/owner fields * Improve supported fields readability * Hide GHES config * Ignore token not found errors when creating * Lock GHES fields when using OSS * Lint fix * Return github-specific config from `GET /webapi/tokens` * Support "token" in `/webapi/yaml/parse/:kind` * Use empty time.Time for token expiry (`POST /webapi/tokens`) * Revert createTokenForDiscoveryHandle changes * Fix acronym casing * Cover enterprise token types in tests * Cover github tokens in existing tests * Tweak handling of `tokenId` * Check expiry is not overwritten * Revert removing tokenId check * Refactor supported fields * Remove use of `X-Teleport-TokenName` # Conflicts: # web/packages/teleport/src/JoinTokens/JoinTokenForms.tsx # web/packages/teleport/src/JoinTokens/UpsertJoinTokenDialog.tsx # web/packages/teleport/src/Roles/RoleEditor/StandardEditor/sections.tsx # web/packages/teleport/src/services/joinToken/types.ts # web/packages/teleport/src/services/yaml/types.ts # web/packages/teleport/src/teleportContext.tsx

* Fix/silence lint issues * Fix spacing on side panel * Return github-specific config from `GET /webapi/tokens` * Add github form for create/edit * Handle unsupported fields during edit * Add js docs for check functions * Add js docs for github check function * Fix casing in test mocks * Fix missing `readonly` prop * Remove unused import * Retain metadata (inc expiry) on token edit * Remove mutual-exclusivity on repo/owner fields * Improve supported fields readability * Hide GHES config * Ignore token not found errors when creating * Lock GHES fields when using OSS * Lint fix * Return github-specific config from `GET /webapi/tokens` * Support "token" in `/webapi/yaml/parse/:kind` * Use empty time.Time for token expiry (`POST /webapi/tokens`) * Revert createTokenForDiscoveryHandle changes * Fix acronym casing * Cover enterprise token types in tests * Cover github tokens in existing tests * Tweak handling of `tokenId` * Check expiry is not overwritten * Revert removing tokenId check * Refactor supported fields * Remove use of `X-Teleport-TokenName`

…54477) * Fix/silence lint issues * Fix spacing on side panel * Return github-specific config from `GET /webapi/tokens` * Add github form for create/edit * Handle unsupported fields during edit * Add js docs for check functions * Add js docs for github check function * Fix casing in test mocks * Fix missing `readonly` prop * Remove unused import * Retain metadata (inc expiry) on token edit * Remove mutual-exclusivity on repo/owner fields * Improve supported fields readability * Hide GHES config * Ignore token not found errors when creating * Lock GHES fields when using OSS * Lint fix * Return github-specific config from `GET /webapi/tokens` * Support "token" in `/webapi/yaml/parse/:kind` * Use empty time.Time for token expiry (`POST /webapi/tokens`) * Revert createTokenForDiscoveryHandle changes * Fix acronym casing * Cover enterprise token types in tests * Cover github tokens in existing tests * Tweak handling of `tokenId` * Check expiry is not overwritten * Revert removing tokenId check * Refactor supported fields * Remove use of `X-Teleport-TokenName`

nicholasmarais1158 added 8 commits April 25, 2025 10:27

Fix/silence lint issues

67af202

Fix spacing on side panel

3645dd7

Return github-specific config from GET /webapi/tokens

f197c29

Add github form for create/edit

c7805d2

Handle unsupported fields during edit

bb2bf9a

Add js docs for check functions

1ad2db8

Add js docs for github check function

8d0d848

Fix casing in test mocks

0353e4b

nicholasmarais1158 added machine-id backport/branch/v16 backport/branch/v17 labels Apr 25, 2025

nicholasmarais1158 self-assigned this Apr 25, 2025

nicholasmarais1158 added 3 commits April 25, 2025 12:25

Fix missing readonly prop

54fa824

Merge branch 'master' into nickmarais/feat/44794-github-join-tokens

99e97b3

Remove unused import

c2a4dbb

ryanclark reviewed Apr 25, 2025

View reviewed changes

nicholasmarais1158 added 10 commits April 28, 2025 09:24

Merge branch 'master' into nickmarais/feat/44794-github-join-tokens

2277a4e

Retain metadata (inc expiry) on token edit

26ebb15

Remove mutual-exclusivity on repo/owner fields

93a2247

Improve supported fields readability

c2d0292

Hide GHES config

e1f827b

Merge branch 'master' into nickmarais/feat/44794-github-join-tokens

6eb7de0

Ignore token not found errors when creating

e8232a8

Lock GHES fields when using OSS

5234b8a

Lint fix

f7865f2

Merge branch 'master' into nickmarais/feat/44794-github-join-tokens

984a1fa

nicholasmarais1158 marked this pull request as ready for review April 29, 2025 08:19

github-actions Bot added size/xl ui labels Apr 29, 2025

nicholasmarais1158 added the size/lg label Apr 29, 2025

zmb3 reviewed Apr 29, 2025

View reviewed changes

strideynet requested a review from avatus April 29, 2025 12:11

Fix acronym casing

928c83e

avatus reviewed Apr 29, 2025

View reviewed changes

nicholasmarais1158 added 8 commits April 30, 2025 09:37

Cover enterprise token types in tests

2b90f08

Cover github tokens in existing tests

e7fd9a5

Tweak handling of tokenId

98c1c72

Check expiry is not overwritten

1311d91

Merge branch 'master' into nickmarais/feat/44794-github-join-tokens-api

7696729

Revert removing tokenId check

f0254a6

Merge branch 'nickmarais/feat/44794-github-join-tokens-api' into nick…

0209744

…marais/feat/44794-github-join-tokens

Refactor supported fields

521b629

nicholasmarais1158 requested a review from avatus April 30, 2025 10:50

avatus approved these changes Apr 30, 2025

View reviewed changes

ryanclark approved these changes May 1, 2025

View reviewed changes

Base automatically changed from nickmarais/feat/44794-github-join-tokens-api to master May 1, 2025 13:49

nicholasmarais1158 added 2 commits May 1, 2025 15:41

Merge branch 'master' into nickmarais/feat/44794-github-join-tokens

c4aa6ec

# Conflicts: # lib/web/join_tokens.go # lib/web/join_tokens_test.go

Remove use of X-Teleport-TokenName

0c27883

nicholasmarais1158 added this pull request to the merge queue May 1, 2025

Merged via the queue into master with commit 0eabb00 May 1, 2025
40 checks passed

nicholasmarais1158 deleted the nickmarais/feat/44794-github-join-tokens branch May 1, 2025 15:15

nicholasmarais1158 mentioned this pull request May 2, 2025

[v16] feat(join-tokens): [44794] Add Join Token form for GitHub #54476

Closed

nicholasmarais1158 mentioned this pull request May 2, 2025

[v17] feat(join-tokens): [44794] Add Join Token form for GitHub #54477

Merged

strideynet mentioned this pull request May 6, 2025

Machine ID: Add GitHub "Create Join Token" form #44794

Closed

		placeholder="gravitational/teleport"
		toolTipContent="Fully qualified name of a GitHub repository (i.e. including the owner)"

Conversation

nicholasmarais1158 commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Changes

Demo

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zmb3 commented Apr 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

backport-bot-workflows Bot commented May 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nicholasmarais1158 commented Apr 25, 2025 •

edited

Loading