✨ Configure email auth by environment variables #1916
Conversation
|
|
|
@ThisIsMissEm is attempting to deploy a commit to the rallly Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Warning Rate limit exceeded@lukevella has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 42 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds environment-controlled feature flags for email login and registration, exposing two env vars and feature flags, gating the email auth provider and related UI, replacing instance-settings-based registration checks with a server helper, updating NextAuth provider wiring, i18n, and docs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant LP as Login Page
participant FF as FeatureFlags (isFeatureEnabled)
participant REG as getRegistrationEnabled
participant IS as InstanceSettings
participant NA as NextAuth
U->>LP: Visit /login
LP->>FF: isFeatureEnabled("emailLogin")
alt emailLogin = true
LP-->>U: Render email login form
LP->>NA: Initialize providers (includes EmailProvider())
else emailLogin = false
LP-->>U: Hide email form
LP-->>U: If no other providers, show "loginNotConfigured"
end
U->>LP: See sign-up option?
LP->>REG: getRegistrationEnabled()
REG->>FF: isFeatureEnabled("registration")
alt registration flag = false
REG-->>LP: false
else registration flag = true
REG->>IS: getInstanceSettings()
IS-->>REG: { disableUserRegistration }
REG-->>LP: !disableUserRegistration
end
LP-->>U: Render Sign up button if true
sequenceDiagram
autonumber
participant NA as NextAuth init
participant FF as FeatureFlags
participant EP as EmailProvider()
participant GP as OtherProviders
NA->>EP: Call EmailProvider()
EP->>FF: isFeatureEnabled("emailLogin")
alt enabled
EP-->>NA: provider object
else disabled
EP-->>NA: null
end
NA->>GP: Initialize Google/Microsoft/OIDC
NA->>NA: providers = [Guest, ...[EP, GP].filter(Boolean)]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@lukevella this is the first of two PRs I'm working on as part of deploying rallly for https://nivenly.org The main thing we noticed was that with the licensing, we couldn't limit registration or email login via environment variables, and additionally we couldn't gate OIDC access by scope (this is a bit more complex), but both of these things made it hard for us to actually enforce the license requirements. |
|
Hi @ThisIsMissEm, can you help me understand how these changes help your organization? Why is disabling the registration page with an environment variable preferred over using the Control Panel? Who benefits from removing the option to login with a OTP/magic link? |
|
@lukevella we generally prefer environment variables for this sort of configuration as it reduces the need to manually go in and configure something after you deploy it. We're wanting to use exclusively SSO for accessing rallly, adding email magic link on top of SSO accounts sidesteps the SSO functionality. That is, the SSO provider handles all the account security requirements, so magic link just isn't useful for us, and in fact downgrades our security. |
lukevella
force-pushed
the
main
branch
2 times, most recently
from
2c3286e to
cf39823
Compare
lukevella
force-pushed
the
feat/configure-email-auth-by-environment-variables
branch
from
47b1248 to
3dc7bba
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements environment variable control for email authentication settings, allowing administrators to disable email login/registration independently through configuration.
- Adds
EMAIL_LOGIN_ENABLEDandREGISTRATION_ENABLEDenvironment variables with default values of "true" - Updates authentication flow to conditionally show email login forms and prevent registration when disabled
- Modifies admin panel to display locked state when registration is controlled by environment variables
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| apps/web/src/env.ts | Defines new environment variables for email login and registration control |
| apps/web/src/lib/feature-flags/config.ts | Integrates environment variables into feature flag system |
| apps/web/src/auth/providers/email.ts | Makes email provider conditional based on feature flag |
| apps/web/src/utils/get-registration-enabled.ts | New utility to check if registration is enabled considering both feature flags and instance settings |
| apps/web/src/app/[locale]/(auth)/login/page.tsx | Updates login page to conditionally show email form and display configuration messages |
| apps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx | Adds UI to show when registration is controlled by environment variable |
| apps/docs/self-hosting/configuration.mdx | Documents the new environment variables |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
apps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/trpc/routers/auth.ts (1)
60-65: Add client‐side handling for disabled registration
The registration UI currently doesn’t catch theUser registration is disabledTRPCError. Wrap yourrequestRegistrationcall in a try/catch (or error boundary), detect theBAD_REQUESTcode or message, map it to the new i18n key (e.g.,registration.disabled), and display a localized, user‐friendly message instead of the raw server string.
♻️ Duplicate comments (1)
apps/web/src/auth/providers/email.ts (1)
9-12: Provider gating via feature flag — good; returns null explicitlyReturning null when disabled matches the factory pattern and avoids ambiguous undefined.
🧹 Nitpick comments (9)
apps/web/src/trpc/routers/auth.ts (1)
60-65: Prefer FORBIDDEN and a machine-readable reason (or localization)400 isn’t ideal for a disabled capability. Use 403 and a stable reason code (client can localize), or fetch a translation on the server.
- const isRegistrationEnabled = await getRegistrationEnabled(); - if (!isRegistrationEnabled) { - throw new TRPCError({ - code: "BAD_REQUEST", - message: "User registration is disabled", - }); - } + const isRegistrationEnabled = await getRegistrationEnabled(); + if (!isRegistrationEnabled) { + throw new TRPCError({ + code: "FORBIDDEN", + message: "registrationDisabled", + }); + }apps/web/public/locales/en/app.json (1)
14-14: Wording nitMore idiomatic phrasing.
- "configuredByEnvironmentVariable": "This setting has been configured by environment variable.", + "configuredByEnvironmentVariable": "This setting is configured via an environment variable.",apps/web/src/next-auth.ts (1)
36-41: Use a typed filter instead of assertionNarrow the type with a user-defined type guard and drop
as Provider[].- ...([ - EmailProvider(), - GoogleProvider(), - OIDCProvider(), - MicrosoftProvider(), - ].filter(Boolean) as Provider[]), + ...( + [EmailProvider(), GoogleProvider(), OIDCProvider(), MicrosoftProvider()].filter( + (p): p is Provider => Boolean(p), + ) + ),apps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx (1)
21-21: Wrap icons with to match design systemPer guidelines, wrap lucide icons with @rallly/ui Icon for sizing/color.
-import { ContainerIcon } from "lucide-react"; +import { ContainerIcon } from "lucide-react"; +import { Icon } from "@rallly/ui/icon";- <Alert variant="note" className="mt-4"> - <ContainerIcon /> + <Alert variant="note" className="mt-4"> + <Icon> + <ContainerIcon /> + </Icon>Also applies to: 123-134
apps/web/src/env.ts (1)
53-55: Optional: coerce to boolean at the schema to avoid string compares downstreamYou can transform these to booleans in env.ts and drop === "true" checks elsewhere.
- EMAIL_LOGIN_ENABLED: z.enum(["true", "false"]).default("true"), - REGISTRATION_ENABLED: z.enum(["true", "false"]).default("true"), + EMAIL_LOGIN_ENABLED: z + .enum(["true", "false"]) + .default("true") + .transform((v) => v === "true"), + REGISTRATION_ENABLED: z + .enum(["true", "false"]) + .default("true") + .transform((v) => v === "true"),If you do this, update usages to treat env.EMAIL_LOGIN_ENABLED/REGISTRATION_ENABLED as boolean.
Also applies to: 121-123
apps/web/src/utils/get-registration-enabled.ts (1)
6-13: Add explicit return type and basic tests (optional)Minor clarity: declare return type. Also worth a small unit test matrix for env/settings combos.
-export async function getRegistrationEnabled() { +export async function getRegistrationEnabled(): Promise<boolean> {I can add tests covering:
- emailLogin=false ⇒ false
- emailLogin=true + registration=false ⇒ false
- both true + instanceSettings.disableUserRegistration=false ⇒ true
- both true + instanceSettings.disableUserRegistration=true ⇒ false
apps/web/src/app/[locale]/(auth)/register/page.tsx (1)
21-26: Parallelize translation and registration checks to shave TTFBFetch both in parallel.
Apply:
- const { t } = await getTranslation(params.locale); - const isRegistrationEnabled = await getRegistrationEnabled(); + const [{ t }, isRegistrationEnabled] = await Promise.all([ + getTranslation(params.locale), + getRegistrationEnabled(), + ]);apps/web/src/auth/providers/email.ts (1)
9-12: Add explicit return type for the provider factoryImproves clarity for call sites and avoids accidental misuse.
Apply:
-export const EmailProvider = () => { +export const EmailProvider = (): + | ReturnType<typeof NodemailerProvider> + | null => {apps/web/src/app/[locale]/(auth)/login/page.tsx (1)
87-100: Minor: avoid rendering an empty providers containerGuard on length to skip an empty grid.
Apply:
- {socialProviders ? ( + {socialProviders.length > 0 ? ( <div className="grid gap-4"> {socialProviders.map((provider) => provider ? ( <SSOProvider key={provider.id} providerId={provider.id} name={provider.options?.name || provider.name} redirectTo={searchParams?.redirectTo} /> ) : null, )} </div> ) : null}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
apps/docs/self-hosting/configuration.mdx(1 hunks)apps/web/public/locales/en/app.json(2 hunks)apps/web/src/app/[locale]/(auth)/login/page.tsx(5 hunks)apps/web/src/app/[locale]/(auth)/register/page.tsx(2 hunks)apps/web/src/app/[locale]/(optional-space)/new/page.tsx(2 hunks)apps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx(6 hunks)apps/web/src/auth/providers/email.ts(1 hunks)apps/web/src/env.ts(2 hunks)apps/web/src/lib/feature-flags/config.ts(1 hunks)apps/web/src/lib/feature-flags/types.ts(1 hunks)apps/web/src/next-auth.ts(1 hunks)apps/web/src/trpc/routers/auth.ts(2 hunks)apps/web/src/utils/get-registration-enabled.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.windsurfrules)
**/*.{js,jsx,ts,tsx}: Use dayjs for date handling
Use tailwindcss for styling
Use react-query for data fetching
Use react-hook-form for form handling
Prefer implicit return values over explicit return values
Use zod for form validation
Prefer double quotes for strings over single quotes
Only add comments when it is necessary to explain code that isn't self-explanatory
Files:
apps/web/src/utils/get-registration-enabled.tsapps/web/src/lib/feature-flags/types.tsapps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/web/src/trpc/routers/auth.tsapps/web/src/env.tsapps/web/src/lib/feature-flags/config.tsapps/web/src/app/[locale]/(auth)/login/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsxapps/web/src/next-auth.tsapps/web/src/auth/providers/email.tsapps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.windsurfrules)
Create separate import statements for types
Files:
apps/web/src/utils/get-registration-enabled.tsapps/web/src/lib/feature-flags/types.tsapps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/web/src/trpc/routers/auth.tsapps/web/src/env.tsapps/web/src/lib/feature-flags/config.tsapps/web/src/app/[locale]/(auth)/login/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsxapps/web/src/next-auth.tsapps/web/src/auth/providers/email.tsapps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx
**/*.ts
📄 CodeRabbit inference engine (.windsurfrules)
On the server use the
getTranslationsfunction from @/i18n/server to get the translations.
Files:
apps/web/src/utils/get-registration-enabled.tsapps/web/src/lib/feature-flags/types.tsapps/web/src/trpc/routers/auth.tsapps/web/src/env.tsapps/web/src/lib/feature-flags/config.tsapps/web/src/next-auth.tsapps/web/src/auth/providers/email.ts
**/*
📄 CodeRabbit inference engine (.windsurfrules)
Always use kebab-case for file names
Files:
apps/web/src/utils/get-registration-enabled.tsapps/web/public/locales/en/app.jsonapps/web/src/lib/feature-flags/types.tsapps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/docs/self-hosting/configuration.mdxapps/web/src/trpc/routers/auth.tsapps/web/src/env.tsapps/web/src/lib/feature-flags/config.tsapps/web/src/app/[locale]/(auth)/login/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsxapps/web/src/next-auth.tsapps/web/src/auth/providers/email.tsapps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Biome for code formatting with 2-space indentation and double quotes
Files:
apps/web/src/utils/get-registration-enabled.tsapps/web/src/lib/feature-flags/types.tsapps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/web/src/trpc/routers/auth.tsapps/web/src/env.tsapps/web/src/lib/feature-flags/config.tsapps/web/src/app/[locale]/(auth)/login/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsxapps/web/src/next-auth.tsapps/web/src/auth/providers/email.tsapps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx
apps/web/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Use the useDialog hook from @rallly/ui/dialog to manage dialog state instead of manual useState
Files:
apps/web/src/utils/get-registration-enabled.tsapps/web/src/lib/feature-flags/types.tsapps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/web/src/trpc/routers/auth.tsapps/web/src/env.tsapps/web/src/lib/feature-flags/config.tsapps/web/src/app/[locale]/(auth)/login/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsxapps/web/src/next-auth.tsapps/web/src/auth/providers/email.tsapps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx
apps/web/src/lib/feature-flags/**
📄 CodeRabbit inference engine (CLAUDE.md)
Store feature flag logic under apps/web/src/lib/feature-flags/
Files:
apps/web/src/lib/feature-flags/types.tsapps/web/src/lib/feature-flags/config.ts
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.windsurfrules)
**/*.{jsx,tsx}: All text in the UI should be translated using either the Trans component or the useTranslation hook
Prefer composable components in the style of shadcn UI over large monolithic components
DropdownMenuItem is a flex container with a preset gap so there is no need to add margins to the children
The size and colour of an icon should be set by wrapping it with the component from @rallly/ui/icon which will give it the correct colour and size.
Keep the props of a component as minimal as possible. Pass only the bare minimum amount of information needed to it.
All text in the UI should be translatable.
Always use a composable patterns when building components
Usecn()from @rallly/ui to compose classes
Prefer using the React module APIs (e.g. React.useState) instead of standalone hooks (e.g. useState)
Files:
apps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/web/src/app/[locale]/(auth)/login/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsxapps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx
**/*.tsx
📄 CodeRabbit inference engine (.windsurfrules)
Add the "use client" directive to the top of any .tsx file that requires client-side javascript
**/*.tsx: Prefer inline prop types for simple React component props instead of named interfaces
Only create named interfaces for component props when the props are reused or complex
Files:
apps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/web/src/app/[locale]/(auth)/login/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsxapps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx
apps/web/src/app/**
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure route handlers follow Next.js App Router conventions
Files:
apps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/web/src/app/[locale]/(auth)/login/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsxapps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx
apps/web/src/trpc/routers/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place tRPC routers under apps/web/src/trpc/routers/
Files:
apps/web/src/trpc/routers/auth.ts
apps/web/src/auth/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep NextAuth.js authentication setup under apps/web/src/auth/
Files:
apps/web/src/auth/providers/email.ts
🧠 Learnings (2)
📚 Learning: 2025-08-18T17:39:22.684Z
Learnt from: CR
PR: lukevella/rallly#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-18T17:39:22.684Z
Learning: Applies to apps/web/src/lib/feature-flags/** : Store feature flag logic under apps/web/src/lib/feature-flags/
Applied to files:
apps/web/src/lib/feature-flags/config.ts
📚 Learning: 2025-08-18T17:39:22.684Z
Learnt from: CR
PR: lukevella/rallly#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-18T17:39:22.684Z
Learning: Applies to apps/web/src/auth/** : Keep NextAuth.js authentication setup under apps/web/src/auth/
Applied to files:
apps/web/src/next-auth.ts
🧬 Code graph analysis (9)
apps/web/src/utils/get-registration-enabled.ts (2)
apps/web/src/lib/feature-flags/server.ts (1)
isFeatureEnabled(5-7)apps/web/src/features/instance-settings/queries.ts (1)
getInstanceSettings(7-27)
apps/web/src/app/[locale]/(optional-space)/new/page.tsx (2)
apps/web/src/next-auth.ts (1)
getLoggedIn(195-198)apps/web/src/utils/get-registration-enabled.ts (1)
getRegistrationEnabled(6-13)
apps/web/src/trpc/routers/auth.ts (1)
apps/web/src/utils/get-registration-enabled.ts (1)
getRegistrationEnabled(6-13)
apps/web/src/lib/feature-flags/config.ts (5)
apps/web/src/env.ts (1)
env(4-143)apps/web/src/lib/feature-flags/types.ts (1)
FeatureFlagConfig(1-7)apps/web/src/lib/storage/constants.ts (1)
isStorageEnabled(3-5)apps/web/src/features/billing/constants.ts (1)
isBillingEnabled(3-3)apps/web/src/features/feedback/constants.ts (1)
isFeedbackEnabled(3-3)
apps/web/src/app/[locale]/(auth)/login/page.tsx (5)
apps/web/src/utils/get-registration-enabled.ts (1)
getRegistrationEnabled(6-13)apps/web/src/i18n/server.ts (1)
getTranslation(8-30)apps/web/src/lib/feature-flags/server.ts (1)
isFeatureEnabled(5-7)apps/web/src/app/[locale]/(auth)/login/components/login-email-form.tsx (1)
LoginWithEmailForm(34-127)apps/web/src/app/[locale]/(auth)/login/components/or-divider.tsx (1)
OrDivider(3-12)
apps/web/src/app/[locale]/(auth)/register/page.tsx (1)
apps/web/src/utils/get-registration-enabled.ts (1)
getRegistrationEnabled(6-13)
apps/web/src/next-auth.ts (4)
apps/web/src/auth/providers/email.ts (1)
EmailProvider(9-49)apps/web/src/auth/providers/google.ts (1)
GoogleProvider(3-11)apps/web/src/auth/providers/oidc.ts (1)
OIDCProvider(7-41)apps/web/src/auth/providers/microsoft.ts (1)
MicrosoftProvider(3-12)
apps/web/src/auth/providers/email.ts (3)
apps/web/src/lib/feature-flags/server.ts (1)
isFeatureEnabled(5-7)apps/web/src/utils/emails.ts (1)
getEmailClient(9-34)packages/utils/src/absolute-url.ts (1)
absoluteUrl(18-38)
apps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx (1)
apps/web/src/lib/feature-flags/client.tsx (1)
useFeatureFlag(26-34)
🔇 Additional comments (17)
apps/web/src/lib/feature-flags/types.ts (1)
5-6: LGTM: new flags fit the modelAdding
emailLoginandregistrationtoFeatureFlagConfigcleanly extendsFeature = keyof FeatureFlagConfig. No issues spotted.apps/web/src/trpc/routers/auth.ts (1)
16-16: LGTM: centralizing registration gateSwitching to
getRegistrationEnabled()keeps the router lean and consistent with feature flags.apps/web/public/locales/en/app.json (1)
41-41: LGTM: concise empty-state copyText is clear and neutral.
apps/docs/self-hosting/configuration.mdx (1)
68-79: Clarify registration applies only to email-based flows, not SSO
Confirmed via code search that REGISTRATION_ENABLED isn’t enforced in NextAuth/OAuth flows and only disables email-based registration; update the ParamField descriptions in apps/docs/self-hosting/configuration.mdx accordingly per the suggested diff.apps/web/src/app/[locale]/(optional-space)/new/page.tsx (2)
11-18: Good switch to server-validated registration gatingUsing getRegistrationEnabled() with Promise.all keeps UI in sync with server rules and caches via getInstanceSettings(). Looks solid.
49-58: Sign-up button correctly gatedRendering Sign up only when isRegistrationEnabled prevents dead links to /register when registration is off.
apps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx (2)
123-123: Confirm Alert variantIs "note" a supported Alert variant in @rallly/ui/alert? If not, switch to "info" to avoid styling fallback.
49-49: Client feature-flag usage looks rightuseFeatureFlag("registration") cleanly reflects env-driven gating in the UI.
apps/web/src/env.ts (1)
53-55: Env flags added with sane defaultsEMAIL_LOGIN_ENABLED and REGISTRATION_ENABLED default to "true" and are server-only. Looks good.
apps/web/src/lib/feature-flags/config.ts (1)
7-16: Flag derivation is consistent with PR intentemailLogin mirrors EMAIL_LOGIN_ENABLED; registration requires both email login and registration env flags. Matches “registration disabled when email login is off.”
apps/web/src/utils/get-registration-enabled.ts (1)
6-13: Server-side source of truth for registration is cleanEarly-return on feature flag and fallback to instance settings is straightforward and cache-friendly via getInstanceSettings().
apps/web/src/app/[locale]/(auth)/register/page.tsx (1)
5-5: Centralized registration gating via server utility — LGTMImport and usage of getRegistrationEnabled with a clear 404 guard reads well and aligns with the new feature-flag flow.
Also applies to: 22-22, 24-26
apps/web/src/auth/providers/email.ts (1)
9-12: Resolved:.filter(Boolean)already drops null providersapps/web/src/app/[locale]/(auth)/login/page.tsx (4)
26-34: Good: concurrent fetch of translation and registration flagPromise.all here is appropriate.
44-46: Server-side gate for email login — LGTMLocal isEmailLoginEnabled via isFeatureEnabled keeps render logic simple.
79-81: Divider gating is correctOnly shows when both email and another provider are present.
61-75: Add missingloginNotConfiguredkey to all non-English locale files
Allapp.jsonfiles underapps/web/public/locales/*excepten/app.jsonare missing theloginNotConfiguredentry—add appropriate translations to prevent fallback to the default string.⛔ Skipped due to learnings
Learnt from: CR PR: lukevella/rallly#0 File: .windsurfrules:0-0 Timestamp: 2025-07-21T13:12:34.705Z Learning: Applies to **/i18n/**/*.json : i18n keys are in camelCase.Learnt from: CR PR: lukevella/rallly#0 File: .windsurfrules:0-0 Timestamp: 2025-07-21T13:12:34.705Z Learning: Applies to **/i18n/**/*.json : Never manually add translations to .json files. This will be handled by our tooling.Learnt from: CR PR: lukevella/rallly#0 File: .windsurfrules:0-0 Timestamp: 2025-07-21T13:12:34.705Z Learning: Applies to **/*.{jsx,tsx} : All text in the UI should be translatable.
apps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx
Outdated
Show resolved
Hide resolved
…nt enabling registration if email login is disabled
lukevella
force-pushed
the
feat/configure-email-auth-by-environment-variables
branch
from
8f6ef00 to
61a0745
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/src/auth/providers/email.ts (1)
22-47: Non-existent user yields silent success; throw to fail sign-in.If user is not found, NextAuth treats the request as ok and shows “check your email” even though nothing is sent. Throw to surface failure.
Apply:
async sendVerificationRequest({ identifier: email, token, url }) { const user = await prisma.user.findUnique({ where: { email, }, select: { name: true, locale: true, }, }); - if (user) { - await getEmailClient(user.locale ?? undefined).sendTemplate( - "LoginEmail", - { - to: email, - props: { - magicLink: absoluteUrl("/auth/login", { - magicLink: url, - }), - code: token, - }, - }, - ); - } + if (!user) { + throw new Error("USER_NOT_FOUND"); + } + + await getEmailClient(user.locale ?? undefined).sendTemplate("LoginEmail", { + to: email, + props: { + magicLink: absoluteUrl("/auth/login", { magicLink: url }), + code: token, + }, + }); },
🧹 Nitpick comments (5)
apps/web/src/auth/providers/email.ts (2)
14-21: Add an explicit return type for EmailProvider to aid inference and DX.-export const EmailProvider = () => { +export const EmailProvider = (): + | ReturnType<typeof NodemailerProvider> + | null => {
16-16: Prefer validated env over process.env for sender address.Keeps env usage consistent with getEmailClient and env.ts validation.
- from: process.env.NOREPLY_EMAIL, + from: env.NOREPLY_EMAIL,Add import:
+import { env } from "@/env";apps/web/src/trpc/routers/auth.ts (1)
60-66: Consider using FORBIDDEN and a translatable reason code.BAD_REQUEST works, but FORBIDDEN better fits policy denial. Also prefer a machine-readable reason to localize on the client.
- throw new TRPCError({ - code: "BAD_REQUEST", - message: "User registration is disabled", - }); + throw new TRPCError({ + code: "FORBIDDEN", + message: "registrationDisabled", + });apps/web/src/utils/get-registration-enabled.ts (1)
6-13: Solid central gate; add explicit return type for clarity.-export async function getRegistrationEnabled() { +export async function getRegistrationEnabled(): Promise<boolean> {apps/web/src/app/[locale]/(auth)/register/page.tsx (1)
22-26: 404 on disabled registration: consider a dedicated UX.Optional: instead of notFound(), render a localized “Registration disabled” page or redirect to /login with a query param so the UI can show a friendly message.
- if (!isRegistrationEnabled) { - return notFound(); - } + if (!isRegistrationEnabled) { + redirect("/login?registrationDisabled=1"); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
apps/docs/self-hosting/configuration.mdx(1 hunks)apps/web/public/locales/en/app.json(2 hunks)apps/web/src/app/[locale]/(auth)/login/page.tsx(5 hunks)apps/web/src/app/[locale]/(auth)/register/page.tsx(2 hunks)apps/web/src/app/[locale]/(optional-space)/new/page.tsx(2 hunks)apps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx(6 hunks)apps/web/src/auth/providers/email.ts(1 hunks)apps/web/src/env.ts(2 hunks)apps/web/src/lib/feature-flags/config.ts(1 hunks)apps/web/src/lib/feature-flags/types.ts(1 hunks)apps/web/src/next-auth.ts(1 hunks)apps/web/src/trpc/routers/auth.ts(2 hunks)apps/web/src/utils/get-registration-enabled.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/web/src/env.ts
- apps/web/src/lib/feature-flags/config.ts
- apps/web/public/locales/en/app.json
- apps/web/src/app/[locale]/control-panel/settings/instance-settings-form.tsx
- apps/web/src/lib/feature-flags/types.ts
- apps/docs/self-hosting/configuration.mdx
- apps/web/src/next-auth.ts
- apps/web/src/app/[locale]/(auth)/login/page.tsx
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.windsurfrules)
**/*.{js,jsx,ts,tsx}: Use dayjs for date handling
Use tailwindcss for styling
Use react-query for data fetching
Use react-hook-form for form handling
Prefer implicit return values over explicit return values
Use zod for form validation
Prefer double quotes for strings over single quotes
Only add comments when it is necessary to explain code that isn't self-explanatory
Files:
apps/web/src/utils/get-registration-enabled.tsapps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsxapps/web/src/auth/providers/email.tsapps/web/src/trpc/routers/auth.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.windsurfrules)
Create separate import statements for types
Files:
apps/web/src/utils/get-registration-enabled.tsapps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsxapps/web/src/auth/providers/email.tsapps/web/src/trpc/routers/auth.ts
**/*.ts
📄 CodeRabbit inference engine (.windsurfrules)
On the server use the
getTranslationsfunction from @/i18n/server to get the translations.
Files:
apps/web/src/utils/get-registration-enabled.tsapps/web/src/auth/providers/email.tsapps/web/src/trpc/routers/auth.ts
**/*
📄 CodeRabbit inference engine (.windsurfrules)
Always use kebab-case for file names
Files:
apps/web/src/utils/get-registration-enabled.tsapps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsxapps/web/src/auth/providers/email.tsapps/web/src/trpc/routers/auth.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Biome for code formatting with 2-space indentation and double quotes
Files:
apps/web/src/utils/get-registration-enabled.tsapps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsxapps/web/src/auth/providers/email.tsapps/web/src/trpc/routers/auth.ts
apps/web/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Use the useDialog hook from @rallly/ui/dialog to manage dialog state instead of manual useState
Files:
apps/web/src/utils/get-registration-enabled.tsapps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsxapps/web/src/auth/providers/email.tsapps/web/src/trpc/routers/auth.ts
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.windsurfrules)
**/*.{jsx,tsx}: All text in the UI should be translated using either the Trans component or the useTranslation hook
Prefer composable components in the style of shadcn UI over large monolithic components
DropdownMenuItem is a flex container with a preset gap so there is no need to add margins to the children
The size and colour of an icon should be set by wrapping it with the component from @rallly/ui/icon which will give it the correct colour and size.
Keep the props of a component as minimal as possible. Pass only the bare minimum amount of information needed to it.
All text in the UI should be translatable.
Always use a composable patterns when building components
Usecn()from @rallly/ui to compose classes
Prefer using the React module APIs (e.g. React.useState) instead of standalone hooks (e.g. useState)
Files:
apps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsx
**/*.tsx
📄 CodeRabbit inference engine (.windsurfrules)
Add the "use client" directive to the top of any .tsx file that requires client-side javascript
**/*.tsx: Prefer inline prop types for simple React component props instead of named interfaces
Only create named interfaces for component props when the props are reused or complex
Files:
apps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsx
apps/web/src/app/**
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure route handlers follow Next.js App Router conventions
Files:
apps/web/src/app/[locale]/(optional-space)/new/page.tsxapps/web/src/app/[locale]/(auth)/register/page.tsx
apps/web/src/auth/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep NextAuth.js authentication setup under apps/web/src/auth/
Files:
apps/web/src/auth/providers/email.ts
apps/web/src/trpc/routers/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place tRPC routers under apps/web/src/trpc/routers/
Files:
apps/web/src/trpc/routers/auth.ts
🧬 Code graph analysis (5)
apps/web/src/utils/get-registration-enabled.ts (2)
apps/web/src/lib/feature-flags/server.ts (1)
isFeatureEnabled(5-7)apps/web/src/features/instance-settings/queries.ts (1)
getInstanceSettings(7-27)
apps/web/src/app/[locale]/(optional-space)/new/page.tsx (2)
apps/web/src/next-auth.ts (1)
getLoggedIn(195-198)apps/web/src/utils/get-registration-enabled.ts (1)
getRegistrationEnabled(6-13)
apps/web/src/app/[locale]/(auth)/register/page.tsx (1)
apps/web/src/utils/get-registration-enabled.ts (1)
getRegistrationEnabled(6-13)
apps/web/src/auth/providers/email.ts (4)
apps/web/src/lib/feature-flags/server.ts (1)
isFeatureEnabled(5-7)packages/utils/src/nanoid.ts (1)
generateOtp(13-13)apps/web/src/utils/emails.ts (1)
getEmailClient(9-34)packages/utils/src/absolute-url.ts (1)
absoluteUrl(18-38)
apps/web/src/trpc/routers/auth.ts (1)
apps/web/src/utils/get-registration-enabled.ts (1)
getRegistrationEnabled(6-13)
🔇 Additional comments (6)
apps/web/src/auth/providers/email.ts (1)
10-12: Provider gating verified
EmailProvider returns null when the feature flag is off, and the NextAuth config applies.filter(Boolean)to its providers array, so falsy entries are correctly removed.apps/web/src/trpc/routers/auth.ts (2)
60-66: Good: centralized registration gate via getRegistrationEnabled().
16-16: No changes needed. Registration is defined asisEmailLoginEnabled && isRegistrationEnabled(config.ts line 16), matching the intended composition so disabling email login also disables registration.apps/web/src/app/[locale]/(optional-space)/new/page.tsx (2)
15-18: LGTM: parallelize fetch with Promise.all.
49-57: LGTM: signup CTA correctly gated by isRegistrationEnabled.apps/web/src/app/[locale]/(auth)/register/page.tsx (1)
5-5: LGTM: uses shared getRegistrationEnabled helper; keeps logic consistent.
lukevella
changed the title
|
Hey @ThisIsMissEm, thanks for your help with this and sorry for the delay. There was an issue that was causing our e2e tests to fail that was not easy to solve. It appears that importing the environment values from I've made a few minor changes and renamed the env vars to I'll merge this and will make a new release shortly. Thank you for your help. |
3 participants
Description
This implements the often requested (based on the discussion forum) feature of being able to disable email login/registration and being able to allow email login but prevent registration via environment variable (previously it was only possible to disable this through the admin panel)
Discussions:
The logic here is:
.default("true")inenv.ts)Notes
.envin the root of the repo symlinked toapps/web/.envfor next.js to pick it up for some reason, when runningpnpm devfrom the root of the project — I'm wondering if turbo is doing something that confuses node'sprocess.cwd()such that the.envfile doesn't load from the root of the repo, butapps/webinstead.Checklist
Please check off all the following items with an "x" in the boxes before requesting a review.
Screenshots
With OIDC enabled and Email login enabled:
With OIDC Enabled and Email registration disabled:
With OIDC Enabled and Email login disabled:
With neither OIDC or Email login enabled:
The admin panel if email registration is disabled (or with email login disabled):
Testing
Whilst I've done extensive manual testing to ensure this all works, I'm not sure how we'd approach automated tests for this.
Summary by CodeRabbit
New Features
Documentation