From a5623a0582e8d88368eecfc255c5730808b2667a Mon Sep 17 00:00:00 2001 From: Marek Libra Date: Tue, 24 Mar 2026 08:57:52 +0100 Subject: [PATCH 1/2] feat(x2a): RepoAuthentication renders summary of required auth tokens Signed-off-by: Marek Libra --- workspaces/x2a/.changeset/long-baboons-dig.md | 5 + .../scaffolder/RepoAuthentication.test.tsx | 1451 +++++++++++++++-- .../x2a/src/scaffolder/RepoAuthentication.tsx | 245 ++- .../x2a/src/scaffolder/useProviderAuth.ts | 297 ++++ .../conversion-project-template.yaml | 2 + 5 files changed, 1752 insertions(+), 248 deletions(-) create mode 100644 workspaces/x2a/.changeset/long-baboons-dig.md create mode 100644 workspaces/x2a/plugins/x2a/src/scaffolder/useProviderAuth.ts diff --git a/workspaces/x2a/.changeset/long-baboons-dig.md b/workspaces/x2a/.changeset/long-baboons-dig.md new file mode 100644 index 0000000000..aa3a724bb9 --- /dev/null +++ b/workspaces/x2a/.changeset/long-baboons-dig.md @@ -0,0 +1,5 @@ +--- +'@red-hat-developer-hub/backstage-plugin-x2a': patch +--- + +Adding summary to the RepoAuthentication widget. diff --git a/workspaces/x2a/plugins/x2a/src/scaffolder/RepoAuthentication.test.tsx b/workspaces/x2a/plugins/x2a/src/scaffolder/RepoAuthentication.test.tsx index b3b2cdf68d..9d861a1bed 100644 --- a/workspaces/x2a/plugins/x2a/src/scaffolder/RepoAuthentication.test.tsx +++ b/workspaces/x2a/plugins/x2a/src/scaffolder/RepoAuthentication.test.tsx @@ -35,8 +35,9 @@ jest.mock('../repoAuth', () => ({ }), })); -import { render, screen, waitFor, act } from '@testing-library/react'; +import { render, screen, waitFor, act, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import { ThemeProvider, createTheme } from '@material-ui/core/styles'; import { RepoAuthentication, repoAuthenticationValidation, @@ -44,6 +45,19 @@ import { import type { FieldExtensionComponentProps } from '@backstage/plugin-scaffolder-react'; import { ApiHolder } from '@backstage/core-plugin-api'; +const statusTheme = createTheme({ + palette: { + status: { + ok: '#71CF88', + warning: '#FFB84D', + error: '#F84C55', + running: '#3E8635', + pending: '#AAAAAA', + background: '#FEFEFE', + }, + }, +} as any); + function toDataUrl(csv: string): string { return `data:text/csv;base64,${Buffer.from(csv).toString('base64')}`; } @@ -64,16 +78,18 @@ const CROSS_PROVIDER_CSV = toDataUrl( 'Cross Project,CP,https://github.com/org/source,main,https://gitlab.com/org/target,main', ); +const flushAsync = () => new Promise(r => setTimeout(r, 0)); + const validatorContext = { apiHolder: { get: jest.fn() } as unknown as ApiHolder, formData: {}, schema: {}, }; -function renderComponent( +function makeProps( overrides: Partial> = {}, -) { - const defaultProps = { +): FieldExtensionComponentProps { + return { onChange: jest.fn(), onBlur: jest.fn(), onFocus: jest.fn(), @@ -97,8 +113,24 @@ function renderComponent( registry: {}, ...overrides, } as unknown as FieldExtensionComponentProps; +} - return render(); +async function renderComponent( + overrides: Partial> = {}, +) { + const props = makeProps(overrides); + let result!: ReturnType; + await act(async () => { + result = render( + + + , + ); + // Yield to the macrotask queue so fire-and-forget async auth + // effects settle their microtasks inside this act() scope. + await flushAsync(); + }); + return result; } describe('RepoAuthentication', () => { @@ -110,16 +142,16 @@ describe('RepoAuthentication', () => { }); describe('rendering', () => { - it('should render title and description', () => { - renderComponent(); + it('should render title and description', async () => { + await renderComponent(); expect(screen.getByText('Repo Auth')).toBeInTheDocument(); expect( screen.getByText('Authenticate with SCM providers'), ).toBeInTheDocument(); }); - it('should show error when csvFieldName is not configured', () => { - renderComponent({ + it('should show error when csvFieldName is not configured', async () => { + await renderComponent({ uiSchema: { 'ui:options': {} }, }); expect( @@ -129,8 +161,8 @@ describe('RepoAuthentication', () => { ).toBeInTheDocument(); }); - it('should not show csvFieldName error when configured', () => { - renderComponent(); + it('should not show csvFieldName error when configured', async () => { + await renderComponent(); expect( screen.queryByText( 'CSV field name is required for RepoAuthentication extension', @@ -141,38 +173,57 @@ describe('RepoAuthentication', () => { describe('CSV parsing errors', () => { it('should show error for invalid CSV data-URL', async () => { - renderComponent({ + await renderComponent({ formContext: { formData: { csvContent: 'not-a-data-url' }, }, }); - await waitFor(() => { - expect( - screen.getByText(/expected a base64-encoded data-URL/), - ).toBeInTheDocument(); - }); + expect( + screen.getByText(/expected a base64-encoded data-URL/), + ).toBeInTheDocument(); }); it('should show error for CSV missing required columns', async () => { const invalidCsv = toDataUrl('name,abbreviation\nProject,PRJ'); - renderComponent({ + await renderComponent({ formContext: { formData: { csvContent: invalidCsv }, }, }); - await waitFor(() => { - expect( - screen.getByText(/CSV is missing required column/), - ).toBeInTheDocument(); + expect( + screen.getByText(/CSV is missing required column/), + ).toBeInTheDocument(); + }); + + it('should display parseError in the bottom error area and not render a table', async () => { + await renderComponent({ + formContext: { + formData: { csvContent: 'not-a-data-url' }, + }, + }); + + expect( + screen.getByText(/expected a base64-encoded data-URL/), + ).toBeInTheDocument(); + expect(screen.queryByRole('table')).not.toBeInTheDocument(); + }); + + it('should not attempt authentication when CSV has a parse error', async () => { + await renderComponent({ + formContext: { + formData: { csvContent: 'not-a-data-url' }, + }, }); + + expect(mockAuthenticate).not.toHaveBeenCalled(); }); }); describe('authentication flow', () => { it('should call authenticate for providers found in CSV', async () => { - renderComponent({ + await renderComponent({ formContext: { formData: { csvContent: VALID_CSV }, }, @@ -185,7 +236,7 @@ describe('RepoAuthentication', () => { it('should call onChange with "authenticated" on success', async () => { const onChange = jest.fn(); - renderComponent({ + await renderComponent({ onChange, formContext: { formData: { csvContent: VALID_CSV }, @@ -198,7 +249,7 @@ describe('RepoAuthentication', () => { }); it('should store tokens via setSecrets on success', async () => { - renderComponent({ + await renderComponent({ formContext: { formData: { csvContent: VALID_CSV }, }, @@ -219,7 +270,7 @@ describe('RepoAuthentication', () => { return Promise.resolve([{ token: `token-for-${provider}`, provider }]); }); - renderComponent({ + await renderComponent({ formContext: { formData: { csvContent: MIXED_CSV }, }, @@ -245,7 +296,7 @@ describe('RepoAuthentication', () => { return Promise.resolve([{ token: `token-for-${provider}`, provider }]); }); - renderComponent({ + await renderComponent({ formContext: { formData: { csvContent: CROSS_PROVIDER_CSV }, }, @@ -265,8 +316,8 @@ describe('RepoAuthentication', () => { }); }); - it('should not call authenticate when csvContent is absent', () => { - renderComponent({ + it('should not call authenticate when csvContent is absent', async () => { + await renderComponent({ formContext: { formData: {} }, }); @@ -275,33 +326,20 @@ describe('RepoAuthentication', () => { it('should re-authenticate when CSV content changes', async () => { const onChange = jest.fn(); - const { rerender } = render( - )} - />, - ); + const props = makeProps({ + onChange, + formContext: { formData: { csvContent: VALID_CSV } }, + }); + + let result!: ReturnType; + await act(async () => { + result = render( + + + , + ); + await flushAsync(); + }); await waitFor(() => { expect(onChange).toHaveBeenCalledWith('authenticated'); @@ -316,33 +354,19 @@ describe('RepoAuthentication', () => { { token: 'new-token', provider: 'gitlab' }, ]); - rerender( - )} - />, - ); + await act(async () => { + result.rerender( + + + , + ); + await flushAsync(); + }); await waitFor(() => { expect(onChange).toHaveBeenCalledWith(undefined); @@ -363,7 +387,7 @@ describe('RepoAuthentication', () => { return Promise.reject(new Error('GitLab auth failed')); }); - renderComponent({ + await renderComponent({ formContext: { formData: { csvContent: MIXED_CSV }, }, @@ -382,10 +406,10 @@ describe('RepoAuthentication', () => { }); describe('error handling', () => { - it('should show error and Try again button when auth fails', async () => { + it('should show error and per-row Retry button when auth fails', async () => { mockAuthenticate.mockRejectedValue(new Error('OAuth dialog cancelled')); - renderComponent({ + await renderComponent({ formContext: { formData: { csvContent: VALID_CSV }, }, @@ -395,14 +419,14 @@ describe('RepoAuthentication', () => { expect(screen.getByText('OAuth dialog cancelled')).toBeInTheDocument(); }); - expect(screen.getByText('Try again')).toBeInTheDocument(); + expect(screen.getByText('Retry')).toBeInTheDocument(); }); it('should call onChange with undefined when auth fails', async () => { mockAuthenticate.mockRejectedValue(new Error('Auth failed')); const onChange = jest.fn(); - renderComponent({ + await renderComponent({ onChange, formContext: { formData: { csvContent: VALID_CSV }, @@ -414,17 +438,17 @@ describe('RepoAuthentication', () => { }); }); - it('should retry authentication when Try again is clicked', async () => { + it('should retry authentication for a single provider when Retry is clicked', async () => { mockAuthenticate.mockRejectedValueOnce(new Error('First failure')); - renderComponent({ + await renderComponent({ formContext: { formData: { csvContent: VALID_CSV }, }, }); await waitFor(() => { - expect(screen.getByText('Try again')).toBeInTheDocument(); + expect(screen.getByText('Retry')).toBeInTheDocument(); }); mockAuthenticate.mockResolvedValue([ @@ -432,38 +456,1259 @@ describe('RepoAuthentication', () => { ]); await act(async () => { - await userEvent.click(screen.getByText('Try again')); + await userEvent.click(screen.getByText('Retry')); + await flushAsync(); }); await waitFor(() => { expect(mockAuthenticate).toHaveBeenCalledTimes(2); }); }); - }); - describe('csvFieldName', () => { - it('should read CSV from the field specified by csvFieldName', async () => { - renderComponent({ - uiSchema: { 'ui:options': { csvFieldName: 'myCustomCsv' } }, + it('should call onChange and setSecrets after a successful retry completes all providers', async () => { + mockAuthenticate.mockRejectedValueOnce( + new Error('OAuth dialog cancelled'), + ); + + const onChange = jest.fn(); + await renderComponent({ + onChange, formContext: { - formData: { myCustomCsv: VALID_CSV }, + formData: { csvContent: VALID_CSV }, }, }); await waitFor(() => { - expect(mockAuthenticate).toHaveBeenCalled(); + expect(screen.getByText('Retry')).toBeInTheDocument(); + }); + + expect(onChange).toHaveBeenCalledWith(undefined); + expect(mockSetSecrets).not.toHaveBeenCalled(); + + mockAuthenticate.mockResolvedValue([ + { token: 'retry-token', provider: 'github' }, + ]); + + await act(async () => { + await userEvent.click(screen.getByText('Retry')); + await flushAsync(); + }); + + await waitFor(() => { + expect(onChange).toHaveBeenCalledWith('authenticated'); + }); + + await waitFor(() => { + expect(mockSetSecrets).toHaveBeenCalledWith( + expect.objectContaining({ + OAUTH_TOKEN_github: 'retry-token', + }), + ); }); }); - it('should not attempt auth when csvFieldName points to empty field', () => { - renderComponent({ - uiSchema: { 'ui:options': { csvFieldName: 'myCustomCsv' } }, + it('should complete authentication after retrying only the failed provider in a mixed scenario', async () => { + let callCount = 0; + mockAuthenticate.mockImplementation(() => { + callCount++; + if (callCount === 1) { + return Promise.resolve([{ token: 'gh-token', provider: 'github' }]); + } + return Promise.reject(new Error('GitLab auth failed')); + }); + + const onChange = jest.fn(); + await renderComponent({ + onChange, + formContext: { + formData: { csvContent: MIXED_CSV }, + }, + }); + + await waitFor(() => { + expect(screen.getByText('GitLab auth failed')).toBeInTheDocument(); + }); + + expect(onChange).toHaveBeenCalledWith(undefined); + expect(mockSetSecrets).not.toHaveBeenCalled(); + + mockAuthenticate.mockResolvedValue([ + { token: 'gl-retry-token', provider: 'gitlab' }, + ]); + + await act(async () => { + await userEvent.click(screen.getByText('Retry')); + await flushAsync(); + }); + + await waitFor(() => { + expect(onChange).toHaveBeenCalledWith('authenticated'); + }); + + await waitFor(() => { + expect(mockSetSecrets).toHaveBeenCalledWith( + expect.objectContaining({ + OAUTH_TOKEN_github: 'gh-token', + OAUTH_TOKEN_gitlab: 'gl-retry-token', + }), + ); + }); + }); + + it('should clear error message when retry starts', async () => { + mockAuthenticate.mockRejectedValueOnce(new Error('Auth error')); + + await renderComponent({ formContext: { formData: { csvContent: VALID_CSV }, }, }); - expect(mockAuthenticate).not.toHaveBeenCalled(); + await waitFor(() => { + expect(screen.getByText('Auth error')).toBeInTheDocument(); + }); + + mockAuthenticate.mockImplementation( + () => new Promise(() => {}), // never resolves + ); + + await act(async () => { + await userEvent.click(screen.getByText('Retry')); + await flushAsync(); + }); + + await waitFor(() => { + expect(screen.queryByText('Auth error')).not.toBeInTheDocument(); + }); + }); + + it('should keep Retry available when retry fails again', async () => { + mockAuthenticate + .mockRejectedValueOnce(new Error('First failure')) + .mockRejectedValueOnce(new Error('Second failure')); + + await renderComponent({ + formContext: { + formData: { csvContent: VALID_CSV }, + }, + }); + + await waitFor(() => { + expect(screen.getByText('First failure')).toBeInTheDocument(); + expect(screen.getByText('Retry')).toBeInTheDocument(); + }); + + await act(async () => { + await userEvent.click(screen.getByText('Retry')); + await flushAsync(); + }); + + await waitFor(() => { + expect(screen.getByText('Second failure')).toBeInTheDocument(); + expect(screen.getByText('Retry')).toBeInTheDocument(); + }); + + mockAuthenticate.mockResolvedValue([ + { token: 'third-token', provider: 'github' }, + ]); + + await act(async () => { + await userEvent.click(screen.getByText('Retry')); + await flushAsync(); + }); + + await waitFor(() => { + expect(mockSetSecrets).toHaveBeenCalledWith( + expect.objectContaining({ + OAUTH_TOKEN_github: 'third-token', + }), + ); + }); + }); + + it('should complete both providers when retried concurrently', async () => { + mockAuthenticate.mockRejectedValue(new Error('Auth failed')); + + const onChange = jest.fn(); + let container!: HTMLElement; + await act(async () => { + const result = render( + + + , + ); + container = result.container; + await flushAsync(); + }); + + await waitFor(() => { + expect(screen.getAllByText('Retry')).toHaveLength(2); + }); + + let resolveGithub!: (value: unknown) => void; + let resolveGitlab!: (value: unknown) => void; + + mockAuthenticate + .mockImplementationOnce( + () => + new Promise(r => { + resolveGithub = r; + }), + ) + .mockImplementationOnce( + () => + new Promise(r => { + resolveGitlab = r; + }), + ); + + const rows = container.querySelectorAll('tbody tr'); + const githubRetry = within(rows[0] as HTMLElement).getByText('Retry'); + + await act(async () => { + await userEvent.click(githubRetry); + await flushAsync(); + }); + + // github is now pending; only gitlab's Retry remains + await act(async () => { + await userEvent.click(screen.getByText('Retry')); + await flushAsync(); + }); + + // Both auths are in-flight — resolve them concurrently + await act(async () => { + resolveGithub([{ token: 'gh-retry-token', provider: 'github' }]); + resolveGitlab([{ token: 'gl-retry-token', provider: 'gitlab' }]); + await flushAsync(); + }); + + await waitFor(() => { + expect(onChange).toHaveBeenCalledWith('authenticated'); + }); + + await waitFor(() => { + expect(mockSetSecrets).toHaveBeenCalledWith( + expect.objectContaining({ + OAUTH_TOKEN_github: 'gh-retry-token', + OAUTH_TOKEN_gitlab: 'gl-retry-token', + }), + ); + }); + }); + + it('should show Retry only for error rows, not for authenticated or pending', async () => { + let callCount = 0; + mockAuthenticate.mockImplementation(() => { + callCount++; + if (callCount === 1) { + return Promise.resolve([{ token: 'gh-token', provider: 'github' }]); + } + return Promise.reject(new Error('GitLab auth failed')); + }); + + const { container } = await renderComponent({ + formContext: { + formData: { csvContent: MIXED_CSV }, + }, + }); + + await waitFor(() => { + expect(screen.getByText('GitLab auth failed')).toBeInTheDocument(); + }); + + const rows = container.querySelectorAll('tbody tr'); + expect(rows).toHaveLength(2); + + const githubRow = rows[0] as HTMLElement; + const gitlabRow = rows[1] as HTMLElement; + + expect(within(githubRow).queryByText('Retry')).not.toBeInTheDocument(); + expect(within(gitlabRow).getByText('Retry')).toBeInTheDocument(); + }); + + it('should display per-provider error messages inline in each row', async () => { + mockAuthenticate.mockImplementation(descriptors => { + const provider = descriptors[0]?.provider ?? 'unknown'; + return Promise.reject(new Error(`${provider} auth failed`)); + }); + + const { container } = await renderComponent({ + formContext: { + formData: { csvContent: MIXED_CSV }, + }, + }); + + await waitFor(() => { + expect(mockAuthenticate).toHaveBeenCalledTimes(2); + }); + + const rows = container.querySelectorAll('tbody tr'); + expect(rows).toHaveLength(2); + + const githubRow = rows[0] as HTMLElement; + const gitlabRow = rows[1] as HTMLElement; + + expect( + within(githubRow).getByText('github auth failed'), + ).toBeInTheDocument(); + expect( + within(gitlabRow).getByText('gitlab auth failed'), + ).toBeInTheDocument(); + }); + + it('should retain other provider errors when retrying one provider', async () => { + mockAuthenticate.mockImplementation(descriptors => { + const provider = descriptors[0]?.provider ?? 'unknown'; + return Promise.reject(new Error(`${provider} auth failed`)); + }); + + const { container } = await renderComponent({ + formContext: { + formData: { csvContent: MIXED_CSV }, + }, + }); + + await waitFor(() => { + expect(screen.getByText('github auth failed')).toBeInTheDocument(); + expect(screen.getByText('gitlab auth failed')).toBeInTheDocument(); + }); + + mockAuthenticate.mockImplementation( + () => new Promise(() => {}), // never resolves + ); + + const rows = container.querySelectorAll('tbody tr'); + const githubRetry = within(rows[0] as HTMLElement).getByText('Retry'); + + await act(async () => { + await userEvent.click(githubRetry); + await flushAsync(); + }); + + // github error cleared, gitlab error remains + await waitFor(() => { + expect( + screen.queryByText('github auth failed'), + ).not.toBeInTheDocument(); + }); + expect(screen.getByText('gitlab auth failed')).toBeInTheDocument(); + }); + }); + + describe('provider table', () => { + it('should render a table with provider name, access, and scope for a single provider', async () => { + await renderComponent({ + formContext: { + formData: { csvContent: VALID_CSV }, + }, + }); + + await waitFor(() => { + expect(screen.getByText('github')).toBeInTheDocument(); + }); + + expect(screen.getByText('Read / Write')).toBeInTheDocument(); + expect(screen.getByText('repo')).toBeInTheDocument(); + }); + + it('should render rows for multiple distinct providers', async () => { + mockAuthenticate.mockImplementation(descriptors => { + const provider = descriptors[0]?.provider ?? 'unknown'; + return Promise.resolve([{ token: `token-for-${provider}`, provider }]); + }); + + await renderComponent({ + formContext: { + formData: { csvContent: MIXED_CSV }, + }, + }); + + await waitFor(() => { + expect(screen.getByText('github')).toBeInTheDocument(); + expect(screen.getByText('gitlab')).toBeInTheDocument(); + }); + + const rows = screen.getAllByRole('row'); + // header + 2 provider rows + expect(rows).toHaveLength(3); + }); + + it('should show Read-only access for source-only providers', async () => { + mockAuthenticate.mockImplementation(descriptors => { + const provider = descriptors[0]?.provider ?? 'unknown'; + return Promise.resolve([{ token: `token-for-${provider}`, provider }]); + }); + + await renderComponent({ + formContext: { + formData: { csvContent: CROSS_PROVIDER_CSV }, + }, + }); + + await waitFor(() => { + expect(screen.getByText('gitlab')).toBeInTheDocument(); + expect(screen.getByText('github')).toBeInTheDocument(); + }); + + // gitlab is target → Read / Write; github is source-only → Read-only + expect(screen.getByText('Read / Write')).toBeInTheDocument(); + expect(screen.getByText('Read-only')).toBeInTheDocument(); + + // gitlab target scope is write_repository; github source scope is repo + expect(screen.getByText('write_repository')).toBeInTheDocument(); + expect(screen.getByText('repo')).toBeInTheDocument(); + }); + + it('should not render the table when no CSV content is provided', async () => { + await renderComponent({ + formContext: { formData: {} }, + }); + + expect(screen.queryByRole('table')).not.toBeInTheDocument(); + }); + + it('should show correct OAuth scope per provider and access level', async () => { + await renderComponent({ + formContext: { + formData: { csvContent: VALID_CSV }, + }, + }); + + await waitFor(() => { + // github target scope is "repo" + expect(screen.getByText('repo')).toBeInTheDocument(); + }); + }); + }); + + describe('provider status tracking', () => { + it('should show authenticated status after successful auth', async () => { + const { container } = await renderComponent({ + formContext: { + formData: { csvContent: VALID_CSV }, + }, + }); + + await waitFor(() => { + expect(mockSetSecrets).toHaveBeenCalled(); + }); + + const rows = container.querySelectorAll('tbody tr'); + expect(rows).toHaveLength(1); + const statusCell = within(rows[0] as HTMLElement).getAllByRole('cell')[3]; + // StatusOK renders with the status--ok class + expect(statusCell.querySelector('[class*="status"]')).toBeInTheDocument(); + }); + + it('should show error status when authentication fails', async () => { + mockAuthenticate.mockRejectedValue(new Error('Auth failed')); + + const { container } = await renderComponent({ + formContext: { + formData: { csvContent: VALID_CSV }, + }, + }); + + await waitFor(() => { + expect(screen.getByText('Auth failed')).toBeInTheDocument(); + }); + + const rows = container.querySelectorAll('tbody tr'); + expect(rows).toHaveLength(1); + const statusCell = within(rows[0] as HTMLElement).getAllByRole('cell')[3]; + expect(statusCell.querySelector('[class*="status"]')).toBeInTheDocument(); + }); + + it('should show mixed statuses when one provider succeeds and another fails', async () => { + let callCount = 0; + mockAuthenticate.mockImplementation(() => { + callCount++; + if (callCount === 1) { + return Promise.resolve([{ token: 'gh-token', provider: 'github' }]); + } + return Promise.reject(new Error('GitLab auth failed')); + }); + + const { container } = await renderComponent({ + formContext: { + formData: { csvContent: MIXED_CSV }, + }, + }); + + await waitFor(() => { + expect(mockAuthenticate).toHaveBeenCalledTimes(2); + }); + + await waitFor(() => { + expect(screen.getByText('GitLab auth failed')).toBeInTheDocument(); + }); + + const rows = container.querySelectorAll('tbody tr'); + expect(rows).toHaveLength(2); + }); + + it('should reset provider statuses when CSV content changes', async () => { + const onChange = jest.fn(); + const props = makeProps({ + onChange, + formContext: { formData: { csvContent: VALID_CSV } }, + }); + + let result!: ReturnType; + await act(async () => { + result = render( + + + , + ); + await flushAsync(); + }); + + await waitFor(() => { + expect(onChange).toHaveBeenCalledWith('authenticated'); + }); + + // Table has 1 provider row with authenticated status + expect(screen.getByText('github')).toBeInTheDocument(); + + const newCsv = toDataUrl( + 'name,abbreviation,sourceRepoUrl,sourceRepoBranch,targetRepoBranch\n' + + 'New Project,NP,https://gitlab.com/org/new-repo,main,main', + ); + + mockAuthenticate.mockResolvedValue([ + { token: 'new-token', provider: 'gitlab' }, + ]); + + await act(async () => { + result.rerender( + + + , + ); + await flushAsync(); + }); + + // After CSV change, onChange is called with undefined (reset) + await waitFor(() => { + expect(onChange).toHaveBeenCalledWith(undefined); + }); + + // New provider appears in the table + await waitFor(() => { + expect(screen.getByText('gitlab')).toBeInTheDocument(); + }); + }); + }); + + describe('concurrent auth failure resilience', () => { + it('should not strand providers as pending when one fails while another is in-flight', async () => { + let resolveGithub!: (value: unknown) => void; + + mockAuthenticate.mockImplementation(descriptors => { + const provider = descriptors[0]?.provider ?? 'unknown'; + if (provider === 'github') { + return new Promise(r => { + resolveGithub = r; + }); + } + return Promise.reject(new Error('GitLab auth failed')); + }); + + const onChange = jest.fn(); + await renderComponent({ + onChange, + formContext: { + formData: { csvContent: MIXED_CSV }, + }, + }); + + // GitLab fails immediately; GitHub is still in-flight + await waitFor(() => { + expect(screen.getByText('GitLab auth failed')).toBeInTheDocument(); + }); + + // Now resolve GitHub — it should NOT be stranded as pending + await act(async () => { + resolveGithub([{ token: 'gh-token', provider: 'github' }]); + await flushAsync(); + }); + + // GitHub should become authenticated, not stuck at pending + const { container } = { + container: screen.getByRole('table').closest('div')!.parentElement!, + }; + const rows = container.querySelectorAll('tbody tr'); + const githubRow = rows[0] as HTMLElement; + + await waitFor(() => { + expect(within(githubRow).queryByText('Retry')).not.toBeInTheDocument(); + }); + + // GitLab should have a Retry button + const gitlabRow = rows[1] as HTMLElement; + expect(within(gitlabRow).getByText('Retry')).toBeInTheDocument(); + + // Retry GitLab to complete everything + mockAuthenticate.mockResolvedValue([ + { token: 'gl-token', provider: 'gitlab' }, + ]); + await act(async () => { + await userEvent.click(within(gitlabRow).getByText('Retry')); + await flushAsync(); + }); + + await waitFor(() => { + expect(onChange).toHaveBeenCalledWith('authenticated'); + }); + }); + }); + + describe('retry staleness', () => { + it('should discard retry results when CSV changes during a retry', async () => { + mockAuthenticate.mockRejectedValueOnce(new Error('First failure')); + + const onChange = jest.fn(); + const props = makeProps({ + onChange, + formContext: { formData: { csvContent: VALID_CSV } }, + }); + + let result!: ReturnType; + await act(async () => { + result = render( + + + , + ); + await flushAsync(); + }); + + await waitFor(() => { + expect(screen.getByText('Retry')).toBeInTheDocument(); + }); + + // Start a retry that will hang (never resolve yet) + let resolveRetry!: (value: unknown) => void; + mockAuthenticate.mockImplementation( + () => + new Promise(r => { + resolveRetry = r; + }), + ); + + await act(async () => { + await userEvent.click(screen.getByText('Retry')); + await flushAsync(); + }); + + // Now change CSV before the retry resolves + const newCsv = toDataUrl( + 'name,abbreviation,sourceRepoUrl,sourceRepoBranch,targetRepoBranch\n' + + 'New Project,NP,https://gitlab.com/org/new-repo,main,main', + ); + + mockAuthenticate.mockResolvedValue([ + { token: 'new-token', provider: 'gitlab' }, + ]); + + await act(async () => { + result.rerender( + + + , + ); + await flushAsync(); + }); + + // Resolve the stale retry — it should be discarded + await act(async () => { + resolveRetry([{ token: 'stale-token', provider: 'github' }]); + await flushAsync(); + }); + + // The new CSV's provider (gitlab) should be authenticated, not github + await waitFor(() => { + expect(mockSetSecrets).toHaveBeenCalledWith( + expect.objectContaining({ + OAUTH_TOKEN_gitlab: 'new-token', + }), + ); + }); + + // The stale github token should NOT be in secrets + expect(mockSetSecrets).not.toHaveBeenCalledWith( + expect.objectContaining({ + OAUTH_TOKEN_github: 'stale-token', + }), + ); + }); + }); + + describe('unmount safety', () => { + it('should not update state after unmount', async () => { + let resolveAuth!: (value: unknown) => void; + mockAuthenticate.mockImplementation( + () => + new Promise(r => { + resolveAuth = r; + }), + ); + + const onChange = jest.fn(); + const { unmount } = await renderComponent({ + onChange, + formContext: { + formData: { csvContent: VALID_CSV }, + }, + }); + + // Auth is in-flight, unmount the component + unmount(); + + // Resolve auth after unmount — should not throw or warn + await act(async () => { + resolveAuth([{ token: 'post-unmount-token', provider: 'github' }]); + await flushAsync(); + }); + + // onChange should never have been called with 'authenticated' + expect(onChange).not.toHaveBeenCalledWith('authenticated'); + expect(mockSetSecrets).not.toHaveBeenCalled(); + }); + }); + + describe('csvFieldName', () => { + it('should read CSV from the field specified by csvFieldName', async () => { + await renderComponent({ + uiSchema: { 'ui:options': { csvFieldName: 'myCustomCsv' } }, + formContext: { + formData: { myCustomCsv: VALID_CSV }, + }, + }); + + await waitFor(() => { + expect(mockAuthenticate).toHaveBeenCalled(); + }); + }); + + it('should not attempt auth when csvFieldName points to empty field', async () => { + await renderComponent({ + uiSchema: { 'ui:options': { csvFieldName: 'myCustomCsv' } }, + formContext: { + formData: { csvContent: VALID_CSV }, + }, + }); + + expect(mockAuthenticate).not.toHaveBeenCalled(); + }); + }); + + describe('non-Error exception handling', () => { + it('should display "Unknown error" when auto-auth throws a non-Error value', async () => { + mockAuthenticate.mockRejectedValue('string-error'); + + await renderComponent({ + formContext: { + formData: { csvContent: VALID_CSV }, + }, + }); + + await waitFor(() => { + expect(screen.getByText('Unknown error')).toBeInTheDocument(); + }); + }); + + it('should display "Unknown error" when retry throws a non-Error value', async () => { + mockAuthenticate.mockRejectedValueOnce(new Error('Initial failure')); + + await renderComponent({ + formContext: { + formData: { csvContent: VALID_CSV }, + }, + }); + + await waitFor(() => { + expect(screen.getByText('Retry')).toBeInTheDocument(); + }); + + mockAuthenticate.mockRejectedValue(42); + + await act(async () => { + await userEvent.click(screen.getByText('Retry')); + await flushAsync(); + }); + + await waitFor(() => { + expect(screen.getByText('Unknown error')).toBeInTheDocument(); + }); + }); + }); + + describe('CSV lifecycle edge cases', () => { + it('should reset state and show "CSV content is required" when CSV is cleared', async () => { + const onChange = jest.fn(); + const props = makeProps({ + onChange, + formContext: { formData: { csvContent: VALID_CSV } }, + }); + + let result!: ReturnType; + await act(async () => { + result = render( + + + , + ); + await flushAsync(); + }); + + await waitFor(() => { + expect(onChange).toHaveBeenCalledWith('authenticated'); + }); + expect(screen.getByRole('table')).toBeInTheDocument(); + + // Clear the CSV + await act(async () => { + result.rerender( + + + , + ); + await flushAsync(); + }); + + expect(onChange).toHaveBeenCalledWith(undefined); + expect(screen.queryByRole('table')).not.toBeInTheDocument(); + expect( + screen.getByText( + 'CSV content is required for RepoAuthentication extension', + ), + ).toBeInTheDocument(); + }); + + it('should re-authenticate from scratch when CSV is cleared then restored', async () => { + const onChange = jest.fn(); + const props = makeProps({ + onChange, + formContext: { formData: { csvContent: VALID_CSV } }, + }); + + let result!: ReturnType; + await act(async () => { + result = render( + + + , + ); + await flushAsync(); + }); + + await waitFor(() => { + expect(onChange).toHaveBeenCalledWith('authenticated'); + }); + expect(mockAuthenticate).toHaveBeenCalledTimes(1); + + // Clear CSV + await act(async () => { + result.rerender( + + + , + ); + await flushAsync(); + }); + + mockAuthenticate.mockResolvedValue([ + { token: 'restored-token', provider: 'github' }, + ]); + + // Restore the same CSV + await act(async () => { + result.rerender( + + + , + ); + await flushAsync(); + }); + + await waitFor(() => { + expect(mockAuthenticate).toHaveBeenCalledTimes(2); + }); + + await waitFor(() => { + expect(mockSetSecrets).toHaveBeenLastCalledWith( + expect.objectContaining({ + OAUTH_TOKEN_github: 'restored-token', + }), + ); + }); + }); + + it('should cancel in-flight auto-auth when CSV changes mid-authentication', async () => { + let resolveFirstAuth!: (value: unknown) => void; + mockAuthenticate.mockImplementation( + () => + new Promise(r => { + resolveFirstAuth = r; + }), + ); + + const onChange = jest.fn(); + const props = makeProps({ + onChange, + formContext: { formData: { csvContent: VALID_CSV } }, + }); + + let result!: ReturnType; + await act(async () => { + result = render( + + + , + ); + await flushAsync(); + }); + + // Auth is in-flight for first CSV. Change CSV now. + const newCsv = toDataUrl( + 'name,abbreviation,sourceRepoUrl,sourceRepoBranch,targetRepoBranch\n' + + 'New Project,NP,https://gitlab.com/org/new-repo,main,main', + ); + + mockAuthenticate.mockResolvedValue([ + { token: 'new-csv-token', provider: 'gitlab' }, + ]); + + await act(async () => { + result.rerender( + + + , + ); + await flushAsync(); + }); + + // Resolve the stale first auth — it should be discarded + await act(async () => { + resolveFirstAuth([{ token: 'stale-token', provider: 'github' }]); + await flushAsync(); + }); + + // New CSV should authenticate successfully + await waitFor(() => { + expect(mockSetSecrets).toHaveBeenCalledWith( + expect.objectContaining({ + OAUTH_TOKEN_gitlab: 'new-csv-token', + }), + ); + }); + + // Stale token should not appear + expect(mockSetSecrets).not.toHaveBeenCalledWith( + expect.objectContaining({ + OAUTH_TOKEN_github: 'stale-token', + }), + ); + }); + + it('should not reset when the same csvContent reference is re-rendered', async () => { + const onChange = jest.fn(); + const csvData = { csvContent: VALID_CSV }; + const props = makeProps({ + onChange, + formContext: { formData: csvData }, + }); + + let result!: ReturnType; + await act(async () => { + result = render( + + + , + ); + await flushAsync(); + }); + + await waitFor(() => { + expect(onChange).toHaveBeenCalledWith('authenticated'); + }); + expect(mockAuthenticate).toHaveBeenCalledTimes(1); + + // Re-render with the same CSV content — should NOT re-auth + onChange.mockClear(); + mockAuthenticate.mockClear(); + + await act(async () => { + result.rerender( + + + , + ); + await flushAsync(); + }); + + expect(mockAuthenticate).not.toHaveBeenCalled(); + expect(onChange).not.toHaveBeenCalledWith(undefined); + }); + + it('should reset initialAuthFailed and re-auth after CSV changes following a failure', async () => { + mockAuthenticate.mockRejectedValue(new Error('All fail')); + + const onChange = jest.fn(); + const props = makeProps({ + onChange, + formContext: { formData: { csvContent: MIXED_CSV } }, + }); + + let result!: ReturnType; + await act(async () => { + result = render( + + + , + ); + await flushAsync(); + }); + + await waitFor(() => { + expect(screen.getAllByText('All fail')).toHaveLength(2); + }); + + // Now change CSV — should clear errors and re-authenticate + mockAuthenticate.mockResolvedValue([ + { token: 'fresh-token', provider: 'github' }, + ]); + + await act(async () => { + result.rerender( + + + , + ); + await flushAsync(); + }); + + await waitFor(() => { + expect(mockSetSecrets).toHaveBeenCalledWith( + expect.objectContaining({ + OAUTH_TOKEN_github: 'fresh-token', + }), + ); + }); + + // No error messages should remain + expect(screen.queryByText('All fail')).not.toBeInTheDocument(); + }); + }); + + describe('completion effect edge cases', () => { + it('should merge provider tokens with pre-existing secrets', async () => { + const scaffolderMock = jest.requireMock( + '@backstage/plugin-scaffolder-react', + ); + const origUseTemplateSecrets = scaffolderMock.useTemplateSecrets; + scaffolderMock.useTemplateSecrets = () => ({ + secrets: { EXISTING_SECRET: 'keep-me' }, + setSecrets: mockSetSecrets, + }); + + try { + await renderComponent({ + formContext: { + formData: { csvContent: VALID_CSV }, + }, + }); + + await waitFor(() => { + expect(mockSetSecrets).toHaveBeenCalledWith( + expect.objectContaining({ + EXISTING_SECRET: 'keep-me', + OAUTH_TOKEN_github: 'mock-augmented-token', + }), + ); + }); + } finally { + scaffolderMock.useTemplateSecrets = origUseTemplateSecrets; + } + }); + + it('should not call setSecrets twice when component re-renders after completion', async () => { + const onChange = jest.fn(); + const props = makeProps({ + onChange, + formContext: { formData: { csvContent: VALID_CSV } }, + }); + + let result!: ReturnType; + await act(async () => { + result = render( + + + , + ); + await flushAsync(); + }); + + await waitFor(() => { + expect(mockSetSecrets).toHaveBeenCalledTimes(1); + }); + + // Force a re-render with the same props (simulates parent re-render) + await act(async () => { + result.rerender( + + + , + ); + await flushAsync(); + }); + + // setSecrets should still be called exactly once + expect(mockSetSecrets).toHaveBeenCalledTimes(1); + }); + + it('should not call onChange("authenticated") after isDone is set', async () => { + const onChange = jest.fn(); + await renderComponent({ + onChange, + formContext: { + formData: { csvContent: VALID_CSV }, + }, + }); + + await waitFor(() => { + expect(onChange).toHaveBeenCalledWith('authenticated'); + }); + + const authenticatedCalls = onChange.mock.calls.filter( + (args: unknown[]) => args[0] === 'authenticated', + ); + expect(authenticatedCalls).toHaveLength(1); + }); + }); + + describe('provider deduplication', () => { + it('should call authenticate once per distinct provider even with multiple CSV rows', async () => { + const multiRowSameProvider = toDataUrl( + 'name,abbreviation,sourceRepoUrl,sourceRepoBranch,targetRepoBranch\n' + + 'Project A,PA,https://github.com/org/repo-a,main,main\n' + + 'Project B,PB,https://github.com/org/repo-b,main,main\n' + + 'Project C,PC,https://github.com/org/repo-c,main,main', + ); + + await renderComponent({ + formContext: { + formData: { csvContent: multiRowSameProvider }, + }, + }); + + await waitFor(() => { + expect(mockAuthenticate).toHaveBeenCalledTimes(1); + }); + + await waitFor(() => { + expect(mockSetSecrets).toHaveBeenCalled(); + }); + + // Table should show only 1 provider row, not 3 + const rows = screen.getAllByRole('row'); + expect(rows).toHaveLength(2); // header + 1 provider + }); + }); + + describe('retry with deferred resolution', () => { + it('should complete when a slow retry eventually resolves', async () => { + mockAuthenticate.mockRejectedValueOnce(new Error('First failure')); + + await renderComponent({ + formContext: { + formData: { csvContent: VALID_CSV }, + }, + }); + + await waitFor(() => { + expect(screen.getByText('Retry')).toBeInTheDocument(); + }); + + let resolveRetry!: (value: unknown) => void; + mockAuthenticate.mockImplementationOnce( + () => + new Promise(r => { + resolveRetry = r; + }), + ); + + await act(async () => { + await userEvent.click(screen.getByText('Retry')); + await flushAsync(); + }); + + // Provider is 'pending' — Retry button is hidden + expect(screen.queryByText('Retry')).not.toBeInTheDocument(); + + // Resolve the deferred retry + await act(async () => { + resolveRetry([{ token: 'deferred-token', provider: 'github' }]); + await flushAsync(); + }); + + await waitFor(() => { + expect(mockSetSecrets).toHaveBeenCalledWith( + expect.objectContaining({ + OAUTH_TOKEN_github: 'deferred-token', + }), + ); + }); }); }); }); diff --git a/workspaces/x2a/plugins/x2a/src/scaffolder/RepoAuthentication.tsx b/workspaces/x2a/plugins/x2a/src/scaffolder/RepoAuthentication.tsx index 148bde6934..3e79a08781 100644 --- a/workspaces/x2a/plugins/x2a/src/scaffolder/RepoAuthentication.tsx +++ b/workspaces/x2a/plugins/x2a/src/scaffolder/RepoAuthentication.tsx @@ -13,23 +13,54 @@ * See the License for the specific language governing permissions and limitations under the License. */ -import { useEffect, useRef, useState } from 'react'; - import { type CustomFieldValidator, FieldExtensionComponentProps, useTemplateSecrets, } from '@backstage/plugin-scaffolder-react'; import { - resolveScmProvider, - parseCsvContent, - ScmProvider, - SCAFFOLDER_SECRET_PREFIX, -} from '@red-hat-developer-hub/backstage-plugin-x2a-common'; -import { Button, Typography } from '@material-ui/core'; + Button, + Table, + TableBody, + TableCell, + TableContainer, + TableHead, + TableRow, + Typography, +} from '@material-ui/core'; +import { makeStyles } from '@material-ui/core/styles'; +import { + StatusError, + StatusOK, + StatusPending, +} from '@backstage/core-components'; import { useScmHostMap } from '../hooks/useScmHostMap'; import { useRepoAuthentication } from '../repoAuth'; +import { type ProviderAuthStatus, useProviderAuth } from './useProviderAuth'; + +const useStyles = makeStyles(theme => ({ + errorText: { + color: theme.palette.error.main, + }, +})); + +const StatusIcon = ({ + status, + children, +}: { + status: ProviderAuthStatus; + children?: React.ReactNode; +}) => { + switch (status) { + case 'authenticated': + return {children}; + case 'error': + return {children}; + default: + return {children}; + } +}; /** * RepoAuthentication extension requests authentication tokens for all the SCM providers @@ -43,15 +74,10 @@ export const RepoAuthentication = ({ uiSchema, schema, }: FieldExtensionComponentProps) => { + const classes = useStyles(); const hostProviderMap = useScmHostMap(); const { secrets, setSecrets } = useTemplateSecrets(); - const repoAuthentication = useRepoAuthentication(); - const [error, setError] = useState(); - const [suppressDialog, setSuppressDialog] = useState(false); - const [isDone, setDone] = useState(false); - - const secretsRef = useRef(secrets); - secretsRef.current = secrets; + const { authenticate } = useRepoAuthentication(); const { title, description } = schema; const csvFieldName = @@ -60,143 +86,72 @@ export const RepoAuthentication = ({ ? formContext?.formData?.[csvFieldName] : undefined; - const prevCsvRef = useRef(csvContent); - - useEffect(() => { - if (csvContent !== prevCsvRef.current) { - prevCsvRef.current = csvContent; - setDone(false); - setSuppressDialog(false); - setError(undefined); - onChange(undefined); - } - }, [csvContent, onChange]); - - useEffect(() => { - if (!csvContent || suppressDialog || isDone) { - return undefined; - } - - setError(undefined); - - let projectsToCreate; - try { - projectsToCreate = parseCsvContent(csvContent); - } catch (e) { - setError(e instanceof Error ? e.message : 'Unknown error'); - return undefined; - } - - const allTargetProviders: ScmProvider[] = projectsToCreate.map(project => - resolveScmProvider(project.targetRepoUrl, hostProviderMap), - ); - const allSourceProviders: ScmProvider[] = projectsToCreate.map(project => - resolveScmProvider(project.sourceRepoUrl, hostProviderMap), - ); - const distinctTargetProviders = allTargetProviders.filter( - (p, i, arr) => arr.findIndex(q => q.name === p.name) === i, - ); - const distinctSourceProviders = allSourceProviders.filter( - (p, i, arr) => - arr.findIndex(q => q.name === p.name) === i && - !distinctTargetProviders.some(t => t.name === p.name), - ); - const allDistinctProviders = [ - ...distinctTargetProviders, - ...distinctSourceProviders, - ]; - - let cancelled = false; - const authCsvSnapshot = csvContent; - - const doAuthAsync = async () => { - const providerTokens = new Map(); - - const authenticateProvider = async ( - provider: ScmProvider, - readOnly: boolean, - ) => { - try { - const tokens = await repoAuthentication.authenticate([ - provider.getAuthTokenDescriptor(readOnly), - ]); - if (cancelled) { - return; - } - providerTokens.set( - `${SCAFFOLDER_SECRET_PREFIX}${provider.name}`, - tokens[0].token, - ); - } catch (e) { - if (cancelled) { - return; - } - setError(e instanceof Error ? e.message : 'Unknown error'); - setSuppressDialog(true); - } - }; - - await Promise.all([ - ...distinctTargetProviders.map(p => authenticateProvider(p, false)), - ...distinctSourceProviders.map(p => authenticateProvider(p, true)), - ]); - - if ( - cancelled || - authCsvSnapshot !== prevCsvRef.current || - providerTokens.size !== allDistinctProviders.length - ) { - if ( - !cancelled && - authCsvSnapshot === prevCsvRef.current && - providerTokens.size !== allDistinctProviders.length - ) { - onChange(undefined); - } - return; - } - - onChange('authenticated'); - setDone(true); - setSecrets({ - ...secretsRef.current, - ...Object.fromEntries(providerTokens), - }); - }; - - doAuthAsync(); - - return () => { - cancelled = true; - }; - }, [ + const { providerRows, retryProvider, parseError } = useProviderAuth({ csvContent, hostProviderMap, - repoAuthentication, - setSecrets, - suppressDialog, - isDone, + authenticate, onChange, - ]); + secrets: secrets as Record, + setSecrets, + }); return ( <> {title} {description} - {suppressDialog && !isDone && ( - - - + + {providerRows.length > 0 && ( + + + + + Provider + Access + OAuth scope + Status + + + + + {providerRows.map(row => ( + + {row.provider.name} + + {row.readOnly ? 'Read-only' : 'Read / Write'} + + + {Array.isArray(row.scope) + ? row.scope.join(', ') + : row.scope} + + + + {row.error && ( + {row.error} + )} + + + + {row.status === 'error' && ( + + )} + + + ))} + +
+ )} + {!csvFieldName && ( CSV field name is required for RepoAuthentication extension @@ -207,9 +162,9 @@ export const RepoAuthentication = ({ CSV content is required for RepoAuthentication extension )} - {error && ( + {parseError && ( - {error} + {parseError} )} diff --git a/workspaces/x2a/plugins/x2a/src/scaffolder/useProviderAuth.ts b/workspaces/x2a/plugins/x2a/src/scaffolder/useProviderAuth.ts new file mode 100644 index 0000000000..f67534e9d6 --- /dev/null +++ b/workspaces/x2a/plugins/x2a/src/scaffolder/useProviderAuth.ts @@ -0,0 +1,297 @@ +/** + * Copyright Red Hat, 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 { useCallback, useEffect, useMemo, useRef, useState } from 'react'; + +import { + resolveScmProvider, + parseCsvContent, + ScmProvider, + SCAFFOLDER_SECRET_PREFIX, + type AuthToken, + type AuthTokenDescriptor, + type ScmProviderName, +} from '@red-hat-developer-hub/backstage-plugin-x2a-common'; + +export type ProviderAuthStatus = 'pending' | 'authenticated' | 'error'; + +export type ProviderRow = { + provider: ScmProvider; + readOnly: boolean; + scope: string | string[]; + status: ProviderAuthStatus; + error?: string; +}; + +interface UseProviderAuthParams { + csvContent: string | undefined; + hostProviderMap: Map; + authenticate: ( + descriptors: AuthTokenDescriptor[], + ) => Promise>; + onChange: (value: string | undefined) => void; + secrets: Record; + setSecrets: (secrets: Record) => void; +} + +export function useProviderAuth({ + csvContent, + hostProviderMap, + authenticate, + onChange, + secrets, + setSecrets, +}: UseProviderAuthParams) { + const [providerStatuses, setProviderStatuses] = useState< + Record + >({}); + const [providerErrors, setProviderErrors] = useState>( + {}, + ); + const [isDone, setDone] = useState(false); + + const onChangeRef = useRef(onChange); + onChangeRef.current = onChange; + const authenticateRef = useRef(authenticate); + authenticateRef.current = authenticate; + const secretsRef = useRef(secrets); + secretsRef.current = secrets; + const providerTokensRef = useRef>({}); + const prevCsvRef = useRef(csvContent); + + // Ref-based flag: locks out auto-auth after any provider fails during + // the initial run. Using a ref (not state) avoids re-triggering the + // auto-auth effect and cancelling other in-flight provider auths. + const initialAuthFailedRef = useRef(false); + + // Bumped on every CSV change and on unmount so stale async results + // (from auto-auth or retry) are silently discarded. + const authGenerationRef = useRef(0); + + const { distinctTargetProviders, distinctSourceProviders, parseError } = + useMemo(() => { + if (!csvContent) { + return { + distinctTargetProviders: [] as ScmProvider[], + distinctSourceProviders: [] as ScmProvider[], + parseError: undefined, + }; + } + + try { + const projectsToCreate = parseCsvContent(csvContent); + + const allTargetProviders: ScmProvider[] = projectsToCreate.map( + project => resolveScmProvider(project.targetRepoUrl, hostProviderMap), + ); + const allSourceProviders: ScmProvider[] = projectsToCreate.map( + project => resolveScmProvider(project.sourceRepoUrl, hostProviderMap), + ); + const targets = allTargetProviders.filter( + (p, i, arr) => arr.findIndex(q => q.name === p.name) === i, + ); + const sources = allSourceProviders.filter( + (p, i, arr) => + arr.findIndex(q => q.name === p.name) === i && + !targets.some(t => t.name === p.name), + ); + return { + distinctTargetProviders: targets, + distinctSourceProviders: sources, + parseError: undefined, + }; + } catch (e) { + return { + distinctTargetProviders: [] as ScmProvider[], + distinctSourceProviders: [] as ScmProvider[], + parseError: e instanceof Error ? e.message : 'Unknown error', + }; + } + }, [csvContent, hostProviderMap]); + + const providerRows: ProviderRow[] = useMemo(() => { + const rows: ProviderRow[] = []; + for (const p of distinctTargetProviders) { + const desc = p.getAuthTokenDescriptor(false); + rows.push({ + provider: p, + readOnly: false, + scope: desc.scope ?? '', + status: providerStatuses[p.name] ?? 'pending', + error: providerErrors[p.name], + }); + } + for (const p of distinctSourceProviders) { + const desc = p.getAuthTokenDescriptor(true); + rows.push({ + provider: p, + readOnly: true, + scope: desc.scope ?? '', + status: providerStatuses[p.name] ?? 'pending', + error: providerErrors[p.name], + }); + } + return rows; + }, [ + distinctTargetProviders, + distinctSourceProviders, + providerStatuses, + providerErrors, + ]); + + // Reset all auth state when CSV content changes. + useEffect(() => { + if (csvContent !== prevCsvRef.current) { + prevCsvRef.current = csvContent; + authGenerationRef.current++; + initialAuthFailedRef.current = false; + setDone(false); + setProviderStatuses({}); + setProviderErrors({}); + providerTokensRef.current = {}; + onChangeRef.current(undefined); + } + }, [csvContent]); + + // Auto-authenticate all providers on first render or CSV change. + useEffect(() => { + if (!csvContent || initialAuthFailedRef.current || isDone || parseError) { + return undefined; + } + + const targets = distinctTargetProviders; + const sources = distinctSourceProviders; + const allDistinctProviders = [...targets, ...sources]; + + if (allDistinctProviders.length === 0) { + return undefined; + } + + let cancelled = false; + + const doAuthAsync = async () => { + const authenticateProvider = async ( + provider: ScmProvider, + readOnly: boolean, + ) => { + try { + const tokens = await authenticateRef.current([ + provider.getAuthTokenDescriptor(readOnly), + ]); + if (cancelled) return; + + providerTokensRef.current[ + `${SCAFFOLDER_SECRET_PREFIX}${provider.name}` + ] = tokens[0].token; + setProviderStatuses(prev => ({ + ...prev, + [provider.name]: 'authenticated', + })); + } catch (e) { + if (cancelled) return; + + setProviderStatuses(prev => ({ + ...prev, + [provider.name]: 'error', + })); + setProviderErrors(prev => ({ + ...prev, + [provider.name]: e instanceof Error ? e.message : 'Unknown error', + })); + initialAuthFailedRef.current = true; + } + }; + + await Promise.all([ + ...targets.map(p => authenticateProvider(p, false)), + ...sources.map(p => authenticateProvider(p, true)), + ]); + + if (cancelled) return; + + const tokenCount = Object.keys(providerTokensRef.current).length; + if (tokenCount !== allDistinctProviders.length) { + onChangeRef.current(undefined); + } + }; + + doAuthAsync(); + + return () => { + cancelled = true; + }; + // csvContent is the meaningful trigger for re-authentication. + // distinctTarget/SourceProviders are derived from it and captured above. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [csvContent, isDone, parseError]); + + // Complete: all providers authenticated → set secrets and notify parent. + useEffect(() => { + if (isDone || providerRows.length === 0) return; + if (!providerRows.every(r => r.status === 'authenticated')) return; + + onChangeRef.current('authenticated'); + setDone(true); + setSecrets({ + ...secretsRef.current, + ...providerTokensRef.current, + }); + }, [providerRows, isDone, setSecrets]); + + const retryProvider = useCallback( + async (provider: ScmProvider, readOnly: boolean) => { + const generation = authGenerationRef.current; + + setProviderStatuses(prev => ({ + ...prev, + [provider.name]: 'pending', + })); + setProviderErrors(prev => { + const { [provider.name]: _, ...rest } = prev; + return rest; + }); + + try { + const tokens = await authenticateRef.current([ + provider.getAuthTokenDescriptor(readOnly), + ]); + + if (generation !== authGenerationRef.current) return; + + providerTokensRef.current[ + `${SCAFFOLDER_SECRET_PREFIX}${provider.name}` + ] = tokens[0].token; + setProviderStatuses(prev => ({ + ...prev, + [provider.name]: 'authenticated', + })); + } catch (e) { + if (generation !== authGenerationRef.current) return; + + setProviderStatuses(prev => ({ + ...prev, + [provider.name]: 'error', + })); + setProviderErrors(prev => ({ + ...prev, + [provider.name]: e instanceof Error ? e.message : 'Unknown error', + })); + } + }, + [], + ); + + return { providerRows, retryProvider, parseError }; +} diff --git a/workspaces/x2a/templates/conversion-project-template.yaml b/workspaces/x2a/templates/conversion-project-template.yaml index 761a7f8275..7a6952bd65 100644 --- a/workspaces/x2a/templates/conversion-project-template.yaml +++ b/workspaces/x2a/templates/conversion-project-template.yaml @@ -153,6 +153,8 @@ spec: inputMethod: const: csv then: + required: + - repoAuthentication properties: repoAuthentication: type: string From 86902b6c49b6bda105cd89ff1fb13aaa146b7b7f Mon Sep 17 00:00:00 2001 From: Marek Libra Date: Tue, 24 Mar 2026 18:09:35 +0100 Subject: [PATCH 2/2] comment the useProviderAuth flow --- .../scaffolder/RepoAuthentication.test.tsx | 2 +- .../x2a/src/scaffolder/useProviderAuth.ts | 53 +++++++++++++++++-- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/workspaces/x2a/plugins/x2a/src/scaffolder/RepoAuthentication.test.tsx b/workspaces/x2a/plugins/x2a/src/scaffolder/RepoAuthentication.test.tsx index 9d861a1bed..51840f5449 100644 --- a/workspaces/x2a/plugins/x2a/src/scaffolder/RepoAuthentication.test.tsx +++ b/workspaces/x2a/plugins/x2a/src/scaffolder/RepoAuthentication.test.tsx @@ -26,7 +26,7 @@ jest.mock('@backstage/plugin-scaffolder-react', () => ({ })); jest.mock('../hooks/useScmHostMap', () => ({ - useScmHostMap: () => new Map(), + useScmHostMap: () => new Map(), })); jest.mock('../repoAuth', () => ({ diff --git a/workspaces/x2a/plugins/x2a/src/scaffolder/useProviderAuth.ts b/workspaces/x2a/plugins/x2a/src/scaffolder/useProviderAuth.ts index f67534e9d6..3617356ac6 100644 --- a/workspaces/x2a/plugins/x2a/src/scaffolder/useProviderAuth.ts +++ b/workspaces/x2a/plugins/x2a/src/scaffolder/useProviderAuth.ts @@ -46,6 +46,21 @@ interface UseProviderAuthParams { setSecrets: (secrets: Record) => void; } +/** + * Manages the full lifecycle of SCM provider authentication for a CSV import. + * + * Three effects divide the work: + * 1. **Reset** - clears all auth state when csvContent changes. + * 2. **Auto-auth** - fires one OAuth dialog per distinct provider. + * All providers authenticate concurrently via Promise.all. + * If any fail the user can retry them individually. + * 3. **Completion** - once every provider row reaches "authenticated", + * stores tokens as scaffolder secrets and signals the parent form. + * + * Callbacks/props that change on every render (onChange, authenticate, + * secrets) are kept in refs so the effects' dependency arrays stay + * minimal and don't cause unwanted re-runs or cancellations. + */ export function useProviderAuth({ csvContent, hostProviderMap, @@ -62,6 +77,8 @@ export function useProviderAuth({ ); const [isDone, setDone] = useState(false); + // Refs for values that may change every render - used inside effects + // and callbacks without adding them to dependency arrays. const onChangeRef = useRef(onChange); onChangeRef.current = onChange; const authenticateRef = useRef(authenticate); @@ -76,7 +93,7 @@ export function useProviderAuth({ // auto-auth effect and cancelling other in-flight provider auths. const initialAuthFailedRef = useRef(false); - // Bumped on every CSV change and on unmount so stale async results + // Bumped on every CSV change so stale async results // (from auto-auth or retry) are silently discarded. const authGenerationRef = useRef(0); @@ -99,6 +116,8 @@ export function useProviderAuth({ const allSourceProviders: ScmProvider[] = projectsToCreate.map( project => resolveScmProvider(project.sourceRepoUrl, hostProviderMap), ); + // Deduplicate: targets get read-write access, sources that also + // appear as targets are excluded (the read-write token covers both). const targets = allTargetProviders.filter( (p, i, arr) => arr.findIndex(q => q.name === p.name) === i, ); @@ -121,6 +140,7 @@ export function useProviderAuth({ } }, [csvContent, hostProviderMap]); + // Build the provider rows for the table. const providerRows: ProviderRow[] = useMemo(() => { const rows: ProviderRow[] = []; for (const p of distinctTargetProviders) { @@ -151,7 +171,11 @@ export function useProviderAuth({ providerErrors, ]); - // Reset all auth state when CSV content changes. + // Effect: Reset + // Clears all auth state when CSV content changes. prevCsvRef lets us + // distinguish a real CSV change from a no-op re-render with the same + // value. The generation bump invalidates any in-flight retryProvider + // calls that were started for the previous CSV. useEffect(() => { if (csvContent !== prevCsvRef.current) { prevCsvRef.current = csvContent; @@ -165,7 +189,10 @@ export function useProviderAuth({ } }, [csvContent]); - // Auto-authenticate all providers on first render or CSV change. + // Effect: Auto-auth + // Fires one OAuth dialog per distinct provider, all concurrently. + // If the effect re-runs the cleanup sets + // `cancelled = true` so stale promises are discarded. useEffect(() => { if (!csvContent || initialAuthFailedRef.current || isDone || parseError) { return undefined; @@ -179,6 +206,11 @@ export function useProviderAuth({ return undefined; } + // Guard flag set by the cleanup function below. React calls the + // cleanup when this effect's dependencies change (e.g. csvContent) + // or when the component unmounts. Any in-flight authenticate() + // promises that settle after that point check this flag and skip + // their state updates so we never write stale tokens/statuses. let cancelled = false; const doAuthAsync = async () => { @@ -210,6 +242,9 @@ export function useProviderAuth({ ...prev, [provider.name]: e instanceof Error ? e.message : 'Unknown error', })); + // Mark via ref (not state) so we stop auto-auth on future + // renders without re-triggering this effect's cleanup and + // cancelling other providers still authenticating. initialAuthFailedRef.current = true; } }; @@ -221,6 +256,9 @@ export function useProviderAuth({ if (cancelled) return; + // If some providers failed, signal incomplete auth to the parent + // so the form validation blocks progression. The completion effect + // will fire onChange('authenticated') once every row reaches 'authenticated' const tokenCount = Object.keys(providerTokensRef.current).length; if (tokenCount !== allDistinctProviders.length) { onChangeRef.current(undefined); @@ -237,7 +275,9 @@ export function useProviderAuth({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [csvContent, isDone, parseError]); - // Complete: all providers authenticated → set secrets and notify parent. + // Effect: Completion + // Kept separate from auto-auth so that manual retries (retryProvider) + // also trigger completion without duplicating the secret-storing logic. useEffect(() => { if (isDone || providerRows.length === 0) return; if (!providerRows.every(r => r.status === 'authenticated')) return; @@ -250,6 +290,11 @@ export function useProviderAuth({ }); }, [providerRows, isDone, setSecrets]); + // Retry a single provider after a failure. The auto-auth effect is + // not re-run but directly updates statuses/tokens and fires + // the completion-effect. A captured generation counter guards + // against staleness: if the input changes while a retry is in-flight, + // the generation will have been bumped and the stale result is silently discarded. const retryProvider = useCallback( async (provider: ScmProvider, readOnly: boolean) => { const generation = authGenerationRef.current;