Skip to content

Commit b450bd0

Browse files
feat(graphql-server): identity pre-check via connectedAccounts query
Replace try/catch IDENTITY_ACCOUNT_NOT_FOUND pattern with explicit identity existence check. Query connected_accounts table using ctx.pool (bypasses RLS) before entering the sign-in transaction. Benefits: - Cleaner control flow (if/else instead of exception handling) - Email verification check happens before transaction starts - Avoids transaction abort on expected "not found" case Addresses reviewer feedback from PR #1141 and #1220. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 496ed7a commit b450bd0

1 file changed

Lines changed: 48 additions & 41 deletions

File tree

graphql/server/src/middleware/oauth.ts

Lines changed: 48 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { getNodeEnv } from '@pgpmjs/env';
2525
import type { ConstructiveOptions } from '@constructive-io/graphql-types';
2626
import type {
2727
AuthSettings,
28+
ConnectedAccountsConfig,
2829
ConstructiveContext,
2930
EncryptedSecretsConfig,
3031
IdentityProvidersConfig,
@@ -172,24 +173,26 @@ interface OAuthModules {
172173
encryptedSecrets: EncryptedSecretsConfig;
173174
userAuth: UserAuthConfig;
174175
authSettings: AuthSettings | undefined;
176+
connectedAccounts: ConnectedAccountsConfig | undefined;
175177
}
176178

177179
async function resolveOAuthModules(
178180
ctx: ConstructiveContext,
179181
): Promise<OAuthModules | null> {
180-
const [identityProviders, encryptedSecrets, userAuth, authSettings] =
182+
const [identityProviders, encryptedSecrets, userAuth, authSettings, connectedAccounts] =
181183
await Promise.all([
182184
ctx.useModule('identityProviders'),
183185
ctx.useModule('encryptedSecrets'),
184186
ctx.useModule('userAuth'),
185187
ctx.useModule('authSettings'),
188+
ctx.useModule('connectedAccounts'),
186189
]);
187190

188191
if (!identityProviders || !encryptedSecrets || !userAuth) {
189192
return null;
190193
}
191194

192-
return { identityProviders, encryptedSecrets, userAuth, authSettings };
195+
return { identityProviders, encryptedSecrets, userAuth, authSettings, connectedAccounts };
193196
}
194197

195198
// =============================================================================
@@ -601,7 +604,39 @@ export function createOAuthRoutes(_opts: ConstructiveOptions): Router {
601604
}
602605

603606
const userAgent = req.get('user-agent') || '';
604-
const { userAuth } = modules;
607+
const { identityProviders, connectedAccounts } = modules;
608+
const authPrivateSchema = identityProviders.privateSchemaName;
609+
610+
// Check if identity exists using ctx.pool (bypasses RLS)
611+
let identityExists = false;
612+
if (connectedAccounts) {
613+
const checkSql = `
614+
SELECT 1 FROM "${connectedAccounts.privateSchemaName}"."${connectedAccounts.tableName}"
615+
WHERE service = $1 AND identifier = $2
616+
LIMIT 1
617+
`;
618+
const checkResult = await ctx.pool.query(checkSql, [
619+
profile.provider,
620+
profile.providerId,
621+
]);
622+
identityExists = checkResult.rows.length > 0;
623+
}
624+
625+
const emailVerified = isEmailVerified(profile);
626+
627+
// For signup, check email verification requirement before entering transaction
628+
if (!identityExists && requireVerifiedEmail && !emailVerified) {
629+
log.warn(
630+
`[oauth] Rejecting unverified email for signup: ${profile.email}`,
631+
);
632+
return redirectToError(
633+
res,
634+
baseUrl,
635+
errorRedirectPath,
636+
'EMAIL_NOT_VERIFIED',
637+
provider,
638+
);
639+
}
605640

606641
// Use withPgClient to run sign_in/sign_up within a properly scoped
607642
// RLS transaction. pgSettings (role, claims, request_id) are applied
@@ -615,7 +650,6 @@ export function createOAuthRoutes(_opts: ConstructiveOptions): Router {
615650
[userAgent, targetOrigin],
616651
);
617652

618-
const emailVerified = isEmailVerified(profile);
619653
const details = {
620654
provider: profile.provider,
621655
sub: profile.providerId,
@@ -626,15 +660,13 @@ export function createOAuthRoutes(_opts: ConstructiveOptions): Router {
626660
raw_userinfo: profile.raw,
627661
};
628662

629-
// sign_in_identity lives in the userAuth schema, NOT assumed to
630-
// be in the RLS privateSchema.
631-
const signInSql = `
632-
SELECT * FROM "${userAuth.schemaName}".sign_in_identity(
633-
$1::text, $2::text, $3::jsonb, $4::text, 'access_token'::text, $5::boolean, $6::text
634-
)
635-
`;
636-
637-
try {
663+
if (identityExists) {
664+
// Identity exists, sign in
665+
const signInSql = `
666+
SELECT * FROM "${authPrivateSchema}".sign_in_identity(
667+
$1::text, $2::text, $3::jsonb, $4::text, 'access_token'::text, $5::boolean, $6::text
668+
)
669+
`;
638670
const signInResult = await client.query(signInSql, [
639671
profile.provider,
640672
profile.providerId,
@@ -644,31 +676,17 @@ export function createOAuthRoutes(_opts: ConstructiveOptions): Router {
644676
deviceToken,
645677
]);
646678
return signInResult.rows[0] || {};
647-
} catch (err: any) {
648-
const errorMessage = err.message || '';
649-
650-
if (!errorMessage.includes('IDENTITY_ACCOUNT_NOT_FOUND')) {
651-
throw err;
652-
}
653-
679+
} else {
680+
// Identity doesn't exist, sign up
654681
log.info(
655682
`[oauth] Account not found for ${profile.email}, attempting signup`,
656683
);
657684

658-
if (requireVerifiedEmail && !emailVerified) {
659-
log.warn(
660-
`[oauth] Rejecting unverified email for signup: ${profile.email}`,
661-
);
662-
return { _error: 'EMAIL_NOT_VERIFIED' } as any;
663-
}
664-
665-
// sign_up_identity also lives in the userAuth schema
666685
const signUpSql = `
667-
SELECT * FROM "${userAuth.schemaName}".sign_up_identity(
686+
SELECT * FROM "${authPrivateSchema}".sign_up_identity(
668687
$1::text, $2::text, $3::text, $4::jsonb, 'access_token'::text, $5::boolean, $6::text
669688
)
670689
`;
671-
672690
const signUpResult = await client.query(signUpSql, [
673691
profile.provider,
674692
profile.providerId,
@@ -682,17 +700,6 @@ export function createOAuthRoutes(_opts: ConstructiveOptions): Router {
682700
},
683701
);
684702

685-
// Handle error sentinels from within the transaction
686-
if ((result as any)._error === 'EMAIL_NOT_VERIFIED') {
687-
return redirectToError(
688-
res,
689-
baseUrl,
690-
errorRedirectPath,
691-
'EMAIL_NOT_VERIFIED',
692-
provider,
693-
);
694-
}
695-
696703
// Handle MFA required
697704
if (result.mfa_required && result.mfa_challenge_token) {
698705
log.info(`[oauth] MFA required for ${profile.email}`);

0 commit comments

Comments
 (0)