diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoCreate.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoCreate.test.ts index f15747fd6b..a9ef8c3daa 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoCreate.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/githubRepoCreate.test.ts @@ -42,6 +42,7 @@ const mockOctokit = { }, teams: { addOrUpdateRepoPermissionsInOrg: jest.fn(), + getByName: jest.fn(), }, }, }; @@ -96,6 +97,13 @@ describe('github:repo:create', () => { data: { type: 'Organization' }, }); + mockOctokit.rest.teams.getByName.mockResolvedValue({ + data: { + name: 'blam', + id: 42, + }, + }); + mockOctokit.rest.repos.createInOrg.mockResolvedValue({ data: {} }); await action.handler(mockContext); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts index 25f343da9e..dc8c4cd856 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/github/helpers.ts @@ -135,6 +135,10 @@ export async function createGithubRepoWithCollaboratorsAndTopics( username: owner, }); + if (access?.startsWith(`${owner}/`)) { + await validateAccessTeam(client, access); + } + const repoCreationPromise = user.data.type === 'Organization' ? client.rest.repos.createInOrg({ @@ -190,22 +194,13 @@ export async function createGithubRepoWithCollaboratorsAndTopics( if (access?.startsWith(`${owner}/`)) { const [, team] = access.split('/'); - try { - await client.rest.teams.addOrUpdateRepoPermissionsInOrg({ - org: owner, - team_slug: team, - owner, - repo, - permission: 'admin', - }); - } catch (e) { - if (e.data.message === 'Not Found') { - const message = `Received 'Not Found' from the API; one of org: - ${owner}, team: ${team} or repo: ${repo} was not found within GitHub.`; - logger.warn(message); - throw new Error(message, { cause: e }); - } - } + await client.rest.teams.addOrUpdateRepoPermissionsInOrg({ + org: owner, + team_slug: team, + owner, + repo, + permission: 'admin', + }); // No need to add access if it's the person who owns the personal account } else if (access && access !== owner) { await client.rest.repos.addCollaborator({ @@ -360,3 +355,22 @@ function extractCollaboratorName( if ('user' in collaborator) return collaborator.user; return collaborator.team; } + +async function validateAccessTeam(client: Octokit, access: string) { + const [org, team_slug] = access.split('/'); + try { + // Below rule disabled because of a 'getByName' check for a different library + // incorrectly triggers here. + // eslint-disable-next-line testing-library/no-await-sync-query + await client.rest.teams.getByName({ + org, + team_slug, + }); + } catch (e) { + if (e.response.data.message === 'Not Found') { + const message = `Received 'Not Found' from the API; one of org: + ${org} or team: ${team_slug} was not found within GitHub.`; + throw new Error(message, { cause: e }); + } + } +} diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts index 0edeafecfd..94b51f3e94 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.test.ts @@ -45,6 +45,7 @@ const mockOctokit = { replaceAllTopics: jest.fn(), }, teams: { + getByName: jest.fn(), addOrUpdateRepoPermissionsInOrg: jest.fn(), }, }, @@ -96,11 +97,41 @@ describe('publish:github', () => { }); }); + it('should fail to create if the team is not found in the org', async () => { + mockOctokit.rest.users.getByUsername.mockResolvedValue({ + data: { type: 'Organization' }, + }); + + mockOctokit.rest.teams.getByName.mockRejectedValue({ + response: { + status: 404, + data: { + message: 'Not Found', + documentation_url: + 'https://docs.github.com/en/rest/teams/teams#add-or-update-team-repository-permissions', + }, + }, + }); + + await expect(action.handler(mockContext)).rejects.toThrow( + "Received 'Not Found' from the API;", + ); + + expect(mockOctokit.rest.repos.createInOrg).not.toHaveBeenCalled(); + }); + it('should call the githubApis with the correct values for createInOrg', async () => { mockOctokit.rest.users.getByUsername.mockResolvedValue({ data: { type: 'Organization' }, }); + mockOctokit.rest.teams.getByName.mockResolvedValue({ + data: { + name: 'blam', + id: 42, + }, + }); + mockOctokit.rest.repos.createInOrg.mockResolvedValue({ data: {} }); await action.handler(mockContext); @@ -523,24 +554,23 @@ describe('publish:github', () => { data: { type: 'User' }, }); - mockOctokit.rest.repos.createForAuthenticatedUser.mockResolvedValue({ - data: { - clone_url: 'https://github.com/clone/url.git', - html_url: 'https://github.com/html/url', - }, - }); - - mockOctokit.rest.teams.addOrUpdateRepoPermissionsInOrg.mockRejectedValue({ - status: 404, - data: { - message: 'Not Found', - documentation_url: - 'https://docs.github.com/en/rest/teams/teams#add-or-update-team-repository-permissions', + mockOctokit.rest.teams.getByName.mockRejectedValue({ + response: { + status: 404, + data: { + message: 'Not Found', + documentation_url: + 'https://docs.github.com/en/rest/teams/teams#add-or-update-team-repository-permissions', + }, }, }); await expect(action.handler(mockContext)).rejects.toThrow( "Received 'Not Found' from the API;", ); + + expect( + mockOctokit.rest.repos.createForAuthenticatedUser, + ).not.toHaveBeenCalled(); }); it('should add outside collaborators when provided', async () => {