diff --git a/README.md b/README.md index 46fcb82..6ce0701 100644 --- a/README.md +++ b/README.md @@ -22,20 +22,31 @@ Default configuration: commands: close: enabled: false + permission: all label: allowedLabels: [] # any label is allowed enabled: false + permission: all removeLabel: allowedLabels: [] # any label is allowed enabled: false + permission: all reopen: enabled: false + permission: all reviewer: enabled: false + permission: all transfer: enabled: false + permission: all ``` +`permission` is optional and defaults to `all`. Supported values are: + +- `all` - anyone who can comment can use the command +- `member` - only organization members and owners can use the command + ## Getting started First you will need to create a GitHub app. Add the permissions required for the commands you are using (see next section), and tick "Subscribe to events" > "Issue comment" diff --git a/app/command-enabled.js b/app/command-enabled.js index ff84516..10a99a6 100644 --- a/app/command-enabled.js +++ b/app/command-enabled.js @@ -2,6 +2,8 @@ const enabled = { enabled: true, }; +const orgMemberAssociations = new Set(["MEMBER", "OWNER"]); + function notEnabled(command) { return { enabled: false, @@ -9,25 +11,66 @@ function notEnabled(command) { }; } +function invalidPermission(command, permission) { + return { + enabled: false, + error: `The \`${command}\` command has an unsupported permission level \`${permission}\``, + }; +} + +function notPermitted(command) { + return { + enabled: false, + error: `The \`${command}\` command is restricted to organization members`, + }; +} + function trimLabels(labels) { return labels.map((it) => it.trim()); } -export function transferEnabled(config) { - if (!config.commands.transfer.enabled) { +function permissionEnabled(command, commandConfig, authorAssociation) { + const permission = commandConfig.permission ?? "all"; + + if (permission === "all") { + return enabled; + } + + if (permission === "member") { + return orgMemberAssociations.has(authorAssociation) + ? enabled + : notPermitted(command); + } + + return invalidPermission(command, permission); +} + +export function transferEnabled(config, authorAssociation) { + const transferConfig = config.commands.transfer; + + if (!transferConfig.enabled) { return notEnabled("transfer"); } - return enabled; + return permissionEnabled("transfer", transferConfig, authorAssociation); } -export function labelEnabled(config, labels) { +export function labelEnabled(config, labels, authorAssociation) { const labelConfig = config.commands.label; if (!labelConfig.enabled) { return notEnabled("label"); } + const permissionResult = permissionEnabled( + "label", + labelConfig, + authorAssociation, + ); + if (!permissionResult.enabled) { + return permissionResult; + } + const allowedLabels = trimLabels(labelConfig.allowedLabels); if ( // if length is = 0 then all labels are allowed @@ -45,28 +88,41 @@ export function labelEnabled(config, labels) { return enabled; } -export function closeEnabled(config) { - if (!config.commands.close.enabled) { +export function closeEnabled(config, authorAssociation) { + const closeConfig = config.commands.close; + + if (!closeConfig.enabled) { return notEnabled("close"); } - return enabled; + return permissionEnabled("close", closeConfig, authorAssociation); } -export function reopenEnabled(config) { - if (!config.commands.reopen.enabled) { +export function reopenEnabled(config, authorAssociation) { + const reopenConfig = config.commands.reopen; + + if (!reopenConfig.enabled) { return notEnabled("reopen"); } - return enabled; + return permissionEnabled("reopen", reopenConfig, authorAssociation); } -export function removeLabelEnabled(config, labels) { +export function removeLabelEnabled(config, labels, authorAssociation) { const labelConfig = config.commands.removeLabel; if (!labelConfig.enabled) { return notEnabled("remove-label"); } + const permissionResult = permissionEnabled( + "remove-label", + labelConfig, + authorAssociation, + ); + if (!permissionResult.enabled) { + return permissionResult; + } + const allowedLabels = trimLabels(labelConfig.allowedLabels); if ( // if length is = 0 then all labels are allowed @@ -84,10 +140,12 @@ export function removeLabelEnabled(config, labels) { return enabled; } -export function reviewerEnabled(config) { - if (!config.commands.reviewer.enabled) { +export function reviewerEnabled(config, authorAssociation) { + const reviewerConfig = config.commands.reviewer; + + if (!reviewerConfig.enabled) { return notEnabled("reviewer"); } - return enabled; + return permissionEnabled("reviewer", reviewerConfig, authorAssociation); } diff --git a/app/command-enabled.test.js b/app/command-enabled.test.js index 929774c..702617d 100644 --- a/app/command-enabled.test.js +++ b/app/command-enabled.test.js @@ -8,6 +8,245 @@ import { } from "./command-enabled.js"; describe("command-enabled", () => { + describe("permission levels", () => { + test("defaults to all permissions when not configured", () => { + const sut = closeEnabled( + { + commands: { + close: { + enabled: true, + }, + }, + }, + "CONTRIBUTOR", + ); + + expect(sut.enabled).toEqual(true); + }); + + test.each([ + [ + "transfer", + () => + transferEnabled( + { + commands: { + transfer: { + enabled: true, + permission: "member", + }, + }, + }, + "MEMBER", + ), + ], + [ + "close", + () => + closeEnabled( + { + commands: { + close: { + enabled: true, + permission: "member", + }, + }, + }, + "MEMBER", + ), + ], + [ + "reopen", + () => + reopenEnabled( + { + commands: { + reopen: { + enabled: true, + permission: "member", + }, + }, + }, + "MEMBER", + ), + ], + [ + "label", + () => + labelEnabled( + { + commands: { + label: { + enabled: true, + permission: "member", + allowedLabels: [], + }, + }, + }, + ["label1"], + "MEMBER", + ), + ], + [ + "remove-label", + () => + removeLabelEnabled( + { + commands: { + removeLabel: { + enabled: true, + permission: "member", + allowedLabels: [], + }, + }, + }, + ["label1"], + "MEMBER", + ), + ], + [ + "reviewer", + () => + reviewerEnabled( + { + commands: { + reviewer: { + enabled: true, + permission: "member", + }, + }, + }, + "MEMBER", + ), + ], + ])("allows organization members to use %s", (_, sutFactory) => { + expect(sutFactory().enabled).toEqual(true); + }); + + test("allows organization owners to use member-only commands", () => { + const sut = reviewerEnabled( + { + commands: { + reviewer: { + enabled: true, + permission: "member", + }, + }, + }, + "OWNER", + ); + + expect(sut.enabled).toEqual(true); + }); + + test.each([ + [ + "transfer", + () => + transferEnabled( + { + commands: { + transfer: { + enabled: true, + permission: "member", + }, + }, + }, + "CONTRIBUTOR", + ), + ], + [ + "close", + () => + closeEnabled( + { + commands: { + close: { + enabled: true, + permission: "member", + }, + }, + }, + "CONTRIBUTOR", + ), + ], + [ + "reopen", + () => + reopenEnabled( + { + commands: { + reopen: { + enabled: true, + permission: "member", + }, + }, + }, + "CONTRIBUTOR", + ), + ], + [ + "label", + () => + labelEnabled( + { + commands: { + label: { + enabled: true, + permission: "member", + allowedLabels: [], + }, + }, + }, + ["label1"], + "CONTRIBUTOR", + ), + ], + [ + "remove-label", + () => + removeLabelEnabled( + { + commands: { + removeLabel: { + enabled: true, + permission: "member", + allowedLabels: [], + }, + }, + }, + ["label1"], + "CONTRIBUTOR", + ), + ], + [ + "reviewer", + () => + reviewerEnabled( + { + commands: { + reviewer: { + enabled: true, + permission: "member", + }, + }, + }, + "CONTRIBUTOR", + ), + ], + ])( + "blocks non-members from using %s when permission is member", + (command, sutFactory) => { + const sut = sutFactory(); + + expect(sut).toEqual({ + enabled: false, + error: `The \`${command}\` command is restricted to organization members`, + }); + }, + ); + }); + describe("transferEnabled", () => { test("is enabled when config is enabled", () => { const sut = transferEnabled({ diff --git a/app/commands/close-command.js b/app/commands/close-command.js index d9d0c4a..ca06aa8 100644 --- a/app/commands/close-command.js +++ b/app/commands/close-command.js @@ -4,6 +4,7 @@ import { closeEnabled } from "../command-enabled.js"; import { closeIssue } from "../github.js"; import { getLogger } from "../logger.js"; import { + extractAuthorAssociation, extractBody, extractHtmlUrl, extractLabelableId, @@ -21,7 +22,7 @@ export class CloseCommand extends Command { } enabled(config) { - return closeEnabled(config); + return closeEnabled(config, extractAuthorAssociation(this.payload)); } async run(authToken) { diff --git a/app/commands/label-command.js b/app/commands/label-command.js index 3737eb5..a5de4e8 100644 --- a/app/commands/label-command.js +++ b/app/commands/label-command.js @@ -6,6 +6,7 @@ import { extractCommaSeparated } from "../converters.js"; import { getLogger } from "../logger.js"; import { + extractAuthorAssociation, extractBody, extractHtmlUrl, extractLabelableId, @@ -24,7 +25,7 @@ export class LabelCommand extends Command { enabled(config) { const labels = extractCommaSeparated(this.matches()[1]); - return labelEnabled(config, labels); + return labelEnabled(config, labels, extractAuthorAssociation(this.payload)); } async run(authToken) { diff --git a/app/commands/remove-label-command.js b/app/commands/remove-label-command.js index 9b09517..0a9b097 100644 --- a/app/commands/remove-label-command.js +++ b/app/commands/remove-label-command.js @@ -5,6 +5,7 @@ import { Command } from "./command.js"; import { extractCommaSeparated } from "../converters.js"; import { getLogger } from "../logger.js"; import { + extractAuthorAssociation, extractBody, extractHtmlUrl, extractLabelableId, @@ -23,7 +24,11 @@ export class RemoveLabelCommand extends Command { enabled(config) { const removeLabels = extractCommaSeparated(this.matches()[1]); - return removeLabelEnabled(config, removeLabels); + return removeLabelEnabled( + config, + removeLabels, + extractAuthorAssociation(this.payload), + ); } async run(authToken) { diff --git a/app/commands/reopen-command.js b/app/commands/reopen-command.js index 728a19e..e2b6511 100644 --- a/app/commands/reopen-command.js +++ b/app/commands/reopen-command.js @@ -4,6 +4,7 @@ import { reopenIssue } from "../github.js"; import { Command } from "./command.js"; import { getLogger } from "../logger.js"; import { + extractAuthorAssociation, extractBody, extractHtmlUrl, extractLabelableId, @@ -21,7 +22,7 @@ export class ReopenCommand extends Command { } enabled(config) { - return reopenEnabled(config); + return reopenEnabled(config, extractAuthorAssociation(this.payload)); } async run(authToken) { diff --git a/app/commands/reviewer-command.js b/app/commands/reviewer-command.js index 952a119..ae276e9 100644 --- a/app/commands/reviewer-command.js +++ b/app/commands/reviewer-command.js @@ -5,6 +5,7 @@ import { Command } from "./command.js"; import { extractUsersAndTeams } from "../converters.js"; import { getLogger } from "../logger.js"; import { + extractAuthorAssociation, extractBody, extractHtmlUrl, extractLabelableId, @@ -22,7 +23,7 @@ export class ReviewerCommand extends Command { } enabled(config) { - return reviewerEnabled(config); + return reviewerEnabled(config, extractAuthorAssociation(this.payload)); } async run(authToken) { diff --git a/app/commands/transfer-command.js b/app/commands/transfer-command.js index 18cd95f..ebc65ad 100644 --- a/app/commands/transfer-command.js +++ b/app/commands/transfer-command.js @@ -4,6 +4,7 @@ import { transferIssue } from "../github.js"; import { Command } from "./command.js"; import { getLogger } from "../logger.js"; import { + extractAuthorAssociation, extractBody, extractHtmlUrl, extractLabelableId, @@ -21,7 +22,7 @@ export class TransferCommand extends Command { } enabled(config) { - return transferEnabled(config); + return transferEnabled(config, extractAuthorAssociation(this.payload)); } async run(authToken) { diff --git a/app/comment-extractor.js b/app/comment-extractor.js index 89da86b..1edd556 100644 --- a/app/comment-extractor.js +++ b/app/comment-extractor.js @@ -25,3 +25,15 @@ export function extractLabelableId(payload) { return payload.issue.node_id; } + +export function extractAuthorAssociation(payload) { + if (payload.review) { + return payload.review.author_association; + } + + if (payload.comment) { + return payload.comment.author_association; + } + + return payload.pull_request?.author_association; +} diff --git a/app/comment-extractor.test.js b/app/comment-extractor.test.js index 6e8610c..932e6a5 100644 --- a/app/comment-extractor.test.js +++ b/app/comment-extractor.test.js @@ -1,4 +1,5 @@ import { + extractAuthorAssociation, extractBody, extractHtmlUrl, extractLabelableId, @@ -25,6 +26,16 @@ describe("extractors", () => { }), ).toEqual("comment body"); }); + + test("is a pull request body", () => { + expect( + extractBody({ + pull_request: { + body: "pull request body", + }, + }), + ).toEqual("pull request body"); + }); }); describe("extractHtmlUrl", () => { @@ -76,4 +87,36 @@ describe("extractors", () => { ).toEqual("PR_abcdefgh"); }); }); + + describe("extractAuthorAssociation", () => { + test("is a review", () => { + expect( + extractAuthorAssociation({ + review: { + author_association: "MEMBER", + }, + }), + ).toEqual("MEMBER"); + }); + + test("is an issue comment", () => { + expect( + extractAuthorAssociation({ + comment: { + author_association: "OWNER", + }, + }), + ).toEqual("OWNER"); + }); + + test("is a pull request", () => { + expect( + extractAuthorAssociation({ + pull_request: { + author_association: "CONTRIBUTOR", + }, + }), + ).toEqual("CONTRIBUTOR"); + }); + }); }); diff --git a/app/default-config.js b/app/default-config.js index 21d20b1..415f457 100644 --- a/app/default-config.js +++ b/app/default-config.js @@ -3,22 +3,28 @@ export const defaultConfig = { label: { allowedLabels: [], enabled: false, + permission: "all", }, removeLabel: { allowedLabels: [], enabled: false, + permission: "all", }, reopen: { enabled: false, + permission: "all", }, close: { enabled: false, + permission: "all", }, reviewer: { enabled: false, + permission: "all", }, transfer: { enabled: false, + permission: "all", }, }, };