Skip to content

Commit

Permalink
fix(iam): reduce number of calls to the Github API (passportxyz#1375)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lucianHymer
lucianHymer authored Jun 16, 2023
1 parent 672e1ef commit 2caa6c3
Show file tree
Hide file tree
Showing 10 changed files with 245 additions and 413 deletions.
17 changes: 7 additions & 10 deletions platforms/src/Gitcoin/Providers/gitcoinGrantsStatistics.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { Provider, ProviderOptions } from "../../types";
import { getErrorString, ProviderError } from "../../utils/errors";
import { getAddress } from "../../utils/signer";
import axios from "axios";
import { GithubFindMyUserResponse, verifyGithub } from "../../Github/Providers/github";
import { getGithubUserData } from "../../Github/Providers/github";

const AMI_API_TOKEN = process.env.AMI_API_TOKEN;

Expand Down Expand Up @@ -44,14 +44,8 @@ export class GitcoinGrantStatisticsProvider implements Provider {
async verify(payload: RequestPayload, context: ProviderContext): Promise<VerifiedPayload> {
const address = (await getAddress(payload)).toLowerCase();
let valid = false;
let githubUser: GithubFindMyUserResponse = context.githubUser as GithubFindMyUserResponse;
const githubUser = await getGithubUserData(payload.proofs.code, context);
try {
if (!githubUser) {
githubUser = await verifyGithub(payload.proofs.code, context);
context["githubUser"] = githubUser;
}
console.log("gitcoin - githubUser", address, JSON.stringify(githubUser));

// Only check the contribution condition if a valid github id has been received
valid = !githubUser.errors && !!githubUser.id;
if (valid) {
Expand All @@ -69,7 +63,10 @@ export class GitcoinGrantStatisticsProvider implements Provider {
error: gitcoinGrantsStatistic.errors,
record: valid
? {
id: githubUser.id,
// The type was previously incorrectly defined as string on the http response,
// and if we correctly called .toString() here instead of doing the forced cast,
// we would break our ability to hash against all previous records.
id: githubUser.id as unknown as string,
[this._options.recordAttribute]: `${this._options.threshold}`,
}
: undefined,
Expand All @@ -96,7 +93,7 @@ export class GitcoinGrantStatisticsProvider implements Provider {

const getGitcoinStatistics = async (dataUrl: string, handle: string): Promise<GitcoinGrantStatistics> => {
try {
// The gitcoin API excpects lowercase handle
// The gitcoin API expects lowercase handle
const lowerHandle = handle.toLowerCase();
const grantStatisticsRequest = await axios.get(`${dataUrl}?handle=${lowerHandle}`, {
headers: { Authorization: `token ${AMI_API_TOKEN}` },
Expand Down
2 changes: 1 addition & 1 deletion platforms/src/Github/Providers/__tests__/githubForkedRepoProvider.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ describe("Attempt verification", function () {
}
);

expect(mockedAxios.get).toBeCalledTimes(2);
expect(mockedAxios.get).toBeCalledTimes(1);

// Check the request to get the user
expect(mockedAxios.get).toBeCalledWith("https://api.github.com/user", {
Expand Down
10 changes: 5 additions & 5 deletions platforms/src/Github/Providers/__tests__/githubStarredRepoProvider.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,11 @@ const code = "ABC123_ACCESSCODE";
const clientId = process.env.GITHUB_CLIENT_ID;
const clientSecret = process.env.GITHUB_CLIENT_SECRET;

beforeEach(() => {
jest.clearAllMocks();
});

describe("Attempt verification", function () {
beforeEach(() => {
jest.clearAllMocks();
});

it("handles valid verification attempt", async () => {
mockedAxios.post.mockImplementation(async () => {
return validCodeResponse;
Expand Down Expand Up @@ -309,7 +309,7 @@ describe("Attempt verification", function () {
}
);

expect(mockedAxios.get).toBeCalledTimes(2);
expect(mockedAxios.get).toBeCalledTimes(1);

// Check the request to get the user
expect(mockedAxios.get).toBeCalledWith("https://api.github.com/user", {
Expand Down
173 changes: 126 additions & 47 deletions platforms/src/Github/Providers/github.ts
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,44 @@
// ----- Types
import type { ProviderContext, RequestPayload, VerifiedPayload } from "@gitcoin/passport-types";
import type { Provider, ProviderOptions } from "../../types";
import { getErrorString, ProviderError } from "../../utils/errors";
import { getAddress } from "../../utils/signer";
import { ProviderError } from "../../utils/errors";
import axios from "axios";

export type GithubTokenResponse = {
access_token: string;
};

export type GithubFindMyUserResponse = {
errors?: string[] | undefined;
id?: string;
export type GithubUserData = {
public_repos?: number;
id?: number;
login?: string;
followers?: number;
type?: string;
errors?: string[];
};

export type GithubContext = ProviderContext & {
github?: {
userData?: GithubUserData;
accessToken?: string;
repos?: GithubUserRepoData[];
};
};

export type GithubUserRepoData = {
owner?: {
id?: number;
type?: string;
};
fork?: boolean;
forks_count?: number;
stargazers_url?: string;
stargazers_count?: number;
};

export type GithubRepoRequestResponse = {
data?: GithubUserRepoData[];
status?: number;
};

// Export a Github Provider to carry out OAuth and return a record object
Expand All @@ -31,8 +56,7 @@ export class GithubProvider implements Provider {

// verify that the proof object contains valid === "true"
async verify(payload: RequestPayload, context: ProviderContext): Promise<VerifiedPayload> {
const address = (await getAddress(payload)).toLowerCase();
const verifiedPayload = await verifyGithub(payload.proofs.code, context);
const verifiedPayload = await getGithubUserData(payload.proofs.code, context);

const valid = !!(!verifiedPayload.errors && verifiedPayload.id);

Expand All @@ -41,59 +65,114 @@ export class GithubProvider implements Provider {
error: verifiedPayload.errors,
record: valid
? {
id: verifiedPayload.id,
// The type was previously incorrectly defined as string on the http response,
// and if we correctly called .toString() here instead of doing the forced cast,
// we would break our ability to hash against all previous records.
id: verifiedPayload.id as unknown as string,
}
: undefined,
};
}
}

export const requestAccessToken = async (code: string, context: ProviderContext): Promise<string> => {
const clientId = process.env.GITHUB_CLIENT_ID;
const clientSecret = process.env.GITHUB_CLIENT_SECRET;
const accessToken = context.githubAccessToken as string;
export const requestAccessToken = async (code: string, context: GithubContext): Promise<string> => {
if (!context.github?.accessToken) {
const clientId = process.env.GITHUB_CLIENT_ID;
const clientSecret = process.env.GITHUB_CLIENT_SECRET;

if (accessToken) {
return accessToken;
}
// Exchange the code for an access token
const tokenRequest = await axios.post(
`https://github.com/login/oauth/access_token?client_id=${clientId}&client_secret=${clientSecret}&code=${code}`,
{},
{
headers: { Accept: "application/json" },
}
);

// Exchange the code for an access token
const tokenRequest = await axios.post(
`https://github.com/login/oauth/access_token?client_id=${clientId}&client_secret=${clientSecret}&code=${code}`,
{},
{
headers: { Accept: "application/json" },
}
);
const tokenResponse = tokenRequest.data as GithubTokenResponse;

const tokenResponse = tokenRequest.data as GithubTokenResponse;
if (!context.github) context.github = {};
context.github.accessToken = tokenResponse.access_token;
}

context["githubAccessToken"] = tokenResponse.access_token;
return tokenResponse.access_token;
return context.github.accessToken;
};

export const verifyGithub = async (code: string, context: ProviderContext): Promise<GithubFindMyUserResponse> => {
try {
// retrieve user's auth bearer token to authenticate client
export const getGithubUserData = async (code: string, context: GithubContext): Promise<GithubUserData> => {
if (!context.github?.userData) {
try {
// retrieve user's auth bearer token to authenticate client
const accessToken = await requestAccessToken(code, context);

// Now that we have an access token fetch the user details
const userRequest = await axios.get("https://api.github.com/user", {
headers: { Authorization: `token ${accessToken}` },
});

if (!context.github) context.github = {};
context.github.userData = userRequest.data;
} catch (_error) {
const error = _error as ProviderError;
if (error?.response?.status === 429) {
return {
errors: ["Error getting getting github info", "Rate limit exceeded"],
};
}
return {
errors: ["Error getting getting github info", `${error?.message}`],
};
}
}
return context.github.userData;
};

export const getGithubUserRepos = async (code: string, context: GithubContext): Promise<GithubUserRepoData[]> => {
if (!context.github?.repos) {
const userData = await getGithubUserData(code, context);
const accessToken = await requestAccessToken(code, context);
let repoRequest: GithubRepoRequestResponse;

try {
await avoidGithubRateLimit();

// fetch user's repo data
repoRequest = await axios.get(`https://api.github.com/users/${userData.login}/repos?per_page=100`, {
headers: { Authorization: `token ${accessToken}` },
});
// Returns true for first instance of a user's repo that has been forked,
// false if no forks
if (repoRequest.status != 200) {
throw `Repo GET request returned status code ${repoRequest.status} instead of the expected 200`;
}

if (!context.github) context.github = {};
context.github.repos = repoRequest.data;
} catch (e) {
const error = e as {
response: {
data: {
error_description: string;
};
};
request: string;
message: string;
};
if (error.response) {
throw `User GET request returned status code ${repoRequest.status} instead of the expected 200`;
} else if (error.request) {
throw `A request was made, but no response was received: ${error.request}`;
} else {
throw `Error: ${error.message}`;
}
}
}
return context.github.repos;
};

// Now that we have an access token fetch the user details
const userRequest = await axios.get("https://api.github.com/user", {
headers: { Authorization: `token ${accessToken}` },
});
console.log("verifyGithub result:", JSON.stringify(userRequest.data));
// For everything after the initial user load, we need to avoid the secondary rate
// limit by waiting 1 second between requests
export const avoidGithubRateLimit = async (): Promise<void> => {
if (process.env.NODE_ENV === "test") return;

return userRequest.data as GithubFindMyUserResponse;
} catch (_error) {
const error = _error as ProviderError;
console.log("verifyGithub ERROR:", getErrorString(error));
return {
errors: [
"Error getting getting github info",
`${error?.message}`,
`Status ${error.response?.status}: ${error.response?.statusText}`,
`Details: ${JSON.stringify(error?.response?.data)}`,
],
};
}
await new Promise((resolve) => setTimeout(resolve, 1000));
};
76 changes: 13 additions & 63 deletions platforms/src/Github/Providers/githubFiveOrMoreRepos.ts
View file
Original file line number Diff line number Diff line change
@@ -1,21 +1,7 @@
// ----- Types
import type { ProviderContext, RequestPayload, VerifiedPayload } from "@gitcoin/passport-types";
import type { RequestPayload, VerifiedPayload } from "@gitcoin/passport-types";
import type { Provider, ProviderOptions } from "../../types";
import { requestAccessToken } from "./github";

// ----- HTTP Client
import axios from "axios";

export type GithubTokenResponse = {
access_token: string;
};

export type GithubFindMyUserResponse = {
id?: string;
login?: string;
public_repos?: number;
type?: string;
};
import { getGithubUserData, GithubContext, GithubUserData } from "./github";

// Export a Github Provider to carry out OAuth, check if the user has 5 >= repos,
// and return a record object
Expand All @@ -32,61 +18,25 @@ export class FiveOrMoreGithubRepos implements Provider {
}

// verify that the proof object contains valid === "true"
async verify(payload: RequestPayload, context: ProviderContext): Promise<VerifiedPayload> {
async verify(payload: RequestPayload, context: GithubContext): Promise<VerifiedPayload> {
let valid = false,
verifiedPayload: GithubFindMyUserResponse = {};
verifiedPayload: GithubUserData = {},
errors: string[] | undefined;

try {
verifiedPayload = await verifyGithubRepoCount(payload.proofs.code, context);
verifiedPayload = await getGithubUserData(payload.proofs.code, context);
if (verifiedPayload.id && !errors?.length) valid = verifiedPayload.public_repos >= 5;
} catch (e) {
return { valid: false };
} finally {
valid = verifiedPayload && verifiedPayload.public_repos >= 5 && verifiedPayload.id ? true : false;
valid = false;
}

return {
valid: valid,
record: {
id: verifiedPayload.id + "gte5GithubRepos",
},
record: valid
? {
id: verifiedPayload.id.toString() + "gte5GithubRepos",
}
: undefined,
};
}
}

// const requestAccessToken = async (code: string): Promise<string> => {
// const clientId = process.env.GITHUB_CLIENT_ID;
// const clientSecret = process.env.GITHUB_CLIENT_SECRET;

// // Exchange the code for an access token
// const tokenRequest = await axios.post(
// `https://github.com/login/oauth/access_token?client_id=${clientId}&client_secret=${clientSecret}&code=${code}`,
// {},
// {
// headers: { Accept: "application/json" },
// }
// );

// if (tokenRequest.status != 200) {
// throw `Post for request returned status code ${tokenRequest.status} instead of the expected 200`;
// }

// const tokenResponse = tokenRequest.data as GithubTokenResponse;

// return tokenResponse.access_token;
// };

const verifyGithubRepoCount = async (code: string, context: ProviderContext): Promise<GithubFindMyUserResponse> => {
// retrieve user's auth bearer token to authenticate client
const accessToken = await requestAccessToken(code, context);

// Now that we have an access token fetch the user details
const userRequest = await axios.get("https://api.github.com/user", {
headers: { Authorization: `token ${accessToken}` },
});

if (userRequest.status != 200) {
throw `Get user request returned status code ${userRequest.status} instead of the expected 200`;
}

return userRequest.data as GithubFindMyUserResponse;
};
Loading

0 comments on commit 2caa6c3

Please sign in to comment.