Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions web/src/hooks.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,10 @@ const handleLogging: Handle = async ({ event, resolve }) => {

logger.info('Request started', {
method: event.request.method,
url: event.url.pathname + event.url.search,
url: event.url.pathname,
userAgent: event.request.headers.get('user-agent'),
ip: event.getClientAddress(),
userId: event.locals.user?.id,
userEmail: event.locals.user?.email,
})

const response = await resolve(event)
Expand All @@ -120,7 +119,7 @@ const handleLogging: Handle = async ({ event, resolve }) => {

logger.info('Request completed', {
method: event.request.method,
url: event.url.pathname + event.url.search,
url: event.url.pathname,
status: response.status,
duration,
userId: event.locals.user?.id,
Expand All @@ -135,7 +134,7 @@ export const handleError: HandleServerError = ({ error, event }) => {
const logger = event.locals.logger || new Logger('error')

logger.error('Unhandled server error', error as Error, {
url: event.url.pathname + event.url.search,
url: event.url.pathname,
method: event.request.method,
userId: event.locals.user?.id,
requestId: event.locals.requestId,
Expand Down
98 changes: 98 additions & 0 deletions web/src/lib/server/error-serializer.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import { describe, expect, it } from 'vitest'
import { serializeError } from './error-serializer'

describe('serializeError', () => {
it('includes PostgreSQL failure metadata without messages or query values', () => {
const cause = Object.assign(new Error('database message with super-secret-value'), {
name: 'PostgresError',
code: '23514',
severity: 'ERROR',
schema_name: 'public',
table_name: 'user_oauth_credentials',
column_name: undefined,
type_name: undefined,
constraint_name: 'user_oauth_credentials_refresh_token_encrypted',
routine: 'ExecConstraints',
detail: 'Failing row contains super-secret-profile-data',
query: 'INSERT with super-secret-driver-query',
parameters: ['super-secret-token'],
})
const error = Object.assign(
new Error('Failed query: INSERT with super-secret-query\nparams: super-secret-token'),
{
query: 'INSERT with super-secret-query',
params: ['super-secret-token'],
cause,
},
)

const serialized = serializeError(error)

expect(serialized).toMatchObject({
type: 'Error',
message: 'Database query failed',
database: {
sqlState: '23514',
severity: 'ERROR',
schema: 'public',
table: 'user_oauth_credentials',
constraint: 'user_oauth_credentials_refresh_token_encrypted',
routine: 'ExecConstraints',
},
})

const logged = JSON.stringify(serialized)
expect(logged).not.toContain('super-secret')
expect(logged).not.toContain('params')
expect(logged).not.toContain('detail')
})

it('sanitizes query errors without a PostgreSQL cause', () => {
const error = Object.assign(new Error('Failed query: SELECT $1\nparams: secret'), {
query: 'SELECT $1',
params: ['secret'],
cause: Object.assign(new Error('connection failed'), { code: 'ECONNRESET' }),
})

const serialized = serializeError(error)

expect(serialized.message).toBe('Database query failed')
expect(serialized.database).toBeUndefined()
expect(JSON.stringify(serialized)).not.toContain('secret')
})

it('extracts metadata from direct PostgreSQL errors', () => {
const error = Object.assign(new Error('duplicate value: super-secret-email'), {
name: 'PostgresError',
code: '23505',
severity: 'ERROR',
table_name: 'users',
constraint_name: 'users_email_key',
})

const serialized = serializeError(error)

expect(serialized).toMatchObject({
type: 'PostgresError',
message: 'Database query failed',
database: {
sqlState: '23505',
severity: 'ERROR',
table: 'users',
constraint: 'users_email_key',
},
})
expect(JSON.stringify(serialized)).not.toContain('super-secret-email')
})

it('preserves ordinary error messages and stacks', () => {
const error = new Error('ordinary failure')

expect(serializeError(error)).toEqual({
type: 'Error',
message: 'ordinary failure',
stack: error.stack,
database: undefined,
})
})
})
68 changes: 68 additions & 0 deletions web/src/lib/server/error-serializer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
export interface SerializedDatabaseError {
sqlState: string
severity?: string
schema?: string
table?: string
column?: string
dataType?: string
constraint?: string
routine?: string
}

export interface SerializedError {
type: string
message: string
stack?: string
database?: SerializedDatabaseError
}

type ErrorRecord = Record<string, unknown>

function asErrorRecord(value: unknown): ErrorRecord | null {
return typeof value === 'object' && value !== null ? (value as ErrorRecord) : null
}

function getString(record: ErrorRecord, key: string): string | undefined {
const value = record[key]
return typeof value === 'string' ? value : undefined
}

function serializeDatabaseError(cause: unknown): SerializedDatabaseError | undefined {
const record = asErrorRecord(cause)
if (!record) return undefined

const sqlState = getString(record, 'code')
if (!sqlState || !/^[0-9A-Z]{5}$/.test(sqlState)) return undefined

return {
sqlState,
severity: getString(record, 'severity'),
schema: getString(record, 'schema_name'),
table: getString(record, 'table_name'),
column: getString(record, 'column_name'),
dataType: getString(record, 'data_type_name') ?? getString(record, 'type_name'),
constraint: getString(record, 'constraint_name'),
routine: getString(record, 'routine'),
}
}

function stackFrames(stack: string | undefined): string | undefined {
if (!stack) return undefined

const frames = stack.split('\n').filter((line) => line.trimStart().startsWith('at '))
return frames.length > 0 ? frames.join('\n') : undefined
}

export function serializeError(error: Error): SerializedError {
const record = error as Error & ErrorRecord
const isQueryError = typeof record.query === 'string' && Array.isArray(record.params)
const database = serializeDatabaseError(record.cause) ?? serializeDatabaseError(record)
const isDatabaseError = isQueryError || database !== undefined

return {
type: error.name,
message: isDatabaseError ? 'Database query failed' : error.message,
stack: isDatabaseError ? stackFrames(error.stack) : error.stack,
database,
}
}
13 changes: 3 additions & 10 deletions web/src/lib/server/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import pino, { type Logger as PinoLogger } from 'pino'
import { env } from '$env/dynamic/private'
import { dev } from '$app/environment'
import { ulid } from 'ulid'
import { serializeError } from './error-serializer'

const logLevel = env.LOG_LEVEL || (dev ? 'debug' : 'info')
const logPretty = env.LOG_PRETTY === 'true' || dev
Expand Down Expand Up @@ -30,21 +31,13 @@ const pinoConfig: pino.LoggerOptions = {
},
},
serializers: {
error: (err: Error) => ({
type: err.name,
message: err.message,
stack: err.stack,
}),
error: serializeError,
request: (req: any) => ({
method: req.method,
url: req.url,
headers: req.headers,
query: req.query,
params: req.params,
url: typeof req.url === 'string' ? req.url.split('?')[0] : undefined,
}),
response: (res: any) => ({
statusCode: res.statusCode,
headers: res.headers,
}),
},
...(transport && { transport }),
Expand Down
2 changes: 1 addition & 1 deletion web/src/lib/server/oauth/accountLinking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export class AccountLinkingService {

if (!isDomainApproved) {
throw new Error(
`Registration is not allowed for domain: ${profile.email.split('@')[1]}. ` +
'Registration is not allowed for this email domain. ' +
'Please contact your administrator to approve your domain.',
)
}
Expand Down
23 changes: 15 additions & 8 deletions web/src/lib/server/oauth/sso-callback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ import { app } from '$lib/server/config'
import { OAuthStateManager } from '$lib/server/oauth/state'
import { AccountLinkingService } from '$lib/server/oauth/accountLinking'
import { createSession, generateSessionToken, setSessionTokenCookie } from '$lib/server/auth'
import { logger } from '$lib/server/logger'
import { logger, type Logger } from '$lib/server/logger'

export interface SsoCallbackOptions {
provider: string
loadConfig: () => Promise<{ enabled: boolean; [key: string]: any } | null>
loadService: () => Promise<any | null>
createService: (ServiceClass: any, config: any, callbackUrl: string) => any
callbackPath: string
logger?: Logger
}

function getErrorRedirect(error: unknown): string {
Expand All @@ -34,19 +35,22 @@ export async function handleSsoCallback(
options: SsoCallbackOptions,
): Promise<never> {
const { provider, loadConfig, loadService, createService, callbackPath } = options
const callbackLogger = options.logger ?? logger

const code = url.searchParams.get('code')
const state = url.searchParams.get('state')
const error = url.searchParams.get('error')

if (error) {
const errorDescription = url.searchParams.get('error_description') || 'Unknown OAuth error'
logger.error(`${provider} OAuth callback error:`, errorDescription)
callbackLogger.error(`${provider} OAuth callback rejected`, undefined, {
provider,
oauthError: error.slice(0, 100),
})
redirect(302, '/login?error=oauth_error')
}

if (!code || !state) {
logger.error('Missing required OAuth parameters')
callbackLogger.error('Missing required OAuth parameters', undefined, { provider })
redirect(302, '/login?error=invalid_oauth_response')
}

Expand Down Expand Up @@ -107,12 +111,15 @@ export async function handleSsoCallback(
redirectUrl.searchParams.set('linked', provider)
}

logger.info(
`${provider} OAuth authentication successful for user: ${user.email} (${user.id})`,
)
callbackLogger.info(`${provider} OAuth authentication successful`, {
provider,
userId: user.id,
isNewUser,
isLinkedAccount,
})
successUrl = redirectUrl.toString()
} catch (error) {
logger.error(`${provider} OAuth callback error:`, error)
callbackLogger.error(`${provider} OAuth callback error`, error, { provider })
redirect(302, getErrorRedirect(error))
}

Expand Down
5 changes: 3 additions & 2 deletions web/src/routes/(public)/auth/entra/callback/+server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { loadEntraOAuthService } from '$lib/server/oauth/entra'
import { handleSsoCallback } from '$lib/server/oauth/sso-callback'
import type { RequestHandler } from './$types'

export const GET: RequestHandler = async ({ url, cookies }) => {
await handleSsoCallback(url, cookies, {
export const GET: RequestHandler = async ({ url, cookies, locals }) => {
return handleSsoCallback(url, cookies, {
provider: 'entra',
loadConfig: getEntraAuthConfig,
loadService: loadEntraOAuthService,
Expand All @@ -18,6 +18,7 @@ export const GET: RequestHandler = async ({ url, cookies }) => {
callbackUrl,
),
callbackPath: '/auth/entra/callback',
logger: locals.logger,
})
}

Expand Down
5 changes: 3 additions & 2 deletions web/src/routes/(public)/auth/okta/callback/+server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { loadOktaOAuthService } from '$lib/server/oauth/okta'
import { handleSsoCallback } from '$lib/server/oauth/sso-callback'
import type { RequestHandler } from './$types'

export const GET: RequestHandler = async ({ url, cookies }) => {
await handleSsoCallback(url, cookies, {
export const GET: RequestHandler = async ({ url, cookies, locals }) => {
return handleSsoCallback(url, cookies, {
provider: 'okta',
loadConfig: getOktaAuthConfig,
loadService: loadOktaOAuthService,
Expand All @@ -18,6 +18,7 @@ export const GET: RequestHandler = async ({ url, cookies }) => {
callbackUrl,
),
callbackPath: '/auth/okta/callback',
logger: locals.logger,
})
}

Expand Down