Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add flag to allow organization members #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ The CLA Signature Bot has the option to additionally store the signatures on the
| `path-to-signatures` | _optional_ | Path to the signature file in the repository. Default is `./signatures/cla.json`. |
| `branch` | _optional_ | Repository branch to store the signature file. Default is `master` |
| `whitelist` | _optional_ | Comma-separated list of accounts to [ignore](https://github.com/roblox/cla-assistant#Whitelist-Accounts). Example: `user1,user2,bot*` |
| `allow-organization-members` | _optional_ | Automatically allows any users in the same organization as the repository. Default is `false`. |
| `blockchain-storage-flag` | _optional_ | Whether to store the Contributor's signature data in the Ethereum blockchain. May be `true` or `false`. Default is `false`. |
| `blockchain-webhook-endpoint` | _optional_ | The URL to post the blockchain request to. Can be used when running your own [blockchain-services](https://github.com/cla-assistant/blockchain-services) docker container. |
| `use-remote-repo` | _optional_ | Whether to use an alternate repository for storing the signature file than the one running the workflow. If `true` the remote repo name and PAT must be provided. Default is `false`. |
Expand Down
65 changes: 63 additions & 2 deletions __tests__/claRunner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,31 @@ function getPullCheckRunnerMock(settings: IInputSettings): [PullCheckRunner, any
return [pullCheckRunner, rerunLastCheckSpy];
}

function getMockOrganizationMembers(logins: Array<string>) {
return logins.map(login => {
return {
login,
id: 1,
node_id: '',
avatar_url: '',
gravatar_id: '',
url: '',
html_url: '',
followers_url: '',
following_url: '',
gists_url: '',
starred_url: '',
subscriptions_url: '',
organizations_url: '',
repos_url: '',
events_url: '',
received_events_url: '',
type: 'User',
site_admin: false
}
})
}

afterEach(() => {
jest.resetAllMocks();
});
Expand All @@ -115,7 +140,8 @@ it("Successfully constructs with full or empty settings", () => {
remoteRepositoryOwner: "owner",
signatureRegex: /.*/,
signatureText: "signature",
whitelist: ""
whitelist: "",
allowOrganizationMembers: false
} as IInputSettings;

const runner = new ClaRunner({inputSettings: fullSettings});
Expand Down Expand Up @@ -154,7 +180,42 @@ it('Locks the PR when the PR is closed', async () => {
expect(lockCommentSpy).toHaveBeenCalledTimes(1);
});

it('Returns early if there are no authors', async () => {
it("Returns early if all authors are organization members and 'allowOrganizationMembers' is enabled", async () => {
const listMembersSpy = jest.spyOn(mockGitHub.orgs, 'listMembers')
.mockImplementation(async (params) => ({
url: "",
data: getMockOrganizationMembers(['SomeDude', 'SomeDudette', 'SomeEnby']),
status: 200,
headers: {
date: "",
"x-Octokit-media-type": "",
"x-Octokit-request-id": "",
"x-ratelimit-limit": "",
"x-ratelimit-remaining": "",
"x-ratelimit-reset": "",
link: "",
"last-modified": "",
etag: "",
status: "200",
},
[Symbol.iterator]: () => ({next: () => { return { value: null, done: true}}}),
}));
const settings = getSettings();
settings.allowOrganizationMembers = true

const [authors, getAuthorsSpy] = getPullAuthorsMock(settings);

const runner = new ClaRunner({
inputSettings: settings,
pullAuthors: authors
});
const result = await runner.execute();

expect(result).toStrictEqual(true);
expect(getAuthorsSpy).toHaveBeenCalledTimes(1);
});

it('Returns early if all authors are on whitelist', async () => {
const settings = getSettings();
const whitelist = new Whitelist("SomeDude,SomeDudette,SomeEnby");

Expand Down
1 change: 1 addition & 0 deletions __tests__/inputHelper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ it('sets defaults', () => {
expect(settings.repositoryAccessToken).toBe(settings.localAccessToken);
expect(settings.claFilePath).toBeTruthy();
expect(settings.whitelist).toBeFalsy();
expect(settings.allowOrganizationMembers).toBeFalsy();
expect(settings.emptyCommitFlag).toBe(false);

expect(settings.octokitRemote).toBeTruthy();
Expand Down
3 changes: 3 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ inputs:
whitelist:
description: "Comma-separated list of users to exclude from CLA requirement. Can use * characters for wildcards."
default: ""
allow-organization-members:
description: "(Optional) Automatically allows any users in the same organization as the repository"
default: false
signature-text:
description: "The text to require as a signature."
signature-regex:
Expand Down
2 changes: 1 addition & 1 deletion lib/index.js

Large diffs are not rendered by default.

31 changes: 23 additions & 8 deletions src/claRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,26 +49,30 @@ export class ClaRunner {
return true;
}

// Just drop whitelisted authors entirely, no sense in processing them.
let rawAuthors: Author[] = await this.pullAuthors.getAuthors();
rawAuthors = rawAuthors.filter(a => !this.whitelist.isUserWhitelisted(a));
// Just drop whitelisted authors and organization members entirely, no sense in processing them.
const [rawAuthors, organizationMembers]: [Author[], String[]] = await Promise.all([
this.pullAuthors.getAuthors(),
this.getOrganizationMembers()
]);

if (rawAuthors.length === 0) {
const requiredAuthors = rawAuthors.filter(a => !this.whitelist.isUserWhitelisted(a) && !organizationMembers.includes(a.name));

if (requiredAuthors.length === 0) {
core.info("No committers left after whitelisting. Approving pull request.");
return true;
}

core.debug(`Found a total of ${rawAuthors.length} authors after whitelisting.`);
core.debug(`Authors: ${rawAuthors.map(n => n.name).join(', ')}`);
core.debug(`Found a total of ${requiredAuthors.length} authors after whitelisting.`);
core.debug(`Authors: ${requiredAuthors.map(n => n.name).join(', ')}`);

const claFile = await this.claFileRepository.getClaFile();
let authorMap = claFile.mapSignedAuthors(rawAuthors);
let authorMap = claFile.mapSignedAuthors(requiredAuthors);

let newSignature = claFile.addSignature(await this.pullComments.getNewSignatures(authorMap));
if (newSignature.length > 0) {
const newNames = newSignature.map(s => s.name).join(', ');
core.debug(`Found new signatures: ${newNames}.`)
authorMap = claFile.mapSignedAuthors(rawAuthors);
authorMap = claFile.mapSignedAuthors(requiredAuthors);
await Promise.all([
this.claFileRepository.commitClaFile(`Add ${newNames}.`),
this.blockchainPoster.postToBlockchain(newSignature),
Expand Down Expand Up @@ -100,4 +104,15 @@ export class ClaRunner {
core.error(`Failed to lock pull request #${this.settings.pullRequestNumber}.`);
}
}

private async getOrganizationMembers(): Promise<Array<String>> {
if (!this.settings.allowOrganizationMembers) {
return [];
}

const response = await this.settings.octokitLocal.orgs.listMembers({
org: this.settings.localRepositoryOwner,
});
return response.data.map(user => user.login);
}
}
1 change: 1 addition & 0 deletions src/inputHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export function getInputs(): IInputSettings {
settings.claFilePath = core.getInput("path-to-signatures") || "signatures/cla.json";
settings.branch = core.getInput("branch") || "master";
settings.whitelist = core.getInput("whitelist") || "";
settings.allowOrganizationMembers = (core.getInput('allow-organization-members') || 'FALSE').toUpperCase() === 'TRUE';

settings.signatureText = core.getInput("signature-text") || "I have read the CLA Document and I hereby sign the CLA";
settings.signatureRegex = new RegExp(core.getInput("signature-regex") || /^.*I\s*HAVE\s*READ\s*THE\s*CLA\s*DOCUMENT\s*AND\s*I\s*HEREBY\s*SIGN\s*THE\s*CLA/);
Expand Down
5 changes: 5 additions & 0 deletions src/inputSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ export interface IInputSettings {
*/
whitelist: string

/**
* Whether accounts in the same organization as the repository should be allowed automatically
*/
allowOrganizationMembers: boolean

/**
* The regex to search PR comments for as a signature.
*/
Expand Down