feat(oauth): OAuth identity sign-in with cross-origin token exchange

[theothersideofgod]

Summary

Implements OAuth identity sign-in with support for GitHub/Google/Apple providers, cross-origin token exchange, and multi-tenant support.

Based on #749 (Cookie/CSRF infrastructure).

Architecture

Frontend → Auth Server → OAuth Provider
    │           │
    │ signInCrossOrigin (cross-origin)
    │ cookie (same-origin)
    ▼           ▼
  API ←──── PostgreSQL (sign_in_identity)

Commits

Cookie/CSRF Infrastructure (#749)

Commit	Description
7f9b7a4	Cookie utility module
960eefc	CSRF protection (Double Submit Cookie)
2f1f37b	rememberMeDuration setting
d133c29	AuthCookiePlugin for grafserv response interception
e393e6b	Wire into middleware chain
b1a0c73	Return 403 for CSRF errors
b1190bc	Fix cookie setting
c3310d2	Read device token cookie
9b0e35f	Device token unit tests

OAuth Sign-In (#750)

Commit	Description
5be361f	Add emailVerified field tracking
6c752b6	OAuth routes wiring to sign_in_identity
9c147f6	Middleware and multi-tenant tests
563c5b9	Read provider config from database
d5646e7	Multi-tenant baseUrl support
dda188a	Fix route conflicts and parameter order
d200ab5	Cookie/Token mode mutual exclusion + JWT context
b22a513	Require OAUTH_SECRET in all environments

Key Design

1. Cross-Origin vs Same-Origin

• Cross-origin: Returns one-time token → exchange via signInCrossOrigin for access_token
• Same-origin: Sets HttpOnly cookie directly

2. Security

• State signing (HMAC-SHA256) for CSRF protection
• OAUTH_SECRET environment variable required
• Shadow attack defense: reject unverified emails for auto-signup

3. Multi-Tenant

• Provider config stored in database (identity_providers table)
• Client secret encrypted (encrypted_secrets)
• BaseUrl derived from request dynamically

Environment Variables

Variable	Required	Description
OAUTH_SECRET	✅	State signing secret

Tests

• oauth.test.ts (939 lines) - middleware, multi-tenant isolation, error handling

🤖 Generated with Claude Code

[devin-ai-integration]

Code Review — OAuth Identity Sign-In

Nice work on the overall architecture here. The secrets handling (encrypted_secrets module for client secrets, HMAC-SHA256 state signing with timing-safe comparison), the database integration (identity provider allowlist guard, proper JWT context on dedicated db client), and the cross-origin vs same-origin token flow are all well-designed.

PR #1117 (Cookie lifecycle & CSRF) has been reviewed and is good to merge. This PR (#1141) needs the following fixes before we can merge it. Once addressed, please rebase onto main so the full CI test suite runs — currently only 2 Socket Security checks have executed on this branch.

---

Must Fix

1. Test env var mismatch — callback tests don't actually validate callback logic

graphql/server/src/middleware/__tests__/oauth.test.ts lines 64, 68:

// Test sets:
process.env.OAUTH_STATE_SECRET = 'test-secret-key-for-testing';

// But the code reads (oauth.ts line 34):
const secret = process.env.OAUTH_SECRET;

Wrong env var name. getStateSecret() throws (caught by verifySignedState's try/catch → returns null), so every callback test using createValidState() fails at state verification instead of testing what it claims to test. The Shadow Attack, MFA, and Multi-Tenancy tests (~600 lines) are all hitting INVALID_STATE rather than exercising the actual callback logic.

Fix: Change OAUTH_STATE_SECRET → OAUTH_SECRET in beforeEach/afterEach.

2. Dead code — standalone callSignInIdentity and callSignUpIdentity

graphql/server/src/middleware/oauth.ts lines 203–298:

These two functions use pool.query() directly, but the actual callback handler (line 510+) uses a dedicated dbClient with JWT context (set_config for user_agent, origin). The standalone functions are never called. If someone refactors and uses them, they'd skip JWT context setup — sessions would have NULL audit fields.

Fix: Delete both functions entirely.

3. Internal error messages leaked in redirect URL

graphql/server/src/middleware/oauth.ts line 648:

errorUrl.searchParams.set('message', error.message || 'Unknown error');

This exposes raw error.message (potentially database errors, function names, or internal details) in the redirect URL visible to the user/attacker. Other error paths in the file correctly use only generic error codes.

Fix: Remove the message param or replace with a generic string.

---

Should Fix (can be in this PR or a fast follow-up)

4. sign_up_identity called without device_token

Line 582–593: The sign_up_identity call doesn't pass the device token. New users signing up via OAuth won't get device tracking on their first session (once devices_module is provisioned). The sign_in_identity call correctly passes it.

5. Untyped config access

Line 321: (opts as any).oauth — the oauth key should be added to ConstructiveOptions so the config is properly typed.

6. expressv4 usage in auth-cookie-plugin.ts

Lines 189, 301: This is fine — expressv4 is Grafserv's internal adapter key name, not an Express version dependency. We're on Express 5.2.1. Just noting so nobody gets confused by it.

---

Follow-Up Items (separate PRs)

7. Device Token — devices_module not provisioned

The middleware plumbing you built for device tokens (reading/writing constructive_device_token cookie, passing device_token to sign_in_identity, checking result.out_device_token) is correct and well-wired. However, the devices_module hasn't been provisioned for the constructive application, so:

• The generated sign_in_identity ignores the device_token input parameter
• There's no OUT out_device_token on the generated function
• result.out_device_token is always undefined
• The device cookie is never actually set

Once devices_module is provisioned and sign_in_identity is regenerated, your middleware code will work as-is without changes.

8. Remember Me — session-cookie duration mismatch

"Remember Me" currently only extends the cookie Max-Age (via remember_me_duration from app_settings_auth). But the DB session still uses default_session_duration (2 weeks). If remember_me_duration is 30 days, the cookie lives 30 days but the session expires in 2 weeks — the user gets silently logged out.

sign_in_identity needs to accept a remember_me flag or custom session duration so the DB session expires_at matches the cookie lifetime. The regular sign_in AST generator already has this wired up — sign_in_identity needs the same treatment.

9. Rate limiting on OAuth initiation

/auth/:provider and /auth/:provider/callback routes don't have their own rate limiting. While sign_in_identity has IP-based rate limiting at the DB level, the initiation route could be used for provider enumeration.

---

What's Done Well

• State signing with HMAC-SHA256, nonce, 10-min expiry, timing-safe comparison
• Client secrets via encrypted_secrets.get() — never in env vars
• Identity provider allowlist guard enforced at the DB level (defence-in-depth)
• JWT context (user_agent, origin) set on dedicated db client before calling sign_in_identity
• Cross-origin one-time token flow vs same-origin HttpOnly cookie flow — clean mutual exclusion
• Shadow attack defense (reject unverified emails for auto-signup)
• emailVerified field added to all OAuth providers (Google, GitHub, Facebook, LinkedIn)
• Multi-tenant isolation via req.api.dbname and privateSchema
• MFA flow support with challenge token redirect
• CSRF correctly skips Bearer auth and anonymous requests

[socket-security]

No dependency changes detected. Learn more about Socket for GitHub.

👍 No dependency changes detected in pull request

[theothersideofgod]

Code Review Response

Thanks for the detailed review @devin-ai-integration!

Must Fix ✅ All Done

#	Issue	Status	Commit
1	Test env var OAUTH_STATE_SECRET → OAUTH_SECRET	✅ Fixed	df61cbea8
2	Dead code callSignInIdentity / callSignUpIdentity	✅ Removed	cc52c0776
3	Internal error message leaked in redirect URL	✅ Fixed	3f8edcf19

Should Fix ✅ All Done

#	Issue	Status	Commit
4	sign_up_identity missing device_token	✅ Fixed	65b0e3eac
5	Untyped (opts as any).oauth	✅ Fixed	f64132306
6	expressv4 naming	ℹ️ N/A	Acknowledged - no change needed

Follow-Up Items ✅ All Done

#	Item	Status	Commit/PR
7	devices_module not provisioned	✅ Done	constructive-db main + PR #1163
8	Remember Me session duration mismatch	✅ Fixed	65b0e3eac
9	Rate limiting on OAuth initiation	✅ Done	65eec9811

Commit Summary

1b0cf827a fix(oauth): correct parameter order for sign_in_identity and sign_up_identity
3150791e6 fix(oauth): use client_secret_id instead of provider id for encrypted_secrets lookup
65b0e3eac fix(oauth): synchronize cookie and session duration with remember_me=true
65eec9811 feat(oauth): add rate limiting to OAuth endpoints
f64132306 refactor(oauth): add typed OAuthRoutesConfig to ConstructiveOptions
3f8edcf19 fix(oauth): remove internal error message from redirect URL
cc52c0776 refactor(oauth): remove unused callSignInIdentity and callSignUpIdentity
df61cbea8 fix(oauth): use correct env var name in tests

---

Ready for rebase onto main and CI run.

[theothersideofgod]

@pyramation Hi, Dan please let devin review this pr again. thanks

[devin-ai-integration]

Follow-Up Review — Verifying Fixes

All 9 items from the original review have been addressed. Nice work on the quick turnaround. Here's the verification:

Must Fix — All Confirmed Fixed

#	Issue	Status
1	Test env var OAUTH_STATE_SECRET → OAUTH_SECRET	Verified — beforeEach now sets process.env.OAUTH_SECRET, createValidState() uses matching key
2	Dead code callSignInIdentity / callSignUpIdentity	Verified — removed entirely, oauth.ts goes straight from SignInIdentityResult interface to generateCrossOriginToken
3	Internal error message leaked in redirect URL	Verified — catch block now uses errorUrl.searchParams.set('error', 'CALLBACK_FAILED') with no message param

Should Fix — All Confirmed Fixed

#	Issue	Status
4	sign_up_identity missing device_token	Verified — signUpSql now has 7 params including $6::text for deviceToken
5	Untyped (opts as any).oauth	Verified — OAuthRoutesConfig interface added, oauth?: OAuthRoutesConfig on ConstructiveOptions
6	expressv4 naming	N/A — acknowledged

Follow-Up Items — All Confirmed Addressed

#	Item	Status
7	devices_module provisioned	Verified — PR #1163 in constructive-db adds device tracking tests
8	Remember Me session duration	Verified — getSessionCookieConfig(authSettings, true) passes rememberMe=true
9	Rate limiting	Verified — oauthInitLimiter (10/min) and oauthCallbackLimiter (30/min) added

Additional Commits (not from original review)

• fix(oauth): use client_secret_id instead of provider id — Correct fix, encrypted_secrets.get() needs the secret row ID, not the provider slug.
• fix(oauth): correct parameter order for sign_in_identity and sign_up_identity — Good catch on parameter ordering.

---

New Issue Found: dbClient double-release

oauth.ts callback handler, inside the IDENTITY_ACCOUNT_NOT_FOUND catch block (around lines 505–521 in the diff):

// Inside catch block:
if (!allowSignup) {
    dbClient.release();  // ← manual release
    return res.redirect(errorUrl.toString());
}
if (requireVerifiedEmail && !profile.emailVerified) {
    dbClient.release();  // ← manual release
    return res.redirect(errorUrl.toString());
}

// ...
} finally {
    dbClient.release();  // ← always runs, even after return
}

JavaScript's finally block executes even when a return is encountered in the try/catch. So on the !allowSignup and !emailVerified paths, dbClient.release() is called twice. With pg, the second release() call returns the client to the pool even though it's already been returned — this can corrupt pool state under concurrent load (the same underlying connection gets checked out to two different requests).

Fix: Remove the two manual dbClient.release() calls. The finally block already handles cleanup on all paths.

[theothersideofgod]

Hi, @pyramation.

Hi, @devin-ai-integration Please review the latest fix (commit de1f282).

Fix for Devin's Follow-Up Review

Commit de1f282c2 addresses the new issue found in the follow-up review:

Issue: dbClient double-release

The catch block had manual dbClient.release() calls before return statements:

if (!allowSignup) {
    dbClient.release();  // ← manual release
    return res.redirect(errorUrl.toString());
}
// ...
} finally {
    dbClient.release();  // ← always runs, even after return
}

JavaScript's finally block executes even when a return is encountered, causing the connection to be released twice. Under concurrent load, this can corrupt pool state (the same connection gets checked out to two different requests).

Fix

Removed the two manual dbClient.release() calls in the catch block. The finally block now handles cleanup on all paths.

---

All tests pass (18/18 in oauth.test.ts).

[theothersideofgod]

Hi, @pyramation

Hi, @devin-ai-integration Please review the latest fix (commit de1f282).

conflict fixed

[pyramation]

I think this must be an old version of constructive-db secrets ?

[pyramation]

we should be able to user our env var tools for this, I think getNodeEnv() from pgpm/env/src/env.ts

[pyramation]

we should look into if this conflicts with our rate limiting, probably good to have this too, but we also rate limit other places. Where does it store this?

[pyramation]

you should not be doing this manually

[pyramation]

this is where we get one of the API's private schemas, that doesn't mean that's where secrets live, that doesn't mean that's where identity lives either.

[pyramation]

const privateSchema = req.api.rlsModule.privateSchema.schemaName;

is this the same privateSchema? Why would you assume sign_in_identity is on there?

[pyramation]

is signup only based on a failed signin? is there any better way to detect?

[devin-ai-integration]

DB Settings Available for OAuth Middleware

constructive-db PR #1301 (now merged) added three new columns to app_settings_auth specifically for the OAuth flow. These should replace the hardcoded constants in oauth.ts. Additionally, several existing settings already cover other hardcoded values.

New settings (from PR #1301) → replace hardcoded constants

Hardcoded Constant	DB Setting (app_settings_auth)	Type	Default
OAUTH_STATE_MAX_AGE = 10 * 60 * 1000	oauth_state_max_age	interval	'10 minutes'
requireVerifiedEmail ?? true	oauth_require_verified_email	boolean	true
errorRedirectPath ?? '/auth/error'	oauth_error_redirect_path	text	'/auth/error'

Existing settings → replace hardcoded values

Hardcoded Value	DB Setting (app_settings_auth)	Notes
allowSignup ?? true	allow_identity_sign_up + allow_sign_up (master switch)	Read from authSettings instead of OAuthRoutesConfig
State cookie httpOnly: true	cookie_httponly	Already exists
State cookie secure: NODE_ENV === 'production'	cookie_secure	Already exists
State cookie sameSite: 'lax'	cookie_samesite	Already exists

express-rate-limit → remove in favor of DB rate limiting

The oauthInitLimiter (10/min) and oauthCallbackLimiter (30/min) use express-rate-limit which is:

• Per-process (not shared across replicas)
• Not tenant-configurable
• Redundant with the existing DB rate limiting (app_settings_rate_limit + auth_ip_rate_limits + auth_rate_limits)

Since sign_in_identity and sign_up_identity already go through the DB rate limiter, the Express middleware can be removed.

Stays as-is

Setting	Why
OAUTH_SECRET (env var)	Server-level HMAC signing secret — correct as env var
OAUTH_STATE_COOKIE = 'oauth_state'	Internal implementation detail, not tenant-configurable

How to read these settings

The settings are available via req.api.authSettings in the middleware, which is the singleton row from app_settings_auth. For example:

const authSettings = req.api.authSettings;
const stateMaxAge = authSettings.oauth_state_max_age;  // interval as string, e.g. '00:10:00'
const requireVerified = authSettings.oauth_require_verified_email;
const errorPath = authSettings.oauth_error_redirect_path;
const allowSignup = authSettings.allow_identity_sign_up;
const cookieSecure = authSettings.cookie_secure;
// etc.

[devin-ai-integration]

This PR has been superseded by #1220 which rewrites the OAuth middleware to use module loaders from express-context.

Key changes in #1220:

• New module loaders: encryptedSecretsLoader, userAuthLoader, identityProvidersLoader — resolve schemas from metaschema_modules_public by database_id instead of hardcoding schema assumptions
• req.constructive.useModule() for all per-database config lookups
• req.constructive.withPgClient() for properly scoped RLS transactions (replaces manual set_config calls)
• Removed express-rate-limit — DB handles rate limiting
• getNodeEnv() instead of process.env.NODE_ENV
• sign_in_identity / sign_up_identity called in userAuth.schemaName — not assumed to be in the RLS privateSchema