Skip to content

Commit

Permalink
Add flag to allow organization members
Browse files Browse the repository at this point in the history
The new `allowOrganizationMembers` flag will automatically allow all
users in the same organization as the repository. It will be as though
all organization members are on the `whitelist`.

If this is enabled for a repository not in an organization, an error
will be thrown.

This input parameter defaults to `false`. Tests and documentation have
been updated.

Fixes #4
  • Loading branch information
Gudahtt committed Aug 26, 2020
1 parent 6595ecd commit dae370d
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 11 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ The CLA Signature Bot has the option to additionally store the signatures on the
| `branch` | _optional_ | Repository branch to store the signature file. Default is `master` |
| `allowlist` | _optional_ | Comma-separated list of accounts to [ignore](https://github.com/roblox/cla-assistant#Allowlist-Accounts). Example: `user1,user2,bot*` |
| `whitelist` | _optional_ | (Deprecated) Alias of 'allowlist' |
| `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
66 changes: 64 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,9 @@ it("Successfully constructs with full or empty settings", () => {
remoteRepositoryOwner: "owner",
signatureRegex: /.*/,
signatureText: "signature",
allowlist: ""
allowlist: "",
whitelist: "",
allowOrganizationMembers: false
} as IInputSettings;

const runner = new ClaRunner({inputSettings: fullSettings});
Expand Down Expand Up @@ -154,7 +181,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 allowlist = new Allowlist("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.allowlist).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 @@ -35,6 +35,9 @@ inputs:
whitelist:
description: "(Deprecated) Alias of 'allowlist'."
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 @@ -53,26 +53,30 @@ export class ClaRunner {
return true
}

// Just drop allowlisted authors entirely, no sense in processing them.
let rawAuthors: Author[] = await this.pullAuthors.getAuthors();
rawAuthors = rawAuthors.filter(a => !this.allowlist.isUserAllowlisted(a));
// Just drop allowlisted 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.allowlist.isUserAllowlisted(a) && !organizationMembers.includes(a.name));

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

core.debug(`Found a total of ${rawAuthors.length} authors after allowlisting.`);
core.debug(`Authors: ${rawAuthors.map(n => n.name).join(', ')}`);
core.debug(`Found a total of ${requiredAuthors.length} authors after allowlisting.`);
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 @@ -104,4 +108,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.allowlist = core.getInput("allowlist") || 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 @@ -76,6 +76,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

0 comments on commit dae370d

Please sign in to comment.