From 3fc50df7e983d773357a9c2c64998fce915a9f0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl-Gerhard=20Lindesva=CC=88rd?= Date: Thu, 5 Nov 2026 21:05:36 +0100 Subject: [PATCH 1/2] fix(dashboard): hide sign-up when registration is closed With ALLOW_REGISTRATION=false the login page still linked to sign-up and /onboarding still showed the form, which the API then refused. A new public query, auth.registrationAllowed, answers with the rule the API enforces on sign-up, so the link is hidden and /onboarding redirects to /login whenever that sign-up would be refused. The first user on a fresh install and a visitor with a live invite still get the form. A redirected invite keeps its id on /login, so an existing user can accept it by signing in. Social sign-in stays on /login. Self-hosted installs need no changes: no new environment variables or migrations. --- apps/start/e2e/auth-registration.spec.ts | 70 +++++++++++++++++++ apps/start/src/routes/_login.login.tsx | 36 +++++++--- apps/start/src/routes/_public.onboarding.tsx | 26 ++++--- .../core/src/modules/auth/auth.constants.ts | 5 ++ .../core/src/modules/auth/auth.rpc.test.ts | 67 +++++++++++++++++- packages/core/src/modules/auth/auth.rpc.ts | 15 +++- 6 files changed, 197 insertions(+), 22 deletions(-) create mode 100644 apps/start/e2e/auth-registration.spec.ts diff --git a/apps/start/e2e/auth-registration.spec.ts b/apps/start/e2e/auth-registration.spec.ts new file mode 100644 index 000000000..bff4eb89b --- /dev/null +++ b/apps/start/e2e/auth-registration.spec.ts @@ -0,0 +1,70 @@ +// 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('link', { name: 'Create an account' }) + ).toHaveCount(0); + }); +}); diff --git a/apps/start/src/routes/_login.login.tsx b/apps/start/src/routes/_login.login.tsx index edec6ef94..ea7e07fec 100644 --- a/apps/start/src/routes/_login.login.tsx +++ b/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( 'last-auth-provider', @@ -149,12 +160,17 @@ function LoginPage() { )} -

- New to OpenPanel?{' '} - - Create an account - -

+ {registrationAllowed && ( +

+ New to OpenPanel?{' '} + + Create an account + +

+ )} ); } diff --git a/apps/start/src/routes/_public.onboarding.tsx b/apps/start/src/routes/_public.onboarding.tsx index 226f726ea..09c8d4d5f 100644 --- a/apps/start/src/routes/_public.onboarding.tsx +++ b/apps/start/src/routes/_public.onboarding.tsx @@ -25,17 +25,25 @@ 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 }, - }); + const registrationAllowed = await context.queryClient.ensureQueryData( + context.trpc.auth.registrationAllowed.queryOptions({ + inviteId: search.inviteId, + }) + ); + 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, diff --git a/packages/core/src/modules/auth/auth.constants.ts b/packages/core/src/modules/auth/auth.constants.ts index c224863a8..c29f6a680 100644 --- a/packages/core/src/modules/auth/auth.constants.ts +++ b/packages/core/src/modules/auth/auth.constants.ts @@ -43,6 +43,11 @@ export type ISignInEmail = z.infer; 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, diff --git a/packages/core/src/modules/auth/auth.rpc.test.ts b/packages/core/src/modules/auth/auth.rpc.test.ts index 6639684c8..e196ec0e3 100644 --- a/packages/core/src/modules/auth/auth.rpc.test.ts +++ b/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 = {} +) { + 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(userCount: number, inviteExpiresAt?: Date) { + return { + user: { count: async () => userCount }, + invite: { + findUnique: async () => + inviteExpiresAt ? { expiresAt: inviteExpiresAt } : null, + }, + } as unknown as Partial; +} + +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(5)); + expect(await anon.registrationAllowed({})).toBe(false); +}); + +test('registrationAllowed stays open for the first user', async () => { + const anon = await anonCaller(closedRegistration(), registrationDb(0)); + 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(5, new Date(Date.now() + HOUR_MS)) + ); + const expired = await anonCaller( + closedRegistration(), + registrationDb(5, new Date(Date.now() - HOUR_MS)) + ); + const unknown = await anonCaller(closedRegistration(), registrationDb(5)); + + 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(5)); + const mangledInviteId = 'x'.repeat(MAX_ID + 1); + expect(await anon.registrationAllowed({ inviteId: mangledInviteId })).toBe( + false + ); +}); diff --git a/packages/core/src/modules/auth/auth.rpc.ts b/packages/core/src/modules/auth/auth.rpc.ts index ff30c4e56..a47b60d5e 100644 --- a/packages/core/src/modules/auth/auth.rpc.ts +++ b/packages/core/src/modules/auth/auth.rpc.ts @@ -9,7 +9,11 @@ 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, @@ -17,6 +21,7 @@ import { 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); From f13f9f7373ca6024f8202ac2b619511c5c9a6cd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl-Gerhard=20Lindesva=CC=88rd?= Date: Thu, 5 Nov 2026 21:28:21 +0100 Subject: [PATCH 2/2] fix(dashboard): keep login working on a malformed invite link An invite id holding a NUL character made the registration check throw in Postgres, which took /login to the error page. Such an id now counts as an invite that does not exist. The sign-up gate on /onboarding always reads a fresh answer, so an invite that expired since the last check no longer shows a form that would be refused. The first-user check asks whether any user exists rather than counting them all. --- apps/start/e2e/auth-registration.spec.ts | 3 +++ apps/start/src/routes/_public.onboarding.tsx | 10 +++++--- .../core/src/modules/auth/auth.rpc.test.ts | 16 ++++++------ .../src/modules/auth/src/registration.test.ts | 25 ++++++++++++++----- .../core/src/modules/auth/src/registration.ts | 11 ++++++-- 5 files changed, 45 insertions(+), 20 deletions(-) diff --git a/apps/start/e2e/auth-registration.spec.ts b/apps/start/e2e/auth-registration.spec.ts index bff4eb89b..302ae2b99 100644 --- a/apps/start/e2e/auth-registration.spec.ts +++ b/apps/start/e2e/auth-registration.spec.ts @@ -63,6 +63,9 @@ test.describe('registration closed', () => { 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); diff --git a/apps/start/src/routes/_public.onboarding.tsx b/apps/start/src/routes/_public.onboarding.tsx index 09c8d4d5f..62d49ef15 100644 --- a/apps/start/src/routes/_public.onboarding.tsx +++ b/apps/start/src/routes/_public.onboarding.tsx @@ -35,11 +35,13 @@ export const Route = createFileRoute('/_public/onboarding')({ } throw redirect({ to: '/' }); } - const registrationAllowed = await context.queryClient.ensureQueryData( - context.trpc.auth.registrationAllowed.queryOptions({ + // 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 } }); diff --git a/packages/core/src/modules/auth/auth.rpc.test.ts b/packages/core/src/modules/auth/auth.rpc.test.ts index e196ec0e3..c86290f2e 100644 --- a/packages/core/src/modules/auth/auth.rpc.test.ts +++ b/packages/core/src/modules/auth/auth.rpc.test.ts @@ -167,9 +167,9 @@ function closedRegistration() { return testCoreConfig({ auth: { ...base.auth, allowRegistration: false } }); } -function registrationDb(userCount: number, inviteExpiresAt?: Date) { +function registrationDb(hasUsers: boolean, inviteExpiresAt?: Date) { return { - user: { count: async () => userCount }, + user: { findFirst: async () => (hasUsers ? { id: 'user_1' } : null) }, invite: { findUnique: async () => inviteExpiresAt ? { expiresAt: inviteExpiresAt } : null, @@ -183,12 +183,12 @@ test('registrationAllowed is open when ALLOW_REGISTRATION is unset', async () => }); test('registrationAllowed is closed to a visitor without an invite', async () => { - const anon = await anonCaller(closedRegistration(), registrationDb(5)); + 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(0)); + const anon = await anonCaller(closedRegistration(), registrationDb(false)); expect(await anon.registrationAllowed({})).toBe(true); }); @@ -196,13 +196,13 @@ test('registrationAllowed opens for a live invite only', async () => { const HOUR_MS = 60 * 60 * 1000; const live = await anonCaller( closedRegistration(), - registrationDb(5, new Date(Date.now() + HOUR_MS)) + registrationDb(true, new Date(Date.now() + HOUR_MS)) ); const expired = await anonCaller( closedRegistration(), - registrationDb(5, new Date(Date.now() - HOUR_MS)) + registrationDb(true, new Date(Date.now() - HOUR_MS)) ); - const unknown = await anonCaller(closedRegistration(), registrationDb(5)); + 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); @@ -210,7 +210,7 @@ test('registrationAllowed opens for a live invite only', async () => { }); test('registrationAllowed answers no to a mangled invite id', async () => { - const anon = await anonCaller(closedRegistration(), registrationDb(5)); + const anon = await anonCaller(closedRegistration(), registrationDb(true)); const mangledInviteId = 'x'.repeat(MAX_ID + 1); expect(await anon.registrationAllowed({ inviteId: mangledInviteId })).toBe( false diff --git a/packages/core/src/modules/auth/src/registration.test.ts b/packages/core/src/modules/auth/src/registration.test.ts index 93ab6778f..7f1d191d8 100644 --- a/packages/core/src/modules/auth/src/registration.test.ts +++ b/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,7 +24,7 @@ function depsWith(auth: { const base = testCoreConfig(); return { db: { - user: { count: mockUserCount }, + user: { findFirst: mockFirstUser }, invite: { findUnique: mockInviteFindUnique }, }, config: testCoreConfig({ auth: { ...base.auth, ...auth } }), @@ -29,21 +32,21 @@ function depsWith(auth: { } 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(); + }); }); diff --git a/packages/core/src/modules/auth/src/registration.ts b/packages/core/src/modules/auth/src/registration.ts index 016617adb..6f7a78d61 100644 --- a/packages/core/src/modules/auth/src/registration.ts +++ b/packages/core/src/modules/auth/src/registration.ts @@ -2,6 +2,8 @@ import type { ServiceDeps } from '../../../services'; type RegistrationDeps = Pick; +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,