From 8ba1b8ba2c2c30b1cec30eb5777c1fda670cbbfc Mon Sep 17 00:00:00 2001 From: Hugues Chocart Date: Thu, 24 Oct 2024 14:03:24 +0800 Subject: [PATCH] fix: security patches (#631) --- packages/backend/src/api/v1/evaluator.ts | 20 ++++++----- packages/backend/src/api/v1/models.ts | 36 +++++++++++-------- packages/backend/src/api/v1/projects/index.ts | 8 +++-- packages/backend/src/api/v1/users.ts | 26 ++++++++------ packages/frontend/pages/team.tsx | 25 +++++++------ 5 files changed, 69 insertions(+), 46 deletions(-) diff --git a/packages/backend/src/api/v1/evaluator.ts b/packages/backend/src/api/v1/evaluator.ts index a25100a0..74a7a313 100644 --- a/packages/backend/src/api/v1/evaluator.ts +++ b/packages/backend/src/api/v1/evaluator.ts @@ -1,8 +1,8 @@ +import { checkAccess } from "@/src/utils/authorization"; import sql from "@/src/utils/db"; import { clearUndefined } from "@/src/utils/ingest"; import Context from "@/src/utils/koa"; import Router from "koa-router"; -import { deserializeLogic } from "shared"; import { z } from "zod"; const evaluators = new Router({ @@ -13,16 +13,20 @@ const evaluators = new Router({ // TODO: route to get the number of runs checks are applied to, for new evaluators // TODO: proper schema validation for params and filters -evaluators.get("/", async (ctx: Context) => { - const { projectId } = ctx.state; +evaluators.get( + "/", + checkAccess("evaluations", "list"), + async (ctx: Context) => { + const { projectId } = ctx.state; - const evaluators = - await sql`select * from evaluator where project_id = ${projectId}`; + const evaluators = + await sql`select * from evaluator where project_id = ${projectId}`; - // TODO: return number of runs the evaluator will be applied to + // TODO: return number of runs the evaluator will be applied to - ctx.body = evaluators; -}); + ctx.body = evaluators; + }, +); evaluators.get("/:id", async (ctx: Context) => { const { projectId } = ctx.state; diff --git a/packages/backend/src/api/v1/models.ts b/packages/backend/src/api/v1/models.ts index d5f8d6de..e3f94af5 100644 --- a/packages/backend/src/api/v1/models.ts +++ b/packages/backend/src/api/v1/models.ts @@ -63,7 +63,7 @@ models.get("/", checkAccess("logs", "list"), async (ctx: Context) => { * schema: * $ref: '#/components/schemas/Model' */ -models.post("/", async (ctx: Context) => { +models.post("/", checkAccess("settings", "read"), async (ctx: Context) => { const { orgId } = ctx.state; const validatedData = ModelSchema.parse(ctx.request.body); @@ -106,21 +106,25 @@ models.post("/", async (ctx: Context) => { * schema: * $ref: '#/components/schemas/Model' */ -models.patch("/:id", async (ctx: Context) => { - const { orgId } = ctx.state; - const { id } = ctx.params; +models.patch( + "/:id", + checkAccess("settings", "update"), + async (ctx: Context) => { + const { orgId } = ctx.state; + const { id } = ctx.params; - const validatedData = ModelSchema.partial().parse(ctx.request.body); + const validatedData = ModelSchema.partial().parse(ctx.request.body); - const [updatedModel] = await sql` + const [updatedModel] = await sql` update model_mapping set ${sql(clearUndefined({ ...validatedData, updatedAt: new Date() }))} where org_id = ${orgId} and id = ${id} returning * `; - ctx.body = updatedModel; -}); + ctx.body = updatedModel; + }, +); /** * @openapi @@ -138,19 +142,23 @@ models.patch("/:id", async (ctx: Context) => { * 200: * description: Successful deletion */ -models.delete("/:id", checkAccess("logs", "delete"), async (ctx: Context) => { - const { orgId } = ctx.state; - const { id } = ctx.params; +models.delete( + "/:id", + checkAccess("settings", "delete"), + async (ctx: Context) => { + const { orgId } = ctx.state; + const { id } = ctx.params; - await sql` + await sql` delete from model_mapping where org_id = ${orgId} and id = ${id} returning * `; - ctx.status = 200; -}); + ctx.status = 200; + }, +); /** * @openapi diff --git a/packages/backend/src/api/v1/projects/index.ts b/packages/backend/src/api/v1/projects/index.ts index 85328981..2a8c6002 100644 --- a/packages/backend/src/api/v1/projects/index.ts +++ b/packages/backend/src/api/v1/projects/index.ts @@ -1,9 +1,10 @@ import { checkAccess, checkProjectAccess } from "@/src/utils/authorization"; import sql from "@/src/utils/db"; import Context from "@/src/utils/koa"; +import { randomUUID } from "crypto"; import Router from "koa-router"; +import { hasAccess } from "shared"; import { z } from "zod"; -import { randomUUID } from "crypto"; const projects = new Router({ prefix: "/projects", @@ -12,6 +13,9 @@ const projects = new Router({ projects.get("/", checkAccess("projects", "read"), async (ctx: Context) => { const { orgId, userId } = ctx.state; + const [{ role: userRole }] = + await sql`select role from account where id = ${userId}`; + const rows = await sql` select distinct on (p.id) p.id, @@ -21,7 +25,7 @@ projects.get("/", checkAccess("projects", "read"), async (ctx: Context) => { ingestion_rule.filters, exists(select * from run where project_id = p.id) as activated, (select api_key from api_key where project_id = p.id and type = 'public') as public_api_key, - (select api_key from api_key where project_id = p.id and type = 'private') as private_api_key, + ${hasAccess(userRole, "privateKeys", "read") ? sql`(select api_key from api_key where project_id = p.id and type = 'private') as private_api_key,` : sql``} (select array_agg(project_id) as id from account_project where account_id = ${userId}) as projects from project p diff --git a/packages/backend/src/api/v1/users.ts b/packages/backend/src/api/v1/users.ts index 86c45877..7c6b8120 100644 --- a/packages/backend/src/api/v1/users.ts +++ b/packages/backend/src/api/v1/users.ts @@ -1,5 +1,3 @@ -import sql from "@/src/utils/db"; -import Router from "koa-router"; import { INVITE_EMAIL, sendEmail, @@ -7,12 +5,14 @@ import { WELCOME_EMAIL, } from "@/src/emails"; import { checkAccess } from "@/src/utils/authorization"; +import config from "@/src/utils/config"; +import sql from "@/src/utils/db"; import Context from "@/src/utils/koa"; import { jwtVerify } from "jose"; -import { roles } from "shared"; +import Router from "koa-router"; +import { hasAccess, roles } from "shared"; import { z } from "zod"; import { signJWT } from "./auth/utils"; -import config from "@/src/utils/config"; const users = new Router({ prefix: "/users", @@ -35,7 +35,8 @@ users.get("/me/org", async (ctx: Context) => { return; } - const users = await sql` + if (hasAccess(user.role, "teamMembers", "list")) { + org.users = await sql` select account.id, account.created_at, @@ -58,8 +59,7 @@ users.get("/me/org", async (ctx: Context) => { account.role, account.name `; - - org.users = users; + } org.license = ctx.state.license || {}; ctx.body = org; }); @@ -327,10 +327,6 @@ users.patch( ); } - // if (role === "owner") { - // ctx.throw(403, "You cannot modify the owner role"); - // } - for (const projectId of projects) { const [project] = await sql`select org_id from project where id = ${projectId}`; @@ -349,8 +345,16 @@ users.patch( ctx.throw(403, "You do not have permission to modify this user"); } + if (role === "billing" && currentUser.role !== "owner") { + ctx.throw( + 403, + "Only owners can add billing members to the organization.", + ); + } + const [userToModify] = await sql`select * from account where id = ${userId}`; + if (!userToModify || userToModify.orgId !== currentUser.orgId) { ctx.throw(404, "User not found in your organization"); } diff --git a/packages/frontend/pages/team.tsx b/packages/frontend/pages/team.tsx index 51abca20..1e94ebed 100644 --- a/packages/frontend/pages/team.tsx +++ b/packages/frontend/pages/team.tsx @@ -37,7 +37,12 @@ import { NextSeo } from "next-seo"; import { z } from "zod"; import { CopyInput } from "@/components/blocks/CopyText"; +import RenamableField from "@/components/blocks/RenamableField"; +import SearchBar from "@/components/blocks/SearchBar"; +import { SettingsCard } from "@/components/blocks/SettingsCard"; import UserAvatar from "@/components/blocks/UserAvatar"; +import { openUpgrade } from "@/components/layout/UpgradeModal"; +import config from "@/utils/config"; import { // useInvitations, useOrg, @@ -45,19 +50,14 @@ import { useProjects, useUser, } from "@/utils/dataHooks"; +import errorHandler from "@/utils/errors"; import { fetcher } from "@/utils/fetcher"; +import { SEAT_ALLOWANCE } from "@/utils/pricing"; +import { useForm } from "@mantine/form"; import { useDisclosure } from "@mantine/hooks"; import { notifications } from "@mantine/notifications"; import { hasAccess, roles } from "shared"; import classes from "./team.module.css"; -import { useForm } from "@mantine/form"; -import SearchBar from "@/components/blocks/SearchBar"; -import { SettingsCard } from "@/components/blocks/SettingsCard"; -import { SEAT_ALLOWANCE } from "@/utils/pricing"; -import { openUpgrade } from "@/components/layout/UpgradeModal"; -import config from "@/utils/config"; -import RenamableField from "@/components/blocks/RenamableField"; -import errorHandler from "@/utils/errors"; function SAMLConfig() { const { org, updateOrg, mutate } = useOrg(); @@ -364,7 +364,10 @@ export function RoleSelect({ {name} {minimal !== true && ( @@ -823,10 +826,10 @@ function MemberList({ users, isInvitation }) { function MemberListCard() { const { org } = useOrg(); - const invitedUsers = org?.users.filter( + const invitedUsers = org?.users?.filter( (user) => user.verified === false && user.role !== "owner", ); - const activatedUsers = org?.users.filter( + const activatedUsers = org?.users?.filter( (user) => user.verified === true || user.role === "owner", );