Skip to content

Commit be9d511

Browse files
committed
feat(auth): additional logging for oauth errors and cancellations
1 parent 3764676 commit be9d511

2 files changed

Lines changed: 211 additions & 7 deletions

File tree

__tests__/routes/betterAuth.ts

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ import { generateShortId } from '../../src/ids';
2929
import { usersFixture } from '../fixture';
3030
import { ioRedisPool } from '../../src/redis';
3131
import * as betterAuthModule from '../../src/betterAuth';
32+
import { rewriteOAuthErrorRedirect } from '../../src/routes/betterAuth';
33+
import type { FastifyRequest } from 'fastify';
3234

3335
jest.mock('../../src/common/paddle/index.ts', () => ({
3436
...(jest.requireActual('../../src/common/paddle/index.ts') as Record<
@@ -395,4 +397,182 @@ describe('betterAuth routes', () => {
395397
getBetterAuthSpy.mockRestore();
396398
});
397399
});
400+
401+
describe('rewriteOAuthErrorRedirect helper', () => {
402+
const makeRequest = (url: string): FastifyRequest =>
403+
({
404+
url,
405+
protocol: 'http',
406+
host: 'localhost',
407+
}) as FastifyRequest;
408+
409+
const makeRedirectResponse = (
410+
location: string | null,
411+
status = 302,
412+
): Response => {
413+
const headers = new Headers();
414+
if (location !== null) {
415+
headers.set('location', location);
416+
}
417+
return new Response(null, { status, headers });
418+
};
419+
420+
it('should return payload with all fields for error redirect', () => {
421+
const result = rewriteOAuthErrorRedirect(
422+
makeRequest('/auth/callback/google'),
423+
makeRedirectResponse(
424+
'/api/auth/error?error=access_denied&error_description=cancelled&state=abc123',
425+
),
426+
);
427+
428+
expect(result).toEqual({
429+
url: `${process.env.COMMENTS_PREFIX}/callback?error=access_denied&error_description=cancelled&state=abc123`,
430+
provider: 'google',
431+
error: 'access_denied',
432+
errorDescription: 'cancelled',
433+
state: 'abc123',
434+
});
435+
});
436+
437+
it('should return payload when state is state_not_found even without error param', () => {
438+
const result = rewriteOAuthErrorRedirect(
439+
makeRequest('/auth/callback/github'),
440+
makeRedirectResponse('/api/auth/error?state=state_not_found'),
441+
);
442+
443+
expect(result).toEqual({
444+
url: `${process.env.COMMENTS_PREFIX}/callback?state=state_not_found`,
445+
provider: 'github',
446+
error: undefined,
447+
errorDescription: undefined,
448+
state: 'state_not_found',
449+
});
450+
});
451+
452+
it('should return undefined for non-callback paths', () => {
453+
const result = rewriteOAuthErrorRedirect(
454+
makeRequest('/auth/sign-in/social'),
455+
makeRedirectResponse('/api/auth/error?error=access_denied'),
456+
);
457+
458+
expect(result).toBeUndefined();
459+
});
460+
461+
it('should return undefined for non-3xx responses', () => {
462+
const result = rewriteOAuthErrorRedirect(
463+
makeRequest('/auth/callback/google'),
464+
makeRedirectResponse('/api/auth/error?error=access_denied', 200),
465+
);
466+
467+
expect(result).toBeUndefined();
468+
});
469+
470+
it('should return undefined when location header is missing', () => {
471+
const result = rewriteOAuthErrorRedirect(
472+
makeRequest('/auth/callback/google'),
473+
makeRedirectResponse(null),
474+
);
475+
476+
expect(result).toBeUndefined();
477+
});
478+
479+
it('should return undefined when redirect has no error params', () => {
480+
const result = rewriteOAuthErrorRedirect(
481+
makeRequest('/auth/callback/google'),
482+
makeRedirectResponse('/some-other-page?state=success'),
483+
);
484+
485+
expect(result).toBeUndefined();
486+
});
487+
488+
it('should return undefined when location already points at webapp callback', () => {
489+
const result = rewriteOAuthErrorRedirect(
490+
makeRequest('/auth/callback/google'),
491+
makeRedirectResponse(
492+
`${process.env.COMMENTS_PREFIX}/callback?error=access_denied`,
493+
),
494+
);
495+
496+
expect(result).toBeUndefined();
497+
});
498+
});
499+
500+
describe('OAuth callback error rewrite', () => {
501+
const mockBetterAuthRedirect = (location: string) =>
502+
jest.spyOn(betterAuthModule, 'getBetterAuth').mockReturnValue({
503+
handler: async () =>
504+
new Response(null, {
505+
status: 302,
506+
headers: { location },
507+
}),
508+
api: {
509+
getSession: async () => null,
510+
setPassword: async () => ({ status: true }),
511+
},
512+
} as ReturnType<typeof betterAuthModule.getBetterAuth>);
513+
514+
it('should rewrite error redirect to webapp callback with params forwarded', async () => {
515+
const spy = mockBetterAuthRedirect(
516+
'/api/auth/error?error=access_denied&error_description=cancelled&state=abc123',
517+
);
518+
519+
const res = await request(app.server).get('/auth/callback/google');
520+
521+
expect(res.status).toBe(302);
522+
expect(res.headers.location).toBe(
523+
`${process.env.COMMENTS_PREFIX}/callback?error=access_denied&error_description=cancelled&state=abc123`,
524+
);
525+
526+
spy.mockRestore();
527+
});
528+
529+
it('should rewrite when state is state_not_found even without error param', async () => {
530+
const spy = mockBetterAuthRedirect(
531+
'/api/auth/error?state=state_not_found',
532+
);
533+
534+
const res = await request(app.server).get('/auth/callback/github');
535+
536+
expect(res.status).toBe(302);
537+
expect(res.headers.location).toBe(
538+
`${process.env.COMMENTS_PREFIX}/callback?state=state_not_found`,
539+
);
540+
541+
spy.mockRestore();
542+
});
543+
544+
it('should pass through redirect without error params', async () => {
545+
const spy = mockBetterAuthRedirect('/some-other-page?state=success');
546+
547+
const res = await request(app.server).get('/auth/callback/google');
548+
549+
expect(res.status).toBe(302);
550+
expect(res.headers.location).toBe('/some-other-page?state=success');
551+
552+
spy.mockRestore();
553+
});
554+
555+
it('should not rewrite when location already points at webapp callback', async () => {
556+
const originalLocation = `${process.env.COMMENTS_PREFIX}/callback?error=access_denied`;
557+
const spy = mockBetterAuthRedirect(originalLocation);
558+
559+
const res = await request(app.server).get('/auth/callback/google');
560+
561+
expect(res.status).toBe(302);
562+
expect(res.headers.location).toBe(originalLocation);
563+
564+
spy.mockRestore();
565+
});
566+
567+
it('should not rewrite non-callback paths', async () => {
568+
const spy = mockBetterAuthRedirect('/api/auth/error?error=access_denied');
569+
570+
const res = await request(app.server).get('/auth/sign-in/social');
571+
572+
expect(res.status).toBe(302);
573+
expect(res.headers.location).toBe('/api/auth/error?error=access_denied');
574+
575+
spy.mockRestore();
576+
});
577+
});
398578
});

src/routes/betterAuth.ts

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,20 @@ export const logoutBetterAuth = async (
122122
const isOAuthCallbackPath = (url: string): boolean =>
123123
/\/auth\/callback\//.test(url);
124124

125-
const rewriteOAuthErrorRedirect = (
125+
const oauthProviderPattern = /\/auth\/callback\/([^/?]+)/;
126+
127+
export type OAuthErrorRewrite = {
128+
url: string;
129+
provider?: string;
130+
error?: string;
131+
errorDescription?: string;
132+
state?: string;
133+
};
134+
135+
export const rewriteOAuthErrorRedirect = (
126136
request: FastifyRequest,
127137
response: Response,
128-
): string | undefined => {
138+
): OAuthErrorRewrite | undefined => {
129139
if (!isOAuthCallbackPath(request.url)) {
130140
return undefined;
131141
}
@@ -167,7 +177,14 @@ const rewriteOAuthErrorRedirect = (
167177
callbackUrl.searchParams.set(key, value);
168178
});
169179

170-
return callbackUrl.toString();
180+
return {
181+
url: callbackUrl.toString(),
182+
provider: request.url.match(oauthProviderPattern)?.[1],
183+
error: redirectUrl.searchParams.get('error') ?? undefined,
184+
errorDescription:
185+
redirectUrl.searchParams.get('error_description') ?? undefined,
186+
state: redirectUrl.searchParams.get('state') ?? undefined,
187+
};
171188
};
172189

173190
const betterAuthRoute = async (fastify: FastifyInstance): Promise<void> => {
@@ -194,13 +211,20 @@ const betterAuthRoute = async (fastify: FastifyInstance): Promise<void> => {
194211
body,
195212
});
196213

197-
const rewrittenUrl = rewriteOAuthErrorRedirect(request, response);
198-
if (rewrittenUrl) {
214+
const rewrite = rewriteOAuthErrorRedirect(request, response);
215+
if (rewrite) {
199216
request.log.warn(
200-
{ originalLocation: response.headers.get('location') },
217+
{
218+
provider: rewrite.provider,
219+
error: rewrite.error,
220+
errorDescription: rewrite.errorDescription,
221+
state: rewrite.state,
222+
userId: request.userId || request.trackingId,
223+
originalLocation: response.headers.get('location'),
224+
},
201225
'OAuth callback error redirect rewritten to webapp',
202226
);
203-
reply.header('location', rewrittenUrl);
227+
reply.header('location', rewrite.url);
204228
reply.status(302);
205229
return reply.send();
206230
}

0 commit comments

Comments
 (0)