Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Discord verification to user profiles #12

Merged
merged 3 commits into from May 19, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 22 additions & 3 deletions app/database.server.ts
Expand Up @@ -96,8 +96,8 @@ export async function createProfileForUser(user: User) {
}

/**
* Updates a User in the database. The following fields are stripped from the
* payload:
* Updates a User Profile in the database. The following fields are stripped
Copy link

Choose a reason for hiding this comment

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

Praise: Thanks for writing good docstrings!

* from the payload:
* - createdAt
* - updatedAt (filled in automatically)
* - userId
Expand All @@ -111,7 +111,6 @@ export async function updateProfileForUser(

// Delete the fields we don't want to alter
delete updatedProfile.createdAt;
delete updatedProfile.updatedAt;
delete updatedProfile.userId;
delete updatedProfile.githubUrl;

Expand All @@ -124,6 +123,26 @@ export async function updateProfileForUser(
.set(updatedProfile, { merge: true });
}

/**
* Updates a User in the database. The following fields are stripped from the
Copy link

Choose a reason for hiding this comment

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

Praise: Thanks for writing good docstrings!

* payload:
* - createdAt
* - updatedAt (filled in automatically)
* - uid
* - githubLogin
*/
export async function updateUser(uid: string, user: Partial<User>) {
// Delete the fields we don't want to alter
delete user.createdAt;
delete user.uid;
delete user.githubLogin;

// Send the correct time stamp
user.updatedAt = new Date().toISOString();

await db.collection(USERS_COLLECTION).doc(uid).set(user, { merge: true });
}

export async function getUserProfileBySlug(
slug: string
): Promise<UserProfile | null> {
Expand Down
62 changes: 62 additions & 0 deletions app/discord.server.ts
@@ -0,0 +1,62 @@
import crypto from "node:crypto";
import DiscordOauth2 from "discord-oauth2";

const DISCORD_REDIRECT_URI =
process.env.NODE_ENV === "production"
Copy link

Choose a reason for hiding this comment

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

Thought (non-blocking): If we do checks like this in many places, we might want to adopt something like the codesee_metaconfig.json file for storing non-sensitive constants that are env specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. This is the only place where we're doing this so far but I'll keep an eye on it.

? "https://opensourcehub.io/verify-discord"
: "http://localhost:3000/verify-discord";

/**
* List of Discord scopes we need for the OAuth handshake. In our case, all
* we want is the user's ID, so we use a single scope.
* @see https://discord.com/developers/docs/topics/oauth2#shared-resources-oauth2-scopes
*/
const DISCORD_SCOPES = ["identify"];
Copy link

Choose a reason for hiding this comment

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

Suggestion (non-blocking): Is there any documentation of Discord's available scopes? Maybe add a link to such documentation in a comment?


function assertDiscordEnvironmentVariables() {
if (!process.env.DISCORD_CLIENT_ID) {
throw new Error("Missing environment variable DISCORD_CLIENT_ID");
}
if (!process.env.DISCORD_CLIENT_SECRET) {
throw new Error("Missing environment variable DISCORD_CLIENT_SECRET");
}
}

export async function getDiscordAccessToken(code: string) {
const discordAuth = new DiscordOauth2();

const token = await discordAuth.tokenRequest({
clientId: process.env.DISCORD_CLIENT_ID,
clientSecret: process.env.DISCORD_CLIENT_SECRET,
Copy link

Choose a reason for hiding this comment

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

Question (maybe blocking): How are we keeping these values secret?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They're pulled from a .env file that isn't checked into source control. These variables will only be available on production and, because all this code happens on the server, they won't be exposed to any client. I do need to fail gracefully for local development, though.

code,
scope: DISCORD_SCOPES,
grantType: "authorization_code",
redirectUri: DISCORD_REDIRECT_URI,
});

return token.access_token;
}

export async function getDiscordUserId(accessToken: string) {
const discordAuth = new DiscordOauth2();
const discordUser = await discordAuth.getUser(accessToken);

return discordUser.id;
}

export function getDiscordOAuthUrl() {
assertDiscordEnvironmentVariables();

const oauth = new DiscordOauth2({
clientId: process.env.DISCORD_CLIENT_ID,
clientSecret: process.env.DISCORD_CLIENT_SECRET,
redirectUri: DISCORD_REDIRECT_URI,
});

const url = oauth.generateAuthUrl({
scope: ["identify"],
state: crypto.randomBytes(16).toString("hex"),
Copy link

Choose a reason for hiding this comment

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

Praise: Thanks for using a real source of entropy!

});

return url;
}
11 changes: 11 additions & 0 deletions app/routes/api/discord-auth-url.ts
@@ -0,0 +1,11 @@
import { ActionFunction, redirect } from "@remix-run/node";
import { getDiscordOAuthUrl } from "~/discord.server";

/**
* Generates an OAuth URL to authenticate with Discord, and then redirects to
* it. Once a user authenticates, we send them back to this website.
*/
export const action: ActionFunction = () => {
const url = getDiscordOAuthUrl();
return redirect(url);
};
51 changes: 44 additions & 7 deletions app/routes/u.github/$slug.tsx
@@ -1,26 +1,41 @@
import { json, LoaderFunction, redirect } from "@remix-run/node";
import { Link, Outlet, useCatch, useLoaderData } from "@remix-run/react";
import { Form, Link, Outlet, useCatch, useLoaderData } from "@remix-run/react";
import { FC } from "react";
import Button from "~/components/Button";
import RootLayout from "~/components/RootLayout";
import { getUserProfileBySlug } from "~/database.server";
import { UserProfile } from "~/types";
import { getCurrentUser, getSession } from "~/session.server";
import { User, UserProfile } from "~/types";

export const loader: LoaderFunction = async ({ params }) => {
export const loader: LoaderFunction = async ({ params, request }) => {
const slug = params.slug;

if (typeof slug !== "string") {
return redirect("/");
}

const userProfile = await getUserProfileBySlug(slug);
const session = await getSession(request.headers.get("Cookie"));
const user = await getCurrentUser(session);
const profile = await getUserProfileBySlug(slug);

if (!userProfile) {
if (!profile) {
throw new Response("Not Found", {
status: 404,
});
}

return json(userProfile);
if (profile.userId === user?.uid) {
// If the project is tied to the current user, send the user data down
return json({ profile, user } as LoaderData);
} else {
// Otherwise, send only the profile data
return json({ profile, user: null } as LoaderData);
}
};

type LoaderData = {
profile: UserProfile;
user: User | null;
};

export function CatchBoundary() {
Expand Down Expand Up @@ -73,7 +88,11 @@ export function CatchBoundary() {
}

const ProfilePage: FC = () => {
const profile = useLoaderData<UserProfile>();
const { profile, user } = useLoaderData<LoaderData>();

const canEdit = user != null;
const hasVerifiedDiscord =
user != null && typeof user.discordUserId === "string";

return (
<RootLayout>
Expand Down Expand Up @@ -110,6 +129,24 @@ const ProfilePage: FC = () => {
)}
</div>
</div>
{canEdit && (
<div>
{hasVerifiedDiscord ? (
<div>
<span
title="You've verified your Discord account"
className="inline-block px-2 py-1 rounded bg-discord-blurple text-white text-xs"
>
Discord verified
</span>
</div>
) : (
<Form action="/api/discord-auth-url" method="post">
<Button type="submit">Verify on Discord</Button>
</Form>
)}
</div>
)}
</div>
</div>
</main>
Expand Down
39 changes: 39 additions & 0 deletions app/routes/verify-discord.ts
@@ -0,0 +1,39 @@
import { LoaderFunction, redirect } from "@remix-run/node";
import { destroySession, getCurrentUser, getSession } from "~/session.server";
import { getDiscordAccessToken, getDiscordUserId } from "~/discord.server";
import { getProfileRouteForUser } from "~/utils/routes";
import { updateUser } from "~/database.server";

export const loader: LoaderFunction = async ({ request }) => {
const session = await getSession(request.headers.get("Cookie"));
Copy link

Choose a reason for hiding this comment

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

Thought/suggestion (non-blocking): The code to get the session and currentUser, and redirect to /login if the user is not logged in feels like it will repeat a lot. Does @remix-run have any concept of middleware? That feels like the place for this sort of thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

const currentUser = await getCurrentUser(session);

if (!currentUser) {
// Redirect to the login page
return redirect("/login", {
headers: {
// It's possible that there is not current user but that the session still
// exists. So to avoid infinite redirects, we destroy the session before
// redirecting.
"Set-Cookie": await destroySession(session),
},
});
}

// Get the `code` param from the URL -- this was added by Discord after
// successful authentication
const url = new URL(request.url);
const code = url.searchParams.get("code");

if (!code) {
throw new Error("No code");
}

const accessToken = await getDiscordAccessToken(code);

const discordUserId = await getDiscordUserId(accessToken);

await updateUser(currentUser.uid, { discordUserId });

return redirect(getProfileRouteForUser(currentUser));
};
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -35,6 +35,7 @@
"classnames": "2.3.1",
"concurrently": "^7.1.0",
"dayjs": "^1.11.2",
"discord-oauth2": "^2.10.0",
"dotenv": "10.0.0",
"firebase": "^9.8.1",
"firebase-admin": "^10.2.0",
Expand Down
3 changes: 3 additions & 0 deletions tailwind.config.js
Expand Up @@ -25,6 +25,9 @@ module.exports = {
menu: "0px 4px 20px rgba(2, 20, 67, 0.12);",
},
colors: {
discord: {
blurple: "#5865F2",
},
light: {
type: "rgba(0, 0, 0, .87)",
"type-medium": "rgba(0, 0, 0, .61)",
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Expand Up @@ -4246,6 +4246,11 @@ dir-glob@^3.0.1:
dependencies:
path-type "^4.0.0"

discord-oauth2@^2.10.0:
version "2.10.0"
resolved "https://registry.yarnpkg.com/discord-oauth2/-/discord-oauth2-2.10.0.tgz#7d31baced2b96c739b194c0ae42254cace3dcecd"
integrity sha512-AV+pjKURHyGTCLCUO1vep83/M8zzlAaTkpjAB6bisBCjsyiQzJGihKR4dlM88UeAaJbyqDxnHIsHafePIO2ssg==

dlv@^1.1.3:
version "1.1.3"
resolved "https://registry.yarnpkg.com/dlv/-/dlv-1.1.3.tgz#5c198a8a11453596e751494d49874bc7732f2e79"
Expand Down