fix(dashboard): hide sign-up when registration is closed

apps/start/e2e/auth-registration.spec.ts

@@ -0,0 +1,73 @@
+// What a signed-out visitor is offered with registration open and closed. The
+// closed half needs an api started with ALLOW_REGISTRATION=false and at least
+// one user, and this run's shell carrying the same variable.
+
+import { FRESH_CONTEXT, gotoHydrated, SHOTS_DIR } from './auth-helpers';
+import { expect, test } from './fixtures';
+
+test.use({ storageState: FRESH_CONTEXT });
+// The dev server hydrates slowly when several suites share it.
+test.describe.configure({ timeout: 120_000 });
+
+const REGISTRATION_CLOSED = process.env.ALLOW_REGISTRATION === 'false';
+const UNKNOWN_INVITE_ID = 'not-a-real-invite';
+
+test.describe('registration open', () => {
+  test.skip(REGISTRATION_CLOSED, 'needs an api with registration open');
+
+  test('login links to sign-up, and sign-up shows its form', async ({
+    page,
+  }) => {
+    await gotoHydrated(page, '/login');
+    await page.getByRole('link', { name: 'Create an account' }).click();
+
+    await expect(page).toHaveURL(/\/onboarding$/);
+    await expect(
+      page.getByRole('heading', { name: 'Create your account' })
+    ).toBeVisible();
+  });
+});
+
+test.describe('registration closed', () => {
+  test.skip(!REGISTRATION_CLOSED, 'needs ALLOW_REGISTRATION=false');
+
+  test('login offers no sign-up and keeps the sign-in options', async ({
+    page,
+  }) => {
+    await gotoHydrated(page, '/login');
+
+    await expect(
+      page.getByRole('heading', { name: 'Welcome back' })
+    ).toBeVisible();
+    await expect(page.getByLabel('Email')).toBeVisible();
+    await expect(
+      page.getByRole('link', { name: 'Create an account' })
+    ).toHaveCount(0);
+    await page.screenshot({ path: `${SHOTS_DIR}/registration-closed.png` });
+  });
+
+  test('sign-up redirects to login', async ({ page }) => {
+    await page.goto('/onboarding');
+
+    await expect(page).toHaveURL(/\/login$/);
+    await expect(
+      page.getByRole('heading', { name: 'Create your account' })
+    ).toHaveCount(0);
+  });
+
+  test('an invite that cannot register lands on login and keeps its id', async ({
+    page,
+  }) => {
+    await page.goto(`/invite/${UNKNOWN_INVITE_ID}`);
+
+    await expect(page).toHaveURL(
+      new RegExp(`/login\\?inviteId=${UNKNOWN_INVITE_ID}$`)
+    );
+    await expect(
+      page.getByRole('heading', { name: 'Welcome back' })
+    ).toBeVisible();
+    await expect(
+      page.getByRole('link', { name: 'Create an account' })
+    ).toHaveCount(0);
+  });
+});

apps/start/src/routes/_login.login.tsx

@@ -46,10 +46,18 @@ export const Route = createFileRoute('/_login/login')({
     correlationId: z.string().optional(),
     inviteId: z.string().optional(),
   }),
-  loader: async ({ context }) => {
-    await context.queryClient.ensureQueryData(
-      context.trpc.auth.providers.queryOptions()
-    );
+  loaderDeps: ({ search }) => ({ inviteId: search.inviteId }),
+  loader: async ({ context, deps }) => {
+    await Promise.all([
+      context.queryClient.ensureQueryData(
+        context.trpc.auth.providers.queryOptions()
+      ),
+      context.queryClient.ensureQueryData(
+        context.trpc.auth.registrationAllowed.queryOptions({
+          inviteId: deps.inviteId,
+        })
+      ),
+    ]);
   },
 });
 
@@ -64,6 +72,9 @@ function LoginPage() {
   const { data: providers } = useSuspenseQuery(
     trpc.auth.providers.queryOptions()
   );
+  const { data: registrationAllowed } = useSuspenseQuery(
+    trpc.auth.registrationAllowed.queryOptions({ inviteId })
+  );
   const hasOAuthProviders = providers.google || providers.github;
   const [lastProvider] = useCookieStore<null | string>(
     'last-auth-provider',
@@ -149,12 +160,17 @@ function LoginPage() {
         </>
       )}
 
-      <p className="text-center text-muted-foreground text-sm">
-        New to OpenPanel?{' '}
-        <a className="font-medium text-foreground underline" href={signUpHref}>
-          Create an account
-        </a>
-      </p>
+      {registrationAllowed && (
+        <p className="text-center text-muted-foreground text-sm">
+          New to OpenPanel?{' '}
+          <a
+            className="font-medium text-foreground underline"
+            href={signUpHref}
+          >
+            Create an account
+          </a>
+        </p>
+      )}
     </div>
   );
 }

apps/start/src/routes/_public.onboarding.tsx

@@ -25,17 +25,27 @@ export const Route = createFileRoute('/_public/onboarding')({
     ],
   }),
   beforeLoad: async ({ context, search }) => {
-    if (!(await isSignedIn(context))) {
-      return;
+    if (await isSignedIn(context)) {
+      // A signed-in user opening an invite link accepts it explicitly there.
+      if (search.inviteId) {
+        throw redirect({
+          to: '/invite/$inviteId',
+          params: { inviteId: search.inviteId },
+        });
+      }
+      throw redirect({ to: '/' });
     }
-    // A signed-in user opening an invite link accepts it explicitly there.
-    if (search.inviteId) {
-      throw redirect({
-        to: '/invite/$inviteId',
-        params: { inviteId: search.inviteId },
-      });
+    // Always fresh: a cached answer outlives an invite that has since expired.
+    const registrationAllowed = await context.queryClient.fetchQuery({
+      ...context.trpc.auth.registrationAllowed.queryOptions({
+        inviteId: search.inviteId,
+      }),
+      staleTime: 0,
+    });
+    if (!registrationAllowed) {
+      // Keeps the invite: an existing user accepts it by signing in.
+      throw redirect({ to: '/login', search: { inviteId: search.inviteId } });
     }
-    throw redirect({ to: '/' });
   },
   component: Component,
   validateSearch,

packages/core/src/modules/auth/auth.constants.ts

@@ -43,6 +43,11 @@ export type ISignInEmail = z.infer<typeof zSignInEmail>;
 
 export const zSignInDev = z.object({ inviteId: zShortId.nullish() });
 
+/** Unbounded: a mangled invite link is a "no", it must not break the login page. */
+export const zRegistrationAllowed = z.object({
+  inviteId: z.string().optional(),
+});
+
 export const zSignUpEmail = z.object({
   firstName: zPersonName,
   lastName: zPersonName,

packages/core/src/modules/auth/auth.rpc.test.ts

@@ -3,8 +3,10 @@
 import { expect, test } from 'bun:test';
 import { testCoreConfig } from '../../../test/config-fixture';
 import { stubHttpCtx, TEST_SESSION } from '../../../test/rpc-fixtures';
+import type { HttpCtx } from '../../context';
 import { makeTrpcContext } from '../../rpc/base';
 import type { CookieOptions } from '../../shared/cookie';
+import { MAX_ID } from '../../shared/limits.constants';
 import { authRouter } from './auth.rpc';
 
 const COOKIE_OPTIONS: CookieOptions = {
@@ -18,8 +20,14 @@ const COOKIE_OPTIONS: CookieOptions = {
 // `TrpcContext.session` is never literally `null`, only its `userId` is.
 const EMPTY_SESSION = { session: null, user: null, userId: null };
 
-async function anonCaller(config = testCoreConfig()) {
-  const { ctx } = stubHttpCtx({ config }, EMPTY_SESSION);
+async function anonCaller(
+  config = testCoreConfig(),
+  db: Partial<HttpCtx['db']> = {}
+) {
+  const { ctx } = stubHttpCtx(
+    { config, db: db as HttpCtx['db'] },
+    EMPTY_SESSION
+  );
   const trpcCtx = await makeTrpcContext(ctx, new Headers(), {
     cookieOptions: COOKIE_OPTIONS,
   });
@@ -153,3 +161,58 @@ test('session never hands the session token to the browser', async () => {
   expect(result.userId).toBe(TEST_SESSION.userId);
   expect(JSON.stringify(result)).not.toContain('cookie-value');
 });
+
+function closedRegistration() {
+  const base = testCoreConfig();
+  return testCoreConfig({ auth: { ...base.auth, allowRegistration: false } });
+}
+
+function registrationDb(hasUsers: boolean, inviteExpiresAt?: Date) {
+  return {
+    user: { findFirst: async () => (hasUsers ? { id: 'user_1' } : null) },
+    invite: {
+      findUnique: async () =>
+        inviteExpiresAt ? { expiresAt: inviteExpiresAt } : null,
+    },
+  } as unknown as Partial<HttpCtx['db']>;
+}
+
+test('registrationAllowed is open when ALLOW_REGISTRATION is unset', async () => {
+  const anon = await anonCaller();
+  expect(await anon.registrationAllowed({})).toBe(true);
+});
+
+test('registrationAllowed is closed to a visitor without an invite', async () => {
+  const anon = await anonCaller(closedRegistration(), registrationDb(true));
+  expect(await anon.registrationAllowed({})).toBe(false);
+});
+
+test('registrationAllowed stays open for the first user', async () => {
+  const anon = await anonCaller(closedRegistration(), registrationDb(false));
+  expect(await anon.registrationAllowed({})).toBe(true);
+});
+
+test('registrationAllowed opens for a live invite only', async () => {
+  const HOUR_MS = 60 * 60 * 1000;
+  const live = await anonCaller(
+    closedRegistration(),
+    registrationDb(true, new Date(Date.now() + HOUR_MS))
+  );
+  const expired = await anonCaller(
+    closedRegistration(),
+    registrationDb(true, new Date(Date.now() - HOUR_MS))
+  );
+  const unknown = await anonCaller(closedRegistration(), registrationDb(true));
+
+  expect(await live.registrationAllowed({ inviteId: 'inv_1' })).toBe(true);
+  expect(await expired.registrationAllowed({ inviteId: 'inv_1' })).toBe(false);
+  expect(await unknown.registrationAllowed({ inviteId: 'inv_1' })).toBe(false);
+});
+
+test('registrationAllowed answers no to a mangled invite id', async () => {
+  const anon = await anonCaller(closedRegistration(), registrationDb(true));
+  const mangledInviteId = 'x'.repeat(MAX_ID + 1);
+  expect(await anon.registrationAllowed({ inviteId: mangledInviteId })).toBe(
+    false
+  );
+});

packages/core/src/modules/auth/auth.rpc.ts

@@ -9,14 +9,19 @@ import {
   publicProcedure,
 } from '../../rpc/base';
 import { enforceRateLimit } from '../../rpc/rate-limit';
-import { zSignInDev, zSignInShare } from './auth.constants';
+import {
+  zRegistrationAllowed,
+  zSignInDev,
+  zSignInShare,
+} from './auth.constants';
 import {
   getConfiguredProviders,
   getTwoFactorStatus,
   signInToShare,
   signInWithDevLogin,
   signOutUser,
 } from './auth.service';
+import { getIsRegistrationAllowed } from './src/registration';
 
 const rateLimit = createRateLimitMiddleware(enforceRateLimit);
 
@@ -39,6 +44,14 @@ export const authRouter = createTRPCRouter({
     getConfiguredProviders(ctx.config)
   ),
 
+  /**
+   * Whether the dashboard should offer sign-up to this visitor. The same rule
+   * the sign-up itself is held to, so an invite that can register sees the form.
+   */
+  registrationAllowed: publicProcedure
+    .input(zRegistrationAllowed)
+    .query(({ input, ctx }) => getIsRegistrationAllowed(ctx, input.inviteId)),
+
   /** Ends either kind of session and clears both cookies. */
   signOut: publicProcedure.mutation(async ({ ctx }) => {
     await signOutUser(ctx, ctx.setCookie, ctx.session?.session?.id);

packages/core/src/modules/auth/src/registration.test.ts

@@ -4,7 +4,10 @@ import type { ServiceDeps } from '../../../services';
 import { getIsRegistrationAllowed } from './registration';
 
 const HOUR_MS = 60 * 60 * 1000;
-const mockUserCount = mock(async () => 5);
+const EXISTING_USER = { id: 'user_1' };
+const mockFirstUser = mock(
+  async (): Promise<{ id: string } | null> => EXISTING_USER
+);
 const mockInviteFindUnique = mock(
   async (): Promise<{ id: string; expiresAt: Date } | null> => null
 );
@@ -21,29 +24,29 @@ function depsWith(auth: {
   const base = testCoreConfig();
   return {
     db: {
-      user: { count: mockUserCount },
+      user: { findFirst: mockFirstUser },
       invite: { findUnique: mockInviteFindUnique },
     },
     config: testCoreConfig({ auth: { ...base.auth, ...auth } }),
   } as unknown as ServiceDeps;
 }
 
 beforeEach(() => {
-  mockUserCount.mockClear();
+  mockFirstUser.mockClear();
   mockInviteFindUnique.mockClear();
   // Not the first user unless a test says otherwise
-  mockUserCount.mockResolvedValue(5);
+  mockFirstUser.mockResolvedValue(EXISTING_USER);
   mockInviteFindUnique.mockResolvedValue(null);
 });
 
 describe('getIsRegistrationAllowed', () => {
   it('allows everything in cloud (ALLOW_REGISTRATION unset)', async () => {
     expect(await getIsRegistrationAllowed(depsWith({}))).toBe(true);
-    expect(mockUserCount).not.toHaveBeenCalled();
+    expect(mockFirstUser).not.toHaveBeenCalled();
   });
 
   it('allows the very first user even when registration is disabled', async () => {
-    mockUserCount.mockResolvedValue(0);
+    mockFirstUser.mockResolvedValue(null);
 
     expect(
       await getIsRegistrationAllowed(depsWith({ allowRegistration: false }))
@@ -115,4 +118,14 @@ describe('getIsRegistrationAllowed', () => {
       await getIsRegistrationAllowed(depsWith({ allowRegistration: true }))
     ).toBe(true);
   });
+
+  it('refuses a NUL in an invite id without asking the database', async () => {
+    expect(
+      await getIsRegistrationAllowed(
+        depsWith({ allowRegistration: false, allowInvitation: true }),
+        'inv\0ite'
+      )
+    ).toBe(false);
+    expect(mockInviteFindUnique).not.toHaveBeenCalled();
+  });
 });

packages/core/src/modules/auth/src/registration.ts

@@ -2,6 +2,8 @@ import type { ServiceDeps } from '../../../services';
 
 type RegistrationDeps = Pick<ServiceDeps, 'db' | 'config'>;
 
+const NUL = '\0';
+
 /**
  * Whether a *new* user may be created right now.
  *
@@ -20,8 +22,8 @@ export async function getIsRegistrationAllowed(
   }
 
   // 1. First user is always allowed
-  const count = await deps.db.user.count();
-  if (count === 0) {
+  const anyUser = await deps.db.user.findFirst({ select: { id: true } });
+  if (!anyUser) {
     return true;
   }
 
@@ -31,6 +33,11 @@ export async function getIsRegistrationAllowed(
       return false;
     }
 
+    // Postgres text cannot hold NUL: no invite matches, and the lookup would throw.
+    if (inviteId.includes(NUL)) {
+      return false;
+    }
+
     const invite = await deps.db.invite.findUnique({
       where: {
         id: inviteId,