From d6558e063ad28058f1a31358f966185fe01c159e Mon Sep 17 00:00:00 2001 From: Matthieu Sieben Date: Tue, 10 Feb 2026 11:43:14 +0100 Subject: [PATCH 1/5] fix(oauth): properly bind oauth state data to the device --- server/api/auth/atproto.get.ts | 270 ++++++++++++++++++++++----------- 1 file changed, 178 insertions(+), 92 deletions(-) diff --git a/server/api/auth/atproto.get.ts b/server/api/auth/atproto.get.ts index 13300aad5..f9e9b21aa 100644 --- a/server/api/auth/atproto.get.ts +++ b/server/api/auth/atproto.get.ts @@ -1,10 +1,13 @@ -import { NodeOAuthClient } from '@atproto/oauth-client-node' +import type { OAuthSession } from '@atproto/oauth-client-node' +import { NodeOAuthClient, OAuthCallbackError } from '@atproto/oauth-client-node' import { createError, getQuery, sendRedirect } from 'h3' +import type { H3Event } from 'h3' import { getOAuthLock } from '#server/utils/atproto/lock' import { useOAuthStorage } from '#server/utils/atproto/storage' import { SLINGSHOT_HOST } from '#shared/utils/constants' import { useServerSession } from '#server/utils/server-session' import { handleResolver } from '#server/utils/atproto/oauth' +import type { DidString } from '@atproto/lex' import { Client } from '@atproto/lex' import * as com from '#shared/types/lexicons/com' import * as app from '#shared/types/lexicons/app' @@ -12,44 +15,6 @@ import { isAtIdentifierString } from '@atproto/lex' // @ts-expect-error virtual file from oauth module import { clientUri } from '#oauth/config' -//I did not have luck with other ones than these. I got this list from the PDS language picker -const OAUTH_LOCALES = new Set(['en', 'fr-FR', 'ja-JP']) - -/** - * Fetch the user's profile record to get their avatar blob reference - * @param did - * @param pds - * @returns - */ -async function getAvatar(did: string, pds: string) { - if (!isAtIdentifierString(did)) { - return undefined - } - - let avatar: string | undefined - try { - const pdsUrl = new URL(pds) - // Only fetch from HTTPS PDS endpoints to prevent SSRF - if (pdsUrl.protocol === 'https:') { - const client = new Client(pdsUrl) - const profileResponse = await client.get(app.bsky.actor.profile, { - repo: did, - rkey: 'self', - }) - - const validatedResponse = app.bsky.actor.profile.main.validate(profileResponse.value) - - if (validatedResponse.avatar?.ref) { - // Use Bluesky CDN for faster image loading - avatar = `https://cdn.bsky.app/img/feed_thumbnail/plain/${did}/${validatedResponse.avatar?.ref}@jpeg` - } - } - } catch { - // Avatar fetch failed, continue without it - } - return avatar -} - export default defineEventHandler(async event => { const config = useRuntimeConfig(event) if (!config.sessionPassword) { @@ -72,16 +37,18 @@ export default defineEventHandler(async event => { handleResolver, }) - const error = query.error - - // user cancelled explicitly - if (error === 'access_denied') { - const returnToURL = getCookie(event, 'auth_return_to') || '/' - deleteCookie(event, 'auth_return_to', { path: '/' }) - return sendRedirect(event, returnToURL) - } + if (query.handle) { + // Initiate auth flow + if ( + typeof query.handle !== 'string' || + (!query.handle.startsWith('https://') && !isAtIdentifierString(query.handle)) + ) { + throw createError({ + statusCode: 400, + message: 'Invalid handle parameter', + }) + } - if (!query.code) { // Validate returnTo is a safe relative path (prevent open redirect) // Only set cookie on initial auth request, not the callback let redirectPath = '/' @@ -95,34 +62,17 @@ export default defineEventHandler(async event => { // Invalid URL, fall back to root } - setCookie(event, 'auth_return_to', redirectPath, { - maxAge: 60 * 5, - httpOnly: true, - // secure only if NOT in dev mode - secure: !import.meta.dev, - }) try { - const handle = query.handle?.toString() - const create = query.create?.toString() - - if (!handle) { - throw createError({ - statusCode: 401, - message: 'Handle not provided in query', - }) - } - - const localeFromQuery = query.locale?.toString() ?? 'en' - const locale = OAUTH_LOCALES.has(localeFromQuery) ? localeFromQuery : 'en' - - const redirectUrl = await atclient.authorize(handle, { + const redirectUrl = await atclient.authorize(query.handle, { scope, - prompt: create ? 'create' : undefined, - ui_locales: locale, + prompt: query.create ? 'create' : undefined, + ui_locales: query.locale?.toString(), + state: encodeOAuthState(event, { redirectPath }), }) + return sendRedirect(event, redirectUrl.toString()) } catch (error) { - const message = error instanceof Error ? error.message : 'Authentication failed.' + const message = error instanceof Error ? error.message : 'Failed to initiate authentication.' return handleApiError(error, { statusCode: 401, @@ -130,44 +80,180 @@ export default defineEventHandler(async event => { message: `${message}. Please login and try again.`, }) } + } else { + // Handle callback + try { + const params = new URLSearchParams(query as Record) + const result = await atclient.callback(params) + + await session.update({ + public: await getMiniProfile(result.session), + }) + + const state = decodeOAuthState(event, result.state) + return sendRedirect(event, state.redirectPath) + } catch (error) { + // user cancelled explicitly + if (query.error === 'access_denied' && error instanceof OAuthCallbackError && error.state) { + const state = decodeOAuthState(event, error.state) + return sendRedirect(event, state.redirectPath) + } + + const message = error instanceof Error ? error.message : 'Authentication failed.' + return handleApiError(error, { + statusCode: 401, + statusMessage: 'Unauthorized', + message: `${message}. Please login and try again.`, + }) + } + } +}) + +type OAuthStateData = { + redirectPath: string +} + +const SID_COOKIE_NAME = 'atproto_oauth_sid' +const SID_COOKIE_VALUE = '1' + +/** + * This function encodes the OAuth state by generating a random SID, storing it + * in a cookie, and returning a JSON string containing the original state and + * the SID. The cookie is used to validate the authenticity of the callback + * request later. + * + * This mechanism allows to bind a particular authentication request to a + * particular client (browser) session, providing protection against CSRF attacks + * and ensuring that the callback is part of an ongoing authentication flow + * initiated by the same client. + * + * Note that this mechanism could use any other unique session mechanism the + * server has with the client (e.g. UserServerSession). We don't do that though + * to avoid polluting the session with ephemeral OAuth-specific data. + * + * @param event The H3 event object, used to set the cookie + * @param state The original OAuth state to encode + * @returns A JSON string encapsulating the original state and the generated SID + */ +function encodeOAuthState(event: H3Event, data: OAuthStateData): string { + const sid = generateRandomHexString() + setCookie(event, `${SID_COOKIE_NAME}:${sid}`, SID_COOKIE_VALUE, { + maxAge: 60 * 5, + httpOnly: true, + // secure only if NOT in dev mode + secure: !import.meta.dev, + }) + return JSON.stringify({ data, sid }) +} + +function generateRandomHexString(byteLength: number = 16): string { + return Array.from(crypto.getRandomValues(new Uint8Array(byteLength)), byte => + byte.toString(16).padStart(2, '0'), + ).join('') +} + +/** + * This function ensures that an oauth state was indeed encoded for the browser + * session performing the oauth callback. + * + * @param event The H3 event object, used to read and delete the cookie + * @param state The JSON string containing the original state and SID + * @returns The original OAuth state if the SID is valid + * @throws An error if the SID is missing or invalid, indicating a potential issue with cookies or expired state + */ +function decodeOAuthState(event: H3Event, state: string | null): OAuthStateData { + if (!state) { + // May happen during transition period (if a user initiated auth flow before + // the release with the new state handling, then tries to complete it after + // the release). + throw createError({ + statusCode: 400, + message: 'Missing state parameter', + }) } - const { session: authSession } = await atclient.callback( - new URLSearchParams(query as Record), - ) + // The state sting was encoded using encodeOAuthState. No need to protect + // against JSON parsing since the StateStore should ensure it's integrity. + const decoded = JSON.parse(state) as { data: OAuthStateData; sid: string } + const sid = getCookie(event, `${SID_COOKIE_NAME}:${decoded.sid}`) + if (sid === SID_COOKIE_VALUE) { + deleteCookie(event, `${SID_COOKIE_NAME}:${decoded.sid}`, { + httpOnly: true, + secure: !import.meta.dev, + }) + } else { + throw createError({ + statusCode: 400, + message: 'Missing authentication state. Please enable cookies and try again.', + }) + } + + return decoded.data +} + +/** + * Fetches the mini profile for the authenticated user, including their avatar if available. + * This is used to populate the session with basic user info after authentication. + * @param authSession The OAuth session containing the user's DID and token info + * @returns An object containing the user's DID, handle, PDS, and avatar URL (if available) + */ +async function getMiniProfile(authSession: OAuthSession) { const client = new Client({ service: `https://${SLINGSHOT_HOST}` }) const response = await client.xrpcSafe(com['bad-example'].identity.resolveMiniDoc, { headers: { 'User-Agent': 'npmx' }, params: { identifier: authSession.did }, }) + if (response.success) { const miniDoc = response.body let avatar: string | undefined = await getAvatar(authSession.did, miniDoc.pds) - await session.update({ - public: { - ...miniDoc, - avatar, - }, - }) + return { + ...miniDoc, + avatar, + } } else { //If slingshot fails we still want to set some key info we need. const pdsBase = (await authSession.getTokenInfo()).aud let avatar: string | undefined = await getAvatar(authSession.did, pdsBase) - await session.update({ - public: { - did: authSession.did, - handle: 'Not available', - pds: pdsBase, - avatar, - }, - }) + return { + did: authSession.did, + handle: 'Not available', + pds: pdsBase, + avatar, + } } +} - const returnToURL = getCookie(event, 'auth_return_to') || '/' - deleteCookie(event, 'auth_return_to') +/** + * Fetch the user's profile record to get their avatar blob reference + * @param did + * @param pds + * @returns + */ +async function getAvatar(did: DidString, pds: string) { + let avatar: string | undefined + try { + const pdsUrl = new URL(pds) + // Only fetch from HTTPS PDS endpoints to prevent SSRF + if (pdsUrl.protocol === 'https:') { + const client = new Client(pdsUrl) + const profileResponse = await client.get(app.bsky.actor.profile, { + repo: did, + rkey: 'self', + }) - return sendRedirect(event, returnToURL) -}) + const validatedResponse = app.bsky.actor.profile.main.validate(profileResponse.value) + + if (validatedResponse.avatar?.ref) { + // Use Bluesky CDN for faster image loading + avatar = `https://cdn.bsky.app/img/feed_thumbnail/plain/${did}/${validatedResponse.avatar?.ref}@jpeg` + } + } + } catch { + // Avatar fetch failed, continue without it + } + return avatar +} From a6b94e437f22b66f98b9ec8f42ec1d5b9f71aef9 Mon Sep 17 00:00:00 2001 From: Matthieu Sieben Date: Tue, 10 Feb 2026 11:59:23 +0100 Subject: [PATCH 2/5] tidy --- server/api/auth/atproto.get.ts | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/server/api/auth/atproto.get.ts b/server/api/auth/atproto.get.ts index f9e9b21aa..010390320 100644 --- a/server/api/auth/atproto.get.ts +++ b/server/api/auth/atproto.get.ts @@ -85,13 +85,21 @@ export default defineEventHandler(async event => { try { const params = new URLSearchParams(query as Record) const result = await atclient.callback(params) + try { + const state = decodeOAuthState(event, result.state) + const profile = await getMiniProfile(result.session) - await session.update({ - public: await getMiniProfile(result.session), - }) - - const state = decodeOAuthState(event, result.state) - return sendRedirect(event, state.redirectPath) + await session.update({ public: profile }) + return sendRedirect(event, state.redirectPath) + } catch (error) { + // If we are unable to cleanly handle the callback, meaning that the + // user won't be able to use the session, we sign them out of the + // session to prevent dangling sessions. This can happen if the state is + // invalid (e.g. user has cookies disabled, or the state expired) or if + // there is an issue fetching the user's profile after authentication. + await result.session.signOut() + throw error + } } catch (error) { // user cancelled explicitly if (query.error === 'access_denied' && error instanceof OAuthCallbackError && error.state) { From 9024740f7e163b85a196f7bec25a3e7d0b2bf558 Mon Sep 17 00:00:00 2001 From: Matthieu Sieben Date: Tue, 10 Feb 2026 12:13:44 +0100 Subject: [PATCH 3/5] @coderabbitai review changes --- server/api/auth/atproto.get.ts | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/server/api/auth/atproto.get.ts b/server/api/auth/atproto.get.ts index 010390320..1b420ec12 100644 --- a/server/api/auth/atproto.get.ts +++ b/server/api/auth/atproto.get.ts @@ -1,17 +1,20 @@ import type { OAuthSession } from '@atproto/oauth-client-node' import { NodeOAuthClient, OAuthCallbackError } from '@atproto/oauth-client-node' -import { createError, getQuery, sendRedirect } from 'h3' +import { createError, getQuery, sendRedirect, setCookie, getCookie, deleteCookie } from 'h3' import type { H3Event } from 'h3' import { getOAuthLock } from '#server/utils/atproto/lock' import { useOAuthStorage } from '#server/utils/atproto/storage' import { SLINGSHOT_HOST } from '#shared/utils/constants' import { useServerSession } from '#server/utils/server-session' import { handleResolver } from '#server/utils/atproto/oauth' +import { handleApiError } from '#server/utils/error-handler' import type { DidString } from '@atproto/lex' import { Client } from '@atproto/lex' import * as com from '#shared/types/lexicons/com' import * as app from '#shared/types/lexicons/app' import { isAtIdentifierString } from '@atproto/lex' +import { scope, getOauthClientMetadata } from '#server/utils/atproto/oauth' +import { UNSET_NUXT_SESSION_PASSWORD } from '#shared/utils/constants' // @ts-expect-error virtual file from oauth module import { clientUri } from '#oauth/config' @@ -121,7 +124,7 @@ type OAuthStateData = { redirectPath: string } -const SID_COOKIE_NAME = 'atproto_oauth_sid' +const SID_COOKIE_PREFIX = 'atproto_oauth_sid' const SID_COOKIE_VALUE = '1' /** @@ -145,11 +148,13 @@ const SID_COOKIE_VALUE = '1' */ function encodeOAuthState(event: H3Event, data: OAuthStateData): string { const sid = generateRandomHexString() - setCookie(event, `${SID_COOKIE_NAME}:${sid}`, SID_COOKIE_VALUE, { + setCookie(event, `${SID_COOKIE_PREFIX}_${sid}`, SID_COOKIE_VALUE, { maxAge: 60 * 5, httpOnly: true, // secure only if NOT in dev mode secure: !import.meta.dev, + sameSite: 'lax', + path: event.path, }) return JSON.stringify({ data, sid }) } @@ -184,11 +189,13 @@ function decodeOAuthState(event: H3Event, state: string | null): OAuthStateData // against JSON parsing since the StateStore should ensure it's integrity. const decoded = JSON.parse(state) as { data: OAuthStateData; sid: string } - const sid = getCookie(event, `${SID_COOKIE_NAME}:${decoded.sid}`) + const sid = getCookie(event, `${SID_COOKIE_PREFIX}_${decoded.sid}`) if (sid === SID_COOKIE_VALUE) { - deleteCookie(event, `${SID_COOKIE_NAME}:${decoded.sid}`, { + deleteCookie(event, `${SID_COOKIE_PREFIX}_${decoded.sid}`, { httpOnly: true, secure: !import.meta.dev, + sameSite: 'lax', + path: event.path, }) } else { throw createError({ @@ -254,10 +261,11 @@ async function getAvatar(did: DidString, pds: string) { }) const validatedResponse = app.bsky.actor.profile.main.validate(profileResponse.value) + const cid = validatedResponse.avatar?.ref - if (validatedResponse.avatar?.ref) { + if (cid) { // Use Bluesky CDN for faster image loading - avatar = `https://cdn.bsky.app/img/feed_thumbnail/plain/${did}/${validatedResponse.avatar?.ref}@jpeg` + avatar = `https://cdn.bsky.app/img/feed_thumbnail/plain/${did}/${cid}@jpeg` } } } catch { From 741c06f6c3e76cced49a45edc1a77fd9228c9bb7 Mon Sep 17 00:00:00 2001 From: Matthieu Sieben Date: Tue, 10 Feb 2026 12:21:47 +0100 Subject: [PATCH 4/5] fix cookie path --- server/api/auth/atproto.get.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/api/auth/atproto.get.ts b/server/api/auth/atproto.get.ts index 1b420ec12..b9280d26d 100644 --- a/server/api/auth/atproto.get.ts +++ b/server/api/auth/atproto.get.ts @@ -154,7 +154,7 @@ function encodeOAuthState(event: H3Event, data: OAuthStateData): string { // secure only if NOT in dev mode secure: !import.meta.dev, sameSite: 'lax', - path: event.path, + path: event.path.split('?', 1)[0], }) return JSON.stringify({ data, sid }) } @@ -195,7 +195,7 @@ function decodeOAuthState(event: H3Event, state: string | null): OAuthStateData httpOnly: true, secure: !import.meta.dev, sameSite: 'lax', - path: event.path, + path: event.path.split('?', 1)[0], }) } else { throw createError({ From 24b9d92f96d557f5b48b39c8818a19802fea391e Mon Sep 17 00:00:00 2001 From: Matthieu Sieben Date: Tue, 10 Feb 2026 13:01:25 +0100 Subject: [PATCH 5/5] tidy --- server/api/auth/atproto.get.ts | 54 ++++++++++++++++++++++------------ 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/server/api/auth/atproto.get.ts b/server/api/auth/atproto.get.ts index b9280d26d..0ff4b2eaf 100644 --- a/server/api/auth/atproto.get.ts +++ b/server/api/auth/atproto.get.ts @@ -104,10 +104,14 @@ export default defineEventHandler(async event => { throw error } } catch (error) { - // user cancelled explicitly - if (query.error === 'access_denied' && error instanceof OAuthCallbackError && error.state) { + if (error instanceof OAuthCallbackError && error.state) { + // Always decode the state, to clean up the cookie const state = decodeOAuthState(event, error.state) - return sendRedirect(event, state.redirectPath) + + // user cancelled explicitly + if (query.error === 'access_denied') { + return sendRedirect(event, state.redirectPath) + } } const message = error instanceof Error ? error.message : 'Authentication failed.' @@ -124,8 +128,7 @@ type OAuthStateData = { redirectPath: string } -const SID_COOKIE_PREFIX = 'atproto_oauth_sid' -const SID_COOKIE_VALUE = '1' +const OAUTH_REQUEST_COOKIE_PREFIX = 'atproto_oauth_req' /** * This function encodes the OAuth state by generating a random SID, storing it @@ -138,17 +141,26 @@ const SID_COOKIE_VALUE = '1' * and ensuring that the callback is part of an ongoing authentication flow * initiated by the same client. * - * Note that this mechanism could use any other unique session mechanism the - * server has with the client (e.g. UserServerSession). We don't do that though - * to avoid polluting the session with ephemeral OAuth-specific data. - * * @param event The H3 event object, used to set the cookie * @param state The original OAuth state to encode * @returns A JSON string encapsulating the original state and the generated SID */ function encodeOAuthState(event: H3Event, data: OAuthStateData): string { - const sid = generateRandomHexString() - setCookie(event, `${SID_COOKIE_PREFIX}_${sid}`, SID_COOKIE_VALUE, { + const id = generateRandomHexString() + // This uses an ephemeral cookie instead of useSession() to avoid polluting + // the session with ephemeral OAuth-specific data. The cookie is set with a + // short expiration time to limit the window of potential misuse, and is + // deleted immediately after validating the callback to clean up any remnants + // of the authentication flow. Using useSession() for this would require + // additional logic to clean up the session in case of expired ephemeral data. + + // We use the id as cookie name to allow multiple concurrent auth flows (e.g. + // user opens multiple tabs and initiates auth in both, or initiates auth, + // waits for a while, then initiates again before completing the first one), + // without risk of cookie value collisions between them. The cookie value is a + // constant since the actual value doesn't matter - it's just used as a flag + // to validate the presence of the cookie on callback. + setCookie(event, `${OAUTH_REQUEST_COOKIE_PREFIX}_${id}`, '1', { maxAge: 60 * 5, httpOnly: true, // secure only if NOT in dev mode @@ -156,7 +168,8 @@ function encodeOAuthState(event: H3Event, data: OAuthStateData): string { sameSite: 'lax', path: event.path.split('?', 1)[0], }) - return JSON.stringify({ data, sid }) + + return JSON.stringify({ data, id }) } function generateRandomHexString(byteLength: number = 16): string { @@ -170,9 +183,9 @@ function generateRandomHexString(byteLength: number = 16): string { * session performing the oauth callback. * * @param event The H3 event object, used to read and delete the cookie - * @param state The JSON string containing the original state and SID - * @returns The original OAuth state if the SID is valid - * @throws An error if the SID is missing or invalid, indicating a potential issue with cookies or expired state + * @param state The JSON string containing the original state and id + * @returns The original OAuth state if the id is valid + * @throws An error if the id is missing or invalid, indicating a potential issue with cookies or expired state */ function decodeOAuthState(event: H3Event, state: string | null): OAuthStateData { if (!state) { @@ -187,11 +200,14 @@ function decodeOAuthState(event: H3Event, state: string | null): OAuthStateData // The state sting was encoded using encodeOAuthState. No need to protect // against JSON parsing since the StateStore should ensure it's integrity. - const decoded = JSON.parse(state) as { data: OAuthStateData; sid: string } + const decoded = JSON.parse(state) as { data: OAuthStateData; id: string } + const requestCookieName = `${OAUTH_REQUEST_COOKIE_PREFIX}_${decoded.id}` - const sid = getCookie(event, `${SID_COOKIE_PREFIX}_${decoded.sid}`) - if (sid === SID_COOKIE_VALUE) { - deleteCookie(event, `${SID_COOKIE_PREFIX}_${decoded.sid}`, { + if (getCookie(event, requestCookieName) != null) { + // The cookie will never be used again since the state store ensure unique + // nonces, but we delete it to clean up any remnants of the authentication + // flow. + deleteCookie(event, requestCookieName, { httpOnly: true, secure: !import.meta.dev, sameSite: 'lax',