From 67af202da8760c2ec063539a4c08cf123f1486ac Mon Sep 17 00:00:00 2001 From: Nick Marais Date: Thu, 17 Apr 2025 17:23:18 +0100 Subject: [PATCH 01/29] Fix/silence lint issues --- web/packages/teleport/src/JoinTokens/JoinTokens.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/web/packages/teleport/src/JoinTokens/JoinTokens.tsx b/web/packages/teleport/src/JoinTokens/JoinTokens.tsx index 1e92c92d4b8a0..c4d6ea292cfa3 100644 --- a/web/packages/teleport/src/JoinTokens/JoinTokens.tsx +++ b/web/packages/teleport/src/JoinTokens/JoinTokens.tsx @@ -134,6 +134,9 @@ export const JoinTokens = () => { useEffect(() => { runJoinTokensAttempt(); + + // runJoinTokensAttempt is not stable and causes a render loop + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); return ( @@ -395,7 +398,7 @@ function TokenDelete({ Delete Join Token? - {attempt.status === 'error' && } + {attempt.status === 'error' && {attempt.statusText}} You are about to delete join token From 3645dd7d2712f231b4542e2712a42b2932ba70c9 Mon Sep 17 00:00:00 2001 From: Nick Marais Date: Thu, 17 Apr 2025 17:23:54 +0100 Subject: [PATCH 02/29] Fix spacing on side panel --- web/packages/teleport/src/JoinTokens/JoinTokens.tsx | 2 +- .../teleport/src/JoinTokens/UpsertJoinTokenDialog.tsx | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/web/packages/teleport/src/JoinTokens/JoinTokens.tsx b/web/packages/teleport/src/JoinTokens/JoinTokens.tsx index c4d6ea292cfa3..82853c3c9420d 100644 --- a/web/packages/teleport/src/JoinTokens/JoinTokens.tsx +++ b/web/packages/teleport/src/JoinTokens/JoinTokens.tsx @@ -164,7 +164,7 @@ export const JoinTokens = () => { )} - + - + From f197c2925700d8287a1cc07b76a09071b3133fb0 Mon Sep 17 00:00:00 2001 From: Nick Marais Date: Wed, 23 Apr 2025 16:15:51 +0100 Subject: [PATCH 03/29] Return github-specific config from `GET /webapi/tokens` --- api/types/provisioning.go | 7 ++++ lib/web/join_tokens_test.go | 81 +++++++++++++++++++++++++++++++++++++ lib/web/ui/join_token.go | 7 ++++ 3 files changed, 95 insertions(+) diff --git a/api/types/provisioning.go b/api/types/provisioning.go index 9768e25ab863e..32589bc591b07 100644 --- a/api/types/provisioning.go +++ b/api/types/provisioning.go @@ -135,6 +135,8 @@ type ProvisionToken interface { SetAllowRules([]*TokenRule) // GetGCPRules will return the GCP rules within this token. GetGCPRules() *ProvisionTokenSpecV2GCP + // GetGithubRules will return the GitHub rules within this token. + GetGithubRules() *ProvisionTokenSpecV2GitHub // GetAWSIIDTTL returns the TTL of EC2 IIDs GetAWSIIDTTL() Duration // GetJoinMethod returns joining method that must be used with this token. @@ -437,6 +439,11 @@ func (p *ProvisionTokenV2) GetGCPRules() *ProvisionTokenSpecV2GCP { return p.Spec.GCP } +// GetGithubRules will return the GitHub rules within this token. +func (p *ProvisionTokenV2) GetGithubRules() *ProvisionTokenSpecV2GitHub { + return p.Spec.GitHub +} + // GetAWSIIDTTL returns the TTL of EC2 IIDs func (p *ProvisionTokenV2) GetAWSIIDTTL() Duration { return p.Spec.AWSIIDTTL diff --git a/lib/web/join_tokens_test.go b/lib/web/join_tokens_test.go index d47e2c1b137f3..8dbbd72bc4133 100644 --- a/lib/web/join_tokens_test.go +++ b/lib/web/join_tokens_test.go @@ -27,6 +27,7 @@ import ( "net/http" "net/url" "regexp" + "slices" "strconv" "testing" "time" @@ -268,6 +269,86 @@ func TestGetTokens(t *testing.T) { } } +func TestGetGithubTokens(t *testing.T) { + t.Parallel() + ctx := context.Background() + + username := "test-user@example.com" + expiry := time.Now().UTC().Add(30 * time.Minute) + + env := newWebPack(t, 1) + proxy := env.proxies[0] + pack := proxy.authPack(t, username, nil /* roles */) + + td := tokenData{ + name: "github-test-token", + expiry: expiry, + roles: types.SystemRoles{types.RoleBot}, + } + + token, err := types.NewProvisionTokenFromSpec(td.name, td.expiry, types.ProvisionTokenSpecV2{ + Roles: td.roles, + BotName: "test-bot", + JoinMethod: types.JoinMethodGitHub, + + GitHub: &types.ProvisionTokenSpecV2GitHub{ + EnterpriseServerHost: "github.example.com", + StaticJWKS: "{\"keys\":[]}", + Allow: []*types.ProvisionTokenSpecV2GitHub_Rule{ + { + Repository: "gravitational/teleport", + RepositoryOwner: "gravitational", + Sub: "test-sub", + Workflow: "test-workflow", + Environment: "test-environment", + Actor: "octocat", + Ref: "ref/heads/main", + RefType: "branch", + }, + }, + }, + }) + require.NoError(t, err) + err = env.server.Auth().CreateToken(ctx, token) + require.NoError(t, err) + + endpoint := pack.clt.Endpoint("webapi", "tokens") + re, err := pack.clt.Get(ctx, endpoint, url.Values{}) + require.NoError(t, err) + + resp := GetTokensResponse{} + require.NoError(t, json.Unmarshal(re.Bytes(), &resp)) + + require.Len(t, resp.Items, 2) // Including a static token + + githubTokenIndex := slices.IndexFunc(resp.Items, func(item ui.JoinToken) bool { return item.Method == types.JoinMethodGitHub }) + require.NotEqual(t, githubTokenIndex, -1) + require.Empty(t, cmp.Diff(resp.Items[githubTokenIndex], ui.JoinToken{ + ID: "github-test-token", + SafeName: "github-test-token", + BotName: "test-bot", + Expiry: expiry, + Roles: types.SystemRoles{"Bot"}, + Method: types.JoinMethodGitHub, + Github: &types.ProvisionTokenSpecV2GitHub{ + EnterpriseServerHost: "github.example.com", + StaticJWKS: "{\"keys\":[]}", + Allow: []*types.ProvisionTokenSpecV2GitHub_Rule{ + { + Repository: "gravitational/teleport", + RepositoryOwner: "gravitational", + Sub: "test-sub", + Workflow: "test-workflow", + Environment: "test-environment", + Actor: "octocat", + Ref: "ref/heads/main", + RefType: "branch", + }, + }, + }, + }, cmpopts.IgnoreFields(ui.JoinToken{}, "Content"))) +} + func TestDeleteToken(t *testing.T) { ctx := context.Background() username := "test-user@example.com" diff --git a/lib/web/ui/join_token.go b/lib/web/ui/join_token.go index be068482aa1ba..2319841375e22 100644 --- a/lib/web/ui/join_token.go +++ b/lib/web/ui/join_token.go @@ -47,6 +47,8 @@ type JoinToken struct { Allow []*types.TokenRule `json:"allow,omitempty"` // GCP allows the configuration of options specific to the "gcp" join method. GCP *types.ProvisionTokenSpecV2GCP `json:"gcp,omitempty"` + // GitHub-specific configuration for the join method. + Github *types.ProvisionTokenSpecV2GitHub `json:"github,omitempty"` // Content is resource yaml content. Content string `json:"content"` } @@ -71,6 +73,11 @@ func MakeJoinToken(token types.ProvisionToken) (*JoinToken, error) { if uiToken.Method == types.JoinMethodGCP { uiToken.GCP = token.GetGCPRules() } + + if uiToken.Method == types.JoinMethodGitHub { + uiToken.Github = token.GetGithubRules() + } + return uiToken, nil } From c7805d29d3f3a6ff08670617f7fd19de840e420e Mon Sep 17 00:00:00 2001 From: Nick Marais Date: Thu, 24 Apr 2025 10:08:37 +0100 Subject: [PATCH 04/29] Add github form for create/edit --- web/packages/design/src/utils/testing.tsx | 6 +- .../components/Validation/rules.test.ts | 22 + .../shared/components/Validation/rules.ts | 21 + .../JoinTokens/JoinTokenGithubForm.test.tsx | 388 ++++++++++++++++++ .../src/JoinTokens/JoinTokenGithubForm.tsx | 263 ++++++++++++ .../teleport/src/JoinTokens/JoinTokens.tsx | 3 +- .../src/JoinTokens/UpsertJoinTokenDialog.tsx | 99 ++++- .../src/services/joinToken/makeJoinToken.ts | 4 +- .../teleport/src/services/joinToken/types.ts | 32 ++ 9 files changed, 824 insertions(+), 14 deletions(-) create mode 100644 web/packages/teleport/src/JoinTokens/JoinTokenGithubForm.test.tsx create mode 100644 web/packages/teleport/src/JoinTokens/JoinTokenGithubForm.tsx diff --git a/web/packages/design/src/utils/testing.tsx b/web/packages/design/src/utils/testing.tsx index 7312f4eb1a5af..b68eecff79cfe 100644 --- a/web/packages/design/src/utils/testing.tsx +++ b/web/packages/design/src/utils/testing.tsx @@ -25,6 +25,7 @@ import { render as testingRender, waitFor, waitForElementToBeRemoved, + within, } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { ReactNode } from 'react'; @@ -78,8 +79,8 @@ screen.debug = () => { }; type RenderOptions = { - wrapper: React.FC; - container: HTMLElement; + wrapper?: React.FC; + container?: HTMLElement; }; export { @@ -95,4 +96,5 @@ export { Router, userEvent, waitForElementToBeRemoved, + within, }; diff --git a/web/packages/shared/components/Validation/rules.test.ts b/web/packages/shared/components/Validation/rules.test.ts index 827264949d559..3a216ea44c87d 100644 --- a/web/packages/shared/components/Validation/rules.test.ts +++ b/web/packages/shared/components/Validation/rules.test.ts @@ -18,6 +18,7 @@ import { arrayOf, + falsyField, requiredConfirmedPassword, requiredEmailLike, requiredField, @@ -212,3 +213,24 @@ test.each([ ])('arrayOf: $name', ({ items, expected }) => { expect(arrayOf(requiredField('required'))(items)()).toEqual(expected); }); + +describe('falsyField', () => { + const errMsg = 'error text'; + const validator = falsyField(errMsg); + + test.each` + input | expected + ${'not empty value'} | ${{ valid: false, message: errMsg }} + ${1} | ${{ valid: false, message: errMsg }} + ${true} | ${{ valid: false, message: errMsg }} + ${{}} | ${{ valid: false, message: errMsg }} + ${[]} | ${{ valid: false, message: errMsg }} + ${''} | ${{ valid: true, message: undefined }} + ${null} | ${{ valid: true, message: undefined }} + ${undefined} | ${{ valid: true, message: undefined }} + ${0} | ${{ valid: true, message: undefined }} + ${false} | ${{ valid: true, message: undefined }} + `('input with: $input', ({ input, expected }) => { + expect(validator(input)()).toEqual(expected); + }); +}); diff --git a/web/packages/shared/components/Validation/rules.ts b/web/packages/shared/components/Validation/rules.ts index aa111dc786c11..668aa9d70a48d 100644 --- a/web/packages/shared/components/Validation/rules.ts +++ b/web/packages/shared/components/Validation/rules.ts @@ -260,6 +260,26 @@ const requiredPort: Rule = port => () => { }; }; +/** + * falsyField checks the the value is falsy + * @param message the message to return if invalid + * @returns validaton result + */ +const falsyField = + (message: string): Rule => + value => + () => { + if (value) { + return { + valid: false, + message, + }; + } + return { + valid: true, + }; + }; + /** * A rule function that combines multiple inner rule functions. All rules must * return `valid`, otherwise it returns a comma separated string containing all @@ -379,4 +399,5 @@ export { requiredPort, arrayOf, precomputed, + falsyField, }; diff --git a/web/packages/teleport/src/JoinTokens/JoinTokenGithubForm.test.tsx b/web/packages/teleport/src/JoinTokens/JoinTokenGithubForm.test.tsx new file mode 100644 index 0000000000000..074025aaa3c79 --- /dev/null +++ b/web/packages/teleport/src/JoinTokens/JoinTokenGithubForm.test.tsx @@ -0,0 +1,388 @@ +/** + * Teleport + * Copyright (C) 2025 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import userEvent from '@testing-library/user-event'; +import { PropsWithChildren } from 'react'; + +import { fireEvent, render, screen, within } from 'design/utils/testing'; +import Validation, { useValidation } from 'shared/components/Validation'; + +import { ThemeProvider } from 'teleport/ThemeProvider'; + +import { JoinTokenGithubForm } from './JoinTokenGithubForm'; +import { NewJoinTokenState } from './UpsertJoinTokenDialog'; + +const populateRuleFieldTest = + ( + field: keyof NewJoinTokenState['github']['rules'][number], + placeholer: string, + value: string + ) => + async () => { + const state = baseState(); + const onUpdate = jest.fn(); + + render( + , + { wrapper: Wrapper } + ); + + fireEvent.change(screen.getByPlaceholderText(placeholer), { + target: { value }, + }); + + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenLastCalledWith( + baseState({ + rules: [ + { + ...state.github.rules[0], + [field]: value, + }, + ], + }) + ); + }; + +const populateFieldTest = + ({ + field, + placeholer, + value, + }: { + field: keyof NewJoinTokenState['github']; + placeholer: string; + value: string; + }) => + async () => { + const state = baseState(); + const onUpdate = jest.fn(); + + render( + , + { wrapper: Wrapper } + ); + + fireEvent.change(screen.getByPlaceholderText(placeholer), { + target: { value }, + }); + + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenLastCalledWith( + baseState({ + [field]: value, + }) + ); + }; + +describe('GithubJoinTokenForm', () => { + it('a rule can be added', async () => { + const state = baseState(); + const onUpdate = jest.fn(); + + render( + , + { wrapper: Wrapper } + ); + + await userEvent.click( + screen.getByRole('button', { name: /Add another GitHub rule/i }) + ); + + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenLastCalledWith( + baseState({ + rules: [ + ...state.github.rules, + { + ref_type: 'any', + }, + ], + }) + ); + }); + + it('delete button is hidden when only one rule exists', async () => { + const state = baseState(); + + render( + , + { wrapper: Wrapper } + ); + + expect(screen.queryByTestId('delete_rule')).not.toBeInTheDocument(); + }); + + it('delete button is visible when more than one rule exists', async () => { + const state = baseState({ + rules: [{}, {}], + }); + + render( + , + { wrapper: Wrapper } + ); + + expect(screen.queryAllByTestId('delete_rule').length).toBe(2); + }); + + it('a rule can be deleted', async () => { + const state = baseState({ + rules: [{}, {}], + }); + const onUpdate = jest.fn(); + + render( + , + { wrapper: Wrapper } + ); + + const rule0 = screen.getByTestId('rule_0'); + const deleteButton0 = within(rule0).getByTestId('delete_rule'); + + await userEvent.click(deleteButton0); + + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenLastCalledWith( + baseState({ + rules: [state.github.rules[0]], + }) + ); + }); + + // eslint-disable-next-line jest/expect-expect + it( + 'repository field can be populated', + populateRuleFieldTest( + 'repository', + 'gravitational/teleport', + 'gravitational/teleport' + ) + ); + + it('repository field is disabled when a repository owner is populated', async () => { + const state = baseState({ + rules: [ + { + repository_owner: 'something', + }, + ], + }); + + render( + , + { wrapper: Wrapper } + ); + + const field = screen.getByPlaceholderText('gravitational/teleport'); + + expect(field).toBeDisabled(); + }); + + it('repository field shows a validation message', async () => { + const state = baseState(); + + render( + , + { wrapper: Wrapper } + ); + + await userEvent.click(screen.getByTestId('submit')); + + expect( + screen.getByText('Either repository name or owner is required') + ).toBeInTheDocument(); + }); + + // eslint-disable-next-line jest/expect-expect + it( + 'repository owner field can be populated', + populateRuleFieldTest('repository_owner', 'gravitational', 'gravitational') + ); + + it('repository owner field is disabled when a repository name is populated', async () => { + const state = baseState({ + rules: [ + { + repository: 'something', + }, + ], + }); + + render( + , + { wrapper: Wrapper } + ); + + const field = screen.getByPlaceholderText('gravitational'); + + expect(field).toBeDisabled(); + }); + + it('repository owner field shows a validation message', async () => { + const state = baseState(); + + render( + , + { wrapper: Wrapper } + ); + + await userEvent.click(screen.getByTestId('submit')); + + expect( + screen.getByText('Either repository owner or name is required') + ).toBeInTheDocument(); + }); + + // eslint-disable-next-line jest/expect-expect + it( + 'workflow field can be populated', + populateRuleFieldTest('workflow', 'my-workflow', 'my-workflow') + ); + + // eslint-disable-next-line jest/expect-expect + it( + 'environment field can be populated', + populateRuleFieldTest('environment', 'production', 'production') + ); + + // eslint-disable-next-line jest/expect-expect + it( + 'ref field can be populated', + populateRuleFieldTest('ref', 'ref/heads/main', 'ref/heads/main') + ); + + it('ref type is disabled when ref is not populated', async () => { + const state = baseState(); + + render( + , + { wrapper: Wrapper } + ); + + expect(screen.getByLabelText('Ref type')).toBeDisabled(); + }); + + it('ref type can be selected', async () => { + const state = baseState({ + rules: [ + { + ref: 'ref/heads/main', + ref_type: 'any', + }, + ], + }); + const onUpdate = jest.fn(); + + render( + , + { wrapper: Wrapper } + ); + + const selectElement = screen.getByLabelText('Ref type'); + expect(selectElement).toBeEnabled(); + + // Seems to be the only way to interact with react-select component + fireEvent.keyDown(selectElement, { key: 'ArrowDown' }); + const existingItem = await screen.findByText('Branch'); + fireEvent.click(existingItem); + + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenLastCalledWith( + baseState({ + rules: [ + { + ...state.github.rules[0], + ref_type: 'branch', + }, + ], + }) + ); + }); + + // eslint-disable-next-line jest/expect-expect + it( + 'server host field can be populated', + populateFieldTest({ + field: 'server_host', + placeholer: 'github.example.com', + value: 'github.example.com', + }) + ); + + // eslint-disable-next-line jest/expect-expect + it( + 'slug field can be populated', + populateFieldTest({ + field: 'enterprise_slug', + placeholer: 'octo-enterprise', + value: 'octo-enterprise', + }) + ); + + // eslint-disable-next-line jest/expect-expect + it( + 'jwks field can be populated', + populateFieldTest({ + field: 'static_jwks', + placeholer: '{"keys":[--snip--]}', + value: '{"keys":[]}', + }) + ); +}); + +const Wrapper = ({ children }: PropsWithChildren) => { + return ( + + + {children} + + + ); +}; + +const SubmitWrapper = ({ children }: PropsWithChildren) => { + const validation = useValidation(); + + return ( + <> + {children} +