Limit pub visibility (#851)

* Allow adding pub and stage members in seed function Co-authored-by: Thomas F. K. Jorna <[email protected]> * Fix seed scripts * Filter pub results when userId is passed * Make tests pass * Add todo comment for making getPubs safer * Fetch and use user in api for filtering pubs * Pass user to get pubs query whenever possible * Restrict access to individual pub page * Skip pub authorization filtering on form fill page since it's authorized by form membership * Allow superadmins to see all pubs * Pub authorization tests (#857) * Initial tests * Clean up a little * Remove redundant stage capability check * Delete integrations from pub page and stop generating token on render * Pub membership test (#858) * Make sure related pubs show up regardless of authorization * Fix missing import --------- Co-authored-by: Thomas F. K. Jorna <[email protected]> Co-authored-by: Allison King <[email protected]>
pubpub · Jan 13, 2025 · 135c6d2 · 135c6d2
1 parent a93ead0
commit 135c6d2
Show file tree

Hide file tree

Showing 21 changed files with 579 additions and 85 deletions.
diff --git a/core/actions/_lib/runActionInstance.ts b/core/actions/_lib/runActionInstance.ts
@@ -38,8 +38,14 @@ const _runActionInstance = async (
 	args: RunActionInstanceArgs & { actionRunId: ActionRunsId },
 	trx = db
 ): Promise<ActionInstanceRunResult> => {
+	const isActionUserInitiated = "userId" in args;
+
 	const pubPromise = getPubsWithRelatedValuesAndChildren(
-		{ pubId: args.pubId, communityId: args.communityId },
+		{
+			pubId: args.pubId,
+			communityId: args.communityId,
+			userId: isActionUserInitiated ? args.userId : undefined,
+		},
 		{
 			withPubType: true,
 			withStage: true,

diff --git a/core/app/api/v0/c/[communitySlug]/site/[...ts-rest]/route.ts b/core/app/api/v0/c/[communitySlug]/site/[...ts-rest]/route.ts
@@ -1,5 +1,8 @@
+import type { User } from "lucia";
+
 import { headers } from "next/headers";
 import { createNextHandler } from "@ts-rest/serverless/next";
+import { jsonObjectFrom } from "kysely/helpers/postgres";
 import { z } from "zod";
 
 import type { Communities, CommunitiesId, PubsId, PubTypesId, StagesId, UsersId } from "db/public";
@@ -39,7 +42,7 @@ import { getCommunitySlug } from "~/lib/server/cache/getCommunitySlug";
 import { findCommunityBySlug } from "~/lib/server/community";
 import { getPubType, getPubTypesForCommunity } from "~/lib/server/pubtype";
 import { getStages } from "~/lib/server/stages";
-import { getSuggestedUsers } from "~/lib/server/user";
+import { getSuggestedUsers, SAFE_USER_SELECT } from "~/lib/server/user";
 
 const baseAuthorizationObject = Object.fromEntries(
 	Object.keys(ApiAccessScope).map(
@@ -77,13 +80,33 @@ const getAuthorization = async () => {
 
 	const validatedAccessToken = await validateApiAccessToken(apiKey, community.id);
 
-	const rules = (await db
+	const rules = await db
 		.selectFrom("api_access_permissions")
-		.selectAll()
+		.selectAll("api_access_permissions")
+		.innerJoin(
+			"api_access_tokens",
+			"api_access_tokens.id",
+			"api_access_permissions.apiAccessTokenId"
+		)
+		.select((eb) =>
+			jsonObjectFrom(
+				eb
+					.selectFrom("users")
+					.select(SAFE_USER_SELECT)
+					.whereRef("users.id", "=", eb.ref("api_access_tokens.issuedById"))
+			).as("user")
+		)
 		.where("api_access_permissions.apiAccessTokenId", "=", validatedAccessToken.id)
-		.execute()) as ApiAccessPermission[];
+		.$castTo<ApiAccessPermission & { user: User }>()
+		.execute();
+
+	const user = rules[0].user;
+	if (!rules[0].user) {
+		throw new NotFoundError(`Unable to locate user associated with api token`);
+	}
 
 	return {
+		user,
 		authorization: rules.reduce((acc, curr) => {
 			const { scope, constraints, accessType } = curr;
 			if (!constraints) {
@@ -103,6 +126,7 @@ type AuthorizationOutput<S extends ApiAccessScope, AT extends ApiAccessType> = {
 	authorization: true | Exclude<(typeof baseAuthorizationObject)[S][AT], false>;
 	community: Communities;
 	lastModifiedBy: LastModifiedBy;
+	user: User;
 };
 
 const checkAuthorization = async <
@@ -127,7 +151,7 @@ const checkAuthorization = async <
 	const authorizationTokenWithBearer = headers().get("Authorization");
 
 	if (authorizationTokenWithBearer) {
-		const { authorization, community, apiAccessTokenId } = await getAuthorization();
+		const { user, authorization, community, apiAccessTokenId } = await getAuthorization();
 
 		const constraints = authorization[token.scope][token.type];
 		if (!constraints) {
@@ -142,6 +166,7 @@ const checkAuthorization = async <
 			authorization: constraints as Exclude<typeof constraints, false>,
 			community,
 			lastModifiedBy,
+			user,
 		};
 	}
 
@@ -171,7 +196,7 @@ const checkAuthorization = async <
 
 	// Handle cases where we only want to check for login but have no specific capability yet
 	if (typeof cookies === "boolean") {
-		return { authorization: true as const, community, lastModifiedBy };
+		return { user, authorization: true as const, community, lastModifiedBy };
 	}
 
 	const can = await userCan(cookies.capability, cookies.target, user.id);
@@ -182,7 +207,7 @@ const checkAuthorization = async <
 		);
 	}
 
-	return { authorization: true as const, community, lastModifiedBy };
+	return { user, authorization: true as const, community, lastModifiedBy };
 };
 
 const shouldReturnRepresentation = () => {
@@ -199,7 +224,7 @@ const handler = createNextHandler(
 	{
 		pubs: {
 			get: async ({ params, query }) => {
-				const { community } = await checkAuthorization({
+				const { user, community } = await checkAuthorization({
 					token: { scope: ApiAccessScope.pub, type: ApiAccessType.read },
 					cookies: {
 						capability: Capabilities.viewPub,
@@ -211,6 +236,7 @@ const handler = createNextHandler(
 					{
 						pubId: params.pubId as PubsId,
 						communityId: community.id,
+						userId: user.id,
 					},
 					query
 				);
@@ -221,7 +247,7 @@ const handler = createNextHandler(
 				};
 			},
 			getMany: async ({ query }) => {
-				const { community } = await checkAuthorization({
+				const { user, community } = await checkAuthorization({
 					token: { scope: ApiAccessScope.pub, type: ApiAccessType.read },
 					// TODO: figure out capability here
 					cookies: false,
@@ -234,6 +260,7 @@ const handler = createNextHandler(
 						communityId: community.id,
 						pubTypeId,
 						stageId,
+						userId: user.id,
 					},
 					rest
 				);
@@ -280,7 +307,7 @@ const handler = createNextHandler(
 				};
 			},
 			update: async ({ params, body }) => {
-				const { community, lastModifiedBy } = await checkAuthorization({
+				const { user, community, lastModifiedBy } = await checkAuthorization({
 					token: { scope: ApiAccessScope.pub, type: ApiAccessType.write },
 					cookies: {
 						capability: Capabilities.updatePubValues,
@@ -316,6 +343,7 @@ const handler = createNextHandler(
 				const pub = await getPubsWithRelatedValuesAndChildren({
 					pubId: params.pubId as PubsId,
 					communityId: community.id,
+					userId: user.id,
 				});
 
 				return {
@@ -350,7 +378,7 @@ const handler = createNextHandler(
 			},
 			relations: {
 				remove: async ({ params, body }) => {
-					const { community, lastModifiedBy } = await checkAuthorization({
+					const { user, community, lastModifiedBy } = await checkAuthorization({
 						token: { scope: ApiAccessScope.pub, type: ApiAccessType.write },
 						cookies: {
 							capability: Capabilities.deletePub,
@@ -429,6 +457,7 @@ const handler = createNextHandler(
 					const pub = await getPubsWithRelatedValuesAndChildren({
 						pubId: params.pubId as PubsId,
 						communityId: community.id,
+						userId: user.id,
 					});
 
 					return {
@@ -437,7 +466,7 @@ const handler = createNextHandler(
 					};
 				},
 				update: async ({ params, body }) => {
-					const { community, lastModifiedBy } = await checkAuthorization({
+					const { user, community, lastModifiedBy } = await checkAuthorization({
 						token: { scope: ApiAccessScope.pub, type: ApiAccessType.write },
 						cookies: {
 							capability: Capabilities.deletePub,
@@ -476,6 +505,7 @@ const handler = createNextHandler(
 					const pub = await getPubsWithRelatedValuesAndChildren({
 						pubId: params.pubId as PubsId,
 						communityId: community.id,
+						userId: user.id,
 					});
 
 					return {
@@ -484,7 +514,7 @@ const handler = createNextHandler(
 					};
 				},
 				replace: async ({ params, body }) => {
-					const { community, lastModifiedBy } = await checkAuthorization({
+					const { user, community, lastModifiedBy } = await checkAuthorization({
 						token: { scope: ApiAccessScope.pub, type: ApiAccessType.write },
 						cookies: {
 							capability: Capabilities.deletePub,
@@ -522,6 +552,7 @@ const handler = createNextHandler(
 					const pub = await getPubsWithRelatedValuesAndChildren({
 						pubId: params.pubId as PubsId,
 						communityId: community.id,
+						userId: user.id,
 					});
 
 					return {

diff --git a/core/app/c/(public)/[communitySlug]/public/forms/[formSlug]/fill/page.tsx b/core/app/c/(public)/[communitySlug]/public/forms/[formSlug]/fill/page.tsx
@@ -137,6 +137,7 @@ export default async function FormPage({
 	if (!community) {
 		return notFound();
 	}
+	const { user, session } = await getLoginData();
 
 	const [form, pub, pubs, pubTypes] = await Promise.all([
 		getForm({
@@ -150,7 +151,7 @@ export default async function FormPage({
 				)
 			: undefined,
 		getPubsWithRelatedValuesAndChildren(
-			{ communityId: community.id },
+			{ communityId: community.id, userId: user?.id },
 			{
 				limit: 30,
 				withStage: true,
@@ -165,8 +166,6 @@ export default async function FormPage({
 		return <NotFound>No form found</NotFound>;
 	}
 
-	const { user, session } = await getLoginData();
-
 	if (!user && !session) {
 		const result = await handleFormToken({
 			params,
@@ -208,7 +207,7 @@ export default async function FormPage({
 
 	const parentPub = pub?.parentId
 		? await getPubsWithRelatedValuesAndChildren(
-				{ pubId: pub.parentId, communityId: community.id },
+				{ pubId: pub.parentId, communityId: community.id, userId: user?.id },
 				{ withStage: true, withLegacyAssignee: true, withPubType: true }
 			)
 		: undefined;

diff --git a/core/app/c/[communitySlug]/pubs/PubList.tsx b/core/app/c/[communitySlug]/pubs/PubList.tsx
@@ -1,6 +1,6 @@
 import { Suspense } from "react";
 
-import type { CommunitiesId } from "db/public";
+import type { CommunitiesId, UsersId } from "db/public";
 import { cn } from "utils";
 
 import { BasicPagination } from "~/app/components/Pagination";
@@ -20,13 +20,14 @@ type PaginatedPubListProps = {
 	 * @default `/c/${communitySlug}/pubs`
 	 */
 	basePath?: string;
+	userId: UsersId;
 };
 
 const PaginatedPubListInner = async (props: PaginatedPubListProps) => {
 	const [count, pubs] = await Promise.all([
 		getPubsCount({ communityId: props.communityId }),
 		getPubsWithRelatedValuesAndChildren(
-			{ communityId: props.communityId },
+			{ communityId: props.communityId, userId: props.userId },
 			{
 				limit: PAGE_SIZE,
 				offset: (props.page - 1) * PAGE_SIZE,
@@ -48,7 +49,14 @@ const PaginatedPubListInner = async (props: PaginatedPubListProps) => {
 	return (
 		<div className={cn("flex flex-col gap-8")}>
 			{pubs.map((pub) => {
-				return <PubRow key={pub.id} pub={pub} searchParams={props.searchParams} />;
+				return (
+					<PubRow
+						key={pub.id}
+						userId={props.userId}
+						pub={pub}
+						searchParams={props.searchParams}
+					/>
+				);
 			})}
 			<BasicPagination
 				basePath={basePath}

diff --git a/core/app/c/[communitySlug]/pubs/[pubId]/edit/page.tsx b/core/app/c/[communitySlug]/pubs/[pubId]/edit/page.tsx
@@ -4,7 +4,7 @@ import { cache } from "react";
 import Link from "next/link";
 import { notFound, redirect } from "next/navigation";
 
-import type { CommunitiesId, PubsId } from "db/public";
+import type { CommunitiesId, PubsId, UsersId } from "db/public";
 import { Capabilities } from "db/src/public/Capabilities";
 import { MembershipType } from "db/src/public/MembershipType";
 import { Button } from "ui/button";
@@ -19,11 +19,20 @@ import { getPubsWithRelatedValuesAndChildren } from "~/lib/server";
 import { findCommunityBySlug } from "~/lib/server/community";
 
 const getPubsWithRelatedValuesAndChildrenCached = cache(
-	async ({ pubId, communityId }: { pubId: PubsId; communityId: CommunitiesId }) => {
+	async ({
+		userId,
+		pubId,
+		communityId,
+	}: {
+		userId?: UsersId;
+		pubId: PubsId;
+		communityId: CommunitiesId;
+	}) => {
 		return getPubsWithRelatedValuesAndChildren(
 			{
 				pubId,
 				communityId,
+				userId,
 			},
 			{
 				withPubType: true,
@@ -97,6 +106,7 @@ export default async function Page({
 	const pub = await getPubsWithRelatedValuesAndChildrenCached({
 		pubId: params.pubId as PubsId,
 		communityId: community.id,
+		userId: user.id,
 	});
 
 	if (!pub) {

diff --git a/core/app/c/[communitySlug]/pubs/[pubId]/page.tsx b/core/app/c/[communitySlug]/pubs/[pubId]/page.tsx
@@ -2,7 +2,7 @@ import type { Metadata } from "next";
 
 import { Suspense } from "react";
 import Link from "next/link";
-import { notFound } from "next/navigation";
+import { notFound, redirect } from "next/navigation";
 
 import type { PubsId } from "db/public";
 import { Capabilities } from "db/src/public/Capabilities";
@@ -86,6 +86,16 @@ export default async function Page({
 		notFound();
 	}
 
+	const canView = await userCan(
+		Capabilities.viewPub,
+		{ type: MembershipType.pub, pubId },
+		user.id
+	);
+
+	if (!canView) {
+		redirect(`/c/${params.communitySlug}/unauthorized`);
+	}
+
 	const canAddMember = await userCan(
 		Capabilities.addPubMember,
 		{
@@ -106,6 +116,8 @@ export default async function Page({
 	const communityMembersPromise = selectCommunityMembers({ communityId: community.id }).execute();
 	const communityStagesPromise = getStages({ communityId: community.id }).execute();
 
+	// We don't pass the userId here because we want to include related pubs regardless of authorization
+	// This is safe because we've already explicitly checked authorization for the root pub
 	const pub = await getPubsWithRelatedValuesAndChildren(
 		{ pubId: params.pubId, communityId: community.id },
 		{
@@ -165,7 +177,6 @@ export default async function Page({
 							</div>
 						</div>
 					) : null}
-
 					<div>
 						<div className="mb-1 text-lg font-bold">Actions</div>
 						{actions && actions.length > 0 && stage ? (

diff --git a/core/app/c/[communitySlug]/pubs/page.tsx b/core/app/c/[communitySlug]/pubs/page.tsx
@@ -37,6 +37,7 @@ export default async function Page({ params, searchParams }: Props) {
 				searchParams={searchParams}
 				page={page}
 				basePath={basePath}
+				userId={user.id}
 			/>
 		</>
 	);

diff --git a/core/app/c/[communitySlug]/stages/[stageId]/page.tsx b/core/app/c/[communitySlug]/stages/[stageId]/page.tsx
@@ -93,6 +93,7 @@ export default async function Page({
 				fallback={<PubListSkeleton amount={stage.pubsCount ?? 2} className="gap-16" />}
 			>
 				<StagePubs
+					userId={user.id}
 					stage={stage}
 					pageContext={{
 						params,