From c2ed9e3693c1db1c07a86122241b58bb0d1598a6 Mon Sep 17 00:00:00 2001 From: Nicolas Meienberger Date: Thu, 5 Mar 2026 21:02:56 +0100 Subject: [PATCH] refactor: correctly delete orphan sessions after idp deletion --- .../create-schedule-form/advanced-section.tsx | 4 +- .../auth.account-and-membership.test.ts | 275 ++++++++++++++++++ .../auth.controller.security.test.ts | 36 +++ app/server/modules/auth/auth.controller.ts | 12 +- app/server/modules/auth/auth.dto.ts | 4 +- app/server/modules/auth/auth.service.ts | 69 +++-- .../sso/__tests__/sso.registration.test.ts | 106 +++++++ .../modules/sso/__tests__/sso.service.test.ts | 19 +- .../sso/middlewares/authorize-registration.ts | 45 +++ app/server/modules/sso/sso.integration.ts | 3 +- app/server/modules/sso/sso.service.ts | 14 +- .../__tests__/validate-custom-params.test.ts | 44 +++ .../restic/helpers/validate-custom-params.ts | 118 +++++--- 13 files changed, 679 insertions(+), 70 deletions(-) create mode 100644 app/server/modules/auth/__tests__/auth.account-and-membership.test.ts create mode 100644 app/server/modules/sso/__tests__/sso.registration.test.ts create mode 100644 app/server/modules/sso/middlewares/authorize-registration.ts create mode 100644 app/server/utils/restic/helpers/__tests__/validate-custom-params.test.ts diff --git a/app/client/modules/backups/components/create-schedule-form/advanced-section.tsx b/app/client/modules/backups/components/create-schedule-form/advanced-section.tsx index d7b03e1f..72d3f831 100644 --- a/app/client/modules/backups/components/create-schedule-form/advanced-section.tsx +++ b/app/client/modules/backups/components/create-schedule-form/advanced-section.tsx @@ -24,8 +24,8 @@ export const AdvancedSection = ({ form }: AdvancedSectionProps) => { Advanced: enter one restic flag per line (e.g.{" "} - --iexclude-larger-than 500M). These are appended to the - backup command as-is. + --exclude-larger-than 500M). Only the supported flag list is + accepted. diff --git a/app/server/modules/auth/__tests__/auth.account-and-membership.test.ts b/app/server/modules/auth/__tests__/auth.account-and-membership.test.ts new file mode 100644 index 00000000..f620fd68 --- /dev/null +++ b/app/server/modules/auth/__tests__/auth.account-and-membership.test.ts @@ -0,0 +1,275 @@ +import { beforeEach, describe, expect, test } from "bun:test"; +import { db } from "~/server/db/db"; +import { account, member, organization, sessionsTable, 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; +} + +async function createMembership({ + userId, + organizationId, + role, +}: { + userId: string; + organizationId: string; + role: "owner" | "admin" | "member"; +}) { + const id = randomId(); + await db.insert(member).values({ id, userId, organizationId, role, createdAt: new Date() }); + return id; +} + +async function createSession({ + userId, + activeOrganizationId, +}: { + userId: string; + activeOrganizationId: string | null; +}) { + await db.insert(sessionsTable).values({ + id: randomId(), + userId, + token: randomSlug("token"), + expiresAt: new Date(Date.now() + 60_000), + activeOrganizationId, + }); +} + +async function createAccount({ userId, providerId }: { userId: string; providerId: string }) { + const id = randomId(); + + await db.insert(account).values({ + id, + accountId: randomSlug("account"), + providerId, + userId, + password: providerId === "credential" ? "password-hash" : null, + }); + + return id; +} + +describe("authService account and membership management", () => { + beforeEach(async () => { + await db.delete(account); + await db.delete(member); + await db.delete(sessionsTable); + await db.delete(organization); + await db.delete(usersTable); + }); + + test("deleteUserAccount removes the selected account and keeps existing sessions", async () => { + const userId = await createUser(`${randomSlug("user")}@example.com`); + const credentialAccountId = await createAccount({ + userId, + providerId: "credential", + }); + await createAccount({ userId, providerId: "oidc-acme" }); + await createSession({ userId, activeOrganizationId: null }); + + const result = await authService.deleteUserAccount(userId, credentialAccountId); + + expect(result).toEqual({ lastAccount: false, notFound: false }); + + const deletedAccount = await db.query.account.findFirst({ + where: { id: credentialAccountId }, + columns: { id: true }, + }); + const remainingSessions = await db.query.sessionsTable.findMany({ + where: { userId }, + columns: { id: true }, + }); + + expect(deletedAccount).toBeUndefined(); + expect(remainingSessions).toHaveLength(1); + }); + + test("deleteUserAccount returns notFound when the account does not belong to the user", async () => { + const userId = await createUser(`${randomSlug("user")}@example.com`); + await createAccount({ userId, providerId: "credential" }); + await createAccount({ userId, providerId: "oidc-acme" }); + + const result = await authService.deleteUserAccount(userId, randomId()); + + expect(result).toEqual({ lastAccount: false, notFound: true }); + }); + + test("deleteUserAccount refuses to delete when it is the user's last account", async () => { + const userId = await createUser(`${randomSlug("user")}@example.com`); + const onlyAccountId = await createAccount({ + userId, + providerId: "credential", + }); + + const result = await authService.deleteUserAccount(userId, onlyAccountId); + + expect(result).toEqual({ lastAccount: true, notFound: false }); + + const existingAccount = await db.query.account.findFirst({ + where: { id: onlyAccountId }, + columns: { id: true }, + }); + + expect(existingAccount).toEqual({ id: onlyAccountId }); + }); + + test("removeOrgMember rehomes active sessions to another organization membership", async () => { + const userId = await createUser(`${randomSlug("user")}@example.com`); + const removedOrgId = await createOrganization("Removed Org"); + const fallbackOrgId = await createOrganization("Fallback Org"); + const membershipId = await createMembership({ + userId, + organizationId: removedOrgId, + role: "member", + }); + await createMembership({ + userId, + organizationId: fallbackOrgId, + role: "member", + }); + await createSession({ userId, activeOrganizationId: removedOrgId }); + + const result = await authService.removeOrgMember(membershipId, removedOrgId); + + expect(result).toEqual({ found: true, isOwner: false }); + + const session = await db.query.sessionsTable.findFirst({ + where: { userId }, + columns: { activeOrganizationId: true }, + }); + const removedMembership = await db.query.member.findFirst({ + where: { id: membershipId }, + columns: { id: true }, + }); + + expect(session?.activeOrganizationId).toBe(fallbackOrgId); + expect(removedMembership).toBeUndefined(); + }); + + test("removeOrgMember revokes sessions when the removed user has no fallback organization", async () => { + const userId = await createUser(`${randomSlug("user")}@example.com`); + const removedOrgId = await createOrganization("Removed Org"); + const membershipId = await createMembership({ + userId, + organizationId: removedOrgId, + role: "member", + }); + await createSession({ userId, activeOrganizationId: removedOrgId }); + + const result = await authService.removeOrgMember(membershipId, removedOrgId); + + expect(result).toEqual({ found: true, isOwner: false }); + + const remainingSessions = await db.query.sessionsTable.findMany({ + where: { userId }, + columns: { id: true }, + }); + const removedMembership = await db.query.member.findFirst({ + where: { id: membershipId }, + columns: { id: true }, + }); + + expect(remainingSessions).toHaveLength(0); + expect(removedMembership).toBeUndefined(); + }); + + test("removeOrgMember returns isOwner=true and does not remove owner membership", async () => { + const userId = await createUser(`${randomSlug("user")}@example.com`); + const orgId = await createOrganization("Owner Org"); + const membershipId = await createMembership({ + userId, + organizationId: orgId, + role: "owner", + }); + + const result = await authService.removeOrgMember(membershipId, orgId); + + expect(result).toEqual({ found: true, isOwner: true }); + + const existingMembership = await db.query.member.findFirst({ + where: { id: membershipId }, + columns: { id: true, role: true }, + }); + + expect(existingMembership).toEqual({ id: membershipId, role: "owner" }); + }); + + test("removeOrgMember returns found=false for missing membership and leaves members/sessions unchanged", async () => { + const userId = await createUser(`${randomSlug("user")}@example.com`); + const orgId = await createOrganization("Existing Org"); + const otherOrgId = await createOrganization("Other Org"); + const membershipId = await createMembership({ + userId, + organizationId: orgId, + role: "member", + }); + await createMembership({ + userId, + organizationId: otherOrgId, + role: "member", + }); + await createSession({ userId, activeOrganizationId: orgId }); + + const membersBefore = await db.query.member.findMany({ + where: { userId }, + columns: { id: true }, + }); + const sessionsBefore = await db.query.sessionsTable.findMany({ + where: { userId }, + columns: { id: true, activeOrganizationId: true }, + }); + + const result = await authService.removeOrgMember(randomId(), orgId); + + expect(result).toEqual({ found: false, isOwner: false }); + + const membersAfter = await db.query.member.findMany({ + where: { userId }, + columns: { id: true }, + }); + const sessionsAfter = await db.query.sessionsTable.findMany({ + where: { userId }, + columns: { id: true, activeOrganizationId: true }, + }); + const membershipStillExists = await db.query.member.findFirst({ + where: { id: membershipId }, + columns: { id: true }, + }); + + expect(membersAfter).toEqual(membersBefore); + expect(sessionsAfter).toEqual(sessionsBefore); + expect(membershipStillExists).toEqual({ id: membershipId }); + }); +}); diff --git a/app/server/modules/auth/__tests__/auth.controller.security.test.ts b/app/server/modules/auth/__tests__/auth.controller.security.test.ts index 37dc941b..a4482d01 100644 --- a/app/server/modules/auth/__tests__/auth.controller.security.test.ts +++ b/app/server/modules/auth/__tests__/auth.controller.security.test.ts @@ -152,6 +152,42 @@ describe("auth controller security", () => { expect(res.status).not.toBe(401); }); } + + test("global admins can delete an account for a user outside their active organization", async () => { + const { headers } = await createTestSessionWithGlobalAdmin(); + const target = await createTestSession(); + + const retainedAccountId = Bun.randomUUIDv7(); + await db.insert(account).values({ + id: retainedAccountId, + accountId: `credential-${retainedAccountId}`, + providerId: "credential", + userId: target.user.id, + password: "password-hash", + }); + + const removableAccountId = Bun.randomUUIDv7(); + await db.insert(account).values({ + id: removableAccountId, + accountId: `oidc-${removableAccountId}`, + providerId: "oidc-acme", + userId: target.user.id, + }); + + const res = await app.request(`/api/v1/auth/admin-users/${target.user.id}/accounts/${removableAccountId}`, { + method: "DELETE", + headers, + }); + + expect(res.status).toBe(200); + + const deletedAccount = await db.query.account.findFirst({ + where: { id: removableAccountId }, + columns: { id: true }, + }); + + expect(deletedAccount).toBeUndefined(); + }); }); describe("invalid session handling", () => { diff --git a/app/server/modules/auth/auth.controller.ts b/app/server/modules/auth/auth.controller.ts index b32d583f..21efd34f 100644 --- a/app/server/modules/auth/auth.controller.ts +++ b/app/server/modules/auth/auth.controller.ts @@ -49,18 +49,16 @@ export const authController = new Hono() .delete("/admin-users/:userId/accounts/:accountId", requireAuth, requireAdmin, deleteUserAccountDto, async (c) => { const userId = c.req.param("userId"); const accountId = c.req.param("accountId"); - const organizationId = c.get("organizationId"); - - const result = await authService.deleteUserAccount(userId, accountId, organizationId); - - if (result.forbidden) { - return c.json({ message: "User is not a member of this organization" }, 403); - } + const result = await authService.deleteUserAccount(userId, accountId); if (result.lastAccount) { return c.json({ message: "Cannot delete the last account of a user" }, 409); } + if (result.notFound) { + return c.json({ message: "Account not found" }, 404); + } + return c.json({ success: true }); }) .get("/deletion-impact/:userId", requireAuth, requireAdmin, getUserDeletionImpactDto, async (c) => { diff --git a/app/server/modules/auth/auth.dto.ts b/app/server/modules/auth/auth.dto.ts index 03859b56..eee61222 100644 --- a/app/server/modules/auth/auth.dto.ts +++ b/app/server/modules/auth/auth.dto.ts @@ -94,8 +94,8 @@ export const deleteUserAccountDto = describeRoute({ 200: { description: "Account deleted successfully", }, - 403: { - description: "Forbidden", + 404: { + description: "Account not found", }, 409: { description: "Cannot delete the last account", diff --git a/app/server/modules/auth/auth.service.ts b/app/server/modules/auth/auth.service.ts index b350ea02..59b7629c 100644 --- a/app/server/modules/auth/auth.service.ts +++ b/app/server/modules/auth/auth.service.ts @@ -97,7 +97,10 @@ export class AuthService { if (affectedUserIds.length > 0) { const memberships = await tx - .select({ userId: member.userId, organizationId: member.organizationId }) + .select({ + userId: member.userId, + organizationId: member.organizationId, + }) .from(member) .where(inArray(member.userId, affectedUserIds)); @@ -127,16 +130,6 @@ export class AuthService { }); } - /** - * Check if a user is a member of an organization - */ - async getUserMembership(userId: string, organizationId: string) { - return db.query.member.findFirst({ - where: { AND: [{ userId }, { organizationId }] }, - columns: { id: true }, - }); - } - /** * Fetch accounts for a list of users, keyed by userId */ @@ -153,7 +146,10 @@ export class AuthService { if (!grouped[row.userId]) { grouped[row.userId] = []; } - grouped[row.userId].push({ id: row.id, providerId: row.providerId }); + grouped[row.userId].push({ + id: row.id, + providerId: row.providerId, + }); } return grouped; } @@ -220,7 +216,30 @@ export class AuthService { return { found: true, isOwner: true } as const; } - await db.delete(member).where(eq(member.id, memberId)); + await db.transaction(async (tx) => { + const fallbackMembership = await tx.query.member.findFirst({ + where: { + AND: [{ userId: targetMember.userId }, { organizationId: { ne: organizationId } }], + }, + columns: { organizationId: true }, + }); + + await tx.delete(member).where(eq(member.id, memberId)); + + if (fallbackMembership?.organizationId) { + await tx + .update(sessionsTable) + .set({ + activeOrganizationId: fallbackMembership.organizationId, + }) + .where( + and(eq(sessionsTable.userId, targetMember.userId), eq(sessionsTable.activeOrganizationId, organizationId)), + ); + return; + } + + await tx.delete(sessionsTable).where(eq(sessionsTable.userId, targetMember.userId)); + }); return { found: true, isOwner: false } as const; } @@ -241,24 +260,32 @@ export class AuthService { /** * Delete a single account for a user, refusing if it is the last one */ - async deleteUserAccount(userId: string, accountId: string, organizationId: string) { - const membership = await this.getUserMembership(userId, organizationId); - if (!membership) { - return { lastAccount: false, forbidden: true }; - } - + async deleteUserAccount(userId: string, accountId: string) { return db.transaction(async (tx) => { + const [targetAccount] = await tx + .select({ id: account.id }) + .from(account) + .where(and(eq(account.id, accountId), eq(account.userId, userId))) + .limit(1); + + if (!targetAccount) { + return { lastAccount: false, notFound: true }; + } + const userAccounts = await tx.query.account.findMany({ where: { userId }, columns: { id: true }, }); if (userAccounts.length <= 1) { - return { lastAccount: true, forbidden: false }; + return { lastAccount: true, notFound: false }; } await tx.delete(account).where(and(eq(account.id, accountId), eq(account.userId, userId))); - return { lastAccount: false, forbidden: false }; + // Sessions cannot be tied to a specific account/provider with the current schema, + // so keep active sessions intact when unlinking a single account. + + return { lastAccount: false, notFound: false }; }); } } diff --git a/app/server/modules/sso/__tests__/sso.registration.test.ts b/app/server/modules/sso/__tests__/sso.registration.test.ts new file mode 100644 index 00000000..8dab023f --- /dev/null +++ b/app/server/modules/sso/__tests__/sso.registration.test.ts @@ -0,0 +1,106 @@ +import { beforeEach, describe, expect, test } from "bun:test"; +import { createApp } from "~/server/app"; +import { config } from "~/server/core/config"; +import { db } from "~/server/db/db"; +import { account, invitation, member, organization, ssoProvider, usersTable, verification } from "~/server/db/schema"; +import { createTestSession, createTestSessionWithOrgAdmin } from "~/test/helpers/auth"; + +const app = createApp(); +const ssoRegisterUrl = new URL("/api/auth/sso/register", config.baseUrl).toString(); + +function buildRegisterBody(organizationId: string, suffix: string) { + return { + providerId: `oidc-${suffix}-${Bun.randomUUIDv7()}`, + issuer: "https://issuer.example.com", + domain: "example.com", + organizationId, + oidcConfig: { + clientId: "client-id", + clientSecret: "client-secret", + skipDiscovery: true, + discoveryEndpoint: "https://issuer.example.com/.well-known/openid-configuration", + authorizationEndpoint: "https://issuer.example.com/oauth2/authorize", + tokenEndpoint: "https://issuer.example.com/oauth2/token", + jwksEndpoint: "https://issuer.example.com/.well-known/jwks.json", + }, + }; +} + +describe("SSO provider registration authorization", () => { + beforeEach(async () => { + await db.delete(member); + await db.delete(account); + await db.delete(invitation); + await db.delete(ssoProvider); + await db.delete(verification); + await db.delete(organization); + await db.delete(usersTable); + }); + + test("allows organization owners to register providers for their active organization", async () => { + const { headers, organizationId } = await createTestSession(); + + const response = await app.request(ssoRegisterUrl, { + method: "POST", + headers: { + ...headers, + "Content-Type": "application/json", + }, + body: JSON.stringify(buildRegisterBody(organizationId, "owner")), + }); + + expect(response.status).toBe(200); + }); + + test("rejects org admins for registration when they are not owners", async () => { + const { headers, organizationId } = await createTestSessionWithOrgAdmin(); + + const response = await app.request(ssoRegisterUrl, { + method: "POST", + headers: { + ...headers, + "Content-Type": "application/json", + }, + body: JSON.stringify(buildRegisterBody(organizationId, "admin")), + }); + + expect(response.status).toBe(403); + + const body = await response.json(); + expect(body.message).toBe("Only organization owners can register SSO providers"); + }); + + test("rejects users who are owners elsewhere but only members of the target organization", async () => { + const { headers, user } = await createTestSession(); + const targetOrgId = Bun.randomUUIDv7(); + + await db.insert(organization).values({ + id: targetOrgId, + name: "Target Org", + slug: `target-org-${Date.now()}`, + createdAt: new Date(), + }); + + await db.insert(member).values({ + id: Bun.randomUUIDv7(), + userId: user.id, + organizationId: targetOrgId, + role: "member", + createdAt: new Date(), + }); + + const response = await app.request(ssoRegisterUrl, { + method: "POST", + headers: { + ...headers, + "Content-Type": "application/json", + }, + body: JSON.stringify(buildRegisterBody(targetOrgId, "cross-org")), + }); + + expect(response.status).toBe(403); + + const body = await response.json(); + expect(body.message).toBe("Only organization owners can register SSO providers"); + }); +}); diff --git a/app/server/modules/sso/__tests__/sso.service.test.ts b/app/server/modules/sso/__tests__/sso.service.test.ts index 7c9eef54..dd3ae8c4 100644 --- a/app/server/modules/sso/__tests__/sso.service.test.ts +++ b/app/server/modules/sso/__tests__/sso.service.test.ts @@ -1,6 +1,6 @@ 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 { account, invitation, member, organization, sessionsTable, ssoProvider, usersTable } from "~/server/db/schema"; import { ssoService } from "../sso.service"; function randomId() { @@ -37,10 +37,20 @@ async function createOrganization(name: string) { return id; } +async function createSession(userId: string) { + await db.insert(sessionsTable).values({ + id: randomId(), + userId, + token: randomSlug("token"), + expiresAt: new Date(Date.now() + 60_000), + }); +} + describe("ssoService.deleteSsoProvider", () => { beforeEach(async () => { await db.delete(member); await db.delete(account); + await db.delete(sessionsTable); await db.delete(invitation); await db.delete(ssoProvider); await db.delete(organization); @@ -119,6 +129,8 @@ describe("ssoService.deleteSsoProvider", () => { userId: accountUserB, }, ]); + await createSession(accountUserA); + await createSession(accountUserB); const deleted = await ssoService.deleteSsoProvider(providerId, org); @@ -132,9 +144,14 @@ describe("ssoService.deleteSsoProvider", () => { where: { providerId }, columns: { id: true }, }); + const remainingSessions = await db.query.sessionsTable.findMany({ + where: { userId: { in: [accountUserA, accountUserB] } }, + columns: { id: true }, + }); expect(remainingProvider).toBeUndefined(); expect(remainingAccounts).toHaveLength(0); + expect(remainingSessions).toHaveLength(0); }); test("deleting a reserved provider id never deletes credential accounts", async () => { diff --git a/app/server/modules/sso/middlewares/authorize-registration.ts b/app/server/modules/sso/middlewares/authorize-registration.ts new file mode 100644 index 00000000..951acfbe --- /dev/null +++ b/app/server/modules/sso/middlewares/authorize-registration.ts @@ -0,0 +1,45 @@ +import { APIError } from "better-auth/api"; +import type { AuthMiddlewareContext } from "~/server/lib/auth"; +import { db } from "~/server/db/db"; + +export const authorizeSsoRegistration = async (ctx: AuthMiddlewareContext) => { + if (ctx.path !== "/sso/register") { + return; + } + + const sessionToken = await ctx.getSignedCookie(ctx.context.authCookies.sessionToken.name, ctx.context.secret); + if (!sessionToken) { + throw new APIError("UNAUTHORIZED"); + } + + const session = await ctx.context.internalAdapter.findSession(sessionToken); + if (!session || session.session.expiresAt < new Date()) { + throw new APIError("UNAUTHORIZED"); + } + + ctx.context.session = session; + + if (!ctx.body || typeof ctx.body !== "object") { + throw new APIError("BAD_REQUEST", { message: "Missing SSO registration payload" }); + } + + const organizationId = (ctx.body as Record).organizationId; + if (typeof organizationId !== "string" || organizationId.length === 0) { + throw new APIError("BAD_REQUEST", { message: "organizationId is required to register an SSO provider" }); + } + + const membership = await db.query.member.findFirst({ + where: { + AND: [{ userId: session.user.id }, { organizationId }], + }, + columns: { role: true }, + }); + + if (!membership) { + throw new APIError("FORBIDDEN", { message: "You are not a member of this organization" }); + } + + if (membership.role !== "owner") { + throw new APIError("FORBIDDEN", { message: "Only organization owners can register SSO providers" }); + } +}; diff --git a/app/server/modules/sso/sso.integration.ts b/app/server/modules/sso/sso.integration.ts index 01932147..fedfc98e 100644 --- a/app/server/modules/sso/sso.integration.ts +++ b/app/server/modules/sso/sso.integration.ts @@ -8,6 +8,7 @@ import { authService } from "../auth/auth.service"; import { ssoService } from "./sso.service"; import { validateSsoProviderId } from "./middlewares/validate-provider-id"; import { validateSsoCallbackUrls } from "./middlewares/validate-callback-urls"; +import { authorizeSsoRegistration } from "./middlewares/authorize-registration"; import { requireSsoInvitation } from "./middlewares/require-invitation"; import { resolveTrustedProvidersForRequest } from "./middlewares/trust-provider-for-linking"; import { isSsoCallbackRequest, extractProviderIdFromContext, normalizeEmail } from "./utils/sso-context"; @@ -143,7 +144,7 @@ export const ssoIntegration = { }, }), - beforeMiddlewares: [validateSsoProviderId, validateSsoCallbackUrls] as const, + beforeMiddlewares: [validateSsoProviderId, validateSsoCallbackUrls, authorizeSsoRegistration] as const, isSsoCallback: isSsoCallbackRequest, diff --git a/app/server/modules/sso/sso.service.ts b/app/server/modules/sso/sso.service.ts index d2359bc6..996041fd 100644 --- a/app/server/modules/sso/sso.service.ts +++ b/app/server/modules/sso/sso.service.ts @@ -1,6 +1,6 @@ import { db } from "~/server/db/db"; -import { ssoProvider, account, invitation, organization } from "~/server/db/schema"; -import { eq, and } from "drizzle-orm"; +import { ssoProvider, account, invitation, organization, sessionsTable } from "~/server/db/schema"; +import { eq, and, inArray } from "drizzle-orm"; import { isReservedSsoProviderId } from "./utils/sso-provider-id"; import { normalizeEmail } from "./utils/sso-context"; @@ -83,9 +83,19 @@ export class SsoService { return true; } + const affectedAccounts = await tx.query.account.findMany({ + where: { providerId: provider.providerId }, + columns: { userId: true }, + }); + const affectedUserIds = [...new Set(affectedAccounts.map((row) => row.userId))]; + await tx.delete(account).where(eq(account.providerId, provider.providerId)); await tx.delete(ssoProvider).where(eq(ssoProvider.id, provider.id)); + if (affectedUserIds.length > 0) { + await tx.delete(sessionsTable).where(inArray(sessionsTable.userId, affectedUserIds)); + } + return true; }); } diff --git a/app/server/utils/restic/helpers/__tests__/validate-custom-params.test.ts b/app/server/utils/restic/helpers/__tests__/validate-custom-params.test.ts new file mode 100644 index 00000000..1599f07b --- /dev/null +++ b/app/server/utils/restic/helpers/__tests__/validate-custom-params.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, test } from "bun:test"; +import { validateCustomResticParams } from "../validate-custom-params"; + +describe("validateCustomResticParams", () => { + test("accepts supported flags and values", () => { + const result = validateCustomResticParams([ + "--no-scan", + "--read-concurrency 8", + "--exclude-larger-than 500M", + "--pack-size=64", + ]); + + expect(result).toBeNull(); + }); + + test("rejects positional arguments", () => { + const result = validateCustomResticParams(["/etc"]); + + expect(result).toContain('Unexpected positional argument "/etc"'); + }); + + test("rejects extra positional arguments after a flag value", () => { + const result = validateCustomResticParams(["--read-concurrency 8 /etc"]); + + expect(result).toContain('Unexpected positional argument "/etc"'); + }); + + test("rejects unsupported path-bearing flags", () => { + const result = validateCustomResticParams(["--cache-dir /tmp/restic-cache"]); + + expect(result).toContain('Unknown or unsupported flag "--cache-dir"'); + }); + + test("rejects dry-run flags", () => { + expect(validateCustomResticParams(["--dry-run"])).toContain('Unknown or unsupported flag "--dry-run"'); + expect(validateCustomResticParams(["-n"])).toContain('Unknown or unsupported flag "-n"'); + }); + + test("rejects missing values for flags that require one", () => { + const result = validateCustomResticParams(["--read-concurrency"]); + + expect(result).toBe('Flag "--read-concurrency" requires a value'); + }); +}); diff --git a/app/server/utils/restic/helpers/validate-custom-params.ts b/app/server/utils/restic/helpers/validate-custom-params.ts index 07e945d8..e17aec76 100644 --- a/app/server/utils/restic/helpers/validate-custom-params.ts +++ b/app/server/utils/restic/helpers/validate-custom-params.ts @@ -14,56 +14,106 @@ const DENIED_FLAGS = new Set([ "--repo", ]); -const ALLOWED_FLAGS = new Set([ - "--verbose", - "-v", - "--no-scan", - "--exclude-larger-than", - "--skip-if-unchanged", - "--exclude-caches", - "--force", - "--use-fs-snapshot", - "--read-concurrency", - "--ignore-ctime", - "--ignore-inode", - "--with-atime", - "--no-cache", - "--cleanup-cache", - "--cache-dir", - "--limit-upload", - "--limit-download", - "--pack-size", - "--dry-run", - "-n", - "--no-lock", - "--json", +type FlagSpec = { + requiresValue: boolean; + validateValue?: (value: string) => string | null; +}; + +const positiveIntegerValue = (value: string) => { + return /^\d+$/.test(value) ? null : `Flag value "${value}" must be a positive integer`; +}; + +const FLAG_SPECS = new Map([ + ["--verbose", { requiresValue: false }], + ["-v", { requiresValue: false }], + ["--no-scan", { requiresValue: false }], + ["--exclude-larger-than", { requiresValue: true }], + ["--skip-if-unchanged", { requiresValue: false }], + ["--exclude-caches", { requiresValue: false }], + ["--force", { requiresValue: false }], + ["--use-fs-snapshot", { requiresValue: false }], + ["--read-concurrency", { requiresValue: true, validateValue: positiveIntegerValue }], + ["--ignore-ctime", { requiresValue: false }], + ["--ignore-inode", { requiresValue: false }], + ["--with-atime", { requiresValue: false }], + ["--no-cache", { requiresValue: false }], + ["--cleanup-cache", { requiresValue: false }], + ["--limit-upload", { requiresValue: true, validateValue: positiveIntegerValue }], + ["--limit-download", { requiresValue: true, validateValue: positiveIntegerValue }], + ["--pack-size", { requiresValue: true, validateValue: positiveIntegerValue }], + ["--no-lock", { requiresValue: false }], ]); -function extractFlagName(token: string): string | null { - if (!token.startsWith("-")) { - return null; - } +const ALLOWED_FLAGS = [...FLAG_SPECS.keys()].join(", "); + +function parseFlagToken(token: string) { const eqIdx = token.indexOf("="); - return eqIdx === -1 ? token : token.slice(0, eqIdx); + + return eqIdx === -1 + ? { flag: token, inlineValue: null as string | null } + : { flag: token.slice(0, eqIdx), inlineValue: token.slice(eqIdx + 1) }; +} + +function validateFlagValue(flag: string, value: string, spec: FlagSpec): string | null { + if (!value) { + return `Flag "${flag}" requires a value`; + } + + if (value.startsWith("-")) { + return `Flag "${flag}" requires a value, but received "${value}"`; + } + + return spec.validateValue?.(value) ?? null; } export function validateCustomResticParams(params: string[]): string | null { for (const param of params) { const tokens = param.trim().split(/\s+/).filter(Boolean); - for (const token of tokens) { - const flag = extractFlagName(token); - if (flag === null) { - continue; + for (let index = 0; index < tokens.length; index += 1) { + const token = tokens[index]; + + if (!token.startsWith("-")) { + return `Unexpected positional argument "${token}" in customResticParams`; } + const { flag, inlineValue } = parseFlagToken(token); + if (DENIED_FLAGS.has(flag)) { return `Flag "${flag}" is not permitted in customResticParams`; } - if (!ALLOWED_FLAGS.has(flag)) { - return `Unknown or unsupported flag "${flag}" in customResticParams. Permitted flags: ${[...ALLOWED_FLAGS].join(", ")}`; + const spec = FLAG_SPECS.get(flag); + if (!spec) { + return `Unknown or unsupported flag "${flag}" in customResticParams. Permitted flags: ${ALLOWED_FLAGS}`; } + + if (!spec.requiresValue) { + if (inlineValue !== null && inlineValue !== "") { + return `Flag "${flag}" does not accept a value`; + } + continue; + } + + if (inlineValue !== null) { + const error = validateFlagValue(flag, inlineValue, spec); + if (error) { + return error; + } + continue; + } + + const nextToken = tokens[index + 1]; + if (!nextToken) { + return `Flag "${flag}" requires a value`; + } + + const error = validateFlagValue(flag, nextToken, spec); + if (error) { + return error; + } + + index += 1; } }