feat(oauth): add identity pre-check with connectedAccounts loader

theothersideofgod · claude · theothersideofgod · commit 998f892f6288 · 2026-05-26T16:50:03.000+08:00
Replace try/catch IDENTITY_ACCOUNT_NOT_FOUND pattern with explicit identity existence check using connected_accounts table query. Changes: - Add connectedAccountsLoader to resolve connected_accounts_module config - Query connected_accounts via ctx.pool (bypasses RLS) before sign-in - Use if/else instead of exception handling for sign_in vs sign_up flow - Add sessionCredentialsSchemaName to userAuthLoader (JOINs with session_credentials_table_id for correct schema resolution) - Add parseIntervalToSeconds in cookie.ts for PgInterval handling - Export ConnectedAccountsConfig, PgInterval types Addresses reviewer feedback from PR #1141 and #1220. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
diff --git a/graphql/server/src/middleware/cookie.ts b/graphql/server/src/middleware/cookie.ts
@@ -1,11 +1,28 @@
 import type { Request, Response } from 'express';
-import type { AuthSettings } from '../types';
+import type { AuthSettings, PgInterval } from '../types';
 
 export const SESSION_COOKIE_NAME = 'constructive_session';
 export const DEVICE_TOKEN_COOKIE_NAME = 'constructive_device_token';
 
 const DEVICE_TOKEN_MAX_AGE = 90 * 24 * 60 * 60; // 90 days in seconds
 
+const parseIntervalToSeconds = (interval: string | PgInterval | null | undefined): number | null => {
+  if (!interval) return null;
+  if (typeof interval === 'string') {
+    const parsed = parseInt(interval, 10);
+    return isNaN(parsed) ? null : parsed;
+  }
+  let totalSeconds = 0;
+  if (interval.years) totalSeconds += interval.years * 365 * 24 * 60 * 60;
+  if (interval.months) totalSeconds += interval.months * 30 * 24 * 60 * 60;
+  if (interval.days) totalSeconds += interval.days * 24 * 60 * 60;
+  if (interval.hours) totalSeconds += interval.hours * 60 * 60;
+  if (interval.minutes) totalSeconds += interval.minutes * 60;
+  if (interval.seconds) totalSeconds += interval.seconds;
+  if (interval.milliseconds) totalSeconds += interval.milliseconds / 1000;
+  return totalSeconds > 0 ? totalSeconds : null;
+};
+
 export interface CookieConfig {
   secure: boolean;
   sameSite: 'strict' | 'lax' | 'none';
@@ -25,11 +42,11 @@ export const getSessionCookieConfig = (
   const DEFAULT_MAX_AGE = 86400; // 24 hours
   let maxAge = DEFAULT_MAX_AGE;
   if (rememberMe && authSettings?.rememberMeDuration) {
-    const parsed = parseInt(authSettings.rememberMeDuration, 10);
-    if (!isNaN(parsed)) maxAge = parsed;
+    const parsed = parseIntervalToSeconds(authSettings.rememberMeDuration);
+    if (parsed !== null) maxAge = parsed;
   } else if (authSettings?.cookieMaxAge) {
-    const parsed = parseInt(authSettings.cookieMaxAge, 10);
-    if (!isNaN(parsed)) maxAge = parsed;
+    const parsed = parseIntervalToSeconds(authSettings.cookieMaxAge);
+    if (parsed !== null) maxAge = parsed;
   }
 
   return {
diff --git a/graphql/server/src/middleware/oauth.ts b/graphql/server/src/middleware/oauth.ts
@@ -25,6 +25,7 @@ import { getNodeEnv } from '@pgpmjs/env';
 import type { ConstructiveOptions } from '@constructive-io/graphql-types';
 import type {
   AuthSettings,
+  ConnectedAccountsConfig,
   ConstructiveContext,
   EncryptedSecretsConfig,
   IdentityProvidersConfig,
@@ -172,24 +173,26 @@ interface OAuthModules {
   encryptedSecrets: EncryptedSecretsConfig;
   userAuth: UserAuthConfig;
   authSettings: AuthSettings | undefined;
+  connectedAccounts: ConnectedAccountsConfig | undefined;
 }
 
 async function resolveOAuthModules(
   ctx: ConstructiveContext,
 ): Promise<OAuthModules | null> {
-  const [identityProviders, encryptedSecrets, userAuth, authSettings] =
+  const [identityProviders, encryptedSecrets, userAuth, authSettings, connectedAccounts] =
     await Promise.all([
       ctx.useModule('identityProviders'),
       ctx.useModule('encryptedSecrets'),
       ctx.useModule('userAuth'),
       ctx.useModule('authSettings'),
+      ctx.useModule('connectedAccounts'),
     ]);
 
   if (!identityProviders || !encryptedSecrets || !userAuth) {
     return null;
   }
 
-  return { identityProviders, encryptedSecrets, userAuth, authSettings };
+  return { identityProviders, encryptedSecrets, userAuth, authSettings, connectedAccounts };
 }
 
 // =============================================================================
@@ -601,7 +604,39 @@ export function createOAuthRoutes(_opts: ConstructiveOptions): Router {
         }
 
         const userAgent = req.get('user-agent') || '';
-        const { userAuth } = modules;
+        const { identityProviders, connectedAccounts } = modules;
+        const authPrivateSchema = identityProviders.privateSchemaName;
+
+        // Check if identity exists using ctx.pool (bypasses RLS)
+        let identityExists = false;
+        if (connectedAccounts) {
+          const checkSql = `
+            SELECT 1 FROM "${connectedAccounts.privateSchemaName}"."${connectedAccounts.tableName}"
+            WHERE service = $1 AND identifier = $2
+            LIMIT 1
+          `;
+          const checkResult = await ctx.pool.query(checkSql, [
+            profile.provider,
+            profile.providerId,
+          ]);
+          identityExists = checkResult.rows.length > 0;
+        }
+
+        const emailVerified = isEmailVerified(profile);
+
+        // For signup, check email verification requirement before entering transaction
+        if (!identityExists && requireVerifiedEmail && !emailVerified) {
+          log.warn(
+            `[oauth] Rejecting unverified email for signup: ${profile.email}`,
+          );
+          return redirectToError(
+            res,
+            baseUrl,
+            errorRedirectPath,
+            'EMAIL_NOT_VERIFIED',
+            provider,
+          );
+        }
 
         // Use withPgClient to run sign_in/sign_up within a properly scoped
         // RLS transaction. pgSettings (role, claims, request_id) are applied
@@ -615,7 +650,6 @@ export function createOAuthRoutes(_opts: ConstructiveOptions): Router {
               [userAgent, targetOrigin],
             );
 
-            const emailVerified = isEmailVerified(profile);
             const details = {
               provider: profile.provider,
               sub: profile.providerId,
@@ -626,15 +660,13 @@ export function createOAuthRoutes(_opts: ConstructiveOptions): Router {
               raw_userinfo: profile.raw,
             };
 
-            // sign_in_identity lives in the userAuth schema, NOT assumed to
-            // be in the RLS privateSchema.
-            const signInSql = `
-              SELECT * FROM "${userAuth.schemaName}".sign_in_identity(
-                $1::text, $2::text, $3::jsonb, $4::text, 'access_token'::text, $5::boolean, $6::text
-              )
-            `;
-
-            try {
+            if (identityExists) {
+              // Identity exists, sign in
+              const signInSql = `
+                SELECT * FROM "${authPrivateSchema}".sign_in_identity(
+                  $1::text, $2::text, $3::jsonb, $4::text, 'access_token'::text, $5::boolean, $6::text
+                )
+              `;
               const signInResult = await client.query(signInSql, [
                 profile.provider,
                 profile.providerId,
@@ -644,31 +676,17 @@ export function createOAuthRoutes(_opts: ConstructiveOptions): Router {
                 deviceToken,
               ]);
               return signInResult.rows[0] || {};
-            } catch (err: any) {
-              const errorMessage = err.message || '';
-
-              if (!errorMessage.includes('IDENTITY_ACCOUNT_NOT_FOUND')) {
-                throw err;
-              }
-
+            } else {
+              // Identity doesn't exist, sign up
               log.info(
                 `[oauth] Account not found for ${profile.email}, attempting signup`,
               );
 
-              if (requireVerifiedEmail && !emailVerified) {
-                log.warn(
-                  `[oauth] Rejecting unverified email for signup: ${profile.email}`,
-                );
-                return { _error: 'EMAIL_NOT_VERIFIED' } as any;
-              }
-
-              // sign_up_identity also lives in the userAuth schema
               const signUpSql = `
-                SELECT * FROM "${userAuth.schemaName}".sign_up_identity(
+                SELECT * FROM "${authPrivateSchema}".sign_up_identity(
                   $1::text, $2::text, $3::text, $4::jsonb, 'access_token'::text, $5::boolean, $6::text
                 )
               `;
-
               const signUpResult = await client.query(signUpSql, [
                 profile.provider,
                 profile.providerId,
@@ -682,17 +700,6 @@ export function createOAuthRoutes(_opts: ConstructiveOptions): Router {
           },
         );
 
-        // Handle error sentinels from within the transaction
-        if ((result as any)._error === 'EMAIL_NOT_VERIFIED') {
-          return redirectToError(
-            res,
-            baseUrl,
-            errorRedirectPath,
-            'EMAIL_NOT_VERIFIED',
-            provider,
-          );
-        }
-
         // Handle MFA required
         if (result.mfa_required && result.mfa_challenge_token) {
           log.info(`[oauth] MFA required for ${profile.email}`);
diff --git a/graphql/server/src/types.ts b/graphql/server/src/types.ts
@@ -11,6 +11,7 @@ export type {
   CorsModuleData,
   DatabaseSettings,
   GenericModuleData,
+  PgInterval,
   PubkeyChallengeSettings,
   PublicKeyChallengeData,
   RlsModule,
diff --git a/packages/express-context/src/index.ts b/packages/express-context/src/index.ts
@@ -52,8 +52,10 @@ export type {
   GenericModuleData,
   InferenceLogConfig,
   PublicKeyChallengeData,
+  PgInterval,
   PubkeyChallengeSettings,
   RlsModule,
+  ConnectedAccountsConfig,
   EncryptedSecretsConfig,
   IdentityProvidersConfig,
   UserAuthConfig,
@@ -98,6 +100,7 @@ export {
   encryptedSecretsLoader,
   userAuthLoader,
   identityProvidersLoader,
+  connectedAccountsLoader,
 } from './loaders';
 
 // Side-effect: Express type augmentation
diff --git a/packages/express-context/src/loaders/connected-accounts.ts b/packages/express-context/src/loaders/connected-accounts.ts
@@ -0,0 +1,63 @@
+/**
+ * Connected Accounts Module Loader
+ *
+ * Resolves the connected_accounts_module config from metaschema_modules_public.
+ * Provides schema names for querying OAuth identity associations.
+ */
+
+import type { LoaderContext, ModuleLoader } from './types';
+import { createModuleLoader } from './create-loader';
+
+// ─── Types ──────────────────────────────────────────────────────────────────
+
+export interface ConnectedAccountsConfig {
+  schemaName: string;
+  privateSchemaName: string;
+  tableName: string;
+}
+
+// ─── SQL ────────────────────────────────────────────────────────────────────
+
+const CONNECTED_ACCOUNTS_MODULE_SQL = `
+  SELECT
+    s.schema_name,
+    ps.schema_name AS private_schema_name,
+    cam.table_name
+  FROM metaschema_modules_public.connected_accounts_module cam
+  JOIN metaschema_public.schema s ON s.id = cam.schema_id
+  JOIN metaschema_public.schema ps ON ps.id = cam.private_schema_id
+  WHERE cam.database_id = $1
+  LIMIT 1
+`;
+
+// ─── Row Types ──────────────────────────────────────────────────────────────
+
+interface ConnectedAccountsModuleRow {
+  schema_name: string;
+  private_schema_name: string;
+  table_name: string;
+}
+
+// ─── Loader ─────────────────────────────────────────────────────────────────
+
+export const connectedAccountsLoader: ModuleLoader<ConnectedAccountsConfig> =
+  createModuleLoader<ConnectedAccountsConfig>({
+    name: 'connectedAccounts',
+    ttlMs: 5 * 60_000,
+    async resolve(ctx: LoaderContext) {
+      const { tenantPool, databaseId } = ctx;
+
+      const result = await tenantPool.query<ConnectedAccountsModuleRow>(
+        CONNECTED_ACCOUNTS_MODULE_SQL,
+        [databaseId],
+      );
+      const row = result.rows[0];
+      if (!row) return undefined;
+
+      return {
+        schemaName: row.schema_name,
+        privateSchemaName: row.private_schema_name,
+        tableName: row.table_name,
+      };
+    },
+  });
diff --git a/packages/express-context/src/loaders/index.ts b/packages/express-context/src/loaders/index.ts
@@ -15,6 +15,7 @@
  *   - encryptedSecrets (metaschema_modules_public.config_secrets_org_module)
  *   - userAuth        (metaschema_modules_public.user_auth_module)
  *   - identityProviders (metaschema_modules_public.identity_providers_module)
+ *   - connectedAccounts (metaschema_modules_public.connected_accounts_module)
  *
  * To add a new per-db lookup, implement a ModuleLoader and register it:
  *
@@ -53,6 +54,7 @@ export { agentChatLoader } from './agent-chat';
 export { encryptedSecretsLoader } from './encrypted-secrets';
 export { userAuthLoader } from './user-auth';
 export { identityProvidersLoader } from './identity-providers';
+export { connectedAccountsLoader } from './connected-accounts';
 
 /**
  * Convenience: create a registry pre-loaded with all built-in loaders.
@@ -70,6 +72,7 @@ import { agentChatLoader } from './agent-chat';
 import { encryptedSecretsLoader } from './encrypted-secrets';
 import { userAuthLoader } from './user-auth';
 import { identityProvidersLoader } from './identity-providers';
+import { connectedAccountsLoader } from './connected-accounts';
 
 export function createDefaultRegistry() {
   const registry = createLoaderRegistry();
@@ -85,5 +88,6 @@ export function createDefaultRegistry() {
   registry.register(encryptedSecretsLoader);
   registry.register(userAuthLoader);
   registry.register(identityProvidersLoader);
+  registry.register(connectedAccountsLoader);
   return registry;
 }
diff --git a/packages/express-context/src/loaders/user-auth.ts b/packages/express-context/src/loaders/user-auth.ts
@@ -13,6 +13,7 @@ import { createModuleLoader } from './create-loader';
 
 export interface UserAuthConfig {
   schemaName: string;
+  sessionCredentialsSchemaName: string;
   signInFunction: string;
   signUpFunction: string;
   signOutFunction: string;
@@ -26,6 +27,7 @@ export interface UserAuthConfig {
 const USER_AUTH_MODULE_SQL = `
   SELECT
     s.schema_name,
+    sc_schema.schema_name AS session_credentials_schema_name,
     uam.sign_in_function,
     uam.sign_up_function,
     uam.sign_out_function,
@@ -34,6 +36,8 @@ const USER_AUTH_MODULE_SQL = `
     uam.extend_token_expires
   FROM metaschema_modules_public.user_auth_module uam
   JOIN metaschema_public.schema s ON s.id = uam.schema_id
+  LEFT JOIN metaschema_public.table sc_table ON sc_table.id = uam.session_credentials_table_id
+  LEFT JOIN metaschema_public.schema sc_schema ON sc_schema.id = sc_table.schema_id
   WHERE uam.database_id = $1
   LIMIT 1
 `;
@@ -42,6 +46,7 @@ const USER_AUTH_MODULE_SQL = `
 
 interface UserAuthModuleRow {
   schema_name: string;
+  session_credentials_schema_name: string | null;
   sign_in_function: string;
   sign_up_function: string;
   sign_out_function: string;
@@ -68,6 +73,8 @@ export const userAuthLoader: ModuleLoader<UserAuthConfig> =
 
       return {
         schemaName: row.schema_name,
+        sessionCredentialsSchemaName:
+          row.session_credentials_schema_name || row.schema_name,
         signInFunction: row.sign_in_function,
         signUpFunction: row.sign_up_function,
         signOutFunction: row.sign_out_function,
diff --git a/packages/express-context/src/types.ts b/packages/express-context/src/types.ts
