Skip to content

Commit d1a0b21

Browse files
committed
Improve panel auth handling further
Include state to prevent CSRF. Handle edge-cases where you've got multiple tabs open, and you log in using one of them. And make sure you get redirected back to the page you came from.
1 parent e0a1a62 commit d1a0b21

6 files changed

Lines changed: 182 additions & 34 deletions

File tree

panel/src/app/(authenticated)/template.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { readAuthData, logout, ModfestAuth } from "@/auth_context"
44
import { Platform, PlatformContext } from "@/platform";
5-
import { useRouter } from "next/navigation"
5+
import { usePathname, useRouter, useSearchParams } from "next/navigation"
66
import { createContext, use, useContext, useEffect, useReducer, useState } from "react";
77

88
export default function Template({ children }: { children: React.ReactNode }) {
@@ -16,7 +16,8 @@ export default function Template({ children }: { children: React.ReactNode }) {
1616
if (a && a.isValid()) {
1717
setAuth(a)
1818
} else {
19-
router.push("/auth/login")
19+
var currentUrl = window.location.pathname + window.location.search + window.location.hash
20+
router.push(`/auth/login?r=${encodeURIComponent(currentUrl)}`)
2021
}
2122
}
2223
}, [auth])

panel/src/app/auth/auth.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,22 @@
11
"use client"
22

3-
export function getRedirectUrl(): string {
4-
// TODO properly handle prerendering here
5-
return typeof window !== 'undefined' ? `${window.location.origin}/auth/callback` : ""
3+
export function getCallbackUrl(): string {
4+
return `${window.location.origin}/auth/callback`
65
}
6+
7+
/**
8+
* Random value which is generated per-browser and then stored in localStorage.
9+
* Only ever generated once (unless the user clears their cache).
10+
*
11+
* See the comments in the login page for more information
12+
*
13+
* The resulting value is guaranteed to be alphanumeric
14+
*/
15+
export function getOauthBrowserKey(): string {
16+
var key = localStorage.getItem("oauthbrowserkey");
17+
if (!key) {
18+
key = crypto.randomUUID().replace("-", "");
19+
localStorage.setItem("oauthbrowserkey", key);
20+
}
21+
return key;
22+
}

panel/src/app/auth/callback/page.tsx

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,21 @@
11
"use client"
22

33
import { useEffect, useState } from "react";
4-
import { getRedirectUrl } from "../auth";
4+
import { getCallbackUrl, getOauthBrowserKey } from "../auth";
55
import { getToken } from "./server_handler";
66
import { ModfestAuth } from "@/auth_context";
7+
import { useRouter } from "next/navigation";
8+
import { AppRouterInstance } from "next/dist/shared/lib/app-router-context.shared-runtime";
9+
10+
// This page completes the OAUTH flow. This is the page where modrinth redirects the user to once they've
11+
// logged in
712

813
export default function Home() {
14+
const router = useRouter()
915
const [failed, setFailed] = useState(false)
1016

1117
useEffect(() => {
12-
authWithModrinthOAuth(setFailed)
18+
authWithModrinthOAuth(router, setFailed)
1319
}, [])
1420

1521
if (failed) {
@@ -30,19 +36,42 @@ export default function Home() {
3036
);
3137
}
3238

33-
async function authWithModrinthOAuth(setFailed: (v: boolean) => void) {
39+
async function authWithModrinthOAuth(router: AppRouterInstance, setFailed: (v: boolean) => void) {
3440
const urlParams = new URLSearchParams(window.location.search);
3541
const code = urlParams.get("code")
42+
const state = urlParams.get("state")
3643

37-
var modrinthToken = await getToken(code!, getRedirectUrl())
38-
if (!("access_token" in modrinthToken) || !("expires_in" in modrinthToken)) {
44+
if (!code || !state) {
3945
setFailed(true)
4046
return
4147
}
4248

43-
const token = new ModfestAuth(modrinthToken["access_token"], modrinthToken["expires_in"])
49+
const [securityToken, redirect] = state.split(".", 2)
50+
if (securityToken !== getOauthBrowserKey()) {
51+
setFailed(true)
52+
return
53+
}
54+
55+
var modrinthToken = await getToken(code!, getCallbackUrl())
56+
if (!("access_token" in modrinthToken) || !("expires_at" in modrinthToken)) {
57+
setFailed(true)
58+
return
59+
}
60+
61+
const token = new ModfestAuth(modrinthToken["access_token"], modrinthToken["expires_at"])
4462
token.saveLocalStorage()
4563

46-
// TODO redirect anywhere on site
47-
window.location.replace(window.location.origin)
64+
redirectBack(router, redirect)
65+
}
66+
67+
export function redirectBack(router: AppRouterInstance, url: string) {
68+
// Quite important, this url is untrusted and we don't want to be redirecting to arbitrary pages
69+
if (!url.startsWith("/")) {
70+
url = "/"
71+
}
72+
// We should never redirect back to the authentication flow, that'll give infinite loops
73+
if (url.startsWith("/auth")) {
74+
url = "/"
75+
}
76+
router.replace(url)
4877
}

panel/src/app/auth/callback/server_handler.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"use server"
22

3-
export async function getToken(code: string, redirect_uri: string): Promise<ModrinthToken> {
3+
export async function getToken(code: string, redirect_uri: string): Promise<ModrinthTokenData> {
44
// This function is run on the server, it needs access to the client secret
55
const modrinthApi = process.env.NEXT_PUBLIC_MODRINTH_API
66

@@ -17,11 +17,23 @@ export async function getToken(code: string, redirect_uri: string): Promise<Modr
1717
grant_type: "authorization_code"
1818
})
1919
})
20-
return <ModrinthToken>(await result.json())
20+
const data = <ModrinthToken>(await result.json())
21+
return {
22+
access_token: data.access_token,
23+
token_type: data.token_type,
24+
// Subtract 5 minutes to account for latency between us and modrinth
25+
expires_at: Date.now() + data.expires_in - 60*5
26+
}
2127
}
2228

2329
export type ModrinthToken = {
2430
access_token: string,
2531
token_type: string,
2632
expires_in: number
2733
}
34+
35+
export type ModrinthTokenData = {
36+
access_token: string,
37+
token_type: string,
38+
expires_at: number
39+
}

panel/src/app/auth/login/inner.tsx

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
"use client"
2+
import { readAuthData } from "@/auth_context";
3+
import { useRouter } from "next/navigation";
4+
import { useEffect, useState } from "react";
5+
import { getOauthBrowserKey, getCallbackUrl } from "../auth";
6+
import { redirectBack } from "../callback/page";
7+
8+
9+
export default function Inner(props: {redirect_url: string}) {
10+
const router = useRouter()
11+
const redirect = props.redirect_url
12+
// We redirect the user to OAUTH themselves with modrinth
13+
// This is documented on https://docs.modrinth.com/guide/oauth/
14+
// The flow is as follows:
15+
// * The user tries to access an authenticated page, they do not have a token in localStorage
16+
// * The user is redirected to this page, the previous page they were on is stored in a query parameter named `r`
17+
// * The user clicks the "log in with Modrinth" button, and gets sent to a modrinth page
18+
// * The user completes the login in modrinth, and is redirected to our special "callback" page
19+
// * The callback page reads the code which modrinth passed through in a query parameter
20+
// * The callback page sends this code to the server, where it's combined with an application secret key to
21+
// obtain a modrinth token
22+
// * The callback page reads the `r` query parameter and redirects back to the original page
23+
24+
// We want to check if the user got sent here erroneously,
25+
// aka if they got authenticated in the meantime
26+
const checkLoggedIn = () => {
27+
const a = readAuthData()
28+
if (a && a.isValid()) {
29+
redirectBack(router, redirect)
30+
}
31+
};
32+
useEffect(() => checkLoggedIn(), []); // Wrapped in useEffect so it only runs on client
33+
34+
// We also want to keep an eye on localStorage. It might be that the user has multiple tabs open,
35+
// and logged in on aNextRouter different tab.
36+
useEffect(() => {
37+
const controller = new AbortController();
38+
addEventListener("storage", () => {
39+
checkLoggedIn()
40+
}, { signal: controller.signal })
41+
return () => controller.abort()
42+
}, [])
43+
44+
// These need to be in a useEffect because they must run on the client and not the server
45+
const [oauthBrowserKey, setOauthBrowserKey] = useState<string | undefined>(undefined)
46+
useEffect(() => {
47+
if (!oauthBrowserKey) {
48+
setOauthBrowserKey(getOauthBrowserKey())
49+
}
50+
}, [oauthBrowserKey]);
51+
52+
const [callbackUrl, setCallbackUrl] = useState<string | undefined>(undefined)
53+
useEffect(() => {
54+
if (!callbackUrl) {
55+
setCallbackUrl(getCallbackUrl())
56+
}
57+
}, [callbackUrl]);
58+
59+
if (!callbackUrl) {
60+
return <main>
61+
<center>
62+
<h1>ModFest panel</h1>
63+
Log in with Modrinth
64+
</center>
65+
</main>
66+
}
67+
68+
// We can provide a "state" parameter to modrinth. This will be passed on unmodified to the callback
69+
// We use it for two reasons. Firstly, we insert a random browser-specific value into it.
70+
// If we didn't do this, anyone could create a link to our callback page. When someone clicks
71+
// that link the callback will blindly assume the token is correct and it'll just be a
72+
// confusing mess. So we store some random value in localStorage, and any attacker won't be
73+
// able to guess that.
74+
// Secondly, we use the state variable to encode the return path.
75+
const state = oauthBrowserKey+"."+redirect
76+
77+
const queryParams = {
78+
// This is the only auth type modrinth supports
79+
"response_type": "code",
80+
// Identifier for the modrinth app
81+
"client_id": process.env.NEXT_PUBLIC_MODRINTH_APP_ID!,
82+
// We only want the bare minimum scope, this should also be set in the modrinth application's settings (https://modrinth.com/settings/applications)
83+
"scope": "USER_READ",
84+
// The state variable (which will be passed on unmodified)
85+
"state": state,
86+
// The url for our callback page
87+
"redirect_uri": callbackUrl,
88+
};
89+
90+
const oathUrl = `${process.env.NEXT_PUBLIC_MODRINTH_SITE}/auth/authorize?` + Object.entries(queryParams).map(([k,v]) => k+"="+encodeURIComponent(v)).join("&")
91+
92+
return (
93+
<main>
94+
<center>
95+
<h1>ModFest panel</h1>
96+
<a href={oathUrl}>Log in with Modrinth</a>
97+
</center>
98+
</main>
99+
);
100+
}

panel/src/app/auth/login/page.tsx

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,16 @@
1-
"use client"
2-
3-
import { getRedirectUrl } from "../auth";
1+
import Inner from "./inner";
42

53
type SearchParams = Promise<{ [key: string]: string | string[] | undefined }>
64
type SearchParamProps = {
75
searchParams: SearchParams;
86
};
97

10-
export default function Home(props: SearchParamProps) {
11-
const modrinthSite = process.env.NEXT_PUBLIC_MODRINTH_SITE
12-
const clientId = process.env.NEXT_PUBLIC_MODRINTH_APP_ID!
13-
const callback = getRedirectUrl()
14-
const scope = `USER_READ`
15-
16-
const oathUrl = `${modrinthSite}/auth/authorize?client_id=${encodeURIComponent(clientId)}&redirect_uri=${encodeURIComponent(callback)}&scope=${encodeURIComponent(scope)}`
17-
18-
return (
19-
<main>
20-
<center>
21-
<h1>ModFest panel</h1>
22-
<a href={oathUrl}>Log in with Modrinth</a>
23-
</center>
24-
</main>
25-
);
8+
export default async function Home(props: SearchParamProps) {
9+
// We obtain the page to redirect to after this whole ordeal.
10+
// If it's not specified we set it to ""
11+
var redirect = (await props.searchParams)["r"] ?? ""
12+
if (typeof(redirect) !== "string") {
13+
redirect = redirect[0]
14+
}
15+
return <Inner redirect_url={redirect}></Inner>
2616
}

0 commit comments

Comments
 (0)