From 9482b58109007453ec1cabe27ffa6996c1e20786 Mon Sep 17 00:00:00 2001 From: hollywood Date: Tue, 26 Jun 2018 16:30:30 -0400 Subject: [PATCH 1/7] Initial method to detect repo creator removal --- lib/RemoveOutsideCollaborators.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/RemoveOutsideCollaborators.js b/lib/RemoveOutsideCollaborators.js index 4f2e443..74d81d1 100644 --- a/lib/RemoveOutsideCollaborators.js +++ b/lib/RemoveOutsideCollaborators.js @@ -35,6 +35,8 @@ class RemoveOutsideCollaborators { update () { var configParams = Object.assign({}, require('./defaults'), this.config || {}) + if (this.isRepoAddedByOrgMember(this.payload)) return + if (this.isRemoveCollaboratorsDisabled(configParams.enableCollaboratorRemoval, configParams.monitorOnly)) return if (this.isExcludedCollaborator(configParams.excludeCollaborators)) return @@ -46,6 +48,13 @@ class RemoveOutsideCollaborators { return this.executeMonitorOnly(configParams) } + isRepoAddedByOrgMember(payload) { + if (payload.member.login === payload.sender.login) { + return true + } + return false + } + isRemoveCollaboratorsDisabled (enableCollaboratorRemoval, monitorOnly) { if (this.payload.action === 'added' && !enableCollaboratorRemoval && !monitorOnly) { return true From 3da8108d9898f329e8ece20d04cc332c6e1e6c83 Mon Sep 17 00:00:00 2001 From: hollywood Date: Tue, 26 Jun 2018 17:00:26 -0400 Subject: [PATCH 2/7] Altered the membership function --- lib/RemoveOutsideCollaborators.js | 6 +++++- test/lib/plugins/RemoveOutsideCollaborators.test.js | 11 ++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/lib/RemoveOutsideCollaborators.js b/lib/RemoveOutsideCollaborators.js index 74d81d1..79bcfea 100644 --- a/lib/RemoveOutsideCollaborators.js +++ b/lib/RemoveOutsideCollaborators.js @@ -49,7 +49,11 @@ class RemoveOutsideCollaborators { } isRepoAddedByOrgMember(payload) { - if (payload.member.login === payload.sender.login) { + const org = this.payload.organization.login + const username = this.payload.member.login + const orgMembership = this.github.orgs.getOrgMembership({org, username}) + + if (orgMembership.state === 'active') { return true } return false diff --git a/test/lib/plugins/RemoveOutsideCollaborators.test.js b/test/lib/plugins/RemoveOutsideCollaborators.test.js index c104208..4d52e03 100644 --- a/test/lib/plugins/RemoveOutsideCollaborators.test.js +++ b/test/lib/plugins/RemoveOutsideCollaborators.test.js @@ -7,7 +7,7 @@ describe('removeOutsideCollaborators', () => { return new RemoveOutsideCollaborators(github, {owner: 'Hollywood', repo: 'test', username: 'Usr45'}, payload, console, yaml) } - let payloadRemoveCollaborator = { action: 'added', member: { login: 'Usr45' }, repository: { name: 'test', owner: { login: 'Usr45' } }, sender: { login: 'Usr45' } } + let payloadRemoveCollaborator = { action: 'added', member: { login: 'Usr45' }, repository: { name: 'test', owner: { login: 'Usr45' } }, organization: { login: 'Albatoss'}, sender: { login: 'Usr45' } } beforeEach(() => { github = { @@ -17,6 +17,9 @@ describe('removeOutsideCollaborators', () => { }, issues: { create: jest.fn().mockImplementation(() => Promise.resolve([])) + }, + orgs: { + getOrgMembership: jest.fn().mockImplementation(() => Promise.resolve([])) } } }) @@ -26,14 +29,20 @@ describe('removeOutsideCollaborators', () => { var spyMonitorOnly beforeEach(() => { + spyIsOrgMember = jest.spyOn(RemoveOutsideCollaborators.prototype, 'isRepoAddedByOrgMember') spyExecuteRemoval = jest.spyOn(RemoveOutsideCollaborators.prototype, 'executeRemoval') spyMonitorOnly = jest.spyOn(RemoveOutsideCollaborators.prototype, 'executeMonitorOnly') }) afterEach(function () { + spyIsOrgMember.mockClear() spyExecuteRemoval.mockClear() spyMonitorOnly.mockClear() }) + it('repo created by an org member', () => { + expect(spyExecuteRemoval).not.toHaveBeenCalled() + }) + it('added and enableCollaboratorRemoval is disabled', () => { const config = configure(payloadRemoveCollaborator, ` monitorOnly: true From 1dad975e71ba8eab1d2022542e66845183700271 Mon Sep 17 00:00:00 2001 From: hollywood Date: Wed, 27 Jun 2018 16:40:23 -0400 Subject: [PATCH 3/7] Added async call to get org membership --- lib/RemoveOutsideCollaborators.js | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/RemoveOutsideCollaborators.js b/lib/RemoveOutsideCollaborators.js index 79bcfea..5b21692 100644 --- a/lib/RemoveOutsideCollaborators.js +++ b/lib/RemoveOutsideCollaborators.js @@ -48,17 +48,21 @@ class RemoveOutsideCollaborators { return this.executeMonitorOnly(configParams) } - isRepoAddedByOrgMember(payload) { - const org = this.payload.organization.login - const username = this.payload.member.login - const orgMembership = this.github.orgs.getOrgMembership({org, username}) + async isRepoAddedByOrgMember() { + const orgParams = { + org: this.payload.organization.login, + username: this.payload.member.login + } + const toOrgParams = Object.assign({}, orgParams || {}) - if (orgMembership.state === 'active') { - return true + try { + const payload = await this.github.orgs.getOrgMembership(toOrgParams) + return payload.state === 'active' + } catch (e) { + console.error } - return false } - + isRemoveCollaboratorsDisabled (enableCollaboratorRemoval, monitorOnly) { if (this.payload.action === 'added' && !enableCollaboratorRemoval && !monitorOnly) { return true From 8dc2e6a3883effa06906acbb7203aa3688365d7a Mon Sep 17 00:00:00 2001 From: hollywood Date: Wed, 27 Jun 2018 19:08:16 -0400 Subject: [PATCH 4/7] Fixed issue where function wasn't returning meaningful data --- lib/RemoveOutsideCollaborators.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/RemoveOutsideCollaborators.js b/lib/RemoveOutsideCollaborators.js index 5b21692..6b06169 100644 --- a/lib/RemoveOutsideCollaborators.js +++ b/lib/RemoveOutsideCollaborators.js @@ -55,12 +55,16 @@ class RemoveOutsideCollaborators { } const toOrgParams = Object.assign({}, orgParams || {}) - try { - const payload = await this.github.orgs.getOrgMembership(toOrgParams) - return payload.state === 'active' - } catch (e) { - console.error - } + //If the user doesn't exist in the org, this function returns a 404 error instead of any type of meaningful message + //Capturing the error seems to be the only way to determine whether or not the user exists. + await this.github.orgs.getOrgMembership(toOrgParams).then(({ data }) => { + + //We also don't want Pending members + const isMember = data.state === 'pending' ? false : true + return isMember + }).catch(() => { + return false + }) } isRemoveCollaboratorsDisabled (enableCollaboratorRemoval, monitorOnly) { From 46bcdcf49fb45b45ede3c857511bd82e974de588 Mon Sep 17 00:00:00 2001 From: hollywood Date: Wed, 27 Jun 2018 20:22:13 -0400 Subject: [PATCH 5/7] Used the wrong org membership method --- lib/RemoveOutsideCollaborators.js | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/lib/RemoveOutsideCollaborators.js b/lib/RemoveOutsideCollaborators.js index 6b06169..30039e9 100644 --- a/lib/RemoveOutsideCollaborators.js +++ b/lib/RemoveOutsideCollaborators.js @@ -48,24 +48,21 @@ class RemoveOutsideCollaborators { return this.executeMonitorOnly(configParams) } - async isRepoAddedByOrgMember() { - const orgParams = { - org: this.payload.organization.login, - username: this.payload.member.login - } - const toOrgParams = Object.assign({}, orgParams || {}) - - //If the user doesn't exist in the org, this function returns a 404 error instead of any type of meaningful message - //Capturing the error seems to be the only way to determine whether or not the user exists. - await this.github.orgs.getOrgMembership(toOrgParams).then(({ data }) => { - - //We also don't want Pending members - const isMember = data.state === 'pending' ? false : true - return isMember - }).catch(() => { - return false - }) +async isRepoAddedByOrgMember() { + const orgParams = { + org: this.payload.organization.login, + username: this.payload.member.login + } + const toOrgParams = Object.assign({}, orgParams || {}) + + try { + const payload = await this.github.orgs.checkMembership(toOrgParams) + return payload.data.state === 'pending' ? false : true + } catch (e) { + //A non org member returns a 404 causing an error to be thrown + return false } +} isRemoveCollaboratorsDisabled (enableCollaboratorRemoval, monitorOnly) { if (this.payload.action === 'added' && !enableCollaboratorRemoval && !monitorOnly) { From 0565ef22f5c0631011471f62f17ddbb9e112789e Mon Sep 17 00:00:00 2001 From: hollywood Date: Wed, 27 Jun 2018 20:56:36 -0400 Subject: [PATCH 6/7] Code workflow changes --- lib/RemoveOutsideCollaborators.js | 76 +++++++++++++++++-------------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/lib/RemoveOutsideCollaborators.js b/lib/RemoveOutsideCollaborators.js index 30039e9..e872a26 100644 --- a/lib/RemoveOutsideCollaborators.js +++ b/lib/RemoveOutsideCollaborators.js @@ -1,30 +1,35 @@ const yaml = require('js-yaml') const noOrgConfig = false +process.on('uncaughtException', function (err) { + console.error(err); + console.log("Node NOT Exiting..."); +}); class RemoveOutsideCollaborators { - static analyze (github, repo, payload, logger) { + + static analyze(github, repo, payload, logger) { const defaults = require('./defaults') const orgRepo = (process.env.ORG_WIDE_REPO_NAME) ? process.env.ORG_WIDE_REPO_NAME : defaults.ORG_WIDE_REPO_NAME const filename = (process.env.FILE_NAME) ? process.env.FILE_NAME : defaults.FILE_NAME return github.repos.getContent({ - owner: repo.owner, - repo: orgRepo, - path: filename - }).catch(() => ({ - noOrgConfig - })) + owner: repo.owner, + repo: orgRepo, + path: filename + }).catch(() => ({ + noOrgConfig + })) .then((orgConfig) => { if ('noOrgConfig' in orgConfig) { - return new RemoveOutsideCollaborators(github, repo, payload, logger, '').update() + return new RemoveOutsideCollaborators(github, repo, payload, logger, '').update() } else { const content = Buffer.from(orgConfig.data.content, 'base64').toString() - return new RemoveOutsideCollaborators(github, repo, payload, logger, content).update() + return new RemoveOutsideCollaborators(github, repo, payload, logger, content).update() } }) } - constructor (github, repo, payload, logger, config) { + constructor(github, repo, payload, logger, config) { this.github = github this.repo = repo this.payload = payload @@ -32,11 +37,15 @@ class RemoveOutsideCollaborators { this.config = yaml.safeLoad(config) } - update () { - var configParams = Object.assign({}, require('./defaults'), this.config || {}) - + update() { if (this.isRepoAddedByOrgMember(this.payload)) return + this.executeWorkflow() + } + + executeWorkflow() { + var configParams = Object.assign({}, require('./defaults'), this.config || {}) + if (this.isRemoveCollaboratorsDisabled(configParams.enableCollaboratorRemoval, configParams.monitorOnly)) return if (this.isExcludedCollaborator(configParams.excludeCollaborators)) return @@ -48,55 +57,54 @@ class RemoveOutsideCollaborators { return this.executeMonitorOnly(configParams) } -async isRepoAddedByOrgMember() { - const orgParams = { - org: this.payload.organization.login, - username: this.payload.member.login - } - const toOrgParams = Object.assign({}, orgParams || {}) + async isRepoAddedByOrgMember() { + var isMember = false + const orgParams = { + org: this.payload.organization.login, + username: this.payload.member.login + } + const toOrgParams = Object.assign({}, orgParams || {}) - try { - const payload = await this.github.orgs.checkMembership(toOrgParams) - return payload.data.state === 'pending' ? false : true - } catch (e) { - //A non org member returns a 404 causing an error to be thrown - return false + this.github.orgs.checkMembership(toOrgParams).then(({ data }) => { + return isMember === 'pending' ? false : true + }).catch(() => { + this.executeWorkflow() + }) } -} - - isRemoveCollaboratorsDisabled (enableCollaboratorRemoval, monitorOnly) { + + isRemoveCollaboratorsDisabled(enableCollaboratorRemoval, monitorOnly) { if (this.payload.action === 'added' && !enableCollaboratorRemoval && !monitorOnly) { return true } return false } - isExcludedCollaborator (excludeCollaborators) { + isExcludedCollaborator(excludeCollaborators) { if (excludeCollaborators.includes(this.payload.member.login)) { return true } return false } - executeRemoval (configParams) { + executeRemoval(configParams) { var issueBody = this.formIssueBody(`

${configParams.removedIssueBody}

`, configParams.ccList) this.createIssue(configParams.removedIssueTitle, issueBody) this.removeCollaborator() } - executeMonitorOnly (configParams) { + executeMonitorOnly(configParams) { var issueBody = this.formIssueBody(`

${configParams.monitorIssueBody}

`, configParams.ccList) this.createIssue(configParams.monitorIssueTitle, issueBody) } - formIssueBody (body, ccList) { + formIssueBody(body, ccList) { const owner = this.payload.sender.login var issueBody = body + `\n\n

Collaborator added: ${owner}

\n\n---` issueBody += (ccList) ? `\n\n
/cc ${ccList}
` : '' return issueBody } - createIssue (title, body) { + createIssue(title, body) { const issueParams = { title: title, body: body @@ -105,7 +113,7 @@ async isRepoAddedByOrgMember() { this.github.issues.create(createIssueParams) } - removeCollaborator () { + removeCollaborator() { const removeParams = { owner: this.payload.repository.owner.login, repo: this.repo.repo, From 0cc967f3f357b83f137fe48e67cf20680e097f58 Mon Sep 17 00:00:00 2001 From: hollywood Date: Wed, 27 Jun 2018 21:05:26 -0400 Subject: [PATCH 7/7] More code formatting, removing useless debug blocks --- lib/RemoveOutsideCollaborators.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/RemoveOutsideCollaborators.js b/lib/RemoveOutsideCollaborators.js index e872a26..1845333 100644 --- a/lib/RemoveOutsideCollaborators.js +++ b/lib/RemoveOutsideCollaborators.js @@ -1,9 +1,5 @@ const yaml = require('js-yaml') const noOrgConfig = false -process.on('uncaughtException', function (err) { - console.error(err); - console.log("Node NOT Exiting..."); -}); class RemoveOutsideCollaborators { @@ -21,10 +17,10 @@ class RemoveOutsideCollaborators { })) .then((orgConfig) => { if ('noOrgConfig' in orgConfig) { - return new RemoveOutsideCollaborators(github, repo, payload, logger, '').update() + return new RemoveOutsideCollaborators(github, repo, payload, logger, '').update() } else { const content = Buffer.from(orgConfig.data.content, 'base64').toString() - return new RemoveOutsideCollaborators(github, repo, payload, logger, content).update() + return new RemoveOutsideCollaborators(github, repo, payload, logger, content).update() } }) } @@ -44,7 +40,7 @@ class RemoveOutsideCollaborators { } executeWorkflow() { - var configParams = Object.assign({}, require('./defaults'), this.config || {}) + var configParams = Object.assign({}, require('./defaults'), this.config || {}) if (this.isRemoveCollaboratorsDisabled(configParams.enableCollaboratorRemoval, configParams.monitorOnly)) return @@ -65,7 +61,9 @@ class RemoveOutsideCollaborators { } const toOrgParams = Object.assign({}, orgParams || {}) - this.github.orgs.checkMembership(toOrgParams).then(({ data }) => { + this.github.orgs.checkMembership(toOrgParams).then(({ + data + }) => { return isMember === 'pending' ? false : true }).catch(() => { this.executeWorkflow()