From 34ac35992561aa19147c01b6bc9a9502c1d43a2b Mon Sep 17 00:00:00 2001 From: "detail-app[bot]" <180357370+detail-app[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 02:49:16 +0000 Subject: [PATCH] fix(sso): enforce global domain uniqueness on provider update --- .../sso/sso-domain-uniqueness.test.ts | 159 ++++++++++++++++++ .../server/api/routers/proprietary/sso.ts | 40 +---- .../server/src/services/proprietary/sso.ts | 40 +++++ 3 files changed, 202 insertions(+), 37 deletions(-) create mode 100644 apps/dokploy/__test__/sso/sso-domain-uniqueness.test.ts diff --git a/apps/dokploy/__test__/sso/sso-domain-uniqueness.test.ts b/apps/dokploy/__test__/sso/sso-domain-uniqueness.test.ts new file mode 100644 index 000000000..d11b6bab1 --- /dev/null +++ b/apps/dokploy/__test__/sso/sso-domain-uniqueness.test.ts @@ -0,0 +1,159 @@ +import { TRPCError } from "@trpc/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// Mock the DB one layer down so the REAL assertSSODomainsGloballyUnique runs +// against controlled provider rows. Mirrors the git-provider-idor.test.ts +// pattern: mocking the exported function would not intercept the internal +// db.query.ssoProvider.findMany call. +const mockDb = vi.hoisted(() => ({ + query: { + ssoProvider: { + findMany: vi.fn(), + }, + }, +})); +vi.mock("@dokploy/server/db", () => ({ db: mockDb })); + +import { assertSSODomainsGloballyUnique } from "@dokploy/server/services/proprietary/sso"; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +// Inputs passed to the guard mirror what the router forwards: `input.domains` +// arrives already lowercased/trimmed/deduped by the ssoProviderBodySchema +// transform, so the guard does not re-normalize the input side. The stored +// `domain` column is a lowercase CSV string (e.g. "acme.com,subsidiary.com"). +const PROVIDER_A = { providerId: "p-org-a", domain: "acme.com,subsidiary.com" }; +const PROVIDER_B = { providerId: "p-org-b", domain: "acme.com" }; + +describe("assertSSODomainsGloballyUnique", () => { + describe("lookup scope", () => { + it("queries ssoProvider globally without an organizationId filter", async () => { + mockDb.query.ssoProvider.findMany.mockResolvedValue([]); + await assertSSODomainsGloballyUnique(["acme.com"], "p-mine"); + const arg = mockDb.query.ssoProvider.findMany.mock.calls?.[0]?.[0]; + expect(arg).toEqual({ + columns: { providerId: true, domain: true }, + }); + // Regression guard: the update guard previously filtered by + // organizationId, which is the bug being fixed. + expect(arg).not.toHaveProperty("where"); + }); + + it("returns the same single findMany call (no per-org re-query)", async () => { + mockDb.query.ssoProvider.findMany.mockResolvedValue([]); + await assertSSODomainsGloballyUnique(["acme.com"], "p-mine"); + expect(mockDb.query.ssoProvider.findMany).toHaveBeenCalledTimes(1); + }); + }); + + describe("update path (excludeProviderId set)", () => { + it("rejects a domain claimed by a provider in a DIFFERENT organization", async () => { + // Org A owns "acme.com,subsidiary.com"; Org B edits its own provider to + // add "acme.com". The global lookup must catch this cross-org collision. + mockDb.query.ssoProvider.findMany.mockResolvedValue([PROVIDER_A]); + await expect( + assertSSODomainsGloballyUnique(["acme.com"], "p-org-b"), + ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + }); + + it("rejects a domain claimed by a provider in the SAME organization", async () => { + mockDb.query.ssoProvider.findMany.mockResolvedValue([PROVIDER_B]); + await expect( + assertSSODomainsGloballyUnique(["acme.com"], "p-other-in-same-org"), + ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + }); + + it("still rejects a different provider's domain even when self is excluded", async () => { + // Editing p-mine (excluded) but p-other in the same org owns acme.com. + mockDb.query.ssoProvider.findMany.mockResolvedValue([ + { providerId: "p-mine", domain: "acme.com" }, + { providerId: "p-other", domain: "acme.com" }, + ]); + await expect( + assertSSODomainsGloballyUnique(["acme.com"], "p-mine"), + ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + }); + + it("skips the provider being edited so keeping its own domains is allowed", async () => { + mockDb.query.ssoProvider.findMany.mockResolvedValue([PROVIDER_B]); + await expect( + assertSSODomainsGloballyUnique(["acme.com"], "p-org-b"), + ).resolves.toBeUndefined(); + }); + + it("matches against any domain in a provider's multi-domain CSV value", async () => { + mockDb.query.ssoProvider.findMany.mockResolvedValue([PROVIDER_A]); + await expect( + assertSSODomainsGloballyUnique(["subsidiary.com"], "p-org-b"), + ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + }); + + it("allows a new domain when the edited provider keeps only unrelated domains", async () => { + mockDb.query.ssoProvider.findMany.mockResolvedValue([ + { providerId: "p-mine", domain: "mine.com" }, + { providerId: "p-other", domain: "other.com" }, + ]); + await expect( + assertSSODomainsGloballyUnique(["fresh.com"], "p-mine"), + ).resolves.toBeUndefined(); + }); + }); + + describe("register path (no excludeProviderId)", () => { + it("rejects a domain already claimed by any provider globally", async () => { + mockDb.query.ssoProvider.findMany.mockResolvedValue([PROVIDER_B]); + await expect( + assertSSODomainsGloballyUnique(["acme.com"]), + ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + }); + + it("rejects when any one of several new domains collides", async () => { + mockDb.query.ssoProvider.findMany.mockResolvedValue([ + { providerId: "p1", domain: "claimed.com" }, + ]); + await expect( + assertSSODomainsGloballyUnique(["fresh.com", "claimed.com"]), + ).rejects.toMatchObject({ code: "BAD_REQUEST" }); + }); + + it("allows all-new domains", async () => { + mockDb.query.ssoProvider.findMany.mockResolvedValue([ + { providerId: "p1", domain: "claimed.com" }, + ]); + await expect( + assertSSODomainsGloballyUnique(["fresh.com", "new.com"]), + ).resolves.toBeUndefined(); + }); + }); + + describe("empty table", () => { + it("returns void when no providers exist", async () => { + mockDb.query.ssoProvider.findMany.mockResolvedValue([]); + await expect( + assertSSODomainsGloballyUnique(["acme.com"], "p-x"), + ).resolves.toBeUndefined(); + }); + }); + + describe("error shape", () => { + it("throws a TRPCError so tRPC maps the HTTP status", async () => { + mockDb.query.ssoProvider.findMany.mockResolvedValue([PROVIDER_B]); + const err = await assertSSODomainsGloballyUnique( + ["acme.com"], + "p-mine", + ).catch((e) => e); + expect(err).toBeInstanceOf(TRPCError); + }); + + it("names the colliding domain in the message", async () => { + mockDb.query.ssoProvider.findMany.mockResolvedValue([PROVIDER_B]); + const err = await assertSSODomainsGloballyUnique( + ["acme.com"], + "p-mine", + ).catch((e) => e); + expect(err.message).toContain("acme.com"); + }); + }); +}); diff --git a/apps/dokploy/server/api/routers/proprietary/sso.ts b/apps/dokploy/server/api/routers/proprietary/sso.ts index 0581ae4db..67b0cf852 100644 --- a/apps/dokploy/server/api/routers/proprietary/sso.ts +++ b/apps/dokploy/server/api/routers/proprietary/sso.ts @@ -4,6 +4,7 @@ import { db } from "@dokploy/server/db"; import { member, ssoProvider, user } from "@dokploy/server/db/schema"; import { ssoProviderBodySchema } from "@dokploy/server/db/schema/sso"; import { + assertSSODomainsGloballyUnique, getOrganizationOwnerId, requestToHeaders, } from "@dokploy/server/index"; @@ -137,25 +138,7 @@ export const ssoRouter = createTRPCRouter({ .where(eq(ssoProvider.id, existing.id)); } - const providers = await db.query.ssoProvider.findMany({ - where: eq(ssoProvider.organizationId, ctx.session.activeOrganizationId), - columns: { providerId: true, domain: true }, - }); - - for (const provider of providers) { - if (provider.providerId === input.providerId) continue; - const providerDomains = provider.domain - .split(",") - .map((d) => d.trim().toLowerCase()); - for (const domain of input.domains) { - if (providerDomains.includes(domain)) { - throw new TRPCError({ - code: "BAD_REQUEST", - message: `Domain ${domain} is already registered for another provider`, - }); - } - } - } + await assertSSODomainsGloballyUnique(input.domains, input.providerId); const issuerChanged = normalizeTrustedOrigin(existing.issuer) !== @@ -263,25 +246,8 @@ export const ssoRouter = createTRPCRouter({ .mutation(async ({ ctx, input }) => { const organizationId = ctx.session.activeOrganizationId; - const providers = await db.query.ssoProvider.findMany({ - columns: { - domain: true, - }, - }); + await assertSSODomainsGloballyUnique(input.domains); - for (const provider of providers) { - const providerDomains = provider.domain - .split(",") - .map((d) => d.trim().toLowerCase()); - for (const domain of input.domains) { - if (providerDomains.includes(domain)) { - throw new TRPCError({ - code: "BAD_REQUEST", - message: `Domain ${domain} is already registered for another provider`, - }); - } - } - } const domain = input.domains.join(","); await auth.registerSSOProvider({ diff --git a/packages/server/src/services/proprietary/sso.ts b/packages/server/src/services/proprietary/sso.ts index 352a042e0..db5ebcc0b 100644 --- a/packages/server/src/services/proprietary/sso.ts +++ b/packages/server/src/services/proprietary/sso.ts @@ -1,5 +1,6 @@ import { db } from "@dokploy/server/db"; import { organization } from "@dokploy/server/db/schema"; +import { TRPCError } from "@trpc/server"; import { eq } from "drizzle-orm"; export const getSSOProviders = async () => { @@ -44,3 +45,42 @@ export const getOrganizationOwnerId = async (organizationId: string) => { if (!org) return null; return org.ownerId; }; + +/** + * Enforces the global uniqueness of SSO email domains across ALL + * organizations. The SSO and social sign-in flows resolve a provider by email + * domain with no organization context, so a single domain must map to at most + * one provider in the whole table — otherwise users can be routed to, or + * provisioned into, the wrong organization's identity provider. + * + * `excludeProviderId` skips the provider being edited so an `update` does not + * collide with itself; `register` omits it because no row exists yet. + */ +export const assertSSODomainsGloballyUnique = async ( + domains: string[], + excludeProviderId?: string, +): Promise => { + const providers = await db.query.ssoProvider.findMany({ + columns: { providerId: true, domain: true }, + }); + + for (const provider of providers) { + if ( + excludeProviderId !== undefined && + provider.providerId === excludeProviderId + ) { + continue; + } + const providerDomains = provider.domain + .split(",") + .map((d) => d.trim().toLowerCase()); + for (const domain of domains) { + if (providerDomains.includes(domain)) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: `Domain ${domain} is already registered for another provider`, + }); + } + } + } +};