diff --git a/.changeset/oauth-profile-error-codes.md b/.changeset/oauth-profile-error-codes.md new file mode 100644 index 0000000..b501269 --- /dev/null +++ b/.changeset/oauth-profile-error-codes.md @@ -0,0 +1,15 @@ +--- +'seamless-auth-api': minor +--- + +Surface actionable OAuth callback failure codes. + +The OAuth callback previously collapsed every profile failure into a generic +`400 { error: 'OAuth login failed' }`, so a user whose provider returned no email +(the most common case, for example a GitHub account with no public email) had no +way to know what to fix. The callback now returns a stable machine-readable `code` +alongside the existing `error` string for the curated, user-actionable cases: +`oauth_missing_email`, `oauth_email_not_verified`, and `oauth_missing_subject`. +Unexpected internal failures still return the generic message with no detail, and +the audit event records the specific reason instead of the blanket +`callback_failed` for the known cases. diff --git a/src/controllers/oauth.ts b/src/controllers/oauth.ts index cdec4de..e943ab6 100644 --- a/src/controllers/oauth.ts +++ b/src/controllers/oauth.ts @@ -18,6 +18,7 @@ import { fetchOAuthProfile, getEnabledOAuthProviders, getOAuthProvider, + OAuthProfileError, resolveOAuthRedirectUri, resolveOAuthUser, serializeOAuthProvider, @@ -157,6 +158,20 @@ export async function finishOAuthLogin(req: Request, res: Response) { }); } catch (error) { logger.error(`OAuth callback failed for provider ${provider.id}: ${error}`); + + if (error instanceof OAuthProfileError) { + await AuthEventService.log({ + type: 'oauth_login_failed', + req, + metadata: { providerId: provider.id, reason: error.code }, + }); + + return res.status(400).json({ + error: error.message, + code: error.code, + }); + } + await AuthEventService.log({ type: 'oauth_login_failed', req, diff --git a/src/routes/oauth.routes.ts b/src/routes/oauth.routes.ts index a10bb8f..9d35f58 100644 --- a/src/routes/oauth.routes.ts +++ b/src/routes/oauth.routes.ts @@ -14,6 +14,7 @@ import { StartOAuthLoginRequestSchema, } from '../schemas/oauth.requests.js'; import { + OAuthLoginErrorResponseSchema, OAuthLoginSuccessResponseSchema, OAuthProvidersResponseSchema, StartOAuthLoginResponseSchema, @@ -65,7 +66,7 @@ oauthRouter.post( body: FinishOAuthLoginRequestSchema, response: { 200: OAuthLoginSuccessResponseSchema, - 400: InternalErrorSchema, + 400: OAuthLoginErrorResponseSchema, 403: InternalErrorSchema, 404: InternalErrorSchema, }, diff --git a/src/schemas/oauth.responses.ts b/src/schemas/oauth.responses.ts index 022e82c..e4085a3 100644 --- a/src/schemas/oauth.responses.ts +++ b/src/schemas/oauth.responses.ts @@ -27,3 +27,11 @@ export const StartOAuthLoginResponseSchema = z.object({ export const OAuthLoginSuccessResponseSchema = RefreshSuccessResponseSchema.omit({ sessionId: true, }); + +export const OAuthLoginErrorResponseSchema = z.object({ + message: z.string().optional(), + error: z.string(), + code: z + .enum(['oauth_missing_email', 'oauth_email_not_verified', 'oauth_missing_subject']) + .optional(), +}); diff --git a/src/services/oauthService.ts b/src/services/oauthService.ts index 14cb6ea..c4dace6 100644 --- a/src/services/oauthService.ts +++ b/src/services/oauthService.ts @@ -36,6 +36,23 @@ export type OAuthProfile = { raw: Record; }; +export type OAuthProfileErrorCode = + | 'oauth_missing_subject' + | 'oauth_missing_email' + | 'oauth_email_not_verified'; + +// Curated, user-actionable profile failures. The controller forwards the code +// to the caller; every other failure stays a bare Error and returns generic. +export class OAuthProfileError extends Error { + readonly code: OAuthProfileErrorCode; + + constructor(code: OAuthProfileErrorCode, message: string) { + super(message); + this.name = 'OAuthProfileError'; + this.code = code; + } +} + function stateSecret() { const explicit = process.env.OAUTH_STATE_SECRET?.trim(); if (explicit) return explicit; @@ -379,11 +396,17 @@ export async function fetchOAuthProfile(provider: OAuthProviderConfig, accessTok const name = getJsonPathValue(raw, provider.nameJsonPath); if (typeof subject !== 'string' && typeof subject !== 'number') { - throw new Error('OAuth profile did not include a provider subject'); + throw new OAuthProfileError( + 'oauth_missing_subject', + 'OAuth profile did not include a provider subject', + ); } if (!email) { - throw new Error('OAuth profile did not include an email address'); + throw new OAuthProfileError( + 'oauth_missing_email', + 'OAuth profile did not include an email address', + ); } const emailVerified = @@ -394,7 +417,7 @@ export async function fetchOAuthProfile(provider: OAuthProviderConfig, accessTok : undefined; if (emailVerified === false || (provider.requireEmailVerified && emailVerified !== true)) { - throw new Error('OAuth profile email is not verified'); + throw new OAuthProfileError('oauth_email_not_verified', 'OAuth profile email is not verified'); } return { diff --git a/tests/integration/oauth/oauth.spec.ts b/tests/integration/oauth/oauth.spec.ts index 782a0bf..ed5a909 100644 --- a/tests/integration/oauth/oauth.spec.ts +++ b/tests/integration/oauth/oauth.spec.ts @@ -302,4 +302,80 @@ describe('OAuth routes', () => { }), ); }); + + const callbackWithProfile = async (profile: Record) => { + const start = await request(app).post('/oauth/google/start').send({ + redirectUri: 'http://localhost:5174/oauth/callback', + }); + + const fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + fetchMock + .mockResolvedValueOnce({ + ok: true, + json: async () => ({ access_token: 'provider-token' }), + }) + .mockResolvedValueOnce({ + ok: true, + json: async () => profile, + }); + + return request(app).post('/oauth/google/callback').send({ + code: 'oauth-code', + state: start.body.state, + }); + }; + + it('surfaces oauth_missing_email when the profile has no email', async () => { + const res = await callbackWithProfile({ sub: 'provider-user', email_verified: true }); + + expect(res.status).toBe(400); + expect(res.body).toEqual({ + error: 'OAuth profile did not include an email address', + code: 'oauth_missing_email', + }); + expect(Session.create).not.toHaveBeenCalled(); + expect(AuthEventService.log).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'oauth_login_failed', + metadata: { providerId: 'google', reason: 'oauth_missing_email' }, + }), + ); + }); + + it('surfaces oauth_email_not_verified when the provider reports an unverified email', async () => { + const res = await callbackWithProfile({ + sub: 'provider-user', + email: 'person@example.com', + email_verified: false, + }); + + expect(res.status).toBe(400); + expect(res.body).toEqual({ + error: 'OAuth profile email is not verified', + code: 'oauth_email_not_verified', + }); + expect(AuthEventService.log).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'oauth_login_failed', + metadata: { providerId: 'google', reason: 'oauth_email_not_verified' }, + }), + ); + }); + + it('surfaces oauth_missing_subject when the profile has no provider subject', async () => { + const res = await callbackWithProfile({ email: 'person@example.com', email_verified: true }); + + expect(res.status).toBe(400); + expect(res.body).toEqual({ + error: 'OAuth profile did not include a provider subject', + code: 'oauth_missing_subject', + }); + expect(AuthEventService.log).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'oauth_login_failed', + metadata: { providerId: 'google', reason: 'oauth_missing_subject' }, + }), + ); + }); });