Skip to content
This repository has been archived by the owner on Sep 19, 2024. It is now read-only.

hotfix: check permission when closing issue #622

Merged
merged 7 commits into from
Aug 17, 2023
Merged
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
5 changes: 5 additions & 0 deletions src/handlers/payout/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { getPenalty, getWalletAddress, getWalletMultiplier, removePenalty } from
import { getBotConfig, getBotContext, getLogger } from "../../bindings";
import {
addLabelToIssue,
checkUserPermissionForRepoAndOrg,
clearAllPriceLabelsOnIssue,
deleteLabel,
generatePermit2Signature,
Expand Down Expand Up @@ -33,6 +34,10 @@ export const handleIssueClosed = async () => {

if (!issue) return;

const userHasPermission = await checkUserPermissionForRepoAndOrg(payload.sender.login, context);
Copy link
Member

@0x4007 0x4007 Aug 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found an issue testing it out.

This is not implemented correctly because I'm an admin of the ubiquibot organization @wannacfuture please review


if (!userHasPermission) return "Permit generation skipped because this issue has been closed by an external contributor.";

const comments = await getAllIssueComments(issue.number);

const wasReopened = await wasIssueReopened(issue.number);
Expand Down
43 changes: 43 additions & 0 deletions src/helpers/issue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,49 @@ export const removeAssignees = async (issue_number: number, assignees: string[])
}
};

export const checkUserPermissionForRepoAndOrg = async (username: string, context: Context): Promise<boolean> => {
const permissionForRepo = await checkUserPermissionForRepo(username, context);
const permissionForOrg = await checkUserPermissionForOrg(username, context);

return permissionForOrg || permissionForRepo;
};

export const checkUserPermissionForRepo = async (username: string, context: Context): Promise<boolean> => {
const logger = getLogger();
const payload = context.payload as Payload;

try {
const res = await context.octokit.rest.repos.checkCollaborator({
owner: payload.repository.owner.login,
repo: payload.repository.name,
username,
});

return res.status === 204;
} catch (e: unknown) {
logger.error(`Checking if user permisson for repo failed!, reason: ${e}`);
return false;
wannacfuture marked this conversation as resolved.
Show resolved Hide resolved
}
};

export const checkUserPermissionForOrg = async (username: string, context: Context): Promise<boolean> => {
const logger = getLogger();
const payload = context.payload as Payload;
if (!payload.organization) return false;

try {
const res = await context.octokit.rest.orgs.checkMembershipForUser({
org: payload.organization.login,
username,
});
// @ts-expect-error This looks like a bug in octokit. (https://github.com/octokit/rest.js/issues/188)
return res.status === 204;
} catch (e: unknown) {
logger.error(`Checking if user permisson for org failed!, reason: ${e}`);
return false;
}
};

export const getUserPermission = async (username: string, context: Context): Promise<string> => {
const logger = getLogger();
const payload = context.payload as Payload;
Expand Down
Loading