Skip to content

feat: OAuth middleware with module loaders for identity/secrets/auth config#1220

Closed
pyramation wants to merge 1 commit into
mainfrom
feat/oauth-module-loaders
Closed

feat: OAuth middleware with module loaders for identity/secrets/auth config#1220
pyramation wants to merge 1 commit into
mainfrom
feat/oauth-module-loaders

Conversation

@pyramation
Copy link
Copy Markdown
Contributor

Summary

Refactors the OAuth/SSO middleware from PR #1141 to use the module loader architecture from express-context, addressing the review issues Dan identified.

What changed

New module loaders (packages/express-context/src/loaders/):

  • encryptedSecretsLoader — resolves encrypted_secrets_module from metaschema_modules_public to find the correct schema for decrypting secrets (replaces hardcoded metaschema JOIN)
  • userAuthLoader — resolves user_auth_module for sign_in/sign_up function names and the schema they live in (no longer assumes privateSchema)
  • identityProvidersLoader — resolves identity_providers_module for provider config table location (schema + table name, not hardcoded)

All three are registered in createDefaultRegistry() and added to BuiltinModuleMap for typed useModule() access.

OAuth middleware (graphql/server/src/middleware/oauth.ts):

  • Uses req.constructive.useModule() to resolve all per-database config (identity providers, encrypted secrets, user auth, auth settings) — no manual metaschema queries
  • Uses req.constructive.withPgClient() for RLS-scoped transactions with automatic pgSettings — replaces manual set_config('jwt.claims.*', ..., false) calls
  • Removes express-rate-limit — DB already handles rate limiting via auth_rate_limits / app_settings_rate_limit
  • Uses getNodeEnv() from @pgpmjs/env instead of process.env.NODE_ENV
  • sign_in_identity / sign_up_identity are called in the userAuth.schemaName — not assumed to be in the RLS privateSchema
  • Extracts email_verified from the raw provider profile data (OAuthProfile doesn't expose it directly)

Server wiring (graphql/server/src/server.ts):

  • Passes createDefaultRegistry() as loaders to createContextMiddleware() — enables useModule() on req.constructive
  • Mounts OAuth routes at /auth

Issues addressed from #1141 review

Comment Issue Resolution
#8 getEncryptedSecretsSchema uses fragile metaschema JOIN with privateSchema Replaced with encryptedSecretsLoader querying encrypted_secrets_module by database_id
#9 process.env.NODE_ENV used directly Replaced with getNodeEnv() from @pgpmjs/env
#10-11 express-rate-limit is per-process, redundant with DB rate limiting Removed entirely
#12 Manual set_config('jwt.claims.*') with false (session-level) Uses withPgClient() which applies pgSettings via SET LOCAL (transaction-scoped)
#13-14 Assumes sign_in_identity/sign_up_identity live in privateSchema Resolved via userAuthLoader which finds the actual schema from user_auth_module
#15 Hardcoded OAUTH_STATE_MAX_AGE, requireVerifiedEmail, errorRedirectPath Uses configurable defaults; email_verified extracted from raw profile

Review & Testing Checklist for Human

This is a medium-risk PR since OAuth is a security-sensitive flow and the module loaders are querying real module tables that must exist in the tenant database.

  • Verify the SQL in encryptedSecretsLoader, userAuthLoader, and identityProvidersLoader matches the actual metaschema_modules_public table schemas in the deployed databases (column names, JOINs, WHERE clauses)
  • Confirm identity_providers_module has a private_schema_id column that JOINs to metaschema_public.schema — the loader assumes this
  • Test a full OAuth flow end-to-end: initiate → provider redirect → callback → sign_in_identity → cookie/token set
  • Verify withPgClient() correctly scopes the jwt.claims.user_agent and jwt.claims.origin settings to the transaction (the true flag in set_config means transaction-local)
  • Confirm createDefaultRegistry() is only called once per server instance (it's called in the constructor, so it should be fine)

Notes

Link to Devin session: https://app.devin.ai/sessions/0796902ebeba4f6ea1900d84deab2fc5
Requested by: @pyramation

…config

Replace manual SQL queries and hardcoded schema assumptions in OAuth
middleware with module loaders from express-context:

New loaders:
- encryptedSecretsLoader: resolves encrypted_secrets schema from
  metaschema_modules_public.encrypted_secrets_module
- userAuthLoader: resolves user_auth_module for sign_in/sign_up
  function names and schema (no longer assumes privateSchema)
- identityProvidersLoader: resolves identity_providers_module for
  provider config table location

OAuth middleware changes:
- Uses req.constructive.useModule() for all schema lookups
- Uses req.constructive.withPgClient() for properly scoped RLS
  transactions (replaces manual set_config calls)
- Removes express-rate-limit (DB already handles rate limiting)
- Uses getNodeEnv() instead of process.env.NODE_ENV
- Extracts email_verified from raw provider profile data
- Passes loaders registry to createContextMiddleware in server.ts

Addresses review comments from PR #1141.
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant