Skip to content

Commit

Permalink
feat(github): improvments according to review
Browse files Browse the repository at this point in the history
    - move the generation & verification of state in the UI
    - generating github URL in the UI
    - dropped code in the iam related to above 2 points
  • Loading branch information
nutrina committed Jun 14, 2022
1 parent a7b4171 commit 5c0826f
Show file tree
Hide file tree
Showing 8 changed files with 82 additions and 105 deletions.
1 change: 1 addition & 0 deletions app/.env-example.env
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ NEXT_PUBLIC_DPOPP_GOOGLE_CLIENT_ID=MY-APP-ID.apps.googleusercontent.com
NEXT_PUBLIC_DPOPP_TWITTER_CLIENT_ID=ABC123456789
NEXT_PUBLIC_DPOPP_TWITTER_CALLBACK=http://localhost:3000/
NEXT_PUBLIC_DPOPP_FACEBOOK_APP_ID=123456789
NEXT_PUBLIC_DPOPP_GITHUB_CLIENT_ID=12345678
NEXT_PUBLIC_DPOPP_GITHUB_CALLBACK=http://localhost:3000/

NEXT_PUBLIC_DPOPP_IAM_URL=http://localhost:80/api/
Expand Down
2 changes: 1 addition & 1 deletion app/components/CardList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ export const CardList = (): JSX.Element => {
<FacebookCard />
<GoogleCard />
<TwitterCard />
<GithubCard />
<BrightidCard />
<PoapCard />
<EnsCard />
<PohCard />
<GithubCard />
</div>
</div>
);
Expand Down
43 changes: 24 additions & 19 deletions app/components/ProviderCards/GithubCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,33 +18,34 @@ import { datadogLogs } from "@datadog/browser-logs";
// Each provider is recognised by its ID
const providerId: PROVIDER_ID = "Github";

function generateUID(length: number) {
return window
.btoa(
Array.from(window.crypto.getRandomValues(new Uint8Array(length * 2)))
.map((b) => String.fromCharCode(b))
.join("")
)
.replace(/[+/]/g, "")
.substring(0, length);
}

export default function GithubCard(): JSX.Element {
const { address, signer, handleAddStamp, allProvidersState } = useContext(UserContext);
const [isLoading, setLoading] = useState(false);
const [state, setState] = useState("");

// Fetch Github OAuth2 url from the IAM procedure
async function handleFetchGithubOAuth(): Promise<void> {
// Fetch data from external API
const res = await fetch(
`${process.env.NEXT_PUBLIC_DPOPP_PROCEDURE_URL?.replace(/\/*?$/, "")}/github/generateAuthUrl`,
{
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({
callback: process.env.NEXT_PUBLIC_DPOPP_GITHUB_CALLBACK,
}),
}
);
const data = await res.json();
// open new window for authUrl
const githubUrl = data.authUrl;
// Generate a new state string and store it in the compoenents state so that we can
// verify it later
const state = "github-" + generateUID(10);
setState(state);

const githubUrl = `https://github.com/login/oauth/authorize?client_id=${process.env.NEXT_PUBLIC_DPOPP_GITHUB_CLIENT_ID}&redirect_uri=${process.env.NEXT_PUBLIC_DPOPP_GITHUB_CALLBACK}&state=${state}`;
openGithubOAuthUrl(githubUrl);
}

// Open Twitter authUrl in centered window
// Open Github authUrl in centered window
function openGithubOAuthUrl(url: string): void {
const width = 600;
const height = 800;
Expand Down Expand Up @@ -72,7 +73,12 @@ export default function GithubCard(): JSX.Element {
if (e.target === "github") {
// pull data from message
const queryCode = e.data.code;
const queryState = e.data.state;

if (state !== e.data.state) {
datadogLogs.logger.error("State mismatch, failed to create Github credential", { provider: "Github" });
setLoading(false);
return;
}

datadogLogs.logger.info("Saving Stamp", { provider: "Github" });
// fetch and store credential
Expand All @@ -85,7 +91,6 @@ export default function GithubCard(): JSX.Element {
address: address || "",
proofs: {
code: queryCode, // provided by github as query params in the redirect
sessionKey: queryState,
},
},
signer as { signMessage: (message: string) => Promise<string> }
Expand Down
2 changes: 1 addition & 1 deletion app/pages/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const App: NextPage = () => {

return <div></div>;
}
// if Twitter oauth then submit message to other windows and close self
// if Github oauth then submit message to other windows and close self
else if ((queryError || queryCode) && queryState && /^github-.*/.test(queryState)) {
// shared message channel between windows (on the same domain)
const channel = new BroadcastChannel("github_oauth_channel");
Expand Down
12 changes: 7 additions & 5 deletions iam/__tests__/github.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import { GithubProvider } from "../src/providers/github";

import { RequestPayload } from "@dpopp/types";
import * as githubOauth from "../src/procedures/githubOauth";

// ----- Libs
import axios from "axios";
Expand Down Expand Up @@ -91,7 +90,7 @@ describe("Attempt verification", function () {
expect(githubPayload).toMatchObject({ valid: false });
});

it("should return invalid payload when there is no id in requestFindMyUser response", async () => {
it("should return invalid payload when there is no id in verifyGithub response", async () => {
mockedAxios.get.mockImplementation(async (url, config) => {
return {
data: {
Expand All @@ -114,8 +113,12 @@ describe("Attempt verification", function () {
expect(githubPayload).toMatchObject({ valid: false });
});

it("should return invalid payload when requestFindMyUser throws", async () => {
const requestFindMyUser = jest.spyOn(githubOauth, "requestFindMyUser").mockRejectedValueOnce("unauthorized");
it("should return invalid payload when a bad status code is returned by github user api", async () => {
mockedAxios.get.mockImplementation(async (url, config) => {
return {
status: 500, // This will cause the method
};
});

const github = new GithubProvider();

Expand All @@ -126,6 +129,5 @@ describe("Attempt verification", function () {
} as unknown as RequestPayload);

expect(githubPayload).toMatchObject({ valid: false });
requestFindMyUser.mockRestore();
});
});
54 changes: 0 additions & 54 deletions iam/src/procedures/githubOauth.ts

This file was deleted.

18 changes: 0 additions & 18 deletions iam/src/procedures/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import { Request, Response, Router } from "express";

import * as twitterOAuth from "./twitterOauth";
import * as githubOAuth from "./githubOauth";
import { triggerBrightidSponsorship, verifyBrightidContextId } from "./brightid";

export const router = Router();
Expand Down Expand Up @@ -36,23 +35,6 @@ router.post("/twitter/generateAuthUrl", (req: Request, res: Response): void => {
}
});

router.post("/github/generateAuthUrl", (req: Request, res: Response): void => {
const { callback } = req.body as GenerateGithubAuthUrlRequestBody;
if (callback) {
const state = githubOAuth.getSessionKey();
const clientId = process.env.GITHUB_CLIENT_ID;

const data = {
state,
authUrl: `https://github.com/login/oauth/authorize?client_id=${clientId}&redirect_uri=${callback}&state=${state}`,
};

res.status(200).send(data);
} else {
res.status(400);
}
});

router.post("/brightid/sponsor", (req: Request, res: Response): void => {
const { contextIdData } = req.body as GenerateBrightidBody;
if (contextIdData) {
Expand Down
55 changes: 48 additions & 7 deletions iam/src/providers/github.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
// ----- Types
import type { RequestPayload, VerifiedPayload } from "@dpopp/types";

// ----- Github OAuth2
import { GithubFindMyUserResponse, requestFindMyUser } from "../procedures/githubOauth";
import type { Provider, ProviderOptions } from "../types";
import axios from "axios";

export type GithubTokenResponse = {
access_token: string;
};

export type GithubFindMyUserResponse = {
id?: string;
login?: string;
type?: string;
};

// Export a Github Provider to carry out OAuth and return a record object
export class GithubProvider implements Provider {
Expand Down Expand Up @@ -40,7 +48,40 @@ export class GithubProvider implements Provider {
}
}

// Perform verification on twitter access token
async function verifyGithub(code: string): Promise<GithubFindMyUserResponse> {
return await requestFindMyUser(code);
}
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 verifyGithub = async (code: string): Promise<GithubFindMyUserResponse> => {
// retrieve user's auth bearer token to authenticate client
const accessToken = await requestAccessToken(code);

// 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;
};

0 comments on commit 5c0826f

Please sign in to comment.