diff --git a/apps/api/src/controllers/oauth-callback.controller.tsx b/apps/api/src/controllers/oauth-callback.controller.tsx index 46d789150..ff311a352 100644 --- a/apps/api/src/controllers/oauth-callback.controller.tsx +++ b/apps/api/src/controllers/oauth-callback.controller.tsx @@ -8,7 +8,12 @@ import { setLastAuthProviderCookie, setSessionTokenCookie, } from '@openpanel/auth'; -import { type Account, connectUserToOrganization, db } from '@openpanel/db'; +import { + type Account, + connectUserToOrganization, + db, + getIsRegistrationAllowed, +} from '@openpanel/db'; import type { FastifyReply, FastifyRequest } from 'fastify'; import { z } from 'zod'; import { LogError } from '@/utils/errors'; @@ -132,6 +137,20 @@ async function handleNewUser({ ); } + // Enforce the self-hosting registration policy here rather than before the + // IdP redirect — this is the first point where we know the user is new, so + // returning users are never caught by it. + if (!(await getIsRegistrationAllowed(inviteId))) { + // Deliberately no `oauthUser` here — this rejects people who are not users, + // so their email and name shouldn't land in application logs. The redirect + // carries `correlationId` (the request id), which is what ties a user's + // error page back to this log line if an operator needs to investigate. + throw new LogError('Registrations are not allowed', { + providerName, + inviteId, + }); + } + const user = await db.user.create({ data: { email: oauthUser.email, diff --git a/packages/db/index.ts b/packages/db/index.ts index 727ea5663..15c8ccea3 100644 --- a/packages/db/index.ts +++ b/packages/db/index.ts @@ -31,6 +31,7 @@ export * from './src/services/profile.service'; export * from './src/services/project.service'; export * from './src/services/reference.service'; export * from './src/services/referrer-spikes.service'; +export * from './src/services/registration.service'; export * from './src/services/reports.service'; export * from './src/services/retention.service'; export * from './src/services/salt.service'; diff --git a/packages/db/src/services/registration.service.test.ts b/packages/db/src/services/registration.service.test.ts new file mode 100644 index 000000000..2283ce4d2 --- /dev/null +++ b/packages/db/src/services/registration.service.test.ts @@ -0,0 +1,80 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const mockUserCount = vi.hoisted(() => vi.fn()); +const mockInviteFindUnique = vi.hoisted(() => vi.fn()); + +vi.mock('../prisma-client', () => ({ + db: { + user: { count: mockUserCount }, + invite: { findUnique: mockInviteFindUnique }, + }, +})); + +import { getIsRegistrationAllowed } from './registration.service'; + +const ORIGINAL_ENV = { ...process.env }; + +beforeEach(() => { + vi.clearAllMocks(); + // Not the first user unless a test says otherwise + mockUserCount.mockResolvedValue(5); + mockInviteFindUnique.mockResolvedValue(null); +}); + +afterEach(() => { + process.env = { ...ORIGINAL_ENV }; +}); + +describe('getIsRegistrationAllowed', () => { + it('allows everything in cloud (ALLOW_REGISTRATION unset)', async () => { + process.env.ALLOW_REGISTRATION = undefined; + delete process.env.ALLOW_REGISTRATION; + + await expect(getIsRegistrationAllowed()).resolves.toBe(true); + expect(mockUserCount).not.toHaveBeenCalled(); + }); + + it('allows the very first user even when registration is disabled', async () => { + process.env.ALLOW_REGISTRATION = 'false'; + mockUserCount.mockResolvedValue(0); + + await expect(getIsRegistrationAllowed()).resolves.toBe(true); + }); + + it('blocks a new user with no invite when registration is disabled', async () => { + process.env.ALLOW_REGISTRATION = 'false'; + + await expect(getIsRegistrationAllowed()).resolves.toBe(false); + }); + + it('allows a new user holding a valid invite when registration is disabled', async () => { + process.env.ALLOW_REGISTRATION = 'false'; + process.env.ALLOW_INVITATION = 'true'; + mockInviteFindUnique.mockResolvedValue({ id: 'invite-1' }); + + await expect(getIsRegistrationAllowed('invite-1')).resolves.toBe(true); + }); + + it('blocks an unknown invite id', async () => { + process.env.ALLOW_REGISTRATION = 'false'; + process.env.ALLOW_INVITATION = 'true'; + mockInviteFindUnique.mockResolvedValue(null); + + await expect(getIsRegistrationAllowed('nope')).resolves.toBe(false); + }); + + it('blocks a valid invite when invitations are disabled', async () => { + process.env.ALLOW_REGISTRATION = 'false'; + process.env.ALLOW_INVITATION = 'false'; + mockInviteFindUnique.mockResolvedValue({ id: 'invite-1' }); + + await expect(getIsRegistrationAllowed('invite-1')).resolves.toBe(false); + expect(mockInviteFindUnique).not.toHaveBeenCalled(); + }); + + it('allows open self-hosted registration', async () => { + process.env.ALLOW_REGISTRATION = 'true'; + + await expect(getIsRegistrationAllowed()).resolves.toBe(true); + }); +}); diff --git a/packages/db/src/services/registration.service.ts b/packages/db/src/services/registration.service.ts new file mode 100644 index 000000000..434254a0e --- /dev/null +++ b/packages/db/src/services/registration.service.ts @@ -0,0 +1,41 @@ +import { db } from '../prisma-client'; + +/** + * Whether a *new* user may be created right now. + * + * This must only be consulted at the point where we know the account does not + * exist yet. Calling it before an identity is known (e.g. when kicking off an + * OAuth redirect) would reject returning users too, since we cannot tell a + * sign-in from a sign-up at that stage. + */ +export async function getIsRegistrationAllowed(inviteId?: string | null) { + // ALLOW_REGISTRATION is always undefined in cloud + if (process.env.ALLOW_REGISTRATION === undefined) { + return true; + } + + // Self-hosting logic + // 1. First user is always allowed + const count = await db.user.count(); + if (count === 0) { + return true; + } + + // 2. If there is an invite, check if it is valid + if (inviteId) { + if (process.env.ALLOW_INVITATION === 'false') { + return false; + } + + const invite = await db.invite.findUnique({ + where: { + id: inviteId, + }, + }); + + return !!invite; + } + + // 3. Otherwise, check if general registration is allowed + return process.env.ALLOW_REGISTRATION !== 'false'; +} diff --git a/packages/trpc/src/routers/auth.ts b/packages/trpc/src/routers/auth.ts index 1442d7630..e765bfdbb 100644 --- a/packages/trpc/src/routers/auth.ts +++ b/packages/trpc/src/routers/auth.ts @@ -26,6 +26,7 @@ import { db, decrypt, encrypt, + getIsRegistrationAllowed, getShareOverviewById, getUserAccount, } from '@openpanel/db'; @@ -75,38 +76,6 @@ async function consumeInviteForUser( } } -async function getIsRegistrationAllowed(inviteId?: string | null) { - // ALLOW_REGISTRATION is always undefined in cloud - if (process.env.ALLOW_REGISTRATION === undefined) { - return true; - } - - // Self-hosting logic - // 1. First user is always allowed - const count = await db.user.count(); - if (count === 0) { - return true; - } - - // 2. If there is an invite, check if it is valid - if (inviteId) { - if (process.env.ALLOW_INVITATION === 'false') { - return false; - } - - const invite = await db.invite.findUnique({ - where: { - id: inviteId, - }, - }); - - return !!invite; - } - - // 3. Otherwise, check if general registration is allowed - return process.env.ALLOW_REGISTRATION !== 'false'; -} - export const authRouter = createTRPCRouter({ signOut: publicProcedure.mutation(async ({ ctx }) => { deleteSessionTokenCookie(ctx.setCookie); @@ -117,14 +86,11 @@ export const authRouter = createTRPCRouter({ signInOAuth: publicProcedure .input(z.object({ provider: zProvider, inviteId: z.string().nullish() })) .mutation(async ({ input, ctx }) => { - const isRegistrationAllowed = await getIsRegistrationAllowed( - input.inviteId - ); - - if (!isRegistrationAllowed) { - throw new TRPCAccessError('Registrations are not allowed'); - } - + // NOTE: no registration check here. At this point we have no identity for + // the caller — the IdP hasn't been hit yet — so we cannot tell a returning + // user from a new sign-up. Gating here locks out every existing OAuth user + // as soon as their session expires. The check lives in the OAuth callback + // (`handleNewUser`), which is the only place we know the user is new. const { provider } = input; if (input.inviteId) {