Skip to content

fix(organization): drop the redundant id input from org update/delete tools #6541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Tyagiquamar wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(organization): drop the redundant id input from org update/delete tools #6541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions apps/api/src/tools/organization/delete.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe("ORGANIZATION_DELETE", () => {
it("preserves existing metadata when archiving", async () => {
const ctx = makeCtx({ description: "an acme org" });

await ORGANIZATION_DELETE.handler({ id: "org-1" }, ctx);
await ORGANIZATION_DELETE.handler({}, ctx);

const call = ctx.update.mock.calls[0]?.[0] as {
data: { metadata: Record<string, unknown> };
Expand All @@ -44,22 +44,23 @@ describe("ORGANIZATION_DELETE", () => {
it("archives fine when there's no prior metadata", async () => {
const ctx = makeCtx(undefined);

await ORGANIZATION_DELETE.handler({ id: "org-1" }, ctx);
await ORGANIZATION_DELETE.handler({}, ctx);

const call = ctx.update.mock.calls[0]?.[0] as {
data: { metadata: Record<string, unknown> };
};
expect(call.data.metadata.archived).toBe(true);
});

it("rejects deleting an organization other than the authenticated one", async () => {
const ctx = makeCtx({ description: "an acme org" });
it("archives the organization resolved from the request context", async () => {
const ctx = makeCtx(undefined);

await ORGANIZATION_DELETE.handler({}, ctx);

await expect(
ORGANIZATION_DELETE.handler({ id: "org-2" }, ctx),
).rejects.toThrow(
"Organization ID does not match authenticated organization",
);
expect(ctx.update.mock.calls.length).toBe(0);
expect(ctx.get).toHaveBeenCalledWith("org-1");
expect(
(ctx.update.mock.calls[0]?.[0] as { organizationId: string })
.organizationId,
).toBe("org-1");
});
});
23 changes: 9 additions & 14 deletions apps/api/src/tools/organization/delete.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { z } from "zod";
import { defineTool } from "../../core/define-tool";
import { requireAuth } from "../../core/studio-context";
import { requireAuth, requireOrganization } from "../../core/studio-context";

export const ORGANIZATION_DELETE = defineTool({
name: "ORGANIZATION_DELETE",
Expand All @@ -19,31 +19,26 @@ export const ORGANIZATION_DELETE = defineTool({
idempotentHint: true,
openWorldHint: false,
},
inputSchema: z.object({
id: z.string(),
}),
inputSchema: z.object({}),

outputSchema: z.object({
success: z.boolean(),
id: z.string(),
}),

handler: async (input, ctx) => {
handler: async (_input, ctx) => {
requireAuth(ctx);
await ctx.access.check();

// Reject a target org the caller isn't authenticated against (see member-remove.ts).
if (input.id !== ctx.organization?.id) {
throw new Error(
"Organization ID does not match authenticated organization",
);
}
// The target org is the one resolved from the URL path; the input carries
// no id, so there is nothing to compare and nothing to spoof.
const org = requireOrganization(ctx);

// Merge into existing metadata — organization.update replaces it wholesale.
const existing = await ctx.boundAuth.organization.get(input.id);
const existing = await ctx.boundAuth.organization.get(org.id);

await ctx.boundAuth.organization.update({
organizationId: input.id,
organizationId: org.id,
data: {
metadata: {
...existing?.metadata,
Expand All @@ -55,7 +50,7 @@ export const ORGANIZATION_DELETE = defineTool({

return {
success: true,
id: input.id,
id: org.id,
};
},
});
21 changes: 9 additions & 12 deletions apps/api/src/tools/organization/update.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe("ORGANIZATION_UPDATE", () => {
it("persists an explicit empty description instead of dropping it", async () => {
const ctx = makeCtx();

await ORGANIZATION_UPDATE.handler({ id: "org-1", description: "" }, ctx);
await ORGANIZATION_UPDATE.handler({ description: "" }, ctx);

expect(ctx.update).toHaveBeenCalledWith({
organizationId: "org-1",
Expand All @@ -36,25 +36,22 @@ describe("ORGANIZATION_UPDATE", () => {
it("still writes a non-empty description", async () => {
const ctx = makeCtx();

await ORGANIZATION_UPDATE.handler(
{ id: "org-1", description: "hello" },
ctx,
);
await ORGANIZATION_UPDATE.handler({ description: "hello" }, ctx);

expect(ctx.update).toHaveBeenCalledWith({
organizationId: "org-1",
data: { metadata: { description: "hello" } },
});
});

it("rejects updating an organization other than the authenticated one", async () => {
it("always targets the organization resolved from the request context", async () => {
const ctx = makeCtx();

await expect(
ORGANIZATION_UPDATE.handler({ id: "org-2", name: "Evil" }, ctx),
).rejects.toThrow(
"Organization ID does not match authenticated organization",
);
expect(ctx.update.mock.calls.length).toBe(0);
await ORGANIZATION_UPDATE.handler({ name: "Renamed" }, ctx);

expect(ctx.update).toHaveBeenCalledWith({
organizationId: "org-1",
data: { name: "Renamed" },
});
});
});
14 changes: 5 additions & 9 deletions apps/api/src/tools/organization/update.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { z } from "zod";
import { defineTool } from "../../core/define-tool";
import { requireAuth } from "../../core/studio-context";
import { requireAuth, requireOrganization } from "../../core/studio-context";

export const ORGANIZATION_UPDATE = defineTool({
name: "ORGANIZATION_UPDATE",
Expand All @@ -19,7 +19,6 @@ export const ORGANIZATION_UPDATE = defineTool({
openWorldHint: false,
},
inputSchema: z.object({
id: z.string(),
name: z.string().min(1).max(255).optional(),
description: z.string().optional(),
}),
Expand All @@ -40,12 +39,9 @@ export const ORGANIZATION_UPDATE = defineTool({
// Check authorization
await ctx.access.check();

// Reject a target org the caller isn't authenticated against (see member-remove.ts).
if (input.id !== ctx.organization?.id) {
throw new Error(
"Organization ID does not match authenticated organization",
);
}
// The target org is the one resolved from the URL path; the input carries
// no id, so there is nothing to compare and nothing to spoof.
const org = requireOrganization(ctx);

// Build update data
// Slug is intentionally NOT updatable: it anchors org URLs (/api/:org/...)
Expand All @@ -58,7 +54,7 @@ export const ORGANIZATION_UPDATE = defineTool({

// Update organization via Better Auth
const result = await ctx.boundAuth.organization.update({
organizationId: input.id,
organizationId: org.id,
data: updateData,
});

Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/components/settings/delete-organization-section.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export function DeleteOrganizationSection() {

const deleteMutation = useMutation({
mutationFn: async () => {
await studio.call("ORGANIZATION_DELETE", { id: org.id });
await studio.call("ORGANIZATION_DELETE", {});
},
onSuccess: () => {
track("organization_deleted", { organization_id: org.id });
Expand Down
3 changes: 1 addition & 2 deletions packages/shared/src/tools/tool-io.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ export interface StudioToolIO {
};
ORGANIZATION_UPDATE: {
input: {
id: string;
name?: string | undefined;
description?: string | undefined;
};
Expand All @@ -60,7 +59,7 @@ export interface StudioToolIO {
};
};
ORGANIZATION_DELETE: {
input: { id: string };
input: { [x: string]: never };
output: { success: boolean; id: string };
};
ORGANIZATION_SETTINGS_GET: {
Expand Down