From 5963fe53619e527cdc2dddab79c8b3212fc0e09d Mon Sep 17 00:00:00 2001 From: UnschooledGamer <76094069+UnschooledGamer@users.noreply.github.com> Date: Sat, 22 Nov 2025 21:53:42 +0530 Subject: [PATCH 01/13] feat: Basic/starter OAUTH implementation with GitHub OAuth integration. --- server/apis/oauth.js | 108 +++++++++++++++ server/routes/apis.js | 1 + server/services/oauth/OAuthProviderFactory.js | 31 +++++ server/services/oauth/OAuthService.js | 131 ++++++++++++++++++ server/services/oauth/SessionStateService.js | 59 ++++++++ server/services/oauth/providers/github.js | 84 +++++++++++ 6 files changed, 414 insertions(+) create mode 100644 server/apis/oauth.js create mode 100644 server/services/oauth/OAuthProviderFactory.js create mode 100644 server/services/oauth/OAuthService.js create mode 100644 server/services/oauth/SessionStateService.js create mode 100644 server/services/oauth/providers/github.js diff --git a/server/apis/oauth.js b/server/apis/oauth.js new file mode 100644 index 0000000..b975fb2 --- /dev/null +++ b/server/apis/oauth.js @@ -0,0 +1,108 @@ +const { Router } = require('express'); +const crypto = require('node:crypto'); +const OAuthProviderFactory = require('../services/oauth/OAuthProviderFactory'); +const SessionStateService = require('../services/oauth/SessionStateService'); +const login = require('../entities/login'); +const user = require('../entities/user'); +const apis = require('../routes/apis'); + +const route = Router(); + +route.get('/:provider', async (req, res) => { + const { provider } = req.params; + + try { + if (!provider) { + return res.status(400).send('No provider provided'); + } + + const oAuthProvider = OAuthProviderFactory.getProvider(provider); + + const state = SessionStateService.generateState(); + console.log(`Auth state token: `, state) + + res.cookie('oauthProvider', provider, { secure: true }); + res.cookie('oauthState', state, { secure: true, signed: true, httpOnly: true, maxAge: 10 * 60 * 1000 }); + + const authURL = oAuthProvider.getAuthorizationUrl(state) + + res.redirect(authURL); + } catch (e) { + console.error(`[OAuth Router] - OAuth initiation (route: ${req.route}) error:`, e); + res.status(400).json({ error: e.message }); + } +}) + +route.get('/:provider/callback', async (req, res) => { + + const { provider } = req.params; + const { code, state, error } = req.query; + + try { + if (!provider) { + return res.status(400).send('No provider provided'); + } + + if(error) { + console.error(`[OAuth Router] - Provider (${provider}) responded with an error: ${error}`); + return; + } + + if(!code) { + console.error(`[OAuth Router] - Provider (${provider}) responded without a code: ${code}`); + } + + const storedState = req.signedCookies.oauthState; + const storedProvider = req.cookies?.oauthProvider; + console.log("[stored] Auth state token, provider and received state ", { storedState, state, provider}); + if(!storedState || !SessionStateService.verifyState(state)) { + res.status(422).send("Invalid State") + return; + } + + if(storedProvider !== provider) { + console.error(`[OAuth Router] - Provider (${provider}) mismatch: ${storedProvider} !== ${provider}`); + res.status(422).send("OAuth Provider mismatch"); + return; + } + + res.clearCookie('oauthProvider'); + res.clearCookie('state'); + + const OAuthProvider = OAuthProviderFactory.getProvider(provider); + + const tokens = await OAuthProvider.getAccessToken(code).catch(() => {}); + + if(!tokens.accessToken || !tokens) { + res.status(401).send("Failed to retrieve access token"); + return; + } + + console.log(`[OAuth Router] - Provider (${provider}) responded with an tokens`, tokens); + + const profile = await OAuthProvider.getUserProfile(tokens.accessToken).catch((e) => { + res.status(401).send(e?.message || "Failed to retrieve user profile"); + }); + + if(!profile) return; + + console.log(`[OAuth Router] - Provider (${provider}) responded with profile`, profile); + + return res.status(200).send(`Fetched From Github, Hello ${profile.username} (${profile.name}`); + + // const userRow = await user.get([user.EMAIL, email]); + // + // // Generate our own random token... + // const token = crypto.randomBytes(64).toString('hex'); + // const expiredAt = moment().add(1, 'week').format('YYYY-MM-DD HH:mm:ss.sss'); + // + // // Store the token we have generated in the login table + // await login.insert([login.USER_ID, userRow[0].id], [login.TOKEN, token], [login.EXPIRED_AT, expiredAt]); + + } catch (e) { + console.error(`[OAuth Router] - OAuth callback (route: ${req.path}) error:`, e); + return res.sendStatus(500) + } +}) + +module.exports = route; \ No newline at end of file diff --git a/server/routes/apis.js b/server/routes/apis.js index a5fb8ac..148d9e6 100644 --- a/server/routes/apis.js +++ b/server/routes/apis.js @@ -13,6 +13,7 @@ apis.use(['/comments', '/comment'], require('../apis/comment')); apis.use('/faqs', require('../apis/faqs')); apis.use('/admin', require('../apis/admin')); apis.use(['/sponsor', '/sponsors'], require('../apis/sponsor')); +apis.use('/oauth', require('../apis/oauth')); // apis.use('/completion', require('../apis/completion')); apis.get('/status', (_req, res) => { diff --git a/server/services/oauth/OAuthProviderFactory.js b/server/services/oauth/OAuthProviderFactory.js new file mode 100644 index 0000000..6b0e00d --- /dev/null +++ b/server/services/oauth/OAuthProviderFactory.js @@ -0,0 +1,31 @@ +const githubOAuthProvider = require('./providers/github'); + +class OAuthProviderFactory { + constructor() { + this.providers = new Map(); + this.initializeProviders(); + } + + /** + * Function responsible to initialize and + * register OAuth Providers. + */ + initializeProviders() { + this.providers.set('github', new githubOAuthProvider()); + } + + getProvider(providerName) { + const provider = this.providers.get(providerName.toLowerCase()); + if (!provider) { + throw new Error(`OAuth provider '${providerName}' not supported`); + } + return provider; + } + + getSupportedProviders() { + return Array.from(this.providers.keys()); + } +} + +// Standalone Class. +module.exports = new OAuthProviderFactory(); \ No newline at end of file diff --git a/server/services/oauth/OAuthService.js b/server/services/oauth/OAuthService.js new file mode 100644 index 0000000..bbf58e2 --- /dev/null +++ b/server/services/oauth/OAuthService.js @@ -0,0 +1,131 @@ +/** + * @typedef OAuthServiceConfig + * @property {string} clientId Required for OAuth. + * @property {string} clientSecret Required for OAuth. + * @property {string} [redirectUri] optional, Redirect URI, which the user/client is redirected after completion. + * @property {string} authorizationUrl Authorization Server URL **Required** to Redirect the User to the URL to connect their accounts with Authorization Server. + * @property {string} tokenUrl Required used for retrieving Access, (sometimes Also) Refresh Tokens. + * @property {string} userInfoUrl Required for Retrieve User's Profile Data. + * @property {string} scopes Required To request permissions for read/write on User's Profile. + * @property {string} providerName Good Name For The Provider. + */ + +class OAuthService { + /** + * @param config {OAuthServiceConfig} + */ + constructor(config) { + Object.freeze(config); + + this.clientId = config.clientId; + this.clientSecret = config.clientSecret; + this.redirectUri = config?.redirectUri; + this.authorizationUrl = config.authorizationUrl; + this.tokenUrl = config.tokenUrl; + this.userInfoUrl = config.userInfoUrl; + this.scopes = config.scopes; + this.providerName = config.providerName; + + ;["clientId", "clientSecret", "authorizationUrl", "tokenUrl", "userInfoUrl", "scopes", "providerName"].forEach(p=> { + if(!this?.[p]) throw RangeError(`"${p}" is required, but not specified in OAuthService Class Configuration`); + }) + } + + // Generate authorization URL + getAuthorizationUrl(state) { + if(!state) throw ReferenceError("state is missing for generating Authorization URL"); + + const params = new URLSearchParams({ + response_type: 'code', + client_id: this.clientId, + state: state, // CSRF protection + scope: this.scopes.join(' '), + }); + + if(this.redirectUri) params.append('redirect_uri', this.redirectUri); + + return `${this.authorizationUrl}?${params.toString()}`; + } + + // Exchange authorization code for access token + async getAccessToken(code) { + if(!code) { + throw Error(`[${this.providerName} - getAccessToken] Code is required: ${code}`); + } + try { + const response = await fetch( + this.tokenUrl, + { + method: 'POST', + body: JSON.stringify({ + client_id: this.clientId, + client_secret: this.clientSecret, + code: code, + redirect_uri: this.redirectUri, + grant_type: 'authorization_code' + }), + headers: { + 'Accept': 'application/json', + 'Content-Type': 'application/json' + } + } + ); + // TODO: Throw Error, In Case where Response's body has error property + console.log(`[${this.providerName} - getAccessToken] Response status: ${response.status} (${response.statusText})`, await response.json()); + + return this.normalizeTokenResponse(await response.json()); + } catch (error) { + console.error(`${this.providerName} token exchange error:`, error.response?.data || error.message); + throw new Error(`Failed to exchange code for token with ${this.providerName}`); + } + } + + // Normalize token response across providers + normalizeTokenResponse(data) { + return { + accessToken: data.access_token, + refreshToken: data.refresh_token || null, + expiresIn: data.expires_in || null, + tokenType: data.token_type || 'Bearer', + scope: data.scope + }; + } + + // Fetch user profile (to be overridden by specific providers) + async getUserProfile(accessToken) { + throw new Error('getUserProfile must be implemented by provider-specific service'); + } + + // Refresh access token + async refreshAccessToken(refreshToken) { + if (!refreshToken) { + throw new Error('No refresh token available'); + } + + try { + const response = await fetch( + this.tokenUrl, + { + method: 'POST', + body: JSON.stringify({ + client_id: this.clientId, + client_secret: this.clientSecret, + refresh_token: refreshToken, + grant_type: 'refresh_token' + }), + headers: { + 'Accept': 'application/json', + 'Content-Type': 'application/json' + } + }, + ); + + return this.normalizeTokenResponse(await response.json()); + } catch (error) { + console.error(`${this.providerName} token refresh error:`, error.response?.data || error.message); + throw new Error(`Failed to refresh token with ${this.providerName}`); + } + } +} + +module.exports = OAuthService; \ No newline at end of file diff --git a/server/services/oauth/SessionStateService.js b/server/services/oauth/SessionStateService.js new file mode 100644 index 0000000..68cb391 --- /dev/null +++ b/server/services/oauth/SessionStateService.js @@ -0,0 +1,59 @@ +const crypto = require('node:crypto'); + +class SessionStateService { + constructor() { + this.states = new Map(); + } + + // Generate a random state token for CSRF protection + generateState() { + const state = crypto.randomBytes(32).toString('hex'); + this.states.set(state, { + createdAt: Date.now(), + used: false + }); + + // Clean up old states (older than 10 minutes) + this.cleanup(); + + return state; + } + + // Verify and consume a state token + verifyState(state) { + const stateData = this.states.get(state); + + if (!stateData) { + return false; + } + + if (stateData.used) { + return false; + } + + // Check if state is older than 10 minutes + const tenMinutes = 10 * 60 * 1000; + if (Date.now() - stateData.createdAt > tenMinutes) { + this.states.delete(state); + return false; + } + + // Mark as used and delete + this.states.delete(state); + return true; + } + + // Clean up old states + cleanup() { + const tenMinutes = 10 * 60 * 1000; + const now = Date.now(); + + for (const [state, data] of this.states.entries()) { + if (now - data.createdAt > tenMinutes) { + this.states.delete(state); + } + } + } +} + +module.exports = new SessionStateService(); \ No newline at end of file diff --git a/server/services/oauth/providers/github.js b/server/services/oauth/providers/github.js new file mode 100644 index 0000000..8c78961 --- /dev/null +++ b/server/services/oauth/providers/github.js @@ -0,0 +1,84 @@ +const OAuthService = require('../OAuthService'); +const { response } = require('express'); + +class githubOAuthProvider extends OAuthService { + constructor() { + super({ + clientId: process.env.GITHUB_CLIENT_ID, + clientSecret: process.env.GITHUB_CLIENT_SECRET, + redirectUri: process.env.GITHUB_CALLBACK_URL, + authorizationUrl: 'https://github.com/login/oauth/authorize', + tokenUrl: 'https://github.com/login/oauth/access_token', + userInfoUrl: 'https://api.github.com/user', + scopes: ['read:user', 'user:email'], + providerName: 'github' + }); + } + + async getUserProfile(accessToken) { + try { + const userProfileResponse = await fetch(this.userInfoUrl, { + headers: { + Authorization: `Bearer ${accessToken}`, + 'Content-Type': 'application/json' + } + }); + + const profile = await userProfileResponse.json(); + + // GitHub may not return email in profile, fetch separately + let email = profile.email; + if (!email) { + const emailResponse = await fetch('https://api.github.com/user/emails', { + headers: { + 'Authorization': `Bearer ${accessToken}`, + 'Accept': 'application/json' + } + }); + + if(!emailResponse.ok) { + // ignore warning, Rethrown after catching.... + throw Error(`Failed to fetch User Email (response status: ${emailResponse.status}, response msg: ${response.json()?.message})`) + } + + emailResponse.data = await emailResponse.json(); + + // Find primary verified email + const primaryEmail = emailResponse.data.find(e => e.primary && e.verified); + email = primaryEmail ? primaryEmail.email : emailResponse.data[0]?.email; + + if(!email) { + // ignore warning, Rethrown after catching.... + throw Error(`${emailResponse.data?.error_description} (see: ${emailResponse.data?.error_uri})`); + } + } + + return { + id: profile.id.toString(), + email: email, + name: profile.name || profile.login, + username: profile.login, + avatar_url: profile.avatar_url, + profile_url: profile.html_url, + bio: profile.bio, + raw: profile + }; + + } catch (e) { + console.error('[githubOAuthProvider - getUserProfile] GitHub profile fetch error:', e?.response?.data || e?.message); + throw e; + } + } + + // GitHub OAuth App tokens don't expire + calculateTokenExpiry(expiresIn) { + return null; + } + + // GitHub OAuth Apps don't support refresh tokens + async refreshAccessToken(refreshToken) { + throw new Error('GitHub OAuth Apps do not support token refresh'); + } +} + +module.exports = githubOAuthProvider; \ No newline at end of file From 291e342aec54b49387f119b893fac0a7635d631a Mon Sep 17 00:00:00 2001 From: UnschooledGamer <76094069+UnschooledGamer@users.noreply.github.com> Date: Wed, 26 Nov 2025 13:14:15 +0530 Subject: [PATCH 02/13] feat: Add new dependencies for better-fetch and zod, and refactor OAuth routes for improved error handling --- package.json | 1 + server/apis/oauth.js | 29 ++++---- server/services/oauth/OAuthService.js | 97 +++++++++++++++++++-------- 3 files changed, 87 insertions(+), 40 deletions(-) diff --git a/package.json b/package.json index 4a32818..7a65178 100644 --- a/package.json +++ b/package.json @@ -35,6 +35,7 @@ "webpack-cli": "6.0.1" }, "dependencies": { + "@better-fetch/fetch": "^1.1.18", "@google-cloud/storage": "^7.16.0", "@googleapis/androidpublisher": "^29.3.0", "autosize": "^6.0.1", diff --git a/server/apis/oauth.js b/server/apis/oauth.js index b975fb2..32ae99b 100644 --- a/server/apis/oauth.js +++ b/server/apis/oauth.js @@ -1,14 +1,12 @@ const { Router } = require('express'); -const crypto = require('node:crypto'); const OAuthProviderFactory = require('../services/oauth/OAuthProviderFactory'); const SessionStateService = require('../services/oauth/SessionStateService'); const login = require('../entities/login'); const user = require('../entities/user'); -const apis = require('../routes/apis'); -const route = Router(); +const router = Router(); -route.get('/:provider', async (req, res) => { +router.get('/:provider', async (req, res) => { const { provider } = req.params; try { @@ -19,7 +17,6 @@ route.get('/:provider', async (req, res) => { const oAuthProvider = OAuthProviderFactory.getProvider(provider); const state = SessionStateService.generateState(); - console.log(`Auth state token: `, state) res.cookie('oauthProvider', provider, { secure: true }); res.cookie('oauthState', state, { secure: true, signed: true, httpOnly: true, maxAge: 10 * 60 * 1000 }); @@ -28,12 +25,12 @@ route.get('/:provider', async (req, res) => { res.redirect(authURL); } catch (e) { - console.error(`[OAuth Router] - OAuth initiation (route: ${req.route}) error:`, e); + console.error(`[OAuth Router] - OAuth initiation (route: ${req.path}) error:`, e); res.status(400).json({ error: e.message }); } }) -route.get('/:provider/callback', async (req, res) => { +router.get('/:provider/callback', async (req, res) => { const { provider } = req.params; const { code, state, error } = req.query; @@ -45,35 +42,41 @@ route.get('/:provider/callback', async (req, res) => { if(error) { console.error(`[OAuth Router] - Provider (${provider}) responded with an error: ${error}`); + res.redirect(`/login?error=${error}&error_description=${error?.error_description}`); return; } if(!code) { console.error(`[OAuth Router] - Provider (${provider}) responded without a code: ${code}`); + res.redirect(`/login?error=missing_code`); + return; } const storedState = req.signedCookies.oauthState; const storedProvider = req.cookies?.oauthProvider; console.log("[stored] Auth state token, provider and received state ", { storedState, state, provider}); if(!storedState || !SessionStateService.verifyState(state)) { - res.status(422).send("Invalid State") + res.clearCookie('oauthState'); + // res.status(422).send("Invalid State") + res.redirect(`/login?error=invalid_state`); return; } if(storedProvider !== provider) { console.error(`[OAuth Router] - Provider (${provider}) mismatch: ${storedProvider} !== ${provider}`); - res.status(422).send("OAuth Provider mismatch"); + // res.status(422).send("OAuth Provider mismatch"); + res.redirect(`/login?error=oauth_provider_mismatch`); return; } - res.clearCookie('oauthProvider'); - res.clearCookie('state'); + // res.clearCookie('oauthProvider'); + res.clearCookie('oauthState'); const OAuthProvider = OAuthProviderFactory.getProvider(provider); const tokens = await OAuthProvider.getAccessToken(code).catch(() => {}); - if(!tokens.accessToken || !tokens) { + if(!tokens?.accessToken || !tokens) { res.status(401).send("Failed to retrieve access token"); return; } @@ -105,4 +108,4 @@ route.get('/:provider/callback', async (req, res) => { } }) -module.exports = route; \ No newline at end of file +module.exports = router; \ No newline at end of file diff --git a/server/services/oauth/OAuthService.js b/server/services/oauth/OAuthService.js index bbf58e2..728c200 100644 --- a/server/services/oauth/OAuthService.js +++ b/server/services/oauth/OAuthService.js @@ -1,3 +1,7 @@ +const { betterFetch } = require('@better-fetch/fetch'); + +const { z } = require('zod'); + /** * @typedef OAuthServiceConfig * @property {string} clientId Required for OAuth. @@ -8,6 +12,7 @@ * @property {string} userInfoUrl Required for Retrieve User's Profile Data. * @property {string} scopes Required To request permissions for read/write on User's Profile. * @property {string} providerName Good Name For The Provider. + * @property {string} [prompt] */ class OAuthService { @@ -25,7 +30,9 @@ class OAuthService { this.userInfoUrl = config.userInfoUrl; this.scopes = config.scopes; this.providerName = config.providerName; + this.prompt = config.prompt || ""; + // biome-ignore lint/complexity/noForEach: ignore ;["clientId", "clientSecret", "authorizationUrl", "tokenUrl", "userInfoUrl", "scopes", "providerName"].forEach(p=> { if(!this?.[p]) throw RangeError(`"${p}" is required, but not specified in OAuthService Class Configuration`); }) @@ -53,29 +60,45 @@ class OAuthService { throw Error(`[${this.providerName} - getAccessToken] Code is required: ${code}`); } try { - const response = await fetch( - this.tokenUrl, - { - method: 'POST', - body: JSON.stringify({ - client_id: this.clientId, - client_secret: this.clientSecret, - code: code, - redirect_uri: this.redirectUri, - grant_type: 'authorization_code' - }), - headers: { - 'Accept': 'application/json', - 'Content-Type': 'application/json' - } - } - ); - // TODO: Throw Error, In Case where Response's body has error property - console.log(`[${this.providerName} - getAccessToken] Response status: ${response.status} (${response.statusText})`, await response.json()); - return this.normalizeTokenResponse(await response.json()); + const { data: response, error } = await betterFetch(this.tokenUrl, { + method: "POST", + body: JSON.stringify({ + client_id: this.clientId, + client_secret: this.clientSecret, + code: code, + redirect_uri: this.redirectUri, + grant_type: 'authorization_code' + }), + headers: { + 'Accept': 'application/json', + 'Content-Type': 'application/json' + }, + output: z.object({ + access_token: z.string(), + refresh_token: z.string().optional(), + scope: z.string(), + token_type: z.string(), + expires_in: z.number().optional(), + }).or( + z.object({ + error: z.string(), + error_description: z.string().optional(), + error_hint: z.string().optional(), + }) + ) + }) + + if(!response || response.error || error) { + console.error(`[${this.providerName} - getAccessToken] Response status: ${response.status} (${response.statusText})`, response|| error); + throw response || error; + } + + console.log(`[${this.providerName} - getAccessToken]`, response); + + return this.normalizeTokenResponse(response); } catch (error) { - console.error(`${this.providerName} token exchange error:`, error.response?.data || error.message); + console.error(`${this.providerName} token exchange error:`, error); throw new Error(`Failed to exchange code for token with ${this.providerName}`); } } @@ -92,6 +115,7 @@ class OAuthService { } // Fetch user profile (to be overridden by specific providers) + // biome-ignore lint/correctness/noUnusedFunctionParameters: unused here as it is overridden. async getUserProfile(accessToken) { throw new Error('getUserProfile must be implemented by provider-specific service'); } @@ -103,7 +127,8 @@ class OAuthService { } try { - const response = await fetch( + + const { data: response, error } = betterFetch( this.tokenUrl, { method: 'POST', @@ -116,13 +141,31 @@ class OAuthService { headers: { 'Accept': 'application/json', 'Content-Type': 'application/json' - } - }, - ); + }, + output: z.object({ + access_token: z.string(), + refresh_token: z.string(), + scope: z.string(), + token_type: z.string(), + expires_in: z.number().optional(), + }).or( + z.object({ + error: z.string(), + error_description: z.string().optional(), + error_hint: z.string().optional(), + }) + ) + } + ) + + if(!response || response.error || error) { + console.error(`[${this.providerName} - refreshAccessToken] Response status: ${response.status} (${response.statusText})`, response|| error); + throw response || error; + } - return this.normalizeTokenResponse(await response.json()); + return this.normalizeTokenResponse(response); } catch (error) { - console.error(`${this.providerName} token refresh error:`, error.response?.data || error.message); + console.error(`${this.providerName} token refresh error:`, error); throw new Error(`Failed to refresh token with ${this.providerName}`); } } From 8133b82562de10b172599da5116dca4b7e191333 Mon Sep 17 00:00:00 2001 From: UnschooledGamer <76094069+UnschooledGamer@users.noreply.github.com> Date: Wed, 26 Nov 2025 19:02:12 +0530 Subject: [PATCH 03/13] feat: Introduce authenticateWithProvider function and refactor OAuth error redirects to use a constant path --- server/apis/oauth.js | 19 +++++++++++-------- server/lib/authenticateWithProvider.js | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 8 deletions(-) create mode 100644 server/lib/authenticateWithProvider.js diff --git a/server/apis/oauth.js b/server/apis/oauth.js index 32ae99b..6cde347 100644 --- a/server/apis/oauth.js +++ b/server/apis/oauth.js @@ -3,6 +3,9 @@ const OAuthProviderFactory = require('../services/oauth/OAuthProviderFactory'); const SessionStateService = require('../services/oauth/SessionStateService'); const login = require('../entities/login'); const user = require('../entities/user'); +const moment = require('moment'); + +const SUCCESS_REDIRECT_PATH = `/login`; const router = Router(); @@ -42,13 +45,13 @@ router.get('/:provider/callback', async (req, res) => { if(error) { console.error(`[OAuth Router] - Provider (${provider}) responded with an error: ${error}`); - res.redirect(`/login?error=${error}&error_description=${error?.error_description}`); + res.redirect(`${SUCCESS_REDIRECT_PATH}?error=${error}&error_description=${error?.error_description}`); return; } if(!code) { console.error(`[OAuth Router] - Provider (${provider}) responded without a code: ${code}`); - res.redirect(`/login?error=missing_code`); + res.redirect(`${SUCCESS_REDIRECT_PATH}?error=missing_code`); return; } @@ -58,14 +61,14 @@ router.get('/:provider/callback', async (req, res) => { if(!storedState || !SessionStateService.verifyState(state)) { res.clearCookie('oauthState'); // res.status(422).send("Invalid State") - res.redirect(`/login?error=invalid_state`); + res.redirect(`${SUCCESS_REDIRECT_PATH}?error=invalid_state`); return; } if(storedProvider !== provider) { console.error(`[OAuth Router] - Provider (${provider}) mismatch: ${storedProvider} !== ${provider}`); // res.status(422).send("OAuth Provider mismatch"); - res.redirect(`/login?error=oauth_provider_mismatch`); + res.redirect(`${SUCCESS_REDIRECT_PATH}?error=oauth_provider_mismatch`); return; } @@ -91,14 +94,14 @@ router.get('/:provider/callback', async (req, res) => { console.log(`[OAuth Router] - Provider (${provider}) responded with profile`, profile); - return res.status(200).send(`Fetched From Github, Hello ${profile.username} (${profile.name}`); + return res.status(200).send(`Fetched From Github, Hello ${profile.username} (${profile.name})`); - // const userRow = await user.get([user.EMAIL, email]); - // + // const userRow = await user.get([user.EMAIL, profile.email]); + // // Generate our own random token... // const token = crypto.randomBytes(64).toString('hex'); // const expiredAt = moment().add(1, 'week').format('YYYY-MM-DD HH:mm:ss.sss'); - // + // // Store the token we have generated in the login table // await login.insert([login.USER_ID, userRow[0].id], [login.TOKEN, token], [login.EXPIRED_AT, expiredAt]); diff --git a/server/lib/authenticateWithProvider.js b/server/lib/authenticateWithProvider.js new file mode 100644 index 0000000..d235884 --- /dev/null +++ b/server/lib/authenticateWithProvider.js @@ -0,0 +1,17 @@ + +async function authenticateWithProvider(providerType, profile, tokens) { + const { id: providerUserId, email, name } = profile; + const { accessToken, refreshToken, expiresIn } = tokens; + + if (!email) { + throw new Error('Email not provided by OAuth provider'); + } + + // Calculate token expiry + const tokenExpiresAt = expiresIn + ? new Date(Date.now() + expiresIn * 1000) + : null; + + // Create a user in user table, & generate a login token for it if one doesn't already exist. + // and return it. +} \ No newline at end of file From e7235c71b2ec8d13a9f1df141eb08b4d91c725cd Mon Sep 17 00:00:00 2001 From: UnschooledGamer <76094069+UnschooledGamer@users.noreply.github.com> Date: Wed, 26 Nov 2025 21:25:47 +0530 Subject: [PATCH 04/13] feat: Enhance OAuth functionality with user authentication and session token management --- server/apis/oauth.js | 33 +++++++----- server/lib/authenticateWithProvider.js | 73 ++++++++++++++++++++++++-- 2 files changed, 90 insertions(+), 16 deletions(-) diff --git a/server/apis/oauth.js b/server/apis/oauth.js index 6cde347..d814ef8 100644 --- a/server/apis/oauth.js +++ b/server/apis/oauth.js @@ -1,14 +1,14 @@ +// TODO: Lookout for vulnerabilities. +// TODO: support for Code challeges +// TODO: Custom State support for remembering redirects. const { Router } = require('express'); const OAuthProviderFactory = require('../services/oauth/OAuthProviderFactory'); const SessionStateService = require('../services/oauth/SessionStateService'); -const login = require('../entities/login'); -const user = require('../entities/user'); -const moment = require('moment'); +const authenticateWithProvider = require('../lib/authenticateWithProvider'); const SUCCESS_REDIRECT_PATH = `/login`; const router = Router(); - router.get('/:provider', async (req, res) => { const { provider } = req.params; @@ -94,16 +94,25 @@ router.get('/:provider/callback', async (req, res) => { console.log(`[OAuth Router] - Provider (${provider}) responded with profile`, profile); - return res.status(200).send(`Fetched From Github, Hello ${profile.username} (${profile.name})`); + // return res.status(200).send(`Fetched From Github, Hello ${profile.username} (${profile.name})`); - // const userRow = await user.get([user.EMAIL, profile.email]); - - // // Generate our own random token... - // const token = crypto.randomBytes(64).toString('hex'); - // const expiredAt = moment().add(1, 'week').format('YYYY-MM-DD HH:mm:ss.sss'); + const loginToken = await authenticateWithProvider(provider, profile, tokens); - // // Store the token we have generated in the login table - // await login.insert([login.USER_ID, userRow[0].id], [login.TOKEN, token], [login.EXPIRED_AT, expiredAt]); + if(!loginToken) { + res.status(500).send({ error: "Session Token issuing failed for Social Login."}); + return; + } + + res.cookie('token', loginToken, { + httpOnly: true, + secure: true, + maxAge: 7 * 24 * 60 * 60 * 1000 // 1 week + }); + + /* + NOTE: Replace the Hardcoded URL with the actual URL given by the frontend. + /* */ + return res.redirect('/user'); } catch (e) { console.error(`[OAuth Router] - OAuth callback (route: ${req.path}) error:`, e); diff --git a/server/lib/authenticateWithProvider.js b/server/lib/authenticateWithProvider.js index d235884..97ca069 100644 --- a/server/lib/authenticateWithProvider.js +++ b/server/lib/authenticateWithProvider.js @@ -1,6 +1,10 @@ +const moment = require('moment'); +const authenticationProvider = require('../entities/authenticationProvider'); +const login = require('../entities/login'); +const User = require('../entities/user'); async function authenticateWithProvider(providerType, profile, tokens) { - const { id: providerUserId, email, name } = profile; + const { id: providerUserId, email, name, username } = profile; const { accessToken, refreshToken, expiresIn } = tokens; if (!email) { @@ -12,6 +16,67 @@ async function authenticateWithProvider(providerType, profile, tokens) { ? new Date(Date.now() + expiresIn * 1000) : null; - // Create a user in user table, & generate a login token for it if one doesn't already exist. - // and return it. -} \ No newline at end of file + let userId; + + const existingLink = await authenticationProvider.get( + [authenticationProvider.ID, authenticationProvider.USER_ID], + [authenticationProvider.PROVIDER, providerType], + [authenticationProvider.PROVIDER_USER_ID, providerUserId] + ); + + if(existingLink.length > 0) { + userId = existingLink[0].user_id; + + + await authenticationProvider.update( + [ + [authenticationProvider.ID, existingLink[0].id], + [authenticationProvider.ACCESS_TOKEN, accessToken], + [authenticationProvider.REFRESH_TOKEN, refreshToken] + ] + ) + } else { + const existingUser = await User.get([User.EMAIL, email]); + + if (existingUser.length > 0) { + userId = existingUser[0].id; + } else { + + await User.insert( + [User.NAME, name], + [User.EMAIL, email], + [User.PASSWORD, ""], + [User.WEBSITE, null], + [User.GITHUB, providerType === "github" ? username : null], + ); + + const newUser = User.get([User.EMAIL, email]); + userId = newUser[0].id; + } + + await authenticationProvider.insert( + [authenticationProvider.USER_ID, userId], + [authenticationProvider.PROVIDER, providerType], + [authenticationProvider.PROVIDER_USER_ID, providerUserId], + [authenticationProvider.ACCESS_TOKEN, accessToken], + [authenticationProvider.REFRESH_TOKEN, refreshToken], + [authenticationProvider.ACCESS_TOKEN_EXPIRES_AT, tokenExpiresAt], + [authenticationProvider.REFRESH_TOKEN_EXPIRES_AT, ""], + [authenticationProvider.SCOPE, tokens.scope] + ); + } + + // break this into a Function, if it gets too repetitive throughout the whole codebase. + const sessionToken = require('node:crypto').randomBytes(64).toString('hex'); + const sessionExpiredAt = moment().add(1, 'week').format('YYYY-MM-DD HH:mm:ss.sss'); + + await login.insert( + [login.USER_ID, userId], + [login.TOKEN, sessionToken], + [login.EXPIRED_AT, sessionExpiredAt] + ) + + return sessionToken; +} + +module.exports = authenticateWithProvider; \ No newline at end of file From 8f707ef360cdcead807dc5309f1411f362a09222 Mon Sep 17 00:00:00 2001 From: UnschooledGamer <76094069+UnschooledGamer@users.noreply.github.com> Date: Thu, 27 Nov 2025 21:47:02 +0530 Subject: [PATCH 05/13] feat: Refactor OAuth session state management and enhance error handling in authentication flow --- server/apis/oauth.js | 20 ++--- server/entities/authenticationProvider.js | 53 ++++++++++++ server/lib/authenticateWithProvider.js | 37 ++++----- server/services/oauth/OAuthProviderFactory.js | 5 +- server/services/oauth/OAuthService.js | 34 ++++---- server/services/oauth/SessionStateService.js | 81 ++++++++++++------- server/services/oauth/providers/github.js | 15 ++-- 7 files changed, 161 insertions(+), 84 deletions(-) create mode 100644 server/entities/authenticationProvider.js diff --git a/server/apis/oauth.js b/server/apis/oauth.js index d814ef8..1a857d0 100644 --- a/server/apis/oauth.js +++ b/server/apis/oauth.js @@ -3,7 +3,9 @@ // TODO: Custom State support for remembering redirects. const { Router } = require('express'); const OAuthProviderFactory = require('../services/oauth/OAuthProviderFactory'); -const SessionStateService = require('../services/oauth/SessionStateService'); +const SessionStateServiceClass = require('../services/oauth/SessionStateService'); +// For single-instance use only. For clustering/horizontal scaling, inject a distributed store (e.g. Redis) into SessionStateService. +const sessionStateService = new SessionStateServiceClass(); const authenticateWithProvider = require('../lib/authenticateWithProvider'); const SUCCESS_REDIRECT_PATH = `/login`; @@ -19,9 +21,9 @@ router.get('/:provider', async (req, res) => { const oAuthProvider = OAuthProviderFactory.getProvider(provider); - const state = SessionStateService.generateState(); + const state = await sessionStateService.generateState(); - res.cookie('oauthProvider', provider, { secure: true }); + res.cookie('oauthProvider', provider, { secure: true, httpOnly: true, sameSite: 'lax' }); res.cookie('oauthState', state, { secure: true, signed: true, httpOnly: true, maxAge: 10 * 60 * 1000 }); const authURL = oAuthProvider.getAuthorizationUrl(state) @@ -36,7 +38,7 @@ router.get('/:provider', async (req, res) => { router.get('/:provider/callback', async (req, res) => { const { provider } = req.params; - const { code, state, error } = req.query; + const { code, state, error, error_description } = req.query; try { if (!provider) { @@ -45,7 +47,7 @@ router.get('/:provider/callback', async (req, res) => { if(error) { console.error(`[OAuth Router] - Provider (${provider}) responded with an error: ${error}`); - res.redirect(`${SUCCESS_REDIRECT_PATH}?error=${error}&error_description=${error?.error_description}`); + res.redirect(`${SUCCESS_REDIRECT_PATH}?error=${error}&error_description=${error_description}`); return; } @@ -57,8 +59,7 @@ router.get('/:provider/callback', async (req, res) => { const storedState = req.signedCookies.oauthState; const storedProvider = req.cookies?.oauthProvider; - console.log("[stored] Auth state token, provider and received state ", { storedState, state, provider}); - if(!storedState || !SessionStateService.verifyState(state)) { + if(!storedState || !(await sessionStateService.verifyState(state))) { res.clearCookie('oauthState'); // res.status(422).send("Invalid State") res.redirect(`${SUCCESS_REDIRECT_PATH}?error=invalid_state`); @@ -84,15 +85,14 @@ router.get('/:provider/callback', async (req, res) => { return; } - console.log(`[OAuth Router] - Provider (${provider}) responded with an tokens`, tokens); - const profile = await OAuthProvider.getUserProfile(tokens.accessToken).catch((e) => { res.status(401).send(e?.message || "Failed to retrieve user profile"); + return null; }); if(!profile) return; - console.log(`[OAuth Router] - Provider (${provider}) responded with profile`, profile); + console.log(`[OAuth Router] - Provider (${provider}) authentication successful for user ID: ${profile.id}`); // return res.status(200).send(`Fetched From Github, Hello ${profile.username} (${profile.name})`); diff --git a/server/entities/authenticationProvider.js b/server/entities/authenticationProvider.js new file mode 100644 index 0000000..a2f8514 --- /dev/null +++ b/server/entities/authenticationProvider.js @@ -0,0 +1,53 @@ +const Entity = require('./entity'); + +const table = `create table if not exists authentication_provider ( + id integer primary key, + user_id integer not null, + provider text not null, + provider_user_id text not null, + access_token text, + refresh_token text, + access_token_expires_at timestamp, + refresh_token_expires_at timestamp, + scope text, + created_at timestamp default current_timestamp, + updated_at timestamp default current_timestamp, + foreign key (user_id) references user(id), + unique(provider, provider_user_id) +);`; + +class AuthenticationProvider extends Entity { + ID = 'id'; + USER_ID = 'user_id'; + PROVIDER = 'provider'; + PROVIDER_USER_ID = 'provider_user_id'; + ACCESS_TOKEN = 'access_token'; + REFRESH_TOKEN = 'refresh_token'; + ACCESS_TOKEN_EXPIRES_AT = 'access_token_expires_at'; + REFRESH_TOKEN_EXPIRES_AT = 'refresh_token_expires_at'; + SCOPE = 'scope'; + CREATED_AT = 'created_at'; + UPDATED_AT = 'updated_at'; + + constructor() { + super(table); + } + + get columns() { + return [ + this.ID, + this.USER_ID, + this.PROVIDER, + this.PROVIDER_USER_ID, + this.ACCESS_TOKEN, + this.REFRESH_TOKEN, + this.ACCESS_TOKEN_EXPIRES_AT, + this.REFRESH_TOKEN_EXPIRES_AT, + this.SCOPE, + this.CREATED_AT, + this.UPDATED_AT, + ]; + } +} + +module.exports = new AuthenticationProvider(); \ No newline at end of file diff --git a/server/lib/authenticateWithProvider.js b/server/lib/authenticateWithProvider.js index 97ca069..5d35f5e 100644 --- a/server/lib/authenticateWithProvider.js +++ b/server/lib/authenticateWithProvider.js @@ -32,27 +32,22 @@ async function authenticateWithProvider(providerType, profile, tokens) { [ [authenticationProvider.ID, existingLink[0].id], [authenticationProvider.ACCESS_TOKEN, accessToken], - [authenticationProvider.REFRESH_TOKEN, refreshToken] + [authenticationProvider.REFRESH_TOKEN, refreshToken], + [authenticationProvider.ACCESS_TOKEN_EXPIRES_AT, tokenExpiresAt], + [authenticationProvider.SCOPE, tokens.scope] ] - ) - } else { - const existingUser = await User.get([User.EMAIL, email]); - - if (existingUser.length > 0) { - userId = existingUser[0].id; - } else { - - await User.insert( - [User.NAME, name], - [User.EMAIL, email], - [User.PASSWORD, ""], - [User.WEBSITE, null], - [User.GITHUB, providerType === "github" ? username : null], - ); - - const newUser = User.get([User.EMAIL, email]); - userId = newUser[0].id; - } + ) } else { + // Atomic, race-safe upsert pattern for user + await User.insertOrIgnore( + [User.NAME, name], + [User.EMAIL, email], + [User.PASSWORD, ""], + [User.WEBSITE, null], + [User.GITHUB, providerType === "github" ? username : null], + ); + // Always fetch the user after insert - ensures you get the correct id whether existing or new + const userRes = await User.get([User.EMAIL, email]); + userId = userRes[0].id; await authenticationProvider.insert( [authenticationProvider.USER_ID, userId], @@ -61,7 +56,7 @@ async function authenticateWithProvider(providerType, profile, tokens) { [authenticationProvider.ACCESS_TOKEN, accessToken], [authenticationProvider.REFRESH_TOKEN, refreshToken], [authenticationProvider.ACCESS_TOKEN_EXPIRES_AT, tokenExpiresAt], - [authenticationProvider.REFRESH_TOKEN_EXPIRES_AT, ""], + [authenticationProvider.REFRESH_TOKEN_EXPIRES_AT, null], [authenticationProvider.SCOPE, tokens.scope] ); } diff --git a/server/services/oauth/OAuthProviderFactory.js b/server/services/oauth/OAuthProviderFactory.js index 6b0e00d..43578f8 100644 --- a/server/services/oauth/OAuthProviderFactory.js +++ b/server/services/oauth/OAuthProviderFactory.js @@ -15,6 +15,9 @@ class OAuthProviderFactory { } getProvider(providerName) { + if (!providerName) { + throw new Error('Provider name is required'); + } const provider = this.providers.get(providerName.toLowerCase()); if (!provider) { throw new Error(`OAuth provider '${providerName}' not supported`); @@ -27,5 +30,5 @@ class OAuthProviderFactory { } } -// Standalone Class. +// Singleton instance module.exports = new OAuthProviderFactory(); \ No newline at end of file diff --git a/server/services/oauth/OAuthService.js b/server/services/oauth/OAuthService.js index 728c200..0f3a9d3 100644 --- a/server/services/oauth/OAuthService.js +++ b/server/services/oauth/OAuthService.js @@ -20,11 +20,10 @@ class OAuthService { * @param config {OAuthServiceConfig} */ constructor(config) { - Object.freeze(config); this.clientId = config.clientId; this.clientSecret = config.clientSecret; - this.redirectUri = config?.redirectUri; + this.redirectUri = config.redirectUri; this.authorizationUrl = config.authorizationUrl; this.tokenUrl = config.tokenUrl; this.userInfoUrl = config.userInfoUrl; @@ -34,7 +33,7 @@ class OAuthService { // biome-ignore lint/complexity/noForEach: ignore ;["clientId", "clientSecret", "authorizationUrl", "tokenUrl", "userInfoUrl", "scopes", "providerName"].forEach(p=> { - if(!this?.[p]) throw RangeError(`"${p}" is required, but not specified in OAuthService Class Configuration`); + if(!this[p]) throw TypeError(`"${p}" is required, but not specified in OAuthService Class Configuration`); }) } @@ -61,15 +60,20 @@ class OAuthService { } try { + const body = { + client_id: this.clientId, + client_secret: this.clientSecret, + code: code, + grant_type: 'authorization_code' + } + + if(this.redirectUri) { + body.redirect_uri = this.redirectUri; + } + const { data: response, error } = await betterFetch(this.tokenUrl, { method: "POST", - body: JSON.stringify({ - client_id: this.clientId, - client_secret: this.clientSecret, - code: code, - redirect_uri: this.redirectUri, - grant_type: 'authorization_code' - }), + body: JSON.stringify(body), headers: { 'Accept': 'application/json', 'Content-Type': 'application/json' @@ -77,7 +81,7 @@ class OAuthService { output: z.object({ access_token: z.string(), refresh_token: z.string().optional(), - scope: z.string(), + scope: z.string().optional(), token_type: z.string(), expires_in: z.number().optional(), }).or( @@ -128,7 +132,7 @@ class OAuthService { try { - const { data: response, error } = betterFetch( + const { data: response, error } = await betterFetch( this.tokenUrl, { method: 'POST', @@ -144,8 +148,8 @@ class OAuthService { }, output: z.object({ access_token: z.string(), - refresh_token: z.string(), - scope: z.string(), + refresh_token: z.string().optional(), + scope: z.string().optional(), token_type: z.string(), expires_in: z.number().optional(), }).or( @@ -159,7 +163,7 @@ class OAuthService { ) if(!response || response.error || error) { - console.error(`[${this.providerName} - refreshAccessToken] Response status: ${response.status} (${response.statusText})`, response|| error); + console.error(`[${this.providerName} - refreshAccessToken] Token refresh failed`, error || response); throw response || error; } diff --git a/server/services/oauth/SessionStateService.js b/server/services/oauth/SessionStateService.js index 68cb391..7abfb8e 100644 --- a/server/services/oauth/SessionStateService.js +++ b/server/services/oauth/SessionStateService.js @@ -1,59 +1,78 @@ const crypto = require('node:crypto'); +// --- Injectable state store abstraction --- +class StateStore { + async set(key, value) { this._map.set(key, value); } + async get(key) { return this._map.get(key); } + async delete(key) { this._map.delete(key); } + async entries() { return Array.from(this._map.entries()); } + constructor() { this._map = new Map(); } +} +// TODO: Implement a Redis/Memcached version for distributed scaling support. + +// --- Main SessionStateService --- class SessionStateService { - constructor() { - this.states = new Map(); + /** + * @param {object} options + * - store: implements set/get/delete/entries (default: in-memory map), swap with Redis/Memcached for horizontal scaling + * - cleanupIntervalMs: how frequently to clean up expired states (default: 60s) + */ + constructor({ store, cleanupIntervalMs = 60000 } = {}) { + /** + * Use an injectable store. Use in-memory only for single-instance/dev. For cluster/horizontal scale, use a Redis/Memcached implementation! + * @type {StateStore} + */ + this.states = store || new StateStore(); + // Clean up expired states periodically + this._cleanupTimer = setInterval(() => this.cleanup(), cleanupIntervalMs); } // Generate a random state token for CSRF protection - generateState() { + async generateState() { const state = crypto.randomBytes(32).toString('hex'); - this.states.set(state, { - createdAt: Date.now(), - used: false - }); - - // Clean up old states (older than 10 minutes) - this.cleanup(); - + await this.states.set(state, { createdAt: Date.now(), used: false }); + // No per-generation cleanup, timer handles it return state; } // Verify and consume a state token - verifyState(state) { - const stateData = this.states.get(state); - - if (!stateData) { - return false; - } - - if (stateData.used) { - return false; - } - - // Check if state is older than 10 minutes + async verifyState(state) { + // Proactive cleanup: remove expired states before checking + await this.cleanup(); + const stateData = await this.states.get(state); + if (!stateData) return false; + if (stateData.used) return false; const tenMinutes = 10 * 60 * 1000; if (Date.now() - stateData.createdAt > tenMinutes) { - this.states.delete(state); + await this.states.delete(state); return false; } - // Mark as used and delete - this.states.delete(state); + await this.states.delete(state); return true; } // Clean up old states - cleanup() { + async cleanup() { const tenMinutes = 10 * 60 * 1000; const now = Date.now(); - - for (const [state, data] of this.states.entries()) { + const entries = await this.states.entries(); + for (const [state, data] of entries) { if (now - data.createdAt > tenMinutes) { - this.states.delete(state); + await this.states.delete(state); } } } + + // Call this to stop the cleanup timer, e.g. on app shutdown + stopCleanupTimer() { + clearInterval(this._cleanupTimer); + } } -module.exports = new SessionStateService(); \ No newline at end of file +/** + * WARNING: The default in-memory state store works ONLY for single-instance applications. + * Deployments with horizontal scaling (multiple app/server instances) must inject a Redis/Memcached store + * implementing the async set/get/delete/entries contract. + */ +module.exports = SessionStateService; \ No newline at end of file diff --git a/server/services/oauth/providers/github.js b/server/services/oauth/providers/github.js index 8c78961..616944b 100644 --- a/server/services/oauth/providers/github.js +++ b/server/services/oauth/providers/github.js @@ -1,5 +1,4 @@ const OAuthService = require('../OAuthService'); -const { response } = require('express'); class githubOAuthProvider extends OAuthService { constructor() { @@ -24,9 +23,13 @@ class githubOAuthProvider extends OAuthService { } }); - const profile = await userProfileResponse.json(); + if (!userProfileResponse.ok) { + const errorData = await userProfileResponse.json().catch(() => ({})); + // ignore warning, Rethrown after catching.... + throw new Error(`Failed to fetch user profile, err message: ${errorData.message || 'Unknown error'}`); + } - // GitHub may not return email in profile, fetch separately + const profile = await userProfileResponse.json(); // GitHub may not return email in profile, fetch separately let email = profile.email; if (!email) { const emailResponse = await fetch('https://api.github.com/user/emails', { @@ -38,18 +41,18 @@ class githubOAuthProvider extends OAuthService { if(!emailResponse.ok) { // ignore warning, Rethrown after catching.... - throw Error(`Failed to fetch User Email (response status: ${emailResponse.status}, response msg: ${response.json()?.message})`) + throw Error(`Failed to fetch User Email (response status: ${emailResponse.status}, response msg: ${emailResponse.json()?.message})`) } emailResponse.data = await emailResponse.json(); // Find primary verified email - const primaryEmail = emailResponse.data.find(e => e.primary && e.verified); + const primaryEmail = emailResponse?.data?.find(e => e.primary && e.verified); email = primaryEmail ? primaryEmail.email : emailResponse.data[0]?.email; if(!email) { // ignore warning, Rethrown after catching.... - throw Error(`${emailResponse.data?.error_description} (see: ${emailResponse.data?.error_uri})`); + throw Error(`${emailResponse.data?.error_description || "No verified email address found for GitHub user"} ${emailResponse?.data?.error_uri ? `(See: ${emailResponse.data.error_uri})` : ""}`); } } From 314e8af0952b91d6ad1ff0ab1a298534838918ce Mon Sep 17 00:00:00 2001 From: UnschooledGamer <76094069+UnschooledGamer@users.noreply.github.com> Date: Fri, 28 Nov 2025 19:29:01 +0530 Subject: [PATCH 06/13] feat: Implement token encryption and decryption for secure storage, enhance authentication flow with updated database triggers, and add new dependencies for improved functionality --- package-lock.json | 39 +++++++++++++++++--- package.json | 4 +- server/entities/authenticationProvider.js | 9 ++++- server/lib/authenticateWithProvider.js | 35 +++++++++++------- server/lib/tokenCrypto.js | 35 ++++++++++++++++++ server/services/oauth/SessionStateService.js | 8 +++- 6 files changed, 107 insertions(+), 23 deletions(-) create mode 100644 server/lib/tokenCrypto.js diff --git a/package-lock.json b/package-lock.json index 063fc78..860b63c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,8 +9,10 @@ "version": "1.0.1", "license": "MIT", "dependencies": { + "@better-fetch/fetch": "^1.1.18", "@google-cloud/storage": "^7.16.0", "@googleapis/androidpublisher": "^29.3.0", + "@noble/ciphers": "^2.0.1", "autosize": "^6.0.1", "cookie-parser": "^1.4.7", "core-js": "^3.45.0", @@ -30,7 +32,8 @@ "marked": "^16.1.2", "moment": "^2.30.1", "nodemailer": "^7.0.7", - "sqlite3": "^5.1.7" + "sqlite3": "^5.1.7", + "zod": "^4.1.13" }, "devDependencies": { "@babel/core": "^7.28.0", @@ -1611,6 +1614,11 @@ "node": ">=6.9.0" } }, + "node_modules/@better-fetch/fetch": { + "version": "1.1.18", + "resolved": "https://registry.npmjs.org/@better-fetch/fetch/-/fetch-1.1.18.tgz", + "integrity": "sha512-rEFOE1MYIsBmoMJtQbl32PGHHXuG2hDxvEd7rUHE0vCBoFQVSDqaVs9hkZEtHCxRoY+CljXKFCOuJ8uxqw1LcA==" + }, "node_modules/@biomejs/biome": { "version": "2.1.4", "resolved": "https://registry.npmjs.org/@biomejs/biome/-/biome-2.1.4.tgz", @@ -1910,6 +1918,18 @@ "@jridgewell/sourcemap-codec": "^1.4.14" } }, + "node_modules/@noble/ciphers": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/@noble/ciphers/-/ciphers-2.0.1.tgz", + "integrity": "sha512-xHK3XHPUW8DTAobU+G0XT+/w+JLM7/8k1UFdB5xg/zTFPnFCobhftzw8wl4Lw2aq/Rvir5pxfZV5fEazmeCJ2g==", + "license": "MIT", + "engines": { + "node": ">= 20.19.0" + }, + "funding": { + "url": "https://paulmillr.com/funding/" + } + }, "node_modules/@npmcli/fs": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/@npmcli/fs/-/fs-1.1.1.tgz", @@ -3505,6 +3525,16 @@ "devtools-protocol": "*" } }, + "node_modules/chromium-bidi/node_modules/zod": { + "version": "3.25.76", + "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", + "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", + "dev": true, + "license": "MIT", + "funding": { + "url": "https://github.com/sponsors/colinhacks" + } + }, "node_modules/clean-stack": { "version": "2.2.0", "resolved": "https://registry.npmjs.org/clean-stack/-/clean-stack-2.2.0.tgz", @@ -9847,10 +9877,9 @@ } }, "node_modules/zod": { - "version": "3.25.76", - "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", - "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", - "dev": true, + "version": "4.1.13", + "resolved": "https://registry.npmjs.org/zod/-/zod-4.1.13.tgz", + "integrity": "sha512-AvvthqfqrAhNH9dnfmrfKzX5upOdjUVJYFqNSlkmGf64gRaTzlPwz99IHYnVs28qYAybvAlBV+H7pn0saFY4Ig==", "license": "MIT", "funding": { "url": "https://github.com/sponsors/colinhacks" diff --git a/package.json b/package.json index 7a65178..5a58d0d 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,7 @@ "@better-fetch/fetch": "^1.1.18", "@google-cloud/storage": "^7.16.0", "@googleapis/androidpublisher": "^29.3.0", + "@noble/ciphers": "^2.0.1", "autosize": "^6.0.1", "cookie-parser": "^1.4.7", "core-js": "^3.45.0", @@ -57,7 +58,8 @@ "marked": "^16.1.2", "moment": "^2.30.1", "nodemailer": "^7.0.7", - "sqlite3": "^5.1.7" + "sqlite3": "^5.1.7", + "zod": "^4.1.13" }, "scripts": { "prepare": "husky", diff --git a/server/entities/authenticationProvider.js b/server/entities/authenticationProvider.js index a2f8514..b6cbb89 100644 --- a/server/entities/authenticationProvider.js +++ b/server/entities/authenticationProvider.js @@ -14,7 +14,14 @@ const table = `create table if not exists authentication_provider ( updated_at timestamp default current_timestamp, foreign key (user_id) references user(id), unique(provider, provider_user_id) -);`; +); +create trigger if not exists update_authentication_provider_timestamp + after update on authentication_provider + for each row +begin + update authentication_provider set updated_at = current_timestamp where id = NEW.id; +end; +`; class AuthenticationProvider extends Entity { ID = 'id'; diff --git a/server/lib/authenticateWithProvider.js b/server/lib/authenticateWithProvider.js index 5d35f5e..3955839 100644 --- a/server/lib/authenticateWithProvider.js +++ b/server/lib/authenticateWithProvider.js @@ -2,6 +2,9 @@ const moment = require('moment'); const authenticationProvider = require('../entities/authenticationProvider'); const login = require('../entities/login'); const User = require('../entities/user'); +// Tokens are encrypted while their stored, and SHOULD BE decrypted +// When it's being used as values (i.e API with the tokens retrieved from the DB). +const { decryptToken, encryptToken } = require('../lib/tokenCrypto') async function authenticateWithProvider(providerType, profile, tokens) { const { id: providerUserId, email, name, username } = profile; @@ -31,12 +34,13 @@ async function authenticateWithProvider(providerType, profile, tokens) { await authenticationProvider.update( [ [authenticationProvider.ID, existingLink[0].id], - [authenticationProvider.ACCESS_TOKEN, accessToken], - [authenticationProvider.REFRESH_TOKEN, refreshToken], + [authenticationProvider.ACCESS_TOKEN, encryptToken(accessToken)], + [authenticationProvider.REFRESH_TOKEN, encryptToken(refreshToken)], [authenticationProvider.ACCESS_TOKEN_EXPIRES_AT, tokenExpiresAt], - [authenticationProvider.SCOPE, tokens.scope] + [authenticationProvider.SCOPE, tokens.scope || null] ] - ) } else { + ) + } else { // Atomic, race-safe upsert pattern for user await User.insertOrIgnore( [User.NAME, name], @@ -48,17 +52,20 @@ async function authenticateWithProvider(providerType, profile, tokens) { // Always fetch the user after insert - ensures you get the correct id whether existing or new const userRes = await User.get([User.EMAIL, email]); userId = userRes[0].id; + if (!userRes || userRes.length === 0) { + throw new Error(`Failed to retrieve user with email: ${email}`); + } - await authenticationProvider.insert( - [authenticationProvider.USER_ID, userId], - [authenticationProvider.PROVIDER, providerType], - [authenticationProvider.PROVIDER_USER_ID, providerUserId], - [authenticationProvider.ACCESS_TOKEN, accessToken], - [authenticationProvider.REFRESH_TOKEN, refreshToken], - [authenticationProvider.ACCESS_TOKEN_EXPIRES_AT, tokenExpiresAt], - [authenticationProvider.REFRESH_TOKEN_EXPIRES_AT, null], - [authenticationProvider.SCOPE, tokens.scope] - ); + await authenticationProvider.insert( + [authenticationProvider.USER_ID, userId], + [authenticationProvider.PROVIDER, providerType], + [authenticationProvider.PROVIDER_USER_ID, providerUserId], + [authenticationProvider.ACCESS_TOKEN, encryptToken(accessToken)], + [authenticationProvider.REFRESH_TOKEN, encryptToken(refreshToken)], + [authenticationProvider.ACCESS_TOKEN_EXPIRES_AT, tokenExpiresAt], + [authenticationProvider.REFRESH_TOKEN_EXPIRES_AT, null], + [authenticationProvider.SCOPE, tokens.scope] + ); } // break this into a Function, if it gets too repetitive throughout the whole codebase. diff --git a/server/lib/tokenCrypto.js b/server/lib/tokenCrypto.js new file mode 100644 index 0000000..e7efcd5 --- /dev/null +++ b/server/lib/tokenCrypto.js @@ -0,0 +1,35 @@ +import { xchacha20poly1305 } from '@noble/ciphers/chacha.js'; +import { bytesToHex, hexToBytes, utf8ToBytes, managedNonce } from '@noble/ciphers/utils.js'; + +const KEY_HEX = process.env.TOKEN_ENCRYPTION_KEY; +if (!KEY_HEX || hexToBytes(KEY_HEX).length !== 32) { + throw new Error('TOKEN_ENCRYPTION_KEY must be set to 64 hex chars (32 bytes)'); +} +const KEY = hexToBytes(KEY_HEX); + +export function encryptToken(plaintext) { + try { + const nonce = managedNonce(); // 24 bytes + const msgBytes = utf8ToBytes(plaintext); + const cipher = xchacha20poly1305(KEY, nonce); + const ciphertext = cipher.encrypt(msgBytes); + return { + ciphertext: bytesToHex(ciphertext), + nonce: bytesToHex(nonce) + }; + } catch (e) { + throw new Error('Token encryption failed'); + } +} + +export function decryptToken({ ciphertext, nonce }) { + try { + const ct = hexToBytes(ciphertext); + const n = hexToBytes(nonce); + const cipher = xchacha20poly1305(KEY, n); + const plaintext = cipher.decrypt(ct); + return new TextDecoder().decode(plaintext); + } catch (e) { + throw new Error('Token decryption failed'); + } +} diff --git a/server/services/oauth/SessionStateService.js b/server/services/oauth/SessionStateService.js index 7abfb8e..78b9966 100644 --- a/server/services/oauth/SessionStateService.js +++ b/server/services/oauth/SessionStateService.js @@ -25,6 +25,7 @@ class SessionStateService { this.states = store || new StateStore(); // Clean up expired states periodically this._cleanupTimer = setInterval(() => this.cleanup(), cleanupIntervalMs); + this._cleanupTimer.unref(); // Don't prevent process exit } // Generate a random state token for CSRF protection @@ -38,16 +39,19 @@ class SessionStateService { // Verify and consume a state token async verifyState(state) { // Proactive cleanup: remove expired states before checking - await this.cleanup(); + // await this.cleanup(); const stateData = await this.states.get(state); if (!stateData) return false; if (stateData.used) return false; + // Mark as used immediately to prevent concurrent reuse + stateData.used = true; + await this.states.set(state, stateData); const tenMinutes = 10 * 60 * 1000; if (Date.now() - stateData.createdAt > tenMinutes) { await this.states.delete(state); return false; } - // Mark as used and delete + // delete after successful Verification. await this.states.delete(state); return true; } From 18b75d28632206b80fe24d5628798d70935a1249 Mon Sep 17 00:00:00 2001 From: UnschooledGamer <76094069+UnschooledGamer@users.noreply.github.com> Date: Fri, 28 Nov 2025 19:43:19 +0530 Subject: [PATCH 07/13] feat: Enhance authentication provider with updated timestamp trigger, improve error handling in token retrieval, and add getAndDelete method for session state management --- server/entities/authenticationProvider.js | 4 ++-- server/lib/authenticateWithProvider.js | 10 +++++----- server/services/oauth/SessionStateService.js | 17 ++++++++--------- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/server/entities/authenticationProvider.js b/server/entities/authenticationProvider.js index b6cbb89..77afd25 100644 --- a/server/entities/authenticationProvider.js +++ b/server/entities/authenticationProvider.js @@ -18,10 +18,10 @@ const table = `create table if not exists authentication_provider ( create trigger if not exists update_authentication_provider_timestamp after update on authentication_provider for each row + when OLD.updated_at >= NEW.updated_at begin update authentication_provider set updated_at = current_timestamp where id = NEW.id; -end; -`; +end;`; class AuthenticationProvider extends Entity { ID = 'id'; diff --git a/server/lib/authenticateWithProvider.js b/server/lib/authenticateWithProvider.js index 3955839..5b06847 100644 --- a/server/lib/authenticateWithProvider.js +++ b/server/lib/authenticateWithProvider.js @@ -2,9 +2,9 @@ const moment = require('moment'); const authenticationProvider = require('../entities/authenticationProvider'); const login = require('../entities/login'); const User = require('../entities/user'); -// Tokens are encrypted while their stored, and SHOULD BE decrypted +// Tokens are encrypted while they're stored, and SHOULD BE decrypted // When it's being used as values (i.e API with the tokens retrieved from the DB). -const { decryptToken, encryptToken } = require('../lib/tokenCrypto') +const { encryptToken } = require('../lib/tokenCrypto') async function authenticateWithProvider(providerType, profile, tokens) { const { id: providerUserId, email, name, username } = profile; @@ -51,10 +51,10 @@ async function authenticateWithProvider(providerType, profile, tokens) { ); // Always fetch the user after insert - ensures you get the correct id whether existing or new const userRes = await User.get([User.EMAIL, email]); - userId = userRes[0].id; if (!userRes || userRes.length === 0) { - throw new Error(`Failed to retrieve user with email: ${email}`); + throw new Error(`Failed to retrieve user`); } + userId = userRes[0].id; await authenticationProvider.insert( [authenticationProvider.USER_ID, userId], @@ -64,7 +64,7 @@ async function authenticateWithProvider(providerType, profile, tokens) { [authenticationProvider.REFRESH_TOKEN, encryptToken(refreshToken)], [authenticationProvider.ACCESS_TOKEN_EXPIRES_AT, tokenExpiresAt], [authenticationProvider.REFRESH_TOKEN_EXPIRES_AT, null], - [authenticationProvider.SCOPE, tokens.scope] + [authenticationProvider.SCOPE, tokens.scope || null] ); } diff --git a/server/services/oauth/SessionStateService.js b/server/services/oauth/SessionStateService.js index 78b9966..bf99348 100644 --- a/server/services/oauth/SessionStateService.js +++ b/server/services/oauth/SessionStateService.js @@ -6,6 +6,11 @@ class StateStore { async get(key) { return this._map.get(key); } async delete(key) { this._map.delete(key); } async entries() { return Array.from(this._map.entries()); } + async getAndDelete(key) { + const value = this._map.get(key); + this._map.delete(key); + return value; + } constructor() { this._map = new Map(); } } // TODO: Implement a Redis/Memcached version for distributed scaling support. @@ -31,7 +36,7 @@ class SessionStateService { // Generate a random state token for CSRF protection async generateState() { const state = crypto.randomBytes(32).toString('hex'); - await this.states.set(state, { createdAt: Date.now(), used: false }); + await this.states.set(state, { createdAt: Date.now() }); // Remove 'used' // No per-generation cleanup, timer handles it return state; } @@ -40,19 +45,13 @@ class SessionStateService { async verifyState(state) { // Proactive cleanup: remove expired states before checking // await this.cleanup(); - const stateData = await this.states.get(state); + const stateData = await this.states.getAndDelete(state); if (!stateData) return false; - if (stateData.used) return false; - // Mark as used immediately to prevent concurrent reuse - stateData.used = true; - await this.states.set(state, stateData); const tenMinutes = 10 * 60 * 1000; if (Date.now() - stateData.createdAt > tenMinutes) { - await this.states.delete(state); + // Already deleted: nothing left to clean return false; } - // delete after successful Verification. - await this.states.delete(state); return true; } From 7b64d7fe963650c1bfc10f243fb419e4e521098d Mon Sep 17 00:00:00 2001 From: UnschooledGamer <76094069+UnschooledGamer@users.noreply.github.com> Date: Fri, 5 Dec 2025 19:51:20 +0530 Subject: [PATCH 08/13] feat: Refactor OAuth flow to support code challenge and verifier, enhance error handling, and improve session state management --- server/apis/oauth.js | 80 +++++++++++++------- server/lib/authenticateWithProvider.js | 8 +- server/lib/tokenCrypto.js | 28 ++++--- server/services/oauth/OAuthService.js | 52 +++++++++++-- server/services/oauth/SessionStateService.js | 24 +++++- 5 files changed, 138 insertions(+), 54 deletions(-) diff --git a/server/apis/oauth.js b/server/apis/oauth.js index 1a857d0..5fd6485 100644 --- a/server/apis/oauth.js +++ b/server/apis/oauth.js @@ -8,68 +8,89 @@ const SessionStateServiceClass = require('../services/oauth/SessionStateService' const sessionStateService = new SessionStateServiceClass(); const authenticateWithProvider = require('../lib/authenticateWithProvider'); -const SUCCESS_REDIRECT_PATH = `/login`; - -const router = Router(); -router.get('/:provider', async (req, res) => { +const ERROR_REDIRECT_PATH = `/login`; +const SUCCESS_CALLBACK_URL = `/user` + +/** + * @template REQ + * @template RES + * @param {REQ} req + * @param {RES} res + * @returns + */ +async function handleOAuthSignIn (req, res) { const { provider } = req.params; + const { callbackUrl } = (req?.query || req.body) || {}; + try { if (!provider) { return res.status(400).send('No provider provided'); } const oAuthProvider = OAuthProviderFactory.getProvider(provider); - - const state = await sessionStateService.generateState(); - + const state = await sessionStateService.generateState({ callbackUrl: `${callbackUrl ?? SUCCESS_CALLBACK_URL}` }); res.cookie('oauthProvider', provider, { secure: true, httpOnly: true, sameSite: 'lax' }); - res.cookie('oauthState', state, { secure: true, signed: true, httpOnly: true, maxAge: 10 * 60 * 1000 }); - - const authURL = oAuthProvider.getAuthorizationUrl(state) + res.cookie('oauthState', state.state, { secure: true, signed: true, httpOnly: true, maxAge: 10 * 60 * 1000 }); + const authURL = await oAuthProvider.getAuthorizationUrl({ state: state.state, codeVerifier: state.codeVerifier }) + // console.log(authURL) res.redirect(authURL); } catch (e) { console.error(`[OAuth Router] - OAuth initiation (route: ${req.path}) error:`, e); res.status(400).json({ error: e.message }); } -}) - -router.get('/:provider/callback', async (req, res) => { +} +async function handleOAuthCallback(req, res) { const { provider } = req.params; - const { code, state, error, error_description } = req.query; + const { code, state, error, error_description } = (req?.query || req.body) || {}; try { if (!provider) { return res.status(400).send('No provider provided'); } + if(!state) { + console.error(`[OAuth Router] - Provider (${provider}) responded without a state: ${error}`); + throw res.redirect(`${ERROR_REDIRECT_PATH}?error=missing_state`); + } + if(error) { console.error(`[OAuth Router] - Provider (${provider}) responded with an error: ${error}`); - res.redirect(`${SUCCESS_REDIRECT_PATH}?error=${error}&error_description=${error_description}`); + res.redirect(`${ERROR_REDIRECT_PATH}?error=${error}&error_description=${error_description}`); return; } if(!code) { console.error(`[OAuth Router] - Provider (${provider}) responded without a code: ${code}`); - res.redirect(`${SUCCESS_REDIRECT_PATH}?error=missing_code`); + res.redirect(`${ERROR_REDIRECT_PATH}?error=missing_code`); return; } const storedState = req.signedCookies.oauthState; const storedProvider = req.cookies?.oauthProvider; - if(!storedState || !(await sessionStateService.verifyState(state))) { + + const verifiedState = await sessionStateService.verifyState(state) + + if(!storedState || !verifiedState) { res.clearCookie('oauthState'); // res.status(422).send("Invalid State") - res.redirect(`${SUCCESS_REDIRECT_PATH}?error=invalid_state`); + res.redirect(`${ERROR_REDIRECT_PATH}?error=state_mismatch`); + return; + } + + if(storedState !== state) { + console.log(`State mismatch between cookie & Callback State`, { storedState, state}) + res.clearCookie('oauthState'); + res.redirect(`${ERROR_REDIRECT_PATH}?error=state_mismatch`); return; } if(storedProvider !== provider) { console.error(`[OAuth Router] - Provider (${provider}) mismatch: ${storedProvider} !== ${provider}`); // res.status(422).send("OAuth Provider mismatch"); - res.redirect(`${SUCCESS_REDIRECT_PATH}?error=oauth_provider_mismatch`); + res.redirect(`${ERROR_REDIRECT_PATH}?error=oauth_provider_mismatch`); return; } @@ -78,10 +99,11 @@ router.get('/:provider/callback', async (req, res) => { const OAuthProvider = OAuthProviderFactory.getProvider(provider); - const tokens = await OAuthProvider.getAccessToken(code).catch(() => {}); + const tokens = await OAuthProvider.getAccessToken({ code, codeVerifier: verifiedState.codeVerifier }).catch((e) => ({ error: e})); if(!tokens?.accessToken || !tokens) { - res.status(401).send("Failed to retrieve access token"); + console.log(tokens.error); + res.status(401).send(tokens?.error || "Failed to retrieve access token"); return; } @@ -109,15 +131,21 @@ router.get('/:provider/callback', async (req, res) => { maxAge: 7 * 24 * 60 * 60 * 1000 // 1 week }); - /* - NOTE: Replace the Hardcoded URL with the actual URL given by the frontend. - /* */ - return res.redirect('/user'); + return res.redirect(`${verifiedState.callbackUrl ?? SUCCESS_CALLBACK_URL}`); } catch (e) { console.error(`[OAuth Router] - OAuth callback (route: ${req.path}) error:`, e); return res.sendStatus(500) } -}) +} + +const router = Router(); +router.get('/:provider', handleOAuthSignIn) + +router.post('/:provider', handleOAuthSignIn) + +router.get('/:provider/callback', handleOAuthCallback) + +router.post('/:provider/callback', handleOAuthCallback) module.exports = router; \ No newline at end of file diff --git a/server/lib/authenticateWithProvider.js b/server/lib/authenticateWithProvider.js index 5b06847..4e6c18e 100644 --- a/server/lib/authenticateWithProvider.js +++ b/server/lib/authenticateWithProvider.js @@ -34,8 +34,8 @@ async function authenticateWithProvider(providerType, profile, tokens) { await authenticationProvider.update( [ [authenticationProvider.ID, existingLink[0].id], - [authenticationProvider.ACCESS_TOKEN, encryptToken(accessToken)], - [authenticationProvider.REFRESH_TOKEN, encryptToken(refreshToken)], + [authenticationProvider.ACCESS_TOKEN, await encryptToken(accessToken)], + [authenticationProvider.REFRESH_TOKEN, refreshToken ? await encryptToken(refreshToken) : null], [authenticationProvider.ACCESS_TOKEN_EXPIRES_AT, tokenExpiresAt], [authenticationProvider.SCOPE, tokens.scope || null] ] @@ -60,8 +60,8 @@ async function authenticateWithProvider(providerType, profile, tokens) { [authenticationProvider.USER_ID, userId], [authenticationProvider.PROVIDER, providerType], [authenticationProvider.PROVIDER_USER_ID, providerUserId], - [authenticationProvider.ACCESS_TOKEN, encryptToken(accessToken)], - [authenticationProvider.REFRESH_TOKEN, encryptToken(refreshToken)], + [authenticationProvider.ACCESS_TOKEN, await encryptToken(accessToken)], + [authenticationProvider.REFRESH_TOKEN, refreshToken ? await encryptToken(refreshToken) : null], [authenticationProvider.ACCESS_TOKEN_EXPIRES_AT, tokenExpiresAt], [authenticationProvider.REFRESH_TOKEN_EXPIRES_AT, null], [authenticationProvider.SCOPE, tokens.scope || null] diff --git a/server/lib/tokenCrypto.js b/server/lib/tokenCrypto.js index e7efcd5..19d742d 100644 --- a/server/lib/tokenCrypto.js +++ b/server/lib/tokenCrypto.js @@ -1,35 +1,39 @@ import { xchacha20poly1305 } from '@noble/ciphers/chacha.js'; import { bytesToHex, hexToBytes, utf8ToBytes, managedNonce } from '@noble/ciphers/utils.js'; - +import { subtle } from 'node:crypto' +import { TextEncoder } from 'node:util'; const KEY_HEX = process.env.TOKEN_ENCRYPTION_KEY; if (!KEY_HEX || hexToBytes(KEY_HEX).length !== 32) { throw new Error('TOKEN_ENCRYPTION_KEY must be set to 64 hex chars (32 bytes)'); } -const KEY = hexToBytes(KEY_HEX); -export function encryptToken(plaintext) { +export async function encryptToken(plaintext) { try { - const nonce = managedNonce(); // 24 bytes + const encoder = new TextEncoder() + const data = encoder.encode(KEY_HEX); + const KEY = await subtle.digest('SHA-256', data); + const cipher = managedNonce(xchacha20poly1305)(new Uint8Array(KEY)); // 24 bytes const msgBytes = utf8ToBytes(plaintext); - const cipher = xchacha20poly1305(KEY, nonce); const ciphertext = cipher.encrypt(msgBytes); - return { - ciphertext: bytesToHex(ciphertext), - nonce: bytesToHex(nonce) - }; + + return bytesToHex(ciphertext) } catch (e) { + console.error('Token encryption failed', e); throw new Error('Token encryption failed'); } } -export function decryptToken({ ciphertext, nonce }) { +export async function decryptToken(ciphertext) { try { + const encoder = new TextEncoder() + const data = encoder.encode(KEY_HEX); + const KEY = await subtle.digest('SHA-256', data); const ct = hexToBytes(ciphertext); - const n = hexToBytes(nonce); - const cipher = xchacha20poly1305(KEY, n); + const cipher = managedNonce(xchacha20poly1305)(new Uint8Array(KEY)); const plaintext = cipher.decrypt(ct); return new TextDecoder().decode(plaintext); } catch (e) { + console.error('Token decryption failed', e); throw new Error('Token decryption failed'); } } diff --git a/server/services/oauth/OAuthService.js b/server/services/oauth/OAuthService.js index 0f3a9d3..3d96fdf 100644 --- a/server/services/oauth/OAuthService.js +++ b/server/services/oauth/OAuthService.js @@ -1,5 +1,5 @@ const { betterFetch } = require('@better-fetch/fetch'); - +const { TextEncoder } = require('node:util'); const { z } = require('zod'); /** @@ -31,6 +31,8 @@ class OAuthService { this.providerName = config.providerName; this.prompt = config.prompt || ""; + this.callbackUrl = "/user"; + // biome-ignore lint/complexity/noForEach: ignore ;["clientId", "clientSecret", "authorizationUrl", "tokenUrl", "userInfoUrl", "scopes", "providerName"].forEach(p=> { if(!this[p]) throw TypeError(`"${p}" is required, but not specified in OAuthService Class Configuration`); @@ -38,23 +40,36 @@ class OAuthService { } // Generate authorization URL - getAuthorizationUrl(state) { + async getAuthorizationUrl({ state, codeVerifier }) { if(!state) throw ReferenceError("state is missing for generating Authorization URL"); - + /** Section 4.2 of RFC 7636 -> https://datatracker.ietf.org/doc/html/rfc7636#section-4.2 + * S256 + * code_challenge = BASE64URL-ENCODE(SHA256(ASCII(code_verifier))) + **/ + const code_challenge = await this.#generateCodeChallenge(codeVerifier); + const params = new URLSearchParams({ response_type: 'code', client_id: this.clientId, state: state, // CSRF protection scope: this.scopes.join(' '), + code_challenge: code_challenge, + code_challenge_method: "S256", }); if(this.redirectUri) params.append('redirect_uri', this.redirectUri); + if(codeVerifier) { + // Section 4.2 of RFC 7636 -> https://datatracker.ietf.org/doc/html/rfc7636#section-4 + params.append('code_challenge', code_challenge); + params.append('code_challenge_method', 'S256'); + } return `${this.authorizationUrl}?${params.toString()}`; } // Exchange authorization code for access token - async getAccessToken(code) { + async getAccessToken({ code, codeVerifier }) { + console.log({ code, codeVerifier }) if(!code) { throw Error(`[${this.providerName} - getAccessToken] Code is required: ${code}`); } @@ -67,6 +82,10 @@ class OAuthService { grant_type: 'authorization_code' } + if(codeVerifier) { + body.code_verifier = codeVerifier; + } + if(this.redirectUri) { body.redirect_uri = this.redirectUri; } @@ -94,16 +113,16 @@ class OAuthService { }) if(!response || response.error || error) { - console.error(`[${this.providerName} - getAccessToken] Response status: ${response.status} (${response.statusText})`, response|| error); + console.error(`[${this.providerName} - getAccessToken] `, response|| error); throw response || error; } console.log(`[${this.providerName} - getAccessToken]`, response); return this.normalizeTokenResponse(response); - } catch (error) { - console.error(`${this.providerName} token exchange error:`, error); - throw new Error(`Failed to exchange code for token with ${this.providerName}`); + } catch (e) { + console.error(`${this.providerName} token exchange error:`, e); + throw (e.error ? e : new Error(`Failed to exchange code for token with ${this.providerName}`)) } } @@ -173,6 +192,23 @@ class OAuthService { throw new Error(`Failed to refresh token with ${this.providerName}`); } } + /** + * + * @param {string} codeVerifier + * @returns {Promise} + */ + async #generateCodeChallenge(codeVerifier) { + if(!codeVerifier) throw ReferenceError("codeVerifier is required for generating Code Challenge"); + /** Section 4.2 of RFC 7636 -> https://datatracker.ietf.org/doc/html/rfc7636#section-4.2 + * S256 + * code_challenge = BASE64URL-ENCODE(SHA256(ASCII(code_verifier))) + **/ + + const textEncoder = new TextEncoder(); + const encodedData = textEncoder.encode(codeVerifier); + const hash = await crypto.subtle.digest('SHA-256', encodedData); + return Buffer.from(new Uint8Array(hash)).toString('base64url'); + } } module.exports = OAuthService; \ No newline at end of file diff --git a/server/services/oauth/SessionStateService.js b/server/services/oauth/SessionStateService.js index bf99348..8c46d31 100644 --- a/server/services/oauth/SessionStateService.js +++ b/server/services/oauth/SessionStateService.js @@ -34,11 +34,21 @@ class SessionStateService { } // Generate a random state token for CSRF protection - async generateState() { + async generateState({ callbackUrl }) { + const codeVerifier = this.#makeToken(128) const state = crypto.randomBytes(32).toString('hex'); - await this.states.set(state, { createdAt: Date.now() }); // Remove 'used' + + // Only stateData should be stored in cookie, IF IT's Encrypted, + // stateData SHOULD NOT BE Stored in the cookie, IF IT's not Encrypted, then it should be stored in the DB. + const stateData = { + createdAt: Date.now(), + callbackUrl: callbackUrl, + codeVerifier: codeVerifier, + }; + + await this.states.set(state, stateData); // Remove 'used' // No per-generation cleanup, timer handles it - return state; + return { state, codeVerifier, stateData }; } // Verify and consume a state token @@ -52,7 +62,7 @@ class SessionStateService { // Already deleted: nothing left to clean return false; } - return true; + return stateData; } // Clean up old states @@ -71,6 +81,12 @@ class SessionStateService { stopCleanupTimer() { clearInterval(this._cleanupTimer); } + + #makeToken(length) { + return crypto.randomBytes(Math.ceil((length * 3) / 4)) + .toString("base64url") + .slice(0, length); + } } /** From 269e227e6d922713c9f6ab1b6aaa59a8248eb5bb Mon Sep 17 00:00:00 2001 From: UnschooledGamer <76094069+UnschooledGamer@users.noreply.github.com> Date: Fri, 5 Dec 2025 20:27:27 +0530 Subject: [PATCH 09/13] feat: Add callback URL validation and introduce ALLOWED_CALLBACK_HOSTS for enhanced security in OAuth flow --- constants.js | 6 ++++++ server/apis/oauth.js | 20 +++++++++++++++----- server/lib/authenticateWithProvider.js | 2 +- server/lib/tokenCrypto.js | 2 +- server/services/oauth/OAuthService.js | 3 --- 5 files changed, 23 insertions(+), 10 deletions(-) create mode 100644 constants.js diff --git a/constants.js b/constants.js new file mode 100644 index 0000000..c08669b --- /dev/null +++ b/constants.js @@ -0,0 +1,6 @@ +module.exports = { + ALLOWED_CALLBACK_HOSTS_ARRAY: [ + "https://acode.app/", + "http://localhost:5500" + ] +} \ No newline at end of file diff --git a/server/apis/oauth.js b/server/apis/oauth.js index 5fd6485..acd5c15 100644 --- a/server/apis/oauth.js +++ b/server/apis/oauth.js @@ -11,6 +11,19 @@ const authenticateWithProvider = require('../lib/authenticateWithProvider'); const ERROR_REDIRECT_PATH = `/login`; const SUCCESS_CALLBACK_URL = `/user` +function isValidCallbackUrl(url) { + if (!url) return false; + // Allow relative paths + if (url.startsWith('/') && !url.startsWith('//')) return true; + // Or validate against allowlist + try { + const parsed = new URL(url); + return ALLOWED_CALLBACK_HOSTS.includes(parsed.host); + } catch { + return false; + } +} + /** * @template REQ * @template RES @@ -53,14 +66,11 @@ async function handleOAuthCallback(req, res) { if(!state) { console.error(`[OAuth Router] - Provider (${provider}) responded without a state: ${error}`); - throw res.redirect(`${ERROR_REDIRECT_PATH}?error=missing_state`); + return res.redirect(`${ERROR_REDIRECT_PATH}?error=missing_state`); } - if(error) { console.error(`[OAuth Router] - Provider (${provider}) responded with an error: ${error}`); - res.redirect(`${ERROR_REDIRECT_PATH}?error=${error}&error_description=${error_description}`); - return; - } + res.redirect(`${ERROR_REDIRECT_PATH}?error=${encodeURIComponent(error)}&error_description=${encodeURIComponent(error_description || '')}`); } if(!code) { console.error(`[OAuth Router] - Provider (${provider}) responded without a code: ${code}`); diff --git a/server/lib/authenticateWithProvider.js b/server/lib/authenticateWithProvider.js index 4e6c18e..a95d52c 100644 --- a/server/lib/authenticateWithProvider.js +++ b/server/lib/authenticateWithProvider.js @@ -4,7 +4,7 @@ const login = require('../entities/login'); const User = require('../entities/user'); // Tokens are encrypted while they're stored, and SHOULD BE decrypted // When it's being used as values (i.e API with the tokens retrieved from the DB). -const { encryptToken } = require('../lib/tokenCrypto') +const { encryptToken } = require('../lib/tokenCrypto'); async function authenticateWithProvider(providerType, profile, tokens) { const { id: providerUserId, email, name, username } = profile; diff --git a/server/lib/tokenCrypto.js b/server/lib/tokenCrypto.js index 19d742d..0853a61 100644 --- a/server/lib/tokenCrypto.js +++ b/server/lib/tokenCrypto.js @@ -1,6 +1,6 @@ import { xchacha20poly1305 } from '@noble/ciphers/chacha.js'; import { bytesToHex, hexToBytes, utf8ToBytes, managedNonce } from '@noble/ciphers/utils.js'; -import { subtle } from 'node:crypto' +import { subtle } from 'node:crypto'; import { TextEncoder } from 'node:util'; const KEY_HEX = process.env.TOKEN_ENCRYPTION_KEY; if (!KEY_HEX || hexToBytes(KEY_HEX).length !== 32) { diff --git a/server/services/oauth/OAuthService.js b/server/services/oauth/OAuthService.js index 3d96fdf..e584914 100644 --- a/server/services/oauth/OAuthService.js +++ b/server/services/oauth/OAuthService.js @@ -53,8 +53,6 @@ class OAuthService { client_id: this.clientId, state: state, // CSRF protection scope: this.scopes.join(' '), - code_challenge: code_challenge, - code_challenge_method: "S256", }); if(this.redirectUri) params.append('redirect_uri', this.redirectUri); @@ -69,7 +67,6 @@ class OAuthService { // Exchange authorization code for access token async getAccessToken({ code, codeVerifier }) { - console.log({ code, codeVerifier }) if(!code) { throw Error(`[${this.providerName} - getAccessToken] Code is required: ${code}`); } From b03048746c2539ad6406918f16bceba5d9d70852 Mon Sep 17 00:00:00 2001 From: UnschooledGamer <76094069+UnschooledGamer@users.noreply.github.com> Date: Fri, 5 Dec 2025 20:37:18 +0530 Subject: [PATCH 10/13] fix: Correct callback URL handling, and improve error messages in GitHub OAuth provider --- constants.js | 2 +- server/apis/oauth.js | 18 ++++++++++++------ server/services/oauth/providers/github.js | 2 +- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/constants.js b/constants.js index c08669b..38a8399 100644 --- a/constants.js +++ b/constants.js @@ -1,6 +1,6 @@ module.exports = { ALLOWED_CALLBACK_HOSTS_ARRAY: [ "https://acode.app/", - "http://localhost:5500" + "http://localhost:5500/" ] } \ No newline at end of file diff --git a/server/apis/oauth.js b/server/apis/oauth.js index acd5c15..2d0821d 100644 --- a/server/apis/oauth.js +++ b/server/apis/oauth.js @@ -1,9 +1,8 @@ // TODO: Lookout for vulnerabilities. -// TODO: support for Code challeges -// TODO: Custom State support for remembering redirects. const { Router } = require('express'); const OAuthProviderFactory = require('../services/oauth/OAuthProviderFactory'); const SessionStateServiceClass = require('../services/oauth/SessionStateService'); +const { ALLOWED_CALLBACK_HOSTS_ARRAY } = require('../../constants') // For single-instance use only. For clustering/horizontal scaling, inject a distributed store (e.g. Redis) into SessionStateService. const sessionStateService = new SessionStateServiceClass(); const authenticateWithProvider = require('../lib/authenticateWithProvider'); @@ -18,7 +17,7 @@ function isValidCallbackUrl(url) { // Or validate against allowlist try { const parsed = new URL(url); - return ALLOWED_CALLBACK_HOSTS.includes(parsed.host); + return ALLOWED_CALLBACK_HOSTS_ARRAY.includes(parsed.host); } catch { return false; } @@ -41,8 +40,13 @@ async function handleOAuthSignIn (req, res) { return res.status(400).send('No provider provided'); } + // Validate callback URL to prevent open redirect + if (callbackUrl && !isValidCallbackUrl(callbackUrl)) { + return res.status(400).json({ error: 'Invalid callback URL' }); + } + const oAuthProvider = OAuthProviderFactory.getProvider(provider); - const state = await sessionStateService.generateState({ callbackUrl: `${callbackUrl ?? SUCCESS_CALLBACK_URL}` }); + const state = await sessionStateService.generateState({ callbackUrl: `${isValidCallbackUrl(callbackUrl) ? callbackUrl : SUCCESS_CALLBACK_URL}` }); res.cookie('oauthProvider', provider, { secure: true, httpOnly: true, sameSite: 'lax' }); res.cookie('oauthState', state.state, { secure: true, signed: true, httpOnly: true, maxAge: 10 * 60 * 1000 }); @@ -68,9 +72,11 @@ async function handleOAuthCallback(req, res) { console.error(`[OAuth Router] - Provider (${provider}) responded without a state: ${error}`); return res.redirect(`${ERROR_REDIRECT_PATH}?error=missing_state`); } + if(error) { console.error(`[OAuth Router] - Provider (${provider}) responded with an error: ${error}`); - res.redirect(`${ERROR_REDIRECT_PATH}?error=${encodeURIComponent(error)}&error_description=${encodeURIComponent(error_description || '')}`); } + return res.redirect(`${ERROR_REDIRECT_PATH}?error=${encodeURIComponent(error)}&error_description=${encodeURIComponent(error_description || '')}`); + } if(!code) { console.error(`[OAuth Router] - Provider (${provider}) responded without a code: ${code}`); @@ -141,7 +147,7 @@ async function handleOAuthCallback(req, res) { maxAge: 7 * 24 * 60 * 60 * 1000 // 1 week }); - return res.redirect(`${verifiedState.callbackUrl ?? SUCCESS_CALLBACK_URL}`); + return res.redirect(`${isValidCallbackUrl(verifiedState.callbackUrl) ? verifiedState.callbackUrl : SUCCESS_CALLBACK_URL}`); } catch (e) { console.error(`[OAuth Router] - OAuth callback (route: ${req.path}) error:`, e); diff --git a/server/services/oauth/providers/github.js b/server/services/oauth/providers/github.js index 616944b..407173b 100644 --- a/server/services/oauth/providers/github.js +++ b/server/services/oauth/providers/github.js @@ -41,7 +41,7 @@ class githubOAuthProvider extends OAuthService { if(!emailResponse.ok) { // ignore warning, Rethrown after catching.... - throw Error(`Failed to fetch User Email (response status: ${emailResponse.status}, response msg: ${emailResponse.json()?.message})`) + throw Error(`Failed to fetch User Email (response status: ${emailResponse.status}, response msg: ${await emailResponse.json()?.message})`) } emailResponse.data = await emailResponse.json(); From 71c72844821fd025ef98a737d5a310cd9874ce45 Mon Sep 17 00:00:00 2001 From: UnschooledGamer <76094069+UnschooledGamer@users.noreply.github.com> Date: Fri, 5 Dec 2025 20:54:05 +0530 Subject: [PATCH 11/13] refactor: Update allowed callback hosts and improve error handling in OAuth flow --- constants.js | 3 +-- server/apis/oauth.js | 3 +-- server/services/oauth/providers/github.js | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/constants.js b/constants.js index 38a8399..fcb3043 100644 --- a/constants.js +++ b/constants.js @@ -1,6 +1,5 @@ module.exports = { ALLOWED_CALLBACK_HOSTS_ARRAY: [ - "https://acode.app/", - "http://localhost:5500/" + "acode.app", ] } \ No newline at end of file diff --git a/server/apis/oauth.js b/server/apis/oauth.js index 2d0821d..a7b702f 100644 --- a/server/apis/oauth.js +++ b/server/apis/oauth.js @@ -72,7 +72,7 @@ async function handleOAuthCallback(req, res) { console.error(`[OAuth Router] - Provider (${provider}) responded without a state: ${error}`); return res.redirect(`${ERROR_REDIRECT_PATH}?error=missing_state`); } - + if(error) { console.error(`[OAuth Router] - Provider (${provider}) responded with an error: ${error}`); return res.redirect(`${ERROR_REDIRECT_PATH}?error=${encodeURIComponent(error)}&error_description=${encodeURIComponent(error_description || '')}`); @@ -118,7 +118,6 @@ async function handleOAuthCallback(req, res) { const tokens = await OAuthProvider.getAccessToken({ code, codeVerifier: verifiedState.codeVerifier }).catch((e) => ({ error: e})); if(!tokens?.accessToken || !tokens) { - console.log(tokens.error); res.status(401).send(tokens?.error || "Failed to retrieve access token"); return; } diff --git a/server/services/oauth/providers/github.js b/server/services/oauth/providers/github.js index 407173b..edc2da2 100644 --- a/server/services/oauth/providers/github.js +++ b/server/services/oauth/providers/github.js @@ -41,7 +41,7 @@ class githubOAuthProvider extends OAuthService { if(!emailResponse.ok) { // ignore warning, Rethrown after catching.... - throw Error(`Failed to fetch User Email (response status: ${emailResponse.status}, response msg: ${await emailResponse.json()?.message})`) + throw Error(`Failed to fetch User Email (response status: ${emailResponse.status}, response msg: ${(await emailResponse.json().catch(() => ({})))?.message || 'Unknown error'})`) } emailResponse.data = await emailResponse.json(); From 50761c7591b1a2585dca2eee793870e5e3a22f1a Mon Sep 17 00:00:00 2001 From: UnschooledGamer <76094069+UnschooledGamer@users.noreply.github.com> Date: Fri, 5 Dec 2025 20:57:54 +0530 Subject: [PATCH 12/13] add: localhost to ALLOWED Callbacks in dev env --- constants.js | 1 + 1 file changed, 1 insertion(+) diff --git a/constants.js b/constants.js index fcb3043..046c9ff 100644 --- a/constants.js +++ b/constants.js @@ -1,5 +1,6 @@ module.exports = { ALLOWED_CALLBACK_HOSTS_ARRAY: [ "acode.app", + ...(process.env.NODE_ENV === 'development' ? ["localhost:5500"] : []) ] } \ No newline at end of file From 76b36d74e8f4daf0e8bd2cff193ccc4875ab6a5d Mon Sep 17 00:00:00 2001 From: UnschooledGamer <76094069+UnschooledGamer@users.noreply.github.com> Date: Fri, 5 Dec 2025 21:12:35 +0530 Subject: [PATCH 13/13] add: cookie secret in cookie parser --- server/main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/main.js b/server/main.js index 08eb3dd..b6bdcdf 100644 --- a/server/main.js +++ b/server/main.js @@ -35,7 +35,7 @@ async function main() { next(); }); - app.use(cookieParser()); + app.use(cookieParser(process.env.COOKIE_SECRET)); app.use( fileUpload({ limits: {