From 45770ba8f896f520a77c2373da016bf49062a389 Mon Sep 17 00:00:00 2001 From: Matteo Silvestri Date: Thu, 30 Jun 2022 15:46:46 +0200 Subject: [PATCH 1/6] feat: new setUserAsOwner flag for publish:gitlab action Signed-off-by: Matteo Silvestri --- .../actions/builtin/publish/gitlab.test.ts | 60 +++++++++++++++++++ .../actions/builtin/publish/gitlab.ts | 28 +++++++-- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.test.ts index f427023c69..27261ecd49 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.test.ts @@ -32,6 +32,9 @@ const mockGitlabClient = { Users: { current: jest.fn(), }, + ProjectMembers: { + add: jest.fn(), + }, }; jest.mock('@gitbeaker/node', () => ({ Gitlab: class { @@ -113,6 +116,7 @@ describe('publish:gitlab', () => { }); it('should work when there is a token provided through ctx.input', async () => { + mockGitlabClient.Users.current.mockResolvedValue({ id: 12345 }); mockGitlabClient.Namespaces.show.mockResolvedValue({ id: 1234 }); mockGitlabClient.Projects.create.mockResolvedValue({ http_url_to_repo: 'http://mockurl.git', @@ -135,6 +139,7 @@ describe('publish:gitlab', () => { }); it('should call the correct Gitlab APIs when the owner is an organization', async () => { + mockGitlabClient.Users.current.mockResolvedValue({ id: 12345 }); mockGitlabClient.Namespaces.show.mockResolvedValue({ id: 1234 }); mockGitlabClient.Projects.create.mockResolvedValue({ http_url_to_repo: 'http://mockurl.git', @@ -168,6 +173,7 @@ describe('publish:gitlab', () => { }); it('should call initRepoAndPush with the correct values', async () => { + mockGitlabClient.Users.current.mockResolvedValue({ id: 12345 }); mockGitlabClient.Namespaces.show.mockResolvedValue({ id: 1234 }); mockGitlabClient.Projects.create.mockResolvedValue({ http_url_to_repo: 'http://mockurl.git', @@ -187,6 +193,7 @@ describe('publish:gitlab', () => { }); it('should call initRepoAndPush with the correct default branch', async () => { + mockGitlabClient.Users.current.mockResolvedValue({ id: 12345 }); mockGitlabClient.Namespaces.show.mockResolvedValue({ id: 1234 }); mockGitlabClient.Projects.create.mockResolvedValue({ http_url_to_repo: 'http://mockurl.git', @@ -241,6 +248,7 @@ describe('publish:gitlab', () => { config: customAuthorConfig, }); + mockGitlabClient.Users.current.mockResolvedValue({ id: 12345 }); mockGitlabClient.Namespaces.show.mockResolvedValue({ id: 1234 }); mockGitlabClient.Projects.create.mockResolvedValue({ http_url_to_repo: 'http://mockurl.git', @@ -286,6 +294,7 @@ describe('publish:gitlab', () => { config: customAuthorConfig, }); + mockGitlabClient.Users.current.mockResolvedValue({ id: 12345 }); mockGitlabClient.Namespaces.show.mockResolvedValue({ id: 1234 }); mockGitlabClient.Projects.create.mockResolvedValue({ http_url_to_repo: 'http://mockurl.git', @@ -305,6 +314,7 @@ describe('publish:gitlab', () => { }); it('should call output with the remoteUrl and repoContentsUrl', async () => { + mockGitlabClient.Users.current.mockResolvedValue({ id: 12345 }); mockGitlabClient.Namespaces.show.mockResolvedValue({ id: 1234 }); mockGitlabClient.Projects.create.mockResolvedValue({ http_url_to_repo: 'http://mockurl.git', @@ -321,4 +331,54 @@ describe('publish:gitlab', () => { 'http://mockurl/-/blob/master', ); }); + + it('should call the correct Gitlab APIs when setUserAsOwner option is true and integration config has a token', async () => { + const customAuthorConfig = new ConfigReader({ + integrations: { + gitlab: [ + { + host: 'gitlab.com', + token: 'tokenlols', + apiBaseUrl: 'https://api.gitlab.com', + }, + ], + }, + }); + + const customAuthorIntegrations = + ScmIntegrations.fromConfig(customAuthorConfig); + const customAuthorAction = createPublishGitlabAction({ + integrations: customAuthorIntegrations, + config: customAuthorConfig, + }); + + mockGitlabClient.Namespaces.show.mockResolvedValue({ id: 1234 }); + mockGitlabClient.Users.current.mockResolvedValue({ id: 12345 }); + mockGitlabClient.Projects.create.mockResolvedValue({ + id: 123456, + http_url_to_repo: 'http://mockurl.git', + }); + + await customAuthorAction.handler({ + ...mockContext, + input: { + repoUrl: 'gitlab.com?repo=repo&owner=owner', + token: 'token', + setUserAsOwner: true, + }, + }); + + expect(mockGitlabClient.Namespaces.show).toHaveBeenCalledWith('owner'); + expect(mockGitlabClient.Users.current).toHaveBeenCalled(); + expect(mockGitlabClient.ProjectMembers.add).toHaveBeenCalledWith( + 123456, + 12345, + 50, + ); + expect(mockGitlabClient.Projects.create).toHaveBeenCalledWith({ + namespace_id: 1234, + name: 'repo', + visibility: 'private', + }); + }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts index a7c247f4d9..e36bb0aa68 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts @@ -43,6 +43,7 @@ export function createPublishGitlabAction(options: { gitCommitMessage?: string; gitAuthorName?: string; gitAuthorEmail?: string; + setUserAsOwner?: boolean; }>({ id: 'publish:gitlab', description: @@ -92,6 +93,12 @@ export function createPublishGitlabAction(options: { type: 'string', description: 'The token to use for authorization to GitLab', }, + setUserAsOwner: { + title: 'Set User As Owner', + type: 'boolean', + description: + 'Set the token user as owner of the newly created repository. Requires a token authorized to do the edit in the integration configuration for the matching host', + }, }, }, output: { @@ -116,6 +123,7 @@ export function createPublishGitlabAction(options: { gitCommitMessage = 'initial commit', gitAuthorName, gitAuthorEmail, + setUserAsOwner = false, } = ctx.input; const { owner, repo, host } = parseRepoUrl(repoUrl, integrations); @@ -149,19 +157,29 @@ export function createPublishGitlabAction(options: { id: number; }; + const { id: userId } = (await client.Users.current()) as { + id: number; + }; + if (!targetNamespace) { - const { id } = (await client.Users.current()) as { - id: number; - }; - targetNamespace = id; + targetNamespace = userId; } - const { http_url_to_repo } = await client.Projects.create({ + const { id: projectId, http_url_to_repo } = await client.Projects.create({ namespace_id: targetNamespace, name: repo, visibility: repoVisibility, }); + if (setUserAsOwner && integrationConfig.config.token) { + const adminClient = new Gitlab({ + host: integrationConfig.config.baseUrl, + ['token']: integrationConfig.config.token, + }); + + await adminClient.ProjectMembers.add(projectId, userId, 50); + } + const remoteUrl = (http_url_to_repo as string).replace(/\.git$/, ''); const repoContentsUrl = `${remoteUrl}/-/blob/${defaultBranch}`; From af02f5448353313f511e89be40c86bdeef1571e6 Mon Sep 17 00:00:00 2001 From: Matteo Silvestri Date: Thu, 30 Jun 2022 17:07:05 +0200 Subject: [PATCH 2/6] add changeset Signed-off-by: Matteo Silvestri --- .changeset/pink-cars-pretend.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/pink-cars-pretend.md diff --git a/.changeset/pink-cars-pretend.md b/.changeset/pink-cars-pretend.md new file mode 100644 index 0000000000..d2350b4de2 --- /dev/null +++ b/.changeset/pink-cars-pretend.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +new setUserAsOwner flag for publish:gitlab action From dcf9d886e07e1508ef9820f39126418c19be4248 Mon Sep 17 00:00:00 2001 From: Matteo Silvestri Date: Thu, 30 Jun 2022 17:34:22 +0200 Subject: [PATCH 3/6] edit changeset Signed-off-by: Matteo Silvestri --- .changeset/pink-cars-pretend.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.changeset/pink-cars-pretend.md b/.changeset/pink-cars-pretend.md index d2350b4de2..c46cb99f55 100644 --- a/.changeset/pink-cars-pretend.md +++ b/.changeset/pink-cars-pretend.md @@ -3,3 +3,5 @@ --- new setUserAsOwner flag for publish:gitlab action + +The field default is `false`. When true it will use the token configured in the gitlab integration for the matching host, to try and set the user logged in via `repoUrlPicker` `requestUserCredentials` OAuth flow as owner of the repository created in GitLab. From e8d661d94c1fdc3258f68e32514110e9f8f96586 Mon Sep 17 00:00:00 2001 From: Matteo Silvestri Date: Mon, 4 Jul 2022 15:57:58 +0200 Subject: [PATCH 4/6] update api-report.md Signed-off-by: Matteo Silvestri --- plugins/scaffolder-backend/api-report.md | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 8d302e69f6..2053f01e39 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -388,6 +388,7 @@ export function createPublishGitlabAction(options: { gitCommitMessage?: string | undefined; gitAuthorName?: string | undefined; gitAuthorEmail?: string | undefined; + setUserAsOwner?: boolean | undefined; }>; // @public From e96a7cf1471f452de3387f3c07b22fa2e18e90df Mon Sep 17 00:00:00 2001 From: Matteo Silvestri Date: Tue, 5 Jul 2022 13:47:32 +0200 Subject: [PATCH 5/6] code linting Signed-off-by: Matteo Silvestri --- .../src/scaffolder/actions/builtin/publish/gitlab.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts index e36bb0aa68..80d082bc4b 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts @@ -174,7 +174,7 @@ export function createPublishGitlabAction(options: { if (setUserAsOwner && integrationConfig.config.token) { const adminClient = new Gitlab({ host: integrationConfig.config.baseUrl, - ['token']: integrationConfig.config.token, + token: integrationConfig.config.token, }); await adminClient.ProjectMembers.add(projectId, userId, 50); From 27d130b232623d5577bed13b4d2e06872592f84e Mon Sep 17 00:00:00 2001 From: Matteo Silvestri Date: Tue, 5 Jul 2022 13:48:27 +0200 Subject: [PATCH 6/6] add comment explaining GitLab behaviour Signed-off-by: Matteo Silvestri --- .../src/scaffolder/actions/builtin/publish/gitlab.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts index 80d082bc4b..4fe55b77a2 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/gitlab.ts @@ -171,6 +171,12 @@ export function createPublishGitlabAction(options: { visibility: repoVisibility, }); + // When setUserAsOwner is true the input token is expected to come from an unprivileged user GitLab + // OAuth flow. In this case GitLab works in a way that allows the unprivileged user to + // create the repository, but not to push the default protected branch (e.g. master). + // In order to set the user as owner of the newly created repository we need to check that the + // GitLab integration configuration for the matching host contains a token and use + // such token to bootstrap a new privileged client. if (setUserAsOwner && integrationConfig.config.token) { const adminClient = new Gitlab({ host: integrationConfig.config.baseUrl,