Skip to content

Commit 670e75e

Browse files
richiemcilroyrichiemcilroy
authored
Merge pull request #2892 from yashranaway/fix/web-api-auth-sweep
fix(web): validate mobile reply parents
2 parents d5e397a + 570d9b4 commit 670e75e

2 files changed

Lines changed: 238 additions & 0 deletions

File tree

‎apps/web/__tests__/unit/mobile-comment-parent.test.ts‎

Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
import * as Db from "@cap/database/schema";
2+
import {
3+
CurrentUser,
4+
DatabaseError,
5+
HttpAuthMiddleware,
6+
Organisation,
7+
User,
8+
Video,
9+
} from "@cap/web-domain";
10+
import { type HttpApi, HttpApiBuilder, HttpServer } from "@effect/platform";
11+
import type { SQL } from "drizzle-orm";
12+
import { MySqlDialect, type MySqlTable } from "drizzle-orm/mysql-core";
13+
import { Effect, Layer, Option } from "effect";
14+
import { afterAll, beforeEach, describe, expect, it, vi } from "vitest";
15+
import {
16+
Database,
17+
type DbClient,
18+
} from "../../../../packages/web-backend/src/Database";
19+
20+
const mocks = vi.hoisted(() => ({ notify: vi.fn(async () => undefined) }));
21+
vi.mock("server-only", () => ({}));
22+
vi.mock("@cap/database/auth/auth-options", () => ({ authOptions: () => ({}) }));
23+
vi.mock("@cap/database/emails/config", () => ({ sendEmail: vi.fn() }));
24+
vi.mock("@/actions/loom", () => ({ downloadLoomVideo: vi.fn() }));
25+
vi.mock("@/app/(org)/dashboard/analytics/data", () => ({
26+
getOrgAnalyticsData: vi.fn(),
27+
}));
28+
vi.mock("@/lib/account-deletion-request", () => ({}));
29+
vi.mock("@/lib/desktop-segments-finalization", () => ({}));
30+
vi.mock("@/lib/Notification", () => ({ createNotification: mocks.notify }));
31+
vi.mock("@/lib/sync-video-storage-names", () => ({}));
32+
vi.mock("@/lib/video-processing", () => ({}));
33+
vi.mock("@/workflows/import-loom-video", () => ({}));
34+
vi.mock("workflow/api", () => ({ start: vi.fn() }));
35+
vi.mock("@cap/web-backend", async () => {
36+
const { Context } = await import("effect");
37+
const { Database } = await import(
38+
"../../../../packages/web-backend/src/Database"
39+
);
40+
return {
41+
Database,
42+
Videos: Context.GenericTag("mobile-comment-test/Videos"),
43+
ImageUploads: Context.GenericTag("mobile-comment-test/ImageUploads"),
44+
};
45+
});
46+
47+
const user = {
48+
id: User.UserId.make("viewer"),
49+
email: "viewer@example.com",
50+
activeOrganizationId: Organisation.OrganisationId.make("organization"),
51+
iconUrlOrKey: Option.none(),
52+
};
53+
const videoId = Video.VideoId.make("accessible-video");
54+
type Row = Record<string, unknown>;
55+
let parents: Row[];
56+
let inserted: Row[];
57+
const dialect = new MySqlDialect();
58+
59+
function matchesParent(condition: SQL, row: Row) {
60+
const { sql, params } = dialect.sqlToQuery(condition);
61+
let index = 0;
62+
return sql
63+
.replaceAll(/[()]/g, "")
64+
.split(" and ")
65+
.every((clause) => {
66+
const match = /^`comments`\.`([^`]+)` = \?$/.exec(clause);
67+
if (!match?.[1]) throw new Error(`Unsupported predicate: ${clause}`);
68+
return row[match[1]] === params[index++];
69+
});
70+
}
71+
72+
const database = {
73+
select: (columns?: Record<string, unknown>) => ({
74+
from: (table: MySqlTable) => {
75+
let condition: SQL;
76+
const result = () => {
77+
if (table === Db.users) return [{ preferences: {} }];
78+
if (table === Db.organizations)
79+
return [{ id: user.activeOrganizationId }];
80+
if (table === Db.videos) return [{ ownerId: user.id }];
81+
if (table !== Db.comments) throw new Error("Unexpected table");
82+
if (columns && Object.keys(columns).length === 1) {
83+
return parents.filter((row) => matchesParent(condition, row));
84+
}
85+
return inserted.map((row) => ({
86+
...row,
87+
authorName: "Viewer",
88+
authorImage: null,
89+
authorPreferences: {},
90+
}));
91+
};
92+
const query = {
93+
leftJoin: () => query,
94+
innerJoin: () => query,
95+
where: (value: SQL) => {
96+
condition = value;
97+
return query;
98+
},
99+
limit: async () => result(),
100+
orderBy: async () => result(),
101+
};
102+
return query;
103+
},
104+
}),
105+
insert: (table: MySqlTable) => ({
106+
values: async (row: Row) => {
107+
if (table !== Db.comments) throw new Error("Unexpected insert");
108+
inserted.push(row);
109+
},
110+
}),
111+
};
112+
113+
let dispose: () => Promise<void>;
114+
vi.mock("@/lib/server", async () => {
115+
const { Videos, ImageUploads } = await import("@cap/web-backend");
116+
return {
117+
apiToHandler: (
118+
api: Layer.Layer<HttpApi.Api, never, Database | HttpAuthMiddleware>,
119+
) => {
120+
const web = HttpApiBuilder.toWebHandler(
121+
api.pipe(
122+
Layer.provide(
123+
Layer.succeed(
124+
Database,
125+
Database.make({
126+
use: (callback) =>
127+
Effect.tryPromise({
128+
try: () => callback(database as unknown as DbClient),
129+
catch: (cause) => new DatabaseError({ cause }),
130+
}),
131+
}),
132+
),
133+
),
134+
Layer.provide(Layer.mock(Videos, { _tag: "Videos" })),
135+
Layer.provide(Layer.mock(ImageUploads, { _tag: "ImageUploads" })),
136+
Layer.provide(
137+
Layer.succeed(
138+
HttpAuthMiddleware,
139+
Effect.succeed(CurrentUser.of(user)),
140+
),
141+
),
142+
Layer.merge(HttpServer.layerContext),
143+
),
144+
);
145+
dispose = web.dispose;
146+
return web.handler;
147+
},
148+
};
149+
});
150+
151+
const { POST } = await import("@/app/api/mobile/[...route]/route");
152+
153+
function postComment(parentCommentId: string | null = "parent") {
154+
return POST(
155+
new Request(`http://localhost/api/mobile/caps/${videoId}/comments`, {
156+
method: "POST",
157+
headers: { "Content-Type": "application/json" },
158+
body: JSON.stringify({
159+
content: " A reply ",
160+
timestamp: null,
161+
parentCommentId,
162+
}),
163+
}),
164+
);
165+
}
166+
167+
beforeEach(() => {
168+
parents = [{ id: "parent", videoId, type: "text" }];
169+
inserted = [];
170+
mocks.notify.mockClear();
171+
});
172+
173+
afterAll(async () => {
174+
await dispose();
175+
});
176+
177+
describe("mobile reply parent authorization", () => {
178+
it.each([
179+
{
180+
name: "belongs to another video",
181+
candidates: [{ id: "parent", videoId: "private-video", type: "text" }],
182+
},
183+
{
184+
name: "is not a text comment",
185+
candidates: [{ id: "parent", videoId, type: "emoji" }],
186+
},
187+
{ name: "does not exist", candidates: [] },
188+
])("rejects a parent that $name", async ({ candidates }) => {
189+
parents = candidates;
190+
const response = await postComment();
191+
expect(response.status).toBe(404);
192+
expect(inserted).toEqual([]);
193+
expect(mocks.notify).not.toHaveBeenCalled();
194+
});
195+
196+
it("preserves replies to text comments on the accessible video", async () => {
197+
const response = await postComment();
198+
expect(response.status).toBe(200);
199+
expect(await response.json()).toMatchObject({
200+
content: "A reply",
201+
parentCommentId: "parent",
202+
videoId,
203+
});
204+
expect(inserted).toHaveLength(1);
205+
expect(mocks.notify).toHaveBeenCalledWith(
206+
expect.objectContaining({
207+
type: "reply",
208+
parentCommentId: "parent",
209+
videoId,
210+
}),
211+
);
212+
});
213+
214+
it("preserves top-level comments", async () => {
215+
const response = await postComment(null);
216+
expect(response.status).toBe(200);
217+
expect(inserted).toHaveLength(1);
218+
expect(mocks.notify).toHaveBeenCalledWith(
219+
expect.objectContaining({ type: "comment" }),
220+
);
221+
});
222+
});

‎apps/web/app/api/mobile/[...route]/route.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2111,6 +2111,22 @@ const createMobileComment = Effect.fn("Mobile.createComment")(function* ({
21112111
const id = Comment.CommentId.make(nanoId());
21122112
const now = new Date();
21132113
const database = yield* Database;
2114+
if (parentCommentId) {
2115+
const [parent] = yield* database.use((db) =>
2116+
db
2117+
.select({ id: Db.comments.id })
2118+
.from(Db.comments)
2119+
.where(
2120+
and(
2121+
eq(Db.comments.id, parentCommentId),
2122+
eq(Db.comments.videoId, videoId),
2123+
eq(Db.comments.type, "text"),
2124+
),
2125+
)
2126+
.limit(1),
2127+
);
2128+
if (!parent) return yield* Effect.fail(new HttpApiError.NotFound());
2129+
}
21142130
yield* database.use((db) =>
21152131
db.insert(Db.comments).values({
21162132
id,

0 commit comments

Comments
 (0)