From 48fbbffc82a9de0de310101ab4a4cff535e09e7d Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 5 Jan 2026 09:12:29 +0000 Subject: [PATCH 1/3] fix: robust oauth callback handling and error reporting --- .../api/auth/callback/github.js | 142 ++++++++++++------ 1 file changed, 93 insertions(+), 49 deletions(-) diff --git a/public/edge-functions/api/auth/callback/github.js b/public/edge-functions/api/auth/callback/github.js index cde5fd6..2de7829 100644 --- a/public/edge-functions/api/auth/callback/github.js +++ b/public/edge-functions/api/auth/callback/github.js @@ -1,24 +1,25 @@ export async function onRequest(event) { - const url = new URL(event.request.url); - const code = url.searchParams.get("code"); - const error = url.searchParams.get("error"); + try { + const url = new URL(event.request.url); + const code = url.searchParams.get("code"); + const error = url.searchParams.get("error"); - if (error) { - return new Response(JSON.stringify({ error }), { status: 400 }); - } + if (error) { + return new Response(JSON.stringify({ error }), { status: 400, headers: { "Content-Type": "application/json" } }); + } - if (!code) { - return new Response("Missing code", { status: 400 }); - } + if (!code) { + return new Response(JSON.stringify({ error: "Missing code" }), { status: 400, headers: { "Content-Type": "application/json" } }); + } - const CLIENT_ID = env.CLIENT_ID; - const CLIENT_SECRET = env.CLIENT_SECRET; + // Access env inside try block to catch ReferenceErrors if env is missing + const CLIENT_ID = env.CLIENT_ID; + const CLIENT_SECRET = env.CLIENT_SECRET; - if (!CLIENT_ID || !CLIENT_SECRET) { - return new Response(JSON.stringify({ error: "Server Configuration Error" }), { status: 500 }); - } + if (!CLIENT_ID || !CLIENT_SECRET) { + return new Response(JSON.stringify({ error: "Server Configuration Error" }), { status: 500, headers: { "Content-Type": "application/json" } }); + } - try { // 1. Exchange code for access token const tokenResponse = await fetch('https://github.com/login/oauth/access_token', { method: 'POST', @@ -40,8 +41,8 @@ export async function onRequest(event) { // 2. Fetch User Info const userResponse = await fetch('https://api.github.com/user', { headers: { - 'Authorization': `Bearer ${accessToken}`, - 'User-Agent': 'RailRound-EdgeFunction' + 'Authorization': `Bearer ${accessToken}`, + 'User-Agent': 'RailRound-EdgeFunction' } }); @@ -56,41 +57,84 @@ export async function onRequest(event) { let username = await DB.get(bindingKey); if (!username) { - // Registration / Binding - // Check if a user with this login already exists manually to avoid overwrite - // Strategy: Try `github_login`. If taken, try `github_login_id`. - - let candidateUsername = `github_${githubUser.login}`; - let userKey = `user:${candidateUsername}`; - let existingUser = await DB.get(userKey); - - if (existingUser) { - // Collision or previous manual registration with same name? - // If the existing user has NO github binding, we can't just take it. - // We append ID to be safe. - candidateUsername = `github_${githubUser.login}_${githubUser.id}`; - userKey = `user:${candidateUsername}`; + // Registration / Binding + let candidateUsername = `github_${githubUser.login}`; + let userKey = `user:${candidateUsername}`; + let existingUserStr = await DB.get(userKey); + let recover = false; + + if (existingUserStr) { + // Collision or previous registration? + try { + const existingUser = JSON.parse(existingUserStr); + if (existingUser.bindings && existingUser.bindings.github && existingUser.bindings.github.id === githubUser.id) { + // It's the same user! The binding key was lost. Recover it. + recover = true; + } + } catch (e) { + // Ignore parse error, treat as collision } - username = candidateUsername; - - const userData = { - username, - created_at: new Date().toISOString(), - bindings: { - github: { - id: githubUser.id, - login: githubUser.login, - avatar_url: githubUser.avatar_url, - name: githubUser.name + if (!recover) { + // Real collision, switch to ID-based username + candidateUsername = `github_${githubUser.login}_${githubUser.id}`; + userKey = `user:${candidateUsername}`; + + // Double check if THIS exists (paranoia check / recovering ID-based user) + existingUserStr = await DB.get(userKey); + if (existingUserStr) { + try { + const existingUser = JSON.parse(existingUserStr); + if (existingUser.bindings && existingUser.bindings.github && existingUser.bindings.github.id === githubUser.id) { + recover = true; + } else { + throw new Error("Username collision for backup name"); } - }, - trips: [], - pins: [] - }; + } catch (e) { + if (e.message === "Username collision for backup name") throw e; + // If parse error here, maybe we should overwrite? + // But safer to assume collision. + throw new Error("Username collision for backup name (corrupt data)"); + } + } + } + } + + username = candidateUsername; + + // If we are recovering, we don't need to overwrite the user data, just the binding. + // But updating the user data (e.g. avatar) is good practice. + // So we always put. + + const userData = { + username, + created_at: new Date().toISOString(), + bindings: { + github: { + id: githubUser.id, + login: githubUser.login, + avatar_url: githubUser.avatar_url, + name: githubUser.name + } + }, + trips: [], + pins: [] + }; + + // Merge with existing data if we are recovering to not lose trips? + // "recover" flag means we found a user that IS us. + if (recover && existingUserStr) { + try { + const existing = JSON.parse(existingUserStr); + userData.created_at = existing.created_at || userData.created_at; + userData.trips = existing.trips || []; + userData.pins = existing.pins || []; + // Update bindings and generic info + } catch(e) {} + } - await DB.put(userKey, JSON.stringify(userData)); - await DB.put(bindingKey, username); + await DB.put(userKey, JSON.stringify(userData)); + await DB.put(bindingKey, username); } // 4. Create Session @@ -102,6 +146,6 @@ export async function onRequest(event) { return Response.redirect(`${url.origin}/?token=${token}&username=${username}`, 302); } catch (e) { - return new Response(JSON.stringify({ error: "Auth Failed", details: e.message }), { status: 500 }); + return new Response(JSON.stringify({ error: "Auth Failed", details: e.message }), { status: 500, headers: { "Content-Type": "application/json" } }); } } From c08944b8e3ca3a5e12621cfb11dfa7395b3df5db Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 5 Jan 2026 09:19:38 +0000 Subject: [PATCH 2/3] fix: simplify oauth binding and error on collision --- .../api/auth/callback/github.js | 65 +++---------------- 1 file changed, 8 insertions(+), 57 deletions(-) diff --git a/public/edge-functions/api/auth/callback/github.js b/public/edge-functions/api/auth/callback/github.js index 2de7829..2f293e0 100644 --- a/public/edge-functions/api/auth/callback/github.js +++ b/public/edge-functions/api/auth/callback/github.js @@ -12,7 +12,6 @@ export async function onRequest(event) { return new Response(JSON.stringify({ error: "Missing code" }), { status: 400, headers: { "Content-Type": "application/json" } }); } - // Access env inside try block to catch ReferenceErrors if env is missing const CLIENT_ID = env.CLIENT_ID; const CLIENT_SECRET = env.CLIENT_SECRET; @@ -58,54 +57,18 @@ export async function onRequest(event) { if (!username) { // Registration / Binding - let candidateUsername = `github_${githubUser.login}`; - let userKey = `user:${candidateUsername}`; - let existingUserStr = await DB.get(userKey); - let recover = false; - - if (existingUserStr) { - // Collision or previous registration? - try { - const existingUser = JSON.parse(existingUserStr); - if (existingUser.bindings && existingUser.bindings.github && existingUser.bindings.github.id === githubUser.id) { - // It's the same user! The binding key was lost. Recover it. - recover = true; - } - } catch (e) { - // Ignore parse error, treat as collision - } - - if (!recover) { - // Real collision, switch to ID-based username - candidateUsername = `github_${githubUser.login}_${githubUser.id}`; - userKey = `user:${candidateUsername}`; - - // Double check if THIS exists (paranoia check / recovering ID-based user) - existingUserStr = await DB.get(userKey); - if (existingUserStr) { - try { - const existingUser = JSON.parse(existingUserStr); - if (existingUser.bindings && existingUser.bindings.github && existingUser.bindings.github.id === githubUser.id) { - recover = true; - } else { - throw new Error("Username collision for backup name"); - } - } catch (e) { - if (e.message === "Username collision for backup name") throw e; - // If parse error here, maybe we should overwrite? - // But safer to assume collision. - throw new Error("Username collision for backup name (corrupt data)"); - } - } - } + const candidateUsername = `github_${githubUser.login}`; + const userKey = `user:${candidateUsername}`; + + // Strict check: If username is taken, ERROR. No switching, no recovery. + const existingUser = await DB.get(userKey); + + if (existingUser) { + throw new Error(`Username '${candidateUsername}' is already taken.`); } username = candidateUsername; - // If we are recovering, we don't need to overwrite the user data, just the binding. - // But updating the user data (e.g. avatar) is good practice. - // So we always put. - const userData = { username, created_at: new Date().toISOString(), @@ -121,18 +84,6 @@ export async function onRequest(event) { pins: [] }; - // Merge with existing data if we are recovering to not lose trips? - // "recover" flag means we found a user that IS us. - if (recover && existingUserStr) { - try { - const existing = JSON.parse(existingUserStr); - userData.created_at = existing.created_at || userData.created_at; - userData.trips = existing.trips || []; - userData.pins = existing.pins || []; - // Update bindings and generic info - } catch(e) {} - } - await DB.put(userKey, JSON.stringify(userData)); await DB.put(bindingKey, username); } From c07a8f13b98219cb83155be5981ae3812ff186fa Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 5 Jan 2026 10:04:31 +0000 Subject: [PATCH 3/3] refactor: move oauth callback logic to oauth.js --- .../api/auth/callback/github.js | 102 --------------- public/edge-functions/api/auth/oauth.js | 119 ++++++++++++++++-- 2 files changed, 109 insertions(+), 112 deletions(-) delete mode 100644 public/edge-functions/api/auth/callback/github.js diff --git a/public/edge-functions/api/auth/callback/github.js b/public/edge-functions/api/auth/callback/github.js deleted file mode 100644 index 2f293e0..0000000 --- a/public/edge-functions/api/auth/callback/github.js +++ /dev/null @@ -1,102 +0,0 @@ -export async function onRequest(event) { - try { - const url = new URL(event.request.url); - const code = url.searchParams.get("code"); - const error = url.searchParams.get("error"); - - if (error) { - return new Response(JSON.stringify({ error }), { status: 400, headers: { "Content-Type": "application/json" } }); - } - - if (!code) { - return new Response(JSON.stringify({ error: "Missing code" }), { status: 400, headers: { "Content-Type": "application/json" } }); - } - - const CLIENT_ID = env.CLIENT_ID; - const CLIENT_SECRET = env.CLIENT_SECRET; - - if (!CLIENT_ID || !CLIENT_SECRET) { - return new Response(JSON.stringify({ error: "Server Configuration Error" }), { status: 500, headers: { "Content-Type": "application/json" } }); - } - - // 1. Exchange code for access token - const tokenResponse = await fetch('https://github.com/login/oauth/access_token', { - method: 'POST', - headers: { - 'Accept': 'application/json', - 'Content-Type': 'application/json' - }, - body: JSON.stringify({ - client_id: CLIENT_ID, - client_secret: CLIENT_SECRET, - code: code - }) - }); - - const tokenData = await tokenResponse.json(); - if (tokenData.error) throw new Error(tokenData.error_description || "Failed to get token"); - const accessToken = tokenData.access_token; - - // 2. Fetch User Info - const userResponse = await fetch('https://api.github.com/user', { - headers: { - 'Authorization': `Bearer ${accessToken}`, - 'User-Agent': 'RailRound-EdgeFunction' - } - }); - - if (!userResponse.ok) throw new Error("Failed to fetch GitHub user"); - const githubUser = await userResponse.json(); - - // 3. Database Logic - const DB = globalThis.RAILROUND_KV; - if (!DB) throw new Error("KV Missing"); - - const bindingKey = `binding:github:${githubUser.id}`; - let username = await DB.get(bindingKey); - - if (!username) { - // Registration / Binding - const candidateUsername = `github_${githubUser.login}`; - const userKey = `user:${candidateUsername}`; - - // Strict check: If username is taken, ERROR. No switching, no recovery. - const existingUser = await DB.get(userKey); - - if (existingUser) { - throw new Error(`Username '${candidateUsername}' is already taken.`); - } - - username = candidateUsername; - - const userData = { - username, - created_at: new Date().toISOString(), - bindings: { - github: { - id: githubUser.id, - login: githubUser.login, - avatar_url: githubUser.avatar_url, - name: githubUser.name - } - }, - trips: [], - pins: [] - }; - - await DB.put(userKey, JSON.stringify(userData)); - await DB.put(bindingKey, username); - } - - // 4. Create Session - const token = crypto.randomUUID(); - const sessionKey = `session:${token}`; - await DB.put(sessionKey, username, { expirationTtl: 86400 * 30 }); - - // 5. Redirect to App - return Response.redirect(`${url.origin}/?token=${token}&username=${username}`, 302); - - } catch (e) { - return new Response(JSON.stringify({ error: "Auth Failed", details: e.message }), { status: 500, headers: { "Content-Type": "application/json" } }); - } -} diff --git a/public/edge-functions/api/auth/oauth.js b/public/edge-functions/api/auth/oauth.js index b15271d..9c220a9 100644 --- a/public/edge-functions/api/auth/oauth.js +++ b/public/edge-functions/api/auth/oauth.js @@ -8,26 +8,125 @@ export async function onRequest(event) { return new Response(null, { headers }); } - const url = new URL(event.request.url); - const provider = url.searchParams.get("provider"); + try { + const url = new URL(event.request.url); + const code = url.searchParams.get("code"); + const error = url.searchParams.get("error"); + const provider = url.searchParams.get("provider"); - // GitHub OAuth - if (provider === 'github') { + // 1. Handle Callback (if 'code' is present) + if (code) { + if (error) { + return new Response(JSON.stringify({ error }), { status: 400, headers: { "Content-Type": "application/json" } }); + } + + const CLIENT_ID = env.CLIENT_ID; + const CLIENT_SECRET = env.CLIENT_SECRET; + + if (!CLIENT_ID || !CLIENT_SECRET) { + return new Response(JSON.stringify({ error: "Server Configuration Error" }), { status: 500, headers: { "Content-Type": "application/json" } }); + } + + // Exchange code for access token + const tokenResponse = await fetch('https://github.com/login/oauth/access_token', { + method: 'POST', + headers: { + 'Accept': 'application/json', + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ + client_id: CLIENT_ID, + client_secret: CLIENT_SECRET, + code: code + }) + }); + + const tokenData = await tokenResponse.json(); + if (tokenData.error) throw new Error(tokenData.error_description || "Failed to get token"); + const accessToken = tokenData.access_token; + + // Fetch User Info + const userResponse = await fetch('https://api.github.com/user', { + headers: { + 'Authorization': `Bearer ${accessToken}`, + 'User-Agent': 'RailRound-EdgeFunction' + } + }); + + if (!userResponse.ok) throw new Error("Failed to fetch GitHub user"); + const githubUser = await userResponse.json(); + + // Database Logic + const DB = globalThis.RAILROUND_KV; + if (!DB) throw new Error("KV Missing"); + + const bindingKey = `binding:github:${githubUser.id}`; + let username = await DB.get(bindingKey); + + if (!username) { + // Registration / Binding + const candidateUsername = `github_${githubUser.login}`; + const userKey = `user:${candidateUsername}`; + + // Strict check: If username is taken, ERROR. No switching, no recovery. + const existingUser = await DB.get(userKey); + + if (existingUser) { + throw new Error(`Username '${candidateUsername}' is already taken.`); + } + + username = candidateUsername; + + const userData = { + username, + created_at: new Date().toISOString(), + bindings: { + github: { + id: githubUser.id, + login: githubUser.login, + avatar_url: githubUser.avatar_url, + name: githubUser.name + } + }, + trips: [], + pins: [] + }; + + await DB.put(userKey, JSON.stringify(userData)); + await DB.put(bindingKey, username); + } + + // Create Session + const token = crypto.randomUUID(); + const sessionKey = `session:${token}`; + await DB.put(sessionKey, username, { expirationTtl: 86400 * 30 }); + + // Redirect to App + return Response.redirect(`${url.origin}/?token=${token}&username=${username}`, 302); + } + + // 2. Handle Initiation (if 'provider' is present) + if (provider === 'github') { const clientId = env.CLIENT_ID; if (!clientId) { return new Response(JSON.stringify({ error: "Server Configuration Error: Missing CLIENT_ID" }), { status: 500, headers: { "Content-Type": "application/json" } }); } - const redirectUri = `${url.origin}/api/auth/callback/github`; + // Point back to THIS file for the callback + const redirectUri = `${url.origin}/api/auth/oauth`; const targetUrl = `https://github.com/login/oauth/authorize?client_id=${clientId}&redirect_uri=${redirectUri}&scope=read:user`; return Response.redirect(targetUrl, 302); - } + } - // Google Placeholder (File structure remains, implementation pending) - if (provider === 'google') { + // Google Placeholder + if (provider === 'google') { return new Response(JSON.stringify({ error: "Google Auth not implemented yet" }), { status: 501, headers: { "Content-Type": "application/json" } }); - } + } + + return new Response(JSON.stringify({ error: "Invalid Request" }), { status: 400, headers: { "Content-Type": "application/json" } }); - return new Response(JSON.stringify({ error: "Provider not supported" }), { status: 400, headers: { "Content-Type": "application/json" } }); + } catch (e) { + return new Response(JSON.stringify({ error: "Auth Failed", details: e.message }), { status: 500, headers: { "Content-Type": "application/json" } }); + } }