fix: prevent oidc provider to be named "credential"
This commit is contained in:
parent
3e73b7cc8a
commit
8ce6eb619d
13 changed files with 323 additions and 87 deletions
|
|
@ -90,6 +90,10 @@ export type DeleteSsoProviderErrors = {
|
|||
* Forbidden
|
||||
*/
|
||||
403: unknown;
|
||||
/**
|
||||
* Provider not found
|
||||
*/
|
||||
404: unknown;
|
||||
};
|
||||
|
||||
export type DeleteSsoProviderResponses = {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
CREATE TABLE `sso_provider` (
|
||||
`id` text PRIMARY KEY,
|
||||
`provider_id` text NOT NULL,
|
||||
`provider_id` text NOT NULL UNIQUE,
|
||||
`organization_id` text NOT NULL,
|
||||
`user_id` text,
|
||||
`issuer` text NOT NULL,
|
||||
|
|
@ -13,7 +13,3 @@ CREATE TABLE `sso_provider` (
|
|||
CONSTRAINT `fk_sso_provider_organization_id_organization_id_fk` FOREIGN KEY (`organization_id`) REFERENCES `organization`(`id`) ON DELETE CASCADE,
|
||||
CONSTRAINT `fk_sso_provider_user_id_users_table_id_fk` FOREIGN KEY (`user_id`) REFERENCES `users_table`(`id`) ON DELETE SET NULL
|
||||
);
|
||||
--> statement-breakpoint
|
||||
CREATE UNIQUE INDEX `sso_provider_provider_id_uidx` ON `sso_provider` (`provider_id`);--> statement-breakpoint
|
||||
CREATE INDEX `sso_provider_organization_id_idx` ON `sso_provider` (`organization_id`);--> statement-breakpoint
|
||||
CREATE INDEX `sso_provider_domain_idx` ON `sso_provider` (`domain`);
|
||||
|
|
@ -1,7 +1,7 @@
|
|||
{
|
||||
"version": "7",
|
||||
"dialect": "sqlite",
|
||||
"id": "9fe017f6-0277-40aa-adbc-e165e20796fd",
|
||||
"id": "60a53b0c-395a-4960-9b48-1740388bd2dc",
|
||||
"prevIds": ["3a308c54-d950-464f-9490-fee06985fbeb"],
|
||||
"ddl": [
|
||||
{
|
||||
|
|
@ -2075,48 +2075,6 @@
|
|||
"entityType": "indexes",
|
||||
"table": "sessions_table"
|
||||
},
|
||||
{
|
||||
"columns": [
|
||||
{
|
||||
"value": "provider_id",
|
||||
"isExpression": false
|
||||
}
|
||||
],
|
||||
"isUnique": true,
|
||||
"where": null,
|
||||
"origin": "manual",
|
||||
"name": "sso_provider_provider_id_uidx",
|
||||
"entityType": "indexes",
|
||||
"table": "sso_provider"
|
||||
},
|
||||
{
|
||||
"columns": [
|
||||
{
|
||||
"value": "organization_id",
|
||||
"isExpression": false
|
||||
}
|
||||
],
|
||||
"isUnique": false,
|
||||
"where": null,
|
||||
"origin": "manual",
|
||||
"name": "sso_provider_organization_id_idx",
|
||||
"entityType": "indexes",
|
||||
"table": "sso_provider"
|
||||
},
|
||||
{
|
||||
"columns": [
|
||||
{
|
||||
"value": "domain",
|
||||
"isExpression": false
|
||||
}
|
||||
],
|
||||
"isUnique": false,
|
||||
"where": null,
|
||||
"origin": "manual",
|
||||
"name": "sso_provider_domain_idx",
|
||||
"entityType": "indexes",
|
||||
"table": "sso_provider"
|
||||
},
|
||||
{
|
||||
"columns": [
|
||||
{
|
||||
|
|
@ -2194,6 +2152,13 @@
|
|||
"entityType": "uniques",
|
||||
"table": "sessions_table"
|
||||
},
|
||||
{
|
||||
"columns": ["provider_id"],
|
||||
"nameExplicit": false,
|
||||
"name": "sso_provider_provider_id_unique",
|
||||
"entityType": "uniques",
|
||||
"table": "sso_provider"
|
||||
},
|
||||
{
|
||||
"columns": ["username"],
|
||||
"nameExplicit": false,
|
||||
|
|
@ -173,34 +173,26 @@ export const invitation = sqliteTable(
|
|||
],
|
||||
);
|
||||
|
||||
export const ssoProvider = sqliteTable(
|
||||
"sso_provider",
|
||||
{
|
||||
id: text("id").primaryKey(),
|
||||
providerId: text("provider_id").notNull(),
|
||||
organizationId: text("organization_id")
|
||||
.notNull()
|
||||
.references(() => organization.id, { onDelete: "cascade" }),
|
||||
userId: text("user_id").references(() => usersTable.id, { onDelete: "set null" }),
|
||||
issuer: text("issuer").notNull(),
|
||||
domain: text("domain").notNull(),
|
||||
autoLinkMatchingEmails: int("auto_link_matching_emails", { mode: "boolean" }).notNull().default(false),
|
||||
oidcConfig: text("oidc_config", { mode: "json" }).$type<Record<string, unknown> | null>(),
|
||||
samlConfig: text("saml_config", { mode: "json" }).$type<Record<string, unknown> | null>(),
|
||||
createdAt: integer("created_at", { mode: "timestamp_ms" })
|
||||
.notNull()
|
||||
.default(sql`(unixepoch() * 1000)`),
|
||||
updatedAt: integer("updated_at", { mode: "timestamp_ms" })
|
||||
.notNull()
|
||||
.$onUpdate(() => new Date())
|
||||
.default(sql`(unixepoch() * 1000)`),
|
||||
},
|
||||
(table) => [
|
||||
uniqueIndex("sso_provider_provider_id_uidx").on(table.providerId),
|
||||
index("sso_provider_organization_id_idx").on(table.organizationId),
|
||||
index("sso_provider_domain_idx").on(table.domain),
|
||||
],
|
||||
);
|
||||
export const ssoProvider = sqliteTable("sso_provider", {
|
||||
id: text("id").primaryKey(),
|
||||
providerId: text("provider_id").notNull().unique(),
|
||||
organizationId: text("organization_id")
|
||||
.notNull()
|
||||
.references(() => organization.id, { onDelete: "cascade" }),
|
||||
userId: text("user_id").references(() => usersTable.id, { onDelete: "set null" }),
|
||||
issuer: text("issuer").notNull(),
|
||||
domain: text("domain").notNull(),
|
||||
autoLinkMatchingEmails: int("auto_link_matching_emails", { mode: "boolean" }).notNull().default(false),
|
||||
oidcConfig: text("oidc_config", { mode: "json" }).$type<Record<string, unknown> | null>(),
|
||||
samlConfig: text("saml_config", { mode: "json" }).$type<Record<string, unknown> | null>(),
|
||||
createdAt: integer("created_at", { mode: "timestamp_ms" })
|
||||
.notNull()
|
||||
.default(sql`(unixepoch() * 1000)`),
|
||||
updatedAt: integer("updated_at", { mode: "timestamp_ms" })
|
||||
.notNull()
|
||||
.$onUpdate(() => new Date())
|
||||
.default(sql`(unixepoch() * 1000)`),
|
||||
});
|
||||
|
||||
/**
|
||||
* Volumes Table
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ import { isValidUsername, normalizeUsername } from "~/lib/username";
|
|||
import { ensureOnlyOneUser } from "./auth/middlewares/only-one-user";
|
||||
import { convertLegacyUserOnFirstLogin } from "./auth/middlewares/convert-legacy-user";
|
||||
import { validateSsoCallbackUrls } from "./auth/middlewares/validate-sso-callback-urls";
|
||||
import { validateSsoProviderId } from "./auth/middlewares/validate-sso-provider-id";
|
||||
import { createUserDefaultOrg } from "./auth/helpers/create-default-org";
|
||||
import { isSsoCallbackRequest, requireSsoInvitation } from "./auth/middlewares/require-sso-invitation";
|
||||
import { ssoTrustedProviderLinkingPlugin } from "./auth/plugins/sso-trusted-provider-linking";
|
||||
|
|
@ -37,6 +38,7 @@ export const auth = betterAuth({
|
|||
},
|
||||
hooks: {
|
||||
before: createAuthMiddleware(async (ctx) => {
|
||||
await validateSsoProviderId(ctx);
|
||||
await validateSsoCallbackUrls(ctx);
|
||||
await ensureOnlyOneUser(ctx);
|
||||
await convertLegacyUserOnFirstLogin(ctx);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,42 @@
|
|||
import { describe, expect, test } from "bun:test";
|
||||
import type { AuthMiddlewareContext } from "~/server/lib/auth";
|
||||
import { validateSsoProviderId } from "../validate-sso-provider-id";
|
||||
|
||||
function createContext(path: string, body: Record<string, unknown> = {}): AuthMiddlewareContext {
|
||||
return {
|
||||
path,
|
||||
body,
|
||||
query: {},
|
||||
headers: new Headers(),
|
||||
request: new Request(`http://localhost:3000${path}`),
|
||||
params: {},
|
||||
method: "POST",
|
||||
context: {} as AuthMiddlewareContext["context"],
|
||||
} as AuthMiddlewareContext;
|
||||
}
|
||||
|
||||
describe("validateSsoProviderId", () => {
|
||||
test("allows non-reserved provider id", async () => {
|
||||
const ctx = createContext("/sso/register", { providerId: "acme-oidc" });
|
||||
|
||||
expect(validateSsoProviderId(ctx)).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
test("rejects reserved credential provider id", async () => {
|
||||
const ctx = createContext("/sso/register", { providerId: "credential" });
|
||||
|
||||
expect(validateSsoProviderId(ctx)).rejects.toThrow("reserved");
|
||||
});
|
||||
|
||||
test("rejects reserved credentials provider id case-insensitively", async () => {
|
||||
const ctx = createContext("/sso/register", { providerId: " Credential " });
|
||||
|
||||
expect(validateSsoProviderId(ctx)).rejects.toThrow("reserved");
|
||||
});
|
||||
|
||||
test("skips validation outside register endpoint", async () => {
|
||||
const ctx = createContext("/sign-in/sso", { providerId: "credential" });
|
||||
|
||||
expect(validateSsoProviderId(ctx)).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
25
app/server/lib/auth/middlewares/validate-sso-provider-id.ts
Normal file
25
app/server/lib/auth/middlewares/validate-sso-provider-id.ts
Normal file
|
|
@ -0,0 +1,25 @@
|
|||
import { APIError } from "better-auth/api";
|
||||
import type { AuthMiddlewareContext } from "~/server/lib/auth";
|
||||
import { isReservedSsoProviderId } from "../utils/sso-provider-id";
|
||||
|
||||
export const validateSsoProviderId = async (ctx: AuthMiddlewareContext) => {
|
||||
if (ctx.path !== "/sso/register") {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!ctx.body || typeof ctx.body !== "object") {
|
||||
return;
|
||||
}
|
||||
|
||||
const providerId = (ctx.body as Record<string, unknown>).providerId;
|
||||
|
||||
if (typeof providerId !== "string") {
|
||||
return;
|
||||
}
|
||||
|
||||
if (isReservedSsoProviderId(providerId)) {
|
||||
throw new APIError("BAD_REQUEST", {
|
||||
message: `Invalid providerId. '${providerId}' is reserved and cannot be used for SSO providers.`,
|
||||
});
|
||||
}
|
||||
};
|
||||
9
app/server/lib/auth/utils/sso-provider-id.ts
Normal file
9
app/server/lib/auth/utils/sso-provider-id.ts
Normal file
|
|
@ -0,0 +1,9 @@
|
|||
const RESERVED_SSO_PROVIDER_IDS = new Set(["credential"]);
|
||||
|
||||
export function normalizeSsoProviderId(providerId: string): string {
|
||||
return providerId.trim().toLowerCase();
|
||||
}
|
||||
|
||||
export function isReservedSsoProviderId(providerId: string): boolean {
|
||||
return RESERVED_SSO_PROVIDER_IDS.has(normalizeSsoProviderId(providerId));
|
||||
}
|
||||
|
|
@ -0,0 +1,177 @@
|
|||
import { beforeEach, describe, expect, test } from "bun:test";
|
||||
import { db } from "~/server/db/db";
|
||||
import { account, invitation, member, organization, ssoProvider, usersTable } from "~/server/db/schema";
|
||||
import { authService } from "../auth.service";
|
||||
|
||||
function randomId() {
|
||||
return Bun.randomUUIDv7();
|
||||
}
|
||||
|
||||
function randomSlug(prefix: string) {
|
||||
return `${prefix}-${Math.random().toString(36).slice(2, 8)}`;
|
||||
}
|
||||
|
||||
async function createUser(email: string) {
|
||||
const id = randomId();
|
||||
|
||||
await db.insert(usersTable).values({
|
||||
id,
|
||||
email,
|
||||
name: email.split("@")[0],
|
||||
username: randomSlug("user"),
|
||||
});
|
||||
|
||||
return id;
|
||||
}
|
||||
|
||||
async function createOrganization(name: string) {
|
||||
const id = randomId();
|
||||
|
||||
await db.insert(organization).values({
|
||||
id,
|
||||
name,
|
||||
slug: randomSlug("org"),
|
||||
createdAt: new Date(),
|
||||
});
|
||||
|
||||
return id;
|
||||
}
|
||||
|
||||
describe("authService.deleteSsoProvider", () => {
|
||||
beforeEach(async () => {
|
||||
await db.delete(member);
|
||||
await db.delete(account);
|
||||
await db.delete(invitation);
|
||||
await db.delete(ssoProvider);
|
||||
await db.delete(organization);
|
||||
await db.delete(usersTable);
|
||||
});
|
||||
|
||||
test("does not delete accounts when provider belongs to another organization", async () => {
|
||||
const orgA = await createOrganization("Org A");
|
||||
const orgB = await createOrganization("Org B");
|
||||
const providerOwner = await createUser(`${randomSlug("owner")}@example.com`);
|
||||
const accountUser = await createUser(`${randomSlug("member")}@example.com`);
|
||||
|
||||
const providerId = `oidc-${randomSlug("provider")}`;
|
||||
|
||||
await db.insert(ssoProvider).values({
|
||||
id: randomId(),
|
||||
providerId,
|
||||
organizationId: orgB,
|
||||
userId: providerOwner,
|
||||
issuer: "https://issuer.example.com",
|
||||
domain: "example.com",
|
||||
});
|
||||
|
||||
await db.insert(account).values({
|
||||
id: randomId(),
|
||||
accountId: randomSlug("acct"),
|
||||
providerId,
|
||||
userId: accountUser,
|
||||
});
|
||||
|
||||
const deleted = await authService.deleteSsoProvider(providerId, orgA);
|
||||
|
||||
expect(deleted).toBe(false);
|
||||
|
||||
const remainingProvider = await db.query.ssoProvider.findFirst({
|
||||
where: { providerId },
|
||||
columns: { id: true },
|
||||
});
|
||||
const remainingAccounts = await db.query.account.findMany({
|
||||
where: { providerId },
|
||||
columns: { id: true },
|
||||
});
|
||||
|
||||
expect(remainingProvider).not.toBeUndefined();
|
||||
expect(remainingAccounts).toHaveLength(1);
|
||||
});
|
||||
|
||||
test("deletes provider and linked accounts in the active organization", async () => {
|
||||
const org = await createOrganization("Org A");
|
||||
const providerOwner = await createUser(`${randomSlug("owner")}@example.com`);
|
||||
const accountUserA = await createUser(`${randomSlug("member-a")}@example.com`);
|
||||
const accountUserB = await createUser(`${randomSlug("member-b")}@example.com`);
|
||||
|
||||
const providerId = `oidc-${randomSlug("provider")}`;
|
||||
|
||||
await db.insert(ssoProvider).values({
|
||||
id: randomId(),
|
||||
providerId,
|
||||
organizationId: org,
|
||||
userId: providerOwner,
|
||||
issuer: "https://issuer.example.com",
|
||||
domain: "example.com",
|
||||
});
|
||||
|
||||
await db.insert(account).values([
|
||||
{
|
||||
id: randomId(),
|
||||
accountId: randomSlug("acct-a"),
|
||||
providerId,
|
||||
userId: accountUserA,
|
||||
},
|
||||
{
|
||||
id: randomId(),
|
||||
accountId: randomSlug("acct-b"),
|
||||
providerId,
|
||||
userId: accountUserB,
|
||||
},
|
||||
]);
|
||||
|
||||
const deleted = await authService.deleteSsoProvider(providerId, org);
|
||||
|
||||
expect(deleted).toBe(true);
|
||||
|
||||
const remainingProvider = await db.query.ssoProvider.findFirst({
|
||||
where: { providerId },
|
||||
columns: { id: true },
|
||||
});
|
||||
const remainingAccounts = await db.query.account.findMany({
|
||||
where: { providerId },
|
||||
columns: { id: true },
|
||||
});
|
||||
|
||||
expect(remainingProvider).toBeUndefined();
|
||||
expect(remainingAccounts).toHaveLength(0);
|
||||
});
|
||||
|
||||
test("deleting a reserved provider id never deletes credential accounts", async () => {
|
||||
const org = await createOrganization("Org A");
|
||||
const providerOwner = await createUser(`${randomSlug("owner")}@example.com`);
|
||||
const credentialUser = await createUser(`${randomSlug("credential")}@example.com`);
|
||||
|
||||
await db.insert(ssoProvider).values({
|
||||
id: randomId(),
|
||||
providerId: "credential",
|
||||
organizationId: org,
|
||||
userId: providerOwner,
|
||||
issuer: "https://issuer.example.com",
|
||||
domain: "example.com",
|
||||
});
|
||||
|
||||
await db.insert(account).values({
|
||||
id: randomId(),
|
||||
accountId: randomSlug("credential-acct"),
|
||||
providerId: "credential",
|
||||
userId: credentialUser,
|
||||
});
|
||||
|
||||
const deleted = await authService.deleteSsoProvider("credential", org);
|
||||
|
||||
expect(deleted).toBe(true);
|
||||
|
||||
const remainingProvider = await db.query.ssoProvider.findFirst({
|
||||
where: { providerId: "credential" },
|
||||
columns: { id: true },
|
||||
});
|
||||
const remainingAccounts = await db.query.account.findMany({
|
||||
where: { providerId: "credential" },
|
||||
columns: { id: true },
|
||||
});
|
||||
|
||||
expect(remainingProvider).toBeUndefined();
|
||||
expect(remainingAccounts).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
|
@ -66,7 +66,11 @@ export const authController = new Hono()
|
|||
const providerId = c.req.param("providerId");
|
||||
const organizationId = c.get("organizationId");
|
||||
|
||||
await authService.deleteSsoProvider(providerId, organizationId);
|
||||
const deleted = await authService.deleteSsoProvider(providerId, organizationId);
|
||||
|
||||
if (!deleted) {
|
||||
return c.json({ message: "Provider not found" }, 404);
|
||||
}
|
||||
|
||||
return c.json({ success: true });
|
||||
})
|
||||
|
|
|
|||
|
|
@ -157,6 +157,9 @@ export const deleteSsoProviderDto = describeRoute({
|
|||
200: {
|
||||
description: "SSO provider deleted successfully",
|
||||
},
|
||||
404: {
|
||||
description: "Provider not found",
|
||||
},
|
||||
403: {
|
||||
description: "Forbidden",
|
||||
},
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import {
|
|||
} from "../../db/schema";
|
||||
import { eq, ne, and, count, inArray } from "drizzle-orm";
|
||||
import type { UserDeletionImpactDto } from "./auth.dto";
|
||||
import { isReservedSsoProviderId } from "~/server/lib/auth/utils/sso-provider-id";
|
||||
|
||||
export class AuthService {
|
||||
/**
|
||||
|
|
@ -108,11 +109,25 @@ export class AuthService {
|
|||
* Delete an SSO provider and its associated accounts
|
||||
*/
|
||||
async deleteSsoProvider(providerId: string, organizationId: string) {
|
||||
await db.transaction(async (tx) => {
|
||||
await tx.delete(account).where(eq(account.providerId, providerId));
|
||||
await tx
|
||||
.delete(ssoProvider)
|
||||
.where(and(eq(ssoProvider.providerId, providerId), eq(ssoProvider.organizationId, organizationId)));
|
||||
return db.transaction(async (tx) => {
|
||||
const provider = await tx.query.ssoProvider.findFirst({
|
||||
where: { AND: [{ providerId }, { organizationId }] },
|
||||
columns: { id: true, providerId: true },
|
||||
});
|
||||
|
||||
if (!provider) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (isReservedSsoProviderId(provider.providerId)) {
|
||||
await tx.delete(ssoProvider).where(eq(ssoProvider.id, provider.id));
|
||||
return true;
|
||||
}
|
||||
|
||||
await tx.delete(account).where(eq(account.providerId, provider.providerId));
|
||||
await tx.delete(ssoProvider).where(eq(ssoProvider.id, provider.id));
|
||||
|
||||
return true;
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -28,11 +28,8 @@ staticClients:
|
|||
- id: zerobyte-test
|
||||
name: Zerobyte Test
|
||||
redirectURIs:
|
||||
- "http://localhost:3000/api/auth/sso/callback/test-oidc"
|
||||
- "http://localhost:3000/api/auth/sso/callback/test-oidc-register"
|
||||
- "http://localhost:3000/api/auth/sso/callback/test-oidc-uninvited"
|
||||
- "http://localhost:3000/api/auth/sso/callback/test-oidc-invited"
|
||||
- "http://localhost:3000/api/auth/sso/callback/test-oidc-autolink"
|
||||
- "http://localhost:3000/api/auth/sso/callback/credential"
|
||||
- "http://localhost:3000/api/auth/sso/callback/dex"
|
||||
- "http://localhost:4096/api/auth/sso/callback/test-oidc"
|
||||
- "http://localhost:4096/api/auth/sso/callback/test-oidc-register"
|
||||
- "http://localhost:4096/api/auth/sso/callback/test-oidc-uninvited"
|
||||
|
|
@ -46,3 +43,8 @@ oauth2:
|
|||
logger:
|
||||
level: debug
|
||||
format: json
|
||||
|
||||
# Issuer URL: http://dex:5557/dex
|
||||
# Discovery URL: http://dex:5557/dex/.well-known/openid-configuration
|
||||
# Client ID: zerobyte-test
|
||||
# Client Secret: test-secret-12345
|
||||
|
|
|
|||
Loading…
Reference in a new issue