From 1ea76795a02a7e26bbb77cde8af13f31cb491c13 Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Thu, 23 May 2024 15:21:24 +0200 Subject: [PATCH 01/14] fix(scaffolder): remove waiting until all fields are filled in before requesting user credentials Signed-off-by: Benjamin Janssens --- .changeset/hungry-cups-jog.md | 5 +++++ .../src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx | 5 +---- 2 files changed, 6 insertions(+), 4 deletions(-) create mode 100644 .changeset/hungry-cups-jog.md diff --git a/.changeset/hungry-cups-jog.md b/.changeset/hungry-cups-jog.md new file mode 100644 index 0000000000..af600c4f91 --- /dev/null +++ b/.changeset/hungry-cups-jog.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder': patch +--- + +Removed waiting until all fields are filled in before requesting user credentials diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx index 5a40480889..b8fe7b9420 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx @@ -125,10 +125,7 @@ export const RepoUrlPicker = (props: RepoUrlPickerProps) => { const { requestUserCredentials } = uiSchema?.['ui:options'] ?? {}; const workspace = state.owner ? state.owner : state.project; - if ( - !requestUserCredentials || - !(state.host && workspace && state.repoName) - ) { + if (!requestUserCredentials) { return; } From 0a3f1bb2e0bcdaf63ec95484b57b452a5ec0250e Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Thu, 6 Jun 2024 13:20:55 +0200 Subject: [PATCH 02/14] refactor: use host instead of url for SCM authentication Signed-off-by: Benjamin Janssens --- packages/integration-react/src/api/ScmAuth.ts | 14 +++++++------- packages/integration-react/src/api/ScmAuthApi.ts | 6 +++--- .../fields/RepoUrlPicker/RepoUrlPicker.tsx | 13 ++----------- 3 files changed, 12 insertions(+), 21 deletions(-) diff --git a/packages/integration-react/src/api/ScmAuth.ts b/packages/integration-react/src/api/ScmAuth.ts index a22b15e400..05bb360d90 100644 --- a/packages/integration-react/src/api/ScmAuth.ts +++ b/packages/integration-react/src/api/ScmAuth.ts @@ -49,11 +49,11 @@ class ScmAuthMux implements ScmAuthApi { async getCredentials( options: ScmAuthTokenOptions, ): Promise { - const url = new URL(options.url); - const provider = this.#providers.find(p => p.isUrlSupported(url)); + const { host } = options; + const provider = this.#providers.find(p => p.isHostSupported(host)); if (!provider) { throw new Error( - `No auth provider available for '${options.url}', see https://backstage.io/link?scm-auth`, + `No auth provider available for '${host}', see https://backstage.io/link?scm-auth`, ); } @@ -261,10 +261,10 @@ export class ScmAuth implements ScmAuthApi { } /** - * Checks whether the implementation is able to provide authentication for the given URL. + * Checks whether the implementation is able to provide authentication for the given host. */ - isUrlSupported(url: URL): boolean { - return url.host === this.#host; + isHostSupported(host: string): boolean { + return host === this.#host; } private getAdditionalScopesForProvider( @@ -283,7 +283,7 @@ export class ScmAuth implements ScmAuthApi { async getCredentials( options: ScmAuthTokenOptions, ): Promise { - const { url, additionalScope, ...restOptions } = options; + const { host, additionalScope, ...restOptions } = options; const scopes = this.#scopeMapping.default.slice(); if (additionalScope?.repoWrite) { diff --git a/packages/integration-react/src/api/ScmAuthApi.ts b/packages/integration-react/src/api/ScmAuthApi.ts index be65842bc1..eafc630128 100644 --- a/packages/integration-react/src/api/ScmAuthApi.ts +++ b/packages/integration-react/src/api/ScmAuthApi.ts @@ -27,11 +27,11 @@ import { */ export interface ScmAuthTokenOptions extends AuthRequestOptions { /** - * The URL of the SCM resource to be accessed. + * The host of the SCM resource to be accessed. * - * @example https://github.com/backstage/backstage + * @example github.com */ - url: string; + host: string; /** * Whether to request additional access scope. diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx index b8fe7b9420..ad01ee0fd7 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx @@ -124,8 +124,7 @@ export const RepoUrlPicker = (props: RepoUrlPickerProps) => { async () => { const { requestUserCredentials } = uiSchema?.['ui:options'] ?? {}; - const workspace = state.owner ? state.owner : state.project; - if (!requestUserCredentials) { + if (!requestUserCredentials || !state.host) { return; } @@ -134,19 +133,11 @@ export const RepoUrlPicker = (props: RepoUrlPickerProps) => { return; } - // previously, we were encodeURI for state.host, workspace and state.repoName separately. - // That created an issue where GitLab workspace can be nested like groupA/subgroupB - // when we encodeURi separately and then join, the URL will be malformed and - // resulting in 400 request error from GitLab API - const [encodedHost, encodedRepoName] = [state.host, state.repoName].map( - encodeURIComponent, - ); - // user has requested that we use the users credentials // so lets grab them using the scmAuthApi and pass through // any additional scopes from the ui:options const { token } = await scmAuthApi.getCredentials({ - url: `https://${encodedHost}/${workspace}/${encodedRepoName}`, + host: state.host, additionalScope: { repoWrite: true, customScopes: requestUserCredentials.additionalScopes, From bad9449e3d984f4eb9356ba1ba5b7567d5730ca6 Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Thu, 6 Jun 2024 13:31:41 +0200 Subject: [PATCH 03/14] test: update tests Signed-off-by: Benjamin Janssens --- .../integration-react/src/api/ScmAuth.test.ts | 84 ++++++++----------- 1 file changed, 37 insertions(+), 47 deletions(-) diff --git a/packages/integration-react/src/api/ScmAuth.test.ts b/packages/integration-react/src/api/ScmAuth.test.ts index d252b95b85..de5ad516d7 100644 --- a/packages/integration-react/src/api/ScmAuth.test.ts +++ b/packages/integration-react/src/api/ScmAuth.test.ts @@ -37,9 +37,7 @@ describe('ScmAuth', () => { }), ); - await expect( - api.getCredentials({ url: 'https://github.com/backstage/backstage' }), - ).resolves.toEqual({ + await expect(api.getCredentials({ host: 'github.com' })).resolves.toEqual({ token: 'github-access-token', headers: { Authorization: 'Bearer github-access-token', @@ -47,7 +45,7 @@ describe('ScmAuth', () => { }); await expect( api.getCredentials({ - url: 'https://ghe.example.com/backstage/backstage', + host: 'ghe.example.com', additionalScope: { repoWrite: true, }, @@ -80,13 +78,13 @@ describe('ScmAuth', () => { const githubAuth = ScmAuth.forGithub(mockAuthApi); await expect( - githubAuth.getCredentials({ url: 'http://example.com' }), + githubAuth.getCredentials({ host: 'example.com' }), ).resolves.toMatchObject({ token: 'repo read:org read:user', }); await expect( githubAuth.getCredentials({ - url: 'http://example.com', + host: 'example.com', additionalScope: { repoWrite: true }, }), ).resolves.toMatchObject({ @@ -95,13 +93,13 @@ describe('ScmAuth', () => { const gitlabAuth = ScmAuth.forGitlab(mockAuthApi); await expect( - gitlabAuth.getCredentials({ url: 'http://example.com' }), + gitlabAuth.getCredentials({ host: 'example.com' }), ).resolves.toMatchObject({ token: 'read_user read_api read_repository', }); await expect( gitlabAuth.getCredentials({ - url: 'http://example.com', + host: 'example.com', additionalScope: { repoWrite: true }, }), ).resolves.toMatchObject({ @@ -110,14 +108,14 @@ describe('ScmAuth', () => { const azureAuth = ScmAuth.forAzure(mockAuthApi); await expect( - azureAuth.getCredentials({ url: 'http://example.com' }), + azureAuth.getCredentials({ host: 'example.com' }), ).resolves.toMatchObject({ token: '499b84ac-1321-427f-aa17-267ca6975798/vso.build 499b84ac-1321-427f-aa17-267ca6975798/vso.code 499b84ac-1321-427f-aa17-267ca6975798/vso.graph 499b84ac-1321-427f-aa17-267ca6975798/vso.project 499b84ac-1321-427f-aa17-267ca6975798/vso.profile', }); await expect( azureAuth.getCredentials({ - url: 'http://example.com', + host: 'example.com', additionalScope: { repoWrite: true }, }), ).resolves.toMatchObject({ @@ -127,13 +125,13 @@ describe('ScmAuth', () => { const bitbucketAuth = ScmAuth.forBitbucket(mockAuthApi); await expect( - bitbucketAuth.getCredentials({ url: 'http://example.com' }), + bitbucketAuth.getCredentials({ host: 'example.com' }), ).resolves.toMatchObject({ token: 'account team pullrequest snippet issue', }); await expect( bitbucketAuth.getCredentials({ - url: 'http://example.com', + host: 'example.com', additionalScope: { repoWrite: true }, }), ).resolves.toMatchObject({ @@ -152,7 +150,7 @@ describe('ScmAuth', () => { const githubAuth = ScmAuth.forGithub(mockAuthApi); await expect( githubAuth.getCredentials({ - url: 'http://example.com', + host: 'example.com', additionalScope: { customScopes: { github: ['org:read', 'workflow'] }, }, @@ -164,7 +162,7 @@ describe('ScmAuth', () => { const gitlabAuth = ScmAuth.forGitlab(mockAuthApi); await expect( gitlabAuth.getCredentials({ - url: 'http://example.com', + host: 'example.com', additionalScope: { customScopes: { gitlab: ['write_repository'] } }, }), ).resolves.toMatchObject({ @@ -174,7 +172,7 @@ describe('ScmAuth', () => { const azureAuth = ScmAuth.forAzure(mockAuthApi); await expect( azureAuth.getCredentials({ - url: 'http://example.com', + host: 'example.com', additionalScope: { customScopes: { azure: ['499b84ac-1321-427f-aa17-267ca6975798/vso.org'], @@ -189,7 +187,7 @@ describe('ScmAuth', () => { const bitbucketAuth = ScmAuth.forBitbucket(mockAuthApi); await expect( bitbucketAuth.getCredentials({ - url: 'http://example.com', + host: 'example.com', additionalScope: { customScopes: { bitbucket: ['snippet:write', 'issue:write'] }, }, @@ -204,47 +202,39 @@ describe('ScmAuth', () => { getAccessToken: jest.fn(), }; - const expectUrlSupport = (scm: ScmAuth, url: string) => { - expect(scm.isUrlSupported(new URL(url))).toBe(true); - expect(scm.isUrlSupported(new URL('https://not.supported.com'))).toBe( - false, - ); + const expectHostSupport = (scm: ScmAuth, host: string) => { + expect(scm.isHostSupported(host)).toBe(true); + expect(scm.isHostSupported('not.supported.com')).toBe(false); }; - expectUrlSupport(ScmAuth.forGithub(mockAuthApi), 'https://github.com'); - expectUrlSupport(ScmAuth.forGitlab(mockAuthApi), 'https://gitlab.com'); - expectUrlSupport( - ScmAuth.forAzure(mockAuthApi, {}), - 'https://dev.azure.com', - ); - expectUrlSupport( - ScmAuth.forBitbucket(mockAuthApi, {}), - 'https://bitbucket.org', - ); - expectUrlSupport( + expectHostSupport(ScmAuth.forGithub(mockAuthApi), 'github.com'); + expectHostSupport(ScmAuth.forGitlab(mockAuthApi), 'gitlab.com'); + expectHostSupport(ScmAuth.forAzure(mockAuthApi, {}), 'dev.azure.com'); + expectHostSupport(ScmAuth.forBitbucket(mockAuthApi, {}), 'bitbucket.org'); + expectHostSupport( ScmAuth.forGithub(mockAuthApi, { host: 'example.com' }), - 'https://example.com/abc', + 'example.com', ); - expectUrlSupport( + expectHostSupport( ScmAuth.forGitlab(mockAuthApi, { host: 'example.com' }), - 'http://example.com', + 'example.com', ); - expectUrlSupport( + expectHostSupport( ScmAuth.forAzure(mockAuthApi, { host: 'example.com' }), - 'https://example.com', + 'example.com', ); - expectUrlSupport( + expectHostSupport( ScmAuth.forBitbucket(mockAuthApi, { host: 'example.com:8080' }), - 'https://example.com:8080', + 'example.com:8080', ); }); - it('should throw an error for unknown URLs', async () => { + it('should throw an error for unknown hosts', async () => { const emptyMux = ScmAuth.merge(); await expect( - emptyMux.getCredentials({ url: 'http://example.com' }), + emptyMux.getCredentials({ host: 'example.com' }), ).rejects.toThrow( - "No auth provider available for 'http://example.com', see https://backstage.io/link?scm-auth", + "No auth provider available for 'example.com', see https://backstage.io/link?scm-auth", ); const scmAuth = ScmAuth.merge( @@ -257,17 +247,17 @@ describe('ScmAuth', () => { }), ); await expect( - scmAuth.getCredentials({ url: 'http://example.com' }), + scmAuth.getCredentials({ host: 'example.com' }), ).resolves.toMatchObject({ token: 'token' }); await expect( - scmAuth.getCredentials({ url: 'http://not.example.com' }), + scmAuth.getCredentials({ host: 'not.example.com' }), ).rejects.toThrow( - "No auth provider available for 'http://not.example.com', see https://backstage.io/link?scm-auth", + "No auth provider available for 'not.example.com', see https://backstage.io/link?scm-auth", ); await expect( - scmAuth.getCredentials({ url: 'http://example.com:8080' }), + scmAuth.getCredentials({ host: 'example.com:8080' }), ).rejects.toThrow( - "No auth provider available for 'http://example.com:8080', see https://backstage.io/link?scm-auth", + "No auth provider available for 'example.com:8080', see https://backstage.io/link?scm-auth", ); }); }); From 64e9b71c6305fb949480a52747ca6c427c74bf31 Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Thu, 6 Jun 2024 14:34:59 +0200 Subject: [PATCH 04/14] fix: update getCredentials in CatalogImportClient; update API reports Signed-off-by: Benjamin Janssens --- packages/integration-react/api-report.md | 4 ++-- plugins/catalog-import/src/api/CatalogImportClient.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/integration-react/api-report.md b/packages/integration-react/api-report.md index c1a2e34907..9e30875e79 100644 --- a/packages/integration-react/api-report.md +++ b/packages/integration-react/api-report.md @@ -70,7 +70,7 @@ export class ScmAuth implements ScmAuthApi { }, ): ScmAuth; getCredentials(options: ScmAuthTokenOptions): Promise; - isUrlSupported(url: URL): boolean; + isHostSupported(host: string): boolean; static merge(...providers: ScmAuth[]): ScmAuthApi; } @@ -93,7 +93,7 @@ export interface ScmAuthTokenOptions extends AuthRequestOptions { gitlab?: string[]; }; }; - url: string; + host: string; } // @public diff --git a/plugins/catalog-import/src/api/CatalogImportClient.ts b/plugins/catalog-import/src/api/CatalogImportClient.ts index cecb85f9a3..32bea5d7b4 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.ts @@ -260,7 +260,7 @@ the component will become available.\n\nFor more information, read an \ } = options; const { token } = await this.scmAuthApi.getCredentials({ - url: repositoryUrl, + host: new URL(repositoryUrl).host, additionalScope: { repoWrite: true, }, From 1759266332c26f48518f7458ce323c07896e06c0 Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Thu, 6 Jun 2024 14:51:04 +0200 Subject: [PATCH 05/14] test: update tests Signed-off-by: Benjamin Janssens --- .../components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx index 1fa723c8c6..fe7d2ee7cc 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx @@ -254,7 +254,7 @@ describe('RepoUrlPicker', () => { }); expect(mockScmAuthApi.getCredentials).toHaveBeenCalledWith({ - url: 'https://github.com/backstage/repo123', + host: 'github.com', additionalScope: { repoWrite: true, customScopes: { @@ -321,7 +321,7 @@ describe('RepoUrlPicker', () => { }); expect(mockScmAuthApi.getCredentials).toHaveBeenCalledWith({ - url: 'https://gitlab.example.com/backstage/mysubgroup/repo123', + host: 'gitlab.example.com', additionalScope: { repoWrite: true, }, @@ -375,7 +375,7 @@ describe('RepoUrlPicker', () => { }); expect(mockScmAuthApi.getCredentials).toHaveBeenCalledWith({ - url: 'https://server.bitbucket.org/backstage/repo123', + host: 'server.bitbucket.org', additionalScope: { repoWrite: true, }, From 928193383653e1d20d220bb7c7a5e85cdf1a41de Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Thu, 6 Jun 2024 15:41:54 +0200 Subject: [PATCH 06/14] chore: update changesets Signed-off-by: Benjamin Janssens --- .changeset/cool-stingrays-eat.md | 5 +++++ .changeset/hungry-cups-jog.md | 2 +- .changeset/seven-chefs-share.md | 5 +++++ 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 .changeset/cool-stingrays-eat.md create mode 100644 .changeset/seven-chefs-share.md diff --git a/.changeset/cool-stingrays-eat.md b/.changeset/cool-stingrays-eat.md new file mode 100644 index 0000000000..ba79391b49 --- /dev/null +++ b/.changeset/cool-stingrays-eat.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-import': patch +--- + +Updated `CatalogImportClient` to use the updated `ScmAuthTokenOptions` diff --git a/.changeset/hungry-cups-jog.md b/.changeset/hungry-cups-jog.md index af600c4f91..bd15fa92d3 100644 --- a/.changeset/hungry-cups-jog.md +++ b/.changeset/hungry-cups-jog.md @@ -2,4 +2,4 @@ '@backstage/plugin-scaffolder': patch --- -Removed waiting until all fields are filled in before requesting user credentials +Removed waiting for the workspace and repository fields to be filled in before requesting user credentials diff --git a/.changeset/seven-chefs-share.md b/.changeset/seven-chefs-share.md new file mode 100644 index 0000000000..76cf4682ff --- /dev/null +++ b/.changeset/seven-chefs-share.md @@ -0,0 +1,5 @@ +--- +'@backstage/integration-react': patch +--- + +Property `url` of class `ScmAuthTokenOptions` has been replaced with `host` From a05727d774d1503204b3aa5b0e33b2bd48f3b399 Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Thu, 6 Jun 2024 15:43:59 +0200 Subject: [PATCH 07/14] chore: update changesets Signed-off-by: Benjamin Janssens --- .changeset/seven-chefs-share.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/seven-chefs-share.md b/.changeset/seven-chefs-share.md index 76cf4682ff..31dcfaf75a 100644 --- a/.changeset/seven-chefs-share.md +++ b/.changeset/seven-chefs-share.md @@ -2,4 +2,4 @@ '@backstage/integration-react': patch --- -Property `url` of class `ScmAuthTokenOptions` has been replaced with `host` +Replaced property `url` of class `ScmAuthTokenOptions` with `host` From 13b4ef6135709e672b9768dcad4fe614c0814042 Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Thu, 6 Jun 2024 15:45:25 +0200 Subject: [PATCH 08/14] chore: update changesets Signed-off-by: Benjamin Janssens --- .changeset/seven-chefs-share.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/seven-chefs-share.md b/.changeset/seven-chefs-share.md index 31dcfaf75a..d673c7b239 100644 --- a/.changeset/seven-chefs-share.md +++ b/.changeset/seven-chefs-share.md @@ -2,4 +2,4 @@ '@backstage/integration-react': patch --- -Replaced property `url` of class `ScmAuthTokenOptions` with `host` +Replaced property `url` of class `ScmAuthTokenOptions` with property `host` From 847512f515c611f19051dd8aae1010e4ec1d1bf7 Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Thu, 6 Jun 2024 15:47:36 +0200 Subject: [PATCH 09/14] chore: update changesets Signed-off-by: Benjamin Janssens --- .changeset/seven-chefs-share.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/seven-chefs-share.md b/.changeset/seven-chefs-share.md index d673c7b239..6fa031cafa 100644 --- a/.changeset/seven-chefs-share.md +++ b/.changeset/seven-chefs-share.md @@ -2,4 +2,4 @@ '@backstage/integration-react': patch --- -Replaced property `url` of class `ScmAuthTokenOptions` with property `host` +Replaced property `url` of `ScmAuthTokenOptions` with property `host` From f61a4b3d8f890b540eaf26d141eeb9686a8238da Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Thu, 6 Jun 2024 16:06:48 +0200 Subject: [PATCH 10/14] test: update tests Signed-off-by: Benjamin Janssens --- .../RepoUrlPicker/RepoUrlPicker.test.tsx | 31 +++---------------- 1 file changed, 5 insertions(+), 26 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx index fe7d2ee7cc..9afbaf30f5 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx @@ -213,7 +213,7 @@ describe('RepoUrlPicker', () => {
{JSON.stringify({ secrets })}
); }; - const { getAllByRole, getByTestId } = await renderInTestApp( + const { getByTestId } = await renderInTestApp( { , ); - const [ownerInput, repoInput] = getAllByRole('textbox'); - await act(async () => { - fireEvent.change(ownerInput, { target: { value: 'backstage' } }); - fireEvent.change(repoInput, { target: { value: 'repo123' } }); - // need to wait for the debounce to finish await new Promise(resolve => setTimeout(resolve, 600)); }); @@ -278,7 +273,7 @@ describe('RepoUrlPicker', () => {
{JSON.stringify({ secrets })}
); }; - const { getAllByRole } = await renderInTestApp( + await renderInTestApp( { , ); - const [projectInput, repoInput] = getAllByRole('textbox'); - await act(async () => { - fireEvent.change(projectInput, { - target: { value: 'backstage/mysubgroup' }, - }); - fireEvent.change(repoInput, { target: { value: 'repo123' } }); - // need to wait for the debounce to finish await new Promise(resolve => setTimeout(resolve, 600)); }); @@ -327,6 +315,7 @@ describe('RepoUrlPicker', () => { }, }); }); + it('should call the scmAuthApi with the correct params if only a project is set', async () => { const SecretsComponent = () => { const { secrets } = useTemplateSecrets(); @@ -334,7 +323,7 @@ describe('RepoUrlPicker', () => {
{JSON.stringify({ secrets })}
); }; - const { getAllByRole, getByTestId } = await renderInTestApp( + const { getByTestId } = await renderInTestApp( { , ); - const [projectInput, repoInput] = getAllByRole('textbox'); - await act(async () => { - fireEvent.change(projectInput, { target: { value: 'backstage' } }); - fireEvent.change(repoInput, { target: { value: 'repo123' } }); - // need to wait for the debounce to finish await new Promise(resolve => setTimeout(resolve, 600)); }); @@ -397,7 +381,7 @@ describe('RepoUrlPicker', () => {
{JSON.stringify({ secrets })}
); }; - const { getAllByRole, getByTestId } = await renderInTestApp( + const { getByTestId } = await renderInTestApp( { , ); - const [ownerInput, repoInput] = getAllByRole('textbox'); - await act(async () => { - fireEvent.change(ownerInput, { target: { value: 'backstage' } }); - fireEvent.change(repoInput, { target: { value: 'repo123' } }); - // need to wait for the debounce to finish await new Promise(resolve => setTimeout(resolve, 600)); }); From e3d0da09dcef58e2aad6adf37087b31beb56f4fd Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Thu, 6 Jun 2024 16:25:58 +0200 Subject: [PATCH 11/14] test: improve tests of CatalogImportClient Signed-off-by: Benjamin Janssens --- .../src/api/CatalogImportClient.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/plugins/catalog-import/src/api/CatalogImportClient.test.ts b/plugins/catalog-import/src/api/CatalogImportClient.test.ts index df44ec94c4..73b9d741b9 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.test.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.test.ts @@ -588,6 +588,12 @@ describe('CatalogImportClient', () => { 'https://github.com/backstage/backstage/blob/main/catalog-info.yaml', }); expect(catalogApi.validateEntity).toHaveBeenCalledTimes(1); + expect(scmAuthApi.getCredentials).toHaveBeenCalledWith({ + host: 'github.com', + additionalScope: { + repoWrite: true, + }, + }); expect( (new Octokit().git.createRef as any as jest.Mock).mock.calls[0][0], ).toEqual({ @@ -701,6 +707,13 @@ describe('CatalogImportClient', () => { }), ); + expect(scmAuthApi.getCredentials).toHaveBeenCalledWith({ + host: 'github.com', + additionalScope: { + repoWrite: true, + }, + }); + expect( (new Octokit().git.createRef as any as jest.Mock).mock.calls[0][0], ).toEqual( From d7a8c736f8db1350fae7c5b85138e6159784d050 Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Fri, 14 Jun 2024 11:22:17 +0200 Subject: [PATCH 12/14] refactor: revert ScmAuthTokenOptions API change Signed-off-by: Benjamin Janssens --- .changeset/cool-stingrays-eat.md | 5 -- .changeset/seven-chefs-share.md | 5 -- packages/integration-react/api-report.md | 4 +- .../integration-react/src/api/ScmAuth.test.ts | 84 +++++++++++-------- packages/integration-react/src/api/ScmAuth.ts | 14 ++-- .../integration-react/src/api/ScmAuthApi.ts | 6 +- .../src/api/CatalogImportClient.test.ts | 13 --- .../src/api/CatalogImportClient.ts | 2 +- 8 files changed, 60 insertions(+), 73 deletions(-) delete mode 100644 .changeset/cool-stingrays-eat.md delete mode 100644 .changeset/seven-chefs-share.md diff --git a/.changeset/cool-stingrays-eat.md b/.changeset/cool-stingrays-eat.md deleted file mode 100644 index ba79391b49..0000000000 --- a/.changeset/cool-stingrays-eat.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@backstage/plugin-catalog-import': patch ---- - -Updated `CatalogImportClient` to use the updated `ScmAuthTokenOptions` diff --git a/.changeset/seven-chefs-share.md b/.changeset/seven-chefs-share.md deleted file mode 100644 index 6fa031cafa..0000000000 --- a/.changeset/seven-chefs-share.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@backstage/integration-react': patch ---- - -Replaced property `url` of `ScmAuthTokenOptions` with property `host` diff --git a/packages/integration-react/api-report.md b/packages/integration-react/api-report.md index 9e30875e79..c1a2e34907 100644 --- a/packages/integration-react/api-report.md +++ b/packages/integration-react/api-report.md @@ -70,7 +70,7 @@ export class ScmAuth implements ScmAuthApi { }, ): ScmAuth; getCredentials(options: ScmAuthTokenOptions): Promise; - isHostSupported(host: string): boolean; + isUrlSupported(url: URL): boolean; static merge(...providers: ScmAuth[]): ScmAuthApi; } @@ -93,7 +93,7 @@ export interface ScmAuthTokenOptions extends AuthRequestOptions { gitlab?: string[]; }; }; - host: string; + url: string; } // @public diff --git a/packages/integration-react/src/api/ScmAuth.test.ts b/packages/integration-react/src/api/ScmAuth.test.ts index de5ad516d7..d252b95b85 100644 --- a/packages/integration-react/src/api/ScmAuth.test.ts +++ b/packages/integration-react/src/api/ScmAuth.test.ts @@ -37,7 +37,9 @@ describe('ScmAuth', () => { }), ); - await expect(api.getCredentials({ host: 'github.com' })).resolves.toEqual({ + await expect( + api.getCredentials({ url: 'https://github.com/backstage/backstage' }), + ).resolves.toEqual({ token: 'github-access-token', headers: { Authorization: 'Bearer github-access-token', @@ -45,7 +47,7 @@ describe('ScmAuth', () => { }); await expect( api.getCredentials({ - host: 'ghe.example.com', + url: 'https://ghe.example.com/backstage/backstage', additionalScope: { repoWrite: true, }, @@ -78,13 +80,13 @@ describe('ScmAuth', () => { const githubAuth = ScmAuth.forGithub(mockAuthApi); await expect( - githubAuth.getCredentials({ host: 'example.com' }), + githubAuth.getCredentials({ url: 'http://example.com' }), ).resolves.toMatchObject({ token: 'repo read:org read:user', }); await expect( githubAuth.getCredentials({ - host: 'example.com', + url: 'http://example.com', additionalScope: { repoWrite: true }, }), ).resolves.toMatchObject({ @@ -93,13 +95,13 @@ describe('ScmAuth', () => { const gitlabAuth = ScmAuth.forGitlab(mockAuthApi); await expect( - gitlabAuth.getCredentials({ host: 'example.com' }), + gitlabAuth.getCredentials({ url: 'http://example.com' }), ).resolves.toMatchObject({ token: 'read_user read_api read_repository', }); await expect( gitlabAuth.getCredentials({ - host: 'example.com', + url: 'http://example.com', additionalScope: { repoWrite: true }, }), ).resolves.toMatchObject({ @@ -108,14 +110,14 @@ describe('ScmAuth', () => { const azureAuth = ScmAuth.forAzure(mockAuthApi); await expect( - azureAuth.getCredentials({ host: 'example.com' }), + azureAuth.getCredentials({ url: 'http://example.com' }), ).resolves.toMatchObject({ token: '499b84ac-1321-427f-aa17-267ca6975798/vso.build 499b84ac-1321-427f-aa17-267ca6975798/vso.code 499b84ac-1321-427f-aa17-267ca6975798/vso.graph 499b84ac-1321-427f-aa17-267ca6975798/vso.project 499b84ac-1321-427f-aa17-267ca6975798/vso.profile', }); await expect( azureAuth.getCredentials({ - host: 'example.com', + url: 'http://example.com', additionalScope: { repoWrite: true }, }), ).resolves.toMatchObject({ @@ -125,13 +127,13 @@ describe('ScmAuth', () => { const bitbucketAuth = ScmAuth.forBitbucket(mockAuthApi); await expect( - bitbucketAuth.getCredentials({ host: 'example.com' }), + bitbucketAuth.getCredentials({ url: 'http://example.com' }), ).resolves.toMatchObject({ token: 'account team pullrequest snippet issue', }); await expect( bitbucketAuth.getCredentials({ - host: 'example.com', + url: 'http://example.com', additionalScope: { repoWrite: true }, }), ).resolves.toMatchObject({ @@ -150,7 +152,7 @@ describe('ScmAuth', () => { const githubAuth = ScmAuth.forGithub(mockAuthApi); await expect( githubAuth.getCredentials({ - host: 'example.com', + url: 'http://example.com', additionalScope: { customScopes: { github: ['org:read', 'workflow'] }, }, @@ -162,7 +164,7 @@ describe('ScmAuth', () => { const gitlabAuth = ScmAuth.forGitlab(mockAuthApi); await expect( gitlabAuth.getCredentials({ - host: 'example.com', + url: 'http://example.com', additionalScope: { customScopes: { gitlab: ['write_repository'] } }, }), ).resolves.toMatchObject({ @@ -172,7 +174,7 @@ describe('ScmAuth', () => { const azureAuth = ScmAuth.forAzure(mockAuthApi); await expect( azureAuth.getCredentials({ - host: 'example.com', + url: 'http://example.com', additionalScope: { customScopes: { azure: ['499b84ac-1321-427f-aa17-267ca6975798/vso.org'], @@ -187,7 +189,7 @@ describe('ScmAuth', () => { const bitbucketAuth = ScmAuth.forBitbucket(mockAuthApi); await expect( bitbucketAuth.getCredentials({ - host: 'example.com', + url: 'http://example.com', additionalScope: { customScopes: { bitbucket: ['snippet:write', 'issue:write'] }, }, @@ -202,39 +204,47 @@ describe('ScmAuth', () => { getAccessToken: jest.fn(), }; - const expectHostSupport = (scm: ScmAuth, host: string) => { - expect(scm.isHostSupported(host)).toBe(true); - expect(scm.isHostSupported('not.supported.com')).toBe(false); + const expectUrlSupport = (scm: ScmAuth, url: string) => { + expect(scm.isUrlSupported(new URL(url))).toBe(true); + expect(scm.isUrlSupported(new URL('https://not.supported.com'))).toBe( + false, + ); }; - expectHostSupport(ScmAuth.forGithub(mockAuthApi), 'github.com'); - expectHostSupport(ScmAuth.forGitlab(mockAuthApi), 'gitlab.com'); - expectHostSupport(ScmAuth.forAzure(mockAuthApi, {}), 'dev.azure.com'); - expectHostSupport(ScmAuth.forBitbucket(mockAuthApi, {}), 'bitbucket.org'); - expectHostSupport( + expectUrlSupport(ScmAuth.forGithub(mockAuthApi), 'https://github.com'); + expectUrlSupport(ScmAuth.forGitlab(mockAuthApi), 'https://gitlab.com'); + expectUrlSupport( + ScmAuth.forAzure(mockAuthApi, {}), + 'https://dev.azure.com', + ); + expectUrlSupport( + ScmAuth.forBitbucket(mockAuthApi, {}), + 'https://bitbucket.org', + ); + expectUrlSupport( ScmAuth.forGithub(mockAuthApi, { host: 'example.com' }), - 'example.com', + 'https://example.com/abc', ); - expectHostSupport( + expectUrlSupport( ScmAuth.forGitlab(mockAuthApi, { host: 'example.com' }), - 'example.com', + 'http://example.com', ); - expectHostSupport( + expectUrlSupport( ScmAuth.forAzure(mockAuthApi, { host: 'example.com' }), - 'example.com', + 'https://example.com', ); - expectHostSupport( + expectUrlSupport( ScmAuth.forBitbucket(mockAuthApi, { host: 'example.com:8080' }), - 'example.com:8080', + 'https://example.com:8080', ); }); - it('should throw an error for unknown hosts', async () => { + it('should throw an error for unknown URLs', async () => { const emptyMux = ScmAuth.merge(); await expect( - emptyMux.getCredentials({ host: 'example.com' }), + emptyMux.getCredentials({ url: 'http://example.com' }), ).rejects.toThrow( - "No auth provider available for 'example.com', see https://backstage.io/link?scm-auth", + "No auth provider available for 'http://example.com', see https://backstage.io/link?scm-auth", ); const scmAuth = ScmAuth.merge( @@ -247,17 +257,17 @@ describe('ScmAuth', () => { }), ); await expect( - scmAuth.getCredentials({ host: 'example.com' }), + scmAuth.getCredentials({ url: 'http://example.com' }), ).resolves.toMatchObject({ token: 'token' }); await expect( - scmAuth.getCredentials({ host: 'not.example.com' }), + scmAuth.getCredentials({ url: 'http://not.example.com' }), ).rejects.toThrow( - "No auth provider available for 'not.example.com', see https://backstage.io/link?scm-auth", + "No auth provider available for 'http://not.example.com', see https://backstage.io/link?scm-auth", ); await expect( - scmAuth.getCredentials({ host: 'example.com:8080' }), + scmAuth.getCredentials({ url: 'http://example.com:8080' }), ).rejects.toThrow( - "No auth provider available for 'example.com:8080', see https://backstage.io/link?scm-auth", + "No auth provider available for 'http://example.com:8080', see https://backstage.io/link?scm-auth", ); }); }); diff --git a/packages/integration-react/src/api/ScmAuth.ts b/packages/integration-react/src/api/ScmAuth.ts index 05bb360d90..a22b15e400 100644 --- a/packages/integration-react/src/api/ScmAuth.ts +++ b/packages/integration-react/src/api/ScmAuth.ts @@ -49,11 +49,11 @@ class ScmAuthMux implements ScmAuthApi { async getCredentials( options: ScmAuthTokenOptions, ): Promise { - const { host } = options; - const provider = this.#providers.find(p => p.isHostSupported(host)); + const url = new URL(options.url); + const provider = this.#providers.find(p => p.isUrlSupported(url)); if (!provider) { throw new Error( - `No auth provider available for '${host}', see https://backstage.io/link?scm-auth`, + `No auth provider available for '${options.url}', see https://backstage.io/link?scm-auth`, ); } @@ -261,10 +261,10 @@ export class ScmAuth implements ScmAuthApi { } /** - * Checks whether the implementation is able to provide authentication for the given host. + * Checks whether the implementation is able to provide authentication for the given URL. */ - isHostSupported(host: string): boolean { - return host === this.#host; + isUrlSupported(url: URL): boolean { + return url.host === this.#host; } private getAdditionalScopesForProvider( @@ -283,7 +283,7 @@ export class ScmAuth implements ScmAuthApi { async getCredentials( options: ScmAuthTokenOptions, ): Promise { - const { host, additionalScope, ...restOptions } = options; + const { url, additionalScope, ...restOptions } = options; const scopes = this.#scopeMapping.default.slice(); if (additionalScope?.repoWrite) { diff --git a/packages/integration-react/src/api/ScmAuthApi.ts b/packages/integration-react/src/api/ScmAuthApi.ts index eafc630128..be65842bc1 100644 --- a/packages/integration-react/src/api/ScmAuthApi.ts +++ b/packages/integration-react/src/api/ScmAuthApi.ts @@ -27,11 +27,11 @@ import { */ export interface ScmAuthTokenOptions extends AuthRequestOptions { /** - * The host of the SCM resource to be accessed. + * The URL of the SCM resource to be accessed. * - * @example github.com + * @example https://github.com/backstage/backstage */ - host: string; + url: string; /** * Whether to request additional access scope. diff --git a/plugins/catalog-import/src/api/CatalogImportClient.test.ts b/plugins/catalog-import/src/api/CatalogImportClient.test.ts index 73b9d741b9..df44ec94c4 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.test.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.test.ts @@ -588,12 +588,6 @@ describe('CatalogImportClient', () => { 'https://github.com/backstage/backstage/blob/main/catalog-info.yaml', }); expect(catalogApi.validateEntity).toHaveBeenCalledTimes(1); - expect(scmAuthApi.getCredentials).toHaveBeenCalledWith({ - host: 'github.com', - additionalScope: { - repoWrite: true, - }, - }); expect( (new Octokit().git.createRef as any as jest.Mock).mock.calls[0][0], ).toEqual({ @@ -707,13 +701,6 @@ describe('CatalogImportClient', () => { }), ); - expect(scmAuthApi.getCredentials).toHaveBeenCalledWith({ - host: 'github.com', - additionalScope: { - repoWrite: true, - }, - }); - expect( (new Octokit().git.createRef as any as jest.Mock).mock.calls[0][0], ).toEqual( diff --git a/plugins/catalog-import/src/api/CatalogImportClient.ts b/plugins/catalog-import/src/api/CatalogImportClient.ts index 32bea5d7b4..cecb85f9a3 100644 --- a/plugins/catalog-import/src/api/CatalogImportClient.ts +++ b/plugins/catalog-import/src/api/CatalogImportClient.ts @@ -260,7 +260,7 @@ the component will become available.\n\nFor more information, read an \ } = options; const { token } = await this.scmAuthApi.getCredentials({ - host: new URL(repositoryUrl).host, + url: repositoryUrl, additionalScope: { repoWrite: true, }, From 872ac0b5186db6f9e98942678ed9b27ab14ba120 Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Fri, 14 Jun 2024 11:37:16 +0200 Subject: [PATCH 13/14] refactor: update RepoUrlPicker to be compatible with the original API Signed-off-by: Benjamin Janssens --- .../components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx | 6 +++--- .../src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx index 9afbaf30f5..573838b960 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx @@ -249,7 +249,7 @@ describe('RepoUrlPicker', () => { }); expect(mockScmAuthApi.getCredentials).toHaveBeenCalledWith({ - host: 'github.com', + url: 'https://github.com', additionalScope: { repoWrite: true, customScopes: { @@ -309,7 +309,7 @@ describe('RepoUrlPicker', () => { }); expect(mockScmAuthApi.getCredentials).toHaveBeenCalledWith({ - host: 'gitlab.example.com', + url: 'https://gitlab.example.com', additionalScope: { repoWrite: true, }, @@ -359,7 +359,7 @@ describe('RepoUrlPicker', () => { }); expect(mockScmAuthApi.getCredentials).toHaveBeenCalledWith({ - host: 'server.bitbucket.org', + url: 'https://server.bitbucket.org', additionalScope: { repoWrite: true, }, diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx index ad01ee0fd7..85245639b0 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.tsx @@ -137,7 +137,7 @@ export const RepoUrlPicker = (props: RepoUrlPickerProps) => { // so lets grab them using the scmAuthApi and pass through // any additional scopes from the ui:options const { token } = await scmAuthApi.getCredentials({ - host: state.host, + url: `https://${state.host}`, additionalScope: { repoWrite: true, customScopes: requestUserCredentials.additionalScopes, From 2c4374a97cc34ed001f4ca6903ba035a55a1d120 Mon Sep 17 00:00:00 2001 From: Benjamin Janssens Date: Fri, 14 Jun 2024 11:51:43 +0200 Subject: [PATCH 14/14] chore: remove unnecessary test Signed-off-by: Benjamin Janssens --- .../RepoUrlPicker/RepoUrlPicker.test.tsx | 58 ------------------- 1 file changed, 58 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx index 573838b960..59c09c106c 100644 --- a/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/RepoUrlPicker/RepoUrlPicker.test.tsx @@ -316,64 +316,6 @@ describe('RepoUrlPicker', () => { }); }); - it('should call the scmAuthApi with the correct params if only a project is set', async () => { - const SecretsComponent = () => { - const { secrets } = useTemplateSecrets(); - return ( -
{JSON.stringify({ secrets })}
- ); - }; - const { getByTestId } = await renderInTestApp( - - -
, - }} - /> - - - , - ); - - await act(async () => { - // need to wait for the debounce to finish - await new Promise(resolve => setTimeout(resolve, 600)); - }); - - expect(mockScmAuthApi.getCredentials).toHaveBeenCalledWith({ - url: 'https://server.bitbucket.org', - additionalScope: { - repoWrite: true, - }, - }); - - const currentSecrets = JSON.parse( - getByTestId('current-secrets').textContent!, - ); - - expect(currentSecrets).toEqual({ - secrets: { testKey: 'abc123' }, - }); - }); - it('should not call the scmAuthApi if secret is available in the state', async () => { const SecretsComponent = () => { const { secrets } = useTemplateSecrets();