feat(auth): dynamic signup/login ban lists via AWS AppConfig#4911
Conversation
- Move blocked-domain/allowlist/MX gating from env vars into AWS AppConfig (queried at runtime via the AppConfig Data SDK with a 30s in-process cache); env vars remain a fallback for self-hosted/OSS. - Add a new bannedEmails denylist that blocks a specific address at both sign-in and sign-up. - Generic, profile-agnostic AppConfig client so future config (feature flags) reuses the same plumbing; AppConfig client shares the same credential resolution as the S3 client. - Defense in depth: authenticateApiKeyFromHeader now rejects keys belonging to banned users.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Auth hooks and user-create hooks now await that config for domain denylist, login allowlist, and MX signup checks; Adds shared API key auth left-joins the user row and treats keys owned by banned users as invalid (defense in depth). Reviewed by Cursor Bugbot for commit c66a2a4. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
@greptile review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e0c19ea. Configure here.
Greptile SummaryThis PR moves signup/login gating lists (blocked domains, login allowlist, blocked MX hosts) from static env-var parsing into a dynamic AWS AppConfig source with a 30 s in-process cache and stale-while-revalidate, while keeping env vars as a fallback for self-hosted deployments. It also adds a defense-in-depth check in
Confidence Score: 4/5Safe to merge with awareness that the bannedEmails denylist and the OAuth allowlist bypass noted in prior review rounds remain open; those gaps are pre-existing or out of scope for this PR. The AppConfig client correctly handles token rotation, parse errors, concurrent cold fetches, and the NextPollIntervalInSeconds back-off — all issues raised in prior review rounds are addressed. The defense-in-depth banned-user check in the API key path is clean. The blockedMxHosts comparison relies on caller-supplied pre-lowercased values without enforcing this at the comparison site, which is a minor contract gap. The overall change delivers its stated goal reliably, but the open items from prior threads limit full confidence in the feature's stated guarantees. apps/sim/lib/auth/auth.ts and apps/sim/lib/auth/access-control.ts for the open gaps noted in prior review threads; the new AppConfig plumbing files look solid. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant BetterAuth as BetterAuth onRequest
participant AC as getAccessControlConfig
participant AppConfig as AWS AppConfig
participant Cache as In-process Cache
Client->>BetterAuth: POST /sign-in or /sign-up
BetterAuth->>AC: getAccessControlConfig()
AC->>Cache: check entry (loaded?)
alt cold (never polled)
Cache-->>AC: "entry.loaded = false"
AC->>AppConfig: StartConfigurationSession
AppConfig-->>AC: InitialConfigurationToken
AC->>AppConfig: GetLatestConfiguration
AppConfig-->>AC: payload + NextPollConfigurationToken
AC->>Cache: store value, mark loaded, set expiresAt
else warm and fresh
Cache-->>AC: return cached value
else warm and stale (background)
Cache-->>AC: return cached value
AC->>AppConfig: poll in background (async)
end
AC-->>BetterAuth: AccessControlConfig (or env fallback)
BetterAuth->>BetterAuth: allowlist / blockedDomains / MX checks
BetterAuth-->>Client: 200 OK or 403 FORBIDDEN
Reviews (5): Last reviewed commit: "fix(appconfig): preserve session token o..." | Re-trigger Greptile |
Greptile SummaryThis PR moves signup/login gating lists (blocked domains, login allowlists, blocked MX hosts) from static env vars into AWS AppConfig with a 30-second in-process stale-while-revalidate cache, and adds a new
Confidence Score: 3/5The existing auth paths are largely preserved and the env-var fallback is solid, but the new bannedEmails feature has a meaningful coverage gap for OAuth users that should be addressed before relying on it in production. The bannedEmails check in the request middleware only fires when ctx.body?.email is present, which is true for email/password flows but not for OAuth (Google, Microsoft) sign-ins. An existing user whose address is added to the AppConfig bannedEmails list can continue to authenticate via OAuth indefinitely — the databaseHooks guard only covers new registrations. The AppConfig cache client also has a cold-start concurrency gap and resets session tokens on parse errors, but those issues are operationally self-healing. The rest of the change — credential extraction, MX-host list injection, API-key banned-user check — is clean and well-tested. apps/sim/lib/auth/auth.ts (bannedEmails OAuth bypass) and apps/sim/lib/core/config/appconfig.ts (cold-fetch concurrency, token reset on parse error) Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant AuthMiddleware as hooks.before (auth.ts)
participant AccessControl as getAccessControlConfig
participant AppConfig as AWS AppConfig
participant Cache as In-process Cache
participant DBHook as databaseHooks.user.create.before
Client->>AuthMiddleware: POST /sign-in/email or /sign-up/email
AuthMiddleware->>AccessControl: getAccessControlConfig()
AccessControl->>Cache: cache.get(key)
alt "Cache cold (value === null)"
Cache-->>AccessControl: null
AccessControl->>AppConfig: StartConfigurationSession
AppConfig-->>AccessControl: InitialConfigurationToken
AccessControl->>AppConfig: GetLatestConfiguration(token)
AppConfig-->>AccessControl: config JSON + NextToken
AccessControl->>Cache: "cache.set(key, parsed value, TTL=30s)"
else Cache warm
Cache-->>AccessControl: cached value
else Cache stale
Cache-->>AccessControl: cached value (served immediately)
AccessControl->>AppConfig: poll in background (void)
end
AccessControl-->>AuthMiddleware: AccessControlConfig
AuthMiddleware->>AuthMiddleware: check bannedEmails (ctx.body?.email only)
AuthMiddleware->>AuthMiddleware: check allowedLoginEmails/Domains
AuthMiddleware->>AuthMiddleware: check blockedSignupDomains (sign-up)
AuthMiddleware->>AuthMiddleware: validateSignupEmailMx(blockedMxHosts)
AuthMiddleware-->>Client: 403 FORBIDDEN (if blocked)
Note over AuthMiddleware,DBHook: For sign-up, user creation proceeds if hooks.before passes
AuthMiddleware->>DBHook: user.create.before(user)
DBHook->>AccessControl: getAccessControlConfig() [warm cache]
DBHook->>DBHook: check bannedEmails + blockedSignupDomains
DBHook-->>Client: Error (if blocked by DB hook)
Reviews (2): Last reviewed commit: "feat(auth): dynamic signup/login ban lis..." | Re-trigger Greptile |
- Remove bannedEmails sign-in check: better-auth's admin plugin already blocks banned users at sign-in (session.create.before, all providers). bannedEmails is now a signup-only denylist via user.create.before, which also closed the OAuth/email-OTP sign-in bypass the bots flagged. - AppConfig cache: track a 'loaded' flag so an empty/unseeded profile warms the cache instead of re-polling on every request; honor NextPollIntervalInSeconds to avoid throttling; dedupe concurrent cold fetches behind one in-flight poll to avoid racing the rotating session token.
|
Addressed the review findings in 9ee5b1f:
Empty AppConfig never warms cache (Bugbot, medium)
No cold-start dedup (Greptile) All gates green: 30 unit tests, |
|
@greptile review |
- Remove the bannedEmails denylist entirely (better-auth banning + blockedSignupDomains cover the cases). - Move isAppConfigEnabled into feature-flags.ts and gate it on isHosted, so AppConfig is hosted-only; self-hosted/OSS always uses the env-var fallback and never constructs the AWS client.
|
Pushed 19631cf:
All gates green: 30 tests, tsc clean, lint:check clean, check:api-validation:strict passed. Note: the |
|
@greptile review |
Narrow the token-resetting catch to only the network calls. A JSON/parse failure no longer discards the already-rotated session token (the round trip succeeded), so the next poll reuses it instead of opening a new StartConfigurationSession.
|
Fixed in c66a2a4 — valid P1. Narrowed the token-resetting Added a test asserting a parse error doesn't throw, warms the cache, and starts exactly one session. 31 tests, tsc/lint/api-validation all clean. |
|
@greptile review |

Summary
lib/core/config/appconfig.ts) so future config (feature flags) reuses the same session/cache/IAM plumbing.getAwsCredentialsFromEnv), so it works wherever S3 does (ECS task role or explicit keys).authenticateApiKeyFromHeadernow rejects API keys belonging to banned users (banning already deletes keys; this catches any survivor).Pairs with infra PR simstudioai/infra#201 (creates the AppConfig app/env/
access-controlprofile, IAM, and injectsAPPCONFIG_APPLICATION/APPCONFIG_ENVIRONMENT). Falls back to env vars until that's deployed + the profile is seeded.Type of Change
Testing
Tested manually. 29 unit tests pass (new
appconfig,access-control, banned-user API key case, updated MX validation).tsc --noEmitclean, fullbun run lint:checkclean,bun run check:api-validation:strictpassed.Checklist